Merge pull request #406 from mll-lab/fix-default-namespaces

Fix the default namespace resolution

src/Execution/QueryFilter.php

@@ -41,7 +41,7 @@ class QueryFilter
     public static function getInstance(FieldValue $value): QueryFilter
     {
         $handler = static::QUERY_FILTER_KEY
-            . '.' . strtolower($value->getNodeName())
+            . '.' . strtolower($value->getParentName())
             . '.' . strtolower($value->getFieldName());
 
         // Get an existing instance or register a new one

src/Schema/AST/ASTBuilder.php

@@ -54,9 +54,11 @@ public static function generate(string $schema): DocumentAST
     protected static function applyNodeManipulators(DocumentAST $document): DocumentAST
     {
         return $document
-            ->typeExtensionDefinitions()
-            // This is just temporarily merged together
-            ->concat($document->typeDefinitions())
+            ->typeDefinitions()
+            // Iterate over both of those at once, as it does not matter at this point
+            ->concat(
+                $document->typeExtensions()
+            )
             ->reduce(
                 function (DocumentAST $document, Node $node) {
                     $nodeManipulators = resolve(DirectiveRegistry::class)->nodeManipulators($node);
@@ -86,7 +88,7 @@ function (ObjectTypeDefinitionNode $objectType) use ($document) {
                 $name = $objectType->name->value;
 
                 $objectType = $document
-                    ->typeExtensionDefinitions($name)
+                    ->extensionsForType($name)
                     ->reduce(
                         function (ObjectTypeDefinitionNode $relatedObjectType, ObjectTypeExtensionNode $typeExtension) {
                             $relatedObjectType->fields = ASTHelper::mergeUniqueNodeList(

src/Schema/AST/DocumentAST.php

@@ -35,25 +35,48 @@ class DocumentAST implements \Serializable
      *
      * @var Collection
      */
-    protected $typeExtensions;
-    
+    protected $typeExtensionsMap;
+
     /**
      * @param DocumentNode $documentNode
      */
     public function __construct(DocumentNode $documentNode)
     {
         // We can not store type extensions in the map, since they do not have unique names
-        list($this->typeExtensions, $definitionNodes) = collect($documentNode->definitions)
+        list($typeExtensions, $definitionNodes) = collect($documentNode->definitions)
             ->partition(function(DefinitionNode $definitionNode){
                 return $definitionNode instanceof TypeExtensionNode;
             });
-
+
+        $this->typeExtensionsMap = $typeExtensions
+            ->mapWithKeys(function(TypeExtensionNode $node){
+                return [$this->typeExtensionUniqueKey($node) => $node];
+            });
+
         $this->definitionMap = $definitionNodes
             ->mapWithKeys(function(DefinitionNode $node){
                return [$node->name->value => $node];
             });
     }
-
+
+    /**
+     * Return a unique key that identifies a type extension.
+     *
+     * @param TypeExtensionNode $typeExtensionNode
+     *
+     * @return string
+     */
+    protected function typeExtensionUniqueKey(TypeExtensionNode $typeExtensionNode): string
+    {
+        $fieldNames = collect($typeExtensionNode->fields)
+            ->map(function($field){
+                return $field->name->value;
+            })
+            ->implode(':');
+
+        return $typeExtensionNode->name->value . $fieldNames;
+    }
+
     /**
      * Create a new DocumentAST instance from a schema.
      *
@@ -89,16 +112,12 @@ public static function fromSource(string $schema): DocumentAST
      */
     public function serialize(): string
     {
-        return \serialize([
-            'definitionMap' => $this->definitionMap
+        return \serialize(
+            $this->definitionMap
                 ->mapWithKeys(function(DefinitionNode $node, string $key){
                     return [$key => AST::toArray($node)];
-                }),
-            'typeExtensions' => $this->typeExtensions
-                ->map(function(TypeExtensionNode $node){
-                    return AST::toArray($node);
                 })
-        ]);
+        );
     }
 
     /**
@@ -110,16 +129,10 @@ public function serialize(): string
      */
     public function unserialize($serialized)
     {
-        $unserialize = \unserialize($serialized);
-
-        $this->definitionMap = $unserialize['definitionMap']
+        $this->definitionMap = \unserialize($serialized)
             ->mapWithKeys(function(array $node, string $key){
                 return [$key => AST::fromArray($node)];
             });
-        $this->typeExtensions = $unserialize['typeExtensions']
-            ->map(function(array $node){
-                return AST::fromArray($node);
-            });
     }
 
     /**
@@ -151,28 +164,30 @@ public function directiveDefinitions(): Collection
     }
 
     /**
-     * Get all definitions for type extensions.
-     *
-     * Without a name, it simply return all TypeExtensions.
-     * If a name is given, it may return multiple type extensions
-     * that apply to a named type.
+     * Get all extensions that apply to a named type.
      *
-     * @param string|null $extendedTypeName
+     * @param string $extendedTypeName
      *
      * @return Collection
      */
-    public function typeExtensionDefinitions(string $extendedTypeName = null): Collection
+    public function extensionsForType(string $extendedTypeName): Collection
     {
-        if(is_null($extendedTypeName)){
-            return $this->typeExtensions;
-        }
-
-        return $this->typeExtensions
+        return $this->typeExtensionsMap
             ->filter(function (TypeExtensionNode $typeExtension) use ($extendedTypeName): bool {
                 return $extendedTypeName === $typeExtension->name->value;
             });
     }
 
+    /**
+     * Return all the type extensions.
+     *
+     * @return Collection
+     */
+    public function typeExtensions(): Collection
+    {
+        return $this->typeExtensionsMap;
+    }
+
     /**
      * Get all definitions for object types.
      *
@@ -290,7 +305,7 @@ public function addFieldToQueryType(FieldDefinitionNode $field): DocumentAST
 
         return $this;
     }
-    
+
     /**
      * @param DefinitionNode $newDefinition
      *
@@ -299,9 +314,11 @@ public function addFieldToQueryType(FieldDefinitionNode $field): DocumentAST
     public function setDefinition(DefinitionNode $newDefinition): DocumentAST
     {
         if($newDefinition instanceof TypeExtensionNode){
-            if(! $this->typeExtensions->search($newDefinition, true)) {
-                $this->typeExtensions->push($newDefinition);
-            }
+
+            $this->typeExtensionsMap->put(
+                $this->typeExtensionUniqueKey($newDefinition),
+                $newDefinition
+            );
         } else {
             $this->definitionMap->put(
                 $newDefinition->name->value,

src/Schema/DirectiveRegistry.php

@@ -184,6 +184,34 @@ protected function associatedDirectivesOfType(Node $node, string $directiveClass
             });
     }
 
+    /**
+     * Get a single directive of a type that belongs to an AST node.
+     *
+     * Use this for directives types that can only occur once, such as field resolvers.
+     * This throws if more than one such directive is found.
+     *
+     * @param Node $node
+     * @param string $directiveClass
+     *
+     * @throws DirectiveException
+     *
+     * @return Directive|null
+     */
+    protected function singleDirectiveOfType(Node $node, string $directiveClass)
+    {
+        $directives = $this->associatedDirectivesOfType($node, $directiveClass);
+
+        if ($directives->count() > 1) {
+            $directiveNames = $directives->implode(', ');
+
+            throw new DirectiveException(
+                "Node [{$node->name->value}] can only have one directive of type [{$directiveClass}] but found [{$directiveNames}]"
+            );
+        }
+
+        return $directives->first();
+    }
+
     /**
      * @param Node $node
      *
@@ -225,17 +253,10 @@ public function argManipulators(InputValueDefinitionNode $inputValueDefinition):
      */
     public function nodeResolver(TypeDefinitionNode $node)
     {
-        $resolvers = $this->associatedDirectivesOfType($node, NodeResolver::class);
-
-        if ($resolvers->count() > 1) {
-            $resolverNames = $resolvers->implode(', ');
-
-            throw new DirectiveException("Type [{$node->name->value}] has more then one resolver directive: [{$resolverNames}]");
-        }
-
-        return $resolvers->first();
+        /** @noinspection PhpIncompatibleReturnTypeInspection */
+        return $this->singleDirectiveOfType($node, NodeResolver::class);
     }
-    
+
     /**
      * Check if the given node has a type resolver directive handler assigned to it.
      *
@@ -287,17 +308,7 @@ public function hasFieldMiddleware($field): bool
      */
     public function fieldResolver($field)
     {
-        $resolvers = $this->associatedDirectivesOfType($field, FieldResolver::class);
-
-        if ($resolvers->count() > 1) {
-            $resolverNames = $resolvers->implode(', ');
-
-            throw new DirectiveException(
-                "Field [{$field->name->value}] has more then one resolver directive: [{$resolverNames}]"
-            );
-        }
-
-        return $resolvers->first();
+        return $this->singleDirectiveOfType($field, FieldResolver::class);
     }
 
     /**

src/Schema/Directives/BaseDirective.php

@@ -91,32 +91,13 @@ public function directiveHasArgument(string $name): bool
      *
      * @return \Closure
      */
-    public function getMethodArgument(string $argumentName): \Closure
+    public function getResolverFromArgument(string $argumentName): \Closure
     {
-        // A method argument is expected to contain a class and a method name, seperated by an @ symbol
-        // e.g. App\My\Class@methodName
-        $argumentParts = explode('@', $this->directiveArgValue($argumentName));
+        list($className, $methodName) = $this->getMethodArgumentParts($argumentName);
 
-        if (
-            count($argumentParts) !== 2
-            || empty($argumentParts[0])
-            || empty($argumentParts[1])
-        ){
-            throw new DirectiveException("Directive '{$this->name()}' must have an argument '{$argumentName}' with 'ClassName@methodName'");
-        }
-
-        $className = $this->namespaceClassName($argumentParts[0]);
-        $methodName = $argumentParts[1];
-
-        if (! method_exists($className, $methodName)) {
-            throw new DirectiveException("Method '{$methodName}' does not exist on class '{$className}'");
-        }
+        $namespacedClassName = $this->namespaceClassName($className);
 
-        // TODO convert this back once we require PHP 7.1
-        // return \Closure::fromCallable([resolve($className), $methodName]);
-        return function() use ($className, $methodName){
-            return resolve($className)->{$methodName}(...func_get_args());
-        };
+        return \construct_resolver($namespacedClassName, $methodName);
     }
 
     /**
@@ -181,4 +162,37 @@ protected function namespaceClassName(string $classCandidate, array $namespacesT
 
         return $className;
     }
+
+    /**
+     * Split a single method argument into its parts.
+     *
+     * A method argument is expected to contain a class and a method name, separated by an @ symbol.
+     * e.g. "App\My\Class@methodName"
+     * This validates that exactly two parts are given and are not empty.
+     *
+     * @param string $argumentName
+     *
+     * @throws DirectiveException
+     *
+     * @return array [string $className, string $methodName]
+     */
+    protected function getMethodArgumentParts(string $argumentName): array
+    {
+        $argumentParts = explode(
+            '@',
+            $this->directiveArgValue($argumentName)
+        );
+
+        if (
+            count($argumentParts) !== 2
+            || empty($argumentParts[0])
+            || empty($argumentParts[1])
+        ){
+            throw new DirectiveException(
+                "Directive '{$this->name()}' must have an argument '{$argumentName}' in the form 'ClassName@methodName'"
+            );
+        }
+
+        return $argumentParts;
+    }
 }

src/Schema/Directives/Fields/CacheDirective.php

@@ -35,7 +35,7 @@ public function name(): string
     public function handleField(FieldValue $value, \Closure $next)
     {
         $this->setNodeKey(
-            $value->getNode()
+            $value->getParent()
         );
 
         $value = $next($value);

src/Schema/Directives/Fields/ComplexityDirective.php

@@ -36,12 +36,12 @@ public function handleField(FieldValue $value, \Closure $next): FieldValue
         return $next(
             $value->setComplexity(
                 $this->directiveHasArgument('resolver')
-                ? $this->getMethodArgument('resolver')
-                : function ($childrenComplexity, $args) {
-                    $complexity = array_get($args, 'first', array_get($args, 'count', 1));
+                    ? $this->getResolverFromArgument('resolver')
+                    : function ($childrenComplexity, $args) {
+                        $complexity = array_get($args, 'first', array_get($args, 'count', 1));
 
-                    return $childrenComplexity * $complexity;
-                }
+                        return $childrenComplexity * $complexity;
+                    }
             )
         );
     }