From f95ef49a9514a62dc93bf561c495e9e2d2bc661b Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Sun, 24 Apr 2022 13:38:43 +0300 Subject: [PATCH 01/10] Draft implementation for generic iterables https://github.com/thecodingmachine/graphqlite/issues/436 --- src/Mappers/Root/IteratorTypeMapper.php | 46 ++++++++++++++++++- tests/Mappers/Root/IteratorTypeMapperTest.php | 10 ++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/Mappers/Root/IteratorTypeMapper.php b/src/Mappers/Root/IteratorTypeMapper.php index 9fde3004bb..f34965dec3 100644 --- a/src/Mappers/Root/IteratorTypeMapper.php +++ b/src/Mappers/Root/IteratorTypeMapper.php @@ -16,6 +16,7 @@ use phpDocumentor\Reflection\DocBlock; use phpDocumentor\Reflection\Type; use phpDocumentor\Reflection\Types\Array_; +use phpDocumentor\Reflection\Types\Collection; use phpDocumentor\Reflection\Types\Compound; use phpDocumentor\Reflection\Types\Object_; use ReflectionClass; @@ -54,7 +55,7 @@ public function __construct(RootTypeMapperInterface $next, RootTypeMapperInterfa */ public function toGraphQLOutputType(Type $type, ?OutputType $subType, $reflector, DocBlock $docBlockObj): OutputType { - if (! $type instanceof Compound) { + if (! $type instanceof Compound && ! $type instanceof Collection) { try { return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); } catch (CannotMapTypeException $e) { @@ -73,6 +74,20 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, $reflector } } + if ($type instanceof Collection) { + $result = $this->toGraphQLTypeFromCollection($type, function (Type $type, ?OutputType $subType) use ($reflector, $docBlockObj) { + return $this->topRootTypeMapper->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); + }, true); + + if ($result === null) { + return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); + } + Assert::isInstanceOf($result, OutputType::class); + Assert::notInstanceOf($result, NonNull::class); + + return $result; + } + $result = $this->toGraphQLType($type, function (Type $type, ?OutputType $subType) use ($reflector, $docBlockObj) { return $this->topRootTypeMapper->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); }, true); @@ -222,6 +237,35 @@ private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $ return $graphQlType; } + /** + * @param Collection $type + * + * @return (OutputType&GraphQLType)|(InputType&GraphQLType)|null + */ + private function toGraphQLTypeFromCollection(Collection $type, Closure $topToGraphQLType, bool $isOutputType) + { + $singleDocBlockType = $type->getValueType(); + + $subGraphQlType = $valueType = $topToGraphQLType($singleDocBlockType, null); + if ($valueType !== null && $isOutputType) { + $subGraphQlType = new ListOfType($valueType); + } + //$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(); + assert($subGraphQlType instanceof OutputType && $subGraphQlType instanceof GraphQLType); + } + + $graphQlType = $topToGraphQLType($singleDocBlockType, $subGraphQlType); + if ($graphQlType instanceof NonNull) { + $graphQlType = $graphQlType->getWrappedType(); + } + + return new ListOfType($graphQlType); + } + /** * Removes the iterator type from $types * diff --git a/tests/Mappers/Root/IteratorTypeMapperTest.php b/tests/Mappers/Root/IteratorTypeMapperTest.php index 062e18b732..f56e2e719f 100644 --- a/tests/Mappers/Root/IteratorTypeMapperTest.php +++ b/tests/Mappers/Root/IteratorTypeMapperTest.php @@ -33,6 +33,16 @@ public function testOutputNullableValueIterator(): void $this->assertInstanceOf(IntType::class, $result->getWrappedType()->getWrappedType()); } + public function testOutputGenericNullableValueIterator(): void + { + $typeMapper = $this->getRootTypeMapper(); + + $result = $typeMapper->toGraphQLOutputType($this->resolveType('ArrayObject'), 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(); From 4ed4f73d9216a8a28cf9c2e811e82deab3e6caab Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Sun, 24 Apr 2022 23:04:05 +0300 Subject: [PATCH 02/10] Draft v2 implementation for generic iterables (separated mapper) https://github.com/thecodingmachine/graphqlite/issues/436 --- src/Mappers/Root/AbstractListTypeMapper.php | 97 +++++++++++++++++++ src/Mappers/Root/IteratorTypeMapper.php | 48 +-------- tests/AbstractQueryProviderTest.php | 2 + .../Root/AbstractListTypeMapperTest.php | 45 +++++++++ tests/Mappers/Root/IteratorTypeMapperTest.php | 10 -- 5 files changed, 146 insertions(+), 56 deletions(-) create mode 100644 src/Mappers/Root/AbstractListTypeMapper.php create mode 100644 tests/Mappers/Root/AbstractListTypeMapperTest.php diff --git a/src/Mappers/Root/AbstractListTypeMapper.php b/src/Mappers/Root/AbstractListTypeMapper.php new file mode 100644 index 0000000000..f6c17179ee --- /dev/null +++ b/src/Mappers/Root/AbstractListTypeMapper.php @@ -0,0 +1,97 @@ +topRootTypeMapper = $topRootTypeMapper; + $this->next = $next; + } + + /** + * @param (OutputType&GraphQLType)|null $subType + * @param ReflectionMethod|ReflectionProperty $reflector + * + * @return OutputType&GraphQLType + */ + public function toGraphQLOutputType(Type $type, ?OutputType $subType, $reflector, DocBlock $docBlockObj): OutputType + { + if (! $type instanceof AbstractList) { + return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); + } + + $result = $this->toGraphQLType($type, + function (Type $type, ?OutputType $subType) use ($reflector, $docBlockObj) { + return $this->topRootTypeMapper->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); + }); + + if ($result === null) { + return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); + } + Assert::isInstanceOf($result, OutputType::class); + + return $result; + } + + /** + * @param (InputType&GraphQLType)|null $subType + * @param ReflectionMethod|ReflectionProperty $reflector + * + * @return InputType&GraphQLType + */ + public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, $reflector, DocBlock $docBlockObj): InputType + { + // ToDo: what (and how) should we do here about collections? + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $reflector, $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); + } + + /** + * @param AbstractList $type + * + * @return (OutputType&GraphQLType)|(InputType&GraphQLType) + */ + private function toGraphQLType(AbstractList $type, Closure $topToGraphQLType) + { + $singleDocBlockType = $type->getValueType(); + + $subGraphQlType = $topToGraphQLType($singleDocBlockType, null); + $graphQlType = $topToGraphQLType($singleDocBlockType, $subGraphQlType); + + return new ListOfType($graphQlType); + } +} diff --git a/src/Mappers/Root/IteratorTypeMapper.php b/src/Mappers/Root/IteratorTypeMapper.php index f34965dec3..dd9e4d6a13 100644 --- a/src/Mappers/Root/IteratorTypeMapper.php +++ b/src/Mappers/Root/IteratorTypeMapper.php @@ -16,7 +16,6 @@ use phpDocumentor\Reflection\DocBlock; use phpDocumentor\Reflection\Type; use phpDocumentor\Reflection\Types\Array_; -use phpDocumentor\Reflection\Types\Collection; use phpDocumentor\Reflection\Types\Compound; use phpDocumentor\Reflection\Types\Object_; use ReflectionClass; @@ -55,7 +54,7 @@ public function __construct(RootTypeMapperInterface $next, RootTypeMapperInterfa */ public function toGraphQLOutputType(Type $type, ?OutputType $subType, $reflector, DocBlock $docBlockObj): OutputType { - if (! $type instanceof Compound && ! $type instanceof Collection) { + if (! $type instanceof Compound) { try { return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); } catch (CannotMapTypeException $e) { @@ -74,20 +73,6 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, $reflector } } - if ($type instanceof Collection) { - $result = $this->toGraphQLTypeFromCollection($type, function (Type $type, ?OutputType $subType) use ($reflector, $docBlockObj) { - return $this->topRootTypeMapper->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); - }, true); - - if ($result === null) { - return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); - } - Assert::isInstanceOf($result, OutputType::class); - Assert::notInstanceOf($result, NonNull::class); - - return $result; - } - $result = $this->toGraphQLType($type, function (Type $type, ?OutputType $subType) use ($reflector, $docBlockObj) { return $this->topRootTypeMapper->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); }, true); @@ -111,7 +96,7 @@ public function toGraphQLInputType(Type $type, ?InputType $subType, string $argu { if (! $type instanceof Compound) { //try { - return $this->next->toGraphQLInputType($type, $subType, $argumentName, $reflector, $docBlockObj); + return $this->next->toGraphQLInputType($type, $subType, $argumentName, $reflector, $docBlockObj); /*} catch (CannotMapTypeException $e) { $this->throwIterableMissingTypeHintException($e, $type); @@ -237,35 +222,6 @@ private function toGraphQLType(Compound $type, Closure $topToGraphQLType, bool $ return $graphQlType; } - /** - * @param Collection $type - * - * @return (OutputType&GraphQLType)|(InputType&GraphQLType)|null - */ - private function toGraphQLTypeFromCollection(Collection $type, Closure $topToGraphQLType, bool $isOutputType) - { - $singleDocBlockType = $type->getValueType(); - - $subGraphQlType = $valueType = $topToGraphQLType($singleDocBlockType, null); - if ($valueType !== null && $isOutputType) { - $subGraphQlType = new ListOfType($valueType); - } - //$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(); - assert($subGraphQlType instanceof OutputType && $subGraphQlType instanceof GraphQLType); - } - - $graphQlType = $topToGraphQLType($singleDocBlockType, $subGraphQlType); - if ($graphQlType instanceof NonNull) { - $graphQlType = $graphQlType->getWrappedType(); - } - - return new ListOfType($graphQlType); - } - /** * Removes the iterator type from $types * diff --git a/tests/AbstractQueryProviderTest.php b/tests/AbstractQueryProviderTest.php index 9a1b2ffd69..97f558e99c 100644 --- a/tests/AbstractQueryProviderTest.php +++ b/tests/AbstractQueryProviderTest.php @@ -33,6 +33,7 @@ use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapper; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\Root\AbstractListTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompositeRootTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompoundTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\EnumTypeMapper; @@ -340,6 +341,7 @@ protected function buildRootTypeMapper(): RootTypeMapperInterface if (interface_exists(UnitEnum::class)) { $rootTypeMapper = new EnumTypeMapper($rootTypeMapper, $this->getAnnotationReader(), $arrayAdapter, []); } + $rootTypeMapper = new AbstractListTypeMapper($rootTypeMapper, $topRootTypeMapper); $rootTypeMapper = new CompoundTypeMapper($rootTypeMapper, $topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper()); $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $topRootTypeMapper); diff --git a/tests/Mappers/Root/AbstractListTypeMapperTest.php b/tests/Mappers/Root/AbstractListTypeMapperTest.php new file mode 100644 index 0000000000..632e2a5831 --- /dev/null +++ b/tests/Mappers/Root/AbstractListTypeMapperTest.php @@ -0,0 +1,45 @@ +getRootTypeMapper(); + + $result = $typeMapper->toGraphQLOutputType($this->resolveType('ArrayObject'), null, new ReflectionMethod(__CLASS__, 'testOutputGenericNullableValueIterator'), new DocBlock()); + $this->assertInstanceOf(NonNull::class, $result); + $this->assertInstanceOf(ListOfType::class, $result->getWrappedType()); + $this->assertInstanceOf(IntType::class, $result->getWrappedType()->getWrappedType()); + } + + public function testIterableWithTwoArrays(): void + { + $typeMapper = $this->getRootTypeMapper(); + + $result = $typeMapper->toGraphQLOutputType($this->resolveType('array'), null, new ReflectionMethod(__CLASS__, 'testInputIterator'), new DocBlock()); + + $this->assertEquals(1,1); + } + + public function testInputIterator(): void + { + $typeMapper = $this->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()); + } + +} diff --git a/tests/Mappers/Root/IteratorTypeMapperTest.php b/tests/Mappers/Root/IteratorTypeMapperTest.php index f56e2e719f..062e18b732 100644 --- a/tests/Mappers/Root/IteratorTypeMapperTest.php +++ b/tests/Mappers/Root/IteratorTypeMapperTest.php @@ -33,16 +33,6 @@ public function testOutputNullableValueIterator(): void $this->assertInstanceOf(IntType::class, $result->getWrappedType()->getWrappedType()); } - public function testOutputGenericNullableValueIterator(): void - { - $typeMapper = $this->getRootTypeMapper(); - - $result = $typeMapper->toGraphQLOutputType($this->resolveType('ArrayObject'), 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(); From 6c2bfe88e4559da652a9e65673d5518be526d92c Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Sun, 24 Apr 2022 23:06:43 +0300 Subject: [PATCH 03/10] Draft v3 implementation for generic iterables (base type mapper) https://github.com/thecodingmachine/graphqlite/issues/436 --- src/Mappers/Root/AbstractListTypeMapper.php | 97 ------------------- src/Mappers/Root/BaseTypeMapper.php | 3 +- tests/AbstractQueryProviderTest.php | 2 - .../Root/AbstractListTypeMapperTest.php | 45 --------- tests/Mappers/Root/BaseTypeMapperTest.php | 13 ++- 5 files changed, 14 insertions(+), 146 deletions(-) delete mode 100644 src/Mappers/Root/AbstractListTypeMapper.php delete mode 100644 tests/Mappers/Root/AbstractListTypeMapperTest.php diff --git a/src/Mappers/Root/AbstractListTypeMapper.php b/src/Mappers/Root/AbstractListTypeMapper.php deleted file mode 100644 index f6c17179ee..0000000000 --- a/src/Mappers/Root/AbstractListTypeMapper.php +++ /dev/null @@ -1,97 +0,0 @@ -topRootTypeMapper = $topRootTypeMapper; - $this->next = $next; - } - - /** - * @param (OutputType&GraphQLType)|null $subType - * @param ReflectionMethod|ReflectionProperty $reflector - * - * @return OutputType&GraphQLType - */ - public function toGraphQLOutputType(Type $type, ?OutputType $subType, $reflector, DocBlock $docBlockObj): OutputType - { - if (! $type instanceof AbstractList) { - return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); - } - - $result = $this->toGraphQLType($type, - function (Type $type, ?OutputType $subType) use ($reflector, $docBlockObj) { - return $this->topRootTypeMapper->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); - }); - - if ($result === null) { - return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); - } - Assert::isInstanceOf($result, OutputType::class); - - return $result; - } - - /** - * @param (InputType&GraphQLType)|null $subType - * @param ReflectionMethod|ReflectionProperty $reflector - * - * @return InputType&GraphQLType - */ - public function toGraphQLInputType(Type $type, ?InputType $subType, string $argumentName, $reflector, DocBlock $docBlockObj): InputType - { - // ToDo: what (and how) should we do here about collections? - return $this->next->toGraphQLInputType($type, $subType, $argumentName, $reflector, $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); - } - - /** - * @param AbstractList $type - * - * @return (OutputType&GraphQLType)|(InputType&GraphQLType) - */ - private function toGraphQLType(AbstractList $type, Closure $topToGraphQLType) - { - $singleDocBlockType = $type->getValueType(); - - $subGraphQlType = $topToGraphQLType($singleDocBlockType, null); - $graphQlType = $topToGraphQLType($singleDocBlockType, $subGraphQlType); - - return new ListOfType($graphQlType); - } -} diff --git a/src/Mappers/Root/BaseTypeMapper.php b/src/Mappers/Root/BaseTypeMapper.php index 29b090f463..45bcd2844a 100644 --- a/src/Mappers/Root/BaseTypeMapper.php +++ b/src/Mappers/Root/BaseTypeMapper.php @@ -17,6 +17,7 @@ use GraphQL\Upload\UploadType; use phpDocumentor\Reflection\DocBlock; use phpDocumentor\Reflection\Type; +use phpDocumentor\Reflection\Types\AbstractList; use phpDocumentor\Reflection\Types\Array_; use phpDocumentor\Reflection\Types\Boolean; use phpDocumentor\Reflection\Types\Float_; @@ -70,7 +71,7 @@ public function toGraphQLOutputType(Type $type, ?OutputType $subType, $reflector return $mappedType; } - if ($type instanceof Array_) { + if ($type instanceof AbstractList) { $innerType = $this->topRootTypeMapper->toGraphQLOutputType($type->getValueType(), $subType, $reflector, $docBlockObj); /*if ($innerType === null) { return $this->next->toGraphQLOutputType($type, $subType, $reflector, $docBlockObj); diff --git a/tests/AbstractQueryProviderTest.php b/tests/AbstractQueryProviderTest.php index 97f558e99c..9a1b2ffd69 100644 --- a/tests/AbstractQueryProviderTest.php +++ b/tests/AbstractQueryProviderTest.php @@ -33,7 +33,6 @@ use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapper; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\Root\BaseTypeMapper; -use TheCodingMachine\GraphQLite\Mappers\Root\AbstractListTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompositeRootTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\CompoundTypeMapper; use TheCodingMachine\GraphQLite\Mappers\Root\EnumTypeMapper; @@ -341,7 +340,6 @@ protected function buildRootTypeMapper(): RootTypeMapperInterface if (interface_exists(UnitEnum::class)) { $rootTypeMapper = new EnumTypeMapper($rootTypeMapper, $this->getAnnotationReader(), $arrayAdapter, []); } - $rootTypeMapper = new AbstractListTypeMapper($rootTypeMapper, $topRootTypeMapper); $rootTypeMapper = new CompoundTypeMapper($rootTypeMapper, $topRootTypeMapper, $this->getTypeRegistry(), $this->getTypeMapper()); $rootTypeMapper = new IteratorTypeMapper($rootTypeMapper, $topRootTypeMapper); diff --git a/tests/Mappers/Root/AbstractListTypeMapperTest.php b/tests/Mappers/Root/AbstractListTypeMapperTest.php deleted file mode 100644 index 632e2a5831..0000000000 --- a/tests/Mappers/Root/AbstractListTypeMapperTest.php +++ /dev/null @@ -1,45 +0,0 @@ -getRootTypeMapper(); - - $result = $typeMapper->toGraphQLOutputType($this->resolveType('ArrayObject'), null, new ReflectionMethod(__CLASS__, 'testOutputGenericNullableValueIterator'), new DocBlock()); - $this->assertInstanceOf(NonNull::class, $result); - $this->assertInstanceOf(ListOfType::class, $result->getWrappedType()); - $this->assertInstanceOf(IntType::class, $result->getWrappedType()->getWrappedType()); - } - - public function testIterableWithTwoArrays(): void - { - $typeMapper = $this->getRootTypeMapper(); - - $result = $typeMapper->toGraphQLOutputType($this->resolveType('array'), null, new ReflectionMethod(__CLASS__, 'testInputIterator'), new DocBlock()); - - $this->assertEquals(1,1); - } - - public function testInputIterator(): void - { - $typeMapper = $this->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()); - } - -} diff --git a/tests/Mappers/Root/BaseTypeMapperTest.php b/tests/Mappers/Root/BaseTypeMapperTest.php index 0cacb886fa..7cd46f2ce4 100644 --- a/tests/Mappers/Root/BaseTypeMapperTest.php +++ b/tests/Mappers/Root/BaseTypeMapperTest.php @@ -2,6 +2,9 @@ namespace TheCodingMachine\GraphQLite\Mappers\Root; +use GraphQL\Type\Definition\IntType; +use GraphQL\Type\Definition\ListOfType; +use GraphQL\Type\Definition\NonNull; use phpDocumentor\Reflection\DocBlock; use phpDocumentor\Reflection\Fqsen; use phpDocumentor\Reflection\Types\Array_; @@ -10,11 +13,19 @@ use phpDocumentor\Reflection\Types\Resource_; use ReflectionMethod; use TheCodingMachine\GraphQLite\AbstractQueryProviderTest; -use TheCodingMachine\GraphQLite\GraphQLRuntimeException; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; class BaseTypeMapperTest extends AbstractQueryProviderTest { + public function testOutputNullableValueIterator(): void + { + $typeMapper = $this->getRootTypeMapper(); + + $result = $typeMapper->toGraphQLOutputType($this->resolveType('ArrayObject'), null, new ReflectionMethod(__CLASS__, 'testOutputNullableValueIterator'), new DocBlock()); + $this->assertInstanceOf(NonNull::class, $result); + $this->assertInstanceOf(ListOfType::class, $result->getWrappedType()); + $this->assertInstanceOf(IntType::class, $result->getWrappedType()->getWrappedType()); + } public function testNullableToGraphQLInputType(): void { From 3222ed8d4e6a02e962632a9a020009f2411c867c Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Tue, 26 Apr 2022 10:56:34 +0300 Subject: [PATCH 04/10] Add iterable generic test case for integration tests https://github.com/thecodingmachine/graphqlite/issues/436 --- .../AggregateControllerQueryProviderTest.php | 2 +- tests/FieldsBuilderTest.php | 31 ++++++++++++++----- tests/Fixtures/TestController.php | 9 ++++++ tests/GlobControllerQueryProviderTest.php | 2 +- 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/tests/AggregateControllerQueryProviderTest.php b/tests/AggregateControllerQueryProviderTest.php index edfd91244b..db5226cee2 100644 --- a/tests/AggregateControllerQueryProviderTest.php +++ b/tests/AggregateControllerQueryProviderTest.php @@ -43,7 +43,7 @@ public function has($id) $aggregateQueryProvider = new AggregateControllerQueryProvider([ 'controller' ], $this->getFieldsBuilder(), $container); $queries = $aggregateQueryProvider->getQueries(); - $this->assertCount(7, $queries); + $this->assertCount(8, $queries); $mutations = $aggregateQueryProvider->getMutations(); $this->assertCount(1, $mutations); diff --git a/tests/FieldsBuilderTest.php b/tests/FieldsBuilderTest.php index 0d984c8cb6..04f7969925 100644 --- a/tests/FieldsBuilderTest.php +++ b/tests/FieldsBuilderTest.php @@ -80,7 +80,7 @@ public function testQueryProvider(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(7, $queries); + $this->assertCount(8, $queries); $usersQuery = $queries['test']; $this->assertSame('test', $usersQuery->name); @@ -201,7 +201,7 @@ public function testQueryProviderWithFixedReturnType(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(7, $queries); + $this->assertCount(8, $queries); $fixedQuery = $queries['testFixReturnType']; $this->assertInstanceOf(IDType::class, $fixedQuery->getType()); @@ -215,7 +215,7 @@ public function testQueryProviderWithComplexFixedReturnType(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(7, $queries); + $this->assertCount(8, $queries); $fixedQuery = $queries['testFixComplexReturnType']; $this->assertInstanceOf(NonNull::class, $fixedQuery->getType()); @@ -406,7 +406,7 @@ public function testQueryProviderWithIterableClass(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(7, $queries); + $this->assertCount(8, $queries); $iterableQuery = $queries['arrayObject']; $this->assertSame('arrayObject', $iterableQuery->name); @@ -423,7 +423,7 @@ public function testQueryProviderWithIterable(): void $queries = $queryProvider->getQueries(new TestController()); - $this->assertCount(7, $queries); + $this->assertCount(8, $queries); $iterableQuery = $queries['iterable']; $this->assertSame('iterable', $iterableQuery->name); @@ -434,6 +434,23 @@ public function testQueryProviderWithIterable(): void $this->assertSame('TestObject', $iterableQuery->getType()->getWrappedType()->getWrappedType()->getWrappedType()->name); } + public function testQueryProviderWithIterableGeneric(): void + { + $queryProvider = $this->buildFieldsBuilder(); + + $queries = $queryProvider->getQueries(new TestController()); + + $this->assertCount(8, $queries); + $iterableQuery = $queries['iterableGeneric']; + + $this->assertSame('iterableGeneric', $iterableQuery->name); + $this->assertInstanceOf(NonNull::class, $iterableQuery->getType()); + $this->assertInstanceOf(ListOfType::class, $iterableQuery->getType()->getWrappedType()); + $this->assertInstanceOf(NonNull::class, $iterableQuery->getType()->getWrappedType()->getWrappedType()); + $this->assertInstanceOf(ObjectType::class, $iterableQuery->getType()->getWrappedType()->getWrappedType()->getWrappedType()); + $this->assertSame('TestObject', $iterableQuery->getType()->getWrappedType()->getWrappedType()->getWrappedType()->name); + } + public function testNoReturnTypeError(): void { $queryProvider = $this->buildFieldsBuilder(); @@ -450,7 +467,7 @@ public function testQueryProviderWithUnion(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(7, $queries); + $this->assertCount(8, $queries); $unionQuery = $queries['union']; $this->assertInstanceOf(NonNull::class, $unionQuery->getType()); @@ -618,7 +635,7 @@ public function testMissingArgument(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(7, $queries); + $this->assertCount(8, $queries); $usersQuery = $queries['test']; $context = []; diff --git a/tests/Fixtures/TestController.php b/tests/Fixtures/TestController.php index 3d3b3b3072..f7708d8ddb 100644 --- a/tests/Fixtures/TestController.php +++ b/tests/Fixtures/TestController.php @@ -103,6 +103,15 @@ public function testIterable(): iterable return array(); } + /** + * @Query(name="iterableGeneric") + * @return iterable + */ + public function testIterableGeneric(): iterable + { + return array(); + } + /** * @Query(name="union") * @return TestObject|TestObject2 diff --git a/tests/GlobControllerQueryProviderTest.php b/tests/GlobControllerQueryProviderTest.php index c0c02c2639..08a1affdcf 100644 --- a/tests/GlobControllerQueryProviderTest.php +++ b/tests/GlobControllerQueryProviderTest.php @@ -39,7 +39,7 @@ public function has($id) $globControllerQueryProvider = new GlobControllerQueryProvider('TheCodingMachine\\GraphQLite\\Fixtures', $this->getFieldsBuilder(), $container, $this->getAnnotationReader(), new Psr16Cache(new NullAdapter()), null, false, false); $queries = $globControllerQueryProvider->getQueries(); - $this->assertCount(7, $queries); + $this->assertCount(8, $queries); $mutations = $globControllerQueryProvider->getMutations(); $this->assertCount(1, $mutations); From ed69062c23a84cfc598274e52b5f76d13661394b Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Tue, 26 Apr 2022 11:41:34 +0300 Subject: [PATCH 05/10] Add a bit of logic for merging types logic to better support generic types https://github.com/thecodingmachine/graphqlite/issues/436 --- src/Mappers/Parameters/TypeHandler.php | 104 +++++++++++++++++++------ 1 file changed, 79 insertions(+), 25 deletions(-) diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index 5a85b682fc..087b485d11 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -14,6 +14,7 @@ use phpDocumentor\Reflection\Type; use phpDocumentor\Reflection\TypeResolver as PhpDocumentorTypeResolver; use phpDocumentor\Reflection\Types\Array_; +use phpDocumentor\Reflection\Types\Collection; use phpDocumentor\Reflection\Types\Compound; use phpDocumentor\Reflection\Types\Iterable_; use phpDocumentor\Reflection\Types\Mixed_; @@ -60,10 +61,13 @@ class TypeHandler implements ParameterHandlerInterface { /** @var PhpDocumentorTypeResolver */ private $phpDocumentorTypeResolver; + /** @var ArgumentResolver */ private $argumentResolver; + /** @var RootTypeMapperInterface */ private $rootTypeMapper; + /** @var TypeResolver */ private $typeResolver; @@ -72,10 +76,10 @@ public function __construct( RootTypeMapperInterface $rootTypeMapper, TypeResolver $typeResolver ) { - $this->argumentResolver = $argumentResolver; - $this->rootTypeMapper = $rootTypeMapper; + $this->argumentResolver = $argumentResolver; + $this->rootTypeMapper = $rootTypeMapper; $this->phpDocumentorTypeResolver = new PhpDocumentorTypeResolver(); - $this->typeResolver = $typeResolver; + $this->typeResolver = $typeResolver; } /** @@ -133,7 +137,7 @@ private function getDocBlockPropertyType(DocBlock $docBlock, ReflectionProperty /** @var Var_[] $varTags */ $varTags = $docBlock->getTagsByName('var'); - if (! $varTags) { + if (!$varTags) { return null; } @@ -144,8 +148,12 @@ private function getDocBlockPropertyType(DocBlock $docBlock, ReflectionProperty return reset($varTags)->getType(); } - public function mapParameter(ReflectionParameter $parameter, DocBlock $docBlock, ?Type $paramTagType, ParameterAnnotations $parameterAnnotations): ParameterInterface - { + public function mapParameter( + ReflectionParameter $parameter, + DocBlock $docBlock, + ?Type $paramTagType, + ParameterAnnotations $parameterAnnotations + ): ParameterInterface { $hideParameter = $parameterAnnotations->getAnnotationByType(HideParameter::class); if ($hideParameter) { if ($parameter->isDefaultValueAvailable() === false) { @@ -165,7 +173,7 @@ public function mapParameter(ReflectionParameter $parameter, DocBlock $docBlock, } } else { $parameterType = $parameter->getType(); - $allowsNull = $parameterType === null ? true : $parameterType->allowsNull(); + $allowsNull = $parameterType === null ? true : $parameterType->allowsNull(); if ($parameterType === null) { $phpdocType = new Mixed_(); @@ -179,7 +187,11 @@ public function mapParameter(ReflectionParameter $parameter, DocBlock $docBlock, try { $declaringFunction = $parameter->getDeclaringFunction(); - Assert::isInstanceOf($declaringFunction, ReflectionMethod::class, 'Parameter of a function passed. Only parameters of methods are supported.'); + Assert::isInstanceOf( + $declaringFunction, + ReflectionMethod::class, + 'Parameter of a function passed. Only parameters of methods are supported.' + ); $type = $this->mapType( $phpdocType, $paramTagType, @@ -197,16 +209,22 @@ public function mapParameter(ReflectionParameter $parameter, DocBlock $docBlock, } $hasDefaultValue = false; - $defaultValue = null; + $defaultValue = null; if ($parameter->allowsNull()) { $hasDefaultValue = true; } if ($parameter->isDefaultValueAvailable()) { $hasDefaultValue = true; - $defaultValue = $parameter->getDefaultValue(); + $defaultValue = $parameter->getDefaultValue(); } - return new InputTypeParameter($parameter->getName(), $type, $hasDefaultValue, $defaultValue, $this->argumentResolver); + return new InputTypeParameter( + $parameter->getName(), + $type, + $hasDefaultValue, + $defaultValue, + $this->argumentResolver + ); } /** @@ -222,8 +240,7 @@ public function mapPropertyType( bool $toInput, ?string $argumentName = null, ?bool $isNullable = null - ): GraphQLType - { + ): GraphQLType { $propertyType = null; // getType function on property reflection is available only since PHP 7.4 @@ -258,7 +275,7 @@ public function mapPropertyType( /** * Maps class property into input property. * - * @param mixed $defaultValue + * @param mixed $defaultValue * * @throws CannotMapTypeException */ @@ -269,8 +286,7 @@ public function mapInputProperty( ?string $inputTypeName = null, $defaultValue = null, ?bool $isNullable = null - ): InputTypeProperty - { + ): InputTypeProperty { $docBlockComment = $docBlock->getSummary() . PHP_EOL . $docBlock->getDescription()->render(); /** @var Var_[] $varTags */ @@ -282,7 +298,7 @@ public function mapInputProperty( if ($isNullable === null) { $varType = $varTag->getType(); if ($varType !== null) { - $isNullable = in_array('null', explode('|', (string) $varType)); + $isNullable = in_array('null', explode('|', (string)$varType)); } } } @@ -308,7 +324,14 @@ public function mapInputProperty( $hasDefault = $defaultValue !== null || $isNullable; $fieldName = $argumentName ?? $refProperty->getName(); - $inputProperty = new InputTypeProperty($refProperty->getName(), $fieldName, $inputType, $hasDefault, $defaultValue, $this->argumentResolver); + $inputProperty = new InputTypeProperty( + $refProperty->getName(), + $fieldName, + $inputType, + $hasDefault, + $defaultValue, + $this->argumentResolver + ); $inputProperty->setDescription(trim($docBlockComment)); return $inputProperty; @@ -329,10 +352,9 @@ private function mapType( $reflector, DocBlock $docBlockObj, ?string $argumentName = null - ): GraphQLType - { + ): 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); } @@ -345,17 +367,39 @@ private function mapType( } if ($mapToInputType === true) { Assert::notNull($argumentName); - $graphQlType = $this->rootTypeMapper->toGraphQLInputType($docBlockType, null, $argumentName, $reflector, $docBlockObj); + $graphQlType = $this->rootTypeMapper->toGraphQLInputType( + $docBlockType, + null, + $argumentName, + $reflector, + $docBlockObj + ); } else { - $graphQlType = $this->rootTypeMapper->toGraphQLOutputType($docBlockType, null, $reflector, $docBlockObj); + $graphQlType = $this->rootTypeMapper->toGraphQLOutputType( + $docBlockType, + null, + $reflector, + $docBlockObj + ); } } else { $completeType = $this->appendTypes($type, $docBlockType); if ($mapToInputType === true) { Assert::notNull($argumentName); - $graphQlType = $this->rootTypeMapper->toGraphQLInputType($completeType, null, $argumentName, $reflector, $docBlockObj); + $graphQlType = $this->rootTypeMapper->toGraphQLInputType( + $completeType, + null, + $argumentName, + $reflector, + $docBlockObj + ); } else { - $graphQlType = $this->rootTypeMapper->toGraphQLOutputType($completeType, null, $reflector, $docBlockObj); + $graphQlType = $this->rootTypeMapper->toGraphQLOutputType( + $completeType, + null, + $reflector, + $docBlockObj + ); } } @@ -376,6 +420,16 @@ private function appendTypes(Type $type, ?Type $docBlockType): Type } $types = [$type]; + + // Try to match generic phpdoc-provided iterables with non-generic return-type-provided iterables + // Example: (return type `\ArrayObject`, phpdoc `\ArrayObject`) + if ($docBlockType instanceof Collection + && $type instanceof Object_ + && (string)$type->getFqsen() === (string)$docBlockType->getFqsen() + ) { + $types = []; + } + if ($docBlockType instanceof Compound) { $docBlockTypes = iterator_to_array($docBlockType); $types = array_merge($types, $docBlockTypes); From cc7faa361d681d3e6e85ffd12a080728d65bfc79 Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Tue, 26 Apr 2022 11:41:47 +0300 Subject: [PATCH 06/10] Add iterable generic test case for integration tests https://github.com/thecodingmachine/graphqlite/issues/436 --- .../AggregateControllerQueryProviderTest.php | 2 +- tests/FieldsBuilderTest.php | 35 ++++++++++++++----- tests/Fixtures/TestController.php | 9 +++++ tests/GlobControllerQueryProviderTest.php | 2 +- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/tests/AggregateControllerQueryProviderTest.php b/tests/AggregateControllerQueryProviderTest.php index db5226cee2..a5db43ca74 100644 --- a/tests/AggregateControllerQueryProviderTest.php +++ b/tests/AggregateControllerQueryProviderTest.php @@ -43,7 +43,7 @@ public function has($id) $aggregateQueryProvider = new AggregateControllerQueryProvider([ 'controller' ], $this->getFieldsBuilder(), $container); $queries = $aggregateQueryProvider->getQueries(); - $this->assertCount(8, $queries); + $this->assertCount(9, $queries); $mutations = $aggregateQueryProvider->getMutations(); $this->assertCount(1, $mutations); diff --git a/tests/FieldsBuilderTest.php b/tests/FieldsBuilderTest.php index 04f7969925..791a09ce89 100644 --- a/tests/FieldsBuilderTest.php +++ b/tests/FieldsBuilderTest.php @@ -80,7 +80,7 @@ public function testQueryProvider(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(8, $queries); + $this->assertCount(9, $queries); $usersQuery = $queries['test']; $this->assertSame('test', $usersQuery->name); @@ -201,7 +201,7 @@ public function testQueryProviderWithFixedReturnType(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(8, $queries); + $this->assertCount(9, $queries); $fixedQuery = $queries['testFixReturnType']; $this->assertInstanceOf(IDType::class, $fixedQuery->getType()); @@ -215,7 +215,7 @@ public function testQueryProviderWithComplexFixedReturnType(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(8, $queries); + $this->assertCount(9, $queries); $fixedQuery = $queries['testFixComplexReturnType']; $this->assertInstanceOf(NonNull::class, $fixedQuery->getType()); @@ -406,7 +406,7 @@ public function testQueryProviderWithIterableClass(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(8, $queries); + $this->assertCount(9, $queries); $iterableQuery = $queries['arrayObject']; $this->assertSame('arrayObject', $iterableQuery->name); @@ -417,13 +417,32 @@ public function testQueryProviderWithIterableClass(): void $this->assertSame('TestObject', $iterableQuery->getType()->getWrappedType()->getWrappedType()->getWrappedType()->name); } + public function testQueryProviderWithIterableGenericClass(): void + { + $controller = new TestController(); + + $queryProvider = $this->buildFieldsBuilder(); + + $queries = $queryProvider->getQueries($controller); + + $this->assertCount(9, $queries); + $iterableQuery = $queries['arrayObjectGeneric']; + + $this->assertSame('arrayObjectGeneric', $iterableQuery->name); + $this->assertInstanceOf(NonNull::class, $iterableQuery->getType()); + $this->assertInstanceOf(ListOfType::class, $iterableQuery->getType()->getWrappedType()); + $this->assertInstanceOf(NonNull::class, $iterableQuery->getType()->getWrappedType()->getWrappedType()); + $this->assertInstanceOf(ObjectType::class, $iterableQuery->getType()->getWrappedType()->getWrappedType()->getWrappedType()); + $this->assertSame('TestObject', $iterableQuery->getType()->getWrappedType()->getWrappedType()->getWrappedType()->name); + } + public function testQueryProviderWithIterable(): void { $queryProvider = $this->buildFieldsBuilder(); $queries = $queryProvider->getQueries(new TestController()); - $this->assertCount(8, $queries); + $this->assertCount(9, $queries); $iterableQuery = $queries['iterable']; $this->assertSame('iterable', $iterableQuery->name); @@ -440,7 +459,7 @@ public function testQueryProviderWithIterableGeneric(): void $queries = $queryProvider->getQueries(new TestController()); - $this->assertCount(8, $queries); + $this->assertCount(9, $queries); $iterableQuery = $queries['iterableGeneric']; $this->assertSame('iterableGeneric', $iterableQuery->name); @@ -467,7 +486,7 @@ public function testQueryProviderWithUnion(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(8, $queries); + $this->assertCount(9, $queries); $unionQuery = $queries['union']; $this->assertInstanceOf(NonNull::class, $unionQuery->getType()); @@ -635,7 +654,7 @@ public function testMissingArgument(): void $queries = $queryProvider->getQueries($controller); - $this->assertCount(8, $queries); + $this->assertCount(9, $queries); $usersQuery = $queries['test']; $context = []; diff --git a/tests/Fixtures/TestController.php b/tests/Fixtures/TestController.php index f7708d8ddb..b11a540863 100644 --- a/tests/Fixtures/TestController.php +++ b/tests/Fixtures/TestController.php @@ -94,6 +94,15 @@ public function testArrayObject(): ArrayObject return new ArrayObject([]); } + /** + * @Query(name="arrayObjectGeneric") + * @return ArrayObject + */ + public function testArrayObjectGeneric(): ArrayObject + { + return new ArrayObject([]); + } + /** * @Query(name="iterable") * @return iterable|TestObject[] diff --git a/tests/GlobControllerQueryProviderTest.php b/tests/GlobControllerQueryProviderTest.php index 08a1affdcf..f1748cbf36 100644 --- a/tests/GlobControllerQueryProviderTest.php +++ b/tests/GlobControllerQueryProviderTest.php @@ -39,7 +39,7 @@ public function has($id) $globControllerQueryProvider = new GlobControllerQueryProvider('TheCodingMachine\\GraphQLite\\Fixtures', $this->getFieldsBuilder(), $container, $this->getAnnotationReader(), new Psr16Cache(new NullAdapter()), null, false, false); $queries = $globControllerQueryProvider->getQueries(); - $this->assertCount(8, $queries); + $this->assertCount(9, $queries); $mutations = $globControllerQueryProvider->getMutations(); $this->assertCount(1, $mutations); From d64dd61f7be41035c3fdd5f811942bdc74392650 Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Tue, 26 Apr 2022 12:31:36 +0300 Subject: [PATCH 07/10] Move logic for matching iterable types logic to another place (more appropriate) https://github.com/thecodingmachine/graphqlite/issues/436 --- src/Mappers/Parameters/TypeHandler.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index 087b485d11..8556a2ae02 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -360,7 +360,17 @@ private function mapType( } $innerType = $type instanceof Nullable ? $type->getActualType() : $type; - if ($innerType instanceof Array_ || $innerType instanceof Iterable_ || $innerType instanceof Mixed_) { + if ( + $innerType instanceof Array_ + || $innerType instanceof Iterable_ + || $innerType instanceof Mixed_ + // Try to match generic phpdoc-provided iterables with non-generic return-type-provided iterables + // Example: (return type `\ArrayObject`, phpdoc `\ArrayObject`) + || ($innerType instanceof Object_ + && $docBlockType instanceof Collection + && (string)$innerType->getFqsen() === (string)$docBlockType->getFqsen() + ) + ) { // We need to use the docBlockType if ($docBlockType === null) { throw CannotMapTypeException::createForMissingPhpDoc($innerType, $reflector, $argumentName); @@ -420,16 +430,6 @@ private function appendTypes(Type $type, ?Type $docBlockType): Type } $types = [$type]; - - // Try to match generic phpdoc-provided iterables with non-generic return-type-provided iterables - // Example: (return type `\ArrayObject`, phpdoc `\ArrayObject`) - if ($docBlockType instanceof Collection - && $type instanceof Object_ - && (string)$type->getFqsen() === (string)$docBlockType->getFqsen() - ) { - $types = []; - } - if ($docBlockType instanceof Compound) { $docBlockTypes = iterator_to_array($docBlockType); $types = array_merge($types, $docBlockTypes); From 3c6daae7f98b7942f8694bf72c8f1ef3f7aa160b Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Tue, 26 Apr 2022 12:40:42 +0300 Subject: [PATCH 08/10] Revert own styling autoformatting changes --- src/Mappers/Parameters/TypeHandler.php | 93 +++++++------------------- 1 file changed, 25 insertions(+), 68 deletions(-) diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index 8556a2ae02..6f360cd5f6 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -61,13 +61,10 @@ class TypeHandler implements ParameterHandlerInterface { /** @var PhpDocumentorTypeResolver */ private $phpDocumentorTypeResolver; - /** @var ArgumentResolver */ private $argumentResolver; - /** @var RootTypeMapperInterface */ private $rootTypeMapper; - /** @var TypeResolver */ private $typeResolver; @@ -76,10 +73,10 @@ public function __construct( RootTypeMapperInterface $rootTypeMapper, TypeResolver $typeResolver ) { - $this->argumentResolver = $argumentResolver; - $this->rootTypeMapper = $rootTypeMapper; + $this->argumentResolver = $argumentResolver; + $this->rootTypeMapper = $rootTypeMapper; $this->phpDocumentorTypeResolver = new PhpDocumentorTypeResolver(); - $this->typeResolver = $typeResolver; + $this->typeResolver = $typeResolver; } /** @@ -137,7 +134,7 @@ private function getDocBlockPropertyType(DocBlock $docBlock, ReflectionProperty /** @var Var_[] $varTags */ $varTags = $docBlock->getTagsByName('var'); - if (!$varTags) { + if (! $varTags) { return null; } @@ -148,12 +145,8 @@ private function getDocBlockPropertyType(DocBlock $docBlock, ReflectionProperty return reset($varTags)->getType(); } - public function mapParameter( - ReflectionParameter $parameter, - DocBlock $docBlock, - ?Type $paramTagType, - ParameterAnnotations $parameterAnnotations - ): ParameterInterface { + public function mapParameter(ReflectionParameter $parameter, DocBlock $docBlock, ?Type $paramTagType, ParameterAnnotations $parameterAnnotations): ParameterInterface + { $hideParameter = $parameterAnnotations->getAnnotationByType(HideParameter::class); if ($hideParameter) { if ($parameter->isDefaultValueAvailable() === false) { @@ -173,7 +166,7 @@ public function mapParameter( } } else { $parameterType = $parameter->getType(); - $allowsNull = $parameterType === null ? true : $parameterType->allowsNull(); + $allowsNull = $parameterType === null ? true : $parameterType->allowsNull(); if ($parameterType === null) { $phpdocType = new Mixed_(); @@ -187,11 +180,7 @@ public function mapParameter( try { $declaringFunction = $parameter->getDeclaringFunction(); - Assert::isInstanceOf( - $declaringFunction, - ReflectionMethod::class, - 'Parameter of a function passed. Only parameters of methods are supported.' - ); + Assert::isInstanceOf($declaringFunction, ReflectionMethod::class, 'Parameter of a function passed. Only parameters of methods are supported.'); $type = $this->mapType( $phpdocType, $paramTagType, @@ -209,22 +198,16 @@ public function mapParameter( } $hasDefaultValue = false; - $defaultValue = null; + $defaultValue = null; if ($parameter->allowsNull()) { $hasDefaultValue = true; } if ($parameter->isDefaultValueAvailable()) { $hasDefaultValue = true; - $defaultValue = $parameter->getDefaultValue(); + $defaultValue = $parameter->getDefaultValue(); } - return new InputTypeParameter( - $parameter->getName(), - $type, - $hasDefaultValue, - $defaultValue, - $this->argumentResolver - ); + return new InputTypeParameter($parameter->getName(), $type, $hasDefaultValue, $defaultValue, $this->argumentResolver); } /** @@ -240,7 +223,8 @@ public function mapPropertyType( bool $toInput, ?string $argumentName = null, ?bool $isNullable = null - ): GraphQLType { + ): GraphQLType + { $propertyType = null; // getType function on property reflection is available only since PHP 7.4 @@ -275,7 +259,7 @@ public function mapPropertyType( /** * Maps class property into input property. * - * @param mixed $defaultValue + * @param mixed $defaultValue * * @throws CannotMapTypeException */ @@ -286,7 +270,8 @@ public function mapInputProperty( ?string $inputTypeName = null, $defaultValue = null, ?bool $isNullable = null - ): InputTypeProperty { + ): InputTypeProperty + { $docBlockComment = $docBlock->getSummary() . PHP_EOL . $docBlock->getDescription()->render(); /** @var Var_[] $varTags */ @@ -298,7 +283,7 @@ public function mapInputProperty( if ($isNullable === null) { $varType = $varTag->getType(); if ($varType !== null) { - $isNullable = in_array('null', explode('|', (string)$varType)); + $isNullable = in_array('null', explode('|', (string) $varType)); } } } @@ -324,14 +309,7 @@ public function mapInputProperty( $hasDefault = $defaultValue !== null || $isNullable; $fieldName = $argumentName ?? $refProperty->getName(); - $inputProperty = new InputTypeProperty( - $refProperty->getName(), - $fieldName, - $inputType, - $hasDefault, - $defaultValue, - $this->argumentResolver - ); + $inputProperty = new InputTypeProperty($refProperty->getName(), $fieldName, $inputType, $hasDefault, $defaultValue, $this->argumentResolver); $inputProperty->setDescription(trim($docBlockComment)); return $inputProperty; @@ -352,9 +330,10 @@ private function mapType( $reflector, DocBlock $docBlockObj, ?string $argumentName = null - ): GraphQLType { + ): 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); } @@ -377,39 +356,17 @@ private function mapType( } if ($mapToInputType === true) { Assert::notNull($argumentName); - $graphQlType = $this->rootTypeMapper->toGraphQLInputType( - $docBlockType, - null, - $argumentName, - $reflector, - $docBlockObj - ); + $graphQlType = $this->rootTypeMapper->toGraphQLInputType($docBlockType, null, $argumentName, $reflector, $docBlockObj); } else { - $graphQlType = $this->rootTypeMapper->toGraphQLOutputType( - $docBlockType, - null, - $reflector, - $docBlockObj - ); + $graphQlType = $this->rootTypeMapper->toGraphQLOutputType($docBlockType, null, $reflector, $docBlockObj); } } else { $completeType = $this->appendTypes($type, $docBlockType); if ($mapToInputType === true) { Assert::notNull($argumentName); - $graphQlType = $this->rootTypeMapper->toGraphQLInputType( - $completeType, - null, - $argumentName, - $reflector, - $docBlockObj - ); + $graphQlType = $this->rootTypeMapper->toGraphQLInputType($completeType, null, $argumentName, $reflector, $docBlockObj); } else { - $graphQlType = $this->rootTypeMapper->toGraphQLOutputType( - $completeType, - null, - $reflector, - $docBlockObj - ); + $graphQlType = $this->rootTypeMapper->toGraphQLOutputType($completeType, null, $reflector, $docBlockObj); } } From ef841d236a776fd90a2a5dfd3d8b6766303e7af3 Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Wed, 27 Apr 2022 10:22:19 +0300 Subject: [PATCH 09/10] Allow to use object type for cannot map type exception --- src/Mappers/CannotMapTypeException.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Mappers/CannotMapTypeException.php b/src/Mappers/CannotMapTypeException.php index d887e55e7f..6000245bb1 100644 --- a/src/Mappers/CannotMapTypeException.php +++ b/src/Mappers/CannotMapTypeException.php @@ -15,6 +15,7 @@ use phpDocumentor\Reflection\Types\Array_; use phpDocumentor\Reflection\Types\Iterable_; use phpDocumentor\Reflection\Types\Mixed_; +use phpDocumentor\Reflection\Types\Object_; use ReflectionMethod; use ReflectionProperty; use TheCodingMachine\GraphQLite\Annotations\ExtendType; @@ -132,7 +133,7 @@ public static function extendTypeWithBadTargetedClass(string $className, ExtendT } /** - * @param Array_|Iterable_|Mixed_ $type + * @param Array_|Iterable_|Object_|Mixed_ $type * @param ReflectionMethod|ReflectionProperty $reflector */ public static function createForMissingPhpDoc(PhpDocumentorType $type, $reflector, ?string $argumentName = null): self @@ -142,6 +143,8 @@ public static function createForMissingPhpDoc(PhpDocumentorType $type, $reflecto $typeStr = 'array'; } elseif ($type instanceof Iterable_) { $typeStr = 'iterable'; + } elseif ($type instanceof Object_) { + $typeStr = \sprintf('object ("%s")', $type->getFqsen()); } elseif ($type instanceof Mixed_) { $typeStr = 'mixed'; } From a15dcf04f1893548836daaca578829050f8c33f5 Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Thu, 28 Apr 2022 13:01:39 +0300 Subject: [PATCH 10/10] Add more tests for generic iterables mapping by base type mapper --- tests/Mappers/Root/BaseTypeMapperTest.php | 58 +++++++++++++++++++---- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/tests/Mappers/Root/BaseTypeMapperTest.php b/tests/Mappers/Root/BaseTypeMapperTest.php index 7cd46f2ce4..11b9eb9bef 100644 --- a/tests/Mappers/Root/BaseTypeMapperTest.php +++ b/tests/Mappers/Root/BaseTypeMapperTest.php @@ -2,9 +2,11 @@ namespace TheCodingMachine\GraphQLite\Mappers\Root; +use GraphQL\Type\Definition\BooleanType; use GraphQL\Type\Definition\IntType; use GraphQL\Type\Definition\ListOfType; use GraphQL\Type\Definition\NonNull; +use GraphQL\Type\Definition\WrappingType; use phpDocumentor\Reflection\DocBlock; use phpDocumentor\Reflection\Fqsen; use phpDocumentor\Reflection\Types\Array_; @@ -13,20 +15,12 @@ use phpDocumentor\Reflection\Types\Resource_; use ReflectionMethod; use TheCodingMachine\GraphQLite\AbstractQueryProviderTest; +use TheCodingMachine\GraphQLite\Fixtures\TestObject; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; +use TheCodingMachine\GraphQLite\Types\MutableObjectType; class BaseTypeMapperTest extends AbstractQueryProviderTest { - public function testOutputNullableValueIterator(): void - { - $typeMapper = $this->getRootTypeMapper(); - - $result = $typeMapper->toGraphQLOutputType($this->resolveType('ArrayObject'), null, new ReflectionMethod(__CLASS__, 'testOutputNullableValueIterator'), new DocBlock()); - $this->assertInstanceOf(NonNull::class, $result); - $this->assertInstanceOf(ListOfType::class, $result->getWrappedType()); - $this->assertInstanceOf(IntType::class, $result->getWrappedType()->getWrappedType()); - } - public function testNullableToGraphQLInputType(): void { $baseTypeMapper = new BaseTypeMapper(new FinalRootTypeMapper($this->getTypeMapper()), $this->getTypeMapper(), $this->getRootTypeMapper()); @@ -62,4 +56,48 @@ public function testUnmappableInputArray(): void $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()); } + + /** + * @param string $phpdocType + * @param class-string $expectedItemType + * + * @return void + * + * @dataProvider genericIterablesProvider + */ + public function testOutputGenericIterables(string $phpdocType, string $expectedItemType, ?string $expectedWrappedItemType = null): void + { + $typeMapper = $this->getRootTypeMapper(); + + $result = $typeMapper->toGraphQLOutputType($this->resolveType($phpdocType), null, new ReflectionMethod(__CLASS__, 'testOutputGenericIterables'), new DocBlock()); + + $this->assertInstanceOf(NonNull::class, $result); + $this->assertInstanceOf(ListOfType::class, $result->getWrappedType()); + $itemType = $result->getWrappedType()->getWrappedType(); + $this->assertInstanceOf($expectedItemType, $itemType); + if (null !== $expectedWrappedItemType) { + $this->assertInstanceOf(WrappingType::class, $itemType); + $this->assertInstanceOf($expectedWrappedItemType, $itemType->getWrappedType()); + } + } + + public function genericIterablesProvider(): iterable + { + yield '\ArrayIterator with nullable int item' => ['\ArrayIterator', IntType::class]; + yield '\ArrayIterator with int item' => ['\ArrayIterator', NonNull::class, IntType::class]; + + // key information cannot be presented in GQL types for now + yield 'iterable with provided int key and test object item' => [ + \sprintf('iterable<%s>', TestObject::class), + NonNull::class, + MutableObjectType::class, + ]; + yield '\Iterator with provided string key and int item' => ['\Iterator', NonNull::class, IntType::class]; + yield '\IteratorAggregate with provided int key and bool item' => ['\IteratorAggregate', NonNull::class, BooleanType::class]; + yield '\Traversable with provided string key and test object item' => [ + \sprintf('\Traversable', TestObject::class), + NonNull::class, + MutableObjectType::class, + ]; + } }