From caa1329d45e751c0355481d24bba6c082e930269 Mon Sep 17 00:00:00 2001 From: Fabio Capucci Date: Mon, 23 Aug 2021 15:13:53 +0200 Subject: [PATCH 1/6] support schema without Query type when using federation --- src/Federation/ASTManipulator.php | 12 +++++++++++ tests/Unit/Federation/SchemaBuilderTest.php | 22 +++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/Federation/ASTManipulator.php b/src/Federation/ASTManipulator.php index 87b87e80a9..2035281a14 100644 --- a/src/Federation/ASTManipulator.php +++ b/src/Federation/ASTManipulator.php @@ -2,8 +2,11 @@ namespace Nuwave\Lighthouse\Federation; +use GraphQL\Language\AST\NameNode; +use GraphQL\Language\AST\NodeList; use GraphQL\Language\AST\ObjectTypeDefinitionNode; use GraphQL\Language\Parser; +use Illuminate\Support\Arr; use Nuwave\Lighthouse\Events\ManipulateAST; use Nuwave\Lighthouse\Exceptions\FederationException; use Nuwave\Lighthouse\Schema\AST\DocumentAST; @@ -73,6 +76,15 @@ protected function addEntityUnion(DocumentAST &$documentAST): void protected function addRootFields(DocumentAST &$documentAST): void { + if (! Arr::has($documentAST->types, 'Query')) { + $documentAST->setTypeDefinition(new ObjectTypeDefinitionNode([ + 'name' => new NameNode(['value' => 'Query']), + 'interfaces' => new NodeList([]), + 'directives' => new NodeList([]), + 'fields' => new NodeList([]), + ])); + } + /** @var \GraphQL\Language\AST\ObjectTypeDefinitionNode $queryType */ $queryType = $documentAST->types['Query']; diff --git a/tests/Unit/Federation/SchemaBuilderTest.php b/tests/Unit/Federation/SchemaBuilderTest.php index 912ec2cfc3..b522499883 100644 --- a/tests/Unit/Federation/SchemaBuilderTest.php +++ b/tests/Unit/Federation/SchemaBuilderTest.php @@ -44,6 +44,28 @@ public function testFederatedSchema(): void $this->assertTrue($queryType->hasField('_service')); } + public function testAllowSchemaWithoutQueryType(): void + { + $schema = $this->buildSchema(/** @lang GraphQL */ ' + type Foo @key(fields: "id") { + id: ID! + foo: String! + } + '); + + $this->assertTrue($schema->hasType('_Entity')); + $this->assertTrue($schema->hasType('_Service')); + + $this->assertTrue($schema->hasType('_Any')); + $this->assertTrue($schema->hasType('_FieldSet')); + + $queryType = $schema->getQueryType(); + $this->assertInstanceOf(ObjectType::class, $queryType); + + $this->assertTrue($queryType->hasField('_entities')); + $this->assertTrue($queryType->hasField('_service')); + } + /** * At least one type needs to be defined with the @key directive. * From 92b8b6f966dd8adeb038040d68e3dade4ef88025 Mon Sep 17 00:00:00 2001 From: Fabio Capucci Date: Mon, 23 Aug 2021 15:20:53 +0200 Subject: [PATCH 2/6] updated changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index efd7cbe002..2c88a32912 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 +### Fixed + +- Support schema without `Query` type when using federation https://github.com/nuwave/lighthouse/pull/1925 + ## v5.22.1 ### Fixed From 439612eacd12ad246017e8cc47e906ceff32c57e Mon Sep 17 00:00:00 2001 From: Fabio Capucci Date: Mon, 23 Aug 2021 19:04:08 +0200 Subject: [PATCH 3/6] Remove Query from federation Service sdl when empty --- src/Federation/FederationPrinter.php | 32 +++++++++++++------ .../Federation/FederationSchemaTest.php | 18 +++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/Federation/FederationPrinter.php b/src/Federation/FederationPrinter.php index 5363c774dc..c2846e60b0 100644 --- a/src/Federation/FederationPrinter.php +++ b/src/Federation/FederationPrinter.php @@ -54,16 +54,28 @@ public static function print(Schema $schema): string } $originalQueryType = Arr::pull($types, 'Query'); - $config->setQuery(new ObjectType([ - 'name' => 'Query', - 'fields' => array_filter( - $originalQueryType->getFields(), - static function (FieldDefinition $field): bool { - return ! in_array($field->name, static::FEDERATION_FIELDS); - } - ), - 'interfaces' => $originalQueryType->getInterfaces(), - ])); + + $newQueryFields = array_filter( + $originalQueryType->getFields(), + static function (FieldDefinition $field): bool { + return ! in_array($field->name, static::FEDERATION_FIELDS); + } + ); + + if (count($newQueryFields ?? []) === 0) { + $config->setQuery(null); + } else { + $config->setQuery(new ObjectType([ + 'name' => 'Query', + 'fields' => array_filter( + $originalQueryType->getFields(), + static function (FieldDefinition $field): bool { + return ! in_array($field->name, static::FEDERATION_FIELDS); + } + ), + 'interfaces' => $originalQueryType->getInterfaces(), + ])); + } $config->setMutation(Arr::pull($types, 'Mutation')); diff --git a/tests/Integration/Federation/FederationSchemaTest.php b/tests/Integration/Federation/FederationSchemaTest.php index 5bba1d6a24..4c9139352d 100644 --- a/tests/Integration/Federation/FederationSchemaTest.php +++ b/tests/Integration/Federation/FederationSchemaTest.php @@ -41,6 +41,24 @@ public function testServiceQueryShouldReturnValidSdl(): void $this->assertStringContainsString($query, $sdl); } + public function testServiceQueryShouldReturnValidSdlWithoutQuery(): void + { + $foo = /** @lang GraphQL */ <<<'GRAPHQL' +type Foo @key(fields: "id") { + id: ID! @external + foo: String! +} + +GRAPHQL; + + $this->schema = $foo; + + $sdl = $this->_serviceSdl(); + + $this->assertStringContainsString($foo, $sdl); + $this->assertStringNotContainsString('type Query', $sdl); + } + public function testFederatedSchemaShouldContainCorrectEntityUnion(): void { $schema = $this->buildSchema(/** @lang GraphQL */ ' From c52e473308c4fcae841e5bf822f70174ffbb6e3b Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 24 Aug 2021 09:06:54 +0200 Subject: [PATCH 4/6] DRY --- src/Console/MutationCommand.php | 4 ++- src/Console/QueryCommand.php | 4 ++- src/Console/SubscriptionCommand.php | 4 ++- src/Federation/ASTManipulator.php | 17 ++++------- src/Federation/FederationPrinter.php | 32 +++++++++------------ tests/Unit/Federation/SchemaBuilderTest.php | 19 +++++------- 6 files changed, 35 insertions(+), 45 deletions(-) diff --git a/src/Console/MutationCommand.php b/src/Console/MutationCommand.php index 5342d5a569..32ab865125 100644 --- a/src/Console/MutationCommand.php +++ b/src/Console/MutationCommand.php @@ -2,13 +2,15 @@ namespace Nuwave\Lighthouse\Console; +use Nuwave\Lighthouse\Schema\RootType; + class MutationCommand extends FieldGeneratorCommand { protected $name = 'lighthouse:mutation'; protected $description = 'Create a class for a single field on the root Mutation type.'; - protected $type = 'Mutation'; + protected $type = RootType::MUTATION; protected function namespaceConfigKey(): string { diff --git a/src/Console/QueryCommand.php b/src/Console/QueryCommand.php index fb2c8d00e2..2a6932e3e3 100644 --- a/src/Console/QueryCommand.php +++ b/src/Console/QueryCommand.php @@ -2,13 +2,15 @@ namespace Nuwave\Lighthouse\Console; +use Nuwave\Lighthouse\Schema\RootType; + class QueryCommand extends FieldGeneratorCommand { protected $name = 'lighthouse:query'; protected $description = 'Create a class for a single field on the root Query type.'; - protected $type = 'Query'; + protected $type = RootType::QUERY; protected function namespaceConfigKey(): string { diff --git a/src/Console/SubscriptionCommand.php b/src/Console/SubscriptionCommand.php index 0ea5eb6a4f..0a5da0d2f3 100644 --- a/src/Console/SubscriptionCommand.php +++ b/src/Console/SubscriptionCommand.php @@ -2,13 +2,15 @@ namespace Nuwave\Lighthouse\Console; +use Nuwave\Lighthouse\Schema\RootType; + class SubscriptionCommand extends LighthouseGeneratorCommand { protected $name = 'lighthouse:subscription'; protected $description = 'Create a class for a single field on the root Subscription type.'; - protected $type = 'Subscription'; + protected $type = RootType::SUBSCRIPTION; protected function namespaceConfigKey(): string { diff --git a/src/Federation/ASTManipulator.php b/src/Federation/ASTManipulator.php index 2035281a14..f6120b1bc4 100644 --- a/src/Federation/ASTManipulator.php +++ b/src/Federation/ASTManipulator.php @@ -2,14 +2,12 @@ namespace Nuwave\Lighthouse\Federation; -use GraphQL\Language\AST\NameNode; -use GraphQL\Language\AST\NodeList; use GraphQL\Language\AST\ObjectTypeDefinitionNode; use GraphQL\Language\Parser; -use Illuminate\Support\Arr; use Nuwave\Lighthouse\Events\ManipulateAST; use Nuwave\Lighthouse\Exceptions\FederationException; use Nuwave\Lighthouse\Schema\AST\DocumentAST; +use Nuwave\Lighthouse\Schema\RootType; class ASTManipulator { @@ -76,17 +74,14 @@ protected function addEntityUnion(DocumentAST &$documentAST): void protected function addRootFields(DocumentAST &$documentAST): void { - if (! Arr::has($documentAST->types, 'Query')) { - $documentAST->setTypeDefinition(new ObjectTypeDefinitionNode([ - 'name' => new NameNode(['value' => 'Query']), - 'interfaces' => new NodeList([]), - 'directives' => new NodeList([]), - 'fields' => new NodeList([]), - ])); + // In federation it is fine for a schema to not have a user-defined root query type, + // since we add two federation related fields to it here. + if (! isset($documentAST->types[RootType::QUERY])) { + $documentAST->types[RootType::QUERY] = Parser::objectTypeDefinition(/** @lang GraphQL */ 'type Query'); } /** @var \GraphQL\Language\AST\ObjectTypeDefinitionNode $queryType */ - $queryType = $documentAST->types['Query']; + $queryType = $documentAST->types[RootType::QUERY]; $queryType->fields [] = Parser::fieldDefinition(/** @lang GraphQL */ ' _entities( diff --git a/src/Federation/FederationPrinter.php b/src/Federation/FederationPrinter.php index c2846e60b0..7641ab93d9 100644 --- a/src/Federation/FederationPrinter.php +++ b/src/Federation/FederationPrinter.php @@ -21,6 +21,7 @@ use Nuwave\Lighthouse\Federation\Directives\KeyDirective; use Nuwave\Lighthouse\Federation\Directives\ProvidesDirective; use Nuwave\Lighthouse\Federation\Directives\RequiresDirective; +use Nuwave\Lighthouse\Schema\RootType; class FederationPrinter { @@ -53,33 +54,26 @@ public static function print(Schema $schema): string unset($types[$type]); } - $originalQueryType = Arr::pull($types, 'Query'); - - $newQueryFields = array_filter( + /** @var \GraphQL\Type\Definition\ObjectType $originalQueryType */ + $originalQueryType = Arr::pull($types, RootType::QUERY); + $queryFieldsWithoutFederation = array_filter( $originalQueryType->getFields(), static function (FieldDefinition $field): bool { return ! in_array($field->name, static::FEDERATION_FIELDS); } ); - - if (count($newQueryFields ?? []) === 0) { - $config->setQuery(null); - } else { - $config->setQuery(new ObjectType([ - 'name' => 'Query', - 'fields' => array_filter( - $originalQueryType->getFields(), - static function (FieldDefinition $field): bool { - return ! in_array($field->name, static::FEDERATION_FIELDS); - } - ), + $newQueryType = count($queryFieldsWithoutFederation) > 0 + ? new ObjectType([ + 'name' => RootType::QUERY, + 'fields' => $queryFieldsWithoutFederation, 'interfaces' => $originalQueryType->getInterfaces(), - ])); - } + ]) + : null; + $config->setQuery($newQueryType); - $config->setMutation(Arr::pull($types, 'Mutation')); + $config->setMutation(Arr::pull($types, RootType::MUTATION)); - $config->setSubscription(Arr::pull($types, 'Subscription')); + $config->setSubscription(Arr::pull($types, RootType::SUBSCRIPTION)); $config->setTypes($types); diff --git a/tests/Unit/Federation/SchemaBuilderTest.php b/tests/Unit/Federation/SchemaBuilderTest.php index b522499883..d0346b2a2e 100644 --- a/tests/Unit/Federation/SchemaBuilderTest.php +++ b/tests/Unit/Federation/SchemaBuilderTest.php @@ -2,8 +2,8 @@ namespace Tests\Unit\Federation; -use GraphQL\Type\Definition\Directive; use GraphQL\Type\Definition\ObjectType; +use GraphQL\Type\Schema; use Nuwave\Lighthouse\Exceptions\FederationException; use Nuwave\Lighthouse\Federation\FederationServiceProvider; use Tests\TestCase; @@ -37,14 +37,10 @@ public function testFederatedSchema(): void $this->assertTrue($schema->hasType('_Any')); $this->assertTrue($schema->hasType('_FieldSet')); - $queryType = $schema->getQueryType(); - $this->assertInstanceOf(ObjectType::class, $queryType); - - $this->assertTrue($queryType->hasField('_entities')); - $this->assertTrue($queryType->hasField('_service')); + $this->assertSchemaHasQueryTypeWithFederationFields($schema); } - public function testAllowSchemaWithoutQueryType(): void + public function testAddsQueryTypeIfNotDefined(): void { $schema = $this->buildSchema(/** @lang GraphQL */ ' type Foo @key(fields: "id") { @@ -53,12 +49,11 @@ public function testAllowSchemaWithoutQueryType(): void } '); - $this->assertTrue($schema->hasType('_Entity')); - $this->assertTrue($schema->hasType('_Service')); - - $this->assertTrue($schema->hasType('_Any')); - $this->assertTrue($schema->hasType('_FieldSet')); + $this->assertSchemaHasQueryTypeWithFederationFields($schema); + } + protected function assertSchemaHasQueryTypeWithFederationFields(Schema $schema): void + { $queryType = $schema->getQueryType(); $this->assertInstanceOf(ObjectType::class, $queryType); From 9aa0fbc02b18edbc15f110c295a583d441d2febd Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 24 Aug 2021 09:08:48 +0200 Subject: [PATCH 5/6] @lang --- tests/Integration/Federation/FederationSchemaTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/Federation/FederationSchemaTest.php b/tests/Integration/Federation/FederationSchemaTest.php index 4c9139352d..59c94b0913 100644 --- a/tests/Integration/Federation/FederationSchemaTest.php +++ b/tests/Integration/Federation/FederationSchemaTest.php @@ -56,7 +56,7 @@ public function testServiceQueryShouldReturnValidSdlWithoutQuery(): void $sdl = $this->_serviceSdl(); $this->assertStringContainsString($foo, $sdl); - $this->assertStringNotContainsString('type Query', $sdl); + $this->assertStringNotContainsString(/** @lang GraphQL */ 'type Query', $sdl); } public function testFederatedSchemaShouldContainCorrectEntityUnion(): void From 0d2094ff3f450e53fb9dcd01dfcd9ecd73144474 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 24 Aug 2021 09:12:01 +0200 Subject: [PATCH 6/6] v5.22.2 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c88a32912..64db0ba20f 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 +## v5.22.2 + ### Fixed - Support schema without `Query` type when using federation https://github.com/nuwave/lighthouse/pull/1925