Making it possible to have nullable compound types

Author: moufmouf

src/Mappers/CannotMapTypeException.php

@@ -177,4 +177,9 @@ public static function createForPhpDocType(PhpDocumentorType $type): self
     {
         return new self("don't know how to handle type " . (string) $type);
     }
+
+    public static function createForNonNullReturnByTypeMapper(): self
+    {
+        return new self('a type mapper returned a GraphQL\Type\Definition\NonNull instance. All instances returned by type mappers should be nullable. It is the role of the NullableTypeMapperAdapter class to make a GraphQL type in a "NonNull". Note: this is an error in the TypeMapper code or in GraphQLite itself. Please check your custom type mappers or open an issue on GitHub if you don\'t have any custom type mapper.');
+    }
 }

src/Mappers/Root/CompoundTypeMapper.php

@@ -138,6 +138,9 @@ private function getTypeFromUnion(array $unionTypes): GraphQLType
         if (count($unionTypes) === 1) {
             $graphQlType = $unionTypes[0];
             Assert::isInstanceOf($graphQlType, NonNull::class);
+            // If we have only one type, let's make it nullable (it is the role of the NullableTypeMapperAdapter to make it non nullable)
+            $graphQlType = $graphQlType->getWrappedType();
+            assert($graphQlType instanceof OutputType);
         } else {
             $badTypes = [];
             $nonNullableUnionTypes = [];

src/Mappers/Root/IteratorTypeMapper.php

@@ -78,6 +78,7 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection
             return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj);
         }
         Assert::isInstanceOf($result, OutputType::class);
+        Assert::notInstanceOf($result, NonNull::class);
 
         return $result;
     }
@@ -99,7 +100,11 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu
         }
 
         $result = $this->toGraphQLType($type, function (Type $type, ?InputType $subType) use ($refMethod, $docBlockObj, $argumentName) {
-            return $this->topRootTypeMapper->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj);
+            $topType = $this->topRootTypeMapper->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj);
+            if ($topType instanceof NonNull) {
+                $topType = $topType->getWrappedType();
+            }
+            return $topType;
         }, false);
         if ($result === null) {
             return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj);
@@ -197,6 +202,9 @@ private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $
 
         if (count($unionTypes) === 1) {
             $graphQlType = $unionTypes[0];
+            if ($graphQlType instanceof NonNull) {
+                $graphQlType = $graphQlType->getWrappedType();
+            }
             /*} elseif ($isOutputType) {
                 // This clearly cannot work. We are only gathering types from arrays and we cannot join arrays (I think)
                 $graphQlType = new UnionType($unionTypes, $this->recursiveTypeMapper);

src/Mappers/Root/NullableTypeMapperAdapter.php

@@ -6,6 +6,7 @@
 
 use GraphQL\Type\Definition\InputType;
 use GraphQL\Type\Definition\NamedType;
+use GraphQL\Type\Definition\NonNull;
 use GraphQL\Type\Definition\NullableType;
 use GraphQL\Type\Definition\OutputType;
 use GraphQL\Type\Definition\Type as GraphQLType;
@@ -57,6 +58,10 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection
 
         $graphQlType = $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj);
 
+        if ($graphQlType instanceof NonNull) {
+            throw CannotMapTypeException::createForNonNullReturnByTypeMapper();
+        }
+
         if (! $isNullable && $graphQlType instanceof NullableType) {
             $graphQlType = GraphQLType::nonNull($graphQlType);
         }

tests/Integration/EndToEndTest.php

@@ -1568,7 +1568,7 @@ public function testNullableResult(){
         if (isset($resultArray['errors']) || !isset($resultArray['data'])) {
             $this->fail('Expected a successful answer. Got '.json_encode($resultArray, JSON_PRETTY_PRINT));
         }
-        $this->assertNull($resultArray['data']['getNullableResult']);
+        $this->assertNull($resultArray['data']['nullableResult']);
     }
 
 }

tests/Mappers/Root/CompoundTypeMapperTest.php

@@ -46,7 +46,7 @@ public function testException3()
         $compoundTypeMapper = $this->getRootTypeMapper();
 
         $this->expectException(CannotMapTypeException::class);
-        $this->expectExceptionMessage('the value must be iterable, but its computed GraphQL type (String!) is not a list.');
+        $this->expectExceptionMessage('the value must be iterable, but its computed GraphQL type (String) is not a list.');
         $compoundTypeMapper->toGraphQLOutputType(new Compound([new Iterable_(), new String_()]), null, new ReflectionMethod(__CLASS__, 'testException1'), new DocBlock());
     }
 }

tests/Mappers/Root/NullableTypeMapperAdapterTest.php

@@ -2,8 +2,14 @@
 
 namespace TheCodingMachine\GraphQLite\Mappers\Root;
 
+use GraphQL\Type\Definition\InputType;
+use GraphQL\Type\Definition\NamedType;
 use GraphQL\Type\Definition\NonNull;
+use GraphQL\Type\Definition\OutputType;
+use GraphQL\Type\Definition\StringType;
+use GraphQL\Type\Definition\Type as GraphQLType;
 use phpDocumentor\Reflection\DocBlock;
+use phpDocumentor\Reflection\Type;
 use ReflectionMethod;
 use TheCodingMachine\GraphQLite\AbstractQueryProviderTest;
 use TheCodingMachine\GraphQLite\Fixtures\TestObject;
@@ -37,4 +43,32 @@ public function testOnlyNull2(): void
         $this->expectExceptionMessage('type-hinting against null only in the PHPDoc is not allowed.');
         $compoundTypeMapper->toGraphQLInputType($this->resolveType('null'), null, 'foo', new ReflectionMethod(__CLASS__, 'testMultipleCompound'), new DocBlock());
     }
+
+    public function testNonNullableReturnedByWrappedMapper(): void
+    {
+        $typeMapper = new NullableTypeMapperAdapter();
+
+        $typeMapper->setNext(new class implements RootTypeMapperInterface {
+
+            public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): OutputType
+            {
+                return new NonNull(new StringType());
+            }
+
+            public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType
+            {
+                throw new \RuntimeException('Not implemented');
+            }
+
+            public function mapNameToType(string $typeName): NamedType
+            {
+                throw new \RuntimeException('Not implemented');
+            }
+        });
+
+
+        $this->expectException(CannotMapTypeException::class);
+        $this->expectExceptionMessage('a type mapper returned a GraphQL\\Type\\Definition\\NonNull instance.');
+        $typeMapper->toGraphQLOutputType($this->resolveType(TestObject::class.'|'.TestObject2::class.'|null'), null, new ReflectionMethod(__CLASS__, 'testMultipleCompound'), new DocBlock());
+    }
 }