diff --git a/benchmarks/HugeSchemaBench.php b/benchmarks/HugeSchemaBench.php index d641bee..fa26228 100644 --- a/benchmarks/HugeSchemaBench.php +++ b/benchmarks/HugeSchemaBench.php @@ -22,7 +22,7 @@ class HugeSchemaBench private $schemaBuilder; /** - * @var array + * @var Schema\Descriptor */ private $descriptor; @@ -54,7 +54,9 @@ class HugeSchemaBench public function benchSchema() { $this->schemaBuilder - ->buildSchema(); + ->buildSchema() + ->getTypeMap() + ; } public function benchSchemaLazy() @@ -75,16 +77,13 @@ class HugeSchemaBench private function createLazySchema() { - $strategy = new LazyResolution( - $this->descriptor, - function($name) { - return $this->schemaBuilder->loadType($name); - } + return new Schema( + Schema\Config::create() + ->setQuery($this->schemaBuilder->buildQueryType()) + // ->setDescriptor($this->descriptor) + ->setTypeLoader(function($name) { + return $this->schemaBuilder->loadType($name); + }) ); - - return new Schema([ - 'query' => $this->schemaBuilder->buildQueryType(), - 'typeResolution' => $strategy, - ]); } } diff --git a/src/Executor/Executor.php b/src/Executor/Executor.php index a7208a8..5e5ea07 100644 --- a/src/Executor/Executor.php +++ b/src/Executor/Executor.php @@ -5,6 +5,7 @@ use GraphQL\Error\Error; use GraphQL\Error\InvariantViolation; use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter; use GraphQL\Executor\Promise\Promise; +use GraphQL\GraphQL; use GraphQL\Language\AST\DocumentNode; use GraphQL\Language\AST\FieldNode; use GraphQL\Language\AST\FragmentDefinitionNode; @@ -18,6 +19,7 @@ use GraphQL\Schema; use GraphQL\Type\Definition\AbstractType; use GraphQL\Type\Definition\Directive; use GraphQL\Type\Definition\FieldDefinition; +use GraphQL\Type\Definition\InterfaceType; use GraphQL\Type\Definition\LeafType; use GraphQL\Type\Definition\ListOfType; use GraphQL\Type\Definition\NonNull; @@ -1021,6 +1023,16 @@ class Executor $runtimeType = $returnType->resolveType($result, $exeContext->contextValue, $info); if (null === $runtimeType) { + if ($returnType instanceof InterfaceType && !$exeContext->schema->getConfig()->descriptor && + !GraphQL::isIgnoredError(GraphQL::WARNING_ON_IMPLEMENTATION_RESOLUTION)) { + trigger_error( + "GraphQL Interface Type `{$returnType->name}` returned `null` from it`s `resolveType` function ". + 'for value: ' . Utils::printSafe($result) . '. Switching to slow resolution method using `isTypeOf` ' . + 'of all possible implementations. It degrades query performance significantly. '. + ' Make sure your `resolveType` always returns valid implementation or throws.', + E_USER_WARNING + ); + } $runtimeType = self::defaultTypeResolver($result, $exeContext->contextValue, $info, $returnType); } diff --git a/src/GraphQL.php b/src/GraphQL.php index db190a1..4ec1e88 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -16,6 +16,20 @@ use GraphQL\Validator\Rules\QueryComplexity; class GraphQL { + const WARNING_ON_IMPLEMENTATION_RESOLUTION = 1; + + private static $ignoredErrors = []; + + public static function setIgnoreError($errorCode, $set = true) + { + self::$ignoredErrors[$errorCode] = $set ? true : null; + } + + public static function isIgnoredError($errorCode) + { + return isset(self::$ignoredErrors[$errorCode]); + } + /** * This is the primary entry point function for fulfilling GraphQL operations * by parsing, validating, and executing a GraphQL document along side a diff --git a/src/Schema.php b/src/Schema.php index ee208e7..985d7d3 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -1,13 +1,15 @@ > - */ - private $possibleTypeMap = []; - - /** - * @var Resolution - */ - private $typeResolutionStrategy; - - /** - * Required for `getTypeMap()` and `getDescriptor()` methods + * Contains actual descriptor for this schema * - * @var EagerResolution + * @var Descriptor */ - private $eagerTypeResolutionStrategy; + private $descriptor; + + /** + * Contains currently resolved schema types + * + * @var Type[] + */ + private $resolvedTypes = []; + + /** + * True when $resolvedTypes contain all possible schema types + * + * @var bool + */ + private $fullyLoaded = false; /** * Schema constructor. - * @param array $config + * + * @param array|Config $config */ public function __construct($config = null) { @@ -74,63 +81,36 @@ class Schema ); list($queryType, $mutationType, $subscriptionType) = func_get_args() + [null, null, null]; - $config = [ - 'query' => $queryType, - 'mutation' => $mutationType, - 'subscription' => $subscriptionType - ]; + $config = Config::create() + ->setQuery($queryType) + ->setMutation($mutationType) + ->setSubscription($subscriptionType) + ; + } else if (is_array($config)) { + $config = Config::create($config); } - $config += [ - 'query' => null, - 'mutation' => null, - 'subscription' => null, - 'types' => [], - 'directives' => null, - 'typeResolution' => null - ]; - - $this->init($config); - } - - /** - * @param array $config - */ - private function init(array $config) - { Utils::invariant( - $config['query'] instanceof ObjectType, - "Schema query must be Object Type but got: " . Utils::getVariableType($config['query']) + $config instanceof Config, + 'Schema constructor expects instance of GraphQL\Schema\Config or an array with keys: %s; but got: %s', + implode(', ', [ + 'query', + 'mutation', + 'subscription', + 'types', + 'directives', + 'descriptor', + 'typeLoader' + ]), + Utils::getVariableType($config) ); Utils::invariant( - !$config['mutation'] || $config['mutation'] instanceof ObjectType, - "Schema mutation must be Object Type if provided but got: " . Utils::getVariableType($config['mutation']) - ); - - Utils::invariant( - !$config['subscription'] || $config['subscription'] instanceof ObjectType, - "Schema subscription must be Object Type if provided but got: " . Utils::getVariableType($config['subscription']) - ); - - Utils::invariant( - !$config['types'] || is_array($config['types']), - "Schema types must be Array if provided but got: " . Utils::getVariableType($config['types']) - ); - - Utils::invariant( - !$config['directives'] || (is_array($config['directives']) && Utils::every($config['directives'], function($d) {return $d instanceof Directive;})), - "Schema directives must be Directive[] if provided but got " . Utils::getVariableType($config['directives']) - ); - - Utils::invariant( - !$config['typeResolution'] || $config['typeResolution'] instanceof Resolution, - "Type resolution strategy is expected to be instance of GraphQL\\Type\\Resolution, but got " . - Utils::getVariableType($config['typeResolution']) + $config->query instanceof ObjectType, + "Schema query must be Object Type but got: " . Utils::getVariableType($config->query) ); $this->config = $config; - $this->typeResolutionStrategy = $config['typeResolution'] ?: $this->getEagerTypeResolutionStrategy(); } /** @@ -138,53 +118,126 @@ class Schema */ public function getQueryType() { - return $this->config['query']; + return $this->config->query; } /** - * @return ObjectType + * @return ObjectType|null */ public function getMutationType() { - return $this->config['mutation']; + return $this->config->mutation; } /** - * @return ObjectType + * @return ObjectType|null */ public function getSubscriptionType() { - return $this->config['subscription']; + return $this->config->subscription; + } + + /** + * @return Config + */ + public function getConfig() + { + return $this->config; } /** * Returns full map of types in this schema. - * Note: internally it will eager-load all types using GraphQL\Type\EagerResolution strategy * * @return Type[] */ public function getTypeMap() { - return $this->getEagerTypeResolutionStrategy()->getTypeMap(); + if (!$this->fullyLoaded) { + if ($this->config->descriptor && $this->config->typeLoader) { + $typesToResolve = array_diff_key($this->config->descriptor->typeMap, $this->resolvedTypes); + foreach ($typesToResolve as $typeName => $_) { + $this->resolvedTypes[$typeName] = $this->loadType($typeName); + } + } else { + $this->resolvedTypes = $this->collectAllTypes(); + } + $this->fullyLoaded = true; + } + return $this->resolvedTypes; } /** + * Returns type by it's name + * * @param string $name * @return Type */ public function getType($name) { - return $this->typeResolutionStrategy->resolveType($name); + return $this->resolveType($name); } /** - * Returns serializable schema representation suitable for GraphQL\Type\LazyResolution + * Returns serializable schema descriptor which can be passed later + * to Schema config to enable a set of performance optimizations * - * @return array + * @return Descriptor */ public function getDescriptor() { - return $this->getEagerTypeResolutionStrategy()->getDescriptor(); + if ($this->descriptor) { + return $this->descriptor; + } + if ($this->config->descriptor) { + return $this->config->descriptor; + } + return $this->descriptor = $this->buildDescriptor(); + } + + /** + * @return Descriptor + */ + public function buildDescriptor() + { + $this->resolvedTypes = $this->collectAllTypes(); + $this->fullyLoaded = true; + + $descriptor = new Descriptor(); + $descriptor->version = '1.0'; + $descriptor->created = time(); + + foreach ($this->resolvedTypes as $type) { + if ($type instanceof ObjectType) { + foreach ($type->getInterfaces() as $interface) { + $descriptor->possibleTypeMap[$interface->name][$type->name] = 1; + } + } else if ($type instanceof UnionType) { + foreach ($type->getTypes() as $innerType) { + $descriptor->possibleTypeMap[$type->name][$innerType->name] = 1; + } + } + $descriptor->typeMap[$type->name] = 1; + } + + return $this->descriptor = $descriptor; + } + + private function collectAllTypes() + { + $initialTypes = [ + $this->config->query, + $this->config->mutation, + $this->config->subscription, + Introspection::_schema() + ]; + if (!empty($this->config->types)) { + $initialTypes = array_merge($initialTypes, $this->config->types); + } + $typeMap = []; + foreach ($initialTypes as $type) { + $typeMap = Utils\TypeInfo::extractTypes($type, $typeMap); + } + return $typeMap + Type::getInternalTypes(); } /** @@ -193,7 +246,47 @@ class Schema */ public function getPossibleTypes(AbstractType $abstractType) { - return $this->typeResolutionStrategy->resolvePossibleTypes($abstractType); + if ($abstractType instanceof UnionType) { + return $abstractType->getTypes(); + } + + /** @var InterfaceType $abstractType */ + $descriptor = $this->getDescriptor(); + + $result = []; + if (isset($descriptor->possibleTypeMap[$abstractType->name])) { + foreach ($descriptor->possibleTypeMap[$abstractType->name] as $typeName => $_) { + $result[] = $this->resolveType($typeName); + } + } + return $result; + } + + public function resolveType($typeOrName) + { + if ($typeOrName instanceof Type) { + if ($typeOrName->name && !isset($this->resolvedTypes[$typeOrName->name])) { + $this->resolvedTypes[$typeOrName->name] = $typeOrName; + } + return $typeOrName; + } + if (!isset($this->resolvedTypes[$typeOrName])) { + $this->resolvedTypes[$typeOrName] = $this->loadType($typeOrName); + } + return $this->resolvedTypes[$typeOrName]; + } + + private function loadType($typeName) + { + $typeLoader = $this->config->typeLoader; + + if (!$typeLoader) { + return $this->defaultTypeLoader($typeName); + } + + $type = $typeLoader($typeName); + // TODO: validate returned value + return $type; } /** @@ -203,23 +296,16 @@ class Schema */ public function isPossibleType(AbstractType $abstractType, ObjectType $possibleType) { - if (!isset($this->possibleTypeMap[$abstractType->name])) { - $tmp = []; - foreach ($this->getPossibleTypes($abstractType) as $type) { - $tmp[$type->name] = 1; - } - - Utils::invariant( - !empty($tmp), - 'Could not find possible implementing types for $%s ' . - 'in schema. Check that schema.types is defined and is an array of ' . - 'all possible types in the schema.', - $abstractType->name - ); - - $this->possibleTypeMap[$abstractType->name] = $tmp; + if ($this->config->descriptor) { + return !empty($this->config->descriptor->possibleTypeMap[$abstractType->name][$possibleType->name]); } - return !empty($this->possibleTypeMap[$abstractType->name][$possibleType->name]); + + if ($abstractType instanceof InterfaceType) { + return $possibleType->implementsInterface($abstractType); + } + + /** @var UnionType $abstractType */ + return $abstractType->isPossibleType($possibleType); } /** @@ -227,7 +313,7 @@ class Schema */ public function getDirectives() { - return isset($this->config['directives']) ? $this->config['directives'] : GraphQL::getInternalDirectives(); + return $this->config->directives ?: GraphQL::getInternalDirectives(); } /** @@ -244,25 +330,20 @@ class Schema return null; } - private function getEagerTypeResolutionStrategy() + /** + * @param $typeName + * @return Type + */ + private function defaultTypeLoader($typeName) { - if (!$this->eagerTypeResolutionStrategy) { - if ($this->typeResolutionStrategy instanceof EagerResolution) { - $this->eagerTypeResolutionStrategy = $this->typeResolutionStrategy; - } else { - // Build type map now to detect any errors within this schema. - $initialTypes = [ - $this->config['query'], - $this->config['mutation'], - $this->config['subscription'], - Introspection::_schema() - ]; - if (!empty($this->config['types'])) { - $initialTypes = array_merge($initialTypes, $this->config['types']); - } - $this->eagerTypeResolutionStrategy = new EagerResolution($initialTypes); - } + // Default type loader simply fallbacks to collecting all types + if (!$this->fullyLoaded) { + $this->resolvedTypes = $this->collectAllTypes(); + $this->fullyLoaded = true; } - return $this->eagerTypeResolutionStrategy; + if (!isset($this->resolvedTypes[$typeName])) { + return null; + } + return $this->resolvedTypes[$typeName]; } } diff --git a/src/Schema/Config.php b/src/Schema/Config.php new file mode 100644 index 0000000..496e9d7 --- /dev/null +++ b/src/Schema/Config.php @@ -0,0 +1,262 @@ +setQuery($options['query']); + } + + if (isset($options['mutation'])) { + Utils::invariant( + $options['mutation'] instanceof ObjectType, + "Schema mutation must be Object Type if provided but got: " . Utils::getVariableType($options['mutation']) + ); + $config->setMutation($options['mutation']); + } + + if (isset($options['subscription'])) { + Utils::invariant( + $options['subscription'] instanceof ObjectType, + "Schema subscription must be Object Type if provided but got: " . Utils::getVariableType($options['subscription']) + ); + $config->setSubscription($options['subscription']); + } + + if (isset($options['types'])) { + Utils::invariant( + is_array($options['types']), + "Schema types must be array if provided but got: " . Utils::getVariableType($options['types']) + ); + $config->setTypes($options['types']); + } + + if (isset($options['directives'])) { + Utils::invariant( + is_array($options['directives']), + "Schema directives must be array if provided but got: " . Utils::getVariableType($options['directives']) + ); + $config->setDirectives($options['directives']); + } + + if (isset($options['typeLoader'])) { + Utils::invariant( + is_callable($options['typeLoader']), + "Schema type loader must be callable if provided but got: " . Utils::getVariableType($options['typeLoader']) + ); + } + + if (isset($options['descriptor'])) { + Utils::invariant( + $options['descriptor'] instanceof Descriptor, + "Schema descriptor must be instance of GraphQL\\Schema\\Descriptor but got: " . Utils::getVariableType($options['descriptor']) + ); + } + } + + return $config; + } + + /** + * @return ObjectType + */ + public function getQuery() + { + return $this->query; + } + + /** + * @param ObjectType $query + * @return Config + */ + public function setQuery(ObjectType $query) + { + $this->query = $query; + return $this; + } + + /** + * @return ObjectType + */ + public function getMutation() + { + return $this->mutation; + } + + /** + * @param ObjectType $mutation + * @return Config + */ + public function setMutation(ObjectType $mutation) + { + $this->mutation = $mutation; + return $this; + } + + /** + * @return ObjectType + */ + public function getSubscription() + { + return $this->subscription; + } + + /** + * @param ObjectType $subscription + * @return Config + */ + public function setSubscription(ObjectType $subscription) + { + $this->subscription = $subscription; + return $this; + } + + /** + * @return Type[] + */ + public function getTypes() + { + return $this->types; + } + + /** + * @param Type[] $types + * @return Config + */ + public function setTypes($types) + { + foreach ($types as $index => $type) { + Utils::invariant( + $type instanceof Type, + 'Schema types must be GraphQL\Type\Definition\Type[], but entry at index "%s" is "%s"', + $index, + Utils::getVariableType($type) + ); + } + + $this->types = $types; + return $this; + } + + /** + * @return Directive[] + */ + public function getDirectives() + { + return $this->directives; + } + + /** + * @param Directive[] $directives + * @return Config + */ + public function setDirectives(array $directives) + { + foreach ($directives as $index => $directive) { + Utils::invariant( + $directive instanceof Directive, + 'Schema directives must be GraphQL\Type\Definition\Directive[] if provided but but entry at index "%s" is "%s"', + $index, + Utils::getVariableType($directive) + ); + } + + $this->directives = $directives; + return $this; + } + + /** + * @return Descriptor + */ + public function getDescriptor() + { + return $this->descriptor; + } + + /** + * @param Descriptor $descriptor + * @return Config + */ + public function setDescriptor(Descriptor $descriptor) + { + $this->descriptor = $descriptor; + return $this; + } + + /** + * @return callable + */ + public function getTypeLoader() + { + return $this->typeLoader; + } + + /** + * @param callable $typeLoader + * @return Config + */ + public function setTypeLoader(callable $typeLoader) + { + $this->typeLoader = $typeLoader; + return $this; + } +} diff --git a/src/Schema/Descriptor.php b/src/Schema/Descriptor.php new file mode 100644 index 0000000..7afa458 --- /dev/null +++ b/src/Schema/Descriptor.php @@ -0,0 +1,61 @@ + $this->typeMap, + 'version' => $this->version, + 'possibleTypeMap' => $this->possibleTypeMap, + 'created' => $this->created + ]; + } + + /** + * Specify data which should be serialized to JSON + * @link http://php.net/manual/en/jsonserializable.jsonserialize.php + * @return mixed data which can be serialized by json_encode, + * which is a value of any type other than a resource. + * @since 5.4.0 + */ + function jsonSerialize() + { + return $this->toArray(); + } +} diff --git a/src/Type/Definition/ObjectType.php b/src/Type/Definition/ObjectType.php index 00e076d..bcdd250 100644 --- a/src/Type/Definition/ObjectType.php +++ b/src/Type/Definition/ObjectType.php @@ -57,6 +57,11 @@ class ObjectType extends Type implements OutputType, CompositeType */ private $interfaces; + /** + * @var array + */ + private $interfaceMap = []; + /** * Keeping reference of config for late bindings and custom app-level metadata * @@ -148,7 +153,9 @@ class ObjectType extends Type implements OutputType, CompositeType if (!$iface instanceof InterfaceType) { throw new InvariantViolation(sprintf('Expecting interface type, got %s', Utils::printSafe($iface))); } + // TODO: return interfaceMap vs interfaces. Possibly breaking change? $this->interfaces[] = $iface; + $this->interfaceMap[$iface->name] = $iface; } } return $this->interfaces; @@ -161,7 +168,8 @@ class ObjectType extends Type implements OutputType, CompositeType public function implementsInterface($iface) { $iface = Type::resolve($iface); - return !!Utils::find($this->getInterfaces(), function($implemented) use ($iface) {return $iface === $implemented;}); + $this->getInterfaces(); + return isset($this->interfaceMap[$iface->name]); } /** diff --git a/src/Utils/BuildSchema.php b/src/Utils/BuildSchema.php index e2463ff..1cfc7ca 100644 --- a/src/Utils/BuildSchema.php +++ b/src/Utils/BuildSchema.php @@ -17,6 +17,7 @@ use GraphQL\Language\AST\TypeDefinitionNode; use GraphQL\Language\AST\TypeNode; use GraphQL\Language\AST\UnionTypeDefinitionNode; use GraphQL\Language\Parser; +use GraphQL\Language\Source; use GraphQL\Language\Token; use GraphQL\Schema; use GraphQL\Type\Definition\Directive; @@ -227,7 +228,7 @@ class BuildSchema $directives[] = Directive::deprecatedDirective(); } - return new Schema([ + $schema = new Schema([ 'query' => $this->getObjectType($this->nodeMap[$queryTypeName]), 'mutation' => $mutationTypeName ? $this->getObjectType($this->nodeMap[$mutationTypeName]) : @@ -238,6 +239,12 @@ class BuildSchema 'types' => $types, 'directives' => $directives, ]); + + // Types in schema are loaded lazily, but we need full scan to ensure that schema is consistent + // Following statement will force Schema to scan all types and fields: + // TODO: replace this call with schema validator once it's ready + $schema->getTypeMap(); + return $schema; } private function getDirective(DirectiveDefinitionNode $directiveNode) @@ -510,7 +517,7 @@ class BuildSchema * document. * * @param Source|string $source - * @return + * @return Schema */ public static function build($source) { diff --git a/src/Validator/Rules/FieldsOnCorrectType.php b/src/Validator/Rules/FieldsOnCorrectType.php index 524b303..9051697 100644 --- a/src/Validator/Rules/FieldsOnCorrectType.php +++ b/src/Validator/Rules/FieldsOnCorrectType.php @@ -39,21 +39,8 @@ class FieldsOnCorrectType if ($type) { $fieldDef = $context->getFieldDef(); if (!$fieldDef) { - // This isn't valid. Let's find suggestions, if any. - $suggestedTypes = []; - if ($type instanceof AbstractType) { - $schema = $context->getSchema(); - $suggestedTypes = self::getSiblingInterfacesIncludingField( - $schema, - $type, - $node->name->value - ); - $suggestedTypes = array_merge($suggestedTypes, - self::getImplementationsIncludingField($schema, $type, $node->name->value) - ); - } $context->reportError(new Error( - static::undefinedFieldMessage($node->name->value, $type->name, $suggestedTypes), + static::undefinedFieldMessage($node->name->value, $type->name), [$node] )); } @@ -61,49 +48,4 @@ class FieldsOnCorrectType } ]; } - - /** - * Return implementations of `type` that include `fieldName` as a valid field. - * - * @param Schema $schema - * @param AbstractType $type - * @param $fieldName - * @return array - */ - static function getImplementationsIncludingField(Schema $schema, AbstractType $type, $fieldName) - { - $types = $schema->getPossibleTypes($type); - $types = Utils::filter($types, function($t) use ($fieldName) {return isset($t->getFields()[$fieldName]);}); - $types = Utils::map($types, function($t) {return $t->name;}); - sort($types); - return $types; - } - - /** - * Go through all of the implementations of type, and find other interaces - * that they implement. If those interfaces include `field` as a valid field, - * return them, sorted by how often the implementations include the other - * interface. - */ - static function getSiblingInterfacesIncludingField(Schema $schema, AbstractType $type, $fieldName) - { - $types = $schema->getPossibleTypes($type); - $suggestedInterfaces = array_reduce($types, function ($acc, $t) use ($fieldName) { - foreach ($t->getInterfaces() as $i) { - if (empty($i->getFields()[$fieldName])) { - continue; - } - if (!isset($acc[$i->name])) { - $acc[$i->name] = 0; - } - $acc[$i->name] += 1; - } - return $acc; - }, []); - $suggestedInterfaceNames = array_keys($suggestedInterfaces); - usort($suggestedInterfaceNames, function($a, $b) use ($suggestedInterfaces) { - return $suggestedInterfaces[$b] - $suggestedInterfaces[$a]; - }); - return $suggestedInterfaceNames; - } } diff --git a/src/Validator/Rules/PossibleFragmentSpreads.php b/src/Validator/Rules/PossibleFragmentSpreads.php index 8d910dc..a9b494d 100644 --- a/src/Validator/Rules/PossibleFragmentSpreads.php +++ b/src/Validator/Rules/PossibleFragmentSpreads.php @@ -7,7 +7,12 @@ use GraphQL\Language\AST\FragmentSpreadNode; use GraphQL\Language\AST\InlineFragmentNode; use GraphQL\Language\AST\Node; use GraphQL\Language\AST\NodeKind; -use GraphQL\Utils; +use GraphQL\Schema; +use GraphQL\Type\Definition\AbstractType; +use GraphQL\Type\Definition\CompositeType; +use GraphQL\Type\Definition\InterfaceType; +use GraphQL\Type\Definition\ObjectType; +use GraphQL\Type\Definition\UnionType; use GraphQL\Validator\ValidationContext; use GraphQL\Utils\TypeInfo; @@ -30,7 +35,7 @@ class PossibleFragmentSpreads $fragType = $context->getType(); $parentType = $context->getParentType(); - if ($fragType && $parentType && !TypeInfo::doTypesOverlap($context->getSchema(), $fragType, $parentType)) { + if ($fragType && $parentType && !$this->doTypesOverlap($context->getSchema(), $fragType, $parentType)) { $context->reportError(new Error( self::typeIncompatibleAnonSpreadMessage($parentType, $fragType), [$node] @@ -42,7 +47,7 @@ class PossibleFragmentSpreads $fragType = $this->getFragmentType($context, $fragName); $parentType = $context->getParentType(); - if ($fragType && $parentType && !TypeInfo::doTypesOverlap($context->getSchema(), $fragType, $parentType)) { + if ($fragType && $parentType && !$this->doTypesOverlap($context->getSchema(), $fragType, $parentType)) { $context->reportError(new Error( self::typeIncompatibleSpreadMessage($fragName, $parentType, $fragType), [$node] @@ -57,4 +62,72 @@ class PossibleFragmentSpreads $frag = $context->getFragment($name); return $frag ? TypeInfo::typeFromAST($context->getSchema(), $frag->typeCondition) : null; } + + private function doTypesOverlap(Schema $schema, CompositeType $fragType, CompositeType $parentType) + { + // Checking in the order of the most frequently used scenarios: + // Parent type === fragment type + if ($parentType === $fragType) { + return true; + } + + // Parent type is interface or union, fragment type is object type + if ($parentType instanceof AbstractType && $fragType instanceof ObjectType) { + return $schema->isPossibleType($parentType, $fragType); + } + + // Parent type is object type, fragment type is interface (or rather rare - union) + if ($parentType instanceof ObjectType && $fragType instanceof AbstractType) { + return $schema->isPossibleType($fragType, $parentType); + } + + // Both are object types: + if ($parentType instanceof ObjectType && $fragType instanceof ObjectType) { + return $parentType === $fragType; + } + + // Both are interfaces + // This case may be assumed valid only when implementations of two interfaces intersect + // But we don't have information about all implementations at runtime + // (getting this information via $schema->getPossibleTypes() requires scanning through whole schema + // which is very costly to do at each request due to PHP "shared nothing" architecture) + // + // So in this case we just make it pass - invalid fragment spreads will be simply ignored during execution + // See also https://github.com/webonyx/graphql-php/issues/69#issuecomment-283954602 + if ($parentType instanceof InterfaceType && $fragType instanceof InterfaceType) { + return true; + + // Note that there is one case when we do have information about all implementations: + // When schema descriptor is defined ($schema->hasDescriptor()) + // BUT we must avoid situation when some query that worked in development had suddenly stopped + // working in production. So staying consistent and always validate. + } + + // Interface within union + if ($parentType instanceof UnionType && $fragType instanceof InterfaceType) { + foreach ($parentType->getTypes() as $type) { + if ($type->implementsInterface($fragType)) { + return true; + } + } + } + + if ($parentType instanceof InterfaceType && $fragType instanceof UnionType) { + foreach ($fragType->getTypes() as $type) { + if ($type->implementsInterface($parentType)) { + return true; + } + } + } + + if ($parentType instanceof UnionType && $fragType instanceof UnionType) { + foreach ($fragType->getTypes() as $type) { + if ($parentType->isPossibleType($type)) { + return true; + } + } + } + + return false; + } } diff --git a/tests/Executor/AbstractTest.php b/tests/Executor/AbstractTest.php index 9babcb9..6f4a9b0 100644 --- a/tests/Executor/AbstractTest.php +++ b/tests/Executor/AbstractTest.php @@ -89,7 +89,29 @@ class AbstractTest extends \PHPUnit_Framework_TestCase ] ]); - $this->assertEquals($expected, Executor::execute($schema, Parser::parse($query))); + GraphQL::setIgnoreError(GraphQL::WARNING_ON_IMPLEMENTATION_RESOLUTION); + $result = Executor::execute($schema, Parser::parse($query)); + $this->assertEquals($expected, $result); + + GraphQL::setIgnoreError(GraphQL::WARNING_ON_IMPLEMENTATION_RESOLUTION, false); + $result = Executor::execute($schema, Parser::parse($query)); + $this->assertEquals(2, count($result->errors)); + $this->assertInstanceOf('PHPUnit_Framework_Error_Warning', $result->errors[0]->getPrevious()); + $this->assertInstanceOf('PHPUnit_Framework_Error_Warning', $result->errors[1]->getPrevious()); + + $this->assertEquals( + 'GraphQL Interface Type `Pet` returned `null` from it`s `resolveType` function for value: '. + 'instance of GraphQL\Tests\Executor\Dog. Switching to slow resolution method using `isTypeOf` of '. + 'all possible implementations. It degrades query performance significantly. '. + 'Make sure your `resolveType` always returns valid implementation or throws.', + $result->errors[0]->getMessage()); + + $this->assertEquals( + 'GraphQL Interface Type `Pet` returned `null` from it`s `resolveType` function for value: '. + 'instance of GraphQL\Tests\Executor\Cat. Switching to slow resolution method using `isTypeOf` of '. + 'all possible implementations. It degrades query performance significantly. '. + 'Make sure your `resolveType` always returns valid implementation or throws.', + $result->errors[1]->getMessage()); } /** diff --git a/tests/Executor/UnionInterfaceTest.php b/tests/Executor/UnionInterfaceTest.php index 8c2c0f8..7d6ac5c 100644 --- a/tests/Executor/UnionInterfaceTest.php +++ b/tests/Executor/UnionInterfaceTest.php @@ -256,7 +256,9 @@ class UnionInterfaceTest extends \PHPUnit_Framework_TestCase ] ]; + GraphQL::setIgnoreError(GraphQL::WARNING_ON_IMPLEMENTATION_RESOLUTION); $this->assertEquals($expected, Executor::execute($this->schema, $ast, $this->john)->toArray()); + GraphQL::setIgnoreError(GraphQL::WARNING_ON_IMPLEMENTATION_RESOLUTION, false); } /** @@ -292,7 +294,9 @@ class UnionInterfaceTest extends \PHPUnit_Framework_TestCase ] ]; + GraphQL::setIgnoreError(GraphQL::WARNING_ON_IMPLEMENTATION_RESOLUTION); $this->assertEquals($expected, Executor::execute($this->schema, $ast, $this->john)->toArray()); + GraphQL::setIgnoreError(GraphQL::WARNING_ON_IMPLEMENTATION_RESOLUTION, false); } /** @@ -347,7 +351,9 @@ class UnionInterfaceTest extends \PHPUnit_Framework_TestCase ] ]; + GraphQL::setIgnoreError(GraphQL::WARNING_ON_IMPLEMENTATION_RESOLUTION); $this->assertEquals($expected, Executor::execute($this->schema, $ast, $this->john)->toArray()); + GraphQL::setIgnoreError(GraphQL::WARNING_ON_IMPLEMENTATION_RESOLUTION, false); } /** diff --git a/tests/Type/DefinitionTest.php b/tests/Type/DefinitionTest.php index f6d7674..aa7c6fb 100644 --- a/tests/Type/DefinitionTest.php +++ b/tests/Type/DefinitionTest.php @@ -579,6 +579,8 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase 'types' => [$user, $blog] ]); + $this->assertFalse($called); + $schema->getType('Query'); $this->assertTrue($called); $this->assertEquals([$node], $blog->getInterfaces()); @@ -619,8 +621,8 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase 'mutation' => $someMutation ]); - $this->assertTrue($called); $this->assertSame($inputObject, $schema->getType('InputObject')); + $this->assertTrue($called); $this->assertEquals(count($inputObject->getFields()), 2); $this->assertSame($inputObject->getField('nested')->getType(), $inputObject); $this->assertSame($someMutation->getField('mutateSomething')->getArg('input')->getType(), $inputObject); @@ -651,8 +653,8 @@ class DefinitionTest extends \PHPUnit_Framework_TestCase 'query' => $query ]); - $this->assertTrue($called); $this->assertSame($interface, $schema->getType('SomeInterface')); + $this->assertTrue($called); $this->assertEquals(count($interface->getFields()), 2); $this->assertSame($interface->getField('nested')->getType(), $interface); $this->assertSame($interface->getField('value')->getType(), Type::string()); diff --git a/tests/Validator/FieldsOnCorrectTypeTest.php b/tests/Validator/FieldsOnCorrectTypeTest.php index 1c99dce..7d4b78b 100644 --- a/tests/Validator/FieldsOnCorrectTypeTest.php +++ b/tests/Validator/FieldsOnCorrectTypeTest.php @@ -208,7 +208,8 @@ class FieldsOnCorrectTypeTest extends TestCase fragment definedOnImplementorsButNotInterface on Pet { nickname }', - [$this->undefinedField('nickname', 'Pet', [ 'Cat', 'Dog' ], 3, 9)] + //[$this->undefinedField('nickname', 'Pet', [ 'Cat', 'Dog' ], 3, 9)] + [$this->undefinedField('nickname', 'Pet', [ ], 3, 9)] ); } @@ -246,7 +247,8 @@ class FieldsOnCorrectTypeTest extends TestCase fragment definedOnImplementorsQueriedOnUnion on CatOrDog { name }', - [$this->undefinedField('name', 'CatOrDog', [ 'Being', 'Pet', 'Canine', 'Cat', 'Dog' ], 3, 9)] + //[$this->undefinedField('name', 'CatOrDog', [ 'Being', 'Pet', 'Canine', 'Cat', 'Dog' ], 3, 9)] + [$this->undefinedField('name', 'CatOrDog', [ ], 3, 9)] ); } diff --git a/tests/Validator/PossibleFragmentSpreadsTest.php b/tests/Validator/PossibleFragmentSpreadsTest.php index 7b51960..d186157 100644 --- a/tests/Validator/PossibleFragmentSpreadsTest.php +++ b/tests/Validator/PossibleFragmentSpreadsTest.php @@ -238,14 +238,15 @@ class PossibleFragmentSpreadsTest extends TestCase */ public function testInterfaceIntoNonOverlappingInterface() { - $this->expectFailsRule(new PossibleFragmentSpreads, ' + // Ideally this should fail, but our new lazy schema doesn't scan through all types and fields + // So we don't have enough knowledge to check interface intersection and always allow this to pass: + + $this->expectPassesRule(new PossibleFragmentSpreads, ' fragment invalidInterfaceWithinInterface on Pet { ...intelligentFragment } fragment intelligentFragment on Intelligent { iq } - ', - [$this->error('intelligentFragment', 'Pet', 'Intelligent', 3, 9)] - ); + '); } /** @@ -253,13 +254,14 @@ class PossibleFragmentSpreadsTest extends TestCase */ public function testInterfaceIntoNonOverlappingInterfaceInInlineFragment() { - $this->expectFailsRule(new PossibleFragmentSpreads, ' + // Ideally this should fail, but our new lazy schema doesn't scan through all types and fields + // So we don't have enough knowledge to check interface intersection and always allow this to pass: + + $this->expectPassesRule(new PossibleFragmentSpreads, ' fragment invalidInterfaceWithinInterfaceAnon on Pet { ...on Intelligent { iq } } - ', - [$this->errorAnon('Pet', 'Intelligent', 3, 9)] - ); + '); } /**