Skip to content

Commit 15185a9

Browse files
sebdesignsebdesign
committed
Ensure find* methods on relationships are accepting Arrayable ids
The regression in laravel#7048 and laravel#19019 is also observed in laravel#9143, because the `find`, `findMany` and `findOrFail` methods are copied from the Eloquent Builder to the `BelongsToMany` and `HasManyThrough` relations. For this reason, we need to convert the passed ids to an array before executing the queries.
1 parent 4c6280c commit 15185a9

File tree

4 files changed

+129
-5
lines changed

4 files changed

+129
-5
lines changed

‎src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Illuminate\Database\Eloquent\Relations;
44

5+
use Illuminate\Contracts\Support\Arrayable;
56
use Illuminate\Database\Eloquent\Builder;
67
use Illuminate\Database\Eloquent\Collection;
78
use Illuminate\Database\Eloquent\Model;
@@ -504,21 +505,31 @@ public function updateOrCreate(array $attributes, array $values = [], array $joi
504505
*/
505506
public function find($id, $columns = ['*'])
506507
{
507-
return is_array($id) ? $this->findMany($id, $columns) : $this->where(
508+
if (is_array($id) || $id instanceof Arrayable) {
509+
return $this->findMany($id, $columns);
510+
}
511+
512+
return $this->where(
508513
$this->getRelated()->getQualifiedKeyName(), '=', $id
509514
)->first($columns);
510515
}
511516

512517
/**
513518
* Find multiple related models by their primary keys.
514519
*
515-
* @param mixed $ids
520+
* @param \Illuminate\Contracts\Support\Arrayable|array $ids
516521
* @param array $columns
517522
* @return \Illuminate\Database\Eloquent\Collection
518523
*/
519524
public function findMany($ids, $columns = ['*'])
520525
{
521-
return empty($ids) ? $this->getRelated()->newCollection() : $this->whereIn(
526+
$ids = $ids instanceof Arrayable ? $ids->toArray() : $ids;
527+
528+
if (empty($ids)) {
529+
return $this->getRelated()->newCollection();
530+
}
531+
532+
return $this->whereIn(
522533
$this->getRelated()->getQualifiedKeyName(), $ids
523534
)->get($columns);
524535
}
@@ -536,6 +547,8 @@ public function findOrFail($id, $columns = ['*'])
536547
{
537548
$result = $this->find($id, $columns);
538549

550+
$id = $id instanceof Arrayable ? $id->toArray() : $id;
551+
539552
if (is_array($id)) {
540553
if (count($result) === count(array_unique($id))) {
541554
return $result;

‎src/Illuminate/Database/Eloquent/Relations/HasManyThrough.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Illuminate\Database\Eloquent\Relations;
44

5+
use Illuminate\Contracts\Support\Arrayable;
56
use Illuminate\Database\Eloquent\Builder;
67
use Illuminate\Database\Eloquent\Collection;
78
use Illuminate\Database\Eloquent\Model;
@@ -285,7 +286,7 @@ public function firstOrFail($columns = ['*'])
285286
*/
286287
public function find($id, $columns = ['*'])
287288
{
288-
if (is_array($id)) {
289+
if (is_array($id) || $id instanceof Arrayable) {
289290
return $this->findMany($id, $columns);
290291
}
291292

@@ -297,12 +298,14 @@ public function find($id, $columns = ['*'])
297298
/**
298299
* Find multiple related models by their primary keys.
299300
*
300-
* @param mixed $ids
301+
* @param \Illuminate\Contracts\Support\Arrayable|array $ids
301302
* @param array $columns
302303
* @return \Illuminate\Database\Eloquent\Collection
303304
*/
304305
public function findMany($ids, $columns = ['*'])
305306
{
307+
$ids = $ids instanceof Arrayable ? $ids->toArray() : $ids;
308+
306309
if (empty($ids)) {
307310
return $this->getRelated()->newCollection();
308311
}
@@ -325,6 +328,8 @@ public function findOrFail($id, $columns = ['*'])
325328
{
326329
$result = $this->find($id, $columns);
327330

331+
$id = $id instanceof Arrayable ? $id->toArray() : $id;
332+
328333
if (is_array($id)) {
329334
if (count($result) === count(array_unique($id))) {
330335
return $result;

‎tests/Database/DatabaseEloquentHasManyThroughIntegrationTest.php‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Illuminate\Database\Eloquent\Model as Eloquent;
77
use Illuminate\Database\Eloquent\ModelNotFoundException;
88
use Illuminate\Database\Eloquent\SoftDeletes;
9+
use Illuminate\Support\Collection;
910
use Illuminate\Support\LazyCollection;
1011
use PHPUnit\Framework\TestCase;
1112

@@ -120,6 +121,55 @@ public function testWhereHasOnARelationWithCustomIntermediateAndLocalKey()
120121
$this->assertCount(1, $country);
121122
}
122123

124+
public function testFind()
125+
{
126+
$this->expectException(ModelNotFoundException::class);
127+
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Database\HasManyThroughTestPost] 1, 2');
128+
129+
HasManyThroughTestCountry::create(['id' => 1, 'name' => 'United States of America', 'shortname' => 'us'])
130+
->users()->create(['id' => 1, 'email' => '[email protected]', 'country_short' => 'us'])
131+
->posts()->create(['id' => 1, 'title' => 'A title', 'body' => 'A body', 'email' => '[email protected]']);
132+
133+
$post = HasManyThroughTestCountry::first()->posts()->find(1);
134+
135+
$this->assertNotNull($post);
136+
$this->assertSame('A title', $post->title);
137+
}
138+
139+
public function testFindMethod()
140+
{
141+
HasManyThroughTestCountry::create(['id' => 1, 'name' => 'United States of America', 'shortname' => 'us'])
142+
->users()->create(['id' => 1, 'email' => '[email protected]', 'country_short' => 'us'])
143+
->posts()->createMany([
144+
['id' => 1, 'title' => 'A title', 'body' => 'A body', 'email' => '[email protected]'],
145+
['id' => 2, 'title' => 'Another title', 'body' => 'Another body', 'email' => '[email protected]'],
146+
]);
147+
148+
$country = HasManyThroughTestCountry::first();
149+
$post = $country->posts()->find(1);
150+
151+
$this->assertNotNull($post);
152+
$this->assertSame('A title', $post->title);
153+
154+
$this->assertCount(2, $country->posts()->find([1, 2]));
155+
$this->assertCount(2, $country->posts()->find(new Collection([1, 2])));
156+
}
157+
158+
public function testFindManyMethod()
159+
{
160+
HasManyThroughTestCountry::create(['id' => 1, 'name' => 'United States of America', 'shortname' => 'us'])
161+
->users()->create(['id' => 1, 'email' => '[email protected]', 'country_short' => 'us'])
162+
->posts()->createMany([
163+
['id' => 1, 'title' => 'A title', 'body' => 'A body', 'email' => '[email protected]'],
164+
['id' => 2, 'title' => 'Another title', 'body' => 'Another body', 'email' => '[email protected]'],
165+
]);
166+
167+
$country = HasManyThroughTestCountry::first();
168+
169+
$this->assertCount(2, $country->posts()->findMany([1, 2]));
170+
$this->assertCount(2, $country->posts()->findMany(new Collection([1, 2])));
171+
}
172+
123173
public function testFirstOrFailThrowsAnException()
124174
{
125175
$this->expectException(ModelNotFoundException::class);
@@ -142,6 +192,30 @@ public function testFindOrFailThrowsAnException()
142192
HasManyThroughTestCountry::first()->posts()->findOrFail(1);
143193
}
144194

195+
public function testFindOrFailWithManyThrowsAnException()
196+
{
197+
$this->expectException(ModelNotFoundException::class);
198+
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Database\HasManyThroughTestPost] 1, 2');
199+
200+
HasManyThroughTestCountry::create(['id' => 1, 'name' => 'United States of America', 'shortname' => 'us'])
201+
->users()->create(['id' => 1, 'email' => '[email protected]', 'country_short' => 'us'])
202+
->posts()->create(['id' => 1, 'title' => 'A title', 'body' => 'A body', 'email' => '[email protected]']);
203+
204+
HasManyThroughTestCountry::first()->posts()->findOrFail([1, 2]);
205+
}
206+
207+
public function testFindOrFailWithManyUsingCollectionThrowsAnException()
208+
{
209+
$this->expectException(ModelNotFoundException::class);
210+
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Database\HasManyThroughTestPost] 1, 2');
211+
212+
HasManyThroughTestCountry::create(['id' => 1, 'name' => 'United States of America', 'shortname' => 'us'])
213+
->users()->create(['id' => 1, 'email' => '[email protected]', 'country_short' => 'us'])
214+
->posts()->create(['id' => 1, 'title' => 'A title', 'body' => 'A body', 'email' => '[email protected]']);
215+
216+
HasManyThroughTestCountry::first()->posts()->findOrFail(new Collection([1, 2]));
217+
}
218+
145219
public function testFirstRetrievesFirstRecord()
146220
{
147221
$this->seedData();

‎tests/Integration/Database/EloquentBelongsToManyTest.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,12 +347,16 @@ public function testFindMethod()
347347
$post->tags()->attach(Tag::all());
348348

349349
$this->assertEquals($tag2->name, $post->tags()->find($tag2->id)->name);
350+
$this->assertCount(0, $post->tags()->findMany([]));
350351
$this->assertCount(2, $post->tags()->findMany([$tag->id, $tag2->id]));
352+
$this->assertCount(0, $post->tags()->findMany(new Collection()));
353+
$this->assertCount(2, $post->tags()->findMany(new Collection([$tag->id, $tag2->id])));
351354
}
352355

353356
public function testFindOrFailMethod()
354357
{
355358
$this->expectException(ModelNotFoundException::class);
359+
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Integration\Database\EloquentBelongsToManyTest\Tag] 10');
356360

357361
$post = Post::create(['title' => Str::random()]);
358362

@@ -363,6 +367,34 @@ public function testFindOrFailMethod()
363367
$post->tags()->findOrFail(10);
364368
}
365369

370+
public function testFindOrFailMethodWithMany()
371+
{
372+
$this->expectException(ModelNotFoundException::class);
373+
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Integration\Database\EloquentBelongsToManyTest\Tag] 10, 11');
374+
375+
$post = Post::create(['title' => Str::random()]);
376+
377+
Tag::create(['name' => Str::random()]);
378+
379+
$post->tags()->attach(Tag::all());
380+
381+
$post->tags()->findOrFail([10, 11]);
382+
}
383+
384+
public function testFindOrFailMethodWithManyUsingCollection()
385+
{
386+
$this->expectException(ModelNotFoundException::class);
387+
$this->expectExceptionMessage('No query results for model [Illuminate\Tests\Integration\Database\EloquentBelongsToManyTest\Tag] 10, 11');
388+
389+
$post = Post::create(['title' => Str::random()]);
390+
391+
Tag::create(['name' => Str::random()]);
392+
393+
$post->tags()->attach(Tag::all());
394+
395+
$post->tags()->findOrFail(new Collection([10, 11]));
396+
}
397+
366398
public function testFindOrNewMethod()
367399
{
368400
$post = Post::create(['title' => Str::random()]);

0 commit comments

Comments
 (0)