diff --git a/src/Mappers/Parameters/TypeHandler.php b/src/Mappers/Parameters/TypeHandler.php index 2565abf0ed..fe881bc876 100644 --- a/src/Mappers/Parameters/TypeHandler.php +++ b/src/Mappers/Parameters/TypeHandler.php @@ -38,6 +38,7 @@ use TheCodingMachine\GraphQLite\InvalidDocBlockRuntimeException; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeExceptionInterface; +use TheCodingMachine\GraphQLite\Mappers\Root\NullableTypeMapperAdapter; use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperInterface; use TheCodingMachine\GraphQLite\Mappers\Root\UndefinedTypeMapper; use TheCodingMachine\GraphQLite\Parameters\DefaultValueParameter; @@ -409,6 +410,12 @@ private function mapType( } $innerType = $type instanceof Nullable ? $type->getActualType() : $type; + if ($innerType instanceof Compound && UndefinedTypeMapper::containsUndefined($innerType)) { + $innerType = NullableTypeMapperAdapter::getNonNullableType( + UndefinedTypeMapper::replaceUndefinedWith($innerType), + ) ?? $innerType; + } + if ( $innerType instanceof Array_ || $innerType instanceof Iterable_ diff --git a/src/Mappers/Root/NullableTypeMapperAdapter.php b/src/Mappers/Root/NullableTypeMapperAdapter.php index 9983884e81..fe1c0bab39 100644 --- a/src/Mappers/Root/NullableTypeMapperAdapter.php +++ b/src/Mappers/Root/NullableTypeMapperAdapter.php @@ -43,7 +43,7 @@ public function toGraphQLOutputType(Type $type, OutputType|GraphQLType|null $sub $isNullable = $this->isNullable($type); if ($isNullable) { - $nonNullableType = $this->getNonNullable($type); + $nonNullableType = self::getNonNullableType($type); if ($nonNullableType === null) { throw CannotMapTypeException::createForNull(); } @@ -69,7 +69,7 @@ public function toGraphQLInputType(Type $type, InputType|null $subType, string $ $isNullable = $this->isNullable($type); if ($isNullable) { - $nonNullableType = $this->getNonNullable($type); + $nonNullableType = self::getNonNullableType($type); if ($nonNullableType === null) { throw CannotMapTypeException::createForNull(); } @@ -119,16 +119,21 @@ private function isNullable(Type $docBlockTypeHint): bool return false; } - private function getNonNullable(Type $type): Type|null + /** + * Reduces a type to its non-nullable form: strips `Nullable`/`Null_` wrappers and, for a Compound, + * drops every null member. Returns the single remaining type, a Compound of the survivors when more + * than one remains, or null when the type was purely null. + */ + public static function getNonNullableType(Type $type): Type|null { if ($type instanceof Null_) { return null; } if ($type instanceof Nullable) { - return $this->getNonNullable($type->getActualType()); + return self::getNonNullableType($type->getActualType()); } if ($type instanceof Compound) { - $types = array_map([$this, 'getNonNullable'], iterator_to_array($type)); + $types = array_map([self::class, 'getNonNullableType'], iterator_to_array($type)); // Remove null values $types = array_values(array_filter($types)); if (count($types) > 1) { diff --git a/tests/Fixtures/Integration/Controllers/ArticleController.php b/tests/Fixtures/Integration/Controllers/ArticleController.php index af8c6a23fa..0ae82b9719 100644 --- a/tests/Fixtures/Integration/Controllers/ArticleController.php +++ b/tests/Fixtures/Integration/Controllers/ArticleController.php @@ -34,6 +34,7 @@ public function updateArticle(UpdateArticleInput $input): Article { $article = new Article('test'); $article->magazine = 'The New Yorker'; + $article->tags = ['tech', 'news']; $article->summary = $input->summary; @@ -41,6 +42,10 @@ public function updateArticle(UpdateArticleInput $input): Article $article->magazine = $input->magazine; } + if ($input->tags !== Undefined::VALUE) { + $article->tags = $input->tags; + } + return $article; } } diff --git a/tests/Fixtures/Integration/Models/Article.php b/tests/Fixtures/Integration/Models/Article.php index 8a2415b269..1a57b366a3 100644 --- a/tests/Fixtures/Integration/Models/Article.php +++ b/tests/Fixtures/Integration/Models/Article.php @@ -19,6 +19,10 @@ class Article extends Post #[Field] public ?string $magazine = null; + /** @var list|null */ + #[Field] + public ?array $tags = null; + #[Field(for: 'Article')] public function localizedTitle(string|null|Undefined $locale): string { diff --git a/tests/Fixtures/Integration/Models/UpdateArticleInput.php b/tests/Fixtures/Integration/Models/UpdateArticleInput.php index d546fdec22..580c601b58 100644 --- a/tests/Fixtures/Integration/Models/UpdateArticleInput.php +++ b/tests/Fixtures/Integration/Models/UpdateArticleInput.php @@ -1,5 +1,7 @@ |Undefined|null $tags */ public function __construct( #[Field] #[Security("magazine != 'NYTimes'")] - public readonly string|null|Undefined $magazine = Undefined::VALUE, + public readonly string|Undefined|null $magazine = Undefined::VALUE, #[Field] public readonly string $summary = 'default', + #[Field] + public readonly array|Undefined|null $tags = Undefined::VALUE, ) { } -} \ No newline at end of file +} diff --git a/tests/Integration/EndToEndTest.php b/tests/Integration/EndToEndTest.php index 31eba39313..ad6a4647a7 100644 --- a/tests/Integration/EndToEndTest.php +++ b/tests/Integration/EndToEndTest.php @@ -2015,6 +2015,39 @@ public function testEndToEndInputUndefinedValue(): void $this->assertSame('test', $data['updateArticle']['localizedTitle']); } + public function testEndToEndInputUndefinedListValue(): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + // A provided list round-trips. + $result = GraphQL::executeQuery($schema, ' + mutation { + updateArticle(input: { tags: ["news", "tech"] }) { + tags + } + } + '); + $data = $this->getSuccessResult($result); + $this->assertSame(['news', 'tech'], $data['updateArticle']['tags']); + + // Same seeded default (['tech', 'news']), two inputs in one operation: omitting tags leaves it + // untouched, while an explicit null nullifies it. + $result = GraphQL::executeQuery($schema, ' + mutation { + untouched: updateArticle(input: { magazine: "The Verge" }) { + tags + } + nullified: updateArticle(input: { tags: null }) { + tags + } + } + '); + $data = $this->getSuccessResult($result); + $this->assertSame(['tech', 'news'], $data['untouched']['tags']); + $this->assertNull($data['nullified']['tags']); + } + public function testEndToEndSchemaIsPrintable(): void { $this->expectNotToPerformAssertions(); diff --git a/tests/Mappers/Parameters/TypeMapperTest.php b/tests/Mappers/Parameters/TypeMapperTest.php index b0c6c09ea3..f0f2be771e 100644 --- a/tests/Mappers/Parameters/TypeMapperTest.php +++ b/tests/Mappers/Parameters/TypeMapperTest.php @@ -16,10 +16,11 @@ use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; use TheCodingMachine\GraphQLite\Parameters\DefaultValueParameter; use TheCodingMachine\GraphQLite\Parameters\InputTypeParameter; -use TheCodingMachine\GraphQLite\Reflection\DocBlock\CachedDocBlockFactory; +use TheCodingMachine\GraphQLite\Undefined; use function assert; use function count; +use function reset; class TypeMapperTest extends AbstractQueryProvider { @@ -92,6 +93,36 @@ public function testMapObjectNullableUnionWorks(): void $this->assertEquals('TestObject2', $unionTypes[1]->name); } + public function testMapUndefinedListParameterDoesNotCreateForbiddenUnion(): void + { + $docBlockFactory = $this->getDocBlockFactory(); + + $typeMapper = new TypeHandler( + $this->getArgumentResolver(), + $this->getRootTypeMapper(), + $this->getTypeResolver(), + $docBlockFactory, + ); + + $refMethod = new ReflectionMethod($this, 'withUndefinedList'); + $refParameter = $refMethod->getParameters()[0]; + $docBlockObj = $docBlockFactory->create($refMethod); + $paramTags = $docBlockObj->getTagsByName('param'); + $paramTagType = reset($paramTags)->getType(); + $annotations = $this->getAnnotationReader()->getParameterAnnotationsPerParameter([$refParameter])['foo']; + + $parameter = $typeMapper->mapParameter($refParameter, $docBlockObj, $paramTagType, $annotations); + + $this->assertInstanceOf(InputTypeParameter::class, $parameter); + assert($parameter instanceof InputTypeParameter); + // The reflection `array` and the phpdoc `list` must resolve to a single `[String!]`, + // not an illegal `array | list` input union once `Undefined` is in the type. + $this->assertSame('[String!]', $parameter->getType()->toString()); + // The `Undefined` default is the "optional field" marker: GraphQL prints no default for it. + $this->assertFalse($parameter->hasDefaultValue()); + $this->assertNull($parameter->getDefaultValue()); + } + public function testHideParameter(): void { $docBlockFactory = $this->getDocBlockFactory(); @@ -163,6 +194,11 @@ private function dummy(): int|string { } + /** @param list|null $foo */ + private function withUndefinedList(array|Undefined|null $foo = Undefined::VALUE): void + { + } + /** @param int $foo Foo parameter */ private function withParamDescription(int $foo): void {