Skip to content

Commit 82df722

Browse files
MariusJamMariusJam
authored
Fix/missing eager joins (#6006)
* Revert "Revert "fix: missing eager joins on to-one relationships (#5992)"" This reverts commit dac49cb. * test: fix related dummies related tests * test: duplicate changes in mongoDB documents
1 parent d2f281e commit 82df722

File tree

10 files changed

+148
-13
lines changed

10 files changed

+148
-13
lines changed

‎features/json/relation.feature

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ Feature: JSON relations support
2525
"badFourthLevel": null,
2626
"id": 1,
2727
"level": 3,
28-
"test": true
28+
"test": true,
29+
"relatedDummies": []
2930
}
3031
"""
3132

‎features/jsonapi/related-resouces-inclusion.feature

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,18 @@ Feature: JSON API Inclusion of Related Resources
483483
"type": "FourthLevel",
484484
"id": "/fourth_levels/1"
485485
}
486+
},
487+
"relatedDummies": {
488+
"data": [
489+
{
490+
"type": "RelatedDummy",
491+
"id": "/related_dummies/1"
492+
},
493+
{
494+
"type": "RelatedDummy",
495+
"id": "/related_dummies/2"
496+
}
497+
]
486498
}
487499
}
488500
},
@@ -581,6 +593,16 @@ Feature: JSON API Inclusion of Related Resources
581593
"_id": 1,
582594
"level": 3,
583595
"test": true
596+
},
597+
"relationships": {
598+
"relatedDummies": {
599+
"data": [
600+
{
601+
"type": "RelatedDummy",
602+
"id": "/related_dummies/1"
603+
}
604+
]
605+
}
584606
}
585607
},
586608
{
@@ -618,6 +640,16 @@ Feature: JSON API Inclusion of Related Resources
618640
"_id": 2,
619641
"level": 3,
620642
"test": true
643+
},
644+
"relationships": {
645+
"relatedDummies": {
646+
"data": [
647+
{
648+
"type": "RelatedDummy",
649+
"id": "/related_dummies/2"
650+
}
651+
]
652+
}
621653
}
622654
},
623655
{
@@ -655,6 +687,16 @@ Feature: JSON API Inclusion of Related Resources
655687
"_id": 3,
656688
"level": 3,
657689
"test": true
690+
},
691+
"relationships": {
692+
"relatedDummies": {
693+
"data": [
694+
{
695+
"type": "RelatedDummy",
696+
"id": "/related_dummies/3"
697+
}
698+
]
699+
}
658700
}
659701
}
660702
]
@@ -802,6 +844,24 @@ Feature: JSON API Inclusion of Related Resources
802844
"_id": 1,
803845
"level": 3,
804846
"test": true
847+
},
848+
"relationships": {
849+
"relatedDummies": {
850+
"data": [
851+
{
852+
"type": "RelatedDummy",
853+
"id": "/related_dummies/1"
854+
},
855+
{
856+
"type": "RelatedDummy",
857+
"id": "/related_dummies/2"
858+
},
859+
{
860+
"type": "RelatedDummy",
861+
"id": "/related_dummies/3"
862+
}
863+
]
864+
}
805865
}
806866
},
807867
{
@@ -1286,6 +1346,16 @@ Feature: JSON API Inclusion of Related Resources
12861346
"_id": 1,
12871347
"level": 3,
12881348
"test": true
1349+
},
1350+
"relationships": {
1351+
"relatedDummies": {
1352+
"data": [
1353+
{
1354+
"type": "RelatedDummy",
1355+
"id": "/related_dummies/1"
1356+
}
1357+
]
1358+
}
12891359
}
12901360
},
12911361
{
@@ -1323,6 +1393,16 @@ Feature: JSON API Inclusion of Related Resources
13231393
"_id": 2,
13241394
"level": 3,
13251395
"test": true
1396+
},
1397+
"relationships": {
1398+
"relatedDummies": {
1399+
"data": [
1400+
{
1401+
"type": "RelatedDummy",
1402+
"id": "/related_dummies/2"
1403+
}
1404+
]
1405+
}
13261406
}
13271407
},
13281408
{
@@ -1360,6 +1440,16 @@ Feature: JSON API Inclusion of Related Resources
13601440
"_id": 3,
13611441
"level": 3,
13621442
"test": true
1443+
},
1444+
"relationships": {
1445+
"relatedDummies": {
1446+
"data": [
1447+
{
1448+
"type": "RelatedDummy",
1449+
"id": "/related_dummies/3"
1450+
}
1451+
]
1452+
}
13631453
}
13641454
}
13651455
]

‎features/main/relation.feature

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ Feature: Relations support
2323
"badFourthLevel": null,
2424
"id": 1,
2525
"level": 3,
26-
"test": true
26+
"test": true,
27+
"relatedDummies": []
2728
}
2829
"""
2930

‎features/main/sub_resource.feature

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,11 @@ Feature: Sub-resource support
376376
"badFourthLevel": null,
377377
"id": 1,
378378
"level": 3,
379-
"test": true
379+
"test": true,
380+
"relatedDummies": [
381+
"/related_dummies/1",
382+
"/related_dummies/2"
383+
]
380384
}
381385
"""
382386

‎src/Doctrine/Orm/Extension/EagerLoadingExtension.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt
168168
if (
169169
null !== $parentAssociation
170170
&& isset($mapping['inversedBy'])
171+
&& $mapping['sourceEntity'] === $mapping['targetEntity']
171172
&& $mapping['inversedBy'] === $parentAssociation
172173
&& $mapping['type'] & ClassMetadata::TO_ONE
173174
) {

‎tests/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Dummy;
3030
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\EmbeddableDummy;
3131
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
32+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\ThirdLevel;
3233
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\UnknownDummy;
3334
use Doctrine\ORM\EntityManager;
3435
use Doctrine\ORM\Mapping\ClassMetadata;
@@ -140,6 +141,7 @@ public function testApplyToItem(): void
140141
$propertyNameCollectionFactoryProphecy->create(RelatedDummy::class)->willReturn($relatedNameCollection)->shouldBeCalled();
141142
$propertyNameCollectionFactoryProphecy->create(EmbeddableDummy::class)->willReturn($relatedEmbedableCollection)->shouldBeCalled();
142143
$propertyNameCollectionFactoryProphecy->create(UnknownDummy::class)->willReturn(new PropertyNameCollection(['id']))->shouldBeCalled();
144+
$propertyNameCollectionFactoryProphecy->create(ThirdLevel::class)->willReturn(new PropertyNameCollection(['id']))->shouldBeCalled();
143145

144146
$propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
145147
$relationPropertyMetadata = new ApiProperty();
@@ -151,6 +153,7 @@ public function testApplyToItem(): void
151153
$propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy4', $callContext)->willReturn($relationPropertyMetadata)->shouldBeCalled();
152154
$propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy5', $callContext)->willReturn($relationPropertyMetadata)->shouldBeCalled();
153155
$propertyMetadataFactoryProphecy->create(Dummy::class, 'singleInheritanceRelation', $callContext)->willReturn($relationPropertyMetadata)->shouldBeCalled();
156+
$propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummies', $callContext)->willReturn($relationPropertyMetadata)->shouldBeCalled();
154157

155158
$idPropertyMetadata = new ApiProperty();
156159
$idPropertyMetadata = $idPropertyMetadata->withIdentifier(true);
@@ -169,7 +172,9 @@ public function testApplyToItem(): void
169172
$propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'notindatabase', $callContext)->willReturn($notInDatabasePropertyMetadata)->shouldBeCalled();
170173
$propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'notreadable', $callContext)->willReturn($notReadablePropertyMetadata)->shouldBeCalled();
171174
$propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'relation', $callContext)->willReturn($relationPropertyMetadata)->shouldBeCalled();
175+
$propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'thirdLevel', $callContext)->willReturn($relationPropertyMetadata)->shouldBeCalled();
172176
$propertyMetadataFactoryProphecy->create(UnknownDummy::class, 'id', $callContext)->willReturn($idPropertyMetadata)->shouldBeCalled();
177+
$propertyMetadataFactoryProphecy->create(ThirdLevel::class, 'id', $callContext)->willReturn($idPropertyMetadata)->shouldBeCalled();
173178

174179
$queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
175180

@@ -181,6 +186,7 @@ public function testApplyToItem(): void
181186
'relatedDummy4' => ['fetch' => ClassMetadataInfo::FETCH_EAGER, 'targetEntity' => UnknownDummy::class],
182187
'relatedDummy5' => ['fetch' => ClassMetadataInfo::FETCH_LAZY, 'targetEntity' => UnknownDummy::class],
183188
'singleInheritanceRelation' => ['fetch' => ClassMetadataInfo::FETCH_EAGER, 'targetEntity' => AbstractDummy::class],
189+
'relatedDummies' => ['fetch' => ClassMetadataInfo::FETCH_EAGER, 'targetEntity' => RelatedDummy::class],
184190
];
185191

186192
$relatedClassMetadataProphecy = $this->prophesize(ClassMetadata::class);
@@ -194,6 +200,7 @@ public function testApplyToItem(): void
194200

195201
$relatedClassMetadataProphecy->associationMappings = [
196202
'relation' => ['fetch' => ClassMetadataInfo::FETCH_EAGER, 'joinColumns' => [['nullable' => false]], 'targetEntity' => UnknownDummy::class],
203+
'thirdLevel' => ['fetch' => ClassMetadataInfo::FETCH_EAGER, 'targetEntity' => ThirdLevel::class, 'sourceEntity' => RelatedDummy::class, 'inversedBy' => 'relatedDummies', 'type' => ClassMetadata::TO_ONE],
197204
];
198205

199206
$relatedClassMetadataProphecy->embeddedClasses = ['embeddedDummy' => ['class' => EmbeddableDummy::class]];
@@ -204,26 +211,38 @@ public function testApplyToItem(): void
204211
$unknownClassMetadataProphecy = $this->prophesize(ClassMetadata::class);
205212
$unknownClassMetadataProphecy->associationMappings = [];
206213

214+
$thirdLevelMetadataProphecy = $this->prophesize(ClassMetadata::class);
215+
$thirdLevelMetadataProphecy->associationMappings = [];
216+
207217
$emProphecy = $this->prophesize(EntityManager::class);
208218
$emProphecy->getClassMetadata(Dummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal());
209219
$emProphecy->getClassMetadata(RelatedDummy::class)->shouldBeCalled()->willReturn($relatedClassMetadataProphecy->reveal());
210220
$emProphecy->getClassMetadata(AbstractDummy::class)->shouldBeCalled()->willReturn($singleInheritanceClassMetadataProphecy->reveal());
211221
$emProphecy->getClassMetadata(UnknownDummy::class)->shouldBeCalled()->willReturn($unknownClassMetadataProphecy->reveal());
222+
$emProphecy->getClassMetadata(ThirdLevel::class)->shouldBeCalled()->willReturn($thirdLevelMetadataProphecy->reveal());
212223

213224
$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
214225
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);
215226
$queryBuilderProphecy->leftJoin('o.relatedDummy', 'relatedDummy_a1')->shouldBeCalledTimes(1);
216227
$queryBuilderProphecy->leftJoin('relatedDummy_a1.relation', 'relation_a2')->shouldBeCalledTimes(1);
217-
$queryBuilderProphecy->innerJoin('o.relatedDummy2', 'relatedDummy2_a3')->shouldBeCalledTimes(1);
218-
$queryBuilderProphecy->leftJoin('o.relatedDummy3', 'relatedDummy3_a4')->shouldBeCalledTimes(1);
219-
$queryBuilderProphecy->leftJoin('o.relatedDummy4', 'relatedDummy4_a5')->shouldBeCalledTimes(1);
220-
$queryBuilderProphecy->leftJoin('o.singleInheritanceRelation', 'singleInheritanceRelation_a6')->shouldBeCalledTimes(1);
228+
$queryBuilderProphecy->leftJoin('relatedDummy_a1.thirdLevel', 'thirdLevel_a3')->shouldBeCalledTimes(1);
229+
$queryBuilderProphecy->innerJoin('o.relatedDummy2', 'relatedDummy2_a4')->shouldBeCalledTimes(1);
230+
$queryBuilderProphecy->leftJoin('o.relatedDummy3', 'relatedDummy3_a5')->shouldBeCalledTimes(1);
231+
$queryBuilderProphecy->leftJoin('o.relatedDummy4', 'relatedDummy4_a6')->shouldBeCalledTimes(1);
232+
$queryBuilderProphecy->leftJoin('o.singleInheritanceRelation', 'singleInheritanceRelation_a7')->shouldBeCalledTimes(1);
233+
$queryBuilderProphecy->leftJoin('o.relatedDummies', 'relatedDummies_a8')->shouldBeCalledTimes(1);
234+
$queryBuilderProphecy->leftJoin('relatedDummies_a8.relation', 'relation_a9')->shouldBeCalledTimes(1);
235+
$queryBuilderProphecy->leftJoin('relatedDummies_a8.thirdLevel', 'thirdLevel_a10')->shouldBeCalledTimes(1);
221236
$queryBuilderProphecy->addSelect('partial relatedDummy_a1.{id,name,embeddedDummy.name}')->shouldBeCalledTimes(1);
237+
$queryBuilderProphecy->addSelect('partial thirdLevel_a3.{id}')->shouldBeCalledTimes(1);
222238
$queryBuilderProphecy->addSelect('partial relation_a2.{id}')->shouldBeCalledTimes(1);
223-
$queryBuilderProphecy->addSelect('partial relatedDummy2_a3.{id}')->shouldBeCalledTimes(1);
224-
$queryBuilderProphecy->addSelect('partial relatedDummy3_a4.{id}')->shouldBeCalledTimes(1);
225-
$queryBuilderProphecy->addSelect('partial relatedDummy4_a5.{id}')->shouldBeCalledTimes(1);
226-
$queryBuilderProphecy->addSelect('singleInheritanceRelation_a6')->shouldBeCalledTimes(1);
239+
$queryBuilderProphecy->addSelect('partial relatedDummy2_a4.{id}')->shouldBeCalledTimes(1);
240+
$queryBuilderProphecy->addSelect('partial relatedDummy3_a5.{id}')->shouldBeCalledTimes(1);
241+
$queryBuilderProphecy->addSelect('partial relatedDummy4_a6.{id}')->shouldBeCalledTimes(1);
242+
$queryBuilderProphecy->addSelect('singleInheritanceRelation_a7')->shouldBeCalledTimes(1);
243+
$queryBuilderProphecy->addSelect('partial relatedDummies_a8.{id,name,embeddedDummy.name}')->shouldBeCalledTimes(1);
244+
$queryBuilderProphecy->addSelect('partial relation_a9.{id}')->shouldBeCalledTimes(1);
245+
$queryBuilderProphecy->addSelect('partial thirdLevel_a10.{id}')->shouldBeCalledTimes(1);
227246
$queryBuilderProphecy->getDQLPart('join')->willReturn([]);
228247
$queryBuilderProphecy->getDQLPart('select')->willReturn([]);
229248

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class RelatedDummy extends ParentDummy implements \Stringable
7272
#[ApiFilter(filterClass: DateFilter::class)]
7373
public $dummyDate;
7474
#[Groups(['barcelona', 'chicago', 'friends'])]
75-
#[ODM\ReferenceOne(targetDocument: ThirdLevel::class, cascade: ['persist'], nullable: true, storeAs: 'id')]
75+
#[ODM\ReferenceOne(targetDocument: ThirdLevel::class, cascade: ['persist'], nullable: true, storeAs: 'id', inversedBy: 'relatedDummies')]
7676
public ?ThirdLevel $thirdLevel = null;
7777
#[Groups(['fakemanytomany', 'friends'])]
7878
#[ODM\ReferenceMany(targetDocument: RelatedToDummyFriend::class, cascade: ['persist'], mappedBy: 'relatedDummy', storeAs: 'id')]

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
use ApiPlatform\Metadata\ApiResource;
1717
use ApiPlatform\Metadata\Get;
1818
use ApiPlatform\Metadata\Link;
19+
use Doctrine\Common\Collections\ArrayCollection;
20+
use Doctrine\Common\Collections\Collection;
1921
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
2022
use Symfony\Component\Serializer\Annotation\Groups;
2123

@@ -49,6 +51,13 @@ class ThirdLevel
4951
public ?FourthLevel $fourthLevel = null;
5052
#[ODM\ReferenceOne(targetDocument: FourthLevel::class, cascade: ['persist'])]
5153
public $badFourthLevel;
54+
#[ODM\ReferenceMany(mappedBy: 'thirdLevel', targetDocument: RelatedDummy::class)]
55+
public Collection|iterable $relatedDummies;
56+
57+
public function __construct()
58+
{
59+
$this->relatedDummies = new ArrayCollection();
60+
}
5261

5362
public function getId(): ?int
5463
{

‎tests/Fixtures/TestBundle/Entity/RelatedDummy.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class RelatedDummy extends ParentDummy implements \Stringable
8686
#[ApiFilter(filterClass: DateFilter::class)]
8787
public $dummyDate;
8888

89-
#[ORM\ManyToOne(targetEntity: ThirdLevel::class, cascade: ['persist'])]
89+
#[ORM\ManyToOne(targetEntity: ThirdLevel::class, cascade: ['persist'], inversedBy: 'relatedDummies')]
9090
#[Groups(['barcelona', 'chicago', 'friends'])]
9191
public ?ThirdLevel $thirdLevel = null;
9292

‎tests/Fixtures/TestBundle/Entity/ThirdLevel.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
use ApiPlatform\Metadata\ApiResource;
1717
use ApiPlatform\Metadata\Get;
1818
use ApiPlatform\Metadata\Link;
19+
use Doctrine\Common\Collections\ArrayCollection;
20+
use Doctrine\Common\Collections\Collection;
1921
use Doctrine\ORM\Mapping as ORM;
2022
use Symfony\Component\Serializer\Annotation\Groups;
2123

@@ -51,6 +53,14 @@ class ThirdLevel
5153
#[ORM\ManyToOne(targetEntity: FourthLevel::class, cascade: ['persist'])]
5254
public $badFourthLevel;
5355

56+
#[ORM\OneToMany(mappedBy: 'thirdLevel', targetEntity: RelatedDummy::class)]
57+
public Collection|iterable $relatedDummies;
58+
59+
public function __construct()
60+
{
61+
$this->relatedDummies = new ArrayCollection();
62+
}
63+
5464
public function getId(): ?int
5565
{
5666
return $this->id;

0 commit comments

Comments
 (0)