diff --git a/src/Execution/Arguments/NestedBelongsTo.php b/src/Execution/Arguments/NestedBelongsTo.php index 7a446f084..f0c857f5c 100644 --- a/src/Execution/Arguments/NestedBelongsTo.php +++ b/src/Execution/Arguments/NestedBelongsTo.php @@ -41,7 +41,7 @@ public function __invoke($model, $args): void } if ($args->has('upsert')) { - $upsertModel = new ResolveNested(new UpsertModel(new SaveModel())); + $upsertModel = new ResolveNested(new UpsertModel(new SaveModel(), $this->relation)); $related = $upsertModel( $this->relation->make(), $args->arguments['upsert']->value, diff --git a/src/Execution/Arguments/NestedManyToMany.php b/src/Execution/Arguments/NestedManyToMany.php index a2ac78e90..f1f83aecc 100644 --- a/src/Execution/Arguments/NestedManyToMany.php +++ b/src/Execution/Arguments/NestedManyToMany.php @@ -52,7 +52,7 @@ public function __invoke($model, $args): void } if ($args->has('upsert')) { - $upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation))); + $upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation), $relation)); foreach ($args->arguments['upsert']->value as $childArgs) { // @phpstan-ignore-next-line Relation&Builder mixin not recognized diff --git a/src/Execution/Arguments/NestedOneToMany.php b/src/Execution/Arguments/NestedOneToMany.php index 8b2c216aa..a5f29a6dd 100644 --- a/src/Execution/Arguments/NestedOneToMany.php +++ b/src/Execution/Arguments/NestedOneToMany.php @@ -40,7 +40,7 @@ public function __invoke($model, $args): void } if ($args->has('upsert')) { - $upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation))); + $upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation), $relation)); foreach ($args->arguments['upsert']->value as $childArgs) { // @phpstan-ignore-next-line Relation&Builder mixin not recognized diff --git a/src/Execution/Arguments/NestedOneToOne.php b/src/Execution/Arguments/NestedOneToOne.php index e08d551c5..18ac991f3 100644 --- a/src/Execution/Arguments/NestedOneToOne.php +++ b/src/Execution/Arguments/NestedOneToOne.php @@ -42,7 +42,7 @@ public function __invoke($model, $args): void } if ($args->has('upsert')) { - $upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation))); + $upsertModel = new ResolveNested(new UpsertModel(new SaveModel($relation), $relation)); $upsertModel($relation->first() ?? $relation->make(), $args->arguments['upsert']->value); } diff --git a/src/Execution/Arguments/UpsertModel.php b/src/Execution/Arguments/UpsertModel.php index bc1984888..46f44de06 100644 --- a/src/Execution/Arguments/UpsertModel.php +++ b/src/Execution/Arguments/UpsertModel.php @@ -2,17 +2,26 @@ namespace Nuwave\Lighthouse\Execution\Arguments; +use GraphQL\Error\Error; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Relations\Relation; use Nuwave\Lighthouse\Support\Contracts\ArgResolver; class UpsertModel implements ArgResolver { + public const CANNOT_UPSERT_UNRELATED_MODEL = 'Cannot upsert a model that is not related to the given parent.'; + /** @var callable|\Nuwave\Lighthouse\Support\Contracts\ArgResolver */ protected $previous; - /** @param callable|\Nuwave\Lighthouse\Support\Contracts\ArgResolver $previous */ - public function __construct(callable $previous) - { + /** + * @param callable|\Nuwave\Lighthouse\Support\Contracts\ArgResolver $previous + * @param \Illuminate\Database\Eloquent\Relations\Relation<\Illuminate\Database\Eloquent\Model>|null $parentRelation + */ + public function __construct( + callable $previous, + protected ?Relation $parentRelation = null, + ) { $this->previous = $previous; } @@ -26,8 +35,15 @@ public function __invoke($model, $args): mixed $id = $this->retrieveID($model, $args); if ($id) { - $existingModel = $model->newQuery() + $existingModel = $this->queryBuilder($model) ->find($id); + if ( + $existingModel === null + && $this->parentRelation !== null + && $model->newQuery()->find($id) !== null + ) { + throw new Error(self::CANNOT_UPSERT_UNRELATED_MODEL); + } if ($existingModel !== null) { $model = $existingModel; @@ -56,4 +72,11 @@ protected function retrieveID(Model $model, ArgumentSet $args) return null; } + + /** @return \Illuminate\Database\Eloquent\Builder<\Illuminate\Database\Eloquent\Model> */ + protected function queryBuilder(Model $model) + { + return $this->parentRelation?->getQuery()->clone() + ?? $model->newQuery(); + } } diff --git a/src/Schema/Directives/UpsertDirective.php b/src/Schema/Directives/UpsertDirective.php index 38c16cb68..48e4f0578 100644 --- a/src/Schema/Directives/UpsertDirective.php +++ b/src/Schema/Directives/UpsertDirective.php @@ -33,6 +33,6 @@ public static function definition(): string protected function makeExecutionFunction(?Relation $parentRelation = null): callable { - return new UpsertModel(new SaveModel($parentRelation)); + return new UpsertModel(new SaveModel($parentRelation), $parentRelation); } } diff --git a/src/Schema/Directives/UpsertManyDirective.php b/src/Schema/Directives/UpsertManyDirective.php index 7cd48f9fc..321581253 100644 --- a/src/Schema/Directives/UpsertManyDirective.php +++ b/src/Schema/Directives/UpsertManyDirective.php @@ -33,6 +33,6 @@ public static function definition(): string protected function makeExecutionFunction(?Relation $parentRelation = null): callable { - return new UpsertModel(new SaveModel($parentRelation)); + return new UpsertModel(new SaveModel($parentRelation), $parentRelation); } } diff --git a/tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php b/tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php index e164f7b50..fd45a78fb 100644 --- a/tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php +++ b/tests/Integration/Execution/MutationExecutor/BelongsToManyTest.php @@ -3,6 +3,7 @@ namespace Tests\Integration\Execution\MutationExecutor; use Faker\Provider\Lorem; +use Nuwave\Lighthouse\Execution\Arguments\UpsertModel; use PHPUnit\Framework\Attributes\DataProvider; use Tests\DBTestCase; use Tests\Utils\Models\Role; @@ -314,6 +315,37 @@ public function testUpsertBelongsToManyWithoutId(): void $this->assertSame('is_user', $role->name); } + public function testNestedUpsertByIDDoesNotModifyUnrelatedBelongsToManyModel(): void + { + $roleA = factory(Role::class)->create(); + $roleB = factory(Role::class)->create(); + $userA = factory(User::class)->create(); + + $roleA->users()->attach($userA); + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation ($roleID: ID!, $userID: ID!) { + upsertRole(input: { + id: $roleID + name: "role-b" + users: { + upsert: [{ id: $userID, name: "hacked" }] + } + }) { + id + } + } + GRAPHQL, [ + 'roleID' => $roleB->id, + 'userID' => $userA->id, + ])->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL); + + $userA->refresh(); + $this->assertSame($roleA->id, $userA->roles()->firstOrFail()->id); + $this->assertNotSame('hacked', $userA->name); + $this->assertCount(0, $roleB->users()->get()); + } + public function testCreateAndConnectWithBelongsToMany(): void { $user = factory(User::class)->make(); @@ -577,24 +609,42 @@ public function testUpdateWithBelongsToMany(string $action): void $this->assertInstanceOf(Role::class, $role); $role->name = 'is_admin'; $role->save(); + $roleID = $role->id; + assert(is_int($roleID)); + $users = factory(User::class, 2)->create(); $role->users() ->attach( - factory(User::class, 2)->create(), + $users, ); - $this->graphQL(/** @lang GraphQL */ <<id; + $secondUserID = $users[1]->id; + assert(is_int($firstUserID)); + assert(is_int($secondUserID)); + + if ($action === 'upsert') { + $unrelatedRole = factory(Role::class)->make(); + $this->assertInstanceOf(Role::class, $unrelatedRole); + $unrelatedRole->name = 'unrelated'; + $unrelatedRole->save(); + + $roleID = $unrelatedRole->id; + assert(is_int($roleID)); + } + + $response = $this->graphQL(/** @lang GraphQL */ <<assertJson([ + GRAPHQL); + + if ($action === 'upsert') { + $response->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL); + + $role->refresh(); + $this->assertCount(2, $role->users()->get()); + $this->assertSame('is_admin', $role->name); + + return; + } + + $response->assertJson([ 'data' => [ "{$action}Role" => [ - 'id' => '1', + 'id' => (string) $roleID, 'name' => 'is_user', 'users' => [ [ - 'id' => '1', + 'id' => (string) $firstUserID, 'name' => 'user1', ], [ - 'id' => '2', + 'id' => (string) $secondUserID, 'name' => 'user2', ], ], diff --git a/tests/Integration/Execution/MutationExecutor/BelongsToTest.php b/tests/Integration/Execution/MutationExecutor/BelongsToTest.php index 660a9f457..55df160e8 100644 --- a/tests/Integration/Execution/MutationExecutor/BelongsToTest.php +++ b/tests/Integration/Execution/MutationExecutor/BelongsToTest.php @@ -4,6 +4,7 @@ use Illuminate\Database\Events\QueryExecuted; use Illuminate\Support\Facades\DB; +use Nuwave\Lighthouse\Execution\Arguments\UpsertModel; use PHPUnit\Framework\Attributes\DataProvider; use Tests\DBTestCase; use Tests\Utils\Models\Role; @@ -278,6 +279,39 @@ public function testUpsertWithNewBelongsTo(): void ]); } + public function testNestedUpsertByIDDoesNotModifyUnrelatedBelongsToModel(): void + { + $userA = factory(User::class)->create(); + $userB = factory(User::class)->create(); + $task = factory(Task::class)->create(); + $task->user()->associate($userB); + $task->save(); + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation ($taskID: ID!, $userID: ID!) { + upsertTask(input: { + id: $taskID + name: "task" + user: { + upsert: { id: $userID, name: "hacked" } + } + }) { + id + } + } + GRAPHQL, + [ + 'taskID' => $task->id, + 'userID' => $userA->id, + ], + )->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL); + + $userA->refresh(); + $task->refresh(); + $this->assertNotSame('hacked', $userA->name); + $this->assertSame($userB->id, $task->user_id); + } + public function testUpsertBelongsToWithoutID(): void { $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' @@ -607,10 +641,37 @@ public function testSavesOnlyOnceWithMultipleBelongsTo(): void public function testUpsertUsingCreateAndUpdateUsingUpsertBelongsTo(): void { - $user = factory(User::class)->make(); - $this->assertInstanceOf(User::class, $user); - $user->name = 'foo'; - $user->save(); + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation { + upsertTask(input: { + id: 1 + name: "foo" + user: { + upsert: { + name: "foo-user" + } + } + }) { + id + name + user { + id + name + } + } + } + GRAPHQL)->assertJson([ + 'data' => [ + 'upsertTask' => [ + 'id' => '1', + 'name' => 'foo', + 'user' => [ + 'id' => '1', + 'name' => 'foo-user', + ], + ], + ], + ]); $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' mutation { diff --git a/tests/Integration/Execution/MutationExecutor/HasManyTest.php b/tests/Integration/Execution/MutationExecutor/HasManyTest.php index bf564e1de..c3453722d 100644 --- a/tests/Integration/Execution/MutationExecutor/HasManyTest.php +++ b/tests/Integration/Execution/MutationExecutor/HasManyTest.php @@ -2,6 +2,7 @@ namespace Tests\Integration\Execution\MutationExecutor; +use Nuwave\Lighthouse\Execution\Arguments\UpsertModel; use PHPUnit\Framework\Attributes\DataProvider; use Tests\DBTestCase; use Tests\Utils\Models\CustomPrimaryKey; @@ -315,6 +316,38 @@ public function testCreateUsingUpsertWithNewHasMany(): void ]); } + public function testNestedUpsertByIDDoesNotModifyUnrelatedHasManyModel(): void + { + $userA = factory(User::class)->create(); + $userB = factory(User::class)->create(); + + $taskA = factory(Task::class)->make(); + $taskA->name = 'from-user-a'; + $taskA->user()->associate($userA); + $taskA->save(); + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation ($userID: ID!, $taskID: ID!) { + upsertUser(input: { + id: $userID + name: "user-b" + tasks: { + upsert: [{ id: $taskID, name: "hacked" }] + } + }) { + id + } + } + GRAPHQL, [ + 'userID' => $userB->id, + 'taskID' => $taskA->id, + ])->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL); + + $taskA->refresh(); + $this->assertSame('from-user-a', $taskA->name); + $this->assertSame($userA->id, $taskA->user_id); + } + /** @return iterable */ public static function existingModelMutations(): iterable { diff --git a/tests/Integration/Execution/MutationExecutor/HasOneTest.php b/tests/Integration/Execution/MutationExecutor/HasOneTest.php index b6856e22c..638ea93e7 100644 --- a/tests/Integration/Execution/MutationExecutor/HasOneTest.php +++ b/tests/Integration/Execution/MutationExecutor/HasOneTest.php @@ -2,6 +2,7 @@ namespace Tests\Integration\Execution\MutationExecutor; +use Nuwave\Lighthouse\Execution\Arguments\UpsertModel; use PHPUnit\Framework\Attributes\DataProvider; use Tests\DBTestCase; use Tests\Utils\Models\Post; @@ -184,6 +185,38 @@ public function testCreateUsingUpsertWithNewHasOne(): void ]); } + public function testNestedUpsertByIDDoesNotModifyUnrelatedHasOneModel(): void + { + $taskA = factory(Task::class)->create(); + $taskB = factory(Task::class)->create(); + + $postA = factory(Post::class)->make(); + $postA->title = 'from-task-a'; + $postA->task()->associate($taskA); + $postA->save(); + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation ($taskID: ID!, $postID: ID!) { + upsertTask(input: { + id: $taskID + name: "task-b" + post: { + upsert: { id: $postID, title: "hacked" } + } + }) { + id + } + } + GRAPHQL, [ + 'taskID' => $taskB->id, + 'postID' => $postA->id, + ])->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL); + + $postA->refresh(); + $this->assertSame('from-task-a', $postA->title); + $this->assertSame($taskA->id, $postA->task_id); + } + public function testUpsertHasOneWithoutID(): void { $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' diff --git a/tests/Integration/Execution/MutationExecutor/MorphManyTest.php b/tests/Integration/Execution/MutationExecutor/MorphManyTest.php index 497cd79de..f71292cc8 100644 --- a/tests/Integration/Execution/MutationExecutor/MorphManyTest.php +++ b/tests/Integration/Execution/MutationExecutor/MorphManyTest.php @@ -2,6 +2,7 @@ namespace Tests\Integration\Execution\MutationExecutor; +use Nuwave\Lighthouse\Execution\Arguments\UpsertModel; use PHPUnit\Framework\Attributes\DataProvider; use Tests\DBTestCase; use Tests\Utils\Models\Image; @@ -226,6 +227,38 @@ public function testUpsertMorphManyWithoutId(): void ]); } + public function testNestedUpsertByIDDoesNotModifyUnrelatedMorphManyModel(): void + { + $taskA = factory(Task::class)->create(); + $taskB = factory(Task::class)->create(); + + $imageA = factory(Image::class)->make(); + $imageA->url = 'from-task-a'; + $imageA->imageable()->associate($taskA); + $imageA->save(); + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation ($taskID: ID!, $imageID: ID!) { + upsertTask(input: { + id: $taskID + name: "task-b" + images: { + upsert: [{ id: $imageID, url: "hacked" }] + } + }) { + id + } + } + GRAPHQL, [ + 'taskID' => $taskB->id, + 'imageID' => $imageA->id, + ])->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL); + + $imageA->refresh(); + $this->assertSame('from-task-a', $imageA->url); + $this->assertSame($taskA->id, $imageA->imageable_id); + } + public function testAllowsNullOperations(): void { factory(Task::class)->create(); diff --git a/tests/Integration/Execution/MutationExecutor/MorphOneTest.php b/tests/Integration/Execution/MutationExecutor/MorphOneTest.php index 915e287a3..89d8e7ad1 100644 --- a/tests/Integration/Execution/MutationExecutor/MorphOneTest.php +++ b/tests/Integration/Execution/MutationExecutor/MorphOneTest.php @@ -2,6 +2,7 @@ namespace Tests\Integration\Execution\MutationExecutor; +use Nuwave\Lighthouse\Execution\Arguments\UpsertModel; use PHPUnit\Framework\Attributes\DataProvider; use Tests\DBTestCase; use Tests\Utils\Models\Image; @@ -177,6 +178,38 @@ public function testUpsertMorphOneWithoutId(): void ]); } + public function testNestedUpsertByIDDoesNotModifyUnrelatedMorphOneModel(): void + { + $taskA = factory(Task::class)->create(); + $taskB = factory(Task::class)->create(); + + $imageA = factory(Image::class)->make(); + $imageA->url = 'from-task-a'; + $imageA->imageable()->associate($taskA); + $imageA->save(); + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation ($taskID: ID!, $imageID: ID!) { + upsertTask(input: { + id: $taskID + name: "task-b" + image: { + upsert: { id: $imageID, url: "hacked" } + } + }) { + id + } + } + GRAPHQL, [ + 'taskID' => $taskB->id, + 'imageID' => $imageA->id, + ])->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL); + + $imageA->refresh(); + $this->assertSame('from-task-a', $imageA->url); + $this->assertSame($taskA->id, $imageA->imageable_id); + } + public function testAllowsNullOperations(): void { factory(Task::class)->create(); @@ -379,8 +412,10 @@ public function testNestedConnectMorphOne(): void $task = factory(Task::class)->create(); $this->assertInstanceOf(Task::class, $task); - $image = factory(Image::class)->create(); + $image = factory(Image::class)->make(); $this->assertInstanceOf(Image::class, $image); + $image->url = 'original'; + $image->save(); $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' mutation ($input: UpdateTaskInput!) { @@ -403,16 +438,10 @@ public function testNestedConnectMorphOne(): void ], ], ], - ])->assertJson([ - 'data' => [ - 'updateTask' => [ - 'id' => '1', - 'name' => 'foo', - 'image' => [ - 'url' => 'foo', - ], - ], - ], - ]); + ])->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL); + + $image->refresh(); + $this->assertSame('original', $image->url); + $this->assertNull($image->imageable_id); } } diff --git a/tests/Integration/Execution/MutationExecutor/MorphToManyTest.php b/tests/Integration/Execution/MutationExecutor/MorphToManyTest.php index 2114e23f3..8e8714946 100644 --- a/tests/Integration/Execution/MutationExecutor/MorphToManyTest.php +++ b/tests/Integration/Execution/MutationExecutor/MorphToManyTest.php @@ -2,8 +2,10 @@ namespace Tests\Integration\Execution\MutationExecutor; +use Nuwave\Lighthouse\Execution\Arguments\UpsertModel; use Tests\DBTestCase; use Tests\Utils\Models\Tag; +use Tests\Utils\Models\Task; final class MorphToManyTest extends DBTestCase { @@ -222,6 +224,37 @@ public function testUpsertATaskWithExistingTagsByUsingSync(): void ]); } + public function testNestedUpsertByIDDoesNotModifyUnrelatedMorphToManyModel(): void + { + $taskA = factory(Task::class)->create(); + $taskB = factory(Task::class)->create(); + $tagA = factory(Tag::class)->create(); + + $taskA->tags()->attach($tagA); + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation ($taskID: ID!, $tagID: ID!) { + upsertTask(input: { + id: $taskID + name: "task-b" + tags: { + upsert: [{ id: $tagID, name: "hacked" }] + } + }) { + id + } + } + GRAPHQL, [ + 'taskID' => $taskB->id, + 'tagID' => $tagA->id, + ])->assertGraphQLErrorMessage(UpsertModel::CANNOT_UPSERT_UNRELATED_MODEL); + + $tagA->refresh(); + $this->assertNotSame('hacked', $tagA->name); + $this->assertCount(1, $taskA->tags()->whereKey($tagA->id)->get()); + $this->assertCount(0, $taskB->tags()->whereKey($tagA->id)->get()); + } + public function testCreateANewTagRelationByUsingCreate(): void { $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL'