Merge pull request #413 from safrpa/schema-validation-improvements

check if container contains given types, schema types unique
graphpql · Feb 21, 2025 · e5b161a · e5b161a
2 parents f867dc2 + 31db406
commit e5b161a
Show file tree

Hide file tree

Showing 13 changed files with 183 additions and 12 deletions.
diff --git a/src/SimpleContainer.php b/src/SimpleContainer.php
@@ -15,6 +15,8 @@
 use Graphpinator\Typesystem\Container;
 use Graphpinator\Typesystem\Contract\NamedType;
 use Graphpinator\Typesystem\Directive as TypesystemDirective;
+use Graphpinator\Typesystem\Exception\DirectiveNamesNotUnique;
+use Graphpinator\Typesystem\Exception\TypeNamesNotUnique;
 
 /**
  * Simple Container implementation
@@ -60,12 +62,26 @@ public function __construct(
             'oneOf' => self::directiveOneOf(),
         ];
 
+        $typeCount = 0;
+
         foreach ($types as $type) {
             $this->types[$type->getName()] = $type;
+            ++$typeCount;
+        }
+
+        if (Graphpinator::$validateSchema && $typeCount !== \count($this->types)) {
+            throw new TypeNamesNotUnique();
         }
 
+        $directivesCount = 0;
+
         foreach ($directives as $directive) {
             $this->directives[$directive->getName()] = $directive;
+            ++$directivesCount;
+        }
+
+        if (Graphpinator::$validateSchema && $directivesCount !== \count($this->directives)) {
+            throw new DirectiveNamesNotUnique();
         }
 
         $this->combinedTypes = \array_merge($this->types, self::$builtInTypes);

diff --git a/src/Typesystem/Exception/DirectiveNamesNotUnique.php b/src/Typesystem/Exception/DirectiveNamesNotUnique.php
@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Graphpinator\Typesystem\Exception;
+
+final class DirectiveNamesNotUnique extends TypeError
+{
+    public const MESSAGE = 'Directive names are not unique.';
+}
diff --git a/src/Typesystem/Exception/RootOperationTypesMustBeWithinContainer.php b/src/Typesystem/Exception/RootOperationTypesMustBeWithinContainer.php
@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Graphpinator\Typesystem\Exception;
+
+final class RootOperationTypesMustBeWithinContainer extends TypeError
+{
+    public const MESSAGE = 'Root operation types must be within container.';
+}
diff --git a/src/Typesystem/Exception/TypeNamesNotUnique.php b/src/Typesystem/Exception/TypeNamesNotUnique.php
@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Graphpinator\Typesystem\Exception;
+
+final class TypeNamesNotUnique extends TypeError
+{
+    public const MESSAGE = 'Type names are not unique.';
+}
diff --git a/src/Typesystem/Schema.php b/src/Typesystem/Schema.php
@@ -12,6 +12,7 @@
 use Graphpinator\Typesystem\DirectiveUsage\DirectiveUsage;
 use Graphpinator\Typesystem\DirectiveUsage\DirectiveUsageSet;
 use Graphpinator\Typesystem\Exception\RootOperationTypesMustBeDifferent;
+use Graphpinator\Typesystem\Exception\RootOperationTypesMustBeWithinContainer;
 use Graphpinator\Typesystem\Field\ResolvableField;
 use Graphpinator\Typesystem\Location\SchemaLocation;
 use Graphpinator\Typesystem\Utils\THasDirectives;
@@ -30,11 +31,7 @@ public function __construct(
     )
     {
         if (Graphpinator::$validateSchema) {
-            if (self::isSame($query, $mutation) ||
-                self::isSame($query, $subscription) ||
-                self::isSame($mutation, $subscription)) {
-                throw new RootOperationTypesMustBeDifferent();
-            }
+            $this->validateSchema();
         }
 
         $this->directiveUsages = new DirectiveUsageSet();
@@ -96,4 +93,19 @@ private static function isSame(?Type $lhs, ?Type $rhs) : bool
         return $lhs === $rhs
             && ($lhs !== null || $rhs !== null);
     }
+
+    private function validateSchema() : void
+    {
+        if (self::isSame($this->query, $this->mutation) ||
+            self::isSame($this->query, $this->subscription) ||
+            self::isSame($this->mutation, $this->subscription)) {
+            throw new RootOperationTypesMustBeDifferent();
+        }
+
+        if ($this->container->getType($this->query->getName()) !== $this->query ||
+            ($this->mutation instanceof Type && $this->container->getType($this->mutation->getName()) !== $this->mutation) ||
+            ($this->subscription instanceof Type && $this->container->getType($this->subscription->getName()) !== $this->subscription)) {
+            throw new RootOperationTypesMustBeWithinContainer();
+        }
+    }
 }
diff --git a/tests/Feature/DeprecatedDirectiveTest.php b/tests/Feature/DeprecatedDirectiveTest.php
@@ -23,6 +23,7 @@ final class DeprecatedDirectiveTest extends TestCase
 {
     private static ?Type $testType = null;
     private static ?InputType $testInputType = null;
+    private static ?Type $query = null;
 
     public static function createTestType() : Type
     {
@@ -372,12 +373,17 @@ private function getContainer() : SimpleContainer
         return new SimpleContainer([
             'TestType' => self::createTestType(),
             'TestInputType' => self::createTestInputType(),
+            'Query' => $this->getQuery(),
         ], []);
     }
 
     private function getQuery() : Type
     {
-        return new class extends Type {
+        if (self::$query instanceof Type) {
+            return self::$query;
+        }
+
+        self::$query = new class extends Type {
             protected const NAME = 'Query';
 
             public function validateNonNullValue($rawValue) : bool
@@ -397,5 +403,7 @@ static function () : void {
                 ]);
             }
         };
+
+        return self::$query;
     }
 }
diff --git a/tests/Feature/MutationTest.php b/tests/Feature/MutationTest.php
@@ -20,6 +20,8 @@ final class MutationTest extends TestCase
     public function testSimple() : void
     {
         $query = new class extends Type {
+            protected const NAME = 'Query';
+
             public function validateNonNullValue(mixed $rawValue) : bool
             {
                 return true;
@@ -31,6 +33,8 @@ protected function getFieldDefinition() : ResolvableFieldSet
             }
         };
         $mutation = new class extends Type {
+            protected const NAME = 'Mutation';
+
             private int $order = 0;
 
             public function validateNonNullValue(mixed $rawValue) : bool
@@ -54,7 +58,7 @@ function ($parent) : int {
                 ]);
             }
         };
-        $container = new SimpleContainer([$query], []);
+        $container = new SimpleContainer([$query, $mutation], []);
         $schema = new Schema($container, $query, $mutation);
 
         $graphpinator = new Graphpinator($schema);

diff --git a/tests/Feature/NativeEnumTest.php b/tests/Feature/NativeEnumTest.php
@@ -40,6 +40,8 @@ public function __construct()
         self::assertEquals('Another description for following enum case', $items->offsetGet('XYZ')->getDescription());
 
         $query = new class ($enumType) extends Type {
+            protected const NAME = 'TestEnum';
+
             public function __construct(
                 private EnumType $enumType,
             )

diff --git a/tests/Feature/OperationDirectiveTest.php b/tests/Feature/OperationDirectiveTest.php
@@ -30,6 +30,8 @@ public function testSimple() : void
         $counter->count = 0;
 
         $query = new class ($counter) extends Type {
+            protected const NAME = 'Query';
+
             public function __construct(
                 private \stdClass $counter,
             )
@@ -56,6 +58,8 @@ function ($parent) : int {
             }
         };
         $mutation = new class ($counter) extends Type {
+            protected const NAME = 'Mutation';
+
             public function __construct(
                 private \stdClass $counter,
             )
@@ -82,6 +86,8 @@ function ($parent) : int {
             }
         };
         $subscription = new class ($counter) extends Type {
+            protected const NAME = 'Subscription';
+
             public function __construct(
                 private \stdClass $counter,
             )
@@ -170,7 +176,7 @@ protected function getFieldDefinition() : ArgumentSet
                 return new ArgumentSet();
             }
         };
-        $container = new SimpleContainer([$query], [$test]);
+        $container = new SimpleContainer([$query, $mutation, $subscription], [$test]);
         $schema = new Schema($container, $query, $mutation, $subscription);
         $graphpinator = new Graphpinator($schema);
 

diff --git a/tests/Feature/SpecifiedByDirectiveTest.php b/tests/Feature/SpecifiedByDirectiveTest.php
@@ -20,6 +20,7 @@
 final class SpecifiedByDirectiveTest extends TestCase
 {
     private static ?ScalarType $testScalar = null;
+    private static ?Type $query = null;
 
     public static function createTestScalar() : ScalarType
     {
@@ -98,12 +99,16 @@ private function getSchema() : Schema
 
     private function getContainer() : SimpleContainer
     {
-        return new SimpleContainer(['TestScalar' => self::createTestScalar()], []);
+        return new SimpleContainer(['TestScalar' => self::createTestScalar(), $this->getQuery()], []);
     }
 
     private function getQuery() : Type
     {
-        return new class extends Type {
+        if (self::$query instanceof Type) {
+            return self::$query;
+        }
+
+        self::$query = new class extends Type {
             protected const NAME = 'Query';
 
             public function validateNonNullValue($rawValue) : bool
@@ -123,5 +128,7 @@ static function () : void {
                 ]);
             }
         };
+
+        return self::$query;
     }
 }
diff --git a/tests/Feature/SubscriptionTest.php b/tests/Feature/SubscriptionTest.php
@@ -23,6 +23,8 @@ final class SubscriptionTest extends TestCase
     public function testSimple() : void
     {
         $query = new class extends Type {
+            protected const NAME = 'Query';
+
             public function validateNonNullValue(mixed $rawValue) : bool
             {
                 return true;
@@ -34,6 +36,8 @@ protected function getFieldDefinition() : ResolvableFieldSet
             }
         };
         $subscription = new class extends Type {
+            protected const NAME = 'Subscription';
+
             public function validateNonNullValue(mixed $rawValue) : bool
             {
                 return true;
@@ -52,7 +56,7 @@ static function ($parent) : int {
                 ]);
             }
         };
-        $container = new SimpleContainer([$query], []);
+        $container = new SimpleContainer([$query, $subscription], []);
         $schema = new Schema($container, $query, null, $subscription);
 
         $graphpinator = new Graphpinator($schema);

diff --git a/tests/Feature/TypeNamesNotUniqueTest.php b/tests/Feature/TypeNamesNotUniqueTest.php
@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Graphpinator\Tests\Feature;
+
+use Graphpinator\SimpleContainer;
+use Graphpinator\Typesystem\Container;
+use Graphpinator\Typesystem\Exception\TypeNamesNotUnique;
+use Graphpinator\Typesystem\Field\ResolvableField;
+use Graphpinator\Typesystem\Field\ResolvableFieldSet;
+use Graphpinator\Typesystem\Type;
+use PHPUnit\Framework\TestCase;
+
+final class TypeNamesNotUniqueTest extends TestCase
+{
+    public function testAllSame() : void
+    {
+        $this->expectException(TypeNamesNotUnique::class);
+        $this->expectExceptionMessage(TypeNamesNotUnique::MESSAGE);
+
+        new SimpleContainer($this->getTypes(), []);
+    }
+
+    /**
+     * @return array<Type>
+     */
+    private function getTypes() : array
+    {
+        $types = [];
+        $types[] = new class extends Type {
+            protected const NAME = 'Query';
+
+            public function validateNonNullValue($rawValue) : bool
+            {
+                return true;
+            }
+
+            protected function getFieldDefinition() : ResolvableFieldSet
+            {
+                return new ResolvableFieldSet([
+                    new ResolvableField(
+                        'field',
+                        Container::Int(),
+                        static function () : int {
+                            return 1;
+                        },
+                    ),
+                ]);
+            }
+        };
+
+        $types[] = new class extends Type {
+            protected const NAME = 'Query';
+
+            public function validateNonNullValue($rawValue) : bool
+            {
+                return true;
+            }
+
+            protected function getFieldDefinition() : ResolvableFieldSet
+            {
+                return new ResolvableFieldSet([
+                    new ResolvableField(
+                        'field',
+                        Container::Int(),
+                        static function () : int {
+                            return 1;
+                        },
+                    ),
+                ]);
+            }
+        };
+
+        return $types;
+    }
+}
diff --git a/tests/Spec/TestSchema.php b/tests/Spec/TestSchema.php
@@ -34,6 +34,7 @@ final class TestSchema
 {
     private static array $types = [];
     private static ?Container $container = null;
+    private static ?Type $query = null;
 
     public static function getSchema() : Schema
     {
@@ -134,7 +135,11 @@ public static function getContainer() : Container
 
     public static function getQuery() : Type
     {
-        return new class extends Type
+        if (self::$query !== null) {
+            return self::$query;
+        }
+
+        return self::$query = new class extends Type
         {
             protected const NAME = 'Query';