From e3ba8f0b23e56bfa4f31f0bad7f2263c93506fca Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 2 Sep 2024 09:50:47 +0200 Subject: [PATCH 1/6] Allow `@deprecated` directive on arguments and input fields --- CHANGELOG.md | 6 ++++++ docs/6/api-reference/directives.md | 13 ++++++------- docs/master/api-reference/directives.md | 13 ++++++------- src/Schema/Directives/DeprecatedDirective.php | 8 ++++---- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 685e6d0d1e..103ab58a1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +## v6.44.0 + +### Added + +- Allow `@deprecated` directive on arguments and input fields https://github.com/nuwave/lighthouse/pull/2607 + ## v6.43.1 ### Changed diff --git a/docs/6/api-reference/directives.md b/docs/6/api-reference/directives.md index 50db50e593..d53a98e6ff 100644 --- a/docs/6/api-reference/directives.md +++ b/docs/6/api-reference/directives.md @@ -1165,17 +1165,16 @@ Marks an element of a GraphQL schema as no longer supported. """ directive @deprecated( """ - Explains why this element was deprecated, usually also including a - suggestion for how to access supported similar data. - Formatted in [Markdown](https://commonmark.org). + Explains why this element was deprecated. + It is also beneficial to suggest what to use instead. + Formatted in Markdown, as specified by [CommonMark](https://commonmark.org). """ reason: String = "No longer supported" -) on FIELD_DEFINITION | ENUM_VALUE +) on FIELD_DEFINITION | ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION | ENUM_VALUE ``` -You can mark fields as deprecated by adding the [@deprecated](#deprecated) directive. -It is recommended to provide a `reason` for the deprecation, as well as a suggestion on -how to move forward. +You can indicate schema elements are no longer supported by adding the [@deprecated](#deprecated) directive. +It is recommended to provide a `reason` for the deprecation, as well as suggest a replacement. ```graphql type Query { diff --git a/docs/master/api-reference/directives.md b/docs/master/api-reference/directives.md index 50db50e593..d53a98e6ff 100644 --- a/docs/master/api-reference/directives.md +++ b/docs/master/api-reference/directives.md @@ -1165,17 +1165,16 @@ Marks an element of a GraphQL schema as no longer supported. """ directive @deprecated( """ - Explains why this element was deprecated, usually also including a - suggestion for how to access supported similar data. - Formatted in [Markdown](https://commonmark.org). + Explains why this element was deprecated. + It is also beneficial to suggest what to use instead. + Formatted in Markdown, as specified by [CommonMark](https://commonmark.org). """ reason: String = "No longer supported" -) on FIELD_DEFINITION | ENUM_VALUE +) on FIELD_DEFINITION | ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION | ENUM_VALUE ``` -You can mark fields as deprecated by adding the [@deprecated](#deprecated) directive. -It is recommended to provide a `reason` for the deprecation, as well as a suggestion on -how to move forward. +You can indicate schema elements are no longer supported by adding the [@deprecated](#deprecated) directive. +It is recommended to provide a `reason` for the deprecation, as well as suggest a replacement. ```graphql type Query { diff --git a/src/Schema/Directives/DeprecatedDirective.php b/src/Schema/Directives/DeprecatedDirective.php index 8607233caa..76e2fef2d1 100644 --- a/src/Schema/Directives/DeprecatedDirective.php +++ b/src/Schema/Directives/DeprecatedDirective.php @@ -14,12 +14,12 @@ public static function definition(): string """ directive @deprecated( """ - Explains why this element was deprecated, usually also including a - suggestion for how to access supported similar data. Formatted - in [Markdown](https://daringfireball.net/projects/markdown). + Explains why this element was deprecated. + It is also beneficial to suggest what to use instead. + Formatted in Markdown, as specified by [CommonMark](https://commonmark.org). """ reason: String = "No longer supported" -) on FIELD_DEFINITION | ENUM_VALUE +) on FIELD_DEFINITION | ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION | ENUM_VALUE GRAPHQL; } } From e6210f7790ed3b6da3a81e91ac258658c470b907 Mon Sep 17 00:00:00 2001 From: Owen Voke Date: Mon, 2 Sep 2024 13:41:17 +0100 Subject: [PATCH 2/6] Ensure `deprecationReason` is set for arguments --- CHANGELOG.md | 3 +++ src/Schema/AST/ASTHelper.php | 2 +- src/Schema/Factories/ArgumentFactory.php | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 103ab58a1c..50d79e46dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +### Fixed +- Ensure `deprecationReason` is set on arguments and input fields https://github.com/nuwave/lighthouse/pull/2609 + ## v6.44.0 ### Added diff --git a/src/Schema/AST/ASTHelper.php b/src/Schema/AST/ASTHelper.php index 0e41037d6f..4f19ad1368 100644 --- a/src/Schema/AST/ASTHelper.php +++ b/src/Schema/AST/ASTHelper.php @@ -327,7 +327,7 @@ public static function qualifiedArgType( } /** Given a collection of directives, returns the string value for the deprecation reason. */ - public static function deprecationReason(EnumValueDefinitionNode|FieldDefinitionNode $node): ?string + public static function deprecationReason(EnumValueDefinitionNode|FieldDefinitionNode|InputValueDefinitionNode $node): ?string { $deprecated = Values::getDirectiveValues( DirectiveDefinition::deprecatedDirective(), diff --git a/src/Schema/Factories/ArgumentFactory.php b/src/Schema/Factories/ArgumentFactory.php index b27ea469c6..d3bab6e1d8 100644 --- a/src/Schema/Factories/ArgumentFactory.php +++ b/src/Schema/Factories/ArgumentFactory.php @@ -50,6 +50,7 @@ public function convert(InputValueDefinitionNode $definitionNode): array 'name' => $definitionNode->name->value, 'description' => $definitionNode->description?->value, 'type' => $type, + 'deprecationReason' => ASTHelper::deprecationReason($definitionNode), 'astNode' => $definitionNode, ]; From d924efec78d451f408d6a8c1c3b257025f0d4c83 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 2 Sep 2024 14:43:29 +0200 Subject: [PATCH 3/6] v6.44.1 --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50d79e46dc..4376ba6ac2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +## v6.44.1 + ### Fixed + - Ensure `deprecationReason` is set on arguments and input fields https://github.com/nuwave/lighthouse/pull/2609 ## v6.44.0 From b7f196ec0962a28f453a097f81ba0e510daec5ff Mon Sep 17 00:00:00 2001 From: Dennis Koster Date: Fri, 6 Sep 2024 09:28:02 +0200 Subject: [PATCH 4/6] Add two tests to ConvertEmptyStringsToNullDirective --- ...ConvertEmptyStringsToNullDirectiveTest.php | 62 +++++++++++++++++++ tests/Utils/Mutations/ReturnReceivedInput.php | 16 +++++ 2 files changed, 78 insertions(+) create mode 100644 tests/Utils/Mutations/ReturnReceivedInput.php diff --git a/tests/Integration/Schema/Directives/ConvertEmptyStringsToNullDirectiveTest.php b/tests/Integration/Schema/Directives/ConvertEmptyStringsToNullDirectiveTest.php index b4aa9caf39..c37e91e899 100644 --- a/tests/Integration/Schema/Directives/ConvertEmptyStringsToNullDirectiveTest.php +++ b/tests/Integration/Schema/Directives/ConvertEmptyStringsToNullDirectiveTest.php @@ -2,6 +2,7 @@ namespace Tests\Integration\Schema\Directives; +use Nuwave\Lighthouse\Schema\Directives\ConvertEmptyStringsToNullDirective; use Tests\TestCase; final class ConvertEmptyStringsToNullDirectiveTest extends TestCase @@ -159,4 +160,65 @@ public function testConvertsNonNullableArgumentsWhenUsedOnArgument(): void ], ]); } + + public function testConvertsEmptyStringToNullWithFieldDirective(): void + { + $this->schema = /** @lang GraphQL */ ' + type Query { + foo(bar: String): FooResponse + @convertEmptyStringsToNull + @field(resolver: "Tests\\\Utils\\\Mutations\\\ReturnReceivedInput") + } + + type FooResponse { + bar: String + } + '; + + $this->graphQL(/** @lang GraphQL */ ' + { + foo(bar: "") { + bar + } + } + ')->assertExactJson([ + 'data' => [ + 'foo' => [ + 'bar' => null, + ], + ], + ]); + } + + public function testConvertsEmptyStringToNullWithGlobalFieldMiddleware(): void + { + config(['lighthouse.field_middleware' => [ + ConvertEmptyStringsToNullDirective::class, + ]]); + + $this->schema = /** @lang GraphQL */ ' + type Query { + foo(bar: String): FooResponse + @field(resolver: "Tests\\\Utils\\\Mutations\\\ReturnReceivedInput") + } + + type FooResponse { + bar: String + } + '; + + $this->graphQL(/** @lang GraphQL */ ' + { + foo(bar: "") { + bar + } + } + ')->assertExactJson([ + 'data' => [ + 'foo' => [ + 'bar' => null, + ], + ], + ]); + } } diff --git a/tests/Utils/Mutations/ReturnReceivedInput.php b/tests/Utils/Mutations/ReturnReceivedInput.php new file mode 100644 index 0000000000..066fcec383 --- /dev/null +++ b/tests/Utils/Mutations/ReturnReceivedInput.php @@ -0,0 +1,16 @@ + $args + * + * @return array + */ + public function __invoke(mixed $root, array $args): array + { + return $args; + } +} From 45e1cbbc9459f38e79cff24b3c9ec4a513ab368b Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 6 Sep 2024 09:38:37 +0200 Subject: [PATCH 5/6] Improve docs for `@convertEmptyStringsToNull` https://github.com/nuwave/lighthouse/issues/2610 --- docs/6/api-reference/directives.md | 5 ++++- docs/master/api-reference/directives.md | 5 ++++- phpstan.neon | 2 +- src/Schema/Directives/ConvertEmptyStringsToNullDirective.php | 5 ++++- tests/Utils/Mutations/ReturnReceivedInput.php | 2 +- 5 files changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/6/api-reference/directives.md b/docs/6/api-reference/directives.md index d53a98e6ff..8129321f25 100644 --- a/docs/6/api-reference/directives.md +++ b/docs/6/api-reference/directives.md @@ -896,7 +896,10 @@ final class ComplexityAnalyzer ```graphql """ -Replaces `""` with `null`. +Replaces incoming empty strings `""` with `null`. + +When used upon fields, empty strings for non-nullable inputs will pass unchanged. +Only explicitly placing this on non-nullable inputs will force the conversion. """ directive @convertEmptyStringsToNull on ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION | FIELD_DEFINITION ``` diff --git a/docs/master/api-reference/directives.md b/docs/master/api-reference/directives.md index d53a98e6ff..8129321f25 100644 --- a/docs/master/api-reference/directives.md +++ b/docs/master/api-reference/directives.md @@ -896,7 +896,10 @@ final class ComplexityAnalyzer ```graphql """ -Replaces `""` with `null`. +Replaces incoming empty strings `""` with `null`. + +When used upon fields, empty strings for non-nullable inputs will pass unchanged. +Only explicitly placing this on non-nullable inputs will force the conversion. """ directive @convertEmptyStringsToNull on ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION | FIELD_DEFINITION ``` diff --git a/phpstan.neon b/phpstan.neon index b6d259e773..02d1c6d327 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -46,7 +46,7 @@ parameters: - '#Call to an undefined method Illuminate\\Database\\Eloquent\\Builder|Illuminate\\Database\\Eloquent\\Relations\\Relation|Illuminate\\Database\\Query\\Builder::(orderBy|where|whereIn|whereNotIn|whereBetween|whereJsonContains|whereNotBetween)\(\)\.#' # Laravel 11 added generics that are handled differently, so we just omit them - - '#generic class Illuminate\\Database\\Eloquent\\Builder but does not specify its types#' + - '#generic class (Illuminate\\Database\\Eloquent\\Builder|Laravel\\Scout\\Builder)( but)? does not specify its types#' # This test cheats and uses reflection to make assertions - path: tests/Unit/Schema/Directives/BaseDirectiveTest.php diff --git a/src/Schema/Directives/ConvertEmptyStringsToNullDirective.php b/src/Schema/Directives/ConvertEmptyStringsToNullDirective.php index 8b9f196451..4fe6c89406 100644 --- a/src/Schema/Directives/ConvertEmptyStringsToNullDirective.php +++ b/src/Schema/Directives/ConvertEmptyStringsToNullDirective.php @@ -16,7 +16,10 @@ public static function definition(): string { return /** @lang GraphQL */ <<<'GRAPHQL' """ -Replaces `""` with `null`. +Replaces incoming empty strings `""` with `null`. + +When used upon fields, empty strings for non-nullable inputs will pass unchanged. +Only explicitly placing this on non-nullable inputs will force the conversion. """ directive @convertEmptyStringsToNull on ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION | FIELD_DEFINITION GRAPHQL; diff --git a/tests/Utils/Mutations/ReturnReceivedInput.php b/tests/Utils/Mutations/ReturnReceivedInput.php index 066fcec383..bf9a5ca0eb 100644 --- a/tests/Utils/Mutations/ReturnReceivedInput.php +++ b/tests/Utils/Mutations/ReturnReceivedInput.php @@ -5,7 +5,7 @@ final class ReturnReceivedInput { /** - * @param array $args + * @param array $args * * @return array */ From 82d02723e2b605bdbe8f1cfbef0574dd4b0859ac Mon Sep 17 00:00:00 2001 From: Dennis Koster Date: Fri, 6 Sep 2024 11:05:02 +0200 Subject: [PATCH 6/6] Apply `@convertEmptyStringsToNull` to input fields when used upon fields --- CHANGELOG.md | 6 ++ .../ConvertEmptyStringsToNullDirective.php | 19 ++-- ...ConvertEmptyStringsToNullDirectiveTest.php | 89 +++++++++++++++++++ 3 files changed, 104 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4376ba6ac2..58a8dcb9cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +## v6.44.2 + +### Fixed + +- Apply `@convertEmptyStringsToNull` to input fields when used upon fields https://github.com/nuwave/lighthouse/issues/2610 + ## v6.44.1 ### Fixed diff --git a/src/Schema/Directives/ConvertEmptyStringsToNullDirective.php b/src/Schema/Directives/ConvertEmptyStringsToNullDirective.php index 4fe6c89406..ea0b9dfd53 100644 --- a/src/Schema/Directives/ConvertEmptyStringsToNullDirective.php +++ b/src/Schema/Directives/ConvertEmptyStringsToNullDirective.php @@ -44,12 +44,13 @@ protected function transformArgumentSet(ArgumentSet $argumentSet): ArgumentSet { foreach ($argumentSet->arguments as $argument) { $namedType = $argument->namedType(); - if ( - $namedType !== null + $argumentValue = $argument->value; + + $isNullableStringType = $namedType !== null && $namedType->name === ScalarType::STRING - && ! $namedType->nonNull - ) { - $argument->value = $this->sanitize($argument->value); + && ! $namedType->nonNull; + if ($isNullableStringType || $argumentValue instanceof ArgumentSet) { + $argument->value = $this->sanitize($argumentValue); } } @@ -63,10 +64,8 @@ protected function transformArgumentSet(ArgumentSet $argumentSet): ArgumentSet */ protected function transformLeaf(mixed $value): mixed { - if ($value === '') { - return null; - } - - return $value; + return $value === '' + ? null + : $value; } } diff --git a/tests/Integration/Schema/Directives/ConvertEmptyStringsToNullDirectiveTest.php b/tests/Integration/Schema/Directives/ConvertEmptyStringsToNullDirectiveTest.php index c37e91e899..62710a7396 100644 --- a/tests/Integration/Schema/Directives/ConvertEmptyStringsToNullDirectiveTest.php +++ b/tests/Integration/Schema/Directives/ConvertEmptyStringsToNullDirectiveTest.php @@ -221,4 +221,93 @@ public function testConvertsEmptyStringToNullWithGlobalFieldMiddleware(): void ], ]); } + + public function testConvertsEmptyStringToNullWithFieldDirectiveAndInputType(): void + { + $this->schema = /** @lang GraphQL */ ' + type Query { + foo(input: FooInput): FooInputResponse + @convertEmptyStringsToNull + @field(resolver: "Tests\\\Utils\\\Mutations\\\ReturnReceivedInput") + } + + input FooInput { + bar: String + } + + type FooInputResponse { + input: FooResponse + } + + type FooResponse { + bar: String + } + '; + + $this->graphQL(/** @lang GraphQL */ ' + { + foo(input: { + bar: "" + }) { + input { + bar + } + } + } + ')->assertExactJson([ + 'data' => [ + 'foo' => [ + 'input' => [ + 'bar' => null, + ], + ], + ], + ]); + } + + public function testConvertsEmptyStringToNullWithGlobalFieldMiddlewareAndInputType(): void + { + config(['lighthouse.field_middleware' => [ + ConvertEmptyStringsToNullDirective::class, + ]]); + + $this->schema = /** @lang GraphQL */ ' + type Query { + foo(input: FooInput): FooInputResponse + @field(resolver: "Tests\\\Utils\\\Mutations\\\ReturnReceivedInput") + } + + input FooInput { + bar: String + } + + type FooInputResponse { + input: FooResponse + } + + type FooResponse { + bar: String + } + '; + + $this->graphQL(/** @lang GraphQL */ ' + { + foo(input: { + bar: "" + }) { + input { + bar + } + } + } + ')->assertExactJson([ + 'data' => [ + 'foo' => [ + 'input' => [ + 'bar' => null, + ], + ], + ], + ]); + } }