From 8cc0764c2b4331b02650a2f9c30fe2a8cb46b6be Mon Sep 17 00:00:00 2001 From: Aleksei Lebedev <1329824+LastDragon-ru@users.noreply.github.com> Date: Mon, 19 Feb 2024 10:13:49 +0400 Subject: [PATCH 1/6] Directives from Enum extensions support. --- src/Schema/AST/ASTBuilder.php | 1 + tests/Unit/Schema/AST/ASTBuilderTest.php | 32 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/Schema/AST/ASTBuilder.php b/src/Schema/AST/ASTBuilder.php index 61bde930f4..297aeba077 100644 --- a/src/Schema/AST/ASTBuilder.php +++ b/src/Schema/AST/ASTBuilder.php @@ -174,6 +174,7 @@ protected function extendEnumType(string $typeName, EnumTypeExtensionNode $typeE $this->assertExtensionMatchesDefinition($typeExtension, $extendedEnum); + $extendedEnum->directives = $extendedEnum->directives->merge($typeExtension->directives); $extendedEnum->values = ASTHelper::mergeUniqueNodeList( $extendedEnum->values, $typeExtension->values, diff --git a/tests/Unit/Schema/AST/ASTBuilderTest.php b/tests/Unit/Schema/AST/ASTBuilderTest.php index b897d9b337..2cf5574e53 100644 --- a/tests/Unit/Schema/AST/ASTBuilderTest.php +++ b/tests/Unit/Schema/AST/ASTBuilderTest.php @@ -12,6 +12,8 @@ use Nuwave\Lighthouse\Exceptions\DefinitionException; use Nuwave\Lighthouse\Schema\AST\ASTBuilder; use Nuwave\Lighthouse\Schema\AST\ASTHelper; +use Nuwave\Lighthouse\Schema\DirectiveLocator; +use Nuwave\Lighthouse\Schema\Directives\BaseDirective; use Nuwave\Lighthouse\Schema\RootType; use Tests\TestCase; @@ -152,6 +154,36 @@ enum MyEnum { $this->assertCount(4, $myEnum->values); } + public function testMergeEnumExtensionDirectives(): void + { + $directive = new class() extends BaseDirective { + public static function definition(): string + { + return /** @lang GraphQL */ 'directive @foo on ENUM'; + } + }; + + $directiveLocator = $this->app->make(DirectiveLocator::class); + $directiveLocator->setResolved('foo', $directive::class); + + $this->schema = /** @lang GraphQL */ ' + enum MyEnum { + ONE + TWO + } + + extend enum MyEnum @foo + + extend enum MyEnum @foo + '; + $documentAST = $this->astBuilder->documentAST(); + + $myEnum = $documentAST->types['MyEnum']; + assert($myEnum instanceof EnumTypeDefinitionNode); + + $this->assertCount(2, $myEnum->directives); + } + public function testMergeUnionExtensionFields(): void { $this->schema = /** @lang GraphQL */ ' From ea720b542a9bbe84f52a8558b2ca69d2634358b4 Mon Sep 17 00:00:00 2001 From: Aleksei Lebedev <1329824+LastDragon-ru@users.noreply.github.com> Date: Mon, 19 Feb 2024 10:20:27 +0400 Subject: [PATCH 2/6] Directives from Scalar extensions support. --- src/Schema/AST/ASTBuilder.php | 20 ++++++++++++++-- tests/Unit/Schema/AST/ASTBuilderTest.php | 30 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/Schema/AST/ASTBuilder.php b/src/Schema/AST/ASTBuilder.php index 297aeba077..db38ef8e93 100644 --- a/src/Schema/AST/ASTBuilder.php +++ b/src/Schema/AST/ASTBuilder.php @@ -10,6 +10,8 @@ use GraphQL\Language\AST\InterfaceTypeExtensionNode; use GraphQL\Language\AST\ObjectTypeDefinitionNode; use GraphQL\Language\AST\ObjectTypeExtensionNode; +use GraphQL\Language\AST\ScalarTypeDefinitionNode; +use GraphQL\Language\AST\ScalarTypeExtensionNode; use GraphQL\Language\AST\UnionTypeDefinitionNode; use GraphQL\Language\AST\UnionTypeExtensionNode; use GraphQL\Language\Parser; @@ -33,6 +35,7 @@ class ASTBuilder ObjectTypeExtensionNode::class => ObjectTypeDefinitionNode::class, InputObjectTypeExtensionNode::class => InputObjectTypeDefinitionNode::class, InterfaceTypeExtensionNode::class => InterfaceTypeDefinitionNode::class, + ScalarTypeExtensionNode::class => ScalarTypeDefinitionNode::class, EnumTypeExtensionNode::class => EnumTypeDefinitionNode::class, UnionTypeExtensionNode::class => UnionTypeDefinitionNode::class, ]; @@ -122,6 +125,8 @@ protected function applyTypeExtensionManipulators(): void || $typeExtension instanceof InterfaceTypeExtensionNode ) { $this->extendObjectLikeType($typeName, $typeExtension); + } elseif ($typeExtension instanceof ScalarTypeExtensionNode) { + $this->extendScalarType($typeName, $typeExtension); } elseif ($typeExtension instanceof EnumTypeExtensionNode) { $this->extendEnumType($typeName, $typeExtension); } elseif ($typeExtension instanceof UnionTypeExtensionNode) { @@ -166,6 +171,17 @@ protected function extendObjectLikeType(string $typeName, ObjectTypeExtensionNod } } + protected function extendScalarType(string $typeName, ScalarTypeExtensionNode $typeExtension): void + { + $extendedScalar = $this->documentAST->types[$typeName] + ?? throw new DefinitionException($this->missingBaseDefinition($typeName, $typeExtension)); + assert($extendedScalar instanceof ScalarTypeDefinitionNode); + + $this->assertExtensionMatchesDefinition($typeExtension, $extendedScalar); + + $extendedScalar->directives = $extendedScalar->directives->merge($typeExtension->directives); + } + protected function extendEnumType(string $typeName, EnumTypeExtensionNode $typeExtension): void { $extendedEnum = $this->documentAST->types[$typeName] @@ -195,12 +211,12 @@ protected function extendUnionType(string $typeName, UnionTypeExtensionNode $typ ); } - protected function missingBaseDefinition(string $typeName, ObjectTypeExtensionNode|InputObjectTypeExtensionNode|InterfaceTypeExtensionNode|EnumTypeExtensionNode|UnionTypeExtensionNode $typeExtension): string + protected function missingBaseDefinition(string $typeName, ObjectTypeExtensionNode|InputObjectTypeExtensionNode|InterfaceTypeExtensionNode|ScalarTypeExtensionNode|EnumTypeExtensionNode|UnionTypeExtensionNode $typeExtension): string { return "Could not find a base definition {$typeName} of kind {$typeExtension->kind} to extend."; } - protected function assertExtensionMatchesDefinition(ObjectTypeExtensionNode|InputObjectTypeExtensionNode|InterfaceTypeExtensionNode|EnumTypeExtensionNode|UnionTypeExtensionNode $extension, ObjectTypeDefinitionNode|InputObjectTypeDefinitionNode|InterfaceTypeDefinitionNode|EnumTypeDefinitionNode|UnionTypeDefinitionNode $definition): void + protected function assertExtensionMatchesDefinition(ObjectTypeExtensionNode|InputObjectTypeExtensionNode|InterfaceTypeExtensionNode|ScalarTypeExtensionNode|EnumTypeExtensionNode|UnionTypeExtensionNode $extension, ObjectTypeDefinitionNode|InputObjectTypeDefinitionNode|InterfaceTypeDefinitionNode|ScalarTypeDefinitionNode|EnumTypeDefinitionNode|UnionTypeDefinitionNode $definition): void { if (static::EXTENSION_TO_DEFINITION_CLASS[$extension::class] !== $definition::class) { throw new DefinitionException("The type extension {$extension->name->value} of kind {$extension->kind} can not extend a definition of kind {$definition->kind}."); diff --git a/tests/Unit/Schema/AST/ASTBuilderTest.php b/tests/Unit/Schema/AST/ASTBuilderTest.php index 2cf5574e53..5946937057 100644 --- a/tests/Unit/Schema/AST/ASTBuilderTest.php +++ b/tests/Unit/Schema/AST/ASTBuilderTest.php @@ -7,6 +7,7 @@ use GraphQL\Language\AST\InterfaceTypeDefinitionNode; use GraphQL\Language\AST\NodeKind; use GraphQL\Language\AST\ObjectTypeDefinitionNode; +use GraphQL\Language\AST\ScalarTypeDefinitionNode; use GraphQL\Language\AST\UnionTypeDefinitionNode; use Illuminate\Support\Collection; use Nuwave\Lighthouse\Exceptions\DefinitionException; @@ -17,6 +18,8 @@ use Nuwave\Lighthouse\Schema\RootType; use Tests\TestCase; +use function assert; + final class ASTBuilderTest extends TestCase { protected ASTBuilder $astBuilder; @@ -130,6 +133,33 @@ interface Named { $this->assertCount(3, $named->fields); } + public function testMergeScalarExtensionDirectives(): void + { + $directive = new class() extends BaseDirective { + public static function definition(): string + { + return /** @lang GraphQL */ 'directive @foo on SCALAR'; + } + }; + + $directiveLocator = $this->app->make(DirectiveLocator::class); + $directiveLocator->setResolved('foo', $directive::class); + + $this->schema = /** @lang GraphQL */ ' + scalar MyScalar + + extend scalar MyScalar @foo + + extend scalar MyScalar @foo + '; + $documentAST = $this->astBuilder->documentAST(); + + $myScalar = $documentAST->types['MyScalar']; + assert($myScalar instanceof ScalarTypeDefinitionNode); + + $this->assertCount(2, $myScalar->directives); + } + public function testMergeEnumExtensionFields(): void { $this->schema = /** @lang GraphQL */ ' From 83282ad2410eededf00fad1517ebdf4dcd650173 Mon Sep 17 00:00:00 2001 From: Aleksei Lebedev <1329824+LastDragon-ru@users.noreply.github.com> Date: Mon, 19 Feb 2024 10:31:23 +0400 Subject: [PATCH 3/6] Directives from Type/Interface/Input extensions support. --- src/Schema/AST/ASTBuilder.php | 1 + tests/Unit/Schema/AST/ASTBuilderTest.php | 87 ++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/src/Schema/AST/ASTBuilder.php b/src/Schema/AST/ASTBuilder.php index db38ef8e93..36cc40e4d3 100644 --- a/src/Schema/AST/ASTBuilder.php +++ b/src/Schema/AST/ASTBuilder.php @@ -161,6 +161,7 @@ protected function extendObjectLikeType(string $typeName, ObjectTypeExtensionNod // @phpstan-ignore-next-line $typeExtension->fields, ); + $extendedObjectLikeType->directives = $extendedObjectLikeType->directives->merge($typeExtension->directives); if ($extendedObjectLikeType instanceof ObjectTypeDefinitionNode) { assert($typeExtension instanceof ObjectTypeExtensionNode, 'We know this because we passed assertExtensionMatchesDefinition().'); diff --git a/tests/Unit/Schema/AST/ASTBuilderTest.php b/tests/Unit/Schema/AST/ASTBuilderTest.php index 5946937057..401c2d96af 100644 --- a/tests/Unit/Schema/AST/ASTBuilderTest.php +++ b/tests/Unit/Schema/AST/ASTBuilderTest.php @@ -54,6 +54,35 @@ public function testMergeTypeExtensionFields(): void $this->assertCount(3, $queryType->fields); } + public function testMergeTypeExtensionDirectives(): void + { + $directive = new class() extends BaseDirective { + public static function definition(): string + { + return /** @lang GraphQL */ 'directive @foo on OBJECT'; + } + }; + + $directiveLocator = $this->app->make(DirectiveLocator::class); + $directiveLocator->setResolved('foo', $directive::class); + + $this->schema = /** @lang GraphQL */ ' + type MyType { + field: String + } + + extend type MyType @foo + + extend type MyType @foo + '; + $documentAST = $this->astBuilder->documentAST(); + + $myType = $documentAST->types['MyType']; + assert($myType instanceof ObjectTypeDefinitionNode); + + $this->assertCount(2, $myType->directives); + } + public function testAllowsExtendingUndefinedRootTypes(): void { $this->schema = /** @lang GraphQL */ ' @@ -110,6 +139,35 @@ public function testMergeInputExtensionFields(): void $this->assertCount(3, $inputs->fields); } + public function testMergeInputExtensionDirectives(): void + { + $directive = new class() extends BaseDirective { + public static function definition(): string + { + return /** @lang GraphQL */ 'directive @foo on INPUT_OBJECT'; + } + }; + + $directiveLocator = $this->app->make(DirectiveLocator::class); + $directiveLocator->setResolved('foo', $directive::class); + + $this->schema = /** @lang GraphQL */ ' + input MyInput { + field: String + } + + extend input MyInput @foo + + extend input MyInput @foo + '; + $documentAST = $this->astBuilder->documentAST(); + + $myInput = $documentAST->types['MyInput']; + assert($myInput instanceof InputObjectTypeDefinitionNode); + + $this->assertCount(2, $myInput->directives); + } + public function testMergeInterfaceExtensionFields(): void { $this->schema = /** @lang GraphQL */ ' @@ -133,6 +191,35 @@ interface Named { $this->assertCount(3, $named->fields); } + public function testMergeInterfaceExtensionDirectives(): void + { + $directive = new class() extends BaseDirective { + public static function definition(): string + { + return /** @lang GraphQL */ 'directive @foo on INTERFACE'; + } + }; + + $directiveLocator = $this->app->make(DirectiveLocator::class); + $directiveLocator->setResolved('foo', $directive::class); + + $this->schema = /** @lang GraphQL */ ' + interface MyInterface { + field: String + } + + extend interface MyInterface @foo + + extend interface MyInterface @foo + '; + $documentAST = $this->astBuilder->documentAST(); + + $myInterface = $documentAST->types['MyInterface']; + assert($myInterface instanceof InterfaceTypeDefinitionNode); + + $this->assertCount(2, $myInterface->directives); + } + public function testMergeScalarExtensionDirectives(): void { $directive = new class() extends BaseDirective { From b2e2a89512667d8324fadaa675be3210fb055879 Mon Sep 17 00:00:00 2001 From: Aleksei Lebedev <1329824+LastDragon-ru@users.noreply.github.com> Date: Tue, 20 Feb 2024 11:11:01 +0400 Subject: [PATCH 4/6] Undefined scalar test. --- tests/Unit/Schema/AST/ASTBuilderTest.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/Unit/Schema/AST/ASTBuilderTest.php b/tests/Unit/Schema/AST/ASTBuilderTest.php index 401c2d96af..f8a6f9070e 100644 --- a/tests/Unit/Schema/AST/ASTBuilderTest.php +++ b/tests/Unit/Schema/AST/ASTBuilderTest.php @@ -322,6 +322,26 @@ public function testMergeUnionExtensionFields(): void $this->assertCount(3, $myUnion->types); } + public function testDoesNotAllowExtendingUndefinedScalar(): void + { + $directive = new class() extends BaseDirective { + public static function definition(): string + { + return /** @lang GraphQL */ 'directive @foo on SCALAR'; + } + }; + + $directiveLocator = $this->app->make(DirectiveLocator::class); + $directiveLocator->setResolved('foo', $directive::class); + + $this->schema = /** @lang GraphQL */ ' + extend scalar MyScalar @foo + '; + + $this->expectExceptionObject(new DefinitionException('Could not find a base definition MyScalar of kind ' . NodeKind::SCALAR_TYPE_EXTENSION . ' to extend.')); + $this->astBuilder->documentAST(); + } + public function testDoesNotAllowExtendingUndefinedTypes(): void { $this->schema = /** @lang GraphQL */ ' From b8dd562100a031ececdab52a57b8ef0f4b1c6a17 Mon Sep 17 00:00:00 2001 From: Aleksei Lebedev <1329824+LastDragon-ru@users.noreply.github.com> Date: Mon, 19 Feb 2024 10:44:45 +0400 Subject: [PATCH 5/6] CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b2e54b487..ee9bfb7244 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +- Merge directives from Scalar/Enum/Type/Input/Interface extension node into target node. + ## v6.33.4 ### Fixed From 001126d0ddafdd4999c447bfd75fcbe754cc24ea Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 1 Mar 2024 11:22:02 +0100 Subject: [PATCH 6/6] Apply suggestions from code review --- CHANGELOG.md | 4 +++- src/Schema/AST/ASTBuilder.php | 5 ++++- tests/Unit/Schema/AST/ASTBuilderTest.php | 14 ++++++-------- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee9bfb7244..7773137325 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,9 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased -- Merge directives from Scalar/Enum/Type/Input/Interface extension node into target node. +### Added + +- Merge directives from Scalar/Enum/Type/Input/Interface extension node into target node https://github.com/nuwave/lighthouse/pull/2512 ## v6.33.4 diff --git a/src/Schema/AST/ASTBuilder.php b/src/Schema/AST/ASTBuilder.php index 36cc40e4d3..8480686169 100644 --- a/src/Schema/AST/ASTBuilder.php +++ b/src/Schema/AST/ASTBuilder.php @@ -217,7 +217,10 @@ protected function missingBaseDefinition(string $typeName, ObjectTypeExtensionNo return "Could not find a base definition {$typeName} of kind {$typeExtension->kind} to extend."; } - protected function assertExtensionMatchesDefinition(ObjectTypeExtensionNode|InputObjectTypeExtensionNode|InterfaceTypeExtensionNode|ScalarTypeExtensionNode|EnumTypeExtensionNode|UnionTypeExtensionNode $extension, ObjectTypeDefinitionNode|InputObjectTypeDefinitionNode|InterfaceTypeDefinitionNode|ScalarTypeDefinitionNode|EnumTypeDefinitionNode|UnionTypeDefinitionNode $definition): void + protected function assertExtensionMatchesDefinition( + ObjectTypeExtensionNode|InputObjectTypeExtensionNode|InterfaceTypeExtensionNode|ScalarTypeExtensionNode|EnumTypeExtensionNode|UnionTypeExtensionNode $extension, + ObjectTypeDefinitionNode|InputObjectTypeDefinitionNode|InterfaceTypeDefinitionNode|ScalarTypeDefinitionNode|EnumTypeDefinitionNode|UnionTypeDefinitionNode $definition, + ): void { if (static::EXTENSION_TO_DEFINITION_CLASS[$extension::class] !== $definition::class) { throw new DefinitionException("The type extension {$extension->name->value} of kind {$extension->kind} can not extend a definition of kind {$definition->kind}."); diff --git a/tests/Unit/Schema/AST/ASTBuilderTest.php b/tests/Unit/Schema/AST/ASTBuilderTest.php index f8a6f9070e..046774bc0d 100644 --- a/tests/Unit/Schema/AST/ASTBuilderTest.php +++ b/tests/Unit/Schema/AST/ASTBuilderTest.php @@ -18,8 +18,6 @@ use Nuwave\Lighthouse\Schema\RootType; use Tests\TestCase; -use function assert; - final class ASTBuilderTest extends TestCase { protected ASTBuilder $astBuilder; @@ -59,7 +57,7 @@ public function testMergeTypeExtensionDirectives(): void $directive = new class() extends BaseDirective { public static function definition(): string { - return /** @lang GraphQL */ 'directive @foo on OBJECT'; + return /** @lang GraphQL */ 'directive @foo repeatable on OBJECT'; } }; @@ -144,7 +142,7 @@ public function testMergeInputExtensionDirectives(): void $directive = new class() extends BaseDirective { public static function definition(): string { - return /** @lang GraphQL */ 'directive @foo on INPUT_OBJECT'; + return /** @lang GraphQL */ 'directive @foo repeatable on INPUT_OBJECT'; } }; @@ -196,7 +194,7 @@ public function testMergeInterfaceExtensionDirectives(): void $directive = new class() extends BaseDirective { public static function definition(): string { - return /** @lang GraphQL */ 'directive @foo on INTERFACE'; + return /** @lang GraphQL */ 'directive @foo repeatable on INTERFACE'; } }; @@ -225,7 +223,7 @@ public function testMergeScalarExtensionDirectives(): void $directive = new class() extends BaseDirective { public static function definition(): string { - return /** @lang GraphQL */ 'directive @foo on SCALAR'; + return /** @lang GraphQL */ 'directive @foo repeatable on SCALAR'; } }; @@ -276,7 +274,7 @@ public function testMergeEnumExtensionDirectives(): void $directive = new class() extends BaseDirective { public static function definition(): string { - return /** @lang GraphQL */ 'directive @foo on ENUM'; + return /** @lang GraphQL */ 'directive @foo repeatable on ENUM'; } }; @@ -327,7 +325,7 @@ public function testDoesNotAllowExtendingUndefinedScalar(): void $directive = new class() extends BaseDirective { public static function definition(): string { - return /** @lang GraphQL */ 'directive @foo on SCALAR'; + return /** @lang GraphQL */ 'directive @foo repeatable on SCALAR'; } };