diff --git a/src/Execution/BatchLoader/RelationBatchLoader.php b/src/Execution/BatchLoader/RelationBatchLoader.php index 71b36697d7..e89ee5256b 100644 --- a/src/Execution/BatchLoader/RelationBatchLoader.php +++ b/src/Execution/BatchLoader/RelationBatchLoader.php @@ -11,9 +11,12 @@ class RelationBatchLoader { /** - * Map from unique model keys to model instances. + * Map from object ids to model instances. * - * @var array + * The same row may be hydrated into multiple instances, e.g. through duplicate pivot rows. + * Each of them gets the relation, so they are not keyed by model key. + * + * @var array */ protected array $parents = []; @@ -40,7 +43,7 @@ public function __construct( public function load(Model $model): Deferred { $modelKey = ModelKey::build($model); - $this->parents[$modelKey] = $model; + $this->parents[spl_object_id($model)] = $model; return new Deferred(function () use ($modelKey) { if (! $this->hasResolved) { diff --git a/src/Execution/ModelsLoader/PaginatedModelsLoader.php b/src/Execution/ModelsLoader/PaginatedModelsLoader.php index 46b04c44b1..236ba96174 100644 --- a/src/Execution/ModelsLoader/PaginatedModelsLoader.php +++ b/src/Execution/ModelsLoader/PaginatedModelsLoader.php @@ -9,6 +9,7 @@ use Illuminate\Database\Eloquent\Relations\HasManyThrough; use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Pagination\LengthAwarePaginator; +use Nuwave\Lighthouse\Execution\Utils\ModelKey; use Nuwave\Lighthouse\Pagination\PaginationArgs; use Nuwave\Lighthouse\Pagination\ZeroPerPageLengthAwarePaginator; use Nuwave\Lighthouse\Support\Utils; @@ -47,6 +48,8 @@ public function extract(Model $model): mixed protected function loadRelatedModels(EloquentCollection $parents): EloquentCollection { $relations = $parents->toBase() + // One query per row, not per instance + ->unique(static fn (Model $model): string => ModelKey::build($model)) ->map(function (Model $model) use ($parents): Relation { $relation = $this->relationInstance($parents); @@ -59,8 +62,6 @@ protected function loadRelatedModels(EloquentCollection $parents): EloquentColle $relation->addSelect($select); } - $relation->initRelation([$model], $this->relation); - return $relation->forPage($this->paginationArgs->page, $this->paginationArgs->first); }); @@ -163,12 +164,13 @@ protected function loadDefaultWith(EloquentCollection $models): void */ protected function associateRelationModels(EloquentCollection $parents, EloquentCollection $relatedModels): void { - $this->relationInstance($parents) - ->match( - $parents->all(), - $relatedModels, - $this->relation, - ); + $relation = $this->relationInstance($parents); + $relation->initRelation($parents->all(), $this->relation); + $relation->match( + $parents->all(), + $relatedModels, + $this->relation, + ); } /** @param \Illuminate\Database\Eloquent\Collection $parents */ diff --git a/tests/Integration/Schema/Directives/HasManyDirectiveTest.php b/tests/Integration/Schema/Directives/HasManyDirectiveTest.php index 8a7e601944..18da0bde0c 100644 --- a/tests/Integration/Schema/Directives/HasManyDirectiveTest.php +++ b/tests/Integration/Schema/Directives/HasManyDirectiveTest.php @@ -2,7 +2,9 @@ namespace Tests\Integration\Schema\Directives; +use Illuminate\Database\Events\QueryExecuted; use Illuminate\Support\Arr; +use Illuminate\Support\Facades\DB; use Nuwave\Lighthouse\Exceptions\DefinitionException; use Nuwave\Lighthouse\Pagination\PaginationArgs; use PHPUnit\Framework\Attributes\DataProvider; @@ -357,6 +359,108 @@ public function testQueryPaginatedHasManyFirst0(): void ]); } + public function testQueryPaginatedHasManyOnEveryInstanceOfTheSameModel(): void + { + $this->schema = /** @lang GraphQL */ <<<'GRAPHQL' + type Role { + users: [User!]! @belongsToMany + } + + type User { + id: ID! + tasks: [Task!]! @hasMany(type: PAGINATOR) + } + + type Task { + id: ID! + } + + type Query { + role: Role @first + } + GRAPHQL; + + $role = factory(Role::class)->create(); + $this->assertInstanceOf(Role::class, $role); + + $userWithTask = factory(User::class)->create(); + $this->assertInstanceOf(User::class, $userWithTask); + + $task = factory(Task::class)->make(); + $this->assertInstanceOf(Task::class, $task); + $userWithTask->tasks()->save($task); + + $userWithoutTasks = factory(User::class)->create(); + $this->assertInstanceOf(User::class, $userWithoutTasks); + + foreach ([$userWithTask, $userWithoutTasks] as $user) { + $role->users()->attach($user, ['meta' => 'first']); + $role->users()->attach($user, ['meta' => 'second']); + } + + $withTask = [ + 'id' => (string) $userWithTask->id, + 'tasks' => [ + 'paginatorInfo' => [ + 'total' => 1, + ], + 'data' => [ + [ + 'id' => (string) $task->id, + ], + ], + ], + ]; + $withoutTasks = [ + 'id' => (string) $userWithoutTasks->id, + 'tasks' => [ + 'paginatorInfo' => [ + 'total' => 0, + ], + 'data' => [], + ], + ]; + + $queries = []; + DB::listen(static function (QueryExecuted $query) use (&$queries): void { + $queries[] = $query->sql; + }); + + $this + ->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + { + role { + users { + id + tasks(first: 10) { + paginatorInfo { + total + } + data { + id + } + } + } + } + } + GRAPHQL) + ->assertExactJson([ + 'data' => [ + 'role' => [ + 'users' => [ + $withTask, + $withTask, + $withoutTasks, + $withoutTasks, + ], + ], + ], + ]); + + // One subquery per distinct user, not one per instance + $this->assertSame(1, substr_count(implode("\n", $queries), 'union all')); + } + public function testQueryPaginatedHasManyWithNonUniqueForeignKey(): void { $this->schema = /** @lang GraphQL */ <<<'GRAPHQL' diff --git a/tests/Integration/Schema/Directives/WithCountDirectiveTest.php b/tests/Integration/Schema/Directives/WithCountDirectiveTest.php index 2a861ad740..a7e7815607 100644 --- a/tests/Integration/Schema/Directives/WithCountDirectiveTest.php +++ b/tests/Integration/Schema/Directives/WithCountDirectiveTest.php @@ -4,6 +4,7 @@ use Nuwave\Lighthouse\Exceptions\DefinitionException; use Tests\DBTestCase; +use Tests\Utils\Models\Role; use Tests\Utils\Models\Task; use Tests\Utils\Models\User; @@ -57,6 +58,57 @@ public function testEagerLoadsRelationCount(): void }); } + public function testEagerLoadsRelationCountOntoEveryInstanceOfTheSameModel(): void + { + $this->schema = /** @lang GraphQL */ <<<'GRAPHQL' + type Query { + role: Role @first + } + + type Role { + users: [User!]! @belongsToMany + } + + type User { + tasksCountLoaded: Boolean! + @withCount(relation: "tasks") + @method + } + GRAPHQL; + + $role = factory(Role::class)->create(); + $this->assertInstanceOf(Role::class, $role); + + $user = factory(User::class)->create(); + $this->assertInstanceOf(User::class, $user); + + $role->users()->attach($user, ['meta' => 'first']); + $role->users()->attach($user, ['meta' => 'second']); + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + { + role { + users { + tasksCountLoaded + } + } + } + GRAPHQL)->assertExactJson([ + 'data' => [ + 'role' => [ + 'users' => [ + [ + 'tasksCountLoaded' => true, + ], + [ + 'tasksCountLoaded' => true, + ], + ], + ], + ], + ]); + } + public function testFailsToEagerLoadRelationCountWithoutRelation(): void { $this->schema = /** @lang GraphQL */ <<<'GRAPHQL' diff --git a/tests/Integration/Schema/Directives/WithDirectiveTest.php b/tests/Integration/Schema/Directives/WithDirectiveTest.php index 30ae1c2bf4..cf77904668 100644 --- a/tests/Integration/Schema/Directives/WithDirectiveTest.php +++ b/tests/Integration/Schema/Directives/WithDirectiveTest.php @@ -9,6 +9,7 @@ use Tests\Utils\Models\Comment; use Tests\Utils\Models\Image; use Tests\Utils\Models\Post; +use Tests\Utils\Models\Role; use Tests\Utils\Models\Task; use Tests\Utils\Models\User; @@ -57,6 +58,57 @@ public function testEagerLoadsRelation(): void ]); } + public function testEagerLoadsRelationOntoEveryInstanceOfTheSameModel(): void + { + $this->schema = /** @lang GraphQL */ <<<'GRAPHQL' + type Query { + role: Role @first + } + + type Role { + users: [User!]! @belongsToMany + } + + type User { + tasksLoaded: Boolean! + @with(relation: "tasks") + @method + } + GRAPHQL; + + $role = factory(Role::class)->create(); + $this->assertInstanceOf(Role::class, $role); + + $user = factory(User::class)->create(); + $this->assertInstanceOf(User::class, $user); + + $role->users()->attach($user, ['meta' => 'first']); + $role->users()->attach($user, ['meta' => 'second']); + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + { + role { + users { + tasksLoaded + } + } + } + GRAPHQL)->assertExactJson([ + 'data' => [ + 'role' => [ + 'users' => [ + [ + 'tasksLoaded' => true, + ], + [ + 'tasksLoaded' => true, + ], + ], + ], + ], + ]); + } + public function testOuterResultHandlerReceivesValueAfterEagerLoading(): void { $this->schema = /** @lang GraphQL */ <<<'GRAPHQL'