Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ambiguity around when schema definition may be omitted #1303

Merged
merged 8 commits into from
Feb 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/Language/Printer.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ public static function doPrint(Node $ast): string
return $instance->printAST($ast);
}

protected function __construct()
{
}
protected function __construct() {}

/**
* Recursively traverse an AST depth-first and produce a pretty string.
Expand Down
63 changes: 30 additions & 33 deletions src/Utils/SchemaPrinter.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,61 +137,58 @@ protected static function printFilteredSchema(Schema $schema, callable $directiv
return \implode("\n\n", \array_filter($elements)) . "\n";
}

protected static function printSchemaDefinition(Schema $schema): string
protected static function printSchemaDefinition(Schema $schema): ?string
{
if (static::isSchemaOfCommonNames($schema)) {
return '';
}

$operationTypes = [];

$queryType = $schema->getQueryType();
if ($queryType !== null) {
$operationTypes[] = " query: {$queryType->name}";
}

$mutationType = $schema->getMutationType();
if ($mutationType !== null) {
$operationTypes[] = " mutation: {$mutationType->name}";
}

$subscriptionType = $schema->getSubscriptionType();
if ($subscriptionType !== null) {
$operationTypes[] = " subscription: {$subscriptionType->name}";

// Special case: When a schema has no root operation types, no valid schema
// definition can be printed.
if ($queryType === null && $mutationType === null && $subscriptionType === null) {
return null;
}

$typesString = \implode("\n", $operationTypes);
// TODO add condition for schema.description
// Only print a schema definition if there is a description or if it should
// not be omitted because of having default type names.
if (! self::hasDefaultRootOperationTypes($schema)) {
return "schema {\n"
. ($queryType !== null ? " query: {$queryType->name}\n" : '')
. ($mutationType !== null ? " mutation: {$mutationType->name}\n" : '')
. ($subscriptionType !== null ? " subscription: {$subscriptionType->name}\n" : '')
. '}';
}

return "schema {\n{$typesString}\n}";
return null;
}

/**
* GraphQL schema define root types for each type of operation. These types are
* the same as any other type and can be named in any manner, however there is
* a common naming convention:.
*
* ```graphql
* schema {
* query: Query
* mutation: Mutation
* subscription: Subscription
* }
* ```
*
* When using this naming convention, the schema description can be omitted.
* When using this naming convention, the schema description can be omitted so
* long as these names are only used for operation types.
*
* Note however that if any of these default names are used elsewhere in the
* schema but not as a root operation type, the schema definition must still
* be printed to avoid ambiguity.
*/
protected static function isSchemaOfCommonNames(Schema $schema): bool
protected static function hasDefaultRootOperationTypes(Schema $schema): bool
{
$queryType = $schema->getQueryType();
if ($queryType !== null && $queryType->name !== 'Query') {
return false;
}

$mutationType = $schema->getMutationType();
if ($mutationType !== null && $mutationType->name !== 'Mutation') {
return false;
}

$subscriptionType = $schema->getSubscriptionType();

return $subscriptionType === null || $subscriptionType->name === 'Subscription';
return $schema->getQueryType() === $schema->getType('Query')
&& $schema->getMutationType() === $schema->getType('Mutation')
&& $schema->getSubscriptionType() === $schema->getType('Subscription');
}

/**
Expand Down
12 changes: 12 additions & 0 deletions tests/Language/SchemaPrinterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use GraphQL\Language\AST\ScalarTypeDefinitionNode;
use GraphQL\Language\Parser;
use GraphQL\Language\Printer;
use GraphQL\Utils\BuildSchema;
use GraphQL\Utils\SchemaPrinter;
use PHPUnit\Framework\TestCase;

use function Safe\file_get_contents;
Expand Down Expand Up @@ -177,4 +179,14 @@ enum UndefinedEnum
$printed
);
}

/**
* it('prints viral schema correctly', () => {.
*/
public function testPrintsViralSchemaCorrectly(): void
{
$schemaSDL = \Safe\file_get_contents(__DIR__ . '/../viralSchema.graphql');
$schema = BuildSchema::build($schemaSDL);
self::assertSame($schemaSDL, SchemaPrinter::doPrint($schema));
}
}
16 changes: 16 additions & 0 deletions tests/Utils/BuildSchemaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,22 @@ public function testThrowsOnUnknownTypes(): void
BuildSchema::build($sdl, null, ['assumeValidSDL' => true])->assertValid();
}

/**
* it('correctly processes viral schema', () => {.
*/
public function testCorrectlyProcessesViralSchema(): void
{
$schema = BuildSchema::build(\Safe\file_get_contents(__DIR__ . '/../viralSchema.graphql'));

$queryType = $schema->getQueryType();
self::assertNotNull($queryType);
self::assertSame('Query', $queryType->name);
self::assertNotNull($schema->getType('Virus'));
self::assertNotNull($schema->getType('Mutation'));
// Though the viral schema has a 'Mutation' type, it is not used for the 'mutation' operation.
self::assertNull($schema->getMutationType());
}

/**
* @see https://github.com/webonyx/graphql-php/issues/997
*/
Expand Down
17 changes: 17 additions & 0 deletions tests/viralSchema.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
schema {
query: Query
}

type Query {
viruses: [Virus!]
}

type Virus {
name: String!
knownMutations: [Mutation!]!
}

type Mutation {
name: String!
geneSequence: String!
}