diff --git a/src/Executor/Executor.php b/src/Executor/Executor.php index cd660ea..08d5edd 100644 --- a/src/Executor/Executor.php +++ b/src/Executor/Executor.php @@ -960,6 +960,18 @@ class Executor return $this->completeListValue($returnType, $fieldNodes, $info, $path, $result); } + // Account for invalid schema definition when typeLoader returns different + // instance than `resolveType` or $field->getType() or $arg->getType() + if ($returnType !== $this->exeContext->schema->getType($returnType->name)) { + $hint = ""; + if ($this->exeContext->schema->getConfig()->typeLoader) { + $hint = "Make sure that type loader returns the same instance as defined in {$info->parentType}.{$info->fieldName}"; + } + throw new InvariantViolation( + "Schema must contain unique named types but contains multiple types named \"$returnType\". $hint" + ); + } + // If field type is Scalar or Enum, serialize to a valid value, returning // null if serialization is not possible. if ($returnType instanceof LeafType) { @@ -1130,18 +1142,24 @@ class Executor $runtimeTypeOrName; if (!$runtimeType instanceof ObjectType) { - throw new Error( + throw new InvariantViolation( "Abstract type {$returnType} must resolve to an Object type at runtime " . "for field {$info->parentType}.{$info->fieldName} with " . - 'value "' . Utils::printSafe($result) . '", received "'. Utils::printSafe($runtimeType) . '".', - $fieldNodes + 'value "' . Utils::printSafe($result) . '", received "'. Utils::printSafe($runtimeType) . '".' ); } if (!$this->exeContext->schema->isPossibleType($returnType, $runtimeType)) { - throw new Error( - "Runtime Object type \"$runtimeType\" is not a possible type for \"$returnType\".", - $fieldNodes + throw new InvariantViolation( + "Runtime Object type \"$runtimeType\" is not a possible type for \"$returnType\"." + ); + } + + if ($runtimeType !== $this->exeContext->schema->getType($runtimeType->name)) { + throw new InvariantViolation( + "Schema must contain unique named types but contains multiple types named \"$runtimeType\". ". + "Make sure that `resolveType` function of abstract type \"{$returnType}\" returns the same ". + "type instance as referenced anywhere else within the schema." ); } diff --git a/tests/Executor/AbstractPromiseTest.php b/tests/Executor/AbstractPromiseTest.php index 363c80c..e4feeef 100644 --- a/tests/Executor/AbstractPromiseTest.php +++ b/tests/Executor/AbstractPromiseTest.php @@ -361,7 +361,7 @@ class AbstractPromiseTest extends \PHPUnit_Framework_TestCase } }'; - $result = GraphQL::execute($schema, $query); + $result = GraphQL::executeAndReturnResult($schema, $query)->toArray(true); $expected = [ 'data' => [ @@ -373,7 +373,7 @@ class AbstractPromiseTest extends \PHPUnit_Framework_TestCase ], 'errors' => [ [ - 'message' => 'Runtime Object type "Human" is not a possible type for "Pet".', + 'debugMessage' => 'Runtime Object type "Human" is not a possible type for "Pet".', 'locations' => [['line' => 2, 'column' => 7]], 'path' => ['pets', 2] ], @@ -462,7 +462,7 @@ class AbstractPromiseTest extends \PHPUnit_Framework_TestCase } }'; - $result = GraphQL::execute($schema, $query); + $result = GraphQL::executeAndReturnResult($schema, $query)->toArray(true); $expected = [ 'data' => [ @@ -474,7 +474,7 @@ class AbstractPromiseTest extends \PHPUnit_Framework_TestCase ], 'errors' => [ [ - 'message' => 'Runtime Object type "Human" is not a possible type for "Pet".', + 'debugMessage' => 'Runtime Object type "Human" is not a possible type for "Pet".', 'locations' => [['line' => 2, 'column' => 7]], 'path' => ['pets', 2] ] diff --git a/tests/Executor/AbstractTest.php b/tests/Executor/AbstractTest.php index dcc9129..7ae295f 100644 --- a/tests/Executor/AbstractTest.php +++ b/tests/Executor/AbstractTest.php @@ -251,12 +251,12 @@ class AbstractTest extends \PHPUnit_Framework_TestCase ] ], 'errors' => [[ - 'message' => 'Runtime Object type "Human" is not a possible type for "Pet".', + 'debugMessage' => 'Runtime Object type "Human" is not a possible type for "Pet".', 'locations' => [['line' => 2, 'column' => 11]], 'path' => ['pets', 2] ]] ]; - $actual = GraphQL::execute($schema, $query); + $actual = GraphQL::executeAndReturnResult($schema, $query)->toArray(true); $this->assertArraySubset($expected, $actual); } @@ -336,7 +336,7 @@ class AbstractTest extends \PHPUnit_Framework_TestCase } }'; - $result = GraphQL::execute($schema, $query); + $result = GraphQL::executeAndReturnResult($schema, $query)->toArray(true); $expected = [ 'data' => [ 'pets' => [ @@ -348,7 +348,7 @@ class AbstractTest extends \PHPUnit_Framework_TestCase ] ], 'errors' => [[ - 'message' => 'Runtime Object type "Human" is not a possible type for "Pet".', + 'debugMessage' => 'Runtime Object type "Human" is not a possible type for "Pet".', 'locations' => [['line' => 2, 'column' => 11]], 'path' => ['pets', 2] ]] @@ -432,4 +432,59 @@ class AbstractTest extends \PHPUnit_Framework_TestCase ] ], $result); } + + public function testHintsOnConflictingTypeInstancesInResolveType() + { + $createTest = function() use (&$iface) { + return new ObjectType([ + 'name' => 'Test', + 'fields' => [ + 'a' => Type::string() + ], + 'interfaces' => function() use ($iface) { + return [$iface]; + } + ]); + }; + + $iface = new InterfaceType([ + 'name' => 'Node', + 'fields' => [ + 'a' => Type::string() + ], + 'resolveType' => function() use (&$createTest) { + return $createTest(); + } + ]); + + $query = new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'node' => $iface, + 'test' => $createTest() + ] + ]); + + $schema = new Schema([ + 'query' => $query, + ]); + $schema->assertValid(); + + $query = ' + { + node { + a + } + } + '; + + $result = Executor::execute($schema, Parser::parse($query), ['node' => ['a' => 'value']]); + + $this->assertEquals( + 'Schema must contain unique named types but contains multiple types named "Test". '. + 'Make sure that `resolveType` function of abstract type "Node" returns the same type instance '. + 'as referenced anywhere else within the schema.', + $result->errors[0]->getMessage() + ); + } } diff --git a/tests/Executor/ExecutorLazySchemaTest.php b/tests/Executor/ExecutorLazySchemaTest.php index 5e8b97c..c3c0704 100644 --- a/tests/Executor/ExecutorLazySchemaTest.php +++ b/tests/Executor/ExecutorLazySchemaTest.php @@ -3,6 +3,7 @@ namespace GraphQL\Tests\Executor; require_once __DIR__ . '/TestClasses.php'; +use GraphQL\Error\InvariantViolation; use GraphQL\Error\Warning; use GraphQL\Executor\ExecutionResult; use GraphQL\Executor\Executor; @@ -113,4 +114,59 @@ class ExecutorLazySchemaTest extends \PHPUnit_Framework_TestCase 'Make sure your `resolveType` always returns valid implementation or throws.', $result->errors[0]->getMessage()); } + + public function testHintsOnConflictingTypeInstancesInDefinitions() + { + $calls = []; + $typeLoader = function($name) use (&$calls) { + $calls[] = $name; + switch ($name) { + case 'Test': + return new ObjectType([ + 'name' => 'Test', + 'fields' => function() { + return [ + 'test' => Type::string(), + ]; + } + ]); + default: + return null; + } + }; + $query = new ObjectType([ + 'name' => 'Query', + 'fields' => function() use ($typeLoader) { + return [ + 'test' => $typeLoader('Test') + ]; + } + ]); + $schema = new Schema([ + 'query' => $query, + 'typeLoader' => $typeLoader + ]); + + $query = ' + { + test { + test + } + } + '; + + $this->assertEquals([], $calls); + $result = Executor::execute($schema, Parser::parse($query), ['test' => ['test' => 'value']]); + $this->assertEquals(['Test', 'Test'], $calls); + + $this->assertEquals( + 'Schema must contain unique named types but contains multiple types named "Test". '. + 'Make sure that type loader returns the same instance as defined in Query.test', + $result->errors[0]->getMessage() + ); + $this->assertInstanceOf( + InvariantViolation::class, + $result->errors[0]->getPrevious() + ); + } }