Merge pull request #492 from spawnia/circular-references

Add schema validation: Input Objects must not contain non-nullable circular references

CHANGELOG.md

@@ -1,4 +1,8 @@
 # Changelog
+
+## Unreleased
+- Add schema validation: Input Objects must not contain non-nullable circular references (#492)
+
 ## v0.13.0
 This release brings several breaking changes. Please refer to [UPGRADE](UPGRADE.md) document for details.
 

CONTRIBUTING.md

@@ -10,8 +10,9 @@ For smaller contributions just use this workflow:
 * Fork the project.
 * Add your features and or bug fixes.
 * Add tests. Tests are important for us.
-* Check your changes using `composer check-all`
-* Send a pull request
+* Check your changes using `composer check-all`.
+* Add an entry to the [Changelog's Unreleases section](CHANGELOG.md#unreleased).
+* Send a pull request.
 
 ## Setup the Development Environment
 First, copy the URL of your fork and `git clone` it to your local machine.

src/Type/SchemaValidationContext.php

@@ -29,6 +29,7 @@
 use GraphQL\Type\Definition\ObjectType;
 use GraphQL\Type\Definition\Type;
 use GraphQL\Type\Definition\UnionType;
+use GraphQL\Type\Validation\InputObjectCircularRefs;
 use GraphQL\Utils\TypeComparators;
 use GraphQL\Utils\Utils;
 use function array_filter;
@@ -48,9 +49,13 @@ class SchemaValidationContext
     /** @var Schema */
     private $schema;
 
+    /** @var InputObjectCircularRefs */
+    private $inputObjectCircularRefs;
+
     public function __construct(Schema $schema)
     {
-        $this->schema = $schema;
+        $this->schema                  = $schema;
+        $this->inputObjectCircularRefs = new InputObjectCircularRefs($this);
     }
 
     /**
@@ -99,7 +104,7 @@ public function validateRootTypes()
      * @param string                                       $message
      * @param Node[]|Node|TypeNode|TypeDefinitionNode|null $nodes
      */
-    private function reportError($message, $nodes = null)
+    public function reportError($message, $nodes = null)
     {
         $nodes = array_filter($nodes && is_array($nodes) ? $nodes : [$nodes]);
         $this->addError(new Error($message, $nodes));
@@ -275,6 +280,9 @@ public function validateTypes()
             } elseif ($type instanceof InputObjectType) {
                 // Ensure Input Object fields are valid.
                 $this->validateInputFields($type);
+
+                // Ensure Input Objects do not contain non-nullable circular references
+                $this->inputObjectCircularRefs->validate($type);
             }
         }
     }

src/Type/Validation/InputObjectCircularRefs.php

@@ -0,0 +1,105 @@
+<?php
+
+declare(strict_types=1);
+
+namespace GraphQL\Type\Validation;
+
+use GraphQL\Language\AST\InputValueDefinitionNode;
+use GraphQL\Type\Definition\InputObjectField;
+use GraphQL\Type\Definition\InputObjectType;
+use GraphQL\Type\Definition\NonNull;
+use GraphQL\Type\SchemaValidationContext;
+use function array_map;
+use function array_pop;
+use function array_slice;
+use function count;
+use function implode;
+
+class InputObjectCircularRefs
+{
+    /** @var SchemaValidationContext */
+    private $schemaValidationContext;
+
+    /**
+     * Tracks already visited types to maintain O(N) and to ensure that cycles
+     * are not redundantly reported.
+     *
+     * @var InputObjectType[]
+     */
+    private $visitedTypes = [];
+
+    /** @var InputObjectField[] */
+    private $fieldPath = [];
+
+    /**
+     * Position in the type path.
+     *
+     * [string $typeName => int $index]
+     *
+     * @var int[]
+     */
+    private $fieldPathIndexByTypeName = [];
+
+    public function __construct(SchemaValidationContext $schemaValidationContext)
+    {
+        $this->schemaValidationContext = $schemaValidationContext;
+    }
+
+    /**
+     * This does a straight-forward DFS to find cycles.
+     * It does not terminate when a cycle was found but continues to explore
+     * the graph to find all possible cycles.
+     */
+    public function validate(InputObjectType $inputObj) : void
+    {
+        if (isset($this->visitedTypes[$inputObj->name])) {
+            return;
+        }
+
+        $this->visitedTypes[$inputObj->name]             = true;
+        $this->fieldPathIndexByTypeName[$inputObj->name] = count($this->fieldPath);
+
+        $fieldMap = $inputObj->getFields();
+        foreach ($fieldMap as $fieldName => $field) {
+            $type = $field->getType();
+
+            if ($type instanceof NonNull) {
+                $fieldType = $type->getWrappedType();
+
+                // If the type of the field is anything else then a non-nullable input object,
+                // there is no chance of an unbreakable cycle
+                if ($fieldType instanceof InputObjectType) {
+                    $this->fieldPath[] = $field;
+
+                    if (! isset($this->fieldPathIndexByTypeName[$fieldType->name])) {
+                        $this->validate($fieldType);
+                    } else {
+                        $cycleIndex = $this->fieldPathIndexByTypeName[$fieldType->name];
+                        $cyclePath  = array_slice($this->fieldPath, $cycleIndex);
+                        $fieldNames = array_map(
+                            static function (InputObjectField $field) : string {
+                                return $field->name;
+                            },
+                            $cyclePath
+                        );
+
+                        $this->schemaValidationContext->reportError(
+                            'Cannot reference Input Object "' . $fieldType->name . '" within itself '
+                            . 'through a series of non-null fields: "' . implode('.', $fieldNames) . '".',
+                            array_map(
+                                static function (InputObjectField $field) : ?InputValueDefinitionNode {
+                                    return $field->astNode;
+                                },
+                                $cyclePath
+                            )
+                        );
+                    }
+                }
+            }
+
+            array_pop($this->fieldPath);
+        }
+
+        unset($this->fieldPathIndexByTypeName[$inputObj->name]);
+    }
+}

tests/Type/ValidationTest.php

@@ -879,6 +879,144 @@ public function testRejectsAnInputObjectTypeWithMissingFields() : void
         );
     }
 
+    /**
+     * @see it('accepts an Input Object with breakable circular reference')
+     */
+    public function testAcceptsAnInputObjectWithBreakableCircularReference() : void
+    {
+        $schema = BuildSchema::build('
+      input AnotherInputObject {
+        parent: SomeInputObject
+      }
+      
+      type Query {
+        field(arg: SomeInputObject): String
+      }
+      
+      input SomeInputObject {
+        self: SomeInputObject
+        arrayOfSelf: [SomeInputObject]
+        nonNullArrayOfSelf: [SomeInputObject]!
+        nonNullArrayOfNonNullSelf: [SomeInputObject!]!
+        intermediateSelf: AnotherInputObject
+      }
+        ');
+        self::assertEquals([], $schema->validate());
+    }
+
+    /**
+     * @see it('rejects an Input Object with non-breakable circular reference')
+     */
+    public function testRejectsAnInputObjectWithNonBreakableCircularReference() : void
+    {
+        $schema = BuildSchema::build('
+      type Query {
+        field(arg: SomeInputObject): String
+      }
+      
+      input SomeInputObject {
+        nonNullSelf: SomeInputObject!
+      }
+        ');
+        $this->assertMatchesValidationMessage(
+            $schema->validate(),
+            [
+                [
+                    'message'   => 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null fields: "nonNullSelf".',
+                    'locations' => [['line' => 7, 'column' => 9]],
+                ],
+            ]
+        );
+    }
+
+    /**
+     * @see it('rejects Input Objects with non-breakable circular reference spread across them')
+     */
+    public function testRejectsInputObjectsWithNonBreakableCircularReferenceSpreadAcrossThem() : void
+    {
+        $schema = BuildSchema::build('
+      type Query {
+        field(arg: SomeInputObject): String
+      }
+      
+      input SomeInputObject {
+        startLoop: AnotherInputObject!
+      }
+      
+      input AnotherInputObject {
+        nextInLoop: YetAnotherInputObject!
+      }
+      
+      input YetAnotherInputObject {
+        closeLoop: SomeInputObject!
+      }
+        ');
+        $this->assertMatchesValidationMessage(
+            $schema->validate(),
+            [
+                [
+                    'message'   => 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null fields: "startLoop.nextInLoop.closeLoop".',
+                    'locations' => [
+                        ['line' => 7, 'column' => 9],
+                        ['line' => 11, 'column' => 9],
+                        ['line' => 15, 'column' => 9],
+                    ],
+                ],
+            ]
+        );
+    }
+
+    /**
+     * @see it('rejects Input Objects with multiple non-breakable circular reference')
+     */
+    public function testRejectsInputObjectsWithMultipleNonBreakableCircularReferences() : void
+    {
+        $schema = BuildSchema::build('
+      type Query {
+        field(arg: SomeInputObject): String
+      }
+      
+      input SomeInputObject {
+        startLoop: AnotherInputObject!
+      }
+      
+      input AnotherInputObject {
+        closeLoop: SomeInputObject!
+        startSecondLoop: YetAnotherInputObject!
+      }
+      
+      input YetAnotherInputObject {
+        closeSecondLoop: AnotherInputObject!
+        nonNullSelf: YetAnotherInputObject!
+      }
+        ');
+        $this->assertMatchesValidationMessage(
+            $schema->validate(),
+            [
+                [
+                    'message'   => 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null fields: "startLoop.closeLoop".',
+                    'locations' => [
+                        ['line' => 7, 'column' => 9],
+                        ['line' => 11, 'column' => 9],
+                    ],
+                ],
+                [
+                    'message'   => 'Cannot reference Input Object "AnotherInputObject" within itself through a series of non-null fields: "startSecondLoop.closeSecondLoop".',
+                    'locations' => [
+                        ['line' => 12, 'column' => 9],
+                        ['line' => 16, 'column' => 9],
+                    ],
+                ],
+                [
+                    'message'   => 'Cannot reference Input Object "YetAnotherInputObject" within itself through a series of non-null fields: "nonNullSelf".',
+                    'locations' => [
+                        ['line' => 17, 'column' => 9],
+                    ],
+                ],
+            ]
+        );
+    }
+
     /**
      * @see it('rejects an Input Object type with incorrectly typed fields')
      */