Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/Mappers/Parameters/TypeHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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_
Expand Down
15 changes: 10 additions & 5 deletions src/Mappers/Root/NullableTypeMapperAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions tests/Fixtures/Integration/Controllers/ArticleController.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,18 @@ public function updateArticle(UpdateArticleInput $input): Article
{
$article = new Article('test');
$article->magazine = 'The New Yorker';
$article->tags = ['tech', 'news'];

$article->summary = $input->summary;

if ($input->magazine !== Undefined::VALUE) {
$article->magazine = $input->magazine;
}

if ($input->tags !== Undefined::VALUE) {
$article->tags = $input->tags;
}

return $article;
}
}
4 changes: 4 additions & 0 deletions tests/Fixtures/Integration/Models/Article.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ class Article extends Post
#[Field]
public ?string $magazine = null;

/** @var list<string>|null */
#[Field]
public ?array $tags = null;

#[Field(for: 'Article')]
public function localizedTitle(string|null|Undefined $locale): string
{
Expand Down
9 changes: 7 additions & 2 deletions tests/Fixtures/Integration/Models/UpdateArticleInput.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace TheCodingMachine\GraphQLite\Fixtures\Integration\Models;

use TheCodingMachine\GraphQLite\Annotations\Field;
Expand All @@ -10,13 +12,16 @@
#[Input]
class UpdateArticleInput
{
/** @param list<string>|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,
)
{
}
}
}
39 changes: 39 additions & 0 deletions tests/Integration/EndToEndTest.php

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test here that checks that a passed null value into the input type is respected as an update? So, the default value would be something other than null, and it's updated to be nullified.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oojacoboo, Sure thing. I added the test case

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-georgiadis sorry to be pedantic about this. But can you please assert the value before the mutation and after, so we can be sure that the change is respected. I don't actually recall if the previous mutations maintain state, or if each mutation within a test is stateless.

Original file line number Diff line number Diff line change
Expand Up @@ -2015,6 +2015,45 @@ 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']);

// An omitted list is Undefined, so the controller leaves the default untouched.
$result = GraphQL::executeQuery($schema, '
mutation {
updateArticle(input: { magazine: "The Verge" }) {
tags
}
}
');
$data = $this->getSuccessResult($result);
$this->assertSame(['tech', 'news'], $data['updateArticle']['tags']);

// An explicit null nullifies the default, proving null is distinct from Undefined.
$result = GraphQL::executeQuery($schema, '
mutation {
updateArticle(input: { tags: null }) {
tags
}
}
');
$data = $this->getSuccessResult($result);
$this->assertNull($data['updateArticle']['tags']);
}

public function testEndToEndSchemaIsPrintable(): void
{
$this->expectNotToPerformAssertions();
Expand Down
38 changes: 37 additions & 1 deletion tests/Mappers/Parameters/TypeMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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<string>` must resolve to a single `[String!]`,
// not an illegal `array | list<string>` 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();
Expand Down Expand Up @@ -163,6 +194,11 @@ private function dummy(): int|string
{
}

/** @param list<string>|null $foo */
private function withUndefinedList(array|Undefined|null $foo = Undefined::VALUE): void
{
}

/** @param int $foo Foo parameter */
private function withParamDescription(int $foo): void
{
Expand Down
Loading