Skip to content

Commit d3ee559

Browse files
soyukasoyuka
authored
Merge pull request #1475 from soyuka/fix-filter-eager-loading
Fix filter eager loading & merge #1449 + test
2 parents ee30666 + 526edc5 commit d3ee559

File tree

6 files changed

+100
-8
lines changed

6 files changed

+100
-8
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,11 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
6969
$queryBuilderClone->andWhere($queryBuilderClone->expr()->in($originAlias, $in->getDQL()));
7070
} else {
7171
// 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
72-
foreach ($classMetadata->identifier as $identifier) {
72+
foreach ($classMetadata->getIdentifier() as $identifier) {
73+
if (!$classMetadata->hasAssociation($identifier)) {
74+
continue;
75+
}
76+
7377
$replacementAlias = $queryNameGenerator->generateJoinAlias($originAlias);
7478
$in = $this->getQueryBuilderWithNewAliases($queryBuilder, $queryNameGenerator, $originAlias, $replacementAlias);
7579
$in->select("IDENTITY($replacementAlias.$identifier)");

‎src/Bridge/Doctrine/Orm/Util/EagerLoadingTrait.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ private function hasFetchEagerAssociation(EntityManager $em, ClassMetadataInfo $
9191
{
9292
$checked[] = $classMetadata->name;
9393

94-
foreach ($classMetadata->associationMappings as $mapping) {
94+
foreach ($classMetadata->getAssociationMappings() as $mapping) {
9595
if (ClassMetadataInfo::FETCH_EAGER === $mapping['fetch']) {
9696
return true;
9797
}

‎src/Bridge/Doctrine/Orm/Util/QueryChecker.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,16 @@ public static function hasOrderByOnToManyJoin(QueryBuilder $queryBuilder, Manage
146146
if (!isset($orderByAliases[$alias])) {
147147
continue;
148148
}
149-
150149
$relationship = QueryJoinParser::getJoinRelationship($join);
151150

152151
if (false !== strpos($relationship, '.')) {
153-
$metadata = QueryJoinParser::getClassMetadataFromJoinAlias($alias, $queryBuilder, $managerRegistry);
154-
if ($metadata->isCollectionValuedAssociation($relationship)) {
152+
/*
153+
* We select the parent alias because it may differ from the origin alias given above
154+
* @see https://github.com/api-platform/core/issues/1313
155+
*/
156+
list($relationAlias, $association) = explode('.', $relationship);
157+
$metadata = QueryJoinParser::getClassMetadataFromJoinAlias($relationAlias, $queryBuilder, $managerRegistry);
158+
if ($metadata->isCollectionValuedAssociation($association)) {
155159
return true;
156160
}
157161
} else {

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

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,9 +246,14 @@ public function testCompositeIdentifiers()
246246
$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
247247
$resourceMetadataFactoryProphecy->create(CompositeRelation::class)->willReturn(new ResourceMetadata(CompositeRelation::class));
248248

249-
$classMetadata = new ClassMetadataInfo(CompositeRelation::class);
249+
$classMetadataProphecy = $this->prophesize(ClassMetadataInfo::class);
250+
$classMetadataProphecy->getIdentifier()->willReturn(['item', 'label']);
251+
$classMetadataProphecy->getAssociationMappings()->willReturn(['item' => ['fetch' => ClassMetadataInfo::FETCH_EAGER]]);
252+
$classMetadataProphecy->hasAssociation('item')->shouldBeCalled()->willReturn(true);
253+
$classMetadataProphecy->hasAssociation('label')->shouldBeCalled()->willReturn(true);
254+
255+
$classMetadata = $classMetadataProphecy->reveal();
250256
$classMetadata->isIdentifierComposite = true;
251-
$classMetadata->identifier = ['item', 'label'];
252257

253258
$em = $this->prophesize(EntityManager::class);
254259
$em->getExpressionBuilder()->shouldBeCalled()->willReturn(new Expr());
@@ -353,6 +358,64 @@ public function testFetchEagerWithNoForceEager()
353358
$this->assertEquals($this->toDQLString($expected), $qb->getDQL());
354359
}
355360

361+
public function testCompositeIdentifiersWithoutAssociation()
362+
{
363+
$resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
364+
$resourceMetadataFactoryProphecy->create(CompositeRelation::class)->willReturn(new ResourceMetadata(CompositeRelation::class));
365+
366+
$classMetadataProphecy = $this->prophesize(ClassMetadataInfo::class);
367+
$classMetadataProphecy->getIdentifier()->willReturn(['item', 'label', 'bar']);
368+
$classMetadataProphecy->getAssociationMappings()->willReturn(['item' => ['fetch' => ClassMetadataInfo::FETCH_EAGER]]);
369+
$classMetadataProphecy->hasAssociation('item')->shouldBeCalled()->willReturn(true);
370+
$classMetadataProphecy->hasAssociation('label')->shouldBeCalled()->willReturn(true);
371+
$classMetadataProphecy->hasAssociation('bar')->shouldBeCalled()->willReturn(false);
372+
373+
$classMetadata = $classMetadataProphecy->reveal();
374+
$classMetadata->isIdentifierComposite = true;
375+
376+
$em = $this->prophesize(EntityManager::class);
377+
$em->getExpressionBuilder()->shouldBeCalled()->willReturn(new Expr());
378+
$em->getClassMetadata(CompositeRelation::class)->shouldBeCalled()->willReturn($classMetadata);
379+
380+
$qb = new QueryBuilder($em->reveal());
381+
382+
$qb->select('o')
383+
->from(CompositeRelation::class, 'o')
384+
->innerJoin('o.compositeItem', 'item')
385+
->innerJoin('o.compositeLabel', 'label')
386+
->where('item.field1 = :foo AND o.bar = :bar')
387+
->setParameter('foo', 1)
388+
->setParameter('bar', 2);
389+
390+
$queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
391+
$queryNameGenerator->generateJoinAlias('item')->shouldBeCalled()->willReturn('item_2');
392+
$queryNameGenerator->generateJoinAlias('label')->shouldBeCalled()->willReturn('label_2');
393+
$queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
394+
395+
$filterEagerLoadingExtension = new FilterEagerLoadingExtension($resourceMetadataFactoryProphecy->reveal(), true);
396+
$filterEagerLoadingExtension->applyToCollection($qb, $queryNameGenerator->reveal(), CompositeRelation::class, 'get');
397+
398+
$expected = <<<SQL
399+
SELECT o
400+
FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeRelation o
401+
INNER JOIN o.compositeItem item
402+
INNER JOIN o.compositeLabel label
403+
WHERE (o.item IN(
404+
SELECT IDENTITY(o_2.item) FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeRelation o_2
405+
INNER JOIN o_2.compositeItem item_2
406+
INNER JOIN o_2.compositeLabel label_2
407+
WHERE item_2.field1 = :foo AND o_2.bar = :bar
408+
)) AND (o.label IN(
409+
SELECT IDENTITY(o_2.label) FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeRelation o_2
410+
INNER JOIN o_2.compositeItem item_2
411+
INNER JOIN o_2.compositeLabel label_2
412+
WHERE item_2.field1 = :foo AND o_2.bar = :bar
413+
))
414+
SQL;
415+
416+
$this->assertEquals($this->toDQLString($expected), $qb->getDQL());
417+
}
418+
356419
private function toDQLString(string $dql): string
357420
{
358421
return preg_replace(['/\s+/', '/\(\s/', '/\s\)/'], [' ', '(', ')'], $dql);

‎tests/Bridge/Doctrine/Orm/Util/QueryCheckerTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,4 +176,25 @@ public function testHasOrderByOnToManyJoinWithClassLeftJoin()
176176

177177
$this->assertTrue(QueryChecker::hasOrderByOnToManyJoin($queryBuilder->reveal(), $managerRegistry->reveal()));
178178
}
179+
180+
/**
181+
* Adds a test on the fix referenced in https://github.com/api-platform/core/pull/1449.
182+
*/
183+
public function testOrderByOnToManyWithRelationAsBasis()
184+
{
185+
$queryBuilder = $this->prophesize(QueryBuilder::class);
186+
$queryBuilder->getRootEntities()->willReturn(['Dummy']);
187+
$queryBuilder->getRootAliases()->willReturn(['d']);
188+
$queryBuilder->getDQLPart('join')->willReturn(['d' => [new Join('LEFT_JOIN', 'd.relatedDummy', 'a_1')]]);
189+
$queryBuilder->getDQLPart('orderBy')->willReturn(['a_1.name' => new OrderBy('a_1.name', 'asc')]);
190+
$classMetadata = $this->prophesize(ClassMetadata::class);
191+
$classMetadata = $this->prophesize(ClassMetadata::class);
192+
$classMetadata->isCollectionValuedAssociation('relatedDummy')->willReturn(true);
193+
$objectManager = $this->prophesize(ObjectManager::class);
194+
$objectManager->getClassMetadata('Dummy')->willReturn($classMetadata->reveal());
195+
$managerRegistry = $this->prophesize(ManagerRegistry::class);
196+
$managerRegistry->getManagerForClass('Dummy')->willReturn($objectManager->reveal());
197+
198+
$this->assertTrue(QueryChecker::hasOrderByOnToManyJoin($queryBuilder->reveal(), $managerRegistry->reveal()));
199+
}
179200
}

‎tests/Bridge/Doctrine/Orm/Util/QueryJoinParserTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public function testGetClassMetadataFromJoinAlias()
3333
$queryBuilder = $this->prophesize(QueryBuilder::class);
3434
$queryBuilder->getRootEntities()->willReturn(['Dummy']);
3535
$queryBuilder->getRootAliases()->willReturn(['d']);
36-
$queryBuilder->getDQLPart('join')->willReturn(['a_1' => new Join('INNER_JOIN', 'relatedDummy', 'a_1', null, 'a_1.name = r.name')]);
36+
$queryBuilder->getDQLPart('join')->willReturn(['a_1' => [new Join('INNER_JOIN', 'relatedDummy', 'a_1', null, 'a_1.name = r.name')]]);
3737
$classMetadata = $this->prophesize(ClassMetadata::class);
3838
$objectManager = $this->prophesize(ObjectManager::class);
3939
$objectManager->getClassMetadata('Dummy')->willReturn($classMetadata->reveal());

0 commit comments

Comments
 (0)