From 760f8205550974ab45dae153eff58f8061cc9ab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Tue, 5 Nov 2019 14:18:29 +0100 Subject: [PATCH 01/13] Fixing union type exception message when types are not "named" --- src/Mappers/CannotMapTypeException.php | 16 ++++++++-------- tests/Mappers/Parameters/TypeMapperTest.php | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Mappers/CannotMapTypeException.php b/src/Mappers/CannotMapTypeException.php index 67a0f84bcc..bcb043d2ce 100644 --- a/src/Mappers/CannotMapTypeException.php +++ b/src/Mappers/CannotMapTypeException.php @@ -47,14 +47,14 @@ public static function createForParseError(Error $error): self */ public static function createForBadTypeInUnion(array $unionTypes): self { - $disallowedTypes = array_filter($unionTypes, static function (Type $type) { - return $type instanceof NamedType; - }); - $disallowedTypeNames = array_map(static function (NamedType $type) { - return $type->name; - }, $disallowedTypes); - - return new self('In GraphQL, you can only use union types between objects. These types cannot be used in union types: ' . implode(', ', $disallowedTypeNames)); + $disallowedTypeNames = array_map(static function (Type $type) { + if ($type instanceof NamedType) { + return $type->name; + } + return (string) $type; + }, $unionTypes); + + return new self('in GraphQL, you can only use union types between objects. These types cannot be used in union types: ' . implode(', ', $disallowedTypeNames)); } public static function mustBeOutputType(string $subTypeName): self diff --git a/tests/Mappers/Parameters/TypeMapperTest.php b/tests/Mappers/Parameters/TypeMapperTest.php index 4ec2068cee..e558fa2561 100644 --- a/tests/Mappers/Parameters/TypeMapperTest.php +++ b/tests/Mappers/Parameters/TypeMapperTest.php @@ -30,7 +30,7 @@ public function testMapScalarUnionException(): void $docBlockObj = $cachedDocBlockFactory->getDocBlock($refMethod); $this->expectException(CannotMapTypeException::class); - $this->expectExceptionMessage('In GraphQL, you can only use union types between objects. These types cannot be used in union types: Int, String'); + $this->expectExceptionMessage('in GraphQL, you can only use union types between objects. These types cannot be used in union types: Int, String'); $typeMapper->mapReturnType($refMethod, $docBlockObj); } From aeb876c9838fed2fd4bbd2a4a11bdb8b3aca760a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Tue, 5 Nov 2019 14:19:10 +0100 Subject: [PATCH 02/13] Adding breaking test --- .../Controllers/ProductController.php | 10 +++++++ tests/Integration/EndToEndTest.php | 29 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/tests/Fixtures/Integration/Controllers/ProductController.php b/tests/Fixtures/Integration/Controllers/ProductController.php index 4ea25bd3ee..d1fb873caf 100644 --- a/tests/Fixtures/Integration/Controllers/ProductController.php +++ b/tests/Fixtures/Integration/Controllers/ProductController.php @@ -4,6 +4,7 @@ namespace TheCodingMachine\GraphQLite\Fixtures\Integration\Controllers; +use ArrayIterator; use DateTimeImmutable; use Porpaginas\Arrays\ArrayResult; use TheCodingMachine\GraphQLite\Annotations\Query; @@ -66,4 +67,13 @@ public function getProduct() { return new SpecialProduct('Special box', 10.99); } + + /** + * @Query(name="getProduct") + * @return (\TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Product|\TheCodingMachine\GraphQLite\Fixtures\Integration\Models\SpecialProduct)[] + */ + public function getProducts2(): ArrayIterator + { + return new ArrayIterator([new SpecialProduct('Special box', 10.99), new SpecialProduct('Special box', 10.99)]); + } } diff --git a/tests/Integration/EndToEndTest.php b/tests/Integration/EndToEndTest.php index aca3253dfe..34fb31e17e 100644 --- a/tests/Integration/EndToEndTest.php +++ b/tests/Integration/EndToEndTest.php @@ -1202,4 +1202,33 @@ public function testEndToEndUnions(){ $this->assertEquals('unicorn', $resultArray['data']['getProduct']['special']); } + public function testEndToEndUnionsInIterables(){ + /** + * @var Schema $schema + */ + $schema = $this->mainContainer->get(Schema::class); + + $queryString = ' + query { + getProducts2{ + __typename + ... on SpecialProduct{ + name + special + } + } + } + '; + + $result = GraphQL::executeQuery( + $schema, + $queryString + ); + $resultArray = $result->toArray(); + + $this->assertEquals('SpecialProduct', $resultArray['data']['getProduct']['__typename'][0]); + $this->assertEquals('Special box', $resultArray['data']['getProduct']['name'][0]); + $this->assertEquals('unicorn', $resultArray['data']['getProduct']['special'][0]); + } + } From 433f30ca3932424a66af7d09935a7a772a348639 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Fri, 8 Nov 2019 18:06:51 +0100 Subject: [PATCH 03/13] Complete refactoring of root type mappers and type handler --- src/GlobControllerQueryProvider.php | 2 + src/Mappers/Parameters/TypeHandler.php | 109 ++++++- src/Mappers/PorpaginasTypeMapper.php | 4 + src/Mappers/Root/BaseTypeMapper.php | 15 +- src/Mappers/Root/CompositeRootTypeMapper.php | 8 +- src/Mappers/Root/CompoundTypeMapper.php | 166 +++++++++++ src/Mappers/Root/IteratorTypeMapper.php | 277 ++++++++++++++++++ .../Root/NullableTypeMapperAdapter.php | 157 ++++++++++ src/Schema.php | 9 +- src/SchemaFactory.php | 22 +- src/TypeMappingRuntimeException.php | 70 +++-- src/TypeRegistry.php | 17 ++ tests/AbstractQueryProviderTest.php | 30 +- tests/FieldsBuilderTest.php | 14 +- .../Controllers/ProductController.php | 2 +- tests/Fixtures/TestController.php | 2 +- tests/Integration/EndToEndTest.php | 31 +- tests/Mappers/Parameters/TypeMapperTest.php | 17 +- tests/Mappers/Root/BaseTypeMapperTest.php | 10 +- tests/SchemaFactoryTest.php | 2 +- tests/SchemaTest.php | 2 +- 21 files changed, 874 insertions(+), 92 deletions(-) create mode 100644 src/Mappers/Root/CompoundTypeMapper.php create mode 100644 src/Mappers/Root/IteratorTypeMapper.php create mode 100644 src/Mappers/Root/NullableTypeMapperAdapter.php diff --git a/src/GlobControllerQueryProvider.php b/src/GlobControllerQueryProvider.php index 322e386dcc..23926ed07b 100644 --- a/src/GlobControllerQueryProvider.php +++ b/src/GlobControllerQueryProvider.php @@ -12,6 +12,7 @@ use Symfony\Component\Cache\Adapter\Psr16Adapter; use Symfony\Contracts\Cache\CacheInterface as CacheContractInterface; use TheCodingMachine\ClassExplorer\Glob\GlobClassExplorer; +use Webmozart\Assert\Assert; use function class_exists; use function interface_exists; use function str_replace; @@ -82,6 +83,7 @@ private function getInstancesList(): array $this->instancesList = $this->cacheContract->get('globQueryProvider', function () { return $this->buildInstancesList(); }); + Assert::isArray($this->instancesList, 'The instance list returned is not an array. There might be an issue with your PSR-16 cache implementation.'); } return $this->instancesList; diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index e7eda82f48..69654ca308 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -46,8 +46,11 @@ use TheCodingMachine\GraphQLite\Types\UnionType; use Webmozart\Assert\Assert; use function array_filter; +use function array_merge; +use function array_unique; use function count; use function iterator_to_array; +use const SORT_REGULAR; class TypeHandler implements ParameterHandlerInterface { @@ -183,10 +186,43 @@ public function mapParameter(ReflectionParameter $parameter, DocBlock $docBlock, private function mapType(Type $type, ?Type $docBlockType, bool $isNullable, bool $mapToInputType, ReflectionMethod $refMethod, DocBlock $docBlockObj, ?string $argumentName = null): GraphQLType { $graphQlType = null; + if ($isNullable && !$type instanceof Nullable) { + // In case a parameter has a default value, let's wrap the main type in a nullable + $type = new Nullable($type); + } + $innerType = $type instanceof Nullable ? $type->getActualType() : $type; + + if ($innerType instanceof Array_ || $innerType instanceof Iterable_ || $innerType instanceof Mixed_) { + // We need to use the docBlockType + if ($docBlockType === null) { + throw TypeMappingRuntimeException::createFromType($type); + } + if ($mapToInputType === true) { + Assert::notNull($argumentName); + $graphQlType = $this->rootTypeMapper->toGraphQLInputType($docBlockType, null, $argumentName, $refMethod, $docBlockObj); + } else { + $graphQlType = $this->rootTypeMapper->toGraphQLOutputType($docBlockType, null, $refMethod, $docBlockObj); + } - if ($type instanceof Array_ || $type instanceof Iterable_ || $type instanceof Mixed_) { - $graphQlType = $this->mapDocBlockType($type, $docBlockType, $isNullable, $mapToInputType, $refMethod, $docBlockObj, $argumentName); + if ($graphQlType === null) { + throw TypeMappingRuntimeException::createFromType($docBlockType); + } + + + //$graphQlType = $this->mapDocBlockType($type, $docBlockType, $isNullable, $mapToInputType, $refMethod, $docBlockObj, $argumentName); } else { + $completeType = $this->appendTypes($type, $docBlockType); + if ($mapToInputType === true) { + Assert::notNull($argumentName); + $graphQlType = $this->rootTypeMapper->toGraphQLInputType($completeType, null, $argumentName, $refMethod, $docBlockObj); + } else { + $graphQlType = $this->rootTypeMapper->toGraphQLOutputType($completeType, null, $refMethod, $docBlockObj); + } + if ($graphQlType === null) { + throw TypeMappingRuntimeException::createFromType($completeType); + } + + /* try { $graphQlType = $this->toGraphQlType($type, null, $mapToInputType, $refMethod, $docBlockObj, $argumentName); // The type is non nullable if the PHP argument is non nullable @@ -210,12 +246,53 @@ private function mapType(Type $type, ?Type $docBlockType, bool $isNullable, bool } $graphQlType = $this->mapIteratorDocBlockType($type, $docBlockType, $isNullable, $refMethod, $docBlockObj, $argumentName); - } + }*/ } return $graphQlType; } + /** + * Appends types together, eventually creating a Compound type and removing duplicates if any. + */ + private function appendTypes(Type $type, ?Type $docBlockType): Type + { + if ($docBlockType === null) { + return $type; + } + + if ($type == $docBlockType) { + return $type; + } + + $types = [ $type ]; + if ($docBlockType instanceof Compound) { + $docBlockTypes = iterator_to_array($docBlockType); + $types = array_merge($types, $docBlockTypes); + } else { + $types[] = $docBlockType; + } + + // Normalize types by changing ?string into string|null + $newTypes = []; + foreach ($types as $currentType) { + if ($currentType instanceof Nullable) { + $newTypes[] = $currentType->getActualType(); + $newTypes[] = new Null_(); + } else { + $newTypes[] = $currentType; + } + } + + $types = array_unique($newTypes, SORT_REGULAR); + + if (count($types) === 1) { + return $types[0]; + } + + return new Compound($types); + } + private function mapDocBlockType(Type $type, ?Type $docBlockType, bool $isNullable, bool $mapToInputType, ReflectionMethod $refMethod, DocBlock $docBlockObj, ?string $argumentName = null): GraphQLType { if ($docBlockType === null) { @@ -262,7 +339,7 @@ private function mapDocBlockType(Type $type, ?Type $docBlockType, bool $isNullab } $graphQlType = new UnionType($unionTypes, $this->recursiveTypeMapper); - $this->typeRegistry->registerType($graphQlType); + $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); } /* elseif (count($filteredDocBlockTypes) === 1) { @@ -304,7 +381,8 @@ private function mapIteratorDocBlockType(Type $type, ?Type $docBlockType, bool $ try { $singleDocBlockType = $this->getTypeInArray($singleDocBlockType); if ($singleDocBlockType !== null) { - $subGraphQlType = $this->toGraphQlType($singleDocBlockType, null, false, $refMethod, $docBlockObj); + $subGraphQlType = $this->mapDocBlockType(new Mixed_(), $singleDocBlockType, $isNullable, false, $refMethod, $docBlockObj, $argumentName); + //$subGraphQlType = $this->toGraphQlType($singleDocBlockType, null, false, $refMethod, $docBlockObj); } else { $subGraphQlType = null; } @@ -329,7 +407,7 @@ private function mapIteratorDocBlockType(Type $type, ?Type $docBlockType, bool $ $graphQlType = $unionTypes[0]; } else { $graphQlType = new UnionType($unionTypes, $this->recursiveTypeMapper); - $this->typeRegistry->registerType($graphQlType); + $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); } if (! $isNullable) { @@ -410,12 +488,12 @@ private function getTypeInArray(Type $typeHint): ?Type private function isNullable(Type $docBlockTypeHint): bool { - if ($docBlockTypeHint instanceof Null_) { + if ($docBlockTypeHint instanceof Null_ || $docBlockTypeHint instanceof Nullable) { return true; } if ($docBlockTypeHint instanceof Compound) { foreach ($docBlockTypeHint as $type) { - if ($type instanceof Null_) { + if ($type instanceof Null_ || $docBlockTypeHint instanceof Nullable) { return true; } } @@ -441,6 +519,19 @@ private function reflectionTypeToPhpDocType(ReflectionType $type, ReflectionClas $phpdocType = $this->phpDocumentorTypeResolver->resolve($type->getName()); Assert::notNull($phpdocType); - return $this->resolveSelf($phpdocType, $reflectionClass); + $phpdocType = $this->resolveSelf($phpdocType, $reflectionClass); + + // FIXME: this is wreaking havoc in all the code. + // FIXME: this is wreaking havoc in all the code. + // FIXME: this is wreaking havoc in all the code. + // FIXME: this is wreaking havoc in all the code. + // FIXME: this is wreaking havoc in all the code. + // FIXME: this is wreaking havoc in all the code. + // FIXME: this is wreaking havoc in all the code. + // FIXME: this is wreaking havoc in all the code. + if ($type->allowsNull()) { + $phpdocType = new Nullable($phpdocType); + } + return $phpdocType; } } diff --git a/src/Mappers/PorpaginasTypeMapper.php b/src/Mappers/PorpaginasTypeMapper.php index efa25be6a2..235194ced4 100644 --- a/src/Mappers/PorpaginasTypeMapper.php +++ b/src/Mappers/PorpaginasTypeMapper.php @@ -5,6 +5,7 @@ namespace TheCodingMachine\GraphQLite\Mappers; use GraphQL\Type\Definition\InputObjectType; +use GraphQL\Type\Definition\NonNull; use GraphQL\Type\Definition\NullableType; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\Type; @@ -70,6 +71,9 @@ public function mapClassToType(string $className, ?OutputType $subType): Mutable */ private function getObjectType(OutputType $subType): MutableInterface { + if ($subType instanceof NonNull) { + $subType = $subType->getWrappedType(true); + } if (! isset($subType->name)) { throw new RuntimeException('Cannot get name property from sub type ' . get_class($subType)); } diff --git a/src/Mappers/Root/BaseTypeMapper.php b/src/Mappers/Root/BaseTypeMapper.php index 98c14840a9..a9c31e2b58 100644 --- a/src/Mappers/Root/BaseTypeMapper.php +++ b/src/Mappers/Root/BaseTypeMapper.php @@ -28,6 +28,7 @@ use TheCodingMachine\GraphQLite\GraphQLRuntimeException; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeExceptionInterface; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; +use TheCodingMachine\GraphQLite\TypeMappingRuntimeException; use TheCodingMachine\GraphQLite\Types\DateTimeType; use TheCodingMachine\GraphQLite\Types\ID; use function ltrim; @@ -41,10 +42,15 @@ class BaseTypeMapper implements RootTypeMapperInterface { /** @var RecursiveTypeMapperInterface */ private $recursiveTypeMapper; + /** + * @var RootTypeMapperInterface + */ + private $topRootTypeMapper; - public function __construct(RecursiveTypeMapperInterface $recursiveTypeMapper) + public function __construct(RecursiveTypeMapperInterface $recursiveTypeMapper, RootTypeMapperInterface $topRootTypeMapper) { $this->recursiveTypeMapper = $recursiveTypeMapper; + $this->topRootTypeMapper = $topRootTypeMapper; } /** @@ -61,7 +67,7 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection return $mappedType; } if ($type instanceof Array_) { - $innerType = $this->toGraphQLOutputType($type->getValueType(), $subType, $refMethod, $docBlockObj); + $innerType = $this->topRootTypeMapper->toGraphQLOutputType($type->getValueType(), $subType, $refMethod, $docBlockObj); if ($innerType === null) { return null; }if ($innerType instanceof NullableType) { @@ -75,6 +81,7 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection return $this->recursiveTypeMapper->mapClassToInterfaceOrType($className, $subType); } + // FIXME: we need to add a $type instanceof Compound here... this will hurt because we need to send back the process to the rootTypeMapper for each inner types... return null; } @@ -91,7 +98,7 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu return $mappedType; } if ($type instanceof Array_) { - $innerType = $this->toGraphQLInputType($type->getValueType(), $subType, $argumentName, $refMethod, $docBlockObj); + $innerType = $this->topRootTypeMapper->toGraphQLInputType($type->getValueType(), $subType, $argumentName, $refMethod, $docBlockObj); if ($innerType === null) { return null; }if ($innerType instanceof NullableType) { @@ -142,7 +149,7 @@ private function mapBaseType(Type $type) case '\\' . UploadedFileInterface::class: return self::getUploadType(); case '\\DateTime': - throw new GraphQLRuntimeException('Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.'); + throw TypeMappingRuntimeException::createFromType($type); case '\\' . ID::class: return GraphQLType::id(); default: diff --git a/src/Mappers/Root/CompositeRootTypeMapper.php b/src/Mappers/Root/CompositeRootTypeMapper.php index 89fc80fcbc..3b9f9f833d 100644 --- a/src/Mappers/Root/CompositeRootTypeMapper.php +++ b/src/Mappers/Root/CompositeRootTypeMapper.php @@ -21,11 +21,17 @@ class CompositeRootTypeMapper implements RootTypeMapperInterface /** * @param RootTypeMapperInterface[] $rootTypeMappers */ - public function __construct(iterable $rootTypeMappers) + public function __construct(iterable $rootTypeMappers = []) { $this->rootTypeMappers = is_array($rootTypeMappers) ? $rootTypeMappers : iterator_to_array($rootTypeMappers); } + public function addRootTypeMapper(RootTypeMapperInterface $rootTypeMapper): void + { + $this->rootTypeMappers[] = $rootTypeMapper; + } + + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType { foreach ($this->rootTypeMappers as $rootTypeMapper) { diff --git a/src/Mappers/Root/CompoundTypeMapper.php b/src/Mappers/Root/CompoundTypeMapper.php new file mode 100644 index 0000000000..d0a7ac5f53 --- /dev/null +++ b/src/Mappers/Root/CompoundTypeMapper.php @@ -0,0 +1,166 @@ +topRootTypeMapper = $topRootTypeMapper; + $this->typeRegistry = $typeRegistry; + $this->recursiveTypeMapper = $recursiveTypeMapper; + } + + /** + * @param (OutputType&GraphQLType)|null $subType + * + * @return (OutputType&GraphQLType)|null + */ + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType + { + if (!$type instanceof Compound) { + return null; + } + + $filteredDocBlockTypes = iterator_to_array($type); + if (empty($filteredDocBlockTypes)) { + throw TypeMappingRuntimeException::createFromType($type); + } + + $unionTypes = []; + $lastException = null; + foreach ($filteredDocBlockTypes as $singleDocBlockType) { + $unionTypes[] = $this->topRootTypeMapper->toGraphQLOutputType($singleDocBlockType, null, $refMethod, $docBlockObj); + } + + return $this->getTypeFromUnion($unionTypes); + } + + /** + * @param (InputType&GraphQLType)|null $subType + * + * @return (InputType&GraphQLType)|null + */ + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType + { + if (!$type instanceof Compound) { + return null; + } + + $filteredDocBlockTypes = iterator_to_array($type); + if (empty($filteredDocBlockTypes)) { + throw TypeMappingRuntimeException::createFromType($type); + } + + $unionTypes = []; + $lastException = null; + foreach ($filteredDocBlockTypes as $singleDocBlockType) { + $unionTypes[] = $this->topRootTypeMapper->toGraphQLInputType($singleDocBlockType, null, $argumentName, $refMethod, $docBlockObj); + } + + return $this->getTypeFromUnion($unionTypes); + } + + /* + * @template T of InputType|OutputType|null + * @param array $unionTypes + * @return T + */ + private function getTypeFromUnion(array $unionTypes) + { + // Remove null values + $unionTypes = array_values(array_filter($unionTypes)); + + $isNullable = false; + + if (count($unionTypes) === 1) { + $graphQlType = $unionTypes[0]; + } else { + $badTypes = []; + $nonNullableUnionTypes = []; + foreach ($unionTypes as $unionType) { + if (!$unionType instanceof NonNull) { + $isNullable = true; + } else { + $unionType = $unionType->getWrappedType(); + } + if ($unionType instanceof ObjectType) { + $nonNullableUnionTypes[] = $unionType; + continue; + } + + $badTypes[] = $unionType; + } + if ($badTypes !== []) { + // TODO! + // TODO! + // TODO! + // TODO! + // We need a middleware to handle this case... + throw CannotMapTypeException::createForBadTypeInUnion($unionTypes); + } + + $graphQlType = new UnionType($nonNullableUnionTypes, $this->recursiveTypeMapper); + $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); + + if (!$isNullable) { + $graphQlType = new NonNull($graphQlType); + } + } + + return $graphQlType; + } + + /** + * Returns a GraphQL type by name. + * If this root type mapper can return this type in "toGraphQLOutputType" or "toGraphQLInputType", it should + * also map these types by name in the "mapNameToType" method. + * + * @param string $typeName The name of the GraphQL type + */ + public function mapNameToType(string $typeName): ?NamedType + { + // TODO: maybe we should map "Union" types here? + return null; + } +} diff --git a/src/Mappers/Root/IteratorTypeMapper.php b/src/Mappers/Root/IteratorTypeMapper.php new file mode 100644 index 0000000000..07e79deef3 --- /dev/null +++ b/src/Mappers/Root/IteratorTypeMapper.php @@ -0,0 +1,277 @@ + ResultIterator + */ +class IteratorTypeMapper implements RootTypeMapperInterface +{ + /** + * @var RootTypeMapperInterface + */ + private $topRootTypeMapper; + /** + * @var TypeRegistry + */ + private $typeRegistry; + /** + * @var RecursiveTypeMapperInterface + */ + private $recursiveTypeMapper; + + public function __construct(RootTypeMapperInterface $topRootTypeMapper, TypeRegistry $typeRegistry, RecursiveTypeMapperInterface $recursiveTypeMapper) + { + $this->topRootTypeMapper = $topRootTypeMapper; + $this->typeRegistry = $typeRegistry; + $this->recursiveTypeMapper = $recursiveTypeMapper; + } + + /** + * @param (OutputType&GraphQLType)|null $subType + * + * @return (OutputType&GraphQLType)|null + */ + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType + { + if (!$type instanceof Compound) { + return null; + } + $iteratorType = null; + $key = null; + $types = iterator_to_array($type); + foreach ($types as $key => $singleDocBlockType) { + if (! ($singleDocBlockType instanceof Object_)) { + continue; + } + + $fqcn = (string) $singleDocBlockType->getFqsen(); + $refClass = new ReflectionClass($fqcn); + // Note : $refClass->isIterable() is only accessible in PHP 7.2 + if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) { + continue; + } + $iteratorType = $singleDocBlockType; + break; + } + + if ($iteratorType === null) { + return null; + } + + // One of the classes in the compound is an iterator. Let's remove it from the list and let's test all other values as potential subTypes. + unset($types[$key]); + + $unionTypes = []; + $lastException = null; + foreach ($types as $singleDocBlockType) { + try { + $singleDocBlockType = $this->getTypeInArray($singleDocBlockType); + if ($singleDocBlockType !== null) { + $subGraphQlType = $this->topRootTypeMapper->toGraphQLOutputType($singleDocBlockType, null, $refMethod, $docBlockObj); + //$subGraphQlType = $this->toGraphQlType($singleDocBlockType, null, false, $refMethod, $docBlockObj); + + // By convention, we trim the NonNull part of the "$subGraphQlType" + if ($subGraphQlType instanceof NonNull) { + $subGraphQlType = $subGraphQlType->getWrappedType(); + } + } else { + $subGraphQlType = null; + } + + $unionTypes[] = $this->topRootTypeMapper->toGraphQLOutputType($iteratorType, $subGraphQlType, $refMethod, $docBlockObj); + + // TODO: add here a scan of the $type variable and do stuff if it is iterable. + // TODO: remove the iterator type if specified in the docblock (@return Iterator|User[]) + // TODO: check there is at least one array (User[]) + } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $e) { + // We have several types. It is ok not to be able to match one. + $lastException = $e; + + if ($singleDocBlockType !== null) { + // The type is an array (like User[]). Let's use that. + $valueType = $this->topRootTypeMapper->toGraphQLOutputType($singleDocBlockType, null, $refMethod, $docBlockObj); + if ($valueType !== null) { + $unionTypes[] = new ListOfType($valueType); + } + } + } + } + + if (empty($unionTypes) && $lastException !== null) { + // We have an issue, let's try without the subType + try { + $result = $this->topRootTypeMapper->toGraphQLOutputType($iteratorType, null, $refMethod, $docBlockObj); + } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $otherException) { + // Still an issue? Let's rethrow the previous exception. + throw $lastException; + } + return $result; + //return $this->mapDocBlockType($type, $docBlockType, $isNullable, false, $refMethod, $docBlockObj); + } + + if (count($unionTypes) === 1) { + $graphQlType = $unionTypes[0]; + } else { + $graphQlType = new UnionType($unionTypes, $this->recursiveTypeMapper); + $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); + } + + return $graphQlType; + } + + /** + * @param (InputType&GraphQLType)|null $subType + * + * @return (InputType&GraphQLType)|null + */ + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType + { + if (!$type instanceof Compound) { + return null; + } + $iteratorType = null; + $key = null; + $types = iterator_to_array($type); + foreach ($types as $key => $singleDocBlockType) { + if (! ($singleDocBlockType instanceof Object_)) { + continue; + } + + $fqcn = (string) $singleDocBlockType->getFqsen(); + $refClass = new ReflectionClass($fqcn); + // Note : $refClass->isIterable() is only accessible in PHP 7.2 + if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) { + continue; + } + $iteratorType = $singleDocBlockType; + break; + } + + if ($iteratorType === null) { + return null; + } + + // One of the classes in the compound is an iterator. Let's remove it from the list and let's test all other values as potential subTypes. + unset($types[$key]); + + $unionTypes = []; + $lastException = null; + foreach ($types as $singleDocBlockType) { + try { + $singleDocBlockType = $this->getTypeInArray($singleDocBlockType); + if ($singleDocBlockType !== null) { + $subGraphQlType = $this->topRootTypeMapper->toGraphQLOutputType($singleDocBlockType, null, $refMethod, $docBlockObj); + //$subGraphQlType = $this->toGraphQlType($singleDocBlockType, null, false, $refMethod, $docBlockObj); + } else { + $subGraphQlType = null; + } + + $unionTypes[] = $this->topRootTypeMapper->toGraphQLInputType($iteratorType, $subGraphQlType, $argumentName, $refMethod, $docBlockObj); + + // TODO: add here a scan of the $type variable and do stuff if it is iterable. + // TODO: remove the iterator type if specified in the docblock (@return Iterator|User[]) + // TODO: check there is at least one array (User[]) + } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $e) { + // We have several types. It is ok not to be able to match one. + $lastException = $e; + + if ($singleDocBlockType !== null) { + // The type is an array (like User[]). Let's use that. + $unionTypes[] = $this->topRootTypeMapper->toGraphQLInputType($singleDocBlockType, null, $argumentName, $refMethod, $docBlockObj); + } + } + } + + if (empty($unionTypes) && $lastException !== null) { + // We have an issue, let's try without the subType + try { + $result = $this->topRootTypeMapper->toGraphQLInputType($iteratorType, null, $argumentName, $refMethod, $docBlockObj); + } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $otherException) { + // Still an issue? Let's rethrow the previous exception. + throw $lastException; + } + return $result; + //return $this->mapDocBlockType($type, $docBlockType, $isNullable, false, $refMethod, $docBlockObj); + } + + if (count($unionTypes) === 1) { + $graphQlType = $unionTypes[0]; + } else { + // There are no union input types. Something went wrong. + return null; + } + + return $graphQlType; + + } + + /** + * Returns a GraphQL type by name. + * If this root type mapper can return this type in "toGraphQLOutputType" or "toGraphQLInputType", it should + * also map these types by name in the "mapNameToType" method. + * + * @param string $typeName The name of the GraphQL type + */ + public function mapNameToType(string $typeName): ?NamedType + { + // TODO: how to handle this? Do we need? + return null; + } + + + /** + * Resolves a list type. + */ + private function getTypeInArray(Type $typeHint): ?Type + { + if (! $typeHint instanceof Array_) { + return null; + } + + return $this->dropNullableType($typeHint->getValueType()); + } + + /** + * Drops "Nullable" types and return the core type. + */ + private function dropNullableType(Type $typeHint): Type + { + if ($typeHint instanceof Nullable) { + return $typeHint->getActualType(); + } + + return $typeHint; + } +} diff --git a/src/Mappers/Root/NullableTypeMapperAdapter.php b/src/Mappers/Root/NullableTypeMapperAdapter.php new file mode 100644 index 0000000000..006a45af64 --- /dev/null +++ b/src/Mappers/Root/NullableTypeMapperAdapter.php @@ -0,0 +1,157 @@ +rootTypeMapper = $rootTypeMapper; + } + + /** + * @param (OutputType&GraphQLType)|null $subType + * + * @return (OutputType&GraphQLType)|null + */ + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType + { + // Let's check a "null" value in the docblock + $isNullable = $this->isNullable($type); + + if ($isNullable) { + $nonNullableType = $this->getNonNullable($type); + if ($nonNullableType === null) { + throw TypeMappingRuntimeException::createFromType($type); + } + $type = $nonNullableType; + } + + $graphQlType = $this->rootTypeMapper->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); + if ($graphQlType === null) { + return null; + } + + if (! $isNullable && $graphQlType instanceof NullableType) { + $graphQlType = GraphQLType::nonNull($graphQlType); + } + + return $graphQlType; + } + + /** + * @param (InputType&GraphQLType)|null $subType + * + * @return (InputType&GraphQLType)|null + */ + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType + { + // Let's check a "null" value in the docblock + $isNullable = $this->isNullable($type); + + if ($isNullable) { + $nonNullableType = $this->getNonNullable($type); + if ($nonNullableType === null) { + throw TypeMappingRuntimeException::createFromType($type); + } + $type = $nonNullableType; + } + + $graphQlType = $this->rootTypeMapper->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); + if ($graphQlType === null) { + return null; + } + + // The type is non nullable if the PHP argument is non nullable + // There is an exception: if the PHP argument is non nullable but points to a factory that can called without passing any argument, + // then, the input type is nullable (and we can still create an empty object). + if (! $isNullable && $graphQlType instanceof NullableType) { + if (!($graphQlType instanceof ResolvableMutableInputObjectType) || $graphQlType->isInstantiableWithoutParameters() !== true) { + $graphQlType = GraphQLType::nonNull($graphQlType); + } + } + + return $graphQlType; + } + + /** + * Returns a GraphQL type by name. + * If this root type mapper can return this type in "toGraphQLOutputType" or "toGraphQLInputType", it should + * also map these types by name in the "mapNameToType" method. + * + * @param string $typeName The name of the GraphQL type + */ + public function mapNameToType(string $typeName): ?NamedType + { + return $this->rootTypeMapper->mapNameToType($typeName); + } + + private function isNullable(Type $docBlockTypeHint): bool + { + if ($docBlockTypeHint instanceof Null_ || $docBlockTypeHint instanceof Nullable) { + return true; + } + if ($docBlockTypeHint instanceof Compound) { + foreach ($docBlockTypeHint as $type) { + if ($this->isNullable($type)) { + return true; + } + } + } + + return false; + } + + private function getNonNullable(Type $type): ?Type + { + if ($type instanceof Null_) { + return null; + } + if ($type instanceof Nullable) { + return $type->getActualType(); + } + if ($type instanceof Compound) { + $types = array_map([$this, 'getNonNullable'], iterator_to_array($type)); + // Remove null values + $types = array_filter($types); + if (count($types) > 1) { + return new Compound($types); + } + return $types[0] ?? null; + } + + return $type; + } +} diff --git a/src/Schema.php b/src/Schema.php index e169e3c902..9938e4de71 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -11,6 +11,7 @@ use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperInterface; use TheCodingMachine\GraphQLite\Types\TypeResolver; +use Webmozart\Assert\Assert; /** * A GraphQL schema that takes into constructor argument a QueryProvider. @@ -19,16 +20,18 @@ */ class Schema extends \GraphQL\Type\Schema { - public function __construct(QueryProviderInterface $queryProvider, RecursiveTypeMapperInterface $recursiveTypeMapper, TypeResolver $typeResolver, ?SchemaConfig $config = null, ?RootTypeMapperInterface $rootTypeMapper = null) + public function __construct(QueryProviderInterface $queryProvider, RecursiveTypeMapperInterface $recursiveTypeMapper, TypeResolver $typeResolver, RootTypeMapperInterface $rootTypeMapper, ?SchemaConfig $config = null) { if ($config === null) { $config = SchemaConfig::create(); } - if ($rootTypeMapper === null) { + // TODO: change parameter order, drop compatibility with 3.0 + Assert::notNull($rootTypeMapper); + /*if ($rootTypeMapper === null) { // For compatibility reasons with 3.0, the $rootTypeMapper parameter is optional. $rootTypeMapper = new BaseTypeMapper($recursiveTypeMapper); - } + }*/ $query = new ObjectType([ 'name' => 'Query', diff --git a/src/SchemaFactory.php b/src/SchemaFactory.php index 38a6b34048..78ca28f71a 100644 --- a/src/SchemaFactory.php +++ b/src/SchemaFactory.php @@ -27,7 +27,10 @@ use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompositeRootTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\CompoundTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\IteratorTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\MyCLabsEnumTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\NullableTypeMapperAdapter; use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\TypeMapperFactoryInterface; use TheCodingMachine\GraphQLite\Mappers\TypeMapperInterface; @@ -324,11 +327,16 @@ public function createSchema(): Schema $compositeTypeMapper = new CompositeTypeMapper(); $recursiveTypeMapper = new RecursiveTypeMapper($compositeTypeMapper, $namingStrategy, $this->cache, $typeRegistry); - $rootTypeMappers = $this->rootTypeMappers; - $rootTypeMappers[] = new MyCLabsEnumTypeMapper(); - $rootTypeMappers[] = new BaseTypeMapper($recursiveTypeMapper); - // Let's put all the root type mappers except the BaseTypeMapper (that needs a recursive type mapper and that will be built later) - $compositeRootTypeMapper = new CompositeRootTypeMapper($rootTypeMappers); + $compositeRootTypeMapper = new CompositeRootTypeMapper(); + $topRootTypeMapper = new NullableTypeMapperAdapter($compositeRootTypeMapper); + + $compositeRootTypeMapper->addRootTypeMapper(new IteratorTypeMapper($topRootTypeMapper, $typeRegistry, $recursiveTypeMapper)); + $compositeRootTypeMapper->addRootTypeMapper(new CompoundTypeMapper($topRootTypeMapper, $typeRegistry, $recursiveTypeMapper)); + foreach ($this->rootTypeMappers as $rootTypeMapper) { + $compositeRootTypeMapper->addRootTypeMapper($rootTypeMapper); + } + $compositeRootTypeMapper->addRootTypeMapper(new MyCLabsEnumTypeMapper()); + $compositeRootTypeMapper->addRootTypeMapper(new BaseTypeMapper($recursiveTypeMapper, $topRootTypeMapper)); $argumentResolver = new ArgumentResolver(); @@ -346,7 +354,7 @@ public function createSchema(): Schema $typeResolver, $cachedDocBlockFactory, $namingStrategy, - $compositeRootTypeMapper, + $topRootTypeMapper, $parameterMiddlewarePipe, $fieldMiddlewarePipe, $typeRegistry @@ -427,6 +435,6 @@ public function createSchema(): Schema $aggregateQueryProvider = new AggregateQueryProvider($queryProviders); - return new Schema($aggregateQueryProvider, $recursiveTypeMapper, $typeResolver, $this->schemaConfig, $compositeRootTypeMapper); + return new Schema($aggregateQueryProvider, $recursiveTypeMapper, $typeResolver, $compositeRootTypeMapper, $this->schemaConfig); } } diff --git a/src/TypeMappingRuntimeException.php b/src/TypeMappingRuntimeException.php index d9137e1b8b..3162160076 100644 --- a/src/TypeMappingRuntimeException.php +++ b/src/TypeMappingRuntimeException.php @@ -60,21 +60,31 @@ public static function wrapWithParamInfo(TypeMappingRuntimeException $previous, } $fqcn = (string) $previous->type->getFqsen(); - $refClass = new ReflectionClass($fqcn); - // Note : $refClass->isIterable() is only accessible in PHP 7.2 - if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) { - throw new GraphQLRuntimeException("Unexpected type in TypeMappingException. Got a non iterable '" . $fqcn . '"'); - } - $message = sprintf( - 'Parameter $%s in %s::%s is type-hinted to "%s", which is iterable. Please provide an additional @param in the PHPDoc block to further specify the type. For instance: @param %s|User[] $%s.', - $parameter->getName(), - $declaringClass->getName(), - $parameter->getDeclaringFunction()->getName(), - $fqcn, - $fqcn, - $parameter->getName() - ); + if ($fqcn === '\\DateTime') { + $message = sprintf( + 'Parameter $%s in %s::%s is type-hinted to "DateTime". Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.', + $parameter->getName(), + $declaringClass->getName(), + $parameter->getDeclaringFunction()->getName() + ); + } else { + $refClass = new ReflectionClass($fqcn); + // Note : $refClass->isIterable() is only accessible in PHP 7.2 + if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) { + throw new GraphQLRuntimeException("Unexpected type in TypeMappingException. Got a non iterable '" . $fqcn . '"'); + } + + $message = sprintf( + 'Parameter $%s in %s::%s is type-hinted to "%s", which is iterable. Please provide an additional @param in the PHPDoc block to further specify the type. For instance: @param %s|User[] $%s.', + $parameter->getName(), + $declaringClass->getName(), + $parameter->getDeclaringFunction()->getName(), + $fqcn, + $fqcn, + $parameter->getName() + ); + } } $e = new self($message, 0, $previous); @@ -105,19 +115,27 @@ public static function wrapWithReturnInfo(TypeMappingRuntimeException $previous, } $fqcn = (string) $previous->type->getFqsen(); - $refClass = new ReflectionClass($fqcn); - // Note : $refClass->isIterable() is only accessible in PHP 7.2 - if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) { - throw new GraphQLRuntimeException("Unexpected type in TypeMappingException. Got a non iterable '" . $fqcn . '"'); + if ($fqcn === '\\DateTime') { + $message = sprintf( + 'Return type in %s::%s is type-hinted to "DateTime". Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.', + $method->getDeclaringClass()->getName(), + $method->getName() + ); + } else { + $refClass = new ReflectionClass($fqcn); + // Note : $refClass->isIterable() is only accessible in PHP 7.2 + if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) { + throw new GraphQLRuntimeException("Unexpected type in TypeMappingException. Got a non iterable '" . $fqcn . '"'); + } + + $message = sprintf( + 'Return type in %s::%s is type-hinted to "%s", which is iterable. Please provide an additional @param in the PHPDoc block to further specify the type. For instance: @return %s|User[]', + $method->getDeclaringClass()->getName(), + $method->getName(), + $fqcn, + $fqcn + ); } - - $message = sprintf( - 'Return type in %s::%s is type-hinted to "%s", which is iterable. Please provide an additional @param in the PHPDoc block to further specify the type. For instance: @return %s|User[]', - $method->getDeclaringClass()->getName(), - $method->getName(), - $fqcn, - $fqcn - ); } $e = new self($message, 0, $previous); diff --git a/src/TypeRegistry.php b/src/TypeRegistry.php index 84095880f3..051b9ea4cd 100644 --- a/src/TypeRegistry.php +++ b/src/TypeRegistry.php @@ -39,6 +39,23 @@ public function registerType(NamedType $type): void $this->outputTypes[$type->name] = $type; } + /** + * A failsafe variant of registerType: + * - Registers the type passed in parameter. + * - If the type is already present, does not fail. Instead, return the old type already available. + * + * @param NamedType&Type $type + * @return NamedType&Type + */ + public function getOrRegisterType(NamedType $type): NamedType + { + if (isset($this->outputTypes[$type->name])) { + return $this->outputTypes[$type->name]; + } + $this->outputTypes[$type->name] = $type; + return $type; + } + public function hasType(string $typeName): bool { return isset($this->outputTypes[$typeName]); diff --git a/tests/AbstractQueryProviderTest.php b/tests/AbstractQueryProviderTest.php index 7f3cee5097..b943d85a29 100644 --- a/tests/AbstractQueryProviderTest.php +++ b/tests/AbstractQueryProviderTest.php @@ -29,7 +29,11 @@ use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompositeRootTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\CompoundTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\IteratorTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\MyCLabsEnumTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\NullableTypeMapperAdapter; +use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\TypeMapperInterface; use TheCodingMachine\GraphQLite\Containers\EmptyContainer; use TheCodingMachine\GraphQLite\Containers\BasicAutoWiringContainer; @@ -65,6 +69,7 @@ abstract class AbstractQueryProviderTest extends TestCase private $typeRegistry; private $lockFactory; private $parameterMiddlewarePipe; + private $rootTypeMapper; protected function getTestObjectType(): MutableObjectType { @@ -289,16 +294,33 @@ protected function buildFieldsBuilder(): FieldsBuilder $this->getTypeResolver(), new CachedDocBlockFactory(new ArrayCache()), new NamingStrategy(), - new CompositeRootTypeMapper([ - new MyCLabsEnumTypeMapper(), - new BaseTypeMapper($this->getTypeMapper()) - ]), + $this->buildRootTypeMapper(), $this->getParameterMiddlewarePipe(), $fieldMiddlewarePipe, $this->getTypeRegistry() ); } + protected function getRootTypeMapper(): RootTypeMapperInterface + { + if ($this->rootTypeMapper === null) { + $this->rootTypeMapper = $this->buildRootTypeMapper(); + } + return $this->rootTypeMapper; + } + + protected function buildRootTypeMapper(): RootTypeMapperInterface + { + $compositeRootTypeMapper = new CompositeRootTypeMapper(); + $topRootTypeMapper = new NullableTypeMapperAdapter($compositeRootTypeMapper); + + $compositeRootTypeMapper->addRootTypeMapper(new IteratorTypeMapper($topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper())); + $compositeRootTypeMapper->addRootTypeMapper(new CompoundTypeMapper($topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper())); + $compositeRootTypeMapper->addRootTypeMapper(new MyCLabsEnumTypeMapper()); + $compositeRootTypeMapper->addRootTypeMapper(new BaseTypeMapper($this->getTypeMapper(), $topRootTypeMapper)); + return $topRootTypeMapper; + } + protected function getFieldsBuilder(): FieldsBuilder { if ($this->fieldsBuilder === null) { diff --git a/tests/FieldsBuilderTest.php b/tests/FieldsBuilderTest.php index 74aac85326..56a3709419 100644 --- a/tests/FieldsBuilderTest.php +++ b/tests/FieldsBuilderTest.php @@ -288,7 +288,7 @@ public function getUser(): ?object $this->getTypeResolver(), new CachedDocBlockFactory(new ArrayCache()), new NamingStrategy(), - new BaseTypeMapper($this->getTypeMapper()), + $this->getRootTypeMapper(), $this->getParameterMiddlewarePipe(), new AuthorizationFieldMiddleware( $authenticationService, @@ -320,7 +320,7 @@ public function isAllowed(string $right, $subject = null): bool $this->getTypeResolver(), new CachedDocBlockFactory(new ArrayCache()), new NamingStrategy(), - new BaseTypeMapper($this->getTypeMapper()), + $this->getRootTypeMapper(), $this->getParameterMiddlewarePipe(), new AuthorizationFieldMiddleware( new VoidAuthenticationService(), @@ -382,7 +382,7 @@ public function testFromSourceFieldsInterface(): void $this->getTypeResolver(), new CachedDocBlockFactory(new ArrayCache()), new NamingStrategy(), - new BaseTypeMapper($this->getTypeMapper()), + $this->getRootTypeMapper(), $this->getParameterMiddlewarePipe(), new AuthorizationFieldMiddleware( new VoidAuthenticationService(), @@ -479,7 +479,7 @@ public function testQueryProviderWithInvalidReturnType(): void $queryProvider->getQueries($controller); } - public function testQueryProviderWithIterableReturnType(): void + /*public function testQueryProviderWithIterableReturnType(): void { $controller = new TestControllerWithIterableReturnType(); @@ -488,7 +488,7 @@ public function testQueryProviderWithIterableReturnType(): void $this->expectException(TypeMappingRuntimeException::class); $this->expectExceptionMessage('Return type in TheCodingMachine\GraphQLite\Fixtures\TestControllerWithIterableReturnType::test is type-hinted to "\ArrayObject", which is iterable. Please provide an additional @param in the PHPDoc block to further specify the type. For instance: @return \ArrayObject|User[]'); $queryProvider->getQueries($controller); - } + }*/ public function testQueryProviderWithArrayReturnType(): void { @@ -512,7 +512,7 @@ public function testQueryProviderWithArrayParams(): void $queryProvider->getQueries($controller); } - public function testQueryProviderWithIterableParams(): void + /*public function testQueryProviderWithIterableParams(): void { $controller = new TestControllerWithIterableParam(); @@ -521,7 +521,7 @@ public function testQueryProviderWithIterableParams(): void $this->expectException(TypeMappingRuntimeException::class); $this->expectExceptionMessage('Parameter $params in TheCodingMachine\GraphQLite\Fixtures\TestControllerWithIterableParam::test is type-hinted to "\ArrayObject", which is iterable. Please provide an additional @param in the PHPDoc block to further specify the type. For instance: @param \ArrayObject|User[] $params.'); $queryProvider->getQueries($controller); - } + }*/ public function testFailWith(): void { diff --git a/tests/Fixtures/Integration/Controllers/ProductController.php b/tests/Fixtures/Integration/Controllers/ProductController.php index d1fb873caf..39369b61c2 100644 --- a/tests/Fixtures/Integration/Controllers/ProductController.php +++ b/tests/Fixtures/Integration/Controllers/ProductController.php @@ -69,7 +69,7 @@ public function getProduct() } /** - * @Query(name="getProduct") + * @Query(name="getProducts2") * @return (\TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Product|\TheCodingMachine\GraphQLite\Fixtures\Integration\Models\SpecialProduct)[] */ public function getProducts2(): ArrayIterator diff --git a/tests/Fixtures/TestController.php b/tests/Fixtures/TestController.php index 46fb035829..fb94725783 100644 --- a/tests/Fixtures/TestController.php +++ b/tests/Fixtures/TestController.php @@ -20,7 +20,7 @@ class TestController * @param bool|null $boolean * @param float|null $float * @param \DateTimeImmutable|null $dateTimeImmutable - * @param \DateTime|\DateTimeInterface|null $dateTime + * @param \DateTimeInterface|null $dateTime * @param string $withDefault * @param null|string $string * @param ID|null $id diff --git a/tests/Integration/EndToEndTest.php b/tests/Integration/EndToEndTest.php index 34fb31e17e..b710e27595 100644 --- a/tests/Integration/EndToEndTest.php +++ b/tests/Integration/EndToEndTest.php @@ -29,7 +29,10 @@ use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompositeRootTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\CompoundTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\IteratorTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\MyCLabsEnumTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\NullableTypeMapperAdapter; use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\TypeMapperInterface; use TheCodingMachine\GraphQLite\Middlewares\AuthorizationFieldMiddleware; @@ -77,7 +80,7 @@ public function createContainer(array $overloadedServices = []): ContainerInterf { $services = [ Schema::class => function(ContainerInterface $container) { - return new Schema($container->get(QueryProviderInterface::class), $container->get(RecursiveTypeMapperInterface::class), $container->get(TypeResolver::class), null, $container->get(RootTypeMapperInterface::class)); + return new Schema($container->get(QueryProviderInterface::class), $container->get(RecursiveTypeMapperInterface::class), $container->get(TypeResolver::class), $container->get(RootTypeMapperInterface::class)); }, QueryProviderInterface::class => function(ContainerInterface $container) { return new GlobControllerQueryProvider('TheCodingMachine\\GraphQLite\\Fixtures\\Integration\\Controllers', $container->get(FieldsBuilder::class), @@ -204,10 +207,10 @@ public function createContainer(array $overloadedServices = []): ContainerInterf return new CachedDocBlockFactory(new ArrayCache()); }, RootTypeMapperInterface::class => function(ContainerInterface $container) { - return new CompositeRootTypeMapper([ - new MyCLabsEnumTypeMapper(), - new BaseTypeMapper($container->get(RecursiveTypeMapperInterface::class)) - ]); + return new NullableTypeMapperAdapter($container->get(CompositeRootTypeMapper::class)); + }, + CompositeRootTypeMapper::class => function(ContainerInterface $container) { + return new CompositeRootTypeMapper(); }, ContainerParameterHandler::class => function(ContainerInterface $container) { return new ContainerParameterHandler($container, true, true); @@ -232,6 +235,14 @@ public function createContainer(array $overloadedServices = []): ContainerInterf $container->get(TypeMapperInterface::class)->addTypeMapper($container->get(GlobTypeMapper::class)); $container->get(TypeMapperInterface::class)->addTypeMapper($container->get(GlobTypeMapper::class.'2')); $container->get(TypeMapperInterface::class)->addTypeMapper($container->get(PorpaginasTypeMapper::class)); + + $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new IteratorTypeMapper($container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class))); + $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new CompoundTypeMapper($container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class))); + $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new IteratorTypeMapper($container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class))); + $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new IteratorTypeMapper($container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class))); + $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new MyCLabsEnumTypeMapper()); + $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new BaseTypeMapper($container->get(RecursiveTypeMapperInterface::class), $container->get(RootTypeMapperInterface::class))); + return $container; } @@ -1195,7 +1206,7 @@ public function testEndToEndUnions(){ $schema, $queryString ); - $resultArray = $result->toArray(); + $resultArray = $result->toArray(Debug::RETHROW_UNSAFE_EXCEPTIONS); $this->assertEquals('SpecialProduct', $resultArray['data']['getProduct']['__typename']); $this->assertEquals('Special box', $resultArray['data']['getProduct']['name']); @@ -1224,11 +1235,11 @@ public function testEndToEndUnionsInIterables(){ $schema, $queryString ); - $resultArray = $result->toArray(); + $resultArray = $result->toArray(Debug::RETHROW_UNSAFE_EXCEPTIONS); - $this->assertEquals('SpecialProduct', $resultArray['data']['getProduct']['__typename'][0]); - $this->assertEquals('Special box', $resultArray['data']['getProduct']['name'][0]); - $this->assertEquals('unicorn', $resultArray['data']['getProduct']['special'][0]); + $this->assertEquals('SpecialProduct', $resultArray['data']['getProducts2'][0]['__typename']); + $this->assertEquals('Special box', $resultArray['data']['getProducts2'][0]['name']); + $this->assertEquals('unicorn', $resultArray['data']['getProducts2'][0]['special']); } } diff --git a/tests/Mappers/Parameters/TypeMapperTest.php b/tests/Mappers/Parameters/TypeMapperTest.php index e558fa2561..f25f90111b 100644 --- a/tests/Mappers/Parameters/TypeMapperTest.php +++ b/tests/Mappers/Parameters/TypeMapperTest.php @@ -19,10 +19,7 @@ class TypeMapperTest extends AbstractQueryProviderTest public function testMapScalarUnionException(): void { - $typeMapper = new TypeHandler($this->getTypeMapper(), $this->getArgumentResolver(), new CompositeRootTypeMapper([ - new MyCLabsEnumTypeMapper(), - new BaseTypeMapper($this->getTypeMapper()) - ]), $this->getTypeResolver(), $this->getTypeRegistry()); + $typeMapper = new TypeHandler($this->getTypeMapper(), $this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver(), $this->getTypeRegistry()); $cachedDocBlockFactory = new CachedDocBlockFactory(new ArrayCache()); @@ -30,16 +27,13 @@ public function testMapScalarUnionException(): void $docBlockObj = $cachedDocBlockFactory->getDocBlock($refMethod); $this->expectException(CannotMapTypeException::class); - $this->expectExceptionMessage('in GraphQL, you can only use union types between objects. These types cannot be used in union types: Int, String'); + $this->expectExceptionMessage('For return type of TheCodingMachine\GraphQLite\Mappers\Parameters\TypeMapperTest::dummy, in GraphQL, you can only use union types between objects. These types cannot be used in union types: Int!, String!'); $typeMapper->mapReturnType($refMethod, $docBlockObj); } public function testHideParameter(): void { - $typeMapper = new TypeHandler($this->getTypeMapper(), $this->getArgumentResolver(), new CompositeRootTypeMapper([ - new MyCLabsEnumTypeMapper(), - new BaseTypeMapper($this->getTypeMapper()) - ]), $this->getTypeResolver(), $this->getTypeRegistry()); + $typeMapper = new TypeHandler($this->getTypeMapper(), $this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver(), $this->getTypeRegistry()); $cachedDocBlockFactory = new CachedDocBlockFactory(new ArrayCache()); @@ -58,10 +52,7 @@ public function testHideParameter(): void public function testHideParameterException(): void { - $typeMapper = new TypeHandler($this->getTypeMapper(), $this->getArgumentResolver(), new CompositeRootTypeMapper([ - new MyCLabsEnumTypeMapper(), - new BaseTypeMapper($this->getTypeMapper()) - ]), $this->getTypeResolver(), $this->getTypeRegistry()); + $typeMapper = new TypeHandler($this->getTypeMapper(), $this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver(), $this->getTypeRegistry()); $cachedDocBlockFactory = new CachedDocBlockFactory(new ArrayCache()); diff --git a/tests/Mappers/Root/BaseTypeMapperTest.php b/tests/Mappers/Root/BaseTypeMapperTest.php index f60ee58d20..1ab1ff0e50 100644 --- a/tests/Mappers/Root/BaseTypeMapperTest.php +++ b/tests/Mappers/Root/BaseTypeMapperTest.php @@ -11,13 +11,14 @@ use ReflectionMethod; use TheCodingMachine\GraphQLite\AbstractQueryProviderTest; use TheCodingMachine\GraphQLite\GraphQLRuntimeException; +use TheCodingMachine\GraphQLite\TypeMappingRuntimeException; class BaseTypeMapperTest extends AbstractQueryProviderTest { public function testNullableToGraphQLInputType(): void { - $baseTypeMapper = new BaseTypeMapper($this->getTypeMapper()); + $baseTypeMapper = new BaseTypeMapper($this->getTypeMapper(), $this->getRootTypeMapper()); $mappedType = $baseTypeMapper->toGraphQLInputType(new Nullable(new Object_(new Fqsen('\\Exception'))), null, 'foo', new ReflectionMethod(BaseTypeMapper::class, '__construct'), new DocBlock()); $this->assertNull($mappedType); @@ -25,16 +26,17 @@ public function testNullableToGraphQLInputType(): void public function testToGraphQLOutputTypeException(): void { - $baseTypeMapper = new BaseTypeMapper($this->getTypeMapper()); + $baseTypeMapper = new BaseTypeMapper($this->getTypeMapper(), $this->getRootTypeMapper()); $this->expectException(GraphQLRuntimeException::class); - $this->expectExceptionMessage('Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.'); + //$this->expectExceptionMessage('Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.'); + $this->expectExceptionMessage('Don\'t know how to handle type \DateTime'); $baseTypeMapper->toGraphQLInputType(new Object_(new Fqsen('\\DateTime')), null, 'foo', new ReflectionMethod(BaseTypeMapper::class, '__construct'), new DocBlock()); } public function testUnmappableArray(): void { - $baseTypeMapper = new BaseTypeMapper($this->getTypeMapper()); + $baseTypeMapper = new BaseTypeMapper($this->getTypeMapper(), $this->getRootTypeMapper()); $mappedType = $baseTypeMapper->toGraphQLOutputType(new Array_(new Resource_()), null, new ReflectionMethod(BaseTypeMapper::class, '__construct'), new DocBlock()); $this->assertNull($mappedType); diff --git a/tests/SchemaFactoryTest.php b/tests/SchemaFactoryTest.php index eff191f955..3ee9d68486 100644 --- a/tests/SchemaFactoryTest.php +++ b/tests/SchemaFactoryTest.php @@ -65,7 +65,7 @@ public function testSetters(): void ->setNamingStrategy(new NamingStrategy()) ->addTypeMapper(new CompositeTypeMapper()) ->addTypeMapperFactory(new StaticClassListTypeMapperFactory([TestSelfType::class])) - ->addRootTypeMapper(new CompositeRootTypeMapper([])) + ->addRootTypeMapper(new CompositeRootTypeMapper()) ->addParameterMiddleware(new ParameterMiddlewarePipe()) ->addQueryProviderFactory(new AggregateControllerQueryProviderFactory([], $container)) ->setSchemaConfig(new SchemaConfig()) diff --git a/tests/SchemaTest.php b/tests/SchemaTest.php index 4639b8a39c..d060beb932 100644 --- a/tests/SchemaTest.php +++ b/tests/SchemaTest.php @@ -21,7 +21,7 @@ public function getMutations(): array } }; - $schema = new Schema($queryProvider, $this->getTypeMapper(), $this->getTypeResolver()); + $schema = new Schema($queryProvider, $this->getTypeMapper(), $this->getTypeResolver(), $this->getRootTypeMapper()); $fields = $schema->getQueryType()->getFields(); $this->assertArrayHasKey('dummyQuery', $fields); From 63141ea85dd338795de8fbd67d515eb0db74890a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Fri, 8 Nov 2019 18:25:31 +0100 Subject: [PATCH 04/13] Fixing PHPStan --- phpstan.neon | 4 +++- src/Mappers/Parameters/TypeHandler.php | 2 +- src/Mappers/Root/CompoundTypeMapper.php | 18 +++++++++++++++--- src/Mappers/Root/IteratorTypeMapper.php | 10 ++++++++-- src/TypeRegistry.php | 4 ++-- 5 files changed, 29 insertions(+), 9 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 42daf7806a..53f8391464 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -5,7 +5,6 @@ parameters: - "#Variable \\$prefetchRefMethod might not be defined.#" - "#Parameter \\#2 $type of class TheCodingMachine\\\\GraphQLite\\\\Parameters\\\\InputTypeParameter constructor expects GraphQL\\\\Type\\\\Definition\\\\InputType&GraphQL\\\\Type\\\\Definition\\\\Type, GraphQL\\\\Type\\\\Definition\\\\InputType|GraphQL\\\\Type\\\\Definition\\\\Type given.#" - "#Parameter \\#1 \\$types of class TheCodingMachine\\\\GraphQLite\\\\Types\\\\UnionType constructor expects array, array given.#" - - "#Access to an undefined property GraphQL\\\\Type\\\\Definition\\\\NamedType::\\$name.#" - "#Parameter .* of class ReflectionMethod constructor expects string, object|string given.#" - "#Method TheCodingMachine\\\\GraphQLite\\\\Types\\\\MutableObjectType::getFields() should return array but returns array|float|int.#" - "#Parameter \\#2 \\$inputTypeNode of static method GraphQL\\\\Utils\\\\AST::typeFromAST() expects GraphQL\\\\Language\\\\AST\\\\ListTypeNode|GraphQL\\\\Language\\\\AST\\\\NamedTypeNode|GraphQL\\\\Language\\\\AST\\\\NonNullTypeNode, GraphQL\\\\Language\\\\AST\\\\ListTypeNode|GraphQL\\\\Language\\\\AST\\\\NameNode|GraphQL\\\\Language\\\\AST\\\\NonNullTypeNode given.#" @@ -14,6 +13,9 @@ parameters: - message: '#Parameter .* of class GraphQL\\Error\\Error constructor expects#' path: src/Exceptions/WebonyxErrorHandler.php + - + message: '#Thrown exceptions in a catch block must bundle the previous exception#' + path: src/Mappers/Root/IteratorTypeMapper.php - '#Call to an undefined method GraphQL\\Error\\ClientAware::getMessage()#' #- # message: '#If condition is always true#' diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index 69654ca308..05889a55f5 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -493,7 +493,7 @@ private function isNullable(Type $docBlockTypeHint): bool } if ($docBlockTypeHint instanceof Compound) { foreach ($docBlockTypeHint as $type) { - if ($type instanceof Null_ || $docBlockTypeHint instanceof Nullable) { + if ($type instanceof Null_ || $type instanceof Nullable) { return true; } } diff --git a/src/Mappers/Root/CompoundTypeMapper.php b/src/Mappers/Root/CompoundTypeMapper.php index d0a7ac5f53..3a9686d85d 100644 --- a/src/Mappers/Root/CompoundTypeMapper.php +++ b/src/Mappers/Root/CompoundTypeMapper.php @@ -73,7 +73,10 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection $unionTypes[] = $this->topRootTypeMapper->toGraphQLOutputType($singleDocBlockType, null, $refMethod, $docBlockObj); } - return $this->getTypeFromUnion($unionTypes); + /** @var OutputType&GraphQLType $return */ + $return = $this->getTypeFromUnion($unionTypes); + + return $return; } /** @@ -98,7 +101,10 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu $unionTypes[] = $this->topRootTypeMapper->toGraphQLInputType($singleDocBlockType, null, $argumentName, $refMethod, $docBlockObj); } - return $this->getTypeFromUnion($unionTypes); + /** @var InputType&GraphQLType $return */ + $return = $this->getTypeFromUnion($unionTypes); + + return $return; } /* @@ -106,7 +112,12 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu * @param array $unionTypes * @return T */ - private function getTypeFromUnion(array $unionTypes) + /** + * @param array<(InputType&GraphQLType)|(OutputType&GraphQLType)> $unionTypes + * @return GraphQLType + * @throws CannotMapTypeException + */ + private function getTypeFromUnion(array $unionTypes): GraphQLType { // Remove null values $unionTypes = array_values(array_filter($unionTypes)); @@ -141,6 +152,7 @@ private function getTypeFromUnion(array $unionTypes) } $graphQlType = new UnionType($nonNullableUnionTypes, $this->recursiveTypeMapper); + /** @var UnionType $graphQlType */ $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); if (!$isNullable) { diff --git a/src/Mappers/Root/IteratorTypeMapper.php b/src/Mappers/Root/IteratorTypeMapper.php index 07e79deef3..8cfce678b4 100644 --- a/src/Mappers/Root/IteratorTypeMapper.php +++ b/src/Mappers/Root/IteratorTypeMapper.php @@ -104,6 +104,7 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection // By convention, we trim the NonNull part of the "$subGraphQlType" if ($subGraphQlType instanceof NonNull) { + /** @var OutputType&GraphQLType $subGraphQlType */ $subGraphQlType = $subGraphQlType->getWrappedType(); } } else { @@ -146,6 +147,7 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection } else { $graphQlType = new UnionType($unionTypes, $this->recursiveTypeMapper); $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); + Assert::isInstanceOf($graphQlType, OutputType::class); } return $graphQlType; @@ -192,8 +194,12 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu try { $singleDocBlockType = $this->getTypeInArray($singleDocBlockType); if ($singleDocBlockType !== null) { - $subGraphQlType = $this->topRootTypeMapper->toGraphQLOutputType($singleDocBlockType, null, $refMethod, $docBlockObj); - //$subGraphQlType = $this->toGraphQlType($singleDocBlockType, null, false, $refMethod, $docBlockObj); + $subGraphQlType = $this->topRootTypeMapper->toGraphQLInputType($singleDocBlockType, null, $argumentName, $refMethod, $docBlockObj); + // By convention, we trim the NonNull part of the "$subGraphQlType" + if ($subGraphQlType instanceof NonNull) { + /** @var InputType&GraphQLType $subGraphQlType */ + $subGraphQlType = $subGraphQlType->getWrappedType(); + } } else { $subGraphQlType = null; } diff --git a/src/TypeRegistry.php b/src/TypeRegistry.php index 051b9ea4cd..7ea306f99c 100644 --- a/src/TypeRegistry.php +++ b/src/TypeRegistry.php @@ -44,8 +44,8 @@ public function registerType(NamedType $type): void * - Registers the type passed in parameter. * - If the type is already present, does not fail. Instead, return the old type already available. * - * @param NamedType&Type $type - * @return NamedType&Type + * @param NamedType&Type&(MutableObjectType|InterfaceType|UnionType|(InputObjectType&ResolvableMutableInputInterface)) $type + * @return NamedType&Type&(MutableObjectType|InterfaceType|UnionType|(InputObjectType&ResolvableMutableInputInterface)) */ public function getOrRegisterType(NamedType $type): NamedType { From 5a49c640cc5e1f52ee205d1ba72ea8bc3a824ba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Tue, 12 Nov 2019 10:20:30 +0100 Subject: [PATCH 05/13] Improving code coverage, removing dead code --- src/Mappers/Parameters/TypeHandler.php | 260 +----------------- src/Mappers/PorpaginasTypeMapper.php | 3 - tests/Mappers/PorpaginasTypeMapperTest.php | 10 + .../Root/NullableTypeMapperAdapterTest.php | 37 +++ 4 files changed, 54 insertions(+), 256 deletions(-) create mode 100644 tests/Mappers/Root/NullableTypeMapperAdapterTest.php diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index 05889a55f5..eaafd97112 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -207,9 +207,6 @@ private function mapType(Type $type, ?Type $docBlockType, bool $isNullable, bool if ($graphQlType === null) { throw TypeMappingRuntimeException::createFromType($docBlockType); } - - - //$graphQlType = $this->mapDocBlockType($type, $docBlockType, $isNullable, $mapToInputType, $refMethod, $docBlockObj, $argumentName); } else { $completeType = $this->appendTypes($type, $docBlockType); if ($mapToInputType === true) { @@ -221,32 +218,6 @@ private function mapType(Type $type, ?Type $docBlockType, bool $isNullable, bool if ($graphQlType === null) { throw TypeMappingRuntimeException::createFromType($completeType); } - - /* - try { - $graphQlType = $this->toGraphQlType($type, null, $mapToInputType, $refMethod, $docBlockObj, $argumentName); - // The type is non nullable if the PHP argument is non nullable - // There is an exception: if the PHP argument is non nullable but points to a factory that can called without passing any argument, - // then, the input type is nullable (and we can still create an empty object). - if (! $isNullable && (! $graphQlType instanceof ResolvableMutableInputObjectType || $graphQlType->isInstantiableWithoutParameters() === false)) { - $graphQlType = GraphQLType::nonNull($graphQlType); - } - } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $e) { - // Is the type iterable? If yes, let's analyze the docblock - // TODO: it would be better not to go through an exception for this. - if (! ($type instanceof Object_)) { - throw $e; - } - - $fqcn = (string) $type->getFqsen(); - $refClass = new ReflectionClass($fqcn); - // Note : $refClass->isIterable() is only accessible in PHP 7.2 - if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) { - throw $e; - } - - $graphQlType = $this->mapIteratorDocBlockType($type, $docBlockType, $isNullable, $refMethod, $docBlockObj, $argumentName); - }*/ } return $graphQlType; @@ -293,213 +264,17 @@ private function appendTypes(Type $type, ?Type $docBlockType): Type return new Compound($types); } - private function mapDocBlockType(Type $type, ?Type $docBlockType, bool $isNullable, bool $mapToInputType, ReflectionMethod $refMethod, DocBlock $docBlockObj, ?string $argumentName = null): GraphQLType - { - if ($docBlockType === null) { - throw TypeMappingRuntimeException::createFromType($type); - } - if (! $isNullable) { - // Let's check a "null" value in the docblock - $isNullable = $this->isNullable($docBlockType); - } - - $filteredDocBlockTypes = $this->typesWithoutNullable($docBlockType); - if (empty($filteredDocBlockTypes)) { - throw TypeMappingRuntimeException::createFromType($type); - } - - $unionTypes = []; - $lastException = null; - foreach ($filteredDocBlockTypes as $singleDocBlockType) { - try { - $unionTypes[] = $this->toGraphQlType($this->dropNullableType($singleDocBlockType), null, $mapToInputType, $refMethod, $docBlockObj, $argumentName); - } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $e) { - // We have several types. It is ok not to be able to match one. - $lastException = $e; - } - } - - if (empty($unionTypes) && $lastException !== null) { - throw $lastException; - } - - if (count($unionTypes) === 1) { - $graphQlType = $unionTypes[0]; - } else { - $badTypes = []; - foreach ($unionTypes as $unionType) { - if ($unionType instanceof ObjectType) { - continue; - } - - $badTypes[] = $unionType; - } - if ($badTypes !== []) { - throw CannotMapTypeException::createForBadTypeInUnion($unionTypes); - } - - $graphQlType = new UnionType($unionTypes, $this->recursiveTypeMapper); - $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); - } - - /* elseif (count($filteredDocBlockTypes) === 1) { - $graphQlType = $this->toGraphQlType($filteredDocBlockTypes[0], $mapToInputType); - } else { - throw new GraphQLException('Union types are not supported (yet)'); - //$graphQlTypes = array_map([$this, 'toGraphQlType'], $filteredDocBlockTypes); - //$$graphQlType = new UnionType($graphQlTypes); - }*/ - - if (! $isNullable) { - $graphQlType = GraphQLType::nonNull($graphQlType); - } - - return $graphQlType; - } - - /** - * Maps a type where the main PHP type is an iterator - */ - private function mapIteratorDocBlockType(Type $type, ?Type $docBlockType, bool $isNullable, ReflectionMethod $refMethod, DocBlock $docBlockObj, ?string $argumentName = null): GraphQLType - { - if ($docBlockType === null) { - throw TypeMappingRuntimeException::createFromType($type); - } - if (! $isNullable) { - // Let's check a "null" value in the docblock - $isNullable = $this->isNullable($docBlockType); - } - - $filteredDocBlockTypes = $this->typesWithoutNullable($docBlockType); - if (empty($filteredDocBlockTypes)) { - throw TypeMappingRuntimeException::createFromType($type); - } - - $unionTypes = []; - $lastException = null; - foreach ($filteredDocBlockTypes as $singleDocBlockType) { - try { - $singleDocBlockType = $this->getTypeInArray($singleDocBlockType); - if ($singleDocBlockType !== null) { - $subGraphQlType = $this->mapDocBlockType(new Mixed_(), $singleDocBlockType, $isNullable, false, $refMethod, $docBlockObj, $argumentName); - //$subGraphQlType = $this->toGraphQlType($singleDocBlockType, null, false, $refMethod, $docBlockObj); - } else { - $subGraphQlType = null; - } - - $unionTypes[] = $this->toGraphQlType($type, $subGraphQlType, false, $refMethod, $docBlockObj); - - // TODO: add here a scan of the $type variable and do stuff if it is iterable. - // TODO: remove the iterator type if specified in the docblock (@return Iterator|User[]) - // TODO: check there is at least one array (User[]) - } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $e) { - // We have several types. It is ok not to be able to match one. - $lastException = $e; - } - } - - if (empty($unionTypes) && $lastException !== null) { - // We have an issue, let's try without the subType - return $this->mapDocBlockType($type, $docBlockType, $isNullable, false, $refMethod, $docBlockObj); - } - - if (count($unionTypes) === 1) { - $graphQlType = $unionTypes[0]; - } else { - $graphQlType = new UnionType($unionTypes, $this->recursiveTypeMapper); - $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); - } - - if (! $isNullable) { - $graphQlType = GraphQLType::nonNull($graphQlType); - } - - return $graphQlType; - } - - /** - * Casts a Type to a GraphQL type. - * Does not deal with nullable. - * - * @return GraphQLType (InputType&GraphQLType)|(OutputType&GraphQLType) - * - * @throws CannotMapTypeExceptionInterface - */ - private function toGraphQlType(Type $type, ?GraphQLType $subType, bool $mapToInputType, ReflectionMethod $refMethod, DocBlock $docBlockObj, ?string $argumentName = null): GraphQLType - { - if ($mapToInputType === true) { - Assert::nullOrIsInstanceOf($subType, InputType::class); - Assert::notNull($argumentName); - $mappedType = $this->rootTypeMapper->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); - } else { - Assert::nullOrIsInstanceOf($subType, OutputType::class); - $mappedType = $this->rootTypeMapper->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); - } - if ($mappedType === null) { - throw TypeMappingRuntimeException::createFromType($type); - } - - return $mappedType; - } - - /** - * Removes "null" from the type (if it is compound). Return an array of types (not a Compound type). - * - * @return Type[] - */ - private function typesWithoutNullable(Type $docBlockTypeHint): array - { - if ($docBlockTypeHint instanceof Compound) { - $docBlockTypeHints = iterator_to_array($docBlockTypeHint); - } else { - $docBlockTypeHints = [$docBlockTypeHint]; - } - - return array_filter($docBlockTypeHints, static function ($item) { - return ! $item instanceof Null_; - }); - } - - /** - * Drops "Nullable" types and return the core type. - */ - private function dropNullableType(Type $typeHint): Type - { - if ($typeHint instanceof Nullable) { - return $typeHint->getActualType(); - } - - return $typeHint; - } - - /** - * Resolves a list type. - */ - private function getTypeInArray(Type $typeHint): ?Type + private function reflectionTypeToPhpDocType(ReflectionType $type, ReflectionClass $reflectionClass): Type { - $typeHint = $this->dropNullableType($typeHint); - - if (! $typeHint instanceof Array_) { - return null; - } + $phpdocType = $this->phpDocumentorTypeResolver->resolve($type->getName()); + Assert::notNull($phpdocType); - return $this->dropNullableType($typeHint->getValueType()); - } + $phpdocType = $this->resolveSelf($phpdocType, $reflectionClass); - private function isNullable(Type $docBlockTypeHint): bool - { - if ($docBlockTypeHint instanceof Null_ || $docBlockTypeHint instanceof Nullable) { - return true; - } - if ($docBlockTypeHint instanceof Compound) { - foreach ($docBlockTypeHint as $type) { - if ($type instanceof Null_ || $type instanceof Nullable) { - return true; - } - } + if ($type->allowsNull()) { + $phpdocType = new Nullable($phpdocType); } - - return false; + return $phpdocType; } /** @@ -513,25 +288,4 @@ private function resolveSelf(Type $type, ReflectionClass $reflectionClass): Type return $type; } - - private function reflectionTypeToPhpDocType(ReflectionType $type, ReflectionClass $reflectionClass): Type - { - $phpdocType = $this->phpDocumentorTypeResolver->resolve($type->getName()); - Assert::notNull($phpdocType); - - $phpdocType = $this->resolveSelf($phpdocType, $reflectionClass); - - // FIXME: this is wreaking havoc in all the code. - // FIXME: this is wreaking havoc in all the code. - // FIXME: this is wreaking havoc in all the code. - // FIXME: this is wreaking havoc in all the code. - // FIXME: this is wreaking havoc in all the code. - // FIXME: this is wreaking havoc in all the code. - // FIXME: this is wreaking havoc in all the code. - // FIXME: this is wreaking havoc in all the code. - if ($type->allowsNull()) { - $phpdocType = new Nullable($phpdocType); - } - return $phpdocType; - } } diff --git a/src/Mappers/PorpaginasTypeMapper.php b/src/Mappers/PorpaginasTypeMapper.php index 235194ced4..b5a9ee81de 100644 --- a/src/Mappers/PorpaginasTypeMapper.php +++ b/src/Mappers/PorpaginasTypeMapper.php @@ -71,9 +71,6 @@ public function mapClassToType(string $className, ?OutputType $subType): Mutable */ private function getObjectType(OutputType $subType): MutableInterface { - if ($subType instanceof NonNull) { - $subType = $subType->getWrappedType(true); - } if (! isset($subType->name)) { throw new RuntimeException('Cannot get name property from sub type ' . get_class($subType)); } diff --git a/tests/Mappers/PorpaginasTypeMapperTest.php b/tests/Mappers/PorpaginasTypeMapperTest.php index ad31ab77e2..115c9a0cad 100644 --- a/tests/Mappers/PorpaginasTypeMapperTest.php +++ b/tests/Mappers/PorpaginasTypeMapperTest.php @@ -5,6 +5,7 @@ use GraphQL\Type\Definition\ListOfType; use GraphQL\Type\Definition\StringType; use Porpaginas\Arrays\ArrayResult; +use Porpaginas\Result; use RuntimeException; use TheCodingMachine\GraphQLite\AbstractQueryProviderTest; use TheCodingMachine\GraphQLite\Fixtures\Mocks\MockResolvableInputObjectType; @@ -84,6 +85,15 @@ public function testException8(): void $mapper->decorateInputTypeForName('foo', $type); } + public function testException9(): void + { + $mapper = $this->getPorpaginasTypeMapper(); + $type = new MockResolvableInputObjectType(['name'=>'foo']); + + $this->expectException(PorpaginasMissingParameterException::class); + $mapper->mapClassToType(Result::class, null); + } + public function testCanMapClassToInputType(): void { diff --git a/tests/Mappers/Root/NullableTypeMapperAdapterTest.php b/tests/Mappers/Root/NullableTypeMapperAdapterTest.php new file mode 100644 index 0000000000..706aed24fe --- /dev/null +++ b/tests/Mappers/Root/NullableTypeMapperAdapterTest.php @@ -0,0 +1,37 @@ +getRootTypeMapper(); + $method = new ReflectionMethod(__CLASS__, 'testNullOnlyForOutputType'); + + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage("Don't know how to handle type null"); + $nullableTypeMapperAdapter->toGraphQLOutputType($null, null, $method, new DocBlock()); + } + + public function testNullOnlyForInputType(): void + { + $null = new Null_(); + + $nullableTypeMapperAdapter = $this->getRootTypeMapper(); + $method = new ReflectionMethod(__CLASS__, 'testNullOnlyForOutputType'); + + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage("Don't know how to handle type null"); + $nullableTypeMapperAdapter->toGraphQLInputType($null, null, 'foo', $method, new DocBlock()); + } +} From a384b4b5b896701990fbf11812555850f88d959a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Tue, 12 Nov 2019 14:06:41 +0100 Subject: [PATCH 06/13] Mutualizing input and output code for iteratorTypeMapper --- phpstan.neon | 4 +- src/Mappers/CannotMapTypeException.php | 2 +- src/Mappers/Parameters/TypeHandler.php | 15 +- src/Mappers/PorpaginasTypeMapper.php | 1 - src/Mappers/Root/BaseTypeMapper.php | 5 +- src/Mappers/Root/CompositeRootTypeMapper.php | 1 - src/Mappers/Root/CompoundTypeMapper.php | 30 +-- src/Mappers/Root/IteratorTypeMapper.php | 231 +++++++----------- .../Root/NullableTypeMapperAdapter.php | 14 +- src/Schema.php | 1 - src/TypeRegistry.php | 2 + 11 files changed, 119 insertions(+), 187 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 53f8391464..49a0305c14 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,7 +4,6 @@ parameters: - "#Property TheCodingMachine\\\\GraphQLite\\\\Types\\\\ResolvableInputObjectType::$resolve \\(array&callable\\) does not accept array#" - "#Variable \\$prefetchRefMethod might not be defined.#" - "#Parameter \\#2 $type of class TheCodingMachine\\\\GraphQLite\\\\Parameters\\\\InputTypeParameter constructor expects GraphQL\\\\Type\\\\Definition\\\\InputType&GraphQL\\\\Type\\\\Definition\\\\Type, GraphQL\\\\Type\\\\Definition\\\\InputType|GraphQL\\\\Type\\\\Definition\\\\Type given.#" - - "#Parameter \\#1 \\$types of class TheCodingMachine\\\\GraphQLite\\\\Types\\\\UnionType constructor expects array, array given.#" - "#Parameter .* of class ReflectionMethod constructor expects string, object|string given.#" - "#Method TheCodingMachine\\\\GraphQLite\\\\Types\\\\MutableObjectType::getFields() should return array but returns array|float|int.#" - "#Parameter \\#2 \\$inputTypeNode of static method GraphQL\\\\Utils\\\\AST::typeFromAST() expects GraphQL\\\\Language\\\\AST\\\\ListTypeNode|GraphQL\\\\Language\\\\AST\\\\NamedTypeNode|GraphQL\\\\Language\\\\AST\\\\NonNullTypeNode, GraphQL\\\\Language\\\\AST\\\\ListTypeNode|GraphQL\\\\Language\\\\AST\\\\NameNode|GraphQL\\\\Language\\\\AST\\\\NonNullTypeNode given.#" @@ -16,6 +15,9 @@ parameters: - message: '#Thrown exceptions in a catch block must bundle the previous exception#' path: src/Mappers/Root/IteratorTypeMapper.php + - + message: '#Parameter \#2 \$subType of method .* expects#' + path: src/Mappers/Root/IteratorTypeMapper.php - '#Call to an undefined method GraphQL\\Error\\ClientAware::getMessage()#' #- # message: '#If condition is always true#' diff --git a/src/Mappers/CannotMapTypeException.php b/src/Mappers/CannotMapTypeException.php index bcb043d2ce..80eb1d03d1 100644 --- a/src/Mappers/CannotMapTypeException.php +++ b/src/Mappers/CannotMapTypeException.php @@ -12,7 +12,6 @@ use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\Type; use TheCodingMachine\GraphQLite\Annotations\ExtendType; -use function array_filter; use function array_map; use function implode; @@ -51,6 +50,7 @@ public static function createForBadTypeInUnion(array $unionTypes): self if ($type instanceof NamedType) { return $type->name; } + return (string) $type; }, $unionTypes); diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index eaafd97112..ed17b430a9 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -4,12 +4,8 @@ namespace TheCodingMachine\GraphQLite\Mappers\Parameters; -use GraphQL\Type\Definition\InputType; -use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\Type as GraphQLType; -use Iterator; -use IteratorAggregate; use phpDocumentor\Reflection\DocBlock; use phpDocumentor\Reflection\DocBlock\Tags\Return_; use phpDocumentor\Reflection\Fqsen; @@ -31,7 +27,6 @@ use TheCodingMachine\GraphQLite\Annotations\ParameterAnnotations; use TheCodingMachine\GraphQLite\Annotations\UseInputType; use TheCodingMachine\GraphQLite\InvalidDocBlockRuntimeException; -use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeExceptionInterface; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperInterface; @@ -41,16 +36,13 @@ use TheCodingMachine\GraphQLite\TypeMappingRuntimeException; use TheCodingMachine\GraphQLite\TypeRegistry; use TheCodingMachine\GraphQLite\Types\ArgumentResolver; -use TheCodingMachine\GraphQLite\Types\ResolvableMutableInputObjectType; use TheCodingMachine\GraphQLite\Types\TypeResolver; -use TheCodingMachine\GraphQLite\Types\UnionType; use Webmozart\Assert\Assert; -use function array_filter; +use const SORT_REGULAR; use function array_merge; use function array_unique; use function count; use function iterator_to_array; -use const SORT_REGULAR; class TypeHandler implements ParameterHandlerInterface { @@ -186,7 +178,7 @@ public function mapParameter(ReflectionParameter $parameter, DocBlock $docBlock, private function mapType(Type $type, ?Type $docBlockType, bool $isNullable, bool $mapToInputType, ReflectionMethod $refMethod, DocBlock $docBlockObj, ?string $argumentName = null): GraphQLType { $graphQlType = null; - if ($isNullable && !$type instanceof Nullable) { + if ($isNullable && ! $type instanceof Nullable) { // In case a parameter has a default value, let's wrap the main type in a nullable $type = new Nullable($type); } @@ -232,7 +224,7 @@ private function appendTypes(Type $type, ?Type $docBlockType): Type return $type; } - if ($type == $docBlockType) { + if ($type === $docBlockType) { return $type; } @@ -274,6 +266,7 @@ private function reflectionTypeToPhpDocType(ReflectionType $type, ReflectionClas if ($type->allowsNull()) { $phpdocType = new Nullable($phpdocType); } + return $phpdocType; } diff --git a/src/Mappers/PorpaginasTypeMapper.php b/src/Mappers/PorpaginasTypeMapper.php index b5a9ee81de..efa25be6a2 100644 --- a/src/Mappers/PorpaginasTypeMapper.php +++ b/src/Mappers/PorpaginasTypeMapper.php @@ -5,7 +5,6 @@ namespace TheCodingMachine\GraphQLite\Mappers; use GraphQL\Type\Definition\InputObjectType; -use GraphQL\Type\Definition\NonNull; use GraphQL\Type\Definition\NullableType; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\Type; diff --git a/src/Mappers/Root/BaseTypeMapper.php b/src/Mappers/Root/BaseTypeMapper.php index a9c31e2b58..1b7422e6dc 100644 --- a/src/Mappers/Root/BaseTypeMapper.php +++ b/src/Mappers/Root/BaseTypeMapper.php @@ -25,7 +25,6 @@ use phpDocumentor\Reflection\Types\String_; use Psr\Http\Message\UploadedFileInterface; use ReflectionMethod; -use TheCodingMachine\GraphQLite\GraphQLRuntimeException; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeExceptionInterface; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; use TheCodingMachine\GraphQLite\TypeMappingRuntimeException; @@ -42,9 +41,7 @@ class BaseTypeMapper implements RootTypeMapperInterface { /** @var RecursiveTypeMapperInterface */ private $recursiveTypeMapper; - /** - * @var RootTypeMapperInterface - */ + /** @var RootTypeMapperInterface */ private $topRootTypeMapper; public function __construct(RecursiveTypeMapperInterface $recursiveTypeMapper, RootTypeMapperInterface $topRootTypeMapper) diff --git a/src/Mappers/Root/CompositeRootTypeMapper.php b/src/Mappers/Root/CompositeRootTypeMapper.php index 3b9f9f833d..98ed05127a 100644 --- a/src/Mappers/Root/CompositeRootTypeMapper.php +++ b/src/Mappers/Root/CompositeRootTypeMapper.php @@ -31,7 +31,6 @@ public function addRootTypeMapper(RootTypeMapperInterface $rootTypeMapper): void $this->rootTypeMappers[] = $rootTypeMapper; } - public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType { foreach ($this->rootTypeMappers as $rootTypeMapper) { diff --git a/src/Mappers/Root/CompoundTypeMapper.php b/src/Mappers/Root/CompoundTypeMapper.php index 3a9686d85d..f3cbeaf651 100644 --- a/src/Mappers/Root/CompoundTypeMapper.php +++ b/src/Mappers/Root/CompoundTypeMapper.php @@ -1,5 +1,6 @@ $unionTypes * @return T */ + /** * @param array<(InputType&GraphQLType)|(OutputType&GraphQLType)> $unionTypes - * @return GraphQLType + * * @throws CannotMapTypeException */ private function getTypeFromUnion(array $unionTypes): GraphQLType @@ -130,7 +124,7 @@ private function getTypeFromUnion(array $unionTypes): GraphQLType $badTypes = []; $nonNullableUnionTypes = []; foreach ($unionTypes as $unionType) { - if (!$unionType instanceof NonNull) { + if (! $unionType instanceof NonNull) { $isNullable = true; } else { $unionType = $unionType->getWrappedType(); @@ -155,7 +149,7 @@ private function getTypeFromUnion(array $unionTypes): GraphQLType /** @var UnionType $graphQlType */ $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); - if (!$isNullable) { + if (! $isNullable) { $graphQlType = new NonNull($graphQlType); } } diff --git a/src/Mappers/Root/IteratorTypeMapper.php b/src/Mappers/Root/IteratorTypeMapper.php index 8cfce678b4..cfb704bdf4 100644 --- a/src/Mappers/Root/IteratorTypeMapper.php +++ b/src/Mappers/Root/IteratorTypeMapper.php @@ -1,13 +1,14 @@ $singleDocBlockType) { - if (! ($singleDocBlockType instanceof Object_)) { - continue; - } - $fqcn = (string) $singleDocBlockType->getFqsen(); - $refClass = new ReflectionClass($fqcn); - // Note : $refClass->isIterable() is only accessible in PHP 7.2 - if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) { - continue; - } - $iteratorType = $singleDocBlockType; - break; + $result = $this->toGraphQLType($type, function (Type $type, ?OutputType $subType) use ($refMethod, $docBlockObj) { + return $this->topRootTypeMapper->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); + }, true); + Assert::nullOrIsInstanceOf($result, OutputType::class); + + return $result; + } + + /** + * @param (InputType&GraphQLType)|null $subType + * + * @return (InputType&GraphQLType)|null + */ + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType + { + if (! $type instanceof Compound) { + return null; } - if ($iteratorType === null) { + $result = $this->toGraphQLType($type, function (Type $type, ?InputType $subType) use ($refMethod, $docBlockObj, $argumentName) { + return $this->topRootTypeMapper->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); + }, true); + Assert::nullOrIsInstanceOf($result, InputType::class); + + return $result; + } + + /** + * Returns a GraphQL type by name. + * If this root type mapper can return this type in "toGraphQLOutputType" or "toGraphQLInputType", it should + * also map these types by name in the "mapNameToType" method. + * + * @param string $typeName The name of the GraphQL type + */ + public function mapNameToType(string $typeName): ?NamedType + { + // TODO: how to handle this? Do we need? + return null; + } + + /** + * Resolves a list type. + */ + private function getTypeInArray(Type $typeHint): ?Type + { + if (! $typeHint instanceof Array_) { return null; } - // One of the classes in the compound is an iterator. Let's remove it from the list and let's test all other values as potential subTypes. - unset($types[$key]); + return $this->dropNullableType($typeHint->getValueType()); + } + + /** + * Drops "Nullable" types and return the core type. + */ + private function dropNullableType(Type $typeHint): Type + { + if ($typeHint instanceof Nullable) { + return $typeHint->getActualType(); + } + + return $typeHint; + } + + /** + * @return (OutputType&GraphQLType)|(InputType&GraphQLType)|null + */ + private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $isOutputType) + { + $types = iterator_to_array($type); + + $iteratorType = $this->splitIteratorFromOtherTypes($types); + if ($iteratorType === null) { + return null; + } $unionTypes = []; $lastException = null; @@ -99,7 +143,7 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection try { $singleDocBlockType = $this->getTypeInArray($singleDocBlockType); if ($singleDocBlockType !== null) { - $subGraphQlType = $this->topRootTypeMapper->toGraphQLOutputType($singleDocBlockType, null, $refMethod, $docBlockObj); + $subGraphQlType = $topToGraphQLType($singleDocBlockType, null); //$subGraphQlType = $this->toGraphQlType($singleDocBlockType, null, false, $refMethod, $docBlockObj); // By convention, we trim the NonNull part of the "$subGraphQlType" @@ -111,18 +155,14 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection $subGraphQlType = null; } - $unionTypes[] = $this->topRootTypeMapper->toGraphQLOutputType($iteratorType, $subGraphQlType, $refMethod, $docBlockObj); - - // TODO: add here a scan of the $type variable and do stuff if it is iterable. - // TODO: remove the iterator type if specified in the docblock (@return Iterator|User[]) - // TODO: check there is at least one array (User[]) + $unionTypes[] = $topToGraphQLType($iteratorType, $subGraphQlType); } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $e) { // We have several types. It is ok not to be able to match one. $lastException = $e; if ($singleDocBlockType !== null) { // The type is an array (like User[]). Let's use that. - $valueType = $this->topRootTypeMapper->toGraphQLOutputType($singleDocBlockType, null, $refMethod, $docBlockObj); + $valueType = $topToGraphQLType($singleDocBlockType, null); if ($valueType !== null) { $unionTypes[] = new ListOfType($valueType); } @@ -133,39 +173,40 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection if (empty($unionTypes) && $lastException !== null) { // We have an issue, let's try without the subType try { - $result = $this->topRootTypeMapper->toGraphQLOutputType($iteratorType, null, $refMethod, $docBlockObj); + $result = $topToGraphQLType($iteratorType, null); } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $otherException) { // Still an issue? Let's rethrow the previous exception. throw $lastException; } + return $result; + //return $this->mapDocBlockType($type, $docBlockType, $isNullable, false, $refMethod, $docBlockObj); } if (count($unionTypes) === 1) { $graphQlType = $unionTypes[0]; - } else { + } elseif ($isOutputType) { $graphQlType = new UnionType($unionTypes, $this->recursiveTypeMapper); $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); Assert::isInstanceOf($graphQlType, OutputType::class); + } else { + // There are no union input types. Something went wrong. + $graphQlType = null; } return $graphQlType; } /** - * @param (InputType&GraphQLType)|null $subType + * Removes the iterator type from $types * - * @return (InputType&GraphQLType)|null + * @param Type[] $types */ - public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType + private function splitIteratorFromOtherTypes(array &$types): ?Type { - if (!$type instanceof Compound) { - return null; - } $iteratorType = null; $key = null; - $types = iterator_to_array($type); foreach ($types as $key => $singleDocBlockType) { if (! ($singleDocBlockType instanceof Object_)) { continue; @@ -188,96 +229,6 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu // One of the classes in the compound is an iterator. Let's remove it from the list and let's test all other values as potential subTypes. unset($types[$key]); - $unionTypes = []; - $lastException = null; - foreach ($types as $singleDocBlockType) { - try { - $singleDocBlockType = $this->getTypeInArray($singleDocBlockType); - if ($singleDocBlockType !== null) { - $subGraphQlType = $this->topRootTypeMapper->toGraphQLInputType($singleDocBlockType, null, $argumentName, $refMethod, $docBlockObj); - // By convention, we trim the NonNull part of the "$subGraphQlType" - if ($subGraphQlType instanceof NonNull) { - /** @var InputType&GraphQLType $subGraphQlType */ - $subGraphQlType = $subGraphQlType->getWrappedType(); - } - } else { - $subGraphQlType = null; - } - - $unionTypes[] = $this->topRootTypeMapper->toGraphQLInputType($iteratorType, $subGraphQlType, $argumentName, $refMethod, $docBlockObj); - - // TODO: add here a scan of the $type variable and do stuff if it is iterable. - // TODO: remove the iterator type if specified in the docblock (@return Iterator|User[]) - // TODO: check there is at least one array (User[]) - } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $e) { - // We have several types. It is ok not to be able to match one. - $lastException = $e; - - if ($singleDocBlockType !== null) { - // The type is an array (like User[]). Let's use that. - $unionTypes[] = $this->topRootTypeMapper->toGraphQLInputType($singleDocBlockType, null, $argumentName, $refMethod, $docBlockObj); - } - } - } - - if (empty($unionTypes) && $lastException !== null) { - // We have an issue, let's try without the subType - try { - $result = $this->topRootTypeMapper->toGraphQLInputType($iteratorType, null, $argumentName, $refMethod, $docBlockObj); - } catch (TypeMappingRuntimeException | CannotMapTypeExceptionInterface $otherException) { - // Still an issue? Let's rethrow the previous exception. - throw $lastException; - } - return $result; - //return $this->mapDocBlockType($type, $docBlockType, $isNullable, false, $refMethod, $docBlockObj); - } - - if (count($unionTypes) === 1) { - $graphQlType = $unionTypes[0]; - } else { - // There are no union input types. Something went wrong. - return null; - } - - return $graphQlType; - - } - - /** - * Returns a GraphQL type by name. - * If this root type mapper can return this type in "toGraphQLOutputType" or "toGraphQLInputType", it should - * also map these types by name in the "mapNameToType" method. - * - * @param string $typeName The name of the GraphQL type - */ - public function mapNameToType(string $typeName): ?NamedType - { - // TODO: how to handle this? Do we need? - return null; - } - - - /** - * Resolves a list type. - */ - private function getTypeInArray(Type $typeHint): ?Type - { - if (! $typeHint instanceof Array_) { - return null; - } - - return $this->dropNullableType($typeHint->getValueType()); - } - - /** - * Drops "Nullable" types and return the core type. - */ - private function dropNullableType(Type $typeHint): Type - { - if ($typeHint instanceof Nullable) { - return $typeHint->getActualType(); - } - - return $typeHint; + return $iteratorType; } } diff --git a/src/Mappers/Root/NullableTypeMapperAdapter.php b/src/Mappers/Root/NullableTypeMapperAdapter.php index 006a45af64..6f5b6540d7 100644 --- a/src/Mappers/Root/NullableTypeMapperAdapter.php +++ b/src/Mappers/Root/NullableTypeMapperAdapter.php @@ -1,12 +1,12 @@ isInstantiableWithoutParameters() !== true) { + if (! ($graphQlType instanceof ResolvableMutableInputObjectType) || $graphQlType->isInstantiableWithoutParameters() !== true) { $graphQlType = GraphQLType::nonNull($graphQlType); } } @@ -149,6 +144,7 @@ private function getNonNullable(Type $type): ?Type if (count($types) > 1) { return new Compound($types); } + return $types[0] ?? null; } diff --git a/src/Schema.php b/src/Schema.php index 9938e4de71..178fc540c8 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -8,7 +8,6 @@ use GraphQL\Type\Definition\Type; use GraphQL\Type\SchemaConfig; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; -use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperInterface; use TheCodingMachine\GraphQLite\Types\TypeResolver; use Webmozart\Assert\Assert; diff --git a/src/TypeRegistry.php b/src/TypeRegistry.php index 7ea306f99c..f9ce17fe91 100644 --- a/src/TypeRegistry.php +++ b/src/TypeRegistry.php @@ -45,6 +45,7 @@ public function registerType(NamedType $type): void * - If the type is already present, does not fail. Instead, return the old type already available. * * @param NamedType&Type&(MutableObjectType|InterfaceType|UnionType|(InputObjectType&ResolvableMutableInputInterface)) $type + * * @return NamedType&Type&(MutableObjectType|InterfaceType|UnionType|(InputObjectType&ResolvableMutableInputInterface)) */ public function getOrRegisterType(NamedType $type): NamedType @@ -53,6 +54,7 @@ public function getOrRegisterType(NamedType $type): NamedType return $this->outputTypes[$type->name]; } $this->outputTypes[$type->name] = $type; + return $type; } From a83f0054257f423ae298aef887b2031d0c1e6b33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Tue, 12 Nov 2019 14:19:43 +0100 Subject: [PATCH 07/13] Ading failing test about array of nullable types --- tests/FieldsBuilderTest.php | 21 +++++++++++++++++++ .../TestControllerWithNullableArray.php | 20 ++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 tests/Fixtures/TestControllerWithNullableArray.php diff --git a/tests/FieldsBuilderTest.php b/tests/FieldsBuilderTest.php index 56a3709419..a24f3ac457 100644 --- a/tests/FieldsBuilderTest.php +++ b/tests/FieldsBuilderTest.php @@ -27,6 +27,7 @@ use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithFailWith; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithInputType; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithInvalidInputType; +use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithNullableArray; use TheCodingMachine\GraphQLite\Fixtures\TestEnum; use TheCodingMachine\GraphQLite\Fixtures\TestTypeWithInvalidPrefetchMethod; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithInvalidReturnType; @@ -688,4 +689,24 @@ public function testSecurityBadQuery(): void $this->expectExceptionMessage('An error occurred while evaluating expression in @Security annotation of method "TheCodingMachine\GraphQLite\Fixtures\TestControllerWithBadSecurity::testBadSecurity": Unexpected token "name" of value "is" around position 6 for expression `this is not valid expression language`.'); $result = $resolve(new stdClass(), [], null, $this->createMock(ResolveInfo::class)); } + + public function testQueryProviderWithNullableArray(): void + { + $controller = new TestControllerWithNullableArray(); + + $queryProvider = $this->buildFieldsBuilder(); + + $queries = $queryProvider->getQueries($controller); + + $this->assertCount(1, $queries); + $usersQuery = $queries[0]; + $this->assertSame('test', $usersQuery->name); + + $this->assertInstanceOf(NonNull::class, $usersQuery->args[0]->getType()); + $this->assertInstanceOf(ListOfType::class, $usersQuery->args[0]->getType()->getWrappedType()); + $this->assertInstanceOf(IntType::class, $usersQuery->args[0]->getType()->getWrappedType()->getWrappedType()); + $this->assertInstanceOf(NonNull::class, $usersQuery->type->getType()); + $this->assertInstanceOf(ListOfType::class, $usersQuery->type->getType()->getWrappedType()); + $this->assertInstanceOf(IntType::class, $usersQuery->type->getType()->getWrappedType()->getWrappedType()); + } } diff --git a/tests/Fixtures/TestControllerWithNullableArray.php b/tests/Fixtures/TestControllerWithNullableArray.php new file mode 100644 index 0000000000..4d025deee8 --- /dev/null +++ b/tests/Fixtures/TestControllerWithNullableArray.php @@ -0,0 +1,20 @@ + $params + * @return array + */ + public function test(array $params): array + { + return $params; + } +} From df7816d8333ad4fe5a4debf94daf926b7526d910 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Tue, 12 Nov 2019 14:26:11 +0100 Subject: [PATCH 08/13] Fixing arrays with nullable types --- src/Mappers/Root/BaseTypeMapper.php | 4 ---- tests/FieldsBuilderTest.php | 6 +++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Mappers/Root/BaseTypeMapper.php b/src/Mappers/Root/BaseTypeMapper.php index 1b7422e6dc..11df07f882 100644 --- a/src/Mappers/Root/BaseTypeMapper.php +++ b/src/Mappers/Root/BaseTypeMapper.php @@ -67,8 +67,6 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection $innerType = $this->topRootTypeMapper->toGraphQLOutputType($type->getValueType(), $subType, $refMethod, $docBlockObj); if ($innerType === null) { return null; - }if ($innerType instanceof NullableType) { - $innerType = GraphQLType::nonNull($innerType); } return GraphQLType::listOf($innerType); @@ -98,8 +96,6 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu $innerType = $this->topRootTypeMapper->toGraphQLInputType($type->getValueType(), $subType, $argumentName, $refMethod, $docBlockObj); if ($innerType === null) { return null; - }if ($innerType instanceof NullableType) { - $innerType = GraphQLType::nonNull($innerType); } return GraphQLType::listOf($innerType); diff --git a/tests/FieldsBuilderTest.php b/tests/FieldsBuilderTest.php index a24f3ac457..2f9be6d435 100644 --- a/tests/FieldsBuilderTest.php +++ b/tests/FieldsBuilderTest.php @@ -705,8 +705,8 @@ public function testQueryProviderWithNullableArray(): void $this->assertInstanceOf(NonNull::class, $usersQuery->args[0]->getType()); $this->assertInstanceOf(ListOfType::class, $usersQuery->args[0]->getType()->getWrappedType()); $this->assertInstanceOf(IntType::class, $usersQuery->args[0]->getType()->getWrappedType()->getWrappedType()); - $this->assertInstanceOf(NonNull::class, $usersQuery->type->getType()); - $this->assertInstanceOf(ListOfType::class, $usersQuery->type->getType()->getWrappedType()); - $this->assertInstanceOf(IntType::class, $usersQuery->type->getType()->getWrappedType()->getWrappedType()); + $this->assertInstanceOf(NonNull::class, $usersQuery->type); + $this->assertInstanceOf(ListOfType::class, $usersQuery->type->getWrappedType()); + $this->assertInstanceOf(IntType::class, $usersQuery->type->getWrappedType()->getWrappedType()); } } From 9cfc3438ded93409b45923e9e540abcfb054bb21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Tue, 12 Nov 2019 18:06:59 +0100 Subject: [PATCH 09/13] Improving code coverage --- src/Mappers/Root/BaseTypeMapper.php | 1 - tests/FieldsBuilderTest.php | 35 +++++++++++++++++++ tests/Fixtures/TestController.php | 2 +- .../TestControllerWithParamDateTime.php | 18 ++++++++++ .../TestControllerWithReturnDateTime.php | 18 ++++++++++ 5 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 tests/Fixtures/TestControllerWithParamDateTime.php create mode 100644 tests/Fixtures/TestControllerWithReturnDateTime.php diff --git a/src/Mappers/Root/BaseTypeMapper.php b/src/Mappers/Root/BaseTypeMapper.php index 11df07f882..b9e5cbf809 100644 --- a/src/Mappers/Root/BaseTypeMapper.php +++ b/src/Mappers/Root/BaseTypeMapper.php @@ -76,7 +76,6 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection return $this->recursiveTypeMapper->mapClassToInterfaceOrType($className, $subType); } - // FIXME: we need to add a $type instanceof Compound here... this will hurt because we need to send back the process to the rootTypeMapper for each inner types... return null; } diff --git a/tests/FieldsBuilderTest.php b/tests/FieldsBuilderTest.php index 2f9be6d435..97428bdc89 100644 --- a/tests/FieldsBuilderTest.php +++ b/tests/FieldsBuilderTest.php @@ -24,10 +24,13 @@ use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithArrayParam; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithArrayReturnType; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithBadSecurity; +use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithParamDateTime; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithFailWith; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithInputType; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithInvalidInputType; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithNullableArray; +use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithParamIterator; +use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithReturnDateTime; use TheCodingMachine\GraphQLite\Fixtures\TestEnum; use TheCodingMachine\GraphQLite\Fixtures\TestTypeWithInvalidPrefetchMethod; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithInvalidReturnType; @@ -409,6 +412,7 @@ public function testQueryProviderWithIterableClass(): void $this->assertCount(7, $queries); $iterableQuery = $queries[3]; + $this->assertSame('arrayObject', $iterableQuery->name); $this->assertInstanceOf(NonNull::class, $iterableQuery->getType()); $this->assertInstanceOf(ListOfType::class, $iterableQuery->getType()->getWrappedType()); $this->assertInstanceOf(NonNull::class, $iterableQuery->getType()->getWrappedType()->getWrappedType()); @@ -425,6 +429,7 @@ public function testQueryProviderWithIterable(): void $this->assertCount(7, $queries); $iterableQuery = $queries[4]; + $this->assertSame('iterable', $iterableQuery->name); $this->assertInstanceOf(NonNull::class, $iterableQuery->getType()); $this->assertInstanceOf(ListOfType::class, $iterableQuery->getType()->getWrappedType()); $this->assertInstanceOf(NonNull::class, $iterableQuery->getType()->getWrappedType()->getWrappedType()); @@ -480,6 +485,10 @@ public function testQueryProviderWithInvalidReturnType(): void $queryProvider->getQueries($controller); } + // FIXME: enable this again when root type mappers are called in a chain (middleware style!) + // FIXME: enable this again when root type mappers are called in a chain (middleware style!) + // FIXME: enable this again when root type mappers are called in a chain (middleware style!) + // FIXME: enable this again when root type mappers are called in a chain (middleware style!) /*public function testQueryProviderWithIterableReturnType(): void { $controller = new TestControllerWithIterableReturnType(); @@ -513,6 +522,10 @@ public function testQueryProviderWithArrayParams(): void $queryProvider->getQueries($controller); } + // FIXME: enable this again when root type mappers are called in a chain (middleware style!) + // FIXME: enable this again when root type mappers are called in a chain (middleware style!) + // FIXME: enable this again when root type mappers are called in a chain (middleware style!) + // FIXME: enable this again when root type mappers are called in a chain (middleware style!) /*public function testQueryProviderWithIterableParams(): void { $controller = new TestControllerWithIterableParam(); @@ -709,4 +722,26 @@ public function testQueryProviderWithNullableArray(): void $this->assertInstanceOf(ListOfType::class, $usersQuery->type->getWrappedType()); $this->assertInstanceOf(IntType::class, $usersQuery->type->getWrappedType()->getWrappedType()); } + + public function testQueryProviderWithParamDateTime(): void + { + $controller = new TestControllerWithParamDateTime(); + + $queryProvider = $this->buildFieldsBuilder(); + + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage('Parameter $dateTime in TheCodingMachine\GraphQLite\Fixtures\TestControllerWithParamDateTime::test is type-hinted to "DateTime". Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.'); + $queries = $queryProvider->getQueries($controller); + } + + public function testQueryProviderWithReturnDateTime(): void + { + $controller = new TestControllerWithReturnDateTime(); + + $queryProvider = $this->buildFieldsBuilder(); + + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage('Return type in TheCodingMachine\GraphQLite\Fixtures\TestControllerWithReturnDateTime::test is type-hinted to "DateTime". Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.'); + $queries = $queryProvider->getQueries($controller); + } } diff --git a/tests/Fixtures/TestController.php b/tests/Fixtures/TestController.php index fb94725783..3a208a3610 100644 --- a/tests/Fixtures/TestController.php +++ b/tests/Fixtures/TestController.php @@ -95,7 +95,7 @@ public function testArrayObject(): ArrayObject } /** - * @Query(name="arrayObject") + * @Query(name="iterable") * @return iterable|TestObject[] */ public function testIterable(): iterable diff --git a/tests/Fixtures/TestControllerWithParamDateTime.php b/tests/Fixtures/TestControllerWithParamDateTime.php new file mode 100644 index 0000000000..c3cfb33fc7 --- /dev/null +++ b/tests/Fixtures/TestControllerWithParamDateTime.php @@ -0,0 +1,18 @@ + Date: Mon, 18 Nov 2019 22:33:48 +0100 Subject: [PATCH 10/13] Migrating RootTypeMapper to a 'middleware' mode --- phpstan.neon | 3 + src/FactoryContext.php | 2 +- src/FieldsBuilder.php | 5 +- src/Mappers/AbstractTypeMapper.php | 3 +- src/Mappers/CannotMapTypeException.php | 9 +- src/Mappers/CompositeTypeMapper.php | 3 +- src/Mappers/Parameters/TypeHandler.php | 19 +--- src/Mappers/PorpaginasTypeMapper.php | 3 +- src/Mappers/RecursiveTypeMapper.php | 7 +- src/Mappers/RecursiveTypeMapperInterface.php | 3 +- src/Mappers/Root/BaseTypeMapper.php | 35 +++--- src/Mappers/Root/CompositeRootTypeMapper.php | 76 ------------- src/Mappers/Root/CompoundTypeMapper.php | 90 ++++++++++----- src/Mappers/Root/FinalRootTypeMapper.php | 64 +++++++++++ src/Mappers/Root/IteratorTypeMapper.php | 34 ++++-- src/Mappers/Root/MyCLabsEnumTypeMapper.php | 43 +++++-- .../Root/NullableTypeMapperAdapter.php | 28 ++--- .../Root/RootTypeMapperFactoryContext.php | 106 ++++++++++++++++++ .../Root/RootTypeMapperFactoryInterface.php | 14 +++ src/Mappers/Root/RootTypeMapperInterface.php | 10 +- src/Mappers/StaticTypeMapper.php | 3 +- src/Mappers/TypeMapperInterface.php | 3 +- src/SchemaFactory.php | 56 +++++---- tests/AbstractQueryProviderTest.php | 20 ++-- tests/FieldsBuilderTest.php | 9 +- tests/Integration/EndToEndTest.php | 21 ++-- tests/Mappers/CompositeTypeMapperTest.php | 3 +- tests/Mappers/Parameters/TypeMapperTest.php | 7 +- tests/Mappers/Root/BaseTypeMapperTest.php | 25 +++-- .../Root/CompositeRootTypeMapperTest.php | 47 -------- .../Root/MyCLabsEnumTypeMapperTest.php | 10 +- tests/Mappers/Root/VoidRootTypeMapper.php | 58 ++++++++++ .../Root/VoidRootTypeMapperFactory.php | 13 +++ tests/RootTypeMapperFactoryContextTest.php | 38 +++++++ tests/SchemaFactoryTest.php | 4 +- 35 files changed, 575 insertions(+), 299 deletions(-) delete mode 100644 src/Mappers/Root/CompositeRootTypeMapper.php create mode 100644 src/Mappers/Root/FinalRootTypeMapper.php create mode 100644 src/Mappers/Root/RootTypeMapperFactoryContext.php create mode 100644 src/Mappers/Root/RootTypeMapperFactoryInterface.php delete mode 100644 tests/Mappers/Root/CompositeRootTypeMapperTest.php create mode 100644 tests/Mappers/Root/VoidRootTypeMapper.php create mode 100644 tests/Mappers/Root/VoidRootTypeMapperFactory.php create mode 100644 tests/RootTypeMapperFactoryContextTest.php diff --git a/phpstan.neon b/phpstan.neon index 49a0305c14..0ab67031ae 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -18,6 +18,9 @@ parameters: - message: '#Parameter \#2 \$subType of method .* expects#' path: src/Mappers/Root/IteratorTypeMapper.php + - + message: '#Parameter \#2 \$subType of method .* expects#' + path: src/Mappers/Root/CompoundTypeMapper.php - '#Call to an undefined method GraphQL\\Error\\ClientAware::getMessage()#' #- # message: '#If condition is always true#' diff --git a/src/FactoryContext.php b/src/FactoryContext.php index 569d951bc6..d9062a3f3b 100644 --- a/src/FactoryContext.php +++ b/src/FactoryContext.php @@ -14,7 +14,7 @@ * Those classes are made available to factories implementing QueryProviderFactoryInterface * or TypeMapperFactoryInterface */ -class FactoryContext +final class FactoryContext { /** @var AnnotationReader */ private $annotationReader; diff --git a/src/FieldsBuilder.php b/src/FieldsBuilder.php index 1d5f170085..c1f5afd783 100644 --- a/src/FieldsBuilder.php +++ b/src/FieldsBuilder.php @@ -65,15 +65,14 @@ public function __construct( NamingStrategyInterface $namingStrategy, RootTypeMapperInterface $rootTypeMapper, ParameterMiddlewareInterface $parameterMapper, - FieldMiddlewareInterface $fieldMiddleware, - TypeRegistry $typeRegistry + FieldMiddlewareInterface $fieldMiddleware ) { $this->annotationReader = $annotationReader; $this->recursiveTypeMapper = $typeMapper; $this->typeResolver = $typeResolver; $this->cachedDocBlockFactory = $cachedDocBlockFactory; $this->namingStrategy = $namingStrategy; - $this->typeMapper = new TypeHandler($typeMapper, $argumentResolver, $rootTypeMapper, $typeResolver, $typeRegistry); + $this->typeMapper = new TypeHandler($argumentResolver, $rootTypeMapper, $typeResolver); $this->parameterMapper = $parameterMapper; $this->fieldMiddleware = $fieldMiddleware; } diff --git a/src/Mappers/AbstractTypeMapper.php b/src/Mappers/AbstractTypeMapper.php index 3bbab88833..6179a715af 100644 --- a/src/Mappers/AbstractTypeMapper.php +++ b/src/Mappers/AbstractTypeMapper.php @@ -5,6 +5,7 @@ namespace TheCodingMachine\GraphQLite\Mappers; use GraphQL\Type\Definition\InputObjectType; +use GraphQL\Type\Definition\NamedType; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\Type; use Psr\Container\ContainerInterface; @@ -308,7 +309,7 @@ public function mapClassToInputType(string $className): ResolvableMutableInputIn * * @param string $typeName The name of the GraphQL type * - * @return Type&((ResolvableMutableInputInterface&InputObjectType)|MutableObjectType|MutableInterfaceType) + * @return NamedType&Type&((ResolvableMutableInputInterface&InputObjectType)|MutableObjectType|MutableInterfaceType) * * @throws CannotMapTypeExceptionInterface * @throws ReflectionException diff --git a/src/Mappers/CannotMapTypeException.php b/src/Mappers/CannotMapTypeException.php index 80eb1d03d1..3ab94b40ef 100644 --- a/src/Mappers/CannotMapTypeException.php +++ b/src/Mappers/CannotMapTypeException.php @@ -47,16 +47,17 @@ public static function createForParseError(Error $error): self public static function createForBadTypeInUnion(array $unionTypes): self { $disallowedTypeNames = array_map(static function (Type $type) { - if ($type instanceof NamedType) { - return $type->name; - } - return (string) $type; }, $unionTypes); return new self('in GraphQL, you can only use union types between objects. These types cannot be used in union types: ' . implode(', ', $disallowedTypeNames)); } + public static function createForUnionInInputType(Type $type): self + { + return new self('in GraphQL, you can only use union types in output types, not in input types. You cannot use this type: ' . $type); + } + public static function mustBeOutputType(string $subTypeName): self { return new self('type "' . $subTypeName . '" must be an output type.'); diff --git a/src/Mappers/CompositeTypeMapper.php b/src/Mappers/CompositeTypeMapper.php index c68d2f31ae..90c6f5e8be 100644 --- a/src/Mappers/CompositeTypeMapper.php +++ b/src/Mappers/CompositeTypeMapper.php @@ -5,6 +5,7 @@ namespace TheCodingMachine\GraphQLite\Mappers; use GraphQL\Type\Definition\InputObjectType; +use GraphQL\Type\Definition\NamedType; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\Type; use TheCodingMachine\GraphQLite\Types\MutableInterface; @@ -120,7 +121,7 @@ public function mapClassToInputType(string $className): ResolvableMutableInputIn * * @param string $typeName The name of the GraphQL type * - * @return Type&((ResolvableMutableInputInterface&InputObjectType)|MutableInterface) + * @return NamedType&Type&((ResolvableMutableInputInterface&InputObjectType)|MutableInterface) */ public function mapNameToType(string $typeName): Type { diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index ed17b430a9..b5a1392d69 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -28,13 +28,11 @@ use TheCodingMachine\GraphQLite\Annotations\UseInputType; use TheCodingMachine\GraphQLite\InvalidDocBlockRuntimeException; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeExceptionInterface; -use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperInterface; use TheCodingMachine\GraphQLite\Parameters\DefaultValueParameter; use TheCodingMachine\GraphQLite\Parameters\InputTypeParameter; use TheCodingMachine\GraphQLite\Parameters\ParameterInterface; use TheCodingMachine\GraphQLite\TypeMappingRuntimeException; -use TheCodingMachine\GraphQLite\TypeRegistry; use TheCodingMachine\GraphQLite\Types\ArgumentResolver; use TheCodingMachine\GraphQLite\Types\TypeResolver; use Webmozart\Assert\Assert; @@ -48,30 +46,22 @@ class TypeHandler implements ParameterHandlerInterface { /** @var PhpDocumentorTypeResolver */ private $phpDocumentorTypeResolver; - /** @var RecursiveTypeMapperInterface */ - private $recursiveTypeMapper; /** @var ArgumentResolver */ private $argumentResolver; /** @var RootTypeMapperInterface */ private $rootTypeMapper; /** @var TypeResolver */ private $typeResolver; - /** @var TypeRegistry */ - private $typeRegistry; public function __construct( - RecursiveTypeMapperInterface $typeMapper, ArgumentResolver $argumentResolver, RootTypeMapperInterface $rootTypeMapper, - TypeResolver $typeResolver, - TypeRegistry $typeRegistry + TypeResolver $typeResolver ) { - $this->recursiveTypeMapper = $typeMapper; $this->argumentResolver = $argumentResolver; $this->rootTypeMapper = $rootTypeMapper; $this->phpDocumentorTypeResolver = new PhpDocumentorTypeResolver(); $this->typeResolver = $typeResolver; - $this->typeRegistry = $typeRegistry; } /** @@ -195,10 +185,6 @@ private function mapType(Type $type, ?Type $docBlockType, bool $isNullable, bool } else { $graphQlType = $this->rootTypeMapper->toGraphQLOutputType($docBlockType, null, $refMethod, $docBlockObj); } - - if ($graphQlType === null) { - throw TypeMappingRuntimeException::createFromType($docBlockType); - } } else { $completeType = $this->appendTypes($type, $docBlockType); if ($mapToInputType === true) { @@ -207,9 +193,6 @@ private function mapType(Type $type, ?Type $docBlockType, bool $isNullable, bool } else { $graphQlType = $this->rootTypeMapper->toGraphQLOutputType($completeType, null, $refMethod, $docBlockObj); } - if ($graphQlType === null) { - throw TypeMappingRuntimeException::createFromType($completeType); - } } return $graphQlType; diff --git a/src/Mappers/PorpaginasTypeMapper.php b/src/Mappers/PorpaginasTypeMapper.php index efa25be6a2..9949f1e958 100644 --- a/src/Mappers/PorpaginasTypeMapper.php +++ b/src/Mappers/PorpaginasTypeMapper.php @@ -5,6 +5,7 @@ namespace TheCodingMachine\GraphQLite\Mappers; use GraphQL\Type\Definition\InputObjectType; +use GraphQL\Type\Definition\NamedType; use GraphQL\Type\Definition\NullableType; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\Type; @@ -134,7 +135,7 @@ public function canMapNameToType(string $typeName): bool * * @param string $typeName The name of the GraphQL type * - * @return Type&((ResolvableMutableInputInterface&InputObjectType)|MutableObjectType|MutableInterfaceType) + * @return NamedType&Type&((ResolvableMutableInputInterface&InputObjectType)|MutableObjectType|MutableInterfaceType) * * @throws CannotMapTypeExceptionInterface */ diff --git a/src/Mappers/RecursiveTypeMapper.php b/src/Mappers/RecursiveTypeMapper.php index 1ededd74c2..489eb2ad5a 100644 --- a/src/Mappers/RecursiveTypeMapper.php +++ b/src/Mappers/RecursiveTypeMapper.php @@ -7,6 +7,7 @@ use GraphQL\Type\Definition\InputObjectType; use GraphQL\Type\Definition\InputType; use GraphQL\Type\Definition\InterfaceType; +use GraphQL\Type\Definition\NamedType; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\Type; use Psr\SimpleCache\CacheInterface; @@ -45,7 +46,7 @@ class RecursiveTypeMapper implements RecursiveTypeMapperInterface /** * An array of interfaces OR object types if no interface matching. * - * @var array + * @var array */ private $interfaces = []; @@ -209,7 +210,7 @@ private function extendType(string $className, MutableInterface $type): void * @param string $className The exact class name to look for (this function does not look into parent classes). * @param (OutputType&Type)|null $subType A subtype (if the main className is an iterator) * - * @return OutputType&Type + * @return OutputType&Type&NamedType * * @throws CannotMapTypeExceptionInterface */ @@ -462,7 +463,7 @@ public function canMapNameToType(string $typeName): bool * * @param string $typeName The name of the GraphQL type * - * @return Type&(InputType|OutputType) + * @return NamedType&Type&(InputType|OutputType) */ public function mapNameToType(string $typeName): Type { diff --git a/src/Mappers/RecursiveTypeMapperInterface.php b/src/Mappers/RecursiveTypeMapperInterface.php index f485a7959a..114178b3e8 100644 --- a/src/Mappers/RecursiveTypeMapperInterface.php +++ b/src/Mappers/RecursiveTypeMapperInterface.php @@ -7,6 +7,7 @@ use GraphQL\Type\Definition\InputObjectType; use GraphQL\Type\Definition\InputType; use GraphQL\Type\Definition\InterfaceType; +use GraphQL\Type\Definition\NamedType; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\Type; use TheCodingMachine\GraphQLite\Types\MutableInterfaceType; @@ -89,7 +90,7 @@ public function canMapNameToType(string $typeName): bool; * * @param string $typeName The name of the GraphQL type * - * @return Type&(InputType|OutputType) + * @return NamedType&Type&(InputType|OutputType) */ public function mapNameToType(string $typeName): Type; diff --git a/src/Mappers/Root/BaseTypeMapper.php b/src/Mappers/Root/BaseTypeMapper.php index b9e5cbf809..1c2e7a51b0 100644 --- a/src/Mappers/Root/BaseTypeMapper.php +++ b/src/Mappers/Root/BaseTypeMapper.php @@ -10,7 +10,6 @@ use GraphQL\Type\Definition\InputType; use GraphQL\Type\Definition\IntType; use GraphQL\Type\Definition\NamedType; -use GraphQL\Type\Definition\NullableType; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\StringType; use GraphQL\Type\Definition\Type as GraphQLType; @@ -43,31 +42,35 @@ class BaseTypeMapper implements RootTypeMapperInterface private $recursiveTypeMapper; /** @var RootTypeMapperInterface */ private $topRootTypeMapper; + /** @var RootTypeMapperInterface */ + private $next; - public function __construct(RecursiveTypeMapperInterface $recursiveTypeMapper, RootTypeMapperInterface $topRootTypeMapper) + public function __construct(RootTypeMapperInterface $next, RecursiveTypeMapperInterface $recursiveTypeMapper, RootTypeMapperInterface $topRootTypeMapper) { $this->recursiveTypeMapper = $recursiveTypeMapper; $this->topRootTypeMapper = $topRootTypeMapper; + $this->next = $next; } /** * @param (OutputType&GraphQLType)|null $subType * - * @return (OutputType&GraphQLType)|null + * @return OutputType&GraphQLType * * @throws CannotMapTypeExceptionInterface */ - public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): OutputType { $mappedType = $this->mapBaseType($type); if ($mappedType !== null) { return $mappedType; } + if ($type instanceof Array_) { $innerType = $this->topRootTypeMapper->toGraphQLOutputType($type->getValueType(), $subType, $refMethod, $docBlockObj); - if ($innerType === null) { - return null; - } + /*if ($innerType === null) { + return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); + }*/ return GraphQLType::listOf($innerType); } @@ -77,15 +80,15 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection return $this->recursiveTypeMapper->mapClassToInterfaceOrType($className, $subType); } - return null; + return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); } /** * @param (InputType&GraphQLType)|null $subType * - * @return (InputType&GraphQLType)|null + * @return InputType&GraphQLType */ - public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType { $mappedType = $this->mapBaseType($type); if ($mappedType !== null) { @@ -93,9 +96,9 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu } if ($type instanceof Array_) { $innerType = $this->topRootTypeMapper->toGraphQLInputType($type->getValueType(), $subType, $argumentName, $refMethod, $docBlockObj); - if ($innerType === null) { - return null; - } + /*if ($innerType === null) { + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); + }*/ return GraphQLType::listOf($innerType); } @@ -105,7 +108,7 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu return $this->recursiveTypeMapper->mapClassToInputType($className); } - return null; + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); } /** @@ -183,7 +186,7 @@ private static function getDateTimeType(): DateTimeType * * @param string $typeName The name of the GraphQL type */ - public function mapNameToType(string $typeName): ?NamedType + public function mapNameToType(string $typeName): NamedType { // No need to map base types, only types added by us. if ($typeName === 'Upload') { @@ -194,6 +197,6 @@ public function mapNameToType(string $typeName): ?NamedType return self::getDateTimeType(); } - return null; + return $this->next->mapNameToType($typeName); } } diff --git a/src/Mappers/Root/CompositeRootTypeMapper.php b/src/Mappers/Root/CompositeRootTypeMapper.php deleted file mode 100644 index 98ed05127a..0000000000 --- a/src/Mappers/Root/CompositeRootTypeMapper.php +++ /dev/null @@ -1,76 +0,0 @@ -rootTypeMappers = is_array($rootTypeMappers) ? $rootTypeMappers : iterator_to_array($rootTypeMappers); - } - - public function addRootTypeMapper(RootTypeMapperInterface $rootTypeMapper): void - { - $this->rootTypeMappers[] = $rootTypeMapper; - } - - public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType - { - foreach ($this->rootTypeMappers as $rootTypeMapper) { - $mappedType = $rootTypeMapper->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); - if ($mappedType !== null) { - return $mappedType; - } - } - - return null; - } - - public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType - { - foreach ($this->rootTypeMappers as $rootTypeMapper) { - $mappedType = $rootTypeMapper->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); - if ($mappedType !== null) { - return $mappedType; - } - } - - return null; - } - - /** - * Returns a GraphQL type by name. - * If this root type mapper can return this type in "toGraphQLOutputType" or "toGraphQLInputType", it should - * also map these types by name in the "mapNameToType" method. - * - * @param string $typeName The name of the GraphQL type - */ - public function mapNameToType(string $typeName): ?NamedType - { - foreach ($this->rootTypeMappers as $rootTypeMapper) { - $mappedType = $rootTypeMapper->mapNameToType($typeName); - if ($mappedType !== null) { - return $mappedType; - } - } - - return null; - } -} diff --git a/src/Mappers/Root/CompoundTypeMapper.php b/src/Mappers/Root/CompoundTypeMapper.php index f3cbeaf651..e787aa4b31 100644 --- a/src/Mappers/Root/CompoundTypeMapper.php +++ b/src/Mappers/Root/CompoundTypeMapper.php @@ -4,7 +4,9 @@ namespace TheCodingMachine\GraphQLite\Mappers\Root; +use Closure; use GraphQL\Type\Definition\InputType; +use GraphQL\Type\Definition\ListOfType; use GraphQL\Type\Definition\NamedType; use GraphQL\Type\Definition\NonNull; use GraphQL\Type\Definition\ObjectType; @@ -13,12 +15,14 @@ use phpDocumentor\Reflection\DocBlock; use phpDocumentor\Reflection\Type; use phpDocumentor\Reflection\Types\Compound; +use phpDocumentor\Reflection\Types\Iterable_; use ReflectionMethod; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; use TheCodingMachine\GraphQLite\TypeMappingRuntimeException; use TheCodingMachine\GraphQLite\TypeRegistry; use TheCodingMachine\GraphQLite\Types\UnionType; +use Webmozart\Assert\Assert; use function array_filter; use function array_values; use function count; @@ -36,53 +40,61 @@ class CompoundTypeMapper implements RootTypeMapperInterface private $typeRegistry; /** @var RecursiveTypeMapperInterface */ private $recursiveTypeMapper; + /** @var RootTypeMapperInterface */ + private $next; - public function __construct(RootTypeMapperInterface $topRootTypeMapper, TypeRegistry $typeRegistry, RecursiveTypeMapperInterface $recursiveTypeMapper) + public function __construct(RootTypeMapperInterface $next, RootTypeMapperInterface $topRootTypeMapper, TypeRegistry $typeRegistry, RecursiveTypeMapperInterface $recursiveTypeMapper) { $this->topRootTypeMapper = $topRootTypeMapper; $this->typeRegistry = $typeRegistry; $this->recursiveTypeMapper = $recursiveTypeMapper; + $this->next = $next; } /** * @param (OutputType&GraphQLType)|null $subType * - * @return (OutputType&GraphQLType)|null + * @return OutputType&GraphQLType */ - public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): OutputType { if (! $type instanceof Compound) { - return null; - } - - $filteredDocBlockTypes = iterator_to_array($type); - if (empty($filteredDocBlockTypes)) { - throw TypeMappingRuntimeException::createFromType($type); + return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); } - $unionTypes = []; - $lastException = null; - foreach ($filteredDocBlockTypes as $singleDocBlockType) { - $unionTypes[] = $this->topRootTypeMapper->toGraphQLOutputType($singleDocBlockType, null, $refMethod, $docBlockObj); - } + $result = $this->toGraphQLType($type, function (Type $type, ?OutputType $subType) use ($refMethod, $docBlockObj) { + return $this->topRootTypeMapper->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); + }, true); - /** @var OutputType&GraphQLType $return */ - $return = $this->getTypeFromUnion($unionTypes); + Assert::isInstanceOf($result, OutputType::class); - return $return; + return $result; } /** * @param (InputType&GraphQLType)|null $subType * - * @return (InputType&GraphQLType)|null + * @return InputType&GraphQLType */ - public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType { if (! $type instanceof Compound) { - return null; + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); } + $result = $this->toGraphQLType($type, function (Type $type, ?InputType $subType) use ($refMethod, $docBlockObj, $argumentName) { + return $this->topRootTypeMapper->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); + }, false); + Assert::isInstanceOf($result, InputType::class); + + return $result; + } + + /** + * @return (OutputType&GraphQLType)|(InputType&GraphQLType) + */ + private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $isOutputType): GraphQLType + { $filteredDocBlockTypes = iterator_to_array($type); if (empty($filteredDocBlockTypes)) { throw TypeMappingRuntimeException::createFromType($type); @@ -90,16 +102,45 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu $unionTypes = []; $lastException = null; + $mustBeIterable = false; foreach ($filteredDocBlockTypes as $singleDocBlockType) { - $unionTypes[] = $this->topRootTypeMapper->toGraphQLInputType($singleDocBlockType, null, $argumentName, $refMethod, $docBlockObj); + if ($singleDocBlockType instanceof Iterable_) { + $mustBeIterable = true; + continue; + } + $unionTypes[] = $topToGraphQLType($singleDocBlockType, null); } - /** @var InputType&GraphQLType $return */ + if ($mustBeIterable && empty($unionTypes)) { + throw TypeMappingRuntimeException::createFromType(new Iterable_()); + } + + /** @var OutputType&GraphQLType $return */ $return = $this->getTypeFromUnion($unionTypes); + if ($mustBeIterable && ! $this->isWrappedListOfType($return)) { + // The compound type is iterable and the other type is not iterable. Both types are incompatible + // For instance: @return iterable|User + // FIXME: better error message! + throw TypeMappingRuntimeException::createFromType(new Iterable_()); + } + + if (! $isOutputType && ($return instanceof UnionType || ($return instanceof NonNull && $return->getWrappedType() instanceof UnionType))) { + throw CannotMapTypeException::createForUnionInInputType($return); + } + return $return; } + private function isWrappedListOfType(GraphQLType $type): bool + { + if ($type instanceof ListOfType) { + return true; + } + + return $type instanceof NonNull && $type->getWrappedType() instanceof ListOfType; + } + /* * @template T of InputType|OutputType|null * @param array $unionTypes @@ -164,9 +205,8 @@ private function getTypeFromUnion(array $unionTypes): GraphQLType * * @param string $typeName The name of the GraphQL type */ - public function mapNameToType(string $typeName): ?NamedType + public function mapNameToType(string $typeName): NamedType { - // TODO: maybe we should map "Union" types here? - return null; + return $this->next->mapNameToType($typeName); } } diff --git a/src/Mappers/Root/FinalRootTypeMapper.php b/src/Mappers/Root/FinalRootTypeMapper.php new file mode 100644 index 0000000000..aadc08fede --- /dev/null +++ b/src/Mappers/Root/FinalRootTypeMapper.php @@ -0,0 +1,64 @@ +recursiveTypeMapper = $recursiveTypeMapper; + } + + /** + * @param (OutputType&GraphQLType)|null $subType + * + * @return OutputType&GraphQLType + */ + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): OutputType + { + throw TypeMappingRuntimeException::createFromType($type); + } + + /** + * @param (InputType&GraphQLType)|null $subType + * + * @return InputType&GraphQLType + */ + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType + { + throw TypeMappingRuntimeException::createFromType($type); + } + + /** + * Returns a GraphQL type by name. + * If this root type mapper can return this type in "toGraphQLOutputType" or "toGraphQLInputType", it should + * also map these types by name in the "mapNameToType" method. + * + * @param string $typeName The name of the GraphQL type + */ + public function mapNameToType(string $typeName): NamedType + { + return $this->recursiveTypeMapper->mapNameToType($typeName); + } +} diff --git a/src/Mappers/Root/IteratorTypeMapper.php b/src/Mappers/Root/IteratorTypeMapper.php index cfb704bdf4..fe02906d92 100644 --- a/src/Mappers/Root/IteratorTypeMapper.php +++ b/src/Mappers/Root/IteratorTypeMapper.php @@ -42,29 +42,36 @@ class IteratorTypeMapper implements RootTypeMapperInterface private $typeRegistry; /** @var RecursiveTypeMapperInterface */ private $recursiveTypeMapper; + /** @var RootTypeMapperInterface */ + private $next; - public function __construct(RootTypeMapperInterface $topRootTypeMapper, TypeRegistry $typeRegistry, RecursiveTypeMapperInterface $recursiveTypeMapper) + public function __construct(RootTypeMapperInterface $next, RootTypeMapperInterface $topRootTypeMapper, TypeRegistry $typeRegistry, RecursiveTypeMapperInterface $recursiveTypeMapper) { $this->topRootTypeMapper = $topRootTypeMapper; $this->typeRegistry = $typeRegistry; $this->recursiveTypeMapper = $recursiveTypeMapper; + $this->next = $next; } /** * @param (OutputType&GraphQLType)|null $subType * - * @return (OutputType&GraphQLType)|null + * @return OutputType&GraphQLType */ - public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): OutputType { if (! $type instanceof Compound) { - return null; + return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); } $result = $this->toGraphQLType($type, function (Type $type, ?OutputType $subType) use ($refMethod, $docBlockObj) { return $this->topRootTypeMapper->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); }, true); - Assert::nullOrIsInstanceOf($result, OutputType::class); + + if ($result === null) { + return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); + } + Assert::isInstanceOf($result, OutputType::class); return $result; } @@ -72,18 +79,21 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection /** * @param (InputType&GraphQLType)|null $subType * - * @return (InputType&GraphQLType)|null + * @return InputType&GraphQLType */ - public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType { if (! $type instanceof Compound) { - return null; + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); } $result = $this->toGraphQLType($type, function (Type $type, ?InputType $subType) use ($refMethod, $docBlockObj, $argumentName) { return $this->topRootTypeMapper->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); - }, true); - Assert::nullOrIsInstanceOf($result, InputType::class); + }, false); + if ($result === null) { + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); + } + Assert::isInstanceOf($result, InputType::class); return $result; } @@ -95,10 +105,10 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu * * @param string $typeName The name of the GraphQL type */ - public function mapNameToType(string $typeName): ?NamedType + public function mapNameToType(string $typeName): NamedType { // TODO: how to handle this? Do we need? - return null; + return $this->next->mapNameToType($typeName); } /** diff --git a/src/Mappers/Root/MyCLabsEnumTypeMapper.php b/src/Mappers/Root/MyCLabsEnumTypeMapper.php index 452b0f8198..cf91136058 100644 --- a/src/Mappers/Root/MyCLabsEnumTypeMapper.php +++ b/src/Mappers/Root/MyCLabsEnumTypeMapper.php @@ -26,18 +26,42 @@ class MyCLabsEnumTypeMapper implements RootTypeMapperInterface { /** @var array */ private $cache = []; + /** @var RootTypeMapperInterface */ + private $next; + + public function __construct(RootTypeMapperInterface $next) + { + $this->next = $next; + } /** - * @return (GraphQLType&OutputType)|null + * @param (OutputType&GraphQLType)|null $subType + * + * @return OutputType&GraphQLType */ - public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): OutputType { - return $this->map($type); + $result = $this->map($type); + if ($result === null) { + return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); + } + + return $result; } - public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType + /** + * @param (InputType&GraphQLType)|null $subType + * + * @return InputType&GraphQLType + */ + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType { - return $this->map($type); + $result = $this->map($type); + if ($result === null) { + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); + } + + return $result; } private function map(Type $type): ?EnumType @@ -82,14 +106,17 @@ private function mapByClassName(string $enumClass): ?EnumType * * @param string $typeName The name of the GraphQL type */ - public function mapNameToType(string $typeName): ?NamedType + public function mapNameToType(string $typeName): NamedType { if (strpos($typeName, 'MyCLabsEnum_') === 0) { $className = str_replace('__', '\\', substr($typeName, 12)); - return $this->mapByClassName($className); + $type = $this->mapByClassName($className); + if ($type !== null) { + return $type; + } } - return null; + return $this->next->mapNameToType($typeName); } } diff --git a/src/Mappers/Root/NullableTypeMapperAdapter.php b/src/Mappers/Root/NullableTypeMapperAdapter.php index 6f5b6540d7..e7bea33a08 100644 --- a/src/Mappers/Root/NullableTypeMapperAdapter.php +++ b/src/Mappers/Root/NullableTypeMapperAdapter.php @@ -29,19 +29,19 @@ class NullableTypeMapperAdapter implements RootTypeMapperInterface { /** @var RootTypeMapperInterface */ - private $rootTypeMapper; + private $next; - public function __construct(RootTypeMapperInterface $rootTypeMapper) + public function setNext(RootTypeMapperInterface $next): void { - $this->rootTypeMapper = $rootTypeMapper; + $this->next = $next; } /** * @param (OutputType&GraphQLType)|null $subType * - * @return (OutputType&GraphQLType)|null + * @return OutputType&GraphQLType */ - public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): OutputType { // Let's check a "null" value in the docblock $isNullable = $this->isNullable($type); @@ -54,10 +54,7 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection $type = $nonNullableType; } - $graphQlType = $this->rootTypeMapper->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); - if ($graphQlType === null) { - return null; - } + $graphQlType = $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); if (! $isNullable && $graphQlType instanceof NullableType) { $graphQlType = GraphQLType::nonNull($graphQlType); @@ -69,9 +66,9 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection /** * @param (InputType&GraphQLType)|null $subType * - * @return (InputType&GraphQLType)|null + * @return InputType&GraphQLType */ - public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType { // Let's check a "null" value in the docblock $isNullable = $this->isNullable($type); @@ -84,10 +81,7 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu $type = $nonNullableType; } - $graphQlType = $this->rootTypeMapper->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); - if ($graphQlType === null) { - return null; - } + $graphQlType = $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); // The type is non nullable if the PHP argument is non nullable // There is an exception: if the PHP argument is non nullable but points to a factory that can called without passing any argument, @@ -108,9 +102,9 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu * * @param string $typeName The name of the GraphQL type */ - public function mapNameToType(string $typeName): ?NamedType + public function mapNameToType(string $typeName): NamedType { - return $this->rootTypeMapper->mapNameToType($typeName); + return $this->next->mapNameToType($typeName); } private function isNullable(Type $docBlockTypeHint): bool diff --git a/src/Mappers/Root/RootTypeMapperFactoryContext.php b/src/Mappers/Root/RootTypeMapperFactoryContext.php new file mode 100644 index 0000000000..6f138cc59c --- /dev/null +++ b/src/Mappers/Root/RootTypeMapperFactoryContext.php @@ -0,0 +1,106 @@ +annotationReader = $annotationReader; + $this->typeResolver = $typeResolver; + $this->namingStrategy = $namingStrategy; + $this->typeRegistry = $typeRegistry; + $this->recursiveTypeMapper = $recursiveTypeMapper; + $this->container = $container; + $this->cache = $cache; + $this->globTtl = $globTtl; + $this->mapTtl = $mapTtl; + } + + public function getAnnotationReader(): AnnotationReader + { + return $this->annotationReader; + } + + public function getTypeResolver(): TypeResolver + { + return $this->typeResolver; + } + + public function getNamingStrategy(): NamingStrategyInterface + { + return $this->namingStrategy; + } + + public function getTypeRegistry(): TypeRegistry + { + return $this->typeRegistry; + } + + public function getRecursiveTypeMapper(): RecursiveTypeMapperInterface + { + return $this->recursiveTypeMapper; + } + + public function getContainer(): ContainerInterface + { + return $this->container; + } + + public function getCache(): CacheInterface + { + return $this->cache; + } + + public function getGlobTtl(): ?int + { + return $this->globTtl; + } + + public function getMapTtl(): ?int + { + return $this->mapTtl; + } +} diff --git a/src/Mappers/Root/RootTypeMapperFactoryInterface.php b/src/Mappers/Root/RootTypeMapperFactoryInterface.php new file mode 100644 index 0000000000..19b0101213 --- /dev/null +++ b/src/Mappers/Root/RootTypeMapperFactoryInterface.php @@ -0,0 +1,14 @@ +rootTypeMappers[] = $rootTypeMapper; + $this->rootTypeMapperFactories[] = $rootTypeMapperFactory; return $this; } @@ -327,16 +329,33 @@ public function createSchema(): Schema $compositeTypeMapper = new CompositeTypeMapper(); $recursiveTypeMapper = new RecursiveTypeMapper($compositeTypeMapper, $namingStrategy, $this->cache, $typeRegistry); - $compositeRootTypeMapper = new CompositeRootTypeMapper(); - $topRootTypeMapper = new NullableTypeMapperAdapter($compositeRootTypeMapper); + $topRootTypeMapper = new NullableTypeMapperAdapter(); - $compositeRootTypeMapper->addRootTypeMapper(new IteratorTypeMapper($topRootTypeMapper, $typeRegistry, $recursiveTypeMapper)); - $compositeRootTypeMapper->addRootTypeMapper(new CompoundTypeMapper($topRootTypeMapper, $typeRegistry, $recursiveTypeMapper)); - foreach ($this->rootTypeMappers as $rootTypeMapper) { - $compositeRootTypeMapper->addRootTypeMapper($rootTypeMapper); + $errorRootTypeMapper = new FinalRootTypeMapper($recursiveTypeMapper); + $rootTypeMapper = new BaseTypeMapper($errorRootTypeMapper, $recursiveTypeMapper, $topRootTypeMapper); + $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper); + + if (! empty($this->rootTypeMapperFactories)) { + $rootSchemaFactoryContext = new RootTypeMapperFactoryContext( + $annotationReader, + $typeResolver, + $namingStrategy, + $typeRegistry, + $recursiveTypeMapper, + $this->container, + $this->cache + ); + + $reversedRootTypeMapperFactories = array_reverse($this->rootTypeMapperFactories); + foreach ($reversedRootTypeMapperFactories as $rootTypeMapperFactory) { + $rootTypeMapper = $rootTypeMapperFactory->create($rootTypeMapper, $rootSchemaFactoryContext); + } } - $compositeRootTypeMapper->addRootTypeMapper(new MyCLabsEnumTypeMapper()); - $compositeRootTypeMapper->addRootTypeMapper(new BaseTypeMapper($recursiveTypeMapper, $topRootTypeMapper)); + + $rootTypeMapper = new CompoundTypeMapper($rootTypeMapper, $topRootTypeMapper, $typeRegistry, $recursiveTypeMapper); + $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $topRootTypeMapper, $typeRegistry, $recursiveTypeMapper); + + $topRootTypeMapper->setNext($rootTypeMapper); $argumentResolver = new ArgumentResolver(); @@ -356,15 +375,14 @@ public function createSchema(): Schema $namingStrategy, $topRootTypeMapper, $parameterMiddlewarePipe, - $fieldMiddlewarePipe, - $typeRegistry + $fieldMiddlewarePipe ); $typeGenerator = new TypeGenerator($annotationReader, $namingStrategy, $typeRegistry, $this->container, $recursiveTypeMapper, $fieldsBuilder); $inputTypeUtils = new InputTypeUtils($annotationReader, $namingStrategy); $inputTypeGenerator = new InputTypeGenerator($inputTypeUtils, $fieldsBuilder); - if (empty($this->typeNamespaces) && empty($this->typeMappers) && empty($this->typeMapperFactories)) { + if (empty($this->typeNamespaces) && empty($this->typeMappers)) { throw new GraphQLRuntimeException('Cannot create schema: no namespace for types found (You must call the SchemaFactory::addTypeNamespace() at least once).'); } @@ -435,6 +453,6 @@ public function createSchema(): Schema $aggregateQueryProvider = new AggregateQueryProvider($queryProviders); - return new Schema($aggregateQueryProvider, $recursiveTypeMapper, $typeResolver, $compositeRootTypeMapper, $this->schemaConfig); + return new Schema($aggregateQueryProvider, $recursiveTypeMapper, $typeResolver, $topRootTypeMapper, $this->schemaConfig); } } diff --git a/tests/AbstractQueryProviderTest.php b/tests/AbstractQueryProviderTest.php index b943d85a29..8ac5b07aa5 100644 --- a/tests/AbstractQueryProviderTest.php +++ b/tests/AbstractQueryProviderTest.php @@ -30,9 +30,11 @@ use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompositeRootTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompoundTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\FinalRootTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\IteratorTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\MyCLabsEnumTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\NullableTypeMapperAdapter; +use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperFactoryContext; use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\TypeMapperInterface; use TheCodingMachine\GraphQLite\Containers\EmptyContainer; @@ -51,6 +53,7 @@ use TheCodingMachine\GraphQLite\Types\ResolvableMutableInputInterface; use TheCodingMachine\GraphQLite\Types\ResolvableMutableInputObjectType; use TheCodingMachine\GraphQLite\Types\TypeResolver; +use function array_reverse; abstract class AbstractQueryProviderTest extends TestCase { @@ -296,8 +299,7 @@ protected function buildFieldsBuilder(): FieldsBuilder new NamingStrategy(), $this->buildRootTypeMapper(), $this->getParameterMiddlewarePipe(), - $fieldMiddlewarePipe, - $this->getTypeRegistry() + $fieldMiddlewarePipe ); } @@ -311,13 +313,15 @@ protected function getRootTypeMapper(): RootTypeMapperInterface protected function buildRootTypeMapper(): RootTypeMapperInterface { - $compositeRootTypeMapper = new CompositeRootTypeMapper(); - $topRootTypeMapper = new NullableTypeMapperAdapter($compositeRootTypeMapper); + $topRootTypeMapper = new NullableTypeMapperAdapter(); - $compositeRootTypeMapper->addRootTypeMapper(new IteratorTypeMapper($topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper())); - $compositeRootTypeMapper->addRootTypeMapper(new CompoundTypeMapper($topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper())); - $compositeRootTypeMapper->addRootTypeMapper(new MyCLabsEnumTypeMapper()); - $compositeRootTypeMapper->addRootTypeMapper(new BaseTypeMapper($this->getTypeMapper(), $topRootTypeMapper)); + $errorRootTypeMapper = new FinalRootTypeMapper($this->getTypeMapper()); + $rootTypeMapper = new BaseTypeMapper($errorRootTypeMapper, $this->getTypeMapper(), $topRootTypeMapper); + $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper); + $rootTypeMapper = new CompoundTypeMapper($rootTypeMapper, $topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper()); + $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper()); + + $topRootTypeMapper->setNext($rootTypeMapper); return $topRootTypeMapper; } diff --git a/tests/FieldsBuilderTest.php b/tests/FieldsBuilderTest.php index 97428bdc89..74c1823d3c 100644 --- a/tests/FieldsBuilderTest.php +++ b/tests/FieldsBuilderTest.php @@ -297,8 +297,7 @@ public function getUser(): ?object new AuthorizationFieldMiddleware( $authenticationService, new VoidAuthorizationService() - ), - $this->getTypeRegistry() + ) ); $fields = $queryProvider->getFields(new TestType(), true); @@ -329,8 +328,7 @@ public function isAllowed(string $right, $subject = null): bool new AuthorizationFieldMiddleware( new VoidAuthenticationService(), $authorizationService - ), - $this->getTypeRegistry() + ) ); $fields = $queryProvider->getFields(new TestType(), true); @@ -391,8 +389,7 @@ public function testFromSourceFieldsInterface(): void new AuthorizationFieldMiddleware( new VoidAuthenticationService(), new VoidAuthorizationService() - ), - $this->getTypeRegistry() + ) ); $fields = $queryProvider->getFields(new TestTypeWithSourceFieldInterface(), true); $this->assertCount(1, $fields); diff --git a/tests/Integration/EndToEndTest.php b/tests/Integration/EndToEndTest.php index b710e27595..175870a9b4 100644 --- a/tests/Integration/EndToEndTest.php +++ b/tests/Integration/EndToEndTest.php @@ -30,6 +30,7 @@ use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompositeRootTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompoundTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\FinalRootTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\IteratorTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\MyCLabsEnumTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\NullableTypeMapperAdapter; @@ -96,8 +97,7 @@ public function createContainer(array $overloadedServices = []): ContainerInterf $container->get(NamingStrategyInterface::class), $container->get(RootTypeMapperInterface::class), $container->get(ParameterMiddlewareInterface::class), - $container->get(FieldMiddlewareInterface::class), - $container->get(TypeRegistry::class) + $container->get(FieldMiddlewareInterface::class) ); }, FieldMiddlewareInterface::class => function(ContainerInterface $container) { @@ -207,10 +207,15 @@ public function createContainer(array $overloadedServices = []): ContainerInterf return new CachedDocBlockFactory(new ArrayCache()); }, RootTypeMapperInterface::class => function(ContainerInterface $container) { - return new NullableTypeMapperAdapter($container->get(CompositeRootTypeMapper::class)); + return new NullableTypeMapperAdapter(); }, - CompositeRootTypeMapper::class => function(ContainerInterface $container) { - return new CompositeRootTypeMapper(); + 'rootTypeMapper' => function(ContainerInterface $container) { + $errorRootTypeMapper = new FinalRootTypeMapper($container->get(RecursiveTypeMapperInterface::class)); + $rootTypeMapper = new BaseTypeMapper($errorRootTypeMapper, $container->get(RecursiveTypeMapperInterface::class), $container->get(RootTypeMapperInterface::class)); + $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper); + $rootTypeMapper = new CompoundTypeMapper($rootTypeMapper, $container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class)); + $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class)); + return $rootTypeMapper; }, ContainerParameterHandler::class => function(ContainerInterface $container) { return new ContainerParameterHandler($container, true, true); @@ -236,13 +241,13 @@ public function createContainer(array $overloadedServices = []): ContainerInterf $container->get(TypeMapperInterface::class)->addTypeMapper($container->get(GlobTypeMapper::class.'2')); $container->get(TypeMapperInterface::class)->addTypeMapper($container->get(PorpaginasTypeMapper::class)); - $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new IteratorTypeMapper($container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class))); - $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new CompoundTypeMapper($container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class))); + $container->get(RootTypeMapperInterface::class)->setNext($container->get('rootTypeMapper')); + /*$container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new CompoundTypeMapper($container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class))); $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new IteratorTypeMapper($container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class))); $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new IteratorTypeMapper($container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class))); $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new MyCLabsEnumTypeMapper()); $container->get(CompositeRootTypeMapper::class)->addRootTypeMapper(new BaseTypeMapper($container->get(RecursiveTypeMapperInterface::class), $container->get(RootTypeMapperInterface::class))); - +*/ return $container; } diff --git a/tests/Mappers/CompositeTypeMapperTest.php b/tests/Mappers/CompositeTypeMapperTest.php index 1c80d82a6b..2ba57902e8 100644 --- a/tests/Mappers/CompositeTypeMapperTest.php +++ b/tests/Mappers/CompositeTypeMapperTest.php @@ -3,6 +3,7 @@ namespace TheCodingMachine\GraphQLite\Mappers; use GraphQL\Type\Definition\InputType; +use GraphQL\Type\Definition\NamedType; use GraphQL\Type\Definition\OutputType; use GraphQL\Type\Definition\Type; use PHPUnit\Framework\TestCase; @@ -79,7 +80,7 @@ public function getSupportedClasses(): array * Returns a GraphQL type by name (can be either an input or output type) * * @param string $typeName The name of the GraphQL type - * @return Type&((ResolvableMutableInputInterface&InputObjectType)|MutableObjectType) + * @return NamedType&Type&((ResolvableMutableInputInterface&InputObjectType)|MutableObjectType) */ public function mapNameToType(string $typeName): Type { diff --git a/tests/Mappers/Parameters/TypeMapperTest.php b/tests/Mappers/Parameters/TypeMapperTest.php index f25f90111b..5ef3959fa0 100644 --- a/tests/Mappers/Parameters/TypeMapperTest.php +++ b/tests/Mappers/Parameters/TypeMapperTest.php @@ -2,6 +2,7 @@ namespace TheCodingMachine\GraphQLite\Mappers\Parameters; +use DateTimeImmutable; use GraphQL\Type\Definition\ResolveInfo; use ReflectionMethod; use Symfony\Component\Cache\Simple\ArrayCache; @@ -19,7 +20,7 @@ class TypeMapperTest extends AbstractQueryProviderTest public function testMapScalarUnionException(): void { - $typeMapper = new TypeHandler($this->getTypeMapper(), $this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver(), $this->getTypeRegistry()); + $typeMapper = new TypeHandler($this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver()); $cachedDocBlockFactory = new CachedDocBlockFactory(new ArrayCache()); @@ -33,7 +34,7 @@ public function testMapScalarUnionException(): void public function testHideParameter(): void { - $typeMapper = new TypeHandler($this->getTypeMapper(), $this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver(), $this->getTypeRegistry()); + $typeMapper = new TypeHandler($this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver()); $cachedDocBlockFactory = new CachedDocBlockFactory(new ArrayCache()); @@ -52,7 +53,7 @@ public function testHideParameter(): void public function testHideParameterException(): void { - $typeMapper = new TypeHandler($this->getTypeMapper(), $this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver(), $this->getTypeRegistry()); + $typeMapper = new TypeHandler($this->getArgumentResolver(), $this->getRootTypeMapper(), $this->getTypeResolver()); $cachedDocBlockFactory = new CachedDocBlockFactory(new ArrayCache()); diff --git a/tests/Mappers/Root/BaseTypeMapperTest.php b/tests/Mappers/Root/BaseTypeMapperTest.php index 1ab1ff0e50..3745b56b5d 100644 --- a/tests/Mappers/Root/BaseTypeMapperTest.php +++ b/tests/Mappers/Root/BaseTypeMapperTest.php @@ -18,15 +18,16 @@ class BaseTypeMapperTest extends AbstractQueryProviderTest public function testNullableToGraphQLInputType(): void { - $baseTypeMapper = new BaseTypeMapper($this->getTypeMapper(), $this->getRootTypeMapper()); + $baseTypeMapper = new BaseTypeMapper(new FinalRootTypeMapper($this->getTypeMapper()), $this->getTypeMapper(), $this->getRootTypeMapper()); - $mappedType = $baseTypeMapper->toGraphQLInputType(new Nullable(new Object_(new Fqsen('\\Exception'))), null, 'foo', new ReflectionMethod(BaseTypeMapper::class, '__construct'), new DocBlock()); - $this->assertNull($mappedType); + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage("Don't know how to handle type ?\Exception"); + $baseTypeMapper->toGraphQLInputType(new Nullable(new Object_(new Fqsen('\\Exception'))), null, 'foo', new ReflectionMethod(BaseTypeMapper::class, '__construct'), new DocBlock()); } public function testToGraphQLOutputTypeException(): void { - $baseTypeMapper = new BaseTypeMapper($this->getTypeMapper(), $this->getRootTypeMapper()); + $baseTypeMapper = new BaseTypeMapper(new FinalRootTypeMapper($this->getTypeMapper()), $this->getTypeMapper(), $this->getRootTypeMapper()); $this->expectException(GraphQLRuntimeException::class); //$this->expectExceptionMessage('Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.'); @@ -34,15 +35,21 @@ public function testToGraphQLOutputTypeException(): void $baseTypeMapper->toGraphQLInputType(new Object_(new Fqsen('\\DateTime')), null, 'foo', new ReflectionMethod(BaseTypeMapper::class, '__construct'), new DocBlock()); } - public function testUnmappableArray(): void + public function testUnmappableOutputArray(): void { - $baseTypeMapper = new BaseTypeMapper($this->getTypeMapper(), $this->getRootTypeMapper()); + $baseTypeMapper = new BaseTypeMapper(new FinalRootTypeMapper($this->getTypeMapper()), $this->getTypeMapper(), $this->getRootTypeMapper()); + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage("Don't know how to handle type resource"); $mappedType = $baseTypeMapper->toGraphQLOutputType(new Array_(new Resource_()), null, new ReflectionMethod(BaseTypeMapper::class, '__construct'), new DocBlock()); - $this->assertNull($mappedType); + } + + public function testUnmappableInputArray(): void + { + $baseTypeMapper = new BaseTypeMapper(new FinalRootTypeMapper($this->getTypeMapper()), $this->getTypeMapper(), $this->getRootTypeMapper()); + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage("Don't know how to handle type resource"); $mappedType = $baseTypeMapper->toGraphQLInputType(new Array_(new Resource_()), null, 'foo', new ReflectionMethod(BaseTypeMapper::class, '__construct'), new DocBlock()); - $this->assertNull($mappedType); } - } diff --git a/tests/Mappers/Root/CompositeRootTypeMapperTest.php b/tests/Mappers/Root/CompositeRootTypeMapperTest.php deleted file mode 100644 index 64c9efe256..0000000000 --- a/tests/Mappers/Root/CompositeRootTypeMapperTest.php +++ /dev/null @@ -1,47 +0,0 @@ -getNullTypeMapper()]); - $this->assertNull($typeMapper->toGraphQLOutputType(new Integer(), null, new ReflectionMethod(CompositeRootTypeMapper::class, '__construct'), new DocBlock())); - } - - public function testToGraphQLOutputType(): void - { - $typeMapper = new CompositeRootTypeMapper([$this->getNullTypeMapper()]); - $this->assertNull($typeMapper->toGraphQLInputType(new Integer(), null, 'foo', new ReflectionMethod(CompositeRootTypeMapper::class, '__construct'), new DocBlock())); - } -} diff --git a/tests/Mappers/Root/MyCLabsEnumTypeMapperTest.php b/tests/Mappers/Root/MyCLabsEnumTypeMapperTest.php index 4fc7688646..3adf80cf67 100644 --- a/tests/Mappers/Root/MyCLabsEnumTypeMapperTest.php +++ b/tests/Mappers/Root/MyCLabsEnumTypeMapperTest.php @@ -6,14 +6,18 @@ use phpDocumentor\Reflection\Types\Object_; use PHPUnit\Framework\TestCase; use ReflectionMethod; +use TheCodingMachine\GraphQLite\AbstractQueryProviderTest; +use TheCodingMachine\GraphQLite\TypeMappingRuntimeException; -class MyCLabsEnumTypeMapperTest extends TestCase +class MyCLabsEnumTypeMapperTest extends AbstractQueryProviderTest { public function testObjectTypeHint(): void { - $mapper = new MyCLabsEnumTypeMapper(); + $mapper = new MyCLabsEnumTypeMapper(new FinalRootTypeMapper($this->getTypeMapper())); - $this->assertNull($mapper->toGraphQLOutputType(new Object_(), null, new ReflectionMethod(__CLASS__, 'testObjectTypeHint'), new DocBlock())); + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage("Don't know how to handle type object"); + $mapper->toGraphQLOutputType(new Object_(), null, new ReflectionMethod(__CLASS__, 'testObjectTypeHint'), new DocBlock()); } } diff --git a/tests/Mappers/Root/VoidRootTypeMapper.php b/tests/Mappers/Root/VoidRootTypeMapper.php new file mode 100644 index 0000000000..366426d3d6 --- /dev/null +++ b/tests/Mappers/Root/VoidRootTypeMapper.php @@ -0,0 +1,58 @@ +next = $next; + } + + /** + * @param (OutputType&GraphQLType)|null $subType + * + * @return OutputType&GraphQLType + */ + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): OutputType + { + return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); + } + + /** + * @param (InputType&GraphQLType)|null $subType + * + * @return InputType&GraphQLType + */ + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType + { + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); + } + + /** + * Returns a GraphQL type by name. + * If this root type mapper can return this type in "toGraphQLOutputType" or "toGraphQLInputType", it should + * also map these types by name in the "mapNameToType" method. + * + * @param string $typeName The name of the GraphQL type + */ + public function mapNameToType(string $typeName): NamedType + { + return $this->next->mapNameToType($typeName); + } +} diff --git a/tests/Mappers/Root/VoidRootTypeMapperFactory.php b/tests/Mappers/Root/VoidRootTypeMapperFactory.php new file mode 100644 index 0000000000..feba5c3b4a --- /dev/null +++ b/tests/Mappers/Root/VoidRootTypeMapperFactory.php @@ -0,0 +1,13 @@ +getAnnotationReader(), + $this->getTypeResolver(), + $namingStrategy, + $this->getTypeRegistry(), + $this->getTypeMapper(), + $container, + $arrayCache + ); + + $this->assertSame($this->getAnnotationReader(), $context->getAnnotationReader()); + $this->assertSame($this->getTypeResolver(), $context->getTypeResolver()); + $this->assertSame($namingStrategy, $context->getNamingStrategy()); + $this->assertSame($this->getTypeRegistry(), $context->getTypeRegistry()); + $this->assertSame($this->getTypeMapper(), $context->getRecursiveTypeMapper()); + $this->assertSame($container, $context->getContainer()); + $this->assertSame($arrayCache, $context->getCache()); + $this->assertSame(2, $context->getGlobTtl()); + $this->assertNull($context->getMapTtl()); + } +} diff --git a/tests/SchemaFactoryTest.php b/tests/SchemaFactoryTest.php index 3ee9d68486..369d1cad4d 100644 --- a/tests/SchemaFactoryTest.php +++ b/tests/SchemaFactoryTest.php @@ -19,6 +19,8 @@ use TheCodingMachine\GraphQLite\Mappers\Parameters\TypeHandler; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\Root\CompositeRootTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\VoidRootTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\VoidRootTypeMapperFactory; use TheCodingMachine\GraphQLite\Mappers\StaticClassListTypeMapperFactory; use TheCodingMachine\GraphQLite\Mappers\TypeMapperFactoryInterface; use TheCodingMachine\GraphQLite\Mappers\TypeMapperInterface; @@ -65,7 +67,7 @@ public function testSetters(): void ->setNamingStrategy(new NamingStrategy()) ->addTypeMapper(new CompositeTypeMapper()) ->addTypeMapperFactory(new StaticClassListTypeMapperFactory([TestSelfType::class])) - ->addRootTypeMapper(new CompositeRootTypeMapper()) + ->addRootTypeMapperFactory(new VoidRootTypeMapperFactory()) ->addParameterMiddleware(new ParameterMiddlewarePipe()) ->addQueryProviderFactory(new AggregateControllerQueryProviderFactory([], $container)) ->setSchemaConfig(new SchemaConfig()) From b119f846f8f026bdcf334ec4e9882f914e8e5089 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Thu, 21 Nov 2019 15:31:10 +0100 Subject: [PATCH 11/13] Refactoring and improving code coverage --- phpstan.neon | 3 - src/Mappers/CannotMapTypeException.php | 16 +++- src/Mappers/Root/CompoundTypeMapper.php | 80 ++++++------------- src/Mappers/Root/IteratorTypeMapper.php | 59 +++++++------- .../Root/NullableTypeMapperAdapter.php | 3 +- src/Schema.php | 9 +-- src/SchemaFactory.php | 2 +- src/TypeMappingRuntimeException.php | 59 +++++--------- tests/AbstractQueryProviderTest.php | 9 ++- tests/FieldsBuilderTest.php | 33 ++++---- .../TestControllerWithUnionInputParam.php | 20 +++++ tests/Integration/EndToEndTest.php | 2 +- tests/Mappers/Root/CompoundTypeMapperTest.php | 53 ++++++++++++ tests/Mappers/Root/IteratorTypeMapperTest.php | 53 ++++++++++++ .../Root/NullableTypeMapperAdapterTest.php | 40 +++++++--- 15 files changed, 270 insertions(+), 171 deletions(-) create mode 100644 tests/Fixtures/TestControllerWithUnionInputParam.php create mode 100644 tests/Mappers/Root/CompoundTypeMapperTest.php create mode 100644 tests/Mappers/Root/IteratorTypeMapperTest.php diff --git a/phpstan.neon b/phpstan.neon index 0ab67031ae..49a0305c14 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -18,9 +18,6 @@ parameters: - message: '#Parameter \#2 \$subType of method .* expects#' path: src/Mappers/Root/IteratorTypeMapper.php - - - message: '#Parameter \#2 \$subType of method .* expects#' - path: src/Mappers/Root/CompoundTypeMapper.php - '#Call to an undefined method GraphQL\\Error\\ClientAware::getMessage()#' #- # message: '#If condition is always true#' diff --git a/src/Mappers/CannotMapTypeException.php b/src/Mappers/CannotMapTypeException.php index 3ab94b40ef..0affee918a 100644 --- a/src/Mappers/CannotMapTypeException.php +++ b/src/Mappers/CannotMapTypeException.php @@ -14,6 +14,7 @@ use TheCodingMachine\GraphQLite\Annotations\ExtendType; use function array_map; use function implode; +use function sprintf; class CannotMapTypeException extends Exception implements CannotMapTypeExceptionInterface { @@ -39,6 +40,17 @@ public static function createForParseError(Error $error): self return new self($error->getMessage(), $error->getCode(), $error); } + public static function createForMissingIteratorValue(string $className, self $e): self + { + $message = sprintf( + '"%s" is iterable. Please provide a more specific type. For instance: %s|User[].', + $className, + $className + ); + + return new self($message, 0, $e); + } + /** * @param Type[] $unionTypes * @@ -53,9 +65,9 @@ public static function createForBadTypeInUnion(array $unionTypes): self return new self('in GraphQL, you can only use union types between objects. These types cannot be used in union types: ' . implode(', ', $disallowedTypeNames)); } - public static function createForUnionInInputType(Type $type): self + public static function createForBadTypeInUnionWithIterable(Type $type): self { - return new self('in GraphQL, you can only use union types in output types, not in input types. You cannot use this type: ' . $type); + return new self('the value must be iterable, but its computed GraphQL type (' . $type . ') is not a list.'); } public static function mustBeOutputType(string $subTypeName): self diff --git a/src/Mappers/Root/CompoundTypeMapper.php b/src/Mappers/Root/CompoundTypeMapper.php index e787aa4b31..36b89f8d00 100644 --- a/src/Mappers/Root/CompoundTypeMapper.php +++ b/src/Mappers/Root/CompoundTypeMapper.php @@ -4,7 +4,6 @@ namespace TheCodingMachine\GraphQLite\Mappers\Root; -use Closure; use GraphQL\Type\Definition\InputType; use GraphQL\Type\Definition\ListOfType; use GraphQL\Type\Definition\NamedType; @@ -62,39 +61,6 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); } - $result = $this->toGraphQLType($type, function (Type $type, ?OutputType $subType) use ($refMethod, $docBlockObj) { - return $this->topRootTypeMapper->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); - }, true); - - Assert::isInstanceOf($result, OutputType::class); - - return $result; - } - - /** - * @param (InputType&GraphQLType)|null $subType - * - * @return InputType&GraphQLType - */ - public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType - { - if (! $type instanceof Compound) { - return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); - } - - $result = $this->toGraphQLType($type, function (Type $type, ?InputType $subType) use ($refMethod, $docBlockObj, $argumentName) { - return $this->topRootTypeMapper->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); - }, false); - Assert::isInstanceOf($result, InputType::class); - - return $result; - } - - /** - * @return (OutputType&GraphQLType)|(InputType&GraphQLType) - */ - private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $isOutputType): GraphQLType - { $filteredDocBlockTypes = iterator_to_array($type); if (empty($filteredDocBlockTypes)) { throw TypeMappingRuntimeException::createFromType($type); @@ -108,28 +74,40 @@ private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $ $mustBeIterable = true; continue; } - $unionTypes[] = $topToGraphQLType($singleDocBlockType, null); + $unionTypes[] = $this->topRootTypeMapper->toGraphQLOutputType($singleDocBlockType, null, $refMethod, $docBlockObj); } if ($mustBeIterable && empty($unionTypes)) { throw TypeMappingRuntimeException::createFromType(new Iterable_()); } - /** @var OutputType&GraphQLType $return */ + /** @var OutputType&NonNull $return */ $return = $this->getTypeFromUnion($unionTypes); if ($mustBeIterable && ! $this->isWrappedListOfType($return)) { // The compound type is iterable and the other type is not iterable. Both types are incompatible // For instance: @return iterable|User - // FIXME: better error message! - throw TypeMappingRuntimeException::createFromType(new Iterable_()); + throw CannotMapTypeException::createForBadTypeInUnionWithIterable($return); } - if (! $isOutputType && ($return instanceof UnionType || ($return instanceof NonNull && $return->getWrappedType() instanceof UnionType))) { - throw CannotMapTypeException::createForUnionInInputType($return); + return $return; + } + + /** + * @param (InputType&GraphQLType)|null $subType + * + * @return InputType&GraphQLType + */ + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType + { + if (! $type instanceof Compound) { + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); } - return $return; + // At this point, the |null has been removed and the |iterable has been removed too. + // So there should only be compound input types, which is forbidden by the GraphQL spec. + // Let's kill this right away + throw TypeMappingRuntimeException::createFromType($type); } private function isWrappedListOfType(GraphQLType $type): bool @@ -157,19 +135,16 @@ private function getTypeFromUnion(array $unionTypes): GraphQLType // Remove null values $unionTypes = array_values(array_filter($unionTypes)); - $isNullable = false; - if (count($unionTypes) === 1) { $graphQlType = $unionTypes[0]; + Assert::isInstanceOf($graphQlType, NonNull::class); } else { $badTypes = []; $nonNullableUnionTypes = []; foreach ($unionTypes as $unionType) { - if (! $unionType instanceof NonNull) { - $isNullable = true; - } else { - $unionType = $unionType->getWrappedType(); - } + // We are sure that each $unionType is not nullable (because nullable types have been filtered in the NullableTypeMapperAdapter already) + Assert::isInstanceOf($unionType, NonNull::class); + $unionType = $unionType->getWrappedType(); if ($unionType instanceof ObjectType) { $nonNullableUnionTypes[] = $unionType; continue; @@ -178,21 +153,12 @@ private function getTypeFromUnion(array $unionTypes): GraphQLType $badTypes[] = $unionType; } if ($badTypes !== []) { - // TODO! - // TODO! - // TODO! - // TODO! - // We need a middleware to handle this case... throw CannotMapTypeException::createForBadTypeInUnion($unionTypes); } $graphQlType = new UnionType($nonNullableUnionTypes, $this->recursiveTypeMapper); /** @var UnionType $graphQlType */ $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); - - if (! $isNullable) { - $graphQlType = new NonNull($graphQlType); - } } return $graphQlType; diff --git a/src/Mappers/Root/IteratorTypeMapper.php b/src/Mappers/Root/IteratorTypeMapper.php index fe02906d92..3a14e5aae0 100644 --- a/src/Mappers/Root/IteratorTypeMapper.php +++ b/src/Mappers/Root/IteratorTypeMapper.php @@ -17,15 +17,12 @@ use phpDocumentor\Reflection\Type; use phpDocumentor\Reflection\Types\Array_; use phpDocumentor\Reflection\Types\Compound; -use phpDocumentor\Reflection\Types\Nullable; use phpDocumentor\Reflection\Types\Object_; use ReflectionClass; use ReflectionMethod; +use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeExceptionInterface; -use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; use TheCodingMachine\GraphQLite\TypeMappingRuntimeException; -use TheCodingMachine\GraphQLite\TypeRegistry; -use TheCodingMachine\GraphQLite\Types\UnionType; use Webmozart\Assert\Assert; use function count; use function iterator_to_array; @@ -38,18 +35,12 @@ class IteratorTypeMapper implements RootTypeMapperInterface { /** @var RootTypeMapperInterface */ private $topRootTypeMapper; - /** @var TypeRegistry */ - private $typeRegistry; - /** @var RecursiveTypeMapperInterface */ - private $recursiveTypeMapper; /** @var RootTypeMapperInterface */ private $next; - public function __construct(RootTypeMapperInterface $next, RootTypeMapperInterface $topRootTypeMapper, TypeRegistry $typeRegistry, RecursiveTypeMapperInterface $recursiveTypeMapper) + public function __construct(RootTypeMapperInterface $next, RootTypeMapperInterface $topRootTypeMapper) { $this->topRootTypeMapper = $topRootTypeMapper; - $this->typeRegistry = $typeRegistry; - $this->recursiveTypeMapper = $recursiveTypeMapper; $this->next = $next; } @@ -61,7 +52,19 @@ public function __construct(RootTypeMapperInterface $next, RootTypeMapperInterfa public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): OutputType { if (! $type instanceof Compound) { - return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); + try { + return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); + } catch (CannotMapTypeException $e) { + if ($type instanceof Object_) { + $fqcn = (string) $type->getFqsen(); + $refClass = new ReflectionClass($fqcn); + // Note : $refClass->isIterable() is only accessible in PHP 7.2 + if ($refClass->isIterable()) { + throw CannotMapTypeException::createForMissingIteratorValue($fqcn, $e); + } + } + throw $e; + } } $result = $this->toGraphQLType($type, function (Type $type, ?OutputType $subType) use ($refMethod, $docBlockObj) { @@ -84,7 +87,12 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType { if (! $type instanceof Compound) { - return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); + //try { + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); + + /*} catch (CannotMapTypeException $e) { + $this->throwIterableMissingTypeHintException($e, $type); + }*/ } $result = $this->toGraphQLType($type, function (Type $type, ?InputType $subType) use ($refMethod, $docBlockObj, $argumentName) { @@ -120,19 +128,7 @@ private function getTypeInArray(Type $typeHint): ?Type return null; } - return $this->dropNullableType($typeHint->getValueType()); - } - - /** - * Drops "Nullable" types and return the core type. - */ - private function dropNullableType(Type $typeHint): Type - { - if ($typeHint instanceof Nullable) { - return $typeHint->getActualType(); - } - - return $typeHint; + return $typeHint->getValueType(); } /** @@ -170,7 +166,7 @@ private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $ // We have several types. It is ok not to be able to match one. $lastException = $e; - if ($singleDocBlockType !== null) { + if ($singleDocBlockType !== null && $isOutputType) { // The type is an array (like User[]). Let's use that. $valueType = $topToGraphQLType($singleDocBlockType, null); if ($valueType !== null) { @@ -196,10 +192,11 @@ private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $ if (count($unionTypes) === 1) { $graphQlType = $unionTypes[0]; - } elseif ($isOutputType) { - $graphQlType = new UnionType($unionTypes, $this->recursiveTypeMapper); - $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); - Assert::isInstanceOf($graphQlType, OutputType::class); + /*} 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); + $graphQlType = $this->typeRegistry->getOrRegisterType($graphQlType); + Assert::isInstanceOf($graphQlType, OutputType::class);*/ } else { // There are no union input types. Something went wrong. $graphQlType = null; diff --git a/src/Mappers/Root/NullableTypeMapperAdapter.php b/src/Mappers/Root/NullableTypeMapperAdapter.php index e7bea33a08..0a5b251365 100644 --- a/src/Mappers/Root/NullableTypeMapperAdapter.php +++ b/src/Mappers/Root/NullableTypeMapperAdapter.php @@ -19,6 +19,7 @@ use TheCodingMachine\GraphQLite\Types\ResolvableMutableInputObjectType; use function array_filter; use function array_map; +use function array_values; use function count; use function iterator_to_array; @@ -134,7 +135,7 @@ private function getNonNullable(Type $type): ?Type if ($type instanceof Compound) { $types = array_map([$this, 'getNonNullable'], iterator_to_array($type)); // Remove null values - $types = array_filter($types); + $types = array_values(array_filter($types)); if (count($types) > 1) { return new Compound($types); } diff --git a/src/Schema.php b/src/Schema.php index 178fc540c8..e905feb890 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -78,7 +78,7 @@ public function __construct(QueryProviderInterface $queryProvider, RecursiveType return $recursiveTypeMapper->getOutputTypes(); }); - $config->setTypeLoader(static function (string $name) use ($recursiveTypeMapper, $query, $mutation, $rootTypeMapper) { + $config->setTypeLoader(static function (string $name) use ($query, $mutation, $rootTypeMapper) { // We need to find a type FROM a GraphQL type name if ($name === 'Query') { return $query; @@ -87,12 +87,7 @@ public function __construct(QueryProviderInterface $queryProvider, RecursiveType return $mutation; } - $type = $rootTypeMapper->mapNameToType($name); - if ($type !== null) { - return $type; - } - - return $recursiveTypeMapper->mapNameToType($name); + return $rootTypeMapper->mapNameToType($name); }); $typeResolver->registerSchema($this); diff --git a/src/SchemaFactory.php b/src/SchemaFactory.php index 0d1ae3e5cf..1726d5809b 100644 --- a/src/SchemaFactory.php +++ b/src/SchemaFactory.php @@ -353,7 +353,7 @@ public function createSchema(): Schema } $rootTypeMapper = new CompoundTypeMapper($rootTypeMapper, $topRootTypeMapper, $typeRegistry, $recursiveTypeMapper); - $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $topRootTypeMapper, $typeRegistry, $recursiveTypeMapper); + $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $topRootTypeMapper); $topRootTypeMapper->setNext($rootTypeMapper); diff --git a/src/TypeMappingRuntimeException.php b/src/TypeMappingRuntimeException.php index 3162160076..3f1f911e65 100644 --- a/src/TypeMappingRuntimeException.php +++ b/src/TypeMappingRuntimeException.php @@ -4,14 +4,12 @@ namespace TheCodingMachine\GraphQLite; -use Iterator; -use IteratorAggregate; use phpDocumentor\Reflection\Type; use phpDocumentor\Reflection\Types\Array_; +use phpDocumentor\Reflection\Types\Compound; use phpDocumentor\Reflection\Types\Iterable_; use phpDocumentor\Reflection\Types\Mixed_; use phpDocumentor\Reflection\Types\Object_; -use ReflectionClass; use ReflectionMethod; use ReflectionParameter; use Webmozart\Assert\Assert; @@ -55,34 +53,27 @@ public static function wrapWithParamInfo(TypeMappingRuntimeException $previous, $parameter->getName() ); } else { - if (! ($previous->type instanceof Object_)) { - throw new GraphQLRuntimeException("Unexpected type in TypeMappingException. Got '" . get_class($previous->type) . '"'); - } - - $fqcn = (string) $previous->type->getFqsen(); - - if ($fqcn === '\\DateTime') { + if ($previous->type instanceof Compound) { $message = sprintf( - 'Parameter $%s in %s::%s is type-hinted to "DateTime". Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.', + 'Parameter $%s in %s::%s is type-hinted to "' . $previous->type . '". Type-hinting a parameter to a union type is forbidden in GraphQL. Only return types can be union types.', $parameter->getName(), $declaringClass->getName(), $parameter->getDeclaringFunction()->getName() ); + } elseif (! ($previous->type instanceof Object_)) { + throw new GraphQLRuntimeException("Unexpected type in TypeMappingException. Got '" . get_class($previous->type) . '"'); } else { - $refClass = new ReflectionClass($fqcn); - // Note : $refClass->isIterable() is only accessible in PHP 7.2 - if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) { - throw new GraphQLRuntimeException("Unexpected type in TypeMappingException. Got a non iterable '" . $fqcn . '"'); + $fqcn = (string) $previous->type->getFqsen(); + + if ($fqcn !== '\\DateTime') { + throw new GraphQLRuntimeException("Unexpected type in TypeMappingException. Got a '" . $fqcn . '"'); } $message = sprintf( - 'Parameter $%s in %s::%s is type-hinted to "%s", which is iterable. Please provide an additional @param in the PHPDoc block to further specify the type. For instance: @param %s|User[] $%s.', + 'Parameter $%s in %s::%s is type-hinted to "DateTime". Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.', $parameter->getName(), $declaringClass->getName(), - $parameter->getDeclaringFunction()->getName(), - $fqcn, - $fqcn, - $parameter->getName() + $parameter->getDeclaringFunction()->getName() ); } } @@ -115,27 +106,15 @@ public static function wrapWithReturnInfo(TypeMappingRuntimeException $previous, } $fqcn = (string) $previous->type->getFqsen(); - if ($fqcn === '\\DateTime') { - $message = sprintf( - 'Return type in %s::%s is type-hinted to "DateTime". Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.', - $method->getDeclaringClass()->getName(), - $method->getName() - ); - } else { - $refClass = new ReflectionClass($fqcn); - // Note : $refClass->isIterable() is only accessible in PHP 7.2 - if (! $refClass->implementsInterface(Iterator::class) && ! $refClass->implementsInterface(IteratorAggregate::class)) { - throw new GraphQLRuntimeException("Unexpected type in TypeMappingException. Got a non iterable '" . $fqcn . '"'); - } - - $message = sprintf( - 'Return type in %s::%s is type-hinted to "%s", which is iterable. Please provide an additional @param in the PHPDoc block to further specify the type. For instance: @return %s|User[]', - $method->getDeclaringClass()->getName(), - $method->getName(), - $fqcn, - $fqcn - ); + if ($fqcn !== '\\DateTime') { + throw new GraphQLRuntimeException("Unexpected type in TypeMappingException. Got a '" . $fqcn . '"'); } + + $message = sprintf( + 'Return type in %s::%s is type-hinted to "DateTime". Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.', + $method->getDeclaringClass()->getName(), + $method->getName() + ); } $e = new self($message, 0, $previous); diff --git a/tests/AbstractQueryProviderTest.php b/tests/AbstractQueryProviderTest.php index 8ac5b07aa5..effd81c0a6 100644 --- a/tests/AbstractQueryProviderTest.php +++ b/tests/AbstractQueryProviderTest.php @@ -13,6 +13,7 @@ use GraphQL\Type\Definition\StringType; use GraphQL\Type\Definition\Type; use Mouf\Picotainer\Picotainer; +use phpDocumentor\Reflection\TypeResolver as PhpDocumentorTypeResolver; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; use Symfony\Component\Cache\Adapter\Psr16Adapter; @@ -319,7 +320,7 @@ protected function buildRootTypeMapper(): RootTypeMapperInterface $rootTypeMapper = new BaseTypeMapper($errorRootTypeMapper, $this->getTypeMapper(), $topRootTypeMapper); $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper); $rootTypeMapper = new CompoundTypeMapper($rootTypeMapper, $topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper()); - $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper()); + $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $topRootTypeMapper); $topRootTypeMapper->setNext($rootTypeMapper); return $topRootTypeMapper; @@ -375,4 +376,10 @@ protected function getTypeRegistry(): TypeRegistry } return $this->typeRegistry; } + + protected function resolveType(string $type): \phpDocumentor\Reflection\Type + { + $phpDocumentorTypeResolver = new PhpDocumentorTypeResolver(); + return $phpDocumentorTypeResolver->resolve($type); + } } diff --git a/tests/FieldsBuilderTest.php b/tests/FieldsBuilderTest.php index 74c1823d3c..5e22ad4d86 100644 --- a/tests/FieldsBuilderTest.php +++ b/tests/FieldsBuilderTest.php @@ -31,6 +31,7 @@ use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithNullableArray; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithParamIterator; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithReturnDateTime; +use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithUnionInputParam; use TheCodingMachine\GraphQLite\Fixtures\TestEnum; use TheCodingMachine\GraphQLite\Fixtures\TestTypeWithInvalidPrefetchMethod; use TheCodingMachine\GraphQLite\Fixtures\TestControllerWithInvalidReturnType; @@ -482,20 +483,16 @@ public function testQueryProviderWithInvalidReturnType(): void $queryProvider->getQueries($controller); } - // FIXME: enable this again when root type mappers are called in a chain (middleware style!) - // FIXME: enable this again when root type mappers are called in a chain (middleware style!) - // FIXME: enable this again when root type mappers are called in a chain (middleware style!) - // FIXME: enable this again when root type mappers are called in a chain (middleware style!) - /*public function testQueryProviderWithIterableReturnType(): void + public function testQueryProviderWithIterableReturnType(): void { $controller = new TestControllerWithIterableReturnType(); $queryProvider = $this->buildFieldsBuilder(); - $this->expectException(TypeMappingRuntimeException::class); - $this->expectExceptionMessage('Return type in TheCodingMachine\GraphQLite\Fixtures\TestControllerWithIterableReturnType::test is type-hinted to "\ArrayObject", which is iterable. Please provide an additional @param in the PHPDoc block to further specify the type. For instance: @return \ArrayObject|User[]'); + $this->expectException(CannotMapTypeException::class); + $this->expectExceptionMessage('For return type of TheCodingMachine\GraphQLite\Fixtures\TestControllerWithIterableReturnType::test, "\ArrayObject" is iterable. Please provide a more specific type. For instance: \ArrayObject|User[].'); $queryProvider->getQueries($controller); - }*/ + } public function testQueryProviderWithArrayReturnType(): void { @@ -519,18 +516,15 @@ public function testQueryProviderWithArrayParams(): void $queryProvider->getQueries($controller); } - // FIXME: enable this again when root type mappers are called in a chain (middleware style!) - // FIXME: enable this again when root type mappers are called in a chain (middleware style!) - // FIXME: enable this again when root type mappers are called in a chain (middleware style!) - // FIXME: enable this again when root type mappers are called in a chain (middleware style!) + // Test disabled because we cannot assume that by providing a more specific type, we will be able to handle the iterable. /*public function testQueryProviderWithIterableParams(): void { $controller = new TestControllerWithIterableParam(); $queryProvider = $this->buildFieldsBuilder(); - $this->expectException(TypeMappingRuntimeException::class); - $this->expectExceptionMessage('Parameter $params in TheCodingMachine\GraphQLite\Fixtures\TestControllerWithIterableParam::test is type-hinted to "\ArrayObject", which is iterable. Please provide an additional @param in the PHPDoc block to further specify the type. For instance: @param \ArrayObject|User[] $params.'); + $this->expectException(CannotMapTypeException::class); + $this->expectExceptionMessage('For parameter $params, in TheCodingMachine\GraphQLite\Fixtures\TestControllerWithIterableParam::test, "\ArrayObject" is iterable. Please provide a more specific type. For instance: \ArrayObject|User[].'); $queryProvider->getQueries($controller); }*/ @@ -741,4 +735,15 @@ public function testQueryProviderWithReturnDateTime(): void $this->expectExceptionMessage('Return type in TheCodingMachine\GraphQLite\Fixtures\TestControllerWithReturnDateTime::test is type-hinted to "DateTime". Type-hinting a parameter against DateTime is not allowed. Please use the DateTimeImmutable type instead.'); $queries = $queryProvider->getQueries($controller); } + + public function testQueryProviderWithUnionInputParam(): void + { + $controller = new TestControllerWithUnionInputParam(); + + $queryProvider = $this->buildFieldsBuilder(); + + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage('Parameter $testObject in TheCodingMachine\GraphQLite\Fixtures\TestControllerWithUnionInputParam::test is type-hinted to "\TheCodingMachine\GraphQLite\Fixtures\TestObject|\TheCodingMachine\GraphQLite\Fixtures\TestObject2". Type-hinting a parameter to a union type is forbidden in GraphQL. Only return types can be union types.'); + $queries = $queryProvider->getQueries($controller); + } } diff --git a/tests/Fixtures/TestControllerWithUnionInputParam.php b/tests/Fixtures/TestControllerWithUnionInputParam.php new file mode 100644 index 0000000000..cd0ebffbe8 --- /dev/null +++ b/tests/Fixtures/TestControllerWithUnionInputParam.php @@ -0,0 +1,20 @@ +get(RecursiveTypeMapperInterface::class), $container->get(RootTypeMapperInterface::class)); $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper); $rootTypeMapper = new CompoundTypeMapper($rootTypeMapper, $container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class)); - $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $container->get(RootTypeMapperInterface::class), $container->get(TypeRegistry::class), $container->get(RecursiveTypeMapperInterface::class)); + $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $container->get(RootTypeMapperInterface::class)); return $rootTypeMapper; }, ContainerParameterHandler::class => function(ContainerInterface $container) { diff --git a/tests/Mappers/Root/CompoundTypeMapperTest.php b/tests/Mappers/Root/CompoundTypeMapperTest.php new file mode 100644 index 0000000000..101335fe49 --- /dev/null +++ b/tests/Mappers/Root/CompoundTypeMapperTest.php @@ -0,0 +1,53 @@ +getTypeMapper()), + new FinalRootTypeMapper($this->getTypeMapper()), + $this->getTypeRegistry(), + $this->getTypeMapper() + ); + + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage("Don't know how to handle type"); + $compoundTypeMapper->toGraphQLOutputType(new Compound([]), null, new ReflectionMethod(__CLASS__, 'testException1'), new DocBlock()); + } + + public function testException2() + { + $compoundTypeMapper = new CompoundTypeMapper( + new FinalRootTypeMapper($this->getTypeMapper()), + new FinalRootTypeMapper($this->getTypeMapper()), + $this->getTypeRegistry(), + $this->getTypeMapper() + ); + + $this->expectException(TypeMappingRuntimeException::class); + $this->expectExceptionMessage("Don't know how to handle type iterable"); + $compoundTypeMapper->toGraphQLOutputType(new Compound([new Iterable_()]), null, new ReflectionMethod(__CLASS__, 'testException1'), new DocBlock()); + } + + 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.'); + $compoundTypeMapper->toGraphQLOutputType(new Compound([new Iterable_(), new String_()]), null, new ReflectionMethod(__CLASS__, 'testException1'), new DocBlock()); + } +} diff --git a/tests/Mappers/Root/IteratorTypeMapperTest.php b/tests/Mappers/Root/IteratorTypeMapperTest.php new file mode 100644 index 0000000000..062e18b732 --- /dev/null +++ b/tests/Mappers/Root/IteratorTypeMapperTest.php @@ -0,0 +1,53 @@ +getRootTypeMapper(); + + // A type like ArrayObject|int[] CAN be mapped to an output type, but NOT to an input type. + $this->expectException(CannotMapTypeException::class); + $this->expectExceptionMessage('cannot map class "ArrayObject" to a known GraphQL input type. Check your TypeMapper configuration.'); + $typeMapper->toGraphQLInputType($this->resolveType('ArrayObject|int[]'), null, 'foo', new ReflectionMethod(__CLASS__, 'testInputIterator'), new DocBlock()); + } + + public function testOutputNullableValueIterator(): void + { + $typeMapper = $this->getRootTypeMapper(); + + $result = $typeMapper->toGraphQLOutputType($this->resolveType('ArrayObject|array'), null, new ReflectionMethod(__CLASS__, 'testInputIterator'), new DocBlock()); + $this->assertInstanceOf(NonNull::class, $result); + $this->assertInstanceOf(ListOfType::class, $result->getWrappedType()); + $this->assertInstanceOf(IntType::class, $result->getWrappedType()->getWrappedType()); + } + + public function testMixIterableWithNonArrayType(): void + { + $typeMapper = $this->getRootTypeMapper(); + + $this->expectException(CannotMapTypeException::class); + $this->expectExceptionMessage('"\ArrayObject" is iterable. Please provide a more specific type. For instance: \ArrayObject|User[].'); + $result = $typeMapper->toGraphQLOutputType($this->resolveType('ArrayObject|'.TestObject::class), null, new ReflectionMethod(__CLASS__, 'testInputIterator'), new DocBlock()); + } + + public function testIterableWithTwoArrays(): void + { + $typeMapper = $this->getRootTypeMapper(); + + $this->expectException(CannotMapTypeException::class); + $this->expectExceptionMessage('"\ArrayObject" is iterable. Please provide a more specific type. For instance: \ArrayObject|User[].'); + $result = $typeMapper->toGraphQLOutputType($this->resolveType('ArrayObject|array|array'), null, new ReflectionMethod(__CLASS__, 'testInputIterator'), new DocBlock()); + } +} diff --git a/tests/Mappers/Root/NullableTypeMapperAdapterTest.php b/tests/Mappers/Root/NullableTypeMapperAdapterTest.php index 706aed24fe..f1dfa47cd7 100644 --- a/tests/Mappers/Root/NullableTypeMapperAdapterTest.php +++ b/tests/Mappers/Root/NullableTypeMapperAdapterTest.php @@ -2,36 +2,50 @@ namespace TheCodingMachine\GraphQLite\Mappers\Root; +use GraphQL\Type\Definition\NonNull; use phpDocumentor\Reflection\DocBlock; +use phpDocumentor\Reflection\Fqsen; +use phpDocumentor\Reflection\TypeResolver as PhpDocumentorTypeResolver; +use phpDocumentor\Reflection\Types\Boolean; +use phpDocumentor\Reflection\Types\Compound; +use phpDocumentor\Reflection\Types\Iterable_; use phpDocumentor\Reflection\Types\Null_; +use phpDocumentor\Reflection\Types\Object_; +use phpDocumentor\Reflection\Types\String_; use PHPUnit\Framework\TestCase; use ReflectionMethod; use TheCodingMachine\GraphQLite\AbstractQueryProviderTest; +use TheCodingMachine\GraphQLite\Fixtures\TestObject; +use TheCodingMachine\GraphQLite\Fixtures\TestObject2; +use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; use TheCodingMachine\GraphQLite\TypeMappingRuntimeException; +use phpDocumentor\Reflection\Type; class NullableTypeMapperAdapterTest extends AbstractQueryProviderTest { - public function testNullOnlyForOutputType(): void + public function testMultipleCompound(): void { - $null = new Null_(); + $compoundTypeMapper = $this->getRootTypeMapper(); - $nullableTypeMapperAdapter = $this->getRootTypeMapper(); - $method = new ReflectionMethod(__CLASS__, 'testNullOnlyForOutputType'); + $result = $compoundTypeMapper->toGraphQLOutputType($this->resolveType(TestObject::class.'|'.TestObject2::class.'|null'), null, new ReflectionMethod(__CLASS__, 'testMultipleCompound'), new DocBlock()); + $this->assertNotInstanceOf(NonNull::class, $result); + } + + public function testOnlyNull(): void + { + $compoundTypeMapper = $this->getRootTypeMapper(); $this->expectException(TypeMappingRuntimeException::class); - $this->expectExceptionMessage("Don't know how to handle type null"); - $nullableTypeMapperAdapter->toGraphQLOutputType($null, null, $method, new DocBlock()); + $this->expectExceptionMessage('Don\'t know how to handle type null'); + $compoundTypeMapper->toGraphQLOutputType($this->resolveType('null'), null, new ReflectionMethod(__CLASS__, 'testMultipleCompound'), new DocBlock()); } - public function testNullOnlyForInputType(): void + public function testOnlyNull2(): void { - $null = new Null_(); - - $nullableTypeMapperAdapter = $this->getRootTypeMapper(); - $method = new ReflectionMethod(__CLASS__, 'testNullOnlyForOutputType'); + $compoundTypeMapper = $this->getRootTypeMapper(); $this->expectException(TypeMappingRuntimeException::class); - $this->expectExceptionMessage("Don't know how to handle type null"); - $nullableTypeMapperAdapter->toGraphQLInputType($null, null, 'foo', $method, new DocBlock()); + $this->expectExceptionMessage('Don\'t know how to handle type null'); + $compoundTypeMapper->toGraphQLInputType($this->resolveType('null'), null, 'foo', new ReflectionMethod(__CLASS__, 'testMultipleCompound'), new DocBlock()); } } From 24b3df8c1e1a8bb9950b3f0b263323b6ea3ce828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Thu, 21 Nov 2019 21:54:18 +0100 Subject: [PATCH 12/13] Upgradding type-resolver to fix tests --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index d80e115472..8f94c78edb 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "thecodingmachine/class-explorer": "^1.0.2", "psr/simple-cache": "^1", "phpdocumentor/reflection-docblock": "^4.3", - "phpdocumentor/type-resolver": "^0.4 || ^1.0", + "phpdocumentor/type-resolver": "^1.0.1", "psr/http-message": "^1", "ecodev/graphql-upload": "^4.0", "webmozart/assert": "^1.4", From b9ffe9bcc1b2b3806629f575b308a282d23acc09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Mon, 25 Nov 2019 22:31:51 +0100 Subject: [PATCH 13/13] Fixing documentation, adding changelog and finishing migrating doc --- docs/CHANGELOG.md | 56 +++++++++++++++++++++++++ docs/custom_types.md | 47 ++++++++++++++++++--- docs/internals.md | 30 ++++++++++++- docs/migrating.md | 12 +++++- src/Mappers/Root/CompoundTypeMapper.php | 3 +- website/sidebars.json | 2 +- 6 files changed, 139 insertions(+), 11 deletions(-) create mode 100644 docs/CHANGELOG.md diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md new file mode 100644 index 0000000000..ba93624b5b --- /dev/null +++ b/docs/CHANGELOG.md @@ -0,0 +1,56 @@ +--- +id: changelog +title: Changelog +sidebar_label: Changelog +--- + +## 4.0-beta1 + +This is a complete refactoring from 3.x. While existing annotations are kept compatible, the internals have completely +changed. + +New features: + +- You can directly [annotate a PHP interface with `@Type` to make it a GraphQL interface](inheritance-interfaces.md#mapping-interfaces) +- You can autowire services in resolvers, thanks to the new `@Autowire` annotation +- Added [user input validation](validation.md) (using the Symfony Validator or the Laravel validator or a custom `@Assertion` annotation +- Improved security handling: + - Unauthorized access to fields can now generate GraphQL errors (rather that schema errors in GraphQLite v3) + - Added fine-grained security using the `@Security` annotation. A field can now be [marked accessible or not depending on the context](fine-grained-security.md). + For instance, you can restrict access to the field "viewsCount" of the type `BlogPost` only for post that the current user wrote. +- Performance: + - You can inject the [Webonyx query plan in a parameter from a resolver](query_plan.md) + - You can use the [dataloader pattern to improve performance drastically via the "prefetchMethod" attribute](prefetch_method.md) +- Customizable error handling has been added: + - You can throw [GraphQL errors](error_handling.md) with `TheCodingMachine\GraphQLite\Exceptions\GraphQLException` + - You can specify the HTTP response code to send with a given error, and the errors "extensions" section + - You can throw [many errors in one exception](error_handling.md#many-errors-for-one-exception) with `TheCodingMachine\GraphQLite\Exceptions\GraphQLAggregateException` +- You can map [a given PHP class to several PHP input types](input_types.md#declaring-several-input-types-for-the-same-php-class) (a PHP class can have several `@Factory` annotations) +- You can force input types using `@UseInputType(for="$id", inputType="ID!")` +- You can extend an input types (just like you could extend an output type in v3) using [the new `@Decorate` annotation](extend_input_type.md) +- In a factory, you can [exclude some optional parameters from the GraphQL schema](input_types#ignoring-some-parameters) + + +Many extension points have been added + +- Added a "root type mapper" (useful to map scalar types to PHP types or to add custom annotations related to resolvers) +- Added ["field middlewares"](field_middlewares.md) (useful to add middleware that modify the way GraphQL fields are handled) +- Added a ["parameter type mapper"](argument_resolving.md) (useful to add customize parameter resolution or add custom annotations related to parameters) + +New framework specific features: + +Symfony: + +- The Symfony bundle now provides a "login" and a "logout" mutation (and also a "me" query) + +Laravel: + +- [Native integration with the Laravel paginator](laravel-package-advanced.md#support-for-pagination) has been added + +Internals: + +- The `FieldsBuilder` class has been split in many different services (`FieldsBuilder`, `TypeHandler`, and a + chain of *root type mappers*) +- The `FieldsBuilderFactory` class has been completely removed. +- Overall, there is not much in common internally between 4.x and 3.x. 4.x is much more flexible with many more hook points + than 3.x. Try it out! diff --git a/docs/custom_types.md b/docs/custom_types.md index 5a3de1072d..0271e0705e 100644 --- a/docs/custom_types.md +++ b/docs/custom_types.md @@ -121,15 +121,16 @@ In order to add a scalar type in GraphQLite, you need to: - create a [Webonyx custom scalar type](https://webonyx.github.io/graphql-php/type-system/scalar-types/#writing-custom-scalar-types). You do this by creating a class that extends `GraphQL\Type\Definition\ScalarType`. - create a "type mapper" that will map PHP types to the GraphQL scalar type. You do this by writing a class implementing the `RootTypeMapperInterface`. +- create a "type mapper factory" that will be in charge of creating your "type mapper". ```php interface RootTypeMapperInterface { - public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType; + public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): OutputType; - public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType; + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): InputType; - public function mapNameToType(string $typeName): ?NamedType; + public function mapNameToType(string $typeName): NamedType; } ``` @@ -138,11 +139,21 @@ to your GraphQL scalar type. Return your scalar type if there is a match or `nul The `mapNameToType` should return your GraphQL scalar type if `$typeName` is the name of your scalar type. +RootTypeMapper are organized **in a chain** (they are actually middlewares). +Each instance of a `RootTypeMapper` holds a reference on the next root type mapper to be called in the chain. + For instance: ```php class AnyScalarTypeMapper implements RootTypeMapperInterface { + /** @var RootTypeMapperInterface */ + private $next; + + public function __construct(RootTypeMapperInterface $next) + { + $this->next = $next; + } public function toGraphQLOutputType(Type $type, ?OutputType $subType, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?OutputType { @@ -150,7 +161,8 @@ class AnyScalarTypeMapper implements RootTypeMapperInterface // AnyScalarType is a class implementing the Webonyx ScalarType type. return AnyScalarType::getInstance(); } - return null; + // If the PHPDoc type is not "Scalar", let's pass the control to the next type mapper in the chain + return $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj); } public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, ReflectionMethod $refMethod, DocBlock $docBlockObj): ?InputType @@ -159,7 +171,8 @@ class AnyScalarTypeMapper implements RootTypeMapperInterface // AnyScalarType is a class implementing the Webonyx ScalarType type. return AnyScalarType::getInstance(); } - return null; + // If the PHPDoc type is not "Scalar", let's pass the control to the next type mapper in the chain + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj); } /** @@ -179,3 +192,27 @@ class AnyScalarTypeMapper implements RootTypeMapperInterface } } ``` + +Now, in order to create an instance of your `AnyScalarTypeMapper` class, you need an instance of the `$next` type mapper in the chain. +How do you get the `$next` type mapper? Through a factory: + +```php +class AnyScalarTypeMapperFactory implements RootTypeMapperFactoryInterface +{ + public function create(RootTypeMapperInterface $next, RootTypeMapperFactoryContext $context): RootTypeMapperInterface + { + return new AnyScalarTypeMapper($next); + } +} +``` + +Now, you need to register this factory in your application, and we are done. + +You can register your own root mapper factories using the `SchemaFactory::addRootTypeMapperFactory()` method. + +```php +$schemaFactory->addRootTypeMapperFactory(new AnyScalarTypeMapperFactory()); +``` + +If you are using the Symfony bundle, the factory will be automatically registered, you have nothing to do (the service +is automatically tagged with the "graphql.root_type_mapper_factory" tag). diff --git a/docs/internals.md b/docs/internals.md index 14f5a29180..aa6c084488 100644 --- a/docs/internals.md +++ b/docs/internals.md @@ -32,8 +32,12 @@ mermaid.initialize({ graph TD classDef custom fill:#cfc,stroke:#7a7,stroke-width:2px,stroke-dasharray: 5, 5; subgraph RootTypeMapperInterface + NullableTypeMapperAdapter-->CompoundTypeMapper + CompoundTypeMapper-->IteratorTypeMapper + IteratorTypeMapper-->YourCustomRootTypeMapper YourCustomRootTypeMapper-->MyCLabsEnumTypeMapper MyCLabsEnumTypeMapper-->BaseTypeMapper + BaseTypeMapper-->FinalRootTypeMapper end subgraph RecursiveTypeMapperInterface BaseTypeMapper-->RecursiveTypeMapper @@ -56,17 +60,39 @@ These type mappers are the first type mappers called. They are responsible for: - mapping scalar types (for instance mapping the "int" PHP type to GraphQL Integer type) + - detecting nullable/non-nullable types (for instance interpreting "?int" or "int|null") - mapping list types (mapping a PHP array to a GraphQL list) + - mapping union types - mapping enums Root type mappers have access to the *context* of a type: they can access the PHP DocBlock and read annotations. If you want to write a custom type mapper that needs access to annotations, it needs to be a "root type mapper". -GraphQLite provide 3 default implementations: +GraphQLite provides 6 classes implementing `RootTypeMapperInterface`: - - `CompositeRootTypeMapper`: a type mapper that delegates mapping to other type mappers using the Composite Design Pattern. + - `NullableTypeMapperAdapter`: a type mapper in charge of making GraphQL types non-nullable if the PHP type is non-nullable + - `CompoundTypeMapper`: a type mapper in charge of union types + - `IteratorTypeMapper`: a type mapper in charge of iterable types (for instance: `MyIterator|User[]`) - `MyCLabsEnumTypeMapper`: maps MyCLabs/enum types to GraphQL enum types - `BaseTypeMapper`: maps scalar types and lists. Passes the control to the "recursive type mappers" if an object is encountered. + - `FinalRootTypeMapper`: the last type mapper of the chain, used to throw error if no other type mapper managed to handle the type. + +Type mappers are organized in a chain; each type-mapper is responsible for calling the next type mapper. + +
+ graph TD + classDef custom fill:#cfc,stroke:#7a7,stroke-width:2px,stroke-dasharray: 5, 5; + subgraph RootTypeMapperInterface + NullableTypeMapperAdapter-->CompoundTypeMapper + CompoundTypeMapper-->IteratorTypeMapper + IteratorTypeMapper-->YourCustomRootTypeMapper + YourCustomRootTypeMapper-->MyCLabsEnumTypeMapper + MyCLabsEnumTypeMapper-->BaseTypeMapper + BaseTypeMapper-->FinalRootTypeMapper + end + class YourCustomRootTypeMapper custom; +
+ ## Class type mappers diff --git a/docs/migrating.md b/docs/migrating.md index a37525e5af..16215b2db6 100644 --- a/docs/migrating.md +++ b/docs/migrating.md @@ -11,10 +11,13 @@ If you are a "regular" GraphQLite user, migration to v4 should be straightforwar - Annotations are mostly untouched. The only annotation that is changed is the `@SourceField` annotation. - Check your code for every places where you use the `@SourceField` annotation: - The "id" attribute has been remove (`@SourceField(id=true)`). Instead, use `@SourceField(outputType="ID")` - - The "logged", "right" and "failWith" attributes have been remove (`@SourceField(logged=true)`). + - The "logged", "right" and "failWith" attributes have been removed (`@SourceField(logged=true)`). Instead, use the annotations attribute with the same annotations you use for the `@Field` annotation: `@SourceField(annotations={@Logged, @FailWith(null)})` -- TODO: change in visibility, new @HideIfUnauthorized +- In GraphQLite v3, the default was to hide a field from the schema if a user has no access to it. + In GraphQLite v4, the default is to still show this field, but to throw an error if the user makes a query on it + (this way, the schema is the same for all users). If you want the old mode, use the new + [`@HideIfUnauthorized` annotation](annotations_reference.md#hideifunauthorized-annotation) - If you are using the Symfony bundle, the Laravel package or the Universal module, you must also upgrade those to 4.0. These package will take care of the wiring for you. Apart for upgrading the packages, you have nothing to do. - If you are relying on the `SchemaFactory` to bootstrap GraphQLite, you have nothing to do. @@ -29,3 +32,8 @@ On the other hand, if you are a power user and if you are wiring GraphQLite serv a look at the `SchemaFactory` class for an example of proper configuration. - The `HydratorInterface` and all implementations are gone. When returning an input object from a TypeMapper, the object must now implement the `ResolvableMutableInputInterface` (an input object type that contains its own resolver) + +Note: we strongly recommend to use the Symfony bundle, the Laravel package, the Universal module or the SchemaManager +to bootstrap GraphQLite. Wiring directly GraphQLite classes (like the `FieldsBuilder`) into your container is not recommended, +as the signature of the constructor of those classes may vary from one minor release to another. +Use the `SchemaManager` instead. diff --git a/src/Mappers/Root/CompoundTypeMapper.php b/src/Mappers/Root/CompoundTypeMapper.php index 36b89f8d00..5e6e3f1503 100644 --- a/src/Mappers/Root/CompoundTypeMapper.php +++ b/src/Mappers/Root/CompoundTypeMapper.php @@ -81,7 +81,6 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, Reflection throw TypeMappingRuntimeException::createFromType(new Iterable_()); } - /** @var OutputType&NonNull $return */ $return = $this->getTypeFromUnion($unionTypes); if ($mustBeIterable && ! $this->isWrappedListOfType($return)) { @@ -128,6 +127,8 @@ private function isWrappedListOfType(GraphQLType $type): bool /** * @param array<(InputType&GraphQLType)|(OutputType&GraphQLType)> $unionTypes * + * @return OutputType&GraphQLType + * * @throws CannotMapTypeException */ private function getTypeFromUnion(array $unionTypes): GraphQLType diff --git a/website/sidebars.json b/website/sidebars.json index 750dab5086..3c2f2bf44a 100755 --- a/website/sidebars.json +++ b/website/sidebars.json @@ -6,6 +6,6 @@ "Security": ["authentication_authorization", "fine-grained-security", "implementing-security"], "Performance": ["query-plan", "prefetch-method"], "Advanced": ["file-uploads", "pagination", "custom-types", "field-middlewares", "argument-resolving", "extend_input_type", "multiple_output_types", "symfony-bundle-advanced", "laravel-package-advanced", "internals", "troubleshooting", "migrating"], - "Reference": ["annotations_reference"] + "Reference": ["annotations_reference", "changelog"] } }