Skip to content

Commit ab9169c

Browse files
author
abluchet
committed
fix eager, left join if previous was a left join
1 parent 418211d commit ab9169c

File tree

2 files changed

+41
-21
lines changed

2 files changed

+41
-21
lines changed

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,11 @@ public function getMetadataProperties(string $resourceClass) : array
6666
*
6767
* @param QueryBuilder $queryBuilder
6868
* @param string $resourceClass
69-
* @param string $originAlias
70-
* @param string $relationAlias
69+
* @param string $originAlias the current entity alias (first o, then a1, a2 etc.)
70+
* @param string $relationAlias the previous relation alias to keep it unique
71+
* @param bool $wasLeftJoin if the relation containing the new one had a left join, we have to force the new one to left join too
7172
*/
72-
private function joinRelations(QueryBuilder $queryBuilder, string $resourceClass, string $originAlias = 'o', string &$relationAlias = 'a')
73+
private function joinRelations(QueryBuilder $queryBuilder, string $resourceClass, string $originAlias = 'o', string &$relationAlias = 'a', bool $wasLeftJoin = false)
7374
{
7475
$classMetadata = $queryBuilder->getEntityManager()->getClassMetadata($resourceClass);
7576
$j = 0;
@@ -82,12 +83,16 @@ private function joinRelations(QueryBuilder $queryBuilder, string $resourceClass
8283
continue;
8384
}
8485

85-
$joinColumns = $mapping['joinColumns'] ?? $mapping['joinTable']['joinColumns'] ?? null;
86+
if (false === $wasLeftJoin) {
87+
$joinColumns = $mapping['joinColumns'] ?? $mapping['joinTable']['joinColumns'] ?? null;
8688

87-
if (null === $joinColumns) {
88-
$method = 'leftJoin';
89+
if (null === $joinColumns) {
90+
$method = 'leftJoin';
91+
} else {
92+
$method = false === $joinColumns[0]['nullable'] ? 'innerJoin' : 'leftJoin';
93+
}
8994
} else {
90-
$method = false === $joinColumns[0]['nullable'] ? 'innerJoin' : 'leftJoin';
95+
$method = 'leftJoin';
9196
}
9297

9398
$associationAlias = $relationAlias.$i;
@@ -110,7 +115,8 @@ private function joinRelations(QueryBuilder $queryBuilder, string $resourceClass
110115
$queryBuilder->addSelect(sprintf('partial %s.{%s}', $associationAlias, implode(',', $select)));
111116

112117
$relationAlias = $relationAlias.++$j;
113-
$this->joinRelations($queryBuilder, $mapping['targetEntity'], $associationAlias, $relationAlias);
118+
119+
$this->joinRelations($queryBuilder, $mapping['targetEntity'], $associationAlias, $relationAlias, $method === 'leftJoin');
114120
}
115121
}
116122
}

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

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use ApiPlatform\Core\Metadata\Property\PropertyNameCollection;
2020
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy;
2121
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
22+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\UnknownDummy;
2223
use Doctrine\ORM\EntityManager;
2324
use Doctrine\ORM\Mapping\ClassMetadata;
2425
use Doctrine\ORM\QueryBuilder;
@@ -96,9 +97,10 @@ public function testApplyToItem()
9697
{
9798
$propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
9899

99-
$relatedNameCollection = new PropertyNameCollection(['id', 'name', 'notindatabase', 'notreadable']);
100+
$relatedNameCollection = new PropertyNameCollection(['id', 'name', 'notindatabase', 'notreadable', 'relation']);
100101

101102
$propertyNameCollectionFactoryProphecy->create(RelatedDummy::class)->willReturn($relatedNameCollection)->shouldBeCalled();
103+
$propertyNameCollectionFactoryProphecy->create(UnknownDummy::class)->willReturn(new PropertyNameCollection(['id']))->shouldBeCalled();
102104

103105
$propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
104106
$relationPropertyMetadata = new PropertyMetadata();
@@ -122,16 +124,19 @@ public function testApplyToItem()
122124
$propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'name')->willReturn($namePropertyMetadata)->shouldBeCalled();
123125
$propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'notindatabase')->willReturn($notInDatabasePropertyMetadata)->shouldBeCalled();
124126
$propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'notreadable')->willReturn($notReadablePropertyMetadata)->shouldBeCalled();
127+
$propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'relation')->willReturn($relationPropertyMetadata)->shouldBeCalled();
128+
$propertyMetadataFactoryProphecy->create(UnknownDummy::class, 'id')->willReturn($idPropertyMetadata)->shouldBeCalled();
125129

126130
$queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
127131

128132
$classMetadataProphecy = $this->prophesize(ClassMetadata::class);
129-
$classMetadataProphecy->getAssociationNames()->shouldBeCalled()->willReturn([0 => 'relatedDummy', 'relatedDummy2', 'relatedDummy3', 'relatedDummy4']);
133+
$classMetadataProphecy->getAssociationNames()->shouldBeCalled()->willReturn(['relatedDummy', 'relatedDummy2', 'relatedDummy3', 'relatedDummy4']);
130134
$classMetadataProphecy->associationMappings = [
131135
'relatedDummy' => ['fetch' => 3, 'joinColumns' => [['nullable' => true]], 'targetEntity' => RelatedDummy::class],
132-
'relatedDummy2' => ['fetch' => 3, 'joinColumns' => [['nullable' => false]], 'targetEntity' => RelatedDummy::class],
133-
'relatedDummy3' => ['fetch' => 3, 'joinTable' => ['joinColumns' => [['nullable' => false]]], 'targetEntity' => RelatedDummy::class],
134-
'relatedDummy4' => ['fetch' => 3, 'targetEntity' => RelatedDummy::class],
136+
'relatedDummy2' => ['fetch' => 3, 'joinColumns' => [['nullable' => false]], 'targetEntity' => UnknownDummy::class],
137+
'relatedDummy3' => ['fetch' => 3, 'joinTable' => ['joinColumns' => [['nullable' => false]]], 'targetEntity' => UnknownDummy::class],
138+
'relatedDummy4' => ['fetch' => 3, 'targetEntity' => UnknownDummy::class],
139+
'relatedDummy5' => ['fetch' => 2, 'targetEntity' => UnknownDummy::class],
135140
];
136141

137142
$relatedClassMetadataProphecy = $this->prophesize(ClassMetadata::class);
@@ -142,26 +147,35 @@ public function testApplyToItem()
142147
}
143148
}
144149

145-
$relatedClassMetadataProphecy->getAssociationNames()->shouldBeCalled()->willReturn([]);
150+
$relatedClassMetadataProphecy->getAssociationNames()->shouldBeCalled()->willReturn(['relation']);
151+
152+
$relatedClassMetadataProphecy->associationMappings = [
153+
'relation' => ['fetch' => 3, 'joinColumns' => [['nullable' => false]], 'targetEntity' => UnknownDummy::class],
154+
];
155+
156+
$unknownClassMetadataProphecy = $this->prophesize(ClassMetadata::class);
157+
$unknownClassMetadataProphecy->getAssociationNames()->shouldBeCalled()->willReturn([]);
146158

147159
$emProphecy = $this->prophesize(EntityManager::class);
148160
$emProphecy->getClassMetadata(Dummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal());
149161
$emProphecy->getClassMetadata(RelatedDummy::class)->shouldBeCalled()->willReturn($relatedClassMetadataProphecy->reveal());
162+
$emProphecy->getClassMetadata(UnknownDummy::class)->shouldBeCalled()->willReturn($unknownClassMetadataProphecy->reveal());
150163

151164
$queryBuilderProphecy->leftJoin('o.relatedDummy', 'a0')->shouldBeCalled(1);
152-
$queryBuilderProphecy->innerJoin('o.relatedDummy2', 'a11')->shouldBeCalled(1);
153-
$queryBuilderProphecy->innerJoin('o.relatedDummy3', 'a122')->shouldBeCalled(1);
154-
$queryBuilderProphecy->leftJoin('o.relatedDummy4', 'a1233')->shouldBeCalled(1);
165+
$queryBuilderProphecy->leftJoin('a0.relation', 'a10')->shouldBeCalled(1);
166+
$queryBuilderProphecy->innerJoin('o.relatedDummy2', 'a111')->shouldBeCalled(1);
167+
$queryBuilderProphecy->innerJoin('o.relatedDummy3', 'a1122')->shouldBeCalled(1);
168+
$queryBuilderProphecy->leftJoin('o.relatedDummy4', 'a11233')->shouldBeCalled(1);
155169
$queryBuilderProphecy->addSelect('partial a0.{id,name}')->shouldBeCalled(1);
156-
$queryBuilderProphecy->addSelect('partial a11.{id,name}')->shouldBeCalled(1);
157-
$queryBuilderProphecy->addSelect('partial a122.{id,name}')->shouldBeCalled(1);
158-
$queryBuilderProphecy->addSelect('partial a1233.{id,name}')->shouldBeCalled(1);
170+
$queryBuilderProphecy->addSelect('partial a10.{id}')->shouldBeCalled(1);
171+
$queryBuilderProphecy->addSelect('partial a111.{id}')->shouldBeCalled(1);
172+
$queryBuilderProphecy->addSelect('partial a1122.{id}')->shouldBeCalled(1);
173+
$queryBuilderProphecy->addSelect('partial a11233.{id}')->shouldBeCalled(1);
159174

160175
$queryBuilderProphecy->getEntityManager()->shouldBeCalled(2)->willReturn($emProphecy->reveal());
161176

162177
$queryBuilder = $queryBuilderProphecy->reveal();
163178
$orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal());
164-
$orderExtensionTest->applyToCollection($queryBuilder, new QueryNameGenerator(), Dummy::class);
165179

166180
$orderExtensionTest->applyToItem($queryBuilder, new QueryNameGenerator(), Dummy::class, []);
167181
}

0 commit comments

Comments
 (0)