Skip to content

Commit 44a7582

Browse files
authored
[9.x] findOrFail with array of ids throws ModelNotFound with all ids instead of only the missing ones (#40845)
* fix: use only the missing Ids when creating ModelNotFoundException When findOrFail is used with an array of IDs, if only one of the Models is missing, we throw ModelNotFoundException, but when we list the models that failed, we include the ones that were found too. This commits changes the exception that is thrown to only include the IDs that were missing. * Update findOrFail tests to report only the missing Ids
1 parent b9203fc commit 44a7582

File tree

3 files changed

+35
-14
lines changed

3 files changed

+35
-14
lines changed

src/Illuminate/Database/Eloquent/Builder.php

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -435,16 +435,22 @@ public function findOrFail($id, $columns = ['*'])
435435
$id = $id instanceof Arrayable ? $id->toArray() : $id;
436436

437437
if (is_array($id)) {
438-
if (count($result) === count(array_unique($id))) {
439-
return $result;
438+
if (count($result) !== count(array_unique($id))) {
439+
throw (new ModelNotFoundException)->setModel(
440+
get_class($this->model), array_diff($id, $result->modelKeys())
441+
);
440442
}
441-
} elseif (! is_null($result)) {
443+
442444
return $result;
443445
}
444446

445-
throw (new ModelNotFoundException)->setModel(
446-
get_class($this->model), $id
447-
);
447+
if (is_null($result)) {
448+
throw (new ModelNotFoundException)->setModel(
449+
get_class($this->model), $id
450+
);
451+
}
452+
453+
return $result;
448454
}
449455

450456
/**

tests/Database/DatabaseEloquentBuilderTest.php

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,21 +130,27 @@ public function testFindOrFailMethodWithManyThrowsModelNotFoundException()
130130
{
131131
$this->expectException(ModelNotFoundException::class);
132132

133+
$model = $this->getMockModel();
134+
$model->shouldReceive('getKey')->andReturn(1);
135+
133136
$builder = m::mock(Builder::class.'[get]', [$this->getMockQueryBuilder()]);
134-
$builder->setModel($this->getMockModel());
137+
$builder->setModel($model);
135138
$builder->getQuery()->shouldReceive('whereIn')->once()->with('foo_table.foo', [1, 2]);
136-
$builder->shouldReceive('get')->with(['column'])->andReturn(new Collection([1]));
139+
$builder->shouldReceive('get')->with(['column'])->andReturn(new Collection([$model]));
137140
$builder->findOrFail([1, 2], ['column']);
138141
}
139142

140143
public function testFindOrFailMethodWithManyUsingCollectionThrowsModelNotFoundException()
141144
{
142145
$this->expectException(ModelNotFoundException::class);
143146

147+
$model = $this->getMockModel();
148+
$model->shouldReceive('getKey')->andReturn(1);
149+
144150
$builder = m::mock(Builder::class.'[get]', [$this->getMockQueryBuilder()]);
145-
$builder->setModel($this->getMockModel());
151+
$builder->setModel($model);
146152
$builder->getQuery()->shouldReceive('whereIn')->once()->with('foo_table.foo', [1, 2]);
147-
$builder->shouldReceive('get')->with(['column'])->andReturn(new Collection([1]));
153+
$builder->shouldReceive('get')->with(['column'])->andReturn(new Collection([$model]));
148154
$builder->findOrFail(new Collection([1, 2]), ['column']);
149155
}
150156

tests/Database/DatabaseEloquentIntegrationTest.php

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -616,26 +616,35 @@ public function testFindOrFailWithSingleIdThrowsModelNotFoundException()
616616
{
617617
$this->expectException(ModelNotFoundException::class);
618618
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Database\EloquentTestUser] 1');
619+
$this->expectExceptionObject(
620+
(new ModelNotFoundException())->setModel(EloquentTestUser::class, [1]),
621+
);
619622

620623
EloquentTestUser::findOrFail(1);
621624
}
622625

623626
public function testFindOrFailWithMultipleIdsThrowsModelNotFoundException()
624627
{
625628
$this->expectException(ModelNotFoundException::class);
626-
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Database\EloquentTestUser] 1, 2');
629+
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Database\EloquentTestUser] 2, 3');
630+
$this->expectExceptionObject(
631+
(new ModelNotFoundException())->setModel(EloquentTestUser::class, [2, 3]),
632+
);
627633

628634
EloquentTestUser::create(['id' => 1, 'email' => '[email protected]']);
629-
EloquentTestUser::findOrFail([1, 2]);
635+
EloquentTestUser::findOrFail([1, 2, 3]);
630636
}
631637

632638
public function testFindOrFailWithMultipleIdsUsingCollectionThrowsModelNotFoundException()
633639
{
634640
$this->expectException(ModelNotFoundException::class);
635-
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Database\EloquentTestUser] 1, 2');
641+
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Database\EloquentTestUser] 2, 3');
642+
$this->expectExceptionObject(
643+
(new ModelNotFoundException())->setModel(EloquentTestUser::class, [2, 3]),
644+
);
636645

637646
EloquentTestUser::create(['id' => 1, 'email' => '[email protected]']);
638-
EloquentTestUser::findOrFail(new Collection([1, 2]));
647+
EloquentTestUser::findOrFail(new Collection([1, 1, 2, 3]));
639648
}
640649

641650
public function testOneToOneRelationship()

0 commit comments

Comments
 (0)