Skip to content

Commit b029c38

Browse files
jotweajosef.wagner
jotwea
and
josef.wagner
authored
GraphQL: Errors with nullish ManyToOne-Relation when using new ResolverFactory (for version 3.2) (#6092)
* fix(graphql): null cases in ResolverFactory * fix: make PropertyMetadataFactoryInterface nullable for backwards compatibility * fix(graphql): pass propertyMetadataFactory from FieldsBuilder to ResolverFactory * use getBuiltinTypes --------- Co-authored-by: josef.wagner <josef.wagner@hf-solutions.co>
1 parent 53a45d7 commit b029c38

20 files changed

+283
-24
lines changed

‎features/graphql/query.feature

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ Feature: GraphQL query support
2222

2323
@createSchema
2424
Scenario: Retrieve an item with different relations to the same resource
25-
Given there are 2 multiRelationsDummy objects having each a manyToOneRelation, 2 manyToManyRelations and 3 oneToManyRelations
25+
Given there are 2 multiRelationsDummy objects having each 1 manyToOneRelation, 2 manyToManyRelations and 3 oneToManyRelations
2626
When I send the following GraphQL request:
2727
"""
2828
{
@@ -33,6 +33,10 @@ Feature: GraphQL query support
3333
id
3434
name
3535
}
36+
manyToOneResolveRelation {
37+
id
38+
name
39+
}
3640
manyToManyRelations {
3741
edges{
3842
node {
@@ -55,10 +59,13 @@ Feature: GraphQL query support
5559
Then the response status code should be 200
5660
And the response should be in JSON
5761
And the header "Content-Type" should be equal to "application/json"
62+
And the JSON node "errors" should not exist
5863
And the JSON node "data.multiRelationsDummy.id" should be equal to "/multi_relations_dummies/2"
5964
And the JSON node "data.multiRelationsDummy.name" should be equal to "Dummy #2"
6065
And the JSON node "data.multiRelationsDummy.manyToOneRelation.id" should not be null
6166
And the JSON node "data.multiRelationsDummy.manyToOneRelation.name" should be equal to "RelatedManyToOneDummy #2"
67+
And the JSON node "data.multiRelationsDummy.manyToOneResolveRelation.id" should not be null
68+
And the JSON node "data.multiRelationsDummy.manyToOneResolveRelation.name" should be equal to "RelatedManyToOneResolveDummy #2"
6269
And the JSON node "data.multiRelationsDummy.manyToManyRelations.edges" should have 2 element
6370
And the JSON node "data.multiRelationsDummy.manyToManyRelations.edges[1].node.id" should not be null
6471
And the JSON node "data.multiRelationsDummy.manyToManyRelations.edges[0].node.name" should match "#RelatedManyToManyDummy(1|2)2#"
@@ -68,6 +75,53 @@ Feature: GraphQL query support
6875
And the JSON node "data.multiRelationsDummy.oneToManyRelations.edges[0].node.name" should match "#RelatedOneToManyDummy(1|3)2#"
6976
And the JSON node "data.multiRelationsDummy.oneToManyRelations.edges[2].node.name" should match "#RelatedOneToManyDummy(1|3)2#"
7077

78+
@createSchema
79+
Scenario: Retrieve an item with different relations (all unset)
80+
Given there are 2 multiRelationsDummy objects having each 0 manyToOneRelation, 0 manyToManyRelations and 0 oneToManyRelations
81+
When I send the following GraphQL request:
82+
"""
83+
{
84+
multiRelationsDummy(id: "/multi_relations_dummies/2") {
85+
id
86+
name
87+
manyToOneRelation {
88+
id
89+
name
90+
}
91+
manyToOneResolveRelation {
92+
id
93+
name
94+
}
95+
manyToManyRelations {
96+
edges{
97+
node {
98+
id
99+
name
100+
}
101+
}
102+
}
103+
oneToManyRelations {
104+
edges{
105+
node {
106+
id
107+
name
108+
}
109+
}
110+
}
111+
}
112+
}
113+
"""
114+
Then the response status code should be 200
115+
And the response should be in JSON
116+
And the header "Content-Type" should be equal to "application/json"
117+
And the JSON node "errors" should not exist
118+
And the JSON node "data.multiRelationsDummy.id" should be equal to "/multi_relations_dummies/2"
119+
And the JSON node "data.multiRelationsDummy.name" should be equal to "Dummy #2"
120+
And the JSON node "data.multiRelationsDummy.manyToOneRelation" should be null
121+
And the JSON node "data.multiRelationsDummy.manyToOneResolveRelation" should be null
122+
And the JSON node "data.multiRelationsDummy.manyToManyRelations.edges" should have 0 element
123+
And the JSON node "data.multiRelationsDummy.oneToManyRelations.edges" should have 0 element
124+
71125
@createSchema @!mongodb
72126
Scenario: Retrieve an item with child relation to the same resource
73127
Given there are tree dummies

‎src/GraphQl/Resolver/Factory/CollectionResolverFactory.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use ApiPlatform\GraphQl\Resolver\Stage\SecurityStageInterface;
2020
use ApiPlatform\GraphQl\Resolver\Stage\SerializeStageInterface;
2121
use ApiPlatform\Metadata\GraphQl\Operation;
22+
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2223
use ApiPlatform\Metadata\Util\CloneTrait;
2324
use GraphQL\Type\Definition\ResolveInfo;
2425
use Psr\Container\ContainerInterface;
@@ -38,7 +39,7 @@ public function __construct(private readonly ReadStageInterface $readStage, priv
3839
{
3940
}
4041

41-
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null): callable
42+
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null, ?PropertyMetadataFactoryInterface $propertyMetadataFactory = null): callable
4243
{
4344
return function (?array $source, array $args, $context, ResolveInfo $info) use ($resourceClass, $rootClass, $operation): ?array {
4445
// If authorization has failed for a relation field (e.g. via ApiProperty security), the field is not present in the source: null can be returned directly to ensure the collection isn't in the response.

‎src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use ApiPlatform\GraphQl\Resolver\Stage\WriteStageInterface;
2525
use ApiPlatform\Metadata\DeleteOperationInterface;
2626
use ApiPlatform\Metadata\GraphQl\Operation;
27+
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2728
use ApiPlatform\Metadata\Util\ClassInfoTrait;
2829
use ApiPlatform\Metadata\Util\CloneTrait;
2930
use GraphQL\Type\Definition\ResolveInfo;
@@ -44,7 +45,7 @@ public function __construct(private readonly ReadStageInterface $readStage, priv
4445
{
4546
}
4647

47-
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null): callable
48+
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null, ?PropertyMetadataFactoryInterface $propertyMetadataFactory = null): callable
4849
{
4950
return function (?array $source, array $args, $context, ResolveInfo $info) use ($resourceClass, $rootClass, $operation): ?array {
5051
if (null === $resourceClass || null === $operation) {

‎src/GraphQl/Resolver/Factory/ItemResolverFactory.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use ApiPlatform\GraphQl\Resolver\Stage\SerializeStageInterface;
2121
use ApiPlatform\Metadata\GraphQl\Operation;
2222
use ApiPlatform\Metadata\GraphQl\Query;
23+
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2324
use ApiPlatform\Metadata\Util\ClassInfoTrait;
2425
use ApiPlatform\Metadata\Util\CloneTrait;
2526
use GraphQL\Type\Definition\ResolveInfo;
@@ -41,7 +42,7 @@ public function __construct(private readonly ReadStageInterface $readStage, priv
4142
{
4243
}
4344

44-
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null): callable
45+
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null, ?PropertyMetadataFactoryInterface $propertyMetadataFactory = null): callable
4546
{
4647
return function (?array $source, array $args, $context, ResolveInfo $info) use ($resourceClass, $rootClass, $operation) {
4748
// Data already fetched and normalized (field or nested resource)

‎src/GraphQl/Resolver/Factory/ItemSubscriptionResolverFactory.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use ApiPlatform\GraphQl\Subscription\MercureSubscriptionIriGeneratorInterface;
2020
use ApiPlatform\GraphQl\Subscription\SubscriptionManagerInterface;
2121
use ApiPlatform\Metadata\GraphQl\Operation;
22+
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2223
use ApiPlatform\Metadata\Util\ClassInfoTrait;
2324
use ApiPlatform\Metadata\Util\CloneTrait;
2425
use GraphQL\Type\Definition\ResolveInfo;
@@ -37,7 +38,7 @@ public function __construct(private readonly ReadStageInterface $readStage, priv
3738
{
3839
}
3940

40-
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null): callable
41+
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null, ?PropertyMetadataFactoryInterface $propertyMetadataFactory = null): callable
4142
{
4243
return function (?array $source, array $args, $context, ResolveInfo $info) use ($resourceClass, $rootClass, $operation): ?array {
4344
if (null === $resourceClass || null === $operation) {

‎src/GraphQl/Resolver/Factory/ResolverFactory.php

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use ApiPlatform\Metadata\GraphQl\Mutation;
1717
use ApiPlatform\Metadata\GraphQl\Operation;
1818
use ApiPlatform\Metadata\GraphQl\Query;
19+
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
1920
use ApiPlatform\State\ProcessorInterface;
2021
use ApiPlatform\State\ProviderInterface;
2122
use GraphQL\Type\Definition\ResolveInfo;
@@ -28,16 +29,18 @@ public function __construct(
2829
) {
2930
}
3031

31-
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null): callable
32+
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null, ?PropertyMetadataFactoryInterface $propertyMetadataFactory = null): callable
3233
{
33-
return function (?array $source, array $args, $context, ResolveInfo $info) use ($resourceClass, $rootClass, $operation) {
34-
// Data already fetched and normalized (field or nested resource)
35-
if ($body = $source[$info->fieldName] ?? null) {
36-
return $body;
37-
}
34+
return function (?array $source, array $args, $context, ResolveInfo $info) use ($resourceClass, $rootClass, $operation, $propertyMetadataFactory) {
35+
if (\array_key_exists($info->fieldName, $source ?? [])) {
36+
$body = $source[$info->fieldName];
3837

39-
if (null === $resourceClass && \array_key_exists($info->fieldName, $source ?? [])) {
40-
return $body;
38+
$propertyMetadata = $rootClass ? $propertyMetadataFactory?->create($rootClass, $info->fieldName) : null;
39+
$type = $propertyMetadata?->getBuiltinTypes()[0] ?? null;
40+
// Data already fetched and normalized (field or nested resource)
41+
if ($body || null === $resourceClass || ($type && !$type->isCollection())) {
42+
return $body;
43+
}
4144
}
4245

4346
// If authorization has failed for a relation field (e.g. via ApiProperty security), the field is not present in the source: null can be returned directly to ensure the collection isn't in the response.

‎src/GraphQl/Resolver/Factory/ResolverFactoryInterface.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
namespace ApiPlatform\GraphQl\Resolver\Factory;
1515

1616
use ApiPlatform\Metadata\GraphQl\Operation;
17+
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
1718

1819
/**
1920
* Builds a GraphQL resolver.
@@ -22,5 +23,5 @@
2223
*/
2324
interface ResolverFactoryInterface
2425
{
25-
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null): callable;
26+
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null, ?PropertyMetadataFactoryInterface $propertyMetadataFactory = null): callable;
2627
}

‎src/GraphQl/Tests/Resolver/Factory/ResolverFactoryTest.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
namespace ApiPlatform\GraphQl\Tests\Resolver\Factory;
1515

1616
use ApiPlatform\GraphQl\Resolver\Factory\ResolverFactory;
17+
use ApiPlatform\Metadata\ApiProperty;
1718
use ApiPlatform\Metadata\GraphQl\Mutation;
1819
use ApiPlatform\Metadata\GraphQl\Operation;
1920
use ApiPlatform\Metadata\GraphQl\Query;
21+
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2022
use ApiPlatform\State\ProcessorInterface;
2123
use ApiPlatform\State\ProviderInterface;
2224
use GraphQL\Type\Definition\ResolveInfo;
@@ -38,11 +40,13 @@ public function testGraphQlResolver(?string $resourceClass = null, ?string $root
3840
$provider->expects($this->once())->method('provide')->with($providedOperation ?: $operation, [], $context)->willReturn($body);
3941
$processor = $this->createMock(ProcessorInterface::class);
4042
$processor->expects($this->once())->method('process')->with($body, $processedOperation ?: $operation, [], $context)->willReturn($returnValue);
43+
$propertyMetadataFactory = $this->createMock(PropertyMetadataFactoryInterface::class);
44+
$propertyMetadataFactory->expects($this->once())->method('create')->with($rootClass, 'test')->willReturn(new ApiProperty(schema: ['type' => 'array']));
4145
$resolveInfo = $this->createMock(ResolveInfo::class);
4246
$resolveInfo->fieldName = 'test';
4347

4448
$resolverFactory = new ResolverFactory($provider, $processor);
45-
$this->assertEquals($resolverFactory->__invoke($resourceClass, $rootClass, $operation)(['test' => null], [], [], $resolveInfo), $returnValue);
49+
$this->assertEquals($resolverFactory->__invoke($resourceClass, $rootClass, $operation, $propertyMetadataFactory)(['test' => null], [], [], $resolveInfo), $returnValue);
4650
}
4751

4852
public function graphQlQueries(): array

‎src/GraphQl/Type/FieldsBuilder.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ private function getResourceFieldConfiguration(?string $property, ?string $field
348348
if ($isStandardGraphqlType || $input) {
349349
$resolve = null;
350350
} else {
351-
$resolve = ($this->itemResolverFactory)($resourceClass, $rootResource, $resourceOperation);
351+
$resolve = ($this->itemResolverFactory)($resourceClass, $rootResource, $resourceOperation, $this->propertyMetadataFactory);
352352
}
353353
} else {
354354
if ($isStandardGraphqlType || $input) {

‎src/Symfony/GraphQl/Resolver/Factory/DataCollectorResolverFactory.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
use ApiPlatform\GraphQl\Resolver\Factory\ResolverFactoryInterface;
1717
use ApiPlatform\Metadata\GraphQl\Operation;
18+
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
1819
use GraphQL\Type\Definition\ResolveInfo;
1920
use Symfony\Component\HttpFoundation\RequestStack;
2021

@@ -24,7 +25,7 @@ public function __construct(private readonly ResolverFactoryInterface $resolverF
2425
{
2526
}
2627

27-
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null): callable
28+
public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null, ?PropertyMetadataFactoryInterface $propertyMetadataFactory = null): callable
2829
{
2930
return function (?array $source, array $args, $context, ResolveInfo $info) use ($resourceClass, $rootClass, $operation) {
3031
if ($this->requestStack && null !== $request = $this->requestStack->getCurrentRequest()) {

‎tests/Behat/DoctrineContext.php

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
use ApiPlatform\Tests\Fixtures\TestBundle\Document\MaxDepthDummy as MaxDepthDummyDocument;
6868
use ApiPlatform\Tests\Fixtures\TestBundle\Document\MultiRelationsDummy as MultiRelationsDummyDocument;
6969
use ApiPlatform\Tests\Fixtures\TestBundle\Document\MultiRelationsRelatedDummy as MultiRelationsRelatedDummyDocument;
70+
use ApiPlatform\Tests\Fixtures\TestBundle\Document\MultiRelationsResolveDummy as MultiRelationsResolveDummyDocument;
7071
use ApiPlatform\Tests\Fixtures\TestBundle\Document\MusicGroup as MusicGroupDocument;
7172
use ApiPlatform\Tests\Fixtures\TestBundle\Document\NetworkPathDummy as NetworkPathDummyDocument;
7273
use ApiPlatform\Tests\Fixtures\TestBundle\Document\NetworkPathRelationDummy as NetworkPathRelationDummyDocument;
@@ -160,6 +161,7 @@
160161
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MaxDepthDummy;
161162
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MultiRelationsDummy;
162163
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MultiRelationsRelatedDummy;
164+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MultiRelationsResolveDummy;
163165
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MusicGroup;
164166
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\NetworkPathDummy;
165167
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\NetworkPathRelationDummy;
@@ -803,17 +805,24 @@ public function thereAreDummyObjectsWithRelatedDummies(int $nb, int $nbrelated):
803805
}
804806

805807
/**
806-
* @Given there are :nb multiRelationsDummy objects having each a manyToOneRelation, :nbmtmr manyToManyRelations and :nbotmr oneToManyRelations
808+
* @Given there are :nb multiRelationsDummy objects having each :nbmtor manyToOneRelation, :nbmtmr manyToManyRelations and :nbotmr oneToManyRelations
807809
*/
808-
public function thereAreMultiRelationsDummyObjectsHavingEachAManyToOneRelationManyToManyRelationsAndOneToManyRelations(int $nb, int $nbmtmr, int $nbotmr): void
810+
public function thereAreMultiRelationsDummyObjectsHavingEachAManyToOneRelationManyToManyRelationsAndOneToManyRelations(int $nb, int $nbmtor, int $nbmtmr, int $nbotmr): void
809811
{
810812
for ($i = 1; $i <= $nb; ++$i) {
811813
$relatedDummy = $this->buildMultiRelationsRelatedDummy();
812814
$relatedDummy->name = 'RelatedManyToOneDummy #'.$i;
813815

816+
$resolveDummy = $this->buildMultiRelationsResolveDummy();
817+
$resolveDummy->name = 'RelatedManyToOneResolveDummy #'.$i;
818+
814819
$dummy = $this->buildMultiRelationsDummy();
815820
$dummy->name = 'Dummy #'.$i;
816-
$dummy->setManyToOneRelation($relatedDummy);
821+
822+
if ($nbmtor) {
823+
$dummy->setManyToOneRelation($relatedDummy);
824+
$dummy->setManyToOneResolveRelation($resolveDummy);
825+
}
817826

818827
for ($j = 1; $j <= $nbmtmr; ++$j) {
819828
$manyToManyItem = $this->buildMultiRelationsRelatedDummy();
@@ -833,6 +842,7 @@ public function thereAreMultiRelationsDummyObjectsHavingEachAManyToOneRelationMa
833842
}
834843

835844
$this->manager->persist($relatedDummy);
845+
$this->manager->persist($resolveDummy);
836846
$this->manager->persist($dummy);
837847
}
838848
$this->manager->flush();
@@ -2627,6 +2637,11 @@ private function buildMultiRelationsRelatedDummy(): MultiRelationsRelatedDummy|M
26272637
return $this->isOrm() ? new MultiRelationsRelatedDummy() : new MultiRelationsRelatedDummyDocument();
26282638
}
26292639

2640+
private function buildMultiRelationsResolveDummy(): MultiRelationsResolveDummy|MultiRelationsResolveDummyDocument
2641+
{
2642+
return $this->isOrm() ? new MultiRelationsResolveDummy() : new MultiRelationsResolveDummyDocument();
2643+
}
2644+
26302645
private function buildMusicGroup(): MusicGroup|MusicGroupDocument
26312646
{
26322647
return $this->isOrm() ? new MusicGroup() : new MusicGroupDocument();

‎tests/Fixtures/TestBundle/Document/MultiRelationsDummy.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ class MultiRelationsDummy
3838
#[ODM\ReferenceOne(targetDocument: MultiRelationsRelatedDummy::class, storeAs: 'id', nullable: true)]
3939
public ?MultiRelationsRelatedDummy $manyToOneRelation = null;
4040

41+
#[ODM\ReferenceOne(targetDocument: MultiRelationsResolveDummy::class, storeAs: 'id', nullable: true)]
42+
public ?MultiRelationsResolveDummy $manyToOneResolveRelation = null;
43+
4144
/** @var Collection<int, MultiRelationsRelatedDummy> */
4245
#[ODM\ReferenceMany(targetDocument: MultiRelationsRelatedDummy::class, storeAs: 'id', nullable: true)]
4346
public Collection $manyToManyRelations;
@@ -67,6 +70,18 @@ public function setManyToOneRelation(?MultiRelationsRelatedDummy $relatedMultiUs
6770
$this->manyToOneRelation = $relatedMultiUsedDummy;
6871
}
6972

73+
public function getManyToOneResolveRelation(): ?MultiRelationsResolveDummy
74+
{
75+
return $this->manyToOneResolveRelation;
76+
}
77+
78+
public function setManyToOneResolveRelation(?MultiRelationsResolveDummy $manyToOneResolveRelation): self
79+
{
80+
$this->manyToOneResolveRelation = $manyToOneResolveRelation;
81+
82+
return $this;
83+
}
84+
7085
public function addManyToManyRelation(MultiRelationsRelatedDummy $relatedMultiUsedDummy): void
7186
{
7287
$this->manyToManyRelations->add($relatedMultiUsedDummy);

0 commit comments

Comments
 (0)