Merge pull request #48 from moufmouf/complexOutputType

moufmouf · web-flow · commit 1fa00232c73c · 2019-04-23T17:27:27.000+02:00
Adding support for complex GraphQL types in "outputType" parameter
diff --git a/src/FieldsBuilder.php b/src/FieldsBuilder.php
@@ -16,6 +16,7 @@
 use phpDocumentor\Reflection\Types\Self_;
 use Psr\Http\Message\UploadedFileInterface;
 use ReflectionMethod;
+use function sprintf;
 use TheCodingMachine\GraphQLite\Annotations\Field;
 use TheCodingMachine\GraphQLite\Annotations\SourceFieldInterface;
 use TheCodingMachine\GraphQLite\Hydrators\HydratorInterface;
@@ -267,9 +268,10 @@ private function getFieldsByAnnotations($controller, string $annotationName, boo
                 $args = $this->mapParameters($parameters, $docBlockObj);
 
                 if ($queryAnnotation->getOutputType()) {
-                    $type = $this->typeResolver->mapNameToType($queryAnnotation->getOutputType());
-                    if (!$type instanceof OutputType) {
-                        throw new \InvalidArgumentException(sprintf("In %s::%s, the 'outputType' parameter in @Type annotation should contain the name of an OutputType. The '%s' type does not implement GraphQL\\Type\\Definition\\OutputType", $refMethod->getDeclaringClass()->getName(), $refMethod->getName(), $queryAnnotation->getOutputType()));
+                    try {
+                        $type = $this->typeResolver->mapNameToOutputType($queryAnnotation->getOutputType());
+                    } catch (CannotMapTypeExceptionInterface $e) {
+                        throw CannotMapTypeException::wrapWithReturnInfo($e, $refMethod);
                     }
                 } else {
                     $type = $this->mapReturnType($refMethod, $docBlockObj);
@@ -403,7 +405,11 @@ private function getQueryFieldsFromSourceFields(array $sourceFields, ReflectionC
                     $type = GraphQLType::nonNull($type);
                 }
             } elseif ($sourceField->getOutputType()) {
-                $type = $this->typeResolver->mapNameToType($sourceField->getOutputType());
+                try {
+                    $type = $this->typeResolver->mapNameToOutputType($sourceField->getOutputType());
+                } catch (CannotMapTypeExceptionInterface $e) {
+                    throw CannotMapTypeException::wrapWithSourceField($e, $refClass, $sourceField);
+                }
             } else {
                 $type = $this->mapReturnType($refMethod, $docBlockObj);
             }
@@ -524,11 +530,6 @@ private function mapParameters(array $refParameters, DocBlock $docBlock): array
         return $args;
     }
 
-    /**
-     * @param Type $type
-     * @param Type|null $docBlockType
-     * @return GraphQLType
-     */
     private function mapType(Type $type, ?Type $docBlockType, bool $isNullable, bool $mapToInputType): GraphQLType
     {
         $graphQlType = null;
diff --git a/src/Mappers/CannotMapTypeException.php b/src/Mappers/CannotMapTypeException.php
@@ -4,8 +4,12 @@
 namespace TheCodingMachine\GraphQLite\Mappers;
 
 
+use GraphQL\Error\SyntaxError;
 use GraphQL\Type\Definition\ObjectType;
+use ReflectionClass;
 use ReflectionMethod;
+use function sprintf;
+use TheCodingMachine\GraphQLite\Annotations\SourceField;
 
 class CannotMapTypeException extends \Exception implements CannotMapTypeExceptionInterface
 {
@@ -24,6 +28,11 @@ public static function createForName(string $name): self
         return new self('cannot find GraphQL type "'.$name.'". Check your TypeMapper configuration.');
     }
 
+    public static function createForParseError(SyntaxError $error): self
+    {
+        return new self($error->getMessage(), $error->getCode(), $error);
+    }
+
     public static function wrapWithParamInfo(CannotMapTypeExceptionInterface $previous, \ReflectionParameter $parameter): self
     {
         $message = sprintf('For parameter $%s, in %s::%s, %s',
@@ -45,6 +54,16 @@ public static function wrapWithReturnInfo(CannotMapTypeExceptionInterface $previ
         return new self($message, 0, $previous);
     }
 
+    public static function wrapWithSourceField(CannotMapTypeExceptionInterface $previous, ReflectionClass $class, SourceField $sourceField): self
+    {
+        $message = sprintf('For @SourceField "%s" declared in "%s", %s',
+            $sourceField->getName(),
+            $class->getName(),
+            $previous->getMessage());
+
+        return new self($message, 0, $previous);
+    }
+
     public static function mustBeOutputType($subTypeName): self
     {
         return new self('type "'.$subTypeName.'" must be an output type.');
diff --git a/src/Types/TypeResolver.php b/src/Types/TypeResolver.php
@@ -3,8 +3,13 @@
 
 namespace TheCodingMachine\GraphQLite\Types;
 
+use GraphQL\Error\Error;
+use GraphQL\Language\Parser;
+use GraphQL\Type\Definition\OutputType;
 use GraphQL\Type\Definition\Type;
+use GraphQL\Type\Definition\WrappingType;
 use GraphQL\Type\Schema;
+use GraphQL\Utils\AST;
 use RuntimeException;
 use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException;
 use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeExceptionInterface;
@@ -36,12 +41,27 @@ public function mapNameToType(string $typeName): Type
         if ($this->schema === null) {
             throw new RuntimeException('You must register a schema first before resolving types.');
         }
-        
-        $type = $this->schema->getType($typeName);
+
+        try {
+            $parsedOutputType = Parser::parseType($typeName);
+            $type = AST::typeFromAST($this->schema, $parsedOutputType);
+        } catch (Error $e) {
+            throw CannotMapTypeException::createForParseError($e);
+        }
+
         if ($type === null) {
             throw CannotMapTypeException::createForName($typeName);
         }
 
         return $type;
     }
+
+    public function mapNameToOutputType(string $typeName): OutputType
+    {
+        $type = $this->mapNameToType($typeName);
+        if (!$type instanceof OutputType || ($type instanceof WrappingType && !$type->getWrappedType() instanceof OutputType)) {
+            throw CannotMapTypeException::mustBeOutputType($typeName);
+        }
+        return $type;
+    }
 }
diff --git a/tests/AggregateControllerQueryProviderTest.php b/tests/AggregateControllerQueryProviderTest.php
@@ -43,7 +43,7 @@ public function has($id)
         $aggregateQueryProvider = new AggregateControllerQueryProvider([ 'controller' ], $this->getControllerQueryProviderFactory(), $this->getTypeMapper(), $container);
 
         $queries = $aggregateQueryProvider->getQueries();
-        $this->assertCount(6, $queries);
+        $this->assertCount(7, $queries);
 
         $mutations = $aggregateQueryProvider->getMutations();
         $this->assertCount(1, $mutations);
diff --git a/tests/FieldsBuilderTest.php b/tests/FieldsBuilderTest.php
@@ -24,8 +24,11 @@
 use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithInvalidReturnType;
 use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithIterableParam;
 use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithIterableReturnType;
+use TheCodingMachine\GraphQLite\Fixtures\TestFieldBadOutputType;
 use TheCodingMachine\GraphQLite\Fixtures\TestObject;
 use TheCodingMachine\GraphQLite\Fixtures\TestSelfType;
+use TheCodingMachine\GraphQLite\Fixtures\TestSourceFieldBadOutputType;
+use TheCodingMachine\GraphQLite\Fixtures\TestSourceFieldBadOutputType2;
 use TheCodingMachine\GraphQLite\Fixtures\TestType;
 use TheCodingMachine\GraphQLite\Fixtures\TestTypeId;
 use TheCodingMachine\GraphQLite\Fixtures\TestTypeMissingAnnotation;
@@ -36,6 +39,7 @@
 use TheCodingMachine\GraphQLite\Containers\EmptyContainer;
 use TheCodingMachine\GraphQLite\Containers\BasicAutoWiringContainer;
 use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException;
+use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeExceptionInterface;
 use TheCodingMachine\GraphQLite\Reflection\CachedDocBlockFactory;
 use TheCodingMachine\GraphQLite\Security\AuthenticationServiceInterface;
 use TheCodingMachine\GraphQLite\Security\AuthorizationServiceInterface;
@@ -55,7 +59,7 @@ public function testQueryProvider()
 
         $queries = $queryProvider->getQueries($controller);
 
-        $this->assertCount(6, $queries);
+        $this->assertCount(7, $queries);
         $usersQuery = $queries[0];
         $this->assertSame('test', $usersQuery->name);
 
@@ -146,12 +150,29 @@ public function testQueryProviderWithFixedReturnType()
 
         $queries = $queryProvider->getQueries($controller);
 
-        $this->assertCount(6, $queries);
+        $this->assertCount(7, $queries);
         $fixedQuery = $queries[1];
 
         $this->assertInstanceOf(IDType::class, $fixedQuery->getType());
     }
 
+    public function testQueryProviderWithComplexFixedReturnType()
+    {
+        $controller = new TestController();
+
+        $queryProvider = $this->buildFieldsBuilder();
+
+        $queries = $queryProvider->getQueries($controller);
+
+        $this->assertCount(7, $queries);
+        $fixedQuery = $queries[6];
+
+        $this->assertInstanceOf(NonNull::class, $fixedQuery->getType());
+        $this->assertInstanceOf(ListOfType::class, $fixedQuery->getType()->getWrappedType());
+        $this->assertInstanceOf(NonNull::class, $fixedQuery->getType()->getWrappedType()->getWrappedType());
+        $this->assertInstanceOf(IDType::class, $fixedQuery->getType()->getWrappedType()->getWrappedType()->getWrappedType());
+    }
+
     public function testNameFromAnnotation()
     {
         $controller = new TestController();
@@ -312,7 +333,7 @@ public function testQueryProviderWithIterableClass()
 
         $queries = $queryProvider->getQueries($controller);
 
-        $this->assertCount(6, $queries);
+        $this->assertCount(7, $queries);
         $iterableQuery = $queries[3];
 
         $this->assertInstanceOf(NonNull::class, $iterableQuery->getType());
@@ -328,7 +349,7 @@ public function testQueryProviderWithIterable()
 
         $queries = $queryProvider->getQueries(new TestController());
 
-        $this->assertCount(6, $queries);
+        $this->assertCount(7, $queries);
         $iterableQuery = $queries[4];
 
         $this->assertInstanceOf(NonNull::class, $iterableQuery->getType());
@@ -353,7 +374,7 @@ public function testQueryProviderWithUnion()
 
         $queries = $queryProvider->getQueries($controller);
 
-        $this->assertCount(6, $queries);
+        $this->assertCount(7, $queries);
         $unionQuery = $queries[5];
 
         $this->assertInstanceOf(NonNull::class, $unionQuery->getType());
@@ -471,4 +492,28 @@ public function testSourceFieldWithFailWith()
 
         $this->assertInstanceOf(StringType::class, $fields['test']->getType());
     }
+
+    public function testSourceFieldBadOutputTypeException()
+    {
+        $queryProvider = $this->buildFieldsBuilder();
+        $this->expectException(CannotMapTypeExceptionInterface::class);
+        $this->expectExceptionMessage('For @SourceField "test" declared in "TheCodingMachine\GraphQLite\Fixtures\TestSourceFieldBadOutputType", cannot find GraphQL type "[NotExists]". Check your TypeMapper configuration.');
+        $queryProvider->getFields(new TestSourceFieldBadOutputType(), true);
+    }
+
+    public function testSourceFieldBadOutputType2Exception()
+    {
+        $queryProvider = $this->buildFieldsBuilder();
+        $this->expectException(CannotMapTypeExceptionInterface::class);
+        $this->expectExceptionMessage('For @SourceField "test" declared in "TheCodingMachine\GraphQLite\Fixtures\TestSourceFieldBadOutputType2", Syntax Error: Expected ], found <EOF>');
+        $queryProvider->getFields(new TestSourceFieldBadOutputType2(), true);
+    }
+
+    public function testBadOutputTypeException()
+    {
+        $queryProvider = $this->buildFieldsBuilder();
+        $this->expectException(CannotMapTypeExceptionInterface::class);
+        $this->expectExceptionMessage('For return type of TheCodingMachine\GraphQLite\Fixtures\TestFieldBadOutputType::test, cannot find GraphQL type "[NotExists]". Check your TypeMapper configuration.');
+        $queryProvider->getFields(new TestFieldBadOutputType(), true);
+    }
 }
diff --git a/tests/Fixtures/TestController.php b/tests/Fixtures/TestController.php
@@ -107,4 +107,12 @@ public function testUnion()
     {
         return new TestObject2('foo');
     }
+
+    /**
+     * @Query(outputType="[ID!]!")
+     */
+    public function testFixComplexReturnType(): array
+    {
+        return ['42'];
+    }
 }
diff --git a/tests/Fixtures/TestFieldBadOutputType.php b/tests/Fixtures/TestFieldBadOutputType.php
@@ -0,0 +1,21 @@
+<?php
+
+
+namespace TheCodingMachine\GraphQLite\Fixtures;
+
+use TheCodingMachine\GraphQLite\Annotations\Field;
+use TheCodingMachine\GraphQLite\Annotations\Type;
+
+/**
+ * @Type(class=TestObject::class)
+ */
+class TestFieldBadOutputType
+{
+    /**
+     * @Field(outputType="[NotExists]")
+     */
+    public function test(): array
+    {
+        return [];
+    }
+}
diff --git a/tests/Fixtures/TestSourceFieldBadOutputType.php b/tests/Fixtures/TestSourceFieldBadOutputType.php
@@ -0,0 +1,15 @@
+<?php
+
+
+namespace TheCodingMachine\GraphQLite\Fixtures;
+
+use TheCodingMachine\GraphQLite\Annotations\SourceField;
+use TheCodingMachine\GraphQLite\Annotations\Type;
+
+/**
+ * @Type(class=TestObject::class)
+ * @SourceField(name="test", outputType="[NotExists]")
+ */
+class TestSourceFieldBadOutputType
+{
+}
diff --git a/tests/Fixtures/TestSourceFieldBadOutputType2.php b/tests/Fixtures/TestSourceFieldBadOutputType2.php
@@ -0,0 +1,15 @@
+<?php
+
+
+namespace TheCodingMachine\GraphQLite\Fixtures;
+
+use TheCodingMachine\GraphQLite\Annotations\SourceField;
+use TheCodingMachine\GraphQLite\Annotations\Type;
+
+/**
+ * @Type(class=TestObject::class)
+ * @SourceField(name="test", outputType="[BadFormat")
+ */
+class TestSourceFieldBadOutputType2
+{
+}
diff --git a/tests/GlobControllerQueryProviderTest.php b/tests/GlobControllerQueryProviderTest.php
@@ -37,7 +37,7 @@ public function has($id)
         $globControllerQueryProvider = new GlobControllerQueryProvider('TheCodingMachine\\GraphQLite\\Fixtures', $this->getControllerQueryProviderFactory(), $this->getTypeMapper(), $container, $this->getLockFactory(), new NullCache(), null, false);
 
         $queries = $globControllerQueryProvider->getQueries();
-        $this->assertCount(6, $queries);
+        $this->assertCount(7, $queries);
 
         $mutations = $globControllerQueryProvider->getMutations();
         $this->assertCount(1, $mutations);

Original file line number	Diff line number	Diff line change
`@@ -107,4 +107,12 @@ public function testUnion()`
`107`	`107`	`{`
`108`	`108`	`return new TestObject2('foo');`
`109`	`109`	`}`
	`110`	`+`
	`111`	`+ /**`
	`112`	`+ * @Query(outputType="[ID!]!")`
	`113`	`+ */`
	`114`	`+ public function testFixComplexReturnType(): array`
	`115`	`+ {`
	`116`	`+ return ['42'];`
	`117`	`+ }`
`110`	`118`	`}`