From 7c19777dffa1a130148736803f8edc35ee0a929b Mon Sep 17 00:00:00 2001 From: Vladimir Razuvaev Date: Wed, 21 Nov 2018 20:11:11 +0700 Subject: [PATCH] Ensure interface has at least 1 concrete type --- src/Type/SchemaValidationContext.php | 20 +++++ tests/Type/ValidationTest.php | 109 +++++++++++++++++++-------- 2 files changed, 97 insertions(+), 32 deletions(-) diff --git a/src/Type/SchemaValidationContext.php b/src/Type/SchemaValidationContext.php index 3762f50..63f54dd 100644 --- a/src/Type/SchemaValidationContext.php +++ b/src/Type/SchemaValidationContext.php @@ -263,6 +263,9 @@ class SchemaValidationContext } elseif ($type instanceof InterfaceType) { // Ensure fields are valid. $this->validateFields($type); + + // Ensure Interfaces include at least 1 Object type. + $this->validateInterfaces($type); } elseif ($type instanceof UnionType) { // Ensure Unions include valid member types. $this->validateUnionMembers($type); @@ -504,6 +507,23 @@ class SchemaValidationContext } } + private function validateInterfaces(InterfaceType $iface) + { + $possibleTypes = $this->schema->getPossibleTypes($iface); + + if (count($possibleTypes) !== 0) { + return; + } + + $this->reportError( + sprintf( + 'Interface %s must be implemented by at least one Object type.', + $iface->name + ), + $iface->astNode + ); + } + /** * @param InterfaceType $iface * diff --git a/tests/Type/ValidationTest.php b/tests/Type/ValidationTest.php index 27ba792..095bc7f 100644 --- a/tests/Type/ValidationTest.php +++ b/tests/Type/ValidationTest.php @@ -21,8 +21,8 @@ use GraphQL\Utils\Utils; use PHPUnit\Framework\TestCase; use function array_map; use function array_merge; -use function count; use function implode; +use function print_r; use function sprintf; class ValidationTest extends TestCase @@ -74,6 +74,11 @@ class ValidationTest extends TestCase }, ]); + $this->SomeInterfaceType = new InterfaceType([ + 'name' => 'SomeInterface', + 'fields' => ['f' => ['type' => Type::string()]], + ]); + $this->SomeObjectType = new ObjectType([ 'name' => 'SomeObject', 'fields' => ['f' => ['type' => Type::string()]], @@ -87,11 +92,6 @@ class ValidationTest extends TestCase 'types' => [$this->SomeObjectType], ]); - $this->SomeInterfaceType = new InterfaceType([ - 'name' => 'SomeInterface', - 'fields' => ['f' => ['type' => Type::string()]], - ]); - $this->SomeEnumType = new EnumType([ 'name' => 'SomeEnum', 'values' => [ @@ -339,34 +339,40 @@ class ValidationTest extends TestCase */ private function assertContainsValidationMessage($array, $messages) { - self::assertCount( - count($messages), - $array, - sprintf('For messages: %s', $messages[0]['message']) . "\n" . - "Received: \n" . - implode( - "\n", - array_map( - static function ($error) { - return $error->getMessage(); - }, - $array - ) + $allErrors = implode( + "\n", + array_map( + static function ($error) { + return $error->getMessage(); + }, + $array ) ); - foreach ($array as $index => $error) { - if (! isset($messages[$index]) || ! $error instanceof Error) { - self::fail('Received unexpected error: ' . $error->getMessage()); + + foreach ($messages as $expected) { + $msg = $expected['message']; + $locations = $expected['locations'] ?? []; + foreach ($array as $actual) { + if ($actual instanceof Error && $actual->getMessage() === $msg) { + $actualLocations = []; + foreach ($actual->getLocations() as $location) { + $actualLocations[] = $location->toArray(); + } + self::assertEquals( + $locations, + $actualLocations, + sprintf( + 'Locations do not match for error: %s\n\nExpected:\n%s\n\nActual:\n%s', + $msg, + print_r($locations, true), + print_r($actualLocations, true) + ) + ); + // Found and valid, so check the next message (and don't fail) + continue 2; + } } - self::assertEquals($messages[$index]['message'], $error->getMessage()); - $errorLocations = []; - foreach ($error->getLocations() as $location) { - $errorLocations[] = $location->toArray(); - } - self::assertEquals( - $messages[$index]['locations'] ?? [], - $errorLocations - ); + self::fail(sprintf("Expected error not found:\n%s\n\nActual errors:\n%s", $msg, $allErrors)); } } @@ -598,12 +604,18 @@ class ValidationTest extends TestCase */ private function schemaWithFieldType($type) : Schema { + $ifaceImplementation = new ObjectType([ + 'name' => 'SomeInterfaceImplementation', + 'fields' => ['f' => ['type' => Type::string()]], + 'interfaces' => [ $this->SomeInterfaceType ], + ]); + return new Schema([ 'query' => new ObjectType([ 'name' => 'Query', 'fields' => ['f' => ['type' => $type]], ]), - 'types' => [$type], + 'types' => [$type, $ifaceImplementation], ]); } @@ -1237,6 +1249,14 @@ class ValidationTest extends TestCase ], ]); + $BadImplementingType = new ObjectType([ + 'name' => 'BadImplementing', + 'interfaces' => [ $BadInterfaceType ], + 'fields' => [ + 'badField' => [ 'type' => $fieldType ], + ], + ]); + return new Schema([ 'query' => new ObjectType([ 'name' => 'Query', @@ -1244,6 +1264,7 @@ class ValidationTest extends TestCase 'f' => ['type' => $BadInterfaceType], ], ]), + 'types' => [ $BadImplementingType, $this->SomeObjectType ], ]); } @@ -1308,6 +1329,30 @@ class ValidationTest extends TestCase ); } + /** + * @see it('rejects an interface not implemented by at least one object') + */ + public function testRejectsAnInterfaceNotImplementedByAtLeastOneObject() + { + $schema = BuildSchema::build(' + type Query { + test: SomeInterface + } + + interface SomeInterface { + foo: String + } + '); + $this->assertContainsValidationMessage( + $schema->validate(), + [[ + 'message' => 'Interface SomeInterface must be implemented by at least one Object type.', + 'locations' => [[ 'line' => 6, 'column' => 7 ]], + ], + ] + ); + } + /** * @see it('accepts an input type as a field arg type') */