Skip to content

Commit

Permalink
Implement support for interfaces implementing interfaces (#740)
Browse files Browse the repository at this point in the history
* Implement support for interfaces implementing interfaces

Closes #728

* Implement tests for interfaces implementing interfaces

This ports the JavaScript tests for `RFC: Allow interfaces to
implement other interfaces` to PHP. This should ensure that there is
sufficient test coverage for the changes made to support interfaces
implementing interfaces.

Tests taken from https://github.com/graphql/graphql-js/pull/2084/files
including any typoes in test description strings to aid in comparison.

* Fix extend implement interface in Parser

This is part of the update to allow interfaces to implement interfaces.
A single extend statement to add an implementation of an interface
without field declarations is valid. This was caught by tests and brings
in a change from graphql/graphql-js#2084

* Validate interface implemented ancestors

Part of the work done to implement interfaces implementing interfaces.
This was caught by test and improves on the previously done changes for
the SchemaValidationContext by implementing
`validateTypeImplementsAncestors` which was missing.

* Properly apply Schema changes for interface extension support

This redoes the work done for the Schema class since it was previously
guessed at. It now more closely follows graphql/graphql-js/pull/2084

* Improve interface extension related typehints

Co-authored-by: Benedikt Franke <[email protected]>

* Refine types

* Remove complex array shape type

* Don't remove but deprecate DANGEROUS_CHANGE_IMPLEMENTED_INTERFACE_ADDED

Removing a public constant is a breaking change and can not be
implemented in a minor version. Instead the internal value is changed to
ensure that existing code keeps working with the new interface
implementation logic.

* Don't remove but deprecate BREAKING_CHANGE_INTERFACE_REMOVED_FROM_OBJECT

Co-authored-by: Benedikt Franke <[email protected]>
Co-authored-by: Benedikt Franke <[email protected]>
  • Loading branch information
3 people authored Jan 23, 2021
1 parent 769a474 commit c4d2f82
Show file tree
Hide file tree
Showing 41 changed files with 1,908 additions and 220 deletions.
6 changes: 3 additions & 3 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,12 @@ parameters:
path: src/Type/Schema.php

-
message: "#^Only booleans are allowed in a negated boolean, \\(callable\\)\\|null given\\.$#"
message: "#^Only booleans are allowed in an if condition, array\\<GraphQL\\\\Error\\\\Error\\|GraphQL\\\\Error\\\\InvariantViolation\\> given\\.$#"
count: 1
path: src/Type/Schema.php

-
message: "#^Only booleans are allowed in an if condition, array\\<GraphQL\\\\Error\\\\Error\\|GraphQL\\\\Error\\\\InvariantViolation\\> given\\.$#"
message: "#^Only booleans are allowed in a negated boolean, \\(callable\\)\\|null given\\.$#"
count: 1
path: src/Type/Schema.php

Expand Down Expand Up @@ -446,7 +446,7 @@ parameters:
path: src/Validator/Rules/KnownDirectives.php

-
message: "#^Only booleans are allowed in a negated boolean, array|null given\\.$#"
message: "#^Only booleans are allowed in a negated boolean, array\\|null given\\.$#"
count: 1
path: src/Validator/Rules/KnownDirectives.php

Expand Down
4 changes: 2 additions & 2 deletions src/Executor/ReferenceExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ private function doesFragmentConditionMatch(Node $fragment, ObjectType $type) :
return true;
}
if ($conditionalType instanceof AbstractType) {
return $this->exeContext->schema->isPossibleType($conditionalType, $type);
return $this->exeContext->schema->isSubType($conditionalType, $type);
}

return false;
Expand Down Expand Up @@ -1283,7 +1283,7 @@ private function ensureValidRuntimeType(
)
);
}
if (! $this->exeContext->schema->isPossibleType($returnType, $runtimeType)) {
if (! $this->exeContext->schema->isSubType($returnType, $runtimeType)) {
throw new InvariantViolation(
sprintf('Runtime Object type "%s" is not a possible type for "%s".', $runtimeType, $returnType)
);
Expand Down
4 changes: 2 additions & 2 deletions src/Experimental/Executor/Collector.php
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ private function doCollectFields(ObjectType $runtimeType, ?SelectionSetNode $sel
continue;
}
} elseif ($conditionType instanceof AbstractType) {
if (! $this->schema->isPossibleType($conditionType, $runtimeType)) {
if (! $this->schema->isSubType($conditionType, $runtimeType)) {
continue;
}
}
Expand All @@ -269,7 +269,7 @@ private function doCollectFields(ObjectType $runtimeType, ?SelectionSetNode $sel
continue;
}
} elseif ($conditionType instanceof AbstractType) {
if (! $this->schema->isPossibleType($conditionType, $runtimeType)) {
if (! $this->schema->isSubType($conditionType, $runtimeType)) {
continue;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Experimental/Executor/CoroutineExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ private function completeValue(CoroutineContext $ctx, Type $type, $value, array

$returnValue = null;
goto CHECKED_RETURN;
} elseif (! $this->schema->isPossibleType($type, $objectType)) {
} elseif (! $this->schema->isSubType($type, $objectType)) {
$this->addError(Error::createLocatedError(
new InvariantViolation(sprintf(
'Runtime Object type "%s" is not a possible type for "%s".',
Expand Down
3 changes: 3 additions & 0 deletions src/Language/AST/InterfaceTypeDefinitionNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ class InterfaceTypeDefinitionNode extends Node implements TypeDefinitionNode
/** @var NodeList<DirectiveNode>|null */
public $directives;

/** @var NodeList<InterfaceTypeDefinitionNode> */
public $interfaces;

/** @var NodeList<FieldDefinitionNode>|null */
public $fields;

Expand Down
3 changes: 3 additions & 0 deletions src/Language/AST/InterfaceTypeExtensionNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ class InterfaceTypeExtensionNode extends Node implements TypeExtensionNode
/** @var NodeList<DirectiveNode>|null */
public $directives;

/** @var NodeList<InterfaceTypeDefinitionNode> */
public $interfaces;

/** @var NodeList<FieldDefinitionNode>|null */
public $fields;
}
9 changes: 7 additions & 2 deletions src/Language/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -1346,12 +1346,14 @@ private function parseInterfaceTypeDefinition() : InterfaceTypeDefinitionNode
$description = $this->parseDescription();
$this->expectKeyword('interface');
$name = $this->parseName();
$interfaces = $this->parseImplementsInterfaces();
$directives = $this->parseDirectives(true);
$fields = $this->parseFieldsDefinition();

return new InterfaceTypeDefinitionNode([
'name' => $name,
'directives' => $directives,
'interfaces' => $interfaces,
'fields' => $fields,
'loc' => $this->loc($start),
'description' => $description,
Expand Down Expand Up @@ -1622,17 +1624,20 @@ private function parseInterfaceTypeExtension() : InterfaceTypeExtensionNode
$this->expectKeyword('extend');
$this->expectKeyword('interface');
$name = $this->parseName();
$interfaces = $this->parseImplementsInterfaces();
$directives = $this->parseDirectives(true);
$fields = $this->parseFieldsDefinition();
if (count($directives) === 0 &&
count($fields) === 0
if (count($interfaces) === 0
&& count($directives) === 0
&& count($fields) === 0
) {
throw $this->unexpected();
}

return new InterfaceTypeExtensionNode([
'name' => $name,
'directives' => $directives,
'interfaces' => $interfaces,
'fields' => $fields,
'loc' => $this->loc($start),
]);
Expand Down
2 changes: 2 additions & 0 deletions src/Language/Printer.php
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ function (InterfaceTypeDefinitionNode $def) : string {
[
'interface',
$def->name,
$this->wrap('implements ', $this->join($def->interfaces, ' & ')),
$this->join($def->directives, ' '),
$this->block($def->fields),
],
Expand Down Expand Up @@ -401,6 +402,7 @@ function (InterfaceTypeDefinitionNode $def) : string {
[
'extend interface',
$def->name,
$this->wrap('implements ', $this->join($def->interfaces, ' & ')),
$this->join($def->directives, ' '),
$this->block($def->fields),
],
Expand Down
4 changes: 2 additions & 2 deletions src/Language/Visitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,15 @@ class Visitor
NodeKind::OBJECT_TYPE_DEFINITION => ['description', 'name', 'interfaces', 'directives', 'fields'],
NodeKind::FIELD_DEFINITION => ['description', 'name', 'arguments', 'type', 'directives'],
NodeKind::INPUT_VALUE_DEFINITION => ['description', 'name', 'type', 'defaultValue', 'directives'],
NodeKind::INTERFACE_TYPE_DEFINITION => ['description', 'name', 'directives', 'fields'],
NodeKind::INTERFACE_TYPE_DEFINITION => ['description', 'name', 'interfaces', 'directives', 'fields'],
NodeKind::UNION_TYPE_DEFINITION => ['description', 'name', 'directives', 'types'],
NodeKind::ENUM_TYPE_DEFINITION => ['description', 'name', 'directives', 'values'],
NodeKind::ENUM_VALUE_DEFINITION => ['description', 'name', 'directives'],
NodeKind::INPUT_OBJECT_TYPE_DEFINITION => ['description', 'name', 'directives', 'fields'],

NodeKind::SCALAR_TYPE_EXTENSION => ['name', 'directives'],
NodeKind::OBJECT_TYPE_EXTENSION => ['name', 'interfaces', 'directives', 'fields'],
NodeKind::INTERFACE_TYPE_EXTENSION => ['name', 'directives', 'fields'],
NodeKind::INTERFACE_TYPE_EXTENSION => ['name', 'interfaces', 'directives', 'fields'],
NodeKind::UNION_TYPE_EXTENSION => ['name', 'directives', 'types'],
NodeKind::ENUM_TYPE_EXTENSION => ['name', 'directives', 'values'],
NodeKind::INPUT_OBJECT_TYPE_EXTENSION => ['name', 'directives', 'fields'],
Expand Down
20 changes: 20 additions & 0 deletions src/Type/Definition/ImplementingType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace GraphQL\Type\Definition;

/**
export type GraphQLImplementingType =
GraphQLObjectType |
GraphQLInterfaceType;
*/
interface ImplementingType
{
public function implementsInterface(InterfaceType $interfaceType) : bool;

/**
* @return array<int, InterfaceType>
*/
public function getInterfaces() : array;
}
65 changes: 62 additions & 3 deletions src/Type/Definition/InterfaceType.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,43 @@
use GraphQL\Error\InvariantViolation;
use GraphQL\Language\AST\InterfaceTypeDefinitionNode;
use GraphQL\Language\AST\InterfaceTypeExtensionNode;
use GraphQL\Type\Schema;
use GraphQL\Utils\Utils;
use function array_map;
use function is_array;
use function is_callable;
use function is_string;
use function sprintf;

class InterfaceType extends Type implements AbstractType, OutputType, CompositeType, NullableType, NamedType
class InterfaceType extends Type implements AbstractType, OutputType, CompositeType, NullableType, NamedType, ImplementingType
{
/** @var InterfaceTypeDefinitionNode|null */
public $astNode;

/** @var InterfaceTypeExtensionNode[] */
/** @var array<int, InterfaceTypeExtensionNode> */
public $extensionASTNodes;

/**
* Lazily initialized.
*
* @var FieldDefinition[]
* @var array<string, FieldDefinition>
*/
private $fields;

/**
* Lazily initialized.
*
* @var array<int, InterfaceType>
*/
private $interfaces;

/**
* Lazily initialized.
*
* @var array<string, InterfaceType>
*/
private $interfaceMap;

/**
* @param mixed[] $config
*/
Expand Down Expand Up @@ -99,6 +116,48 @@ protected function initializeFields() : void
$this->fields = FieldDefinition::defineFieldMap($this, $fields);
}

public function implementsInterface(InterfaceType $interfaceType) : bool
{
if (! isset($this->interfaceMap)) {
$this->interfaceMap = [];
foreach ($this->getInterfaces() as $interface) {
/** @var Type&InterfaceType $interface */
$interface = Schema::resolveType($interface);
$this->interfaceMap[$interface->name] = $interface;
}
}

return isset($this->interfaceMap[$interfaceType->name]);
}

/**
* @return array<int, InterfaceType>
*/
public function getInterfaces() : array
{
if (! isset($this->interfaces)) {
$interfaces = $this->config['interfaces'] ?? [];
if (is_callable($interfaces)) {
$interfaces = $interfaces();
}

if ($interfaces !== null && ! is_array($interfaces)) {
throw new InvariantViolation(
sprintf('%s interfaces must be an Array or a callable which returns an Array.', $this->name)
);
}

/** @var array<int, InterfaceType> $interfaces */
$interfaces = $interfaces === null
? []
: array_map([Schema::class, 'resolveType'], $interfaces);

$this->interfaces = $interfaces;
}

return $this->interfaces;
}

/**
* Resolves concrete ObjectType for given object value
*
Expand Down
8 changes: 4 additions & 4 deletions src/Type/Definition/ObjectType.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
* }
* ]);
*/
class ObjectType extends Type implements OutputType, CompositeType, NullableType, NamedType
class ObjectType extends Type implements OutputType, CompositeType, NullableType, NamedType, ImplementingType
{
/** @var ObjectTypeDefinitionNode|null */
public $astNode;
Expand All @@ -76,14 +76,14 @@ class ObjectType extends Type implements OutputType, CompositeType, NullableType
/**
* Lazily initialized.
*
* @var InterfaceType[]
* @var array<int, InterfaceType>
*/
private $interfaces;

/**
* Lazily initialized.
*
* @var InterfaceType[]
* @var array<string, InterfaceType>
*/
private $interfaceMap;

Expand Down Expand Up @@ -180,7 +180,7 @@ public function implementsInterface(InterfaceType $interfaceType) : bool
}

/**
* @return InterfaceType[]
* @return array<int, InterfaceType>
*/
public function getInterfaces() : array
{
Expand Down
4 changes: 2 additions & 2 deletions src/Type/Introspection.php
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ static function (FieldDefinition $field) : bool {
'interfaces' => [
'type' => Type::listOf(Type::nonNull(self::_type())),
'resolve' => static function ($type) : ?array {
if ($type instanceof ObjectType) {
if ($type instanceof ObjectType || $type instanceof InterfaceType) {
return $type->getInterfaces();
}

Expand Down Expand Up @@ -446,7 +446,7 @@ public static function _typeKind()
],
'INTERFACE' => [
'value' => TypeKind::INTERFACE,
'description' => 'Indicates this type is an interface. `fields` and `possibleTypes` are valid fields.',
'description' => 'Indicates this type is an interface. `fields`, `interfaces`, and `possibleTypes` are valid fields.',
],
'UNION' => [
'value' => TypeKind::UNION,
Expand Down
Loading

0 comments on commit c4d2f82

Please sign in to comment.