Exclude unused standard types from the schema (#974)

webonyx · Oct 19, 2021 · d53d688 · d53d688
1 parent 694088f
commit d53d688
Show file tree

Hide file tree

Showing 12 changed files with 1,004 additions and 1,107 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,7 @@ You can find and compare releases at the [GitHub release page](https://github.co
 - Move class `BlockString` from namespace `GraphQL\Utils` to `GraphQL\Language`
 - Return string-keyed arrays from `GraphQL::getStandardDirectives()`, `GraphQL::getStandardTypes()` and `GraphQL::getStandardValidationRules()`
 - Move complexity related code from `FieldDefinition` to `QueryComplexity`
+- Exclude unused standard types from the schema
 
 ### Added
 

diff --git a/src/Type/Introspection.php b/src/Type/Introspection.php
@@ -10,6 +10,7 @@
 use GraphQL\Language\Printer;
 use GraphQL\Type\Definition\Directive;
 use GraphQL\Type\Definition\EnumType;
+use GraphQL\Type\Definition\EnumValueDefinition;
 use GraphQL\Type\Definition\FieldArgument;
 use GraphQL\Type\Definition\FieldDefinition;
 use GraphQL\Type\Definition\InputObjectField;
@@ -329,7 +330,10 @@ public static function _type(): ObjectType
                         'fields'        => [
                             'type'    => Type::listOf(Type::nonNull(self::_field())),
                             'args'    => [
-                                'includeDeprecated' => ['type' => Type::boolean(), 'defaultValue' => false],
+                                'includeDeprecated' => [
+                                    'type' => Type::boolean(),
+                                    'defaultValue' => false,
+                                ],
                             ],
                             'resolve' => static function (Type $type, $args): ?array {
                                 if ($type instanceof ObjectType || $type instanceof InterfaceType) {
@@ -339,7 +343,8 @@ public static function _type(): ObjectType
                                         $fields = array_filter(
                                             $fields,
                                             static function (FieldDefinition $field): bool {
-                                                return ($field->deprecationReason ?? '') === '';
+                                                return $field->deprecationReason === null
+                                                    || $field->deprecationReason === '';
                                             }
                                         );
                                     }
@@ -377,13 +382,14 @@ static function (FieldDefinition $field): bool {
                             ],
                             'resolve' => static function ($type, $args): ?array {
                                 if ($type instanceof EnumType) {
-                                    $values = array_values($type->getValues());
+                                    $values = $type->getValues();
 
                                     if (! ($args['includeDeprecated'] ?? false)) {
-                                        $values = array_filter(
+                                        return array_filter(
                                             $values,
-                                            static function ($value): bool {
-                                                return ($value->deprecationReason ?? '') === '';
+                                            static function (EnumValueDefinition $value): bool {
+                                                return $value->deprecationReason === null
+                                                    || $value->deprecationReason === '';
                                             }
                                         );
                                     }
@@ -499,7 +505,8 @@ public static function _field(): ObjectType
                         'isDeprecated'      => [
                             'type'    => Type::nonNull(Type::boolean()),
                             'resolve' => static function (FieldDefinition $field): bool {
-                                return (bool) $field->deprecationReason;
+                                return $field->deprecationReason !== null
+                                    && $field->deprecationReason !== '';
                             },
                         ],
                         'deprecationReason' => [
@@ -595,14 +602,15 @@ public static function _enumValue(): ObjectType
                 ],
                 'isDeprecated'      => [
                     'type'    => Type::nonNull(Type::boolean()),
-                    'resolve' => static function ($enumValue): bool {
-                            return (bool) $enumValue->deprecationReason;
+                    'resolve' => static function (EnumValueDefinition $value): bool {
+                        return $value->deprecationReason !== null
+                            && $value->deprecationReason !== '';
                     },
                 ],
                 'deprecationReason' => [
                     'type' => Type::string(),
-                    'resolve' => static function ($enumValue) {
-                            return $enumValue->deprecationReason;
+                    'resolve' => static function (EnumValueDefinition $enumValue): ?string {
+                        return $enumValue->deprecationReason;
                     },
                 ],
             ],
@@ -742,7 +750,6 @@ public static function _directiveLocation(): EnumType
                     'value'       => DirectiveLocation::INPUT_FIELD_DEFINITION,
                     'description' => 'Location adjacent to an input object field definition.',
                 ],
-
             ],
         ]);
     }

diff --git a/src/Type/Schema.php b/src/Type/Schema.php
@@ -135,7 +135,7 @@ public function __construct($config)
             );
         }
 
-        $this->resolvedTypes += Type::getStandardTypes() + Introspection::getTypes();
+        $this->resolvedTypes += Introspection::getTypes();
 
         if (isset($this->config->typeLoader)) {
             return;
@@ -296,7 +296,8 @@ public function getConfig(): SchemaConfig
     public function getType(string $name): ?Type
     {
         if (! isset($this->resolvedTypes[$name])) {
-            $type = $this->loadType($name);
+            $type = Type::getStandardTypes()[$name]
+                ?? $this->loadType($name);
 
             if ($type === null) {
                 return null;

diff --git a/src/Utils/BuildClientSchema.php b/src/Utils/BuildClientSchema.php
@@ -55,7 +55,7 @@ class BuildClientSchema
     private array $options;
 
     /** @var array<string, NamedType&Type> */
-    private array $typeMap;
+    private array $typeMap = [];
 
     /**
      * @param array<string, mixed> $introspectionQuery
@@ -101,26 +101,21 @@ public function buildSchema(): Schema
 
         $schemaIntrospection = $this->introspection['__schema'];
 
-        $this->typeMap = Utils::keyValMap(
-            $schemaIntrospection['types'],
-            static function (array $typeIntrospection) {
-                return $typeIntrospection['name'];
-            },
-            function (array $typeIntrospection): NamedType {
-                return $this->buildType($typeIntrospection);
-            }
-        );
-
         $builtInTypes = array_merge(
             Type::getStandardTypes(),
             Introspection::getTypes()
         );
-        foreach ($builtInTypes as $name => $type) {
-            if (! isset($this->typeMap[$name])) {
-                continue;
+
+        foreach ($schemaIntrospection['types'] as $typeIntrospection) {
+            if (! isset($typeIntrospection['name'])) {
+                throw self::invalidOrIncompleteIntrospectionResult($typeIntrospection);
             }
 
-            $this->typeMap[$name] = $type;
+            $name = $typeIntrospection['name'];
+
+            // Use the built-in singleton types to avoid reconstruction
+            $this->typeMap[$name] = $builtInTypes[$name]
+                ?? $this->buildType($typeIntrospection);
         }
 
         $queryType = isset($schemaIntrospection['queryType'])
@@ -142,17 +137,13 @@ function (array $typeIntrospection): NamedType {
             )
             : [];
 
-        $schemaConfig = new SchemaConfig();
-        $schemaConfig->setQuery($queryType)
+        $schemaConfig = (new SchemaConfig())
+            ->setQuery($queryType)
             ->setMutation($mutationType)
             ->setSubscription($subscriptionType)
             ->setTypes($this->typeMap)
             ->setDirectives($directives)
-            ->setAssumeValid(
-                isset($this->options)
-                && isset($this->options['assumeValid'])
-                && $this->options['assumeValid']
-            );
+            ->setAssumeValid($this->options['assumeValid'] ?? false);
 
         return new Schema($schemaConfig);
     }
@@ -204,6 +195,16 @@ private function getNamedType(string $typeName): NamedType
         return $this->typeMap[$typeName];
     }
 
+    /**
+     * @param array<mixed> $type
+     */
+    public static function invalidOrIncompleteIntrospectionResult(array $type): InvariantViolation
+    {
+        return new InvariantViolation(
+            'Invalid or incomplete introspection result. Ensure that a full introspection query is used in order to build a client schema: ' . json_encode($type) . '.'
+        );
+    }
+
     /**
      * @param array<string, mixed> $typeRef
      */
@@ -254,34 +255,39 @@ public function getInterfaceType(array $typeRef): InterfaceType
 
     /**
      * @param array<string, mixed> $type
+     *
+     * @return Type&NamedType
      */
     private function buildType(array $type): NamedType
     {
-        if (array_key_exists('name', $type) && array_key_exists('kind', $type)) {
-            switch ($type['kind']) {
-                case TypeKind::SCALAR:
-                    return $this->buildScalarDef($type);
+        if (! array_key_exists('kind', $type)) {
+            throw self::invalidOrIncompleteIntrospectionResult($type);
+        }
 
-                case TypeKind::OBJECT:
-                    return $this->buildObjectDef($type);
+        switch ($type['kind']) {
+            case TypeKind::SCALAR:
+                return $this->buildScalarDef($type);
 
-                case TypeKind::INTERFACE:
-                    return $this->buildInterfaceDef($type);
+            case TypeKind::OBJECT:
+                return $this->buildObjectDef($type);
 
-                case TypeKind::UNION:
-                    return $this->buildUnionDef($type);
+            case TypeKind::INTERFACE:
+                return $this->buildInterfaceDef($type);
 
-                case TypeKind::ENUM:
-                    return $this->buildEnumDef($type);
+            case TypeKind::UNION:
+                return $this->buildUnionDef($type);
 
-                case TypeKind::INPUT_OBJECT:
-                    return $this->buildInputObjectDef($type);
-            }
-        }
+            case TypeKind::ENUM:
+                return $this->buildEnumDef($type);
 
-        throw new InvariantViolation(
-            'Invalid or incomplete introspection result. Ensure that a full introspection query is used in order to build a client schema: ' . json_encode($type) . '.'
-        );
+            case TypeKind::INPUT_OBJECT:
+                return $this->buildInputObjectDef($type);
+
+            default:
+                throw new InvariantViolation(
+                    'Invalid or incomplete introspection result. Received type with unknown kind: ' . json_encode($type) . '.'
+                );
+        }
     }
 
     /**

diff --git a/src/Utils/TypeInfo.php b/src/Utils/TypeInfo.php
@@ -43,7 +43,6 @@
 use function array_pop;
 use function count;
 use function is_array;
-use function sprintf;
 
 class TypeInfo
 {
@@ -131,7 +130,7 @@ public static function extractTypes(Type $type, array $typeMap = []): array
         if (isset($typeMap[$type->name])) {
             Utils::invariant(
                 $typeMap[$type->name] === $type,
-                sprintf('Schema must contain unique named types but contains multiple types named "%s" ', $type) .
+                'Schema must contain unique named types but contains multiple types named "' . $type . '" ' .
                 '(see https://webonyx.github.io/graphql-php/type-definitions/#type-registry).'
             );
 

diff --git a/tests/Executor/ExecutorLazySchemaTest.php b/tests/Executor/ExecutorLazySchemaTest.php
@@ -28,38 +28,29 @@
 
 class ExecutorLazySchemaTest extends TestCase
 {
-    /** @var ScalarType */
-    public $someScalarType;
+    public ScalarType $someScalarType;
 
-    /** @var ObjectType */
-    public $someObjectType;
+    public ObjectType $someObjectType;
 
-    /** @var ObjectType */
-    public $otherObjectType;
+    public ObjectType $otherObjectType;
 
-    /** @var ObjectType */
-    public $deeperObjectType;
+    public ObjectType $deeperObjectType;
 
-    /** @var UnionType */
-    public $someUnionType;
+    public UnionType $someUnionType;
 
-    /** @var InterfaceType */
-    public $someInterfaceType;
+    public InterfaceType $someInterfaceType;
 
-    /** @var EnumType */
-    public $someEnumType;
+    public EnumType $someEnumType;
 
-    /** @var InputObjectType */
-    public $someInputObjectType;
+    public InputObjectType $someInputObjectType;
 
-    /** @var ObjectType */
-    public $queryType;
+    public ObjectType $queryType;
 
-    /** @var string[] */
-    public $calls = [];
+    /** @var array<int, string> */
+    public array $calls = [];
 
-    /** @var bool[] */
-    public $loadedTypes = [];
+    /** @var array<string, true> */
+    public array $loadedTypes = [];
 
     public function testWarnsAboutSlowIsTypeOfForLazySchema(): void
     {
@@ -372,36 +363,38 @@ public function testDeepQuery(): void
     {
         $schema = new Schema([
             'query'      => $this->loadType('Query'),
-            'typeLoader' => function ($name) {
-                return $this->loadType($name, true);
-            },
+            'typeLoader' => fn (string $name): Type => $this->loadType($name, true),
         ]);
 
-        $query  = '{ object { object { object { string } } } }';
+        $query     = '{ object { object { object { string } } } }';
+        $rootValue = ['object' => ['object' => ['object' => ['string' => 'test']]]];
+
         $result = Executor::execute(
             $schema,
             Parser::parse($query),
-            ['object' => ['object' => ['object' => ['string' => 'test']]]]
+            $rootValue
         );
 
-        $expected            = [
-            'data' => ['object' => ['object' => ['object' => ['string' => 'test']]]],
-        ];
-        $expectedLoadedTypes = [
-            'Query'       => true,
-            'SomeObject'  => true,
-            'OtherObject' => true,
-        ];
-
-        self::assertEquals($expected, $result->toArray(DebugFlag::INCLUDE_DEBUG_MESSAGE));
-        self::assertEquals($expectedLoadedTypes, $this->loadedTypes);
-
-        $expectedExecutorCalls = [
-            'Query.fields',
-            'SomeObject',
-            'SomeObject.fields',
-        ];
-        self::assertEquals($expectedExecutorCalls, $this->calls);
+        self::assertEquals(
+            ['data' => $rootValue],
+            $result->toArray(DebugFlag::INCLUDE_DEBUG_MESSAGE)
+        );
+        self::assertEquals(
+            [
+                'Query'       => true,
+                'SomeObject'  => true,
+                'OtherObject' => true,
+            ],
+            $this->loadedTypes
+        );
+        self::assertEquals(
+            [
+                'Query.fields',
+                'SomeObject',
+                'SomeObject.fields',
+            ],
+            $this->calls
+        );
     }
 
     public function testResolveUnion(): void