Skip to content

Commit

Permalink
Merge da03cc0 into 602bfa6
Browse files Browse the repository at this point in the history
  • Loading branch information
moufmouf authored Jul 9, 2020
2 parents 602bfa6 + da03cc0 commit 3a65670
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 2 deletions.
5 changes: 5 additions & 0 deletions src/Mappers/CannotMapTypeException.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
}
2 changes: 2 additions & 0 deletions src/Mappers/Root/CompoundTypeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ 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();
} else {
$badTypes = [];
$nonNullableUnionTypes = [];
Expand Down
10 changes: 9 additions & 1 deletion src/Mappers/Root/IteratorTypeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions src/Mappers/Root/NullableTypeMapperAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
12 changes: 12 additions & 0 deletions tests/Fixtures/Integration/Controllers/ContactController.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@


use Porpaginas\Arrays\ArrayResult;
use Porpaginas\Result;
use TheCodingMachine\GraphQLite\Annotations\Mutation;
use TheCodingMachine\GraphQLite\Annotations\Query;
use TheCodingMachine\GraphQLite\Annotations\Security;
Expand Down Expand Up @@ -78,4 +79,15 @@ public function getOtherContact(): Contact
{
return new Contact('Joe');
}

/**
* Test that we can have nullable results from Porpaginas.
*
* @Query()
* @return Result|Contact[]|null
*/
public function getNullableResult(): ?Result
{
return null;
}
}
26 changes: 26 additions & 0 deletions tests/Integration/EndToEndTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1545,4 +1545,30 @@ public function testInputOutputNameConflict(): void

$schema->validate();
}

public function testNullableResult(){
/**
* @var Schema $schema
*/
$schema = $this->mainContainer->get(Schema::class);

$queryString = '
query {
nullableResult {
count
}
}
';

$result = GraphQL::executeQuery(
$schema,
$queryString
);
$resultArray = $result->toArray(Debug::RETHROW_UNSAFE_EXCEPTIONS);
if (isset($resultArray['errors']) || !isset($resultArray['data'])) {
$this->fail('Expected a successful answer. Got '.json_encode($resultArray, JSON_PRETTY_PRINT));
}
$this->assertNull($resultArray['data']['nullableResult']);
}

}
2 changes: 1 addition & 1 deletion tests/Mappers/Root/CompoundTypeMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

0 comments on commit 3a65670

Please sign in to comment.