Skip to content

Commit 9a63fdc

Browse files
soyukasoyuka
authored
Merge pull request #1022 from soyuka/hotfix/filter-eager-loading-order-by-hidden
Fix #1021 order by hidden eager loading
2 parents dfce158 + 7a59035 commit 9a63fdc

File tree

2 files changed

+165
-3
lines changed

2 files changed

+165
-3
lines changed

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,31 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
4848
}
4949

5050
$joinParts = $queryBuilder->getDQLPart('join');
51+
$originAlias = 'o';
5152

52-
if (!$joinParts || !isset($joinParts['o'])) {
53+
if (!$joinParts || !isset($joinParts[$originAlias])) {
5354
return;
5455
}
5556

5657
$queryBuilderClone = clone $queryBuilder;
5758
$queryBuilderClone->resetDQLPart('where');
58-
$queryBuilderClone->andWhere($queryBuilderClone->expr()->in('o', $this->getQueryBuilderWithNewAliases($queryBuilder, $queryNameGenerator)->getDQL()));
59+
60+
$classMetadata = $queryBuilder->getEntityManager()->getClassMetadata($resourceClass);
61+
62+
if (!$classMetadata->isIdentifierComposite) {
63+
$replacementAlias = $queryNameGenerator->generateJoinAlias($originAlias);
64+
$in = $this->getQueryBuilderWithNewAliases($queryBuilder, $queryNameGenerator, $originAlias, $replacementAlias);
65+
$in->select($replacementAlias);
66+
$queryBuilderClone->andWhere($queryBuilderClone->expr()->in($originAlias, $in->getDQL()));
67+
} else {
68+
// Because Doctrine doesn't support WHERE ( foo, bar ) IN () (https://github.com/doctrine/doctrine2/issues/5238), we are building as many subqueries as they are identifiers
69+
foreach ($classMetadata->identifier as $identifier) {
70+
$replacementAlias = $queryNameGenerator->generateJoinAlias($originAlias);
71+
$in = $this->getQueryBuilderWithNewAliases($queryBuilder, $queryNameGenerator, $originAlias, $replacementAlias);
72+
$in->select("IDENTITY($replacementAlias.$identifier)");
73+
$queryBuilderClone->andWhere($queryBuilderClone->expr()->in("$originAlias.$identifier", $in->getDQL()));
74+
}
75+
}
5976

6077
$queryBuilder->resetDQLPart('where');
6178
$queryBuilder->add('where', $queryBuilderClone->getDQLPart('where'));
@@ -72,14 +89,16 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
7289
private function getQueryBuilderWithNewAliases(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $originAlias = 'o', string $replacement = 'o_2')
7390
{
7491
$queryBuilderClone = clone $queryBuilder;
75-
$queryBuilderClone->select($replacement);
7692

7793
$joinParts = $queryBuilder->getDQLPart('join');
7894
$wherePart = $queryBuilder->getDQLPart('where');
7995

8096
//reset parts
8197
$queryBuilderClone->resetDQLPart('join');
8298
$queryBuilderClone->resetDQLPart('where');
99+
$queryBuilderClone->resetDQLPart('orderBy');
100+
$queryBuilderClone->resetDQLPart('groupBy');
101+
$queryBuilderClone->resetDQLPart('having');
83102

84103
//Change from alias
85104
$from = $queryBuilderClone->getDQLPart('from')[0];

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

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
1616
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
1717
use ApiPlatform\Core\Metadata\Resource\ResourceMetadata;
18+
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeRelation;
1819
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar;
1920
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Foo;
2021
use Doctrine\ORM\EntityManager;
22+
use Doctrine\ORM\Mapping\ClassMetadataInfo;
2123
use Doctrine\ORM\Query\Expr;
2224
use Doctrine\ORM\QueryBuilder;
2325

@@ -112,6 +114,7 @@ public function testApplyCollection()
112114

113115
$em = $this->prophesize(EntityManager::class);
114116
$em->getExpressionBuilder()->shouldBeCalled()->willReturn(new Expr());
117+
$em->getClassMetadata(DummyCar::class)->shouldBeCalled()->willReturn(new ClassMetadataInfo(DummyCar::class));
115118

116119
$qb = new QueryBuilder($em->reveal());
117120

@@ -123,10 +126,150 @@ public function testApplyCollection()
123126

124127
$queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
125128
$queryNameGenerator->generateJoinAlias('colors')->shouldBeCalled()->willReturn('colors_2');
129+
$queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
126130

127131
$filterEagerLoadingExtension = new FilterEagerLoadingExtension($resourceMetadataFactoryProphecy->reveal(), true);
128132
$filterEagerLoadingExtension->applyToCollection($qb, $queryNameGenerator->reveal(), DummyCar::class, 'get');
129133

130134
$this->assertEquals('SELECT o FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar o LEFT JOIN o.colors colors WHERE o IN(SELECT o_2 FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar o_2 LEFT JOIN o_2.colors colors_2 WHERE o_2.colors = :foo)', $qb->getDQL());
131135
}
136+
137+
/**
138+
* https://github.com/api-platform/core/issues/1021.
139+
*/
140+
public function testHiddenOrderBy()
141+
{
142+
$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
143+
$resourceMetadataFactoryProphecy->create(DummyCar::class)->willReturn(new ResourceMetadata(DummyCar::class));
144+
145+
$em = $this->prophesize(EntityManager::class);
146+
$em->getExpressionBuilder()->shouldBeCalled()->willReturn(new Expr());
147+
$em->getClassMetadata(DummyCar::class)->shouldBeCalled()->willReturn(new ClassMetadataInfo(DummyCar::class));
148+
149+
$qb = new QueryBuilder($em->reveal());
150+
151+
$qb->select('o', 'CASE WHEN o.dateCreated IS NULL THEN 0 ELSE 1 END AS HIDDEN _o_dateCreated_null_rank')
152+
->from(DummyCar::class, 'o')
153+
->leftJoin('o.colors', 'colors')
154+
->where('o.colors = :foo')
155+
->orderBy('_o_dateCreated_null_rank DESC')
156+
->setParameter('foo', 1);
157+
158+
$queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
159+
$queryNameGenerator->generateJoinAlias('colors')->shouldBeCalled()->willReturn('colors_2');
160+
$queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
161+
$filterEagerLoadingExtension = new FilterEagerLoadingExtension($resourceMetadataFactoryProphecy->reveal(), true);
162+
$filterEagerLoadingExtension->applyToCollection($qb, $queryNameGenerator->reveal(), DummyCar::class, 'get');
163+
164+
$expected = <<<SQL
165+
SELECT o, CASE WHEN o.dateCreated IS NULL THEN 0 ELSE 1 END AS HIDDEN _o_dateCreated_null_rank
166+
FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar o
167+
LEFT JOIN o.colors colors
168+
WHERE o IN(
169+
SELECT o_2 FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar o_2
170+
LEFT JOIN o_2.colors colors_2
171+
WHERE o_2.colors = :foo
172+
) ORDER BY _o_dateCreated_null_rank DESC ASC
173+
SQL;
174+
175+
$this->assertEquals($this->toDQLString($expected), $qb->getDQL());
176+
}
177+
178+
public function testGroupBy()
179+
{
180+
$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
181+
$resourceMetadataFactoryProphecy->create(DummyCar::class)->willReturn(new ResourceMetadata(DummyCar::class));
182+
183+
$em = $this->prophesize(EntityManager::class);
184+
$em->getExpressionBuilder()->shouldBeCalled()->willReturn(new Expr());
185+
$em->getClassMetadata(DummyCar::class)->shouldBeCalled()->willReturn(new ClassMetadataInfo(DummyCar::class));
186+
187+
$qb = new QueryBuilder($em->reveal());
188+
189+
$qb->select('o', 'count(o.id) as counter')
190+
->from(DummyCar::class, 'o')
191+
->leftJoin('o.colors', 'colors')
192+
->where('o.colors = :foo')
193+
->orderBy('o.colors')
194+
->groupBy('o.colors')
195+
->having('counter > 3')
196+
->setParameter('foo', 1);
197+
198+
$queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
199+
$queryNameGenerator->generateJoinAlias('colors')->shouldBeCalled()->willReturn('colors_2');
200+
$queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
201+
$filterEagerLoadingExtension = new FilterEagerLoadingExtension($resourceMetadataFactoryProphecy->reveal(), true);
202+
$filterEagerLoadingExtension->applyToCollection($qb, $queryNameGenerator->reveal(), DummyCar::class, 'get');
203+
204+
$expected = <<<SQL
205+
SELECT o, count(o.id) as counter
206+
FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar o
207+
LEFT JOIN o.colors colors WHERE o
208+
IN(
209+
SELECT o_2 FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar o_2
210+
LEFT JOIN o_2.colors colors_2
211+
WHERE o_2.colors = :foo
212+
)
213+
GROUP BY o.colors HAVING counter > 3
214+
ORDER BY o.colors ASC
215+
SQL;
216+
217+
$this->assertEquals($this->toDQLString($expected), $qb->getDQL());
218+
}
219+
220+
public function testCompositeIdentifiers()
221+
{
222+
$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
223+
$resourceMetadataFactoryProphecy->create(CompositeRelation::class)->willReturn(new ResourceMetadata(CompositeRelation::class));
224+
225+
$classMetadata = new ClassMetadataInfo(CompositeRelation::class);
226+
$classMetadata->isIdentifierComposite = true;
227+
$classMetadata->identifier = ['item', 'label'];
228+
229+
$em = $this->prophesize(EntityManager::class);
230+
$em->getExpressionBuilder()->shouldBeCalled()->willReturn(new Expr());
231+
$em->getClassMetadata(CompositeRelation::class)->shouldBeCalled()->willReturn($classMetadata);
232+
233+
$qb = new QueryBuilder($em->reveal());
234+
235+
$qb->select('o')
236+
->from(CompositeRelation::class, 'o')
237+
->innerJoin('o.compositeItem', 'item')
238+
->innerJoin('o.compositeLabel', 'label')
239+
->where('item.field1 = :foo')
240+
->setParameter('foo', 1);
241+
242+
$queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
243+
$queryNameGenerator->generateJoinAlias('item')->shouldBeCalled()->willReturn('item_2');
244+
$queryNameGenerator->generateJoinAlias('label')->shouldBeCalled()->willReturn('label_2');
245+
$queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
246+
247+
$filterEagerLoadingExtension = new FilterEagerLoadingExtension($resourceMetadataFactoryProphecy->reveal(), true);
248+
$filterEagerLoadingExtension->applyToCollection($qb, $queryNameGenerator->reveal(), CompositeRelation::class, 'get');
249+
250+
$expected = <<<SQL
251+
SELECT o
252+
FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeRelation o
253+
INNER JOIN o.compositeItem item
254+
INNER JOIN o.compositeLabel label
255+
WHERE o.item IN(
256+
SELECT IDENTITY(o_2.item) FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeRelation o_2
257+
INNER JOIN o_2.compositeItem item_2
258+
INNER JOIN o_2.compositeLabel label_2
259+
WHERE item_2.field1 = :foo
260+
) AND o.label IN(
261+
SELECT IDENTITY(o_2.label) FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeRelation o_2
262+
INNER JOIN o_2.compositeItem item_2
263+
INNER JOIN o_2.compositeLabel label_2
264+
WHERE item_2.field1 = :foo
265+
)
266+
SQL;
267+
268+
$this->assertEquals($this->toDQLString($expected), $qb->getDQL());
269+
}
270+
271+
private function toDQLString(string $dql): string
272+
{
273+
return preg_replace('/\\r\\n|\\n/', '', str_replace(' ', '', $dql));
274+
}
132275
}

0 commit comments

Comments
 (0)