diff --git a/CHANGELOG.md b/CHANGELOG.md index 584286f00..d217861a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +### Fixed + +- Honour `SaveAwareArgResolver::runBeforeSave()` in nested mutations instead of only at the top level https://github.com/nuwave/lighthouse/pull/2784 +- Run all pre-save arg resolvers lifted out of `@nest` instead of silently discarding those whose field name collides with a sibling https://github.com/nuwave/lighthouse/pull/2784 + ## v6.69.1 ### Fixed diff --git a/src/Execution/Arguments/ArgPartitioner.php b/src/Execution/Arguments/ArgPartitioner.php index e001f2c74..cff872905 100644 --- a/src/Execution/Arguments/ArgPartitioner.php +++ b/src/Execution/Arguments/ArgPartitioner.php @@ -20,37 +20,19 @@ class ArgPartitioner /** * Partition the arguments into nested and regular. * - * @return array{ - * 0: \Nuwave\Lighthouse\Execution\Arguments\ArgumentSet, - * 1: \Nuwave\Lighthouse\Execution\Arguments\ArgumentSet, - * } - */ - public static function nestedArgResolvers(ArgumentSet $argumentSet, mixed $root): array - { - static::prepareArgResolvers($argumentSet, $root); - - return static::partition( - $argumentSet, - static fn (string $name, Argument $argument): bool => isset($argument->resolver), - ); - } - - /** - * Like nestedArgResolvers(), but excludes SaveAwareArgResolvers that run before save. - * - * Used by SaveModel's ResolveNested wrapper so pre-save resolvers stay in the - * regular set and reach SaveModel for execution before $model->save(). + * SaveAwareArgResolvers where runBeforeSave() returns true stay in the regular set + * so they reach SaveModel for execution before $model->save(). * * @return array{ * 0: \Nuwave\Lighthouse\Execution\Arguments\ArgumentSet, * 1: \Nuwave\Lighthouse\Execution\Arguments\ArgumentSet, * } */ - public static function nestedArgResolversWithoutPreSave(ArgumentSet $argumentSet, mixed $root): array + public static function nestedArgResolvers(ArgumentSet $argumentSet, mixed $root): array { $model = static::prepareArgResolvers($argumentSet, $root); - [$nested, $regular] = static::partition( + return static::partition( $argumentSet, static function (string $name, Argument $argument) use ($root, $model): bool { $resolver = $argument->resolver; @@ -71,13 +53,6 @@ static function (string $name, Argument $argument) use ($root, $model): bool { return true; }, ); - - if ($model !== null) { - assert($root instanceof Model); - static::liftPreSaveResolversFromNest($nested, $regular, $root, $model); - } - - return [$nested, $regular]; } /** @@ -209,13 +184,22 @@ public static function methodReturnsRelation( } /** - * Recursively traverse @nest arguments and lift pre-save resolvers to the regular set - * so they reach SaveModel and execute before $model->save(). + * Recursively collect the pre-save resolvers within @nest arguments, + * so they can run before $root->save() instead of after it. + * + * The result is an ordered list rather than a name-keyed set: + * field names are unique per input type, not across a whole input tree. + * + * Leaves the given arguments untouched, as they may be shared with cached + * or spread ArgumentSets that the caller still owns. * - * @param \ReflectionClass<\Illuminate\Database\Eloquent\Model> $model + * @return list<\Nuwave\Lighthouse\Execution\Arguments\Argument> */ - protected static function liftPreSaveResolversFromNest(ArgumentSet $nested, ArgumentSet $regular, Model $root, \ReflectionClass $model): void + public static function liftPreSaveResolversFromNest(ArgumentSet $nested, Model $root): array { + $model = new \ReflectionClass($root); + $lifted = []; + foreach ($nested->arguments as $argument) { if (! $argument->resolver instanceof NestDirective) { continue; @@ -234,18 +218,22 @@ protected static function liftPreSaveResolversFromNest(ArgumentSet $nested, Argu $resolver = $childArgument->resolver; if (self::shouldRunBeforeSave($resolver, $root)) { - $regular->arguments[$childName] = $childArgument; - unset($nestValue->arguments[$childName]); + $lifted[] = $childArgument; continue; } if ($resolver instanceof NestDirective) { $childNested = new ArgumentSet(); $childNested->arguments[$childName] = $childArgument; - static::liftPreSaveResolversFromNest($childNested, $regular, $root, $model); + + foreach (static::liftPreSaveResolversFromNest($childNested, $root) as $deeplyNested) { + $lifted[] = $deeplyNested; + } } } } + + return $lifted; } /** @return \ReflectionClass<\Illuminate\Database\Eloquent\Model>|null */ diff --git a/src/Execution/Arguments/ResolveNested.php b/src/Execution/Arguments/ResolveNested.php index 83a738181..0079fb152 100644 --- a/src/Execution/Arguments/ResolveNested.php +++ b/src/Execution/Arguments/ResolveNested.php @@ -2,8 +2,10 @@ namespace Nuwave\Lighthouse\Execution\Arguments; +use Illuminate\Database\Eloquent\Model; use Nuwave\Lighthouse\Schema\Directives\NestDirective; use Nuwave\Lighthouse\Support\Contracts\ArgResolver; +use Nuwave\Lighthouse\Support\Contracts\PreSaveArgumentsAware; class ResolveNested implements ArgResolver { @@ -26,13 +28,43 @@ public function __invoke(mixed $root, $args): mixed [$nestedArgs, $regularArgs] = ($this->argPartitioner)($args, $root); assert($nestedArgs instanceof ArgumentSet); - if ($this->previous !== null) { - $root = ($this->previous)($root, $regularArgs); + $previous = $this->previous; + $liftedPreSave = []; + + if ($root instanceof Model && $previous instanceof PreSaveArgumentsAware) { + $liftable = ArgPartitioner::liftPreSaveResolversFromNest($nestedArgs, $root); + + if ($liftable !== []) { + $withPreSave = $previous->withPreSaveArguments($liftable); + + if ($withPreSave !== null) { + $previous = $withPreSave; + $liftedPreSave = $liftable; + } + } } + if ($previous !== null) { + $root = $previous($root, $regularArgs); + } + + $this->resolveNestedArguments($root, $nestedArgs, $liftedPreSave); + + return $root; + } + + /** @param list<\Nuwave\Lighthouse\Execution\Arguments\Argument> $alreadyRun arguments the saver ran before saving */ + protected function resolveNestedArguments(mixed $root, ArgumentSet $nestedArgs, array $alreadyRun): void + { foreach ($nestedArgs->arguments as $nested) { $resolver = $nested->resolver; - assert($resolver !== null, 'we know the resolver is there because we partitioned for it'); + if ($resolver === null) { + continue; + } + + if (in_array($nested, $alreadyRun, true)) { + continue; + } $value = $nested->value; if ($resolver instanceof NestDirective) { @@ -42,14 +74,15 @@ public function __invoke(mixed $root, $args): mixed assert($value instanceof ArgumentSet, 'NestDirective validates that @nest is used on non-list input object types.'); - $nestResolver = new self(null, $this->argPartitioner); - $nestResolver($root, $value); + // Partitioning attaches the resolvers of the children, including implicitly detected relations. + // Its classification is irrelevant here: there is no saver to hand regular arguments to, + // and children the saver did not run must still resolve. + ($this->argPartitioner)($value, $root); + $this->resolveNestedArguments($root, $value, $alreadyRun); continue; } $resolver($root, $value); } - - return $root; } } diff --git a/src/Execution/Arguments/SaveModel.php b/src/Execution/Arguments/SaveModel.php index 4222aebaf..acb9ad51e 100644 --- a/src/Execution/Arguments/SaveModel.php +++ b/src/Execution/Arguments/SaveModel.php @@ -9,15 +9,27 @@ use Illuminate\Database\Eloquent\Relations\MorphTo; use Illuminate\Database\Eloquent\Relations\Relation; use Nuwave\Lighthouse\Support\Contracts\ArgResolver; +use Nuwave\Lighthouse\Support\Contracts\PreSaveArgumentsAware; use Nuwave\Lighthouse\Support\Contracts\SaveAwareArgResolver; -class SaveModel implements ArgResolver +class SaveModel implements ArgResolver, PreSaveArgumentsAware { + /** @var list<\Nuwave\Lighthouse\Execution\Arguments\Argument> */ + protected array $preSaveArguments = []; + public function __construct( /** @var \Illuminate\Database\Eloquent\Relations\Relation<\Illuminate\Database\Eloquent\Model>|null $parentRelation */ protected ?Relation $parentRelation = null, ) {} + public function withPreSaveArguments(array $arguments): static + { + $clone = clone $this; + $clone->preSaveArguments = $arguments; + + return $clone; + } + /** * @param Model $model * @param ArgumentSet $args @@ -62,10 +74,10 @@ public function __invoke($model, $args): Model $morphToResolver($model, $nestedOperations->value); } - foreach ($preSave->arguments as $nested) { - $resolver = $nested->resolver; + foreach ([...array_values($preSave->arguments), ...$this->preSaveArguments] as $preSaveArgument) { + $resolver = $preSaveArgument->resolver; assert($resolver instanceof SaveAwareArgResolver, 'Resolver must be a SaveAwareArgResolver because we partitioned for it.'); - $resolver($model, $nested->value); + $resolver($model, $preSaveArgument->value); } if ($this->parentRelation instanceof HasOneOrMany) { diff --git a/src/Execution/Arguments/UpdateModel.php b/src/Execution/Arguments/UpdateModel.php index c5e998564..81b971fae 100644 --- a/src/Execution/Arguments/UpdateModel.php +++ b/src/Execution/Arguments/UpdateModel.php @@ -5,8 +5,9 @@ use GraphQL\Error\Error; use Illuminate\Support\Arr; use Nuwave\Lighthouse\Support\Contracts\ArgResolver; +use Nuwave\Lighthouse\Support\Contracts\PreSaveArgumentsAware; -class UpdateModel implements ArgResolver +class UpdateModel implements ArgResolver, PreSaveArgumentsAware { public const MISSING_PRIMARY_KEY_FOR_UPDATE = 'Missing primary key for update.'; @@ -19,6 +20,24 @@ public function __construct(callable $previous) $this->previous = $previous; } + public function withPreSaveArguments(array $arguments): ?static + { + $previous = $this->previous; + if (! $previous instanceof PreSaveArgumentsAware) { + return null; + } + + $previousWithPreSave = $previous->withPreSaveArguments($arguments); + if ($previousWithPreSave === null) { + return null; + } + + $clone = clone $this; + $clone->previous = $previousWithPreSave; + + return $clone; + } + /** * @param \Illuminate\Database\Eloquent\Model $model * @param ArgumentSet $args diff --git a/src/Execution/Arguments/UpsertModel.php b/src/Execution/Arguments/UpsertModel.php index 4e7f3589c..4dab2491f 100644 --- a/src/Execution/Arguments/UpsertModel.php +++ b/src/Execution/Arguments/UpsertModel.php @@ -5,10 +5,11 @@ use GraphQL\Error\Error; use Illuminate\Database\Eloquent\Model; use Nuwave\Lighthouse\Support\Contracts\ArgResolver; +use Nuwave\Lighthouse\Support\Contracts\PreSaveArgumentsAware; use function Safe\array_flip; -class UpsertModel implements ArgResolver +class UpsertModel implements ArgResolver, PreSaveArgumentsAware { public const MISSING_IDENTIFYING_COLUMNS_FOR_UPSERT = 'All configured identifying columns must be present and non-null for upsert.'; @@ -24,6 +25,24 @@ public function __construct( $this->previous = $previous; } + public function withPreSaveArguments(array $arguments): ?static + { + $previous = $this->previous; + if (! $previous instanceof PreSaveArgumentsAware) { + return null; + } + + $previousWithPreSave = $previous->withPreSaveArguments($arguments); + if ($previousWithPreSave === null) { + return null; + } + + $clone = clone $this; + $clone->previous = $previousWithPreSave; + + return $clone; + } + /** * @param \Illuminate\Database\Eloquent\Model $model * @param ArgumentSet $args diff --git a/src/Schema/Directives/ModelMutationDirective.php b/src/Schema/Directives/ModelMutationDirective.php index 8c0b12614..3e272a6d6 100644 --- a/src/Schema/Directives/ModelMutationDirective.php +++ b/src/Schema/Directives/ModelMutationDirective.php @@ -67,7 +67,6 @@ protected function executeMutation(Model $model, ArgumentSet|array $args, ?Relat { $update = new ResolveNested( $this->makeExecutionFunction($parentRelation), - [ArgPartitioner::class, 'nestedArgResolversWithoutPreSave'], ); return Utils::mapEach( diff --git a/src/Support/Contracts/PreSaveArgumentsAware.php b/src/Support/Contracts/PreSaveArgumentsAware.php new file mode 100644 index 000000000..17033531e --- /dev/null +++ b/src/Support/Contracts/PreSaveArgumentsAware.php @@ -0,0 +1,23 @@ + $arguments + * + * @return static|null null when this resolver can not run pre-save arguments + */ + public function withPreSaveArguments(array $arguments): ?static; +} diff --git a/tests/Integration/Schema/Directives/CreateDirectiveTest.php b/tests/Integration/Schema/Directives/CreateDirectiveTest.php index e484b269d..b1de20699 100644 --- a/tests/Integration/Schema/Directives/CreateDirectiveTest.php +++ b/tests/Integration/Schema/Directives/CreateDirectiveTest.php @@ -876,4 +876,434 @@ public function testCustomDirectiveSetsModelAttributesBeforeSaveInsideNest(): vo ], ]); } + + public function testCustomDirectiveSetsModelAttributesBeforeSaveInsideNestedHasMany(): void + { + $this->schema .= /** @lang GraphQL */ <<<'GRAPHQL' + type Task { + id: ID! + name: String! + latitude: Float + longitude: Float + } + + type User { + id: ID! + name: String! + tasks: [Task!]! @hasMany + } + + type Mutation { + createUser(input: CreateUserInput! @spread): User @create + } + + input CreateUserInput { + name: String! + tasks: CreateTaskRelation + } + + input CreateTaskRelation { + create: [CreateTaskInput!] + } + + input CreateTaskInput { + name: String! + location: LocationInput @geocode + } + + input LocationInput { + lat: Float! + lng: Float! + } + GRAPHQL; + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation { + createUser(input: { + name: "Geo Parent" + tasks: { + create: [{ + name: "Geo Task" + location: { + lat: 48.1351 + lng: 11.5820 + } + }] + } + }) { + id + name + tasks { + id + name + latitude + longitude + } + } + } + GRAPHQL)->assertJson([ + 'data' => [ + 'createUser' => [ + 'name' => 'Geo Parent', + 'tasks' => [ + [ + 'name' => 'Geo Task', + 'latitude' => 48.1351, + 'longitude' => 11.582, + ], + ], + ], + ], + ]); + } + + public function testCustomDirectiveSetsModelAttributesBeforeSaveInsideNestInsideNestedHasMany(): void + { + $this->schema .= /** @lang GraphQL */ <<<'GRAPHQL' + type Task { + id: ID! + name: String! + latitude: Float + longitude: Float + } + + type User { + id: ID! + name: String! + tasks: [Task!]! @hasMany + } + + type Mutation { + createUser(input: CreateUserInput! @spread): User @create + } + + input CreateUserInput { + name: String! + tasks: CreateTaskRelation + } + + input CreateTaskRelation { + create: [CreateTaskInput!] + } + + input CreateTaskInput { + name: String! + nested: TaskNestedInput @nest + } + + input TaskNestedInput { + location: LocationInput @geocode + } + + input LocationInput { + lat: Float! + lng: Float! + } + GRAPHQL; + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation { + createUser(input: { + name: "Geo Nest Parent" + tasks: { + create: [{ + name: "Geo Nest Task" + nested: { + location: { + lat: 48.1351 + lng: 11.5820 + } + } + }] + } + }) { + id + name + tasks { + id + name + latitude + longitude + } + } + } + GRAPHQL)->assertJson([ + 'data' => [ + 'createUser' => [ + 'name' => 'Geo Nest Parent', + 'tasks' => [ + [ + 'name' => 'Geo Nest Task', + 'latitude' => 48.1351, + 'longitude' => 11.582, + ], + ], + ], + ], + ]); + } + + public function testCustomDirectiveSetsModelAttributesBeforeSaveInsideDeeplyNestedRelation(): void + { + $this->schema .= /** @lang GraphQL */ <<<'GRAPHQL' + type Post { + id: ID! + title: String! + latitude: Float + longitude: Float + } + + type Task { + id: ID! + name: String! + post: Post @hasOne + } + + type User { + id: ID! + name: String! + tasks: [Task!]! @hasMany + } + + type Mutation { + createUser(input: CreateUserInput! @spread): User @create + } + + input CreateUserInput { + name: String! + tasks: CreateTaskRelation + } + + input CreateTaskRelation { + create: [CreateTaskInput!] + } + + input CreateTaskInput { + name: String! + post: CreatePostRelation + } + + input CreatePostRelation { + create: CreatePostInput + } + + input CreatePostInput { + title: String! + location: LocationInput @geocode + } + + input LocationInput { + lat: Float! + lng: Float! + } + GRAPHQL; + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation { + createUser(input: { + name: "Geo Deep Parent" + tasks: { + create: [{ + name: "Geo Deep Task" + post: { + create: { + title: "Geo Deep Post" + location: { + lat: 48.1351 + lng: 11.5820 + } + } + } + }] + } + }) { + id + name + tasks { + id + name + post { + id + title + latitude + longitude + } + } + } + } + GRAPHQL)->assertJson([ + 'data' => [ + 'createUser' => [ + 'name' => 'Geo Deep Parent', + 'tasks' => [ + [ + 'name' => 'Geo Deep Task', + 'post' => [ + 'title' => 'Geo Deep Post', + 'latitude' => 48.1351, + 'longitude' => 11.582, + ], + ], + ], + ], + ], + ]); + } + + /** + * The plain sibling `location` and the same-named `@nest` child both run, in that order. + * Both write the same columns, so the last write wins. + */ + public function testNestChildAndSiblingWithSameNameBothRunInsideNestedHasMany(): void + { + $this->schema .= /** @lang GraphQL */ <<<'GRAPHQL' + type Task { + id: ID! + name: String! + latitude: Float + longitude: Float + } + + type User { + id: ID! + name: String! + tasks: [Task!]! @hasMany + } + + type Mutation { + createUser(input: CreateUserInput! @spread): User @create + } + + input CreateUserInput { + name: String! + tasks: CreateTaskRelation + } + + input CreateTaskRelation { + create: [CreateTaskInput!] + } + + input CreateTaskInput { + name: String! + location: LocationInput @geocode + nested: TaskNestedInput @nest + } + + input TaskNestedInput { + location: LocationInput @geocode + } + + input LocationInput { + lat: Float! + lng: Float! + } + GRAPHQL; + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation { + createUser(input: { + name: "Geo Collision Parent" + tasks: { + create: [{ + name: "Geo Collision Task" + location: { + lat: 1.0 + lng: 2.0 + } + nested: { + location: { + lat: 48.1351 + lng: 11.5820 + } + } + }] + } + }) { + id + name + tasks { + id + name + latitude + longitude + } + } + } + GRAPHQL)->assertJson([ + 'data' => [ + 'createUser' => [ + 'name' => 'Geo Collision Parent', + 'tasks' => [ + [ + 'name' => 'Geo Collision Task', + 'latitude' => 48.1351, + 'longitude' => 11.582, + ], + ], + ], + ], + ]); + } + + /** A `@nest` child named like a plain sibling writes different columns, so both values must persist. */ + public function testNestChildNamedLikePlainSiblingKeepsBoth(): void + { + $this->schema .= /** @lang GraphQL */ <<<'GRAPHQL' + type Task { + id: ID! + name: String! + difficulty: Int + latitude: Float + longitude: Float + } + + type Mutation { + createTask(input: CreateTaskInput! @spread): Task @create + } + + input CreateTaskInput { + name: String! + difficulty: Int + nested: TaskNestedInput @nest + } + + input TaskNestedInput { + difficulty: LocationInput @geocode + } + + input LocationInput { + lat: Float! + lng: Float! + } + GRAPHQL; + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation { + createTask(input: { + name: "Colliding Names" + difficulty: 3 + nested: { + difficulty: { + lat: 48.1351 + lng: 11.5820 + } + } + }) { + name + difficulty + latitude + longitude + } + } + GRAPHQL)->assertJson([ + 'data' => [ + 'createTask' => [ + 'name' => 'Colliding Names', + 'difficulty' => 3, + 'latitude' => 48.1351, + 'longitude' => 11.582, + ], + ], + ]); + } } diff --git a/tests/Integration/Schema/Directives/NestDirectiveTest.php b/tests/Integration/Schema/Directives/NestDirectiveTest.php index 18020540c..61e140dc2 100644 --- a/tests/Integration/Schema/Directives/NestDirectiveTest.php +++ b/tests/Integration/Schema/Directives/NestDirectiveTest.php @@ -351,6 +351,82 @@ public function testSiblingNestBlocksWithSameChildNameLastWins(): void ]); } + /** Same-named children of sibling `@nest` blocks that write different columns must both run. */ + public function testSiblingNestBlocksWithSameChildNameBothRun(): void + { + $this->schema .= /** @lang GraphQL */ <<<'GRAPHQL' + type Mutation { + createUser(input: CreateUserInput! @spread): User @create + } + + input CreateUserInput { + name: String! + alpha: AlphaInput @nest + beta: BetaInput @nest + } + + input AlphaInput { + detail: LocationInput @geocode + } + + input BetaInput { + detail: EmailAddressInput @emailAddress + } + + input LocationInput { + lat: Float! + lng: Float! + } + + input EmailAddressInput { + local: String! + domain: String! + } + + type User { + id: ID! + name: String! + email: String + latitude: Float + longitude: Float + } + GRAPHQL; + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation { + createUser(input: { + name: "Sibling Nest" + alpha: { + detail: { + lat: 48.0 + lng: 11.0 + } + } + beta: { + detail: { + local: "sibling" + domain: "example.com" + } + } + }) { + name + email + latitude + longitude + } + } + GRAPHQL)->assertJson([ + 'data' => [ + 'createUser' => [ + 'name' => 'Sibling Nest', + 'email' => 'sibling@example.com', + 'latitude' => 48.0, + 'longitude' => 11.0, + ], + ], + ]); + } + public function testNullableNestWithNullValue(): void { $this->schema .= /** @lang GraphQL */ <<<'GRAPHQL' diff --git a/tests/Integration/Schema/Directives/UpdateDirectiveTest.php b/tests/Integration/Schema/Directives/UpdateDirectiveTest.php index 6ef61bf91..fb0839dc8 100644 --- a/tests/Integration/Schema/Directives/UpdateDirectiveTest.php +++ b/tests/Integration/Schema/Directives/UpdateDirectiveTest.php @@ -423,4 +423,168 @@ public function testNestedUpdateOnInputList(): void ], ]); } + + public function testCustomDirectiveSetsModelAttributesBeforeSaveInsideNestedHasMany(): void + { + $user = factory(User::class)->create(); + $this->assertInstanceOf(User::class, $user); + + $task = factory(Task::class)->make(); + $this->assertInstanceOf(Task::class, $task); + $task->user()->associate($user); + $task->save(); + + $this->schema .= /** @lang GraphQL */ <<<'GRAPHQL' + type Task { + id: ID! + name: String! + latitude: Float + longitude: Float + } + + type User { + id: ID! + name: String! + tasks: [Task!]! @hasMany + } + + type Mutation { + updateUser(input: UpdateUserInput! @spread): User @update + } + + input UpdateUserInput { + id: ID! + name: String + tasks: UpdateTaskRelation + } + + input UpdateTaskRelation { + update: [UpdateTaskInput!] + } + + input UpdateTaskInput { + id: ID! + name: String + location: LocationInput @geocode + } + + input LocationInput { + lat: Float! + lng: Float! + } + GRAPHQL; + + $this->graphQL(/** @lang GraphQL */ <<id} + name: "Geo Update Parent" + tasks: { + update: [{ + id: {$task->id} + name: "Geo Update Task" + location: { + lat: 48.1351 + lng: 11.5820 + } + }] + } + }) { + name + tasks { + name + latitude + longitude + } + } + } + GRAPHQL)->assertJson([ + 'data' => [ + 'updateUser' => [ + 'name' => 'Geo Update Parent', + 'tasks' => [ + [ + 'name' => 'Geo Update Task', + 'latitude' => 48.1351, + 'longitude' => 11.582, + ], + ], + ], + ], + ]); + } + + /** A `@nest` child named `id` must not clobber the primary key that identifies the row to update. */ + public function testNestChildNamedIdDoesNotClobberPrimaryKey(): void + { + $decoy = new Task(); + $decoy->name = 'Decoy'; + $decoy->save(); + + $task = new Task(); + $task->name = 'Target'; + $task->save(); + + $this->schema .= /** @lang GraphQL */ <<<'GRAPHQL' + type Task { + id: ID! + name: String! + latitude: Float + longitude: Float + } + + type Mutation { + updateTask(input: UpdateTaskInput! @spread): Task @update + } + + input UpdateTaskInput { + id: ID! + name: String + nested: TaskNestedInput @nest + } + + input TaskNestedInput { + id: LocationInput @geocode + } + + input LocationInput { + lat: Float! + lng: Float! + } + GRAPHQL; + + $this->graphQL(/** @lang GraphQL */ <<<'GRAPHQL' + mutation ($id: ID!) { + updateTask(input: { + id: $id + name: "Renamed target" + nested: { + id: { + lat: 48.1351 + lng: 11.5820 + } + } + }) { + id + name + latitude + longitude + } + } + GRAPHQL, [ + 'id' => $task->id, + ])->assertJson([ + 'data' => [ + 'updateTask' => [ + 'id' => (string) $task->id, + 'name' => 'Renamed target', + 'latitude' => 48.1351, + 'longitude' => 11.582, + ], + ], + ]); + + $decoy->refresh(); + $this->assertSame('Decoy', $decoy->name); + } } diff --git a/tests/Integration/Schema/Directives/UpsertDirectiveTest.php b/tests/Integration/Schema/Directives/UpsertDirectiveTest.php index 49324c7c4..e7cdb9fee 100644 --- a/tests/Integration/Schema/Directives/UpsertDirectiveTest.php +++ b/tests/Integration/Schema/Directives/UpsertDirectiveTest.php @@ -550,6 +550,110 @@ public function testUpsertByIdentifyingColumnWithInputSpread(): void $this->assertSame('baz', User::firstOrFail()->name); } + public function testCustomDirectiveSetsModelAttributesBeforeSaveInsideNestedHasMany(): void + { + $user = factory(User::class)->create(); + $this->assertInstanceOf(User::class, $user); + + $task = factory(Task::class)->make(); + $this->assertInstanceOf(Task::class, $task); + $task->user()->associate($user); + $task->save(); + + $this->schema .= /** @lang GraphQL */ <<<'GRAPHQL' + type Task { + id: ID! + name: String! + latitude: Float + longitude: Float + } + + type User { + id: ID! + name: String! + tasks: [Task!]! @hasMany + } + + type Mutation { + updateUser(input: UpdateUserInput! @spread): User @update + } + + input UpdateUserInput { + id: ID! + name: String + tasks: UpsertTaskRelation + } + + input UpsertTaskRelation { + upsert: [UpsertTaskInput!] + } + + input UpsertTaskInput { + id: ID + name: String + location: LocationInput @geocode + } + + input LocationInput { + lat: Float! + lng: Float! + } + GRAPHQL; + + $this->graphQL(/** @lang GraphQL */ <<id} + name: "Geo Upsert Parent" + tasks: { + upsert: [ + { + id: {$task->id} + name: "Geo Upserted Task" + location: { + lat: 48.1351 + lng: 11.5820 + } + } + { + name: "Geo Inserted Task" + location: { + lat: 52.5200 + lng: 13.4050 + } + } + ] + } + }) { + name + tasks { + name + latitude + longitude + } + } + } + GRAPHQL)->assertJson([ + 'data' => [ + 'updateUser' => [ + 'name' => 'Geo Upsert Parent', + 'tasks' => [ + [ + 'name' => 'Geo Upserted Task', + 'latitude' => 48.1351, + 'longitude' => 11.582, + ], + [ + 'name' => 'Geo Inserted Task', + 'latitude' => 52.52, + 'longitude' => 13.405, + ], + ], + ], + ], + ]); + } + public static function resolveType(): Type { $typeRegistry = Container::getInstance()->make(TypeRegistry::class); diff --git a/tests/Unit/Execution/Arguments/ArgPartitionerTest.php b/tests/Unit/Execution/Arguments/ArgPartitionerTest.php index 581cf9a89..62aa900d6 100644 --- a/tests/Unit/Execution/Arguments/ArgPartitionerTest.php +++ b/tests/Unit/Execution/Arguments/ArgPartitionerTest.php @@ -138,31 +138,6 @@ public function testSaveAwareArgResolverWithNonModelRoot(): void ); } - public function testSaveAwareArgResolverWithNonModelRootInWithoutPreSave(): void - { - $argumentSet = new ArgumentSet(); - - $regular = new Argument(); - $argumentSet->arguments['regular'] = $regular; - - $saveAware = new Argument(); - $saveAware->directives->push(new SaveAwareNested()); - $argumentSet->arguments['saveAware'] = $saveAware; - - [$nestedArgs, $regularArgs] = ArgPartitioner::nestedArgResolversWithoutPreSave($argumentSet, null); - - $this->assertSame( - ['regular' => $regular], - $regularArgs->arguments, - ); - - $this->assertSame( - ['saveAware' => $saveAware], - $nestedArgs->arguments, - 'SaveAwareArgResolver should be in nested set when root is not a Model', - ); - } - public function testSaveAwareArgResolverWithModelRoot(): void { $argumentSet = new ArgumentSet(); @@ -174,7 +149,7 @@ public function testSaveAwareArgResolverWithModelRoot(): void $saveAware->directives->push(new SaveAwareNested()); $argumentSet->arguments['saveAware'] = $saveAware; - [$nestedArgs, $regularArgs] = ArgPartitioner::nestedArgResolversWithoutPreSave($argumentSet, new User()); + [$nestedArgs, $regularArgs] = ArgPartitioner::nestedArgResolvers($argumentSet, new User()); $this->assertSame( ['regular' => $regular, 'saveAware' => $saveAware], diff --git a/tests/Unit/Execution/Arguments/ResolveNestedTest.php b/tests/Unit/Execution/Arguments/ResolveNestedTest.php new file mode 100644 index 000000000..8f04e4e47 --- /dev/null +++ b/tests/Unit/Execution/Arguments/ResolveNestedTest.php @@ -0,0 +1,58 @@ +value = new ArgumentSet(); + $preSaveChild->directives->push($saveAwareResolver); + + $nestValue = new ArgumentSet(); + $nestValue->arguments['location'] = $preSaveChild; + + $nest = new Argument(); + $nest->value = $nestValue; + $nest->directives->push(new NestDirective()); + + $name = new Argument(); + $name->value = 'Sepp'; + + $argumentSet = new ArgumentSet(); + $argumentSet->arguments['name'] = $name; + $argumentSet->arguments['nested'] = $nest; + + $model = new User(); + $passedToSaver = null; + $resolveNested = new ResolveNested(static function (mixed $root, ArgumentSet $args) use (&$passedToSaver): mixed { + $passedToSaver = $args; + + return $root; + }); + + $resolveNested($model, $argumentSet); + + $this->assertTrue($saveAwareResolver->wasCalled, 'Pre-save resolvers inside @nest must run even when the saver cannot run them before save'); + $this->assertSame($model, $saveAwareResolver->receivedRoot); + + $this->assertInstanceOf(ArgumentSet::class, $passedToSaver); + $this->assertSame( + ['name' => $name], + $passedToSaver->arguments, + 'Savers that cannot run pre-save arguments must not receive them', + ); + } +} diff --git a/tests/Utils/Directives/EmailAddressDirective.php b/tests/Utils/Directives/EmailAddressDirective.php new file mode 100644 index 000000000..74a5b08ff --- /dev/null +++ b/tests/Utils/Directives/EmailAddressDirective.php @@ -0,0 +1,37 @@ +toArray(); + $model->setAttribute('email', "{$parts['local']}@{$parts['domain']}"); + } +} diff --git a/tests/Utils/Models/Post.php b/tests/Utils/Models/Post.php index 9bb1df79c..c2e98f681 100644 --- a/tests/Utils/Models/Post.php +++ b/tests/Utils/Models/Post.php @@ -21,6 +21,8 @@ * Attributes * @property string $title * @property string|null $body + * @property float|null $latitude + * @property float|null $longitude * * Timestamps * @property \Illuminate\Support\Carbon $created_at diff --git a/tests/Utils/Models/Task.php b/tests/Utils/Models/Task.php index bc6ea94a5..6420ce835 100644 --- a/tests/Utils/Models/Task.php +++ b/tests/Utils/Models/Task.php @@ -22,6 +22,8 @@ * @property string $name * @property int|null $difficulty * @property string|null $guard + * @property float|null $latitude + * @property float|null $longitude * @property \Illuminate\Support\Carbon $completed_at * * Timestamps diff --git a/tests/database/migrations/2018_02_28_000003_create_testbench_tasks_table.php b/tests/database/migrations/2018_02_28_000003_create_testbench_tasks_table.php index e2cb57ecb..bdf411c75 100644 --- a/tests/database/migrations/2018_02_28_000003_create_testbench_tasks_table.php +++ b/tests/database/migrations/2018_02_28_000003_create_testbench_tasks_table.php @@ -16,6 +16,8 @@ public function up(): void ->comment('The purpose of this property is to collide with a native model method name'); $table->unsignedBigInteger('difficulty')->nullable(); $table->unsignedBigInteger('user_id')->nullable(); + $table->double('latitude')->nullable(); + $table->double('longitude')->nullable(); $table->timestamp('completed_at')->nullable(); $table->timestamps(); $table->softDeletes(); diff --git a/tests/database/migrations/2018_02_28_000004_create_testbench_posts_table.php b/tests/database/migrations/2018_02_28_000004_create_testbench_posts_table.php index 547db555c..89236dfad 100644 --- a/tests/database/migrations/2018_02_28_000004_create_testbench_posts_table.php +++ b/tests/database/migrations/2018_02_28_000004_create_testbench_posts_table.php @@ -15,6 +15,8 @@ public function up(): void $table->unsignedBigInteger('task_id'); $table->unsignedBigInteger('user_id')->nullable(); $table->unsignedBigInteger('parent_id')->nullable(); + $table->double('latitude')->nullable(); + $table->double('longitude')->nullable(); $table->timestamps(); $table->softDeletes(); });