From f7a82a9dc2b9a76e7f90e2b6d36ceca607a9132d Mon Sep 17 00:00:00 2001 From: GavG Date: Thu, 27 Jul 2023 17:42:46 +0100 Subject: [PATCH 1/3] WIP: adding identifyingColumns directive to upserts --- src/Execution/Arguments/UpsertModel.php | 35 ++++- src/Schema/Directives/UpsertDirective.php | 11 +- .../Schema/Directives/UpsertDirectiveTest.php | 144 ++++++++++++++++++ 3 files changed, 183 insertions(+), 7 deletions(-) diff --git a/src/Execution/Arguments/UpsertModel.php b/src/Execution/Arguments/UpsertModel.php index a54db03477..fc39ed46e7 100644 --- a/src/Execution/Arguments/UpsertModel.php +++ b/src/Execution/Arguments/UpsertModel.php @@ -9,10 +9,14 @@ class UpsertModel implements ArgResolver /** @var callable|\Nuwave\Lighthouse\Support\Contracts\ArgResolver */ protected $previous; + /** @var array */ + protected array $identifyingColumns; + /** @param callable|\Nuwave\Lighthouse\Support\Contracts\ArgResolver $previous */ - public function __construct(callable $previous) + public function __construct(callable $previous, ?array $identifyingColumns) { $this->previous = $previous; + $this->identifyingColumns = $identifyingColumns ?? []; } /** @@ -22,20 +26,39 @@ public function __construct(callable $previous) public function __invoke($model, $args): mixed { // TODO consider Laravel native ->upsert(), available from 8.10 - $id = $args->arguments['id'] - ?? $args->arguments[$model->getKeyName()] - ?? null; + $existingModel = null; - if ($id !== null) { + if (!empty($this->identifyingColumns)) { $existingModel = $model ->newQuery() - ->find($id->value); + ->firstWhere( + array_intersect_key( + $args->toArray(), + array_flip($this->identifyingColumns) + ) + ); if ($existingModel !== null) { $model = $existingModel; } } + if ($existingModel === null) { + $id = $args->arguments['id'] + ?? $args->arguments[$model->getKeyName()] + ?? null; + + if ($id !== null) { + $existingModel = $model + ->newQuery() + ->find($id->value); + + if ($existingModel !== null) { + $model = $existingModel; + } + } + } + return ($this->previous)($model, $args); } } diff --git a/src/Schema/Directives/UpsertDirective.php b/src/Schema/Directives/UpsertDirective.php index 6bd698b6c3..954983e429 100644 --- a/src/Schema/Directives/UpsertDirective.php +++ b/src/Schema/Directives/UpsertDirective.php @@ -21,6 +21,12 @@ public static function definition(): string """ model: String + """ + Specify the columns by which to upsert the model. + This is optional, defaults to the ID or model Key. + """ + identifyingColumns: [String!] = [] + """ Specify the name of the relation on the parent model. This is only needed when using this directive as a nested arg @@ -33,6 +39,9 @@ public static function definition(): string protected function makeExecutionFunction(Relation $parentRelation = null): callable { - return new UpsertModel(new SaveModel($parentRelation)); + return new UpsertModel( + new SaveModel($parentRelation), + $this->directiveArgValue('identifyingColumns') + ); } } diff --git a/tests/Integration/Schema/Directives/UpsertDirectiveTest.php b/tests/Integration/Schema/Directives/UpsertDirectiveTest.php index b04bbe6a08..249ad2e81c 100644 --- a/tests/Integration/Schema/Directives/UpsertDirectiveTest.php +++ b/tests/Integration/Schema/Directives/UpsertDirectiveTest.php @@ -6,6 +6,7 @@ use Illuminate\Container\Container; use Nuwave\Lighthouse\Schema\TypeRegistry; use Tests\DBTestCase; +use Tests\Utils\Models\Company; use Tests\Utils\Models\Task; use Tests\Utils\Models\User; @@ -198,6 +199,149 @@ interface IUser ]); } + public function testDirectUpsertByIdentifyingColumn(): void + { + $this->schema .= /** @lang GraphQL */ ' + type User { + id: ID! + email: String! + name: String! + } + + type Mutation { + upsertUser(name: String!, email: String!): User @upsert(identifyingColumns: ["email"]) + } + '; + + $this->graphQL(/** @lang GraphQL */ ' + mutation { + upsertUser( + email: "foo@te.st" + name: "bar" + ) { + name + email + } + } + ')->assertJson([ + 'data' => [ + 'upsertUser' => [ + 'email' => "foo@te.st", + 'name' => "bar" + ], + ], + ]); + + $user = User::firstOrFail(); + + $this->assertSame('bar', $user->name); + $this->assertSame('foo@te.st', $user->email); + + $this->graphQL(/** @lang GraphQL */ ' + mutation { + upsertUser( + email: "foo@te.st" + name: "foo" + ) { + name + email + } + } + ')->assertJson([ + 'data' => [ + 'upsertUser' => [ + 'email' => "foo@te.st", + 'name' => "foo" + ], + ], + ]); + + $user->refresh(); + + $this->assertSame('foo', $user->name); + $this->assertSame('foo@te.st', $user->email); + } + + public function testDirectUpsertByIdentifyingColumns(): void + { + $company = factory(Company::class)->create(['id' => 1]); + + $this->schema .= + /** @lang GraphQL */ + ' + type User { + id: ID! + email: String! + name: String! + company_id: ID! + } + + type Mutation { + upsertUser(name: String!, email: String!, company_id:ID!): User @upsert(identifyingColumns: ["name", "company_id"]) + } + '; + + $this->graphQL( + /** @lang GraphQL */ + ' + mutation { + upsertUser( + email: "foo@te.st" + name: "bar" + company_id: 1 + ) { + name + email + company_id + } + } + ')->assertJson([ + 'data' => [ + 'upsertUser' => [ + 'email' => "foo@te.st", + 'name' => "bar", + 'company_id' => 1 + ], + ], + ]); + + $user = User::firstOrFail(); + + $this->assertSame('bar', $user->name); + $this->assertSame('foo@te.st', $user->email); + $this->assertSame(1, $user->company_id); + + $this->graphQL( + /** @lang GraphQL */ + ' + mutation { + upsertUser( + email: "bar@te.st" + name: "bar" + company_id: 1 + ) { + name + email + company_id + } + } + ' + )->assertJson([ + 'data' => [ + 'upsertUser' => [ + 'email' => "bar@te.st", + 'name' => "bar", + 'company_id' => $company->id + ], + ], + ]); + + $user->refresh(); + + $this->assertSame('bar', $user->name); + $this->assertSame('bar@te.st', $user->email); + } + public static function resolveType(): Type { $typeRegistry = Container::getInstance()->make(TypeRegistry::class); From 0e63ba6858943227e3b84b2565d76d0669055240 Mon Sep 17 00:00:00 2001 From: GavG Date: Thu, 27 Jul 2023 16:43:45 +0000 Subject: [PATCH 2/3] Apply php-cs-fixer changes --- src/Execution/Arguments/UpsertModel.php | 8 ++--- src/Schema/Directives/UpsertDirective.php | 2 +- .../Schema/Directives/UpsertDirectiveTest.php | 29 ++++++++++--------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/Execution/Arguments/UpsertModel.php b/src/Execution/Arguments/UpsertModel.php index fc39ed46e7..e726d5bc60 100644 --- a/src/Execution/Arguments/UpsertModel.php +++ b/src/Execution/Arguments/UpsertModel.php @@ -28,14 +28,14 @@ public function __invoke($model, $args): mixed // TODO consider Laravel native ->upsert(), available from 8.10 $existingModel = null; - if (!empty($this->identifyingColumns)) { + if (! empty($this->identifyingColumns)) { $existingModel = $model ->newQuery() ->firstWhere( - array_intersect_key( + array_intersect_key( $args->toArray(), - array_flip($this->identifyingColumns) - ) + array_flip($this->identifyingColumns), + ), ); if ($existingModel !== null) { diff --git a/src/Schema/Directives/UpsertDirective.php b/src/Schema/Directives/UpsertDirective.php index 954983e429..063b25ceec 100644 --- a/src/Schema/Directives/UpsertDirective.php +++ b/src/Schema/Directives/UpsertDirective.php @@ -41,7 +41,7 @@ protected function makeExecutionFunction(Relation $parentRelation = null): calla { return new UpsertModel( new SaveModel($parentRelation), - $this->directiveArgValue('identifyingColumns') + $this->directiveArgValue('identifyingColumns'), ); } } diff --git a/tests/Integration/Schema/Directives/UpsertDirectiveTest.php b/tests/Integration/Schema/Directives/UpsertDirectiveTest.php index 249ad2e81c..848899baff 100644 --- a/tests/Integration/Schema/Directives/UpsertDirectiveTest.php +++ b/tests/Integration/Schema/Directives/UpsertDirectiveTest.php @@ -226,8 +226,8 @@ public function testDirectUpsertByIdentifyingColumn(): void ')->assertJson([ 'data' => [ 'upsertUser' => [ - 'email' => "foo@te.st", - 'name' => "bar" + 'email' => 'foo@te.st', + 'name' => 'bar', ], ], ]); @@ -250,8 +250,8 @@ public function testDirectUpsertByIdentifyingColumn(): void ')->assertJson([ 'data' => [ 'upsertUser' => [ - 'email' => "foo@te.st", - 'name' => "foo" + 'email' => 'foo@te.st', + 'name' => 'foo', ], ], ]); @@ -266,9 +266,9 @@ public function testDirectUpsertByIdentifyingColumns(): void { $company = factory(Company::class)->create(['id' => 1]); - $this->schema .= + $this->schema /** @lang GraphQL */ - ' + .= ' type User { id: ID! email: String! @@ -295,12 +295,13 @@ public function testDirectUpsertByIdentifyingColumns(): void company_id } } - ')->assertJson([ + ', + )->assertJson([ 'data' => [ 'upsertUser' => [ - 'email' => "foo@te.st", - 'name' => "bar", - 'company_id' => 1 + 'email' => 'foo@te.st', + 'name' => 'bar', + 'company_id' => 1, ], ], ]); @@ -325,13 +326,13 @@ public function testDirectUpsertByIdentifyingColumns(): void company_id } } - ' + ', )->assertJson([ 'data' => [ 'upsertUser' => [ - 'email' => "bar@te.st", - 'name' => "bar", - 'company_id' => $company->id + 'email' => 'bar@te.st', + 'name' => 'bar', + 'company_id' => $company->id, ], ], ]); From 0472f0327bbd5324353930c0d8c5f737599afc9b Mon Sep 17 00:00:00 2001 From: GavG Date: Fri, 28 Jul 2023 09:17:43 +0100 Subject: [PATCH 3/3] update changelog --- CHANGELOG.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5da6fe3e5..159f464de6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +### Added + +- Add support for identifyingColumns on upserts + ## v6.15.0 ### Added @@ -524,7 +528,7 @@ You can find and compare releases at the [GitHub release page](https://github.co ### Fixed -- Distinguish between client-safe and non-client-safe errors in `TestResponse::assertGraphQLError()` +- Distinguish between client-safe and non-client-safe errors in `TestResponse::assertGraphQLError()` ## v5.46.0 @@ -831,7 +835,7 @@ You can find and compare releases at the [GitHub release page](https://github.co ### Fixed -- Avoid PHP 8.1 deprecation warning by implementing `__serialize()` and `__unserialize()` https://github.com/nuwave/lighthouse/pull/1987 +- Avoid PHP 8.1 deprecation warning by implementing `__serialize()` and `__unserialize()` https://github.com/nuwave/lighthouse/pull/1987 ### Deprecated @@ -1107,7 +1111,7 @@ You can find and compare releases at the [GitHub release page](https://github.co ### Changed -- Improve performance through [`graphql-php` lazy field definitions](https://github.com/webonyx/graphql-php/pull/861) https://github.com/nuwave/lighthouse/pull/1851 +- Improve performance through [`graphql-php` lazy field definitions](https://github.com/webonyx/graphql-php/pull/861) https://github.com/nuwave/lighthouse/pull/1851 - Load individual subscription fields lazily instead of loading them all eagerly https://github.com/nuwave/lighthouse/pull/1851 - Require `webonyx/graphql-php:^14.7` https://github.com/nuwave/lighthouse/pull/1851