Skip to content

Fix duplicate data in Eager loads relations #2183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 4, 2018
Merged

Fix duplicate data in Eager loads relations #2183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cce3781
Fix duplicate data in Eager loads relations
ilicmsreten Aug 30, 2018
0bd49ab
Comment the re-hydration
soyuka Sep 4, 2018
b840fc4
fix cs
soyuka Sep 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,9 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt

$this->joinRelations($queryBuilder, $queryNameGenerator, $mapping['targetEntity'], $forceEager, $fetchPartial, $associationAlias, $options, $normalizationContext, 'leftJoin' === $method, $joinCount, $currentDepth);
}

// result is discarded (this is just re-hydrating the collections) see http://ocramius.github.io/blog/doctrine-orm-optimization-hydration/
$queryBuilder->getQuery()->getResult();
}

private function addSelect(QueryBuilder $queryBuilder, string $entity, string $associationAlias, array $propertyMetadataOptions)
Expand Down
41 changes: 41 additions & 0 deletions tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\EmbeddableDummy;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\UnknownDummy;
use Doctrine\ORM\AbstractQuery;
use Doctrine\ORM\EntityManager;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataInfo;
Expand Down Expand Up @@ -122,6 +123,9 @@ public function testApplyToCollection()
$queryBuilderProphecy->addSelect('partial relatedDummy_a1.{id,name,embeddedDummy.name}')->shouldBeCalled(1);
$queryBuilderProphecy->addSelect('partial relatedDummy2_a2.{id,name,embeddedDummy.name}')->shouldBeCalled(1);

$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(3);

$queryBuilder = $queryBuilderProphecy->reveal();
$eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false, null, null, true);
$eagerExtensionTest->applyToCollection($queryBuilder, new QueryNameGenerator(), Dummy::class, null, $context);
Expand Down Expand Up @@ -228,6 +232,9 @@ public function testApplyToItem()
$queryBuilderProphecy->addSelect('partial relatedDummy4_a5.{id}')->shouldBeCalled(1);
$queryBuilderProphecy->addSelect('singleInheritanceRelation_a6')->shouldBeCalled(1);

$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(7);

$queryBuilder = $queryBuilderProphecy->reveal();
$orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false, null, null, true);

Expand All @@ -253,6 +260,8 @@ public function testCreateItemWithOperationName()
$emProphecy->getClassMetadata(Dummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal());
$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(1);

$orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false, null, null, true);
$orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, [], 'item_operation', ['groups' => ['foo']]);
Expand All @@ -277,6 +286,8 @@ public function testCreateCollectionWithOperationName()
$emProphecy->getClassMetadata(Dummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal());
$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(1);

$eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false, null, null, true);
$eagerExtensionTest->applyToCollection($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, 'collection_operation', ['groups' => ['foo']]);
Expand All @@ -300,6 +311,8 @@ public function testDenormalizeItemWithCorrectResourceClass()
$queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(1);

$eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false, null, null, true);
$eagerExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), RelatedDummy::class, ['id' => 1], 'item_operation', ['resource_class' => Dummy::class]);
Expand All @@ -322,6 +335,8 @@ public function testDenormalizeItemWithExistingGroups()
$queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(1);

$eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false, null, null, true);
$eagerExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), RelatedDummy::class, ['id' => 1], 'item_operation', [AbstractNormalizer::GROUPS => 'some_groups']);
Expand Down Expand Up @@ -442,6 +457,8 @@ public function testMaxDepth()

$queryBuilderProphecy->innerJoin(Argument::type('string'), Argument::type('string'))->shouldBeCalledTimes(2);
$queryBuilderProphecy->addSelect(Argument::type('string'))->shouldBeCalled();
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(3);

$eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false, null, null, true, $classMetadataFactoryProphecy->reveal());
$eagerExtensionTest->applyToCollection($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class);
Expand Down Expand Up @@ -487,6 +504,9 @@ public function testForceEager()
$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);

$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(2);

$orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true, null, null, true);
$orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, []);
}
Expand Down Expand Up @@ -523,6 +543,9 @@ public function testExtraLazy()
$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);

$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(1);

$orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true, null, null, true);
$orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, []);
}
Expand All @@ -546,6 +569,8 @@ public function testResourceClassNotFoundException()
$queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(1);

$orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true, null, null, true);
$orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, [], null);
Expand All @@ -570,6 +595,8 @@ public function testPropertyNotFoundException()
$queryBuilderProphecy = $this->prophesize(QueryBuilder::class);
$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(1);

$orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true, null, null, true);
$orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, []);
Expand Down Expand Up @@ -599,6 +626,8 @@ public function testResourceClassNotFoundExceptionPropertyNameCollection()
$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);
$queryBuilderProphecy->innerJoin('o.relation', 'relation_a1')->shouldBeCalled(1);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(1);

$orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true, null, null, true);
$orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, []);
Expand Down Expand Up @@ -654,6 +683,8 @@ public function testApplyToCollectionWithSerializerContextBuilder()

$queryBuilderProphecy->leftJoin('o.relatedDummy', 'relatedDummy_a1')->shouldBeCalled(1);
$queryBuilderProphecy->addSelect('partial relatedDummy_a1.{id,name}')->shouldBeCalled(1);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(2);

$request = Request::create('/api/dummies', 'GET', []);

Expand Down Expand Up @@ -718,6 +749,8 @@ public function testAttributes()

$queryBuilderProphecy->leftJoin('o.relatedDummy', 'relatedDummy_a1')->shouldBeCalled(1);
$queryBuilderProphecy->addSelect('partial relatedDummy_a1.{id,name}')->shouldBeCalled(1);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(2);

$request = Request::create('/api/dummies', 'GET', []);

Expand Down Expand Up @@ -759,6 +792,8 @@ public function testNotInAttributes()

$queryBuilderProphecy->getRootAliases()->willReturn(['o']);
$queryBuilderProphecy->getEntityManager()->willReturn($emProphecy);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(1);

$request = Request::create('/api/dummies', 'GET', []);

Expand Down Expand Up @@ -808,6 +843,8 @@ public function testApplyToCollectionNoPartial()
$queryBuilderProphecy->innerJoin('o.relatedDummy2', 'relatedDummy2_a2')->shouldBeCalled(1);
$queryBuilderProphecy->addSelect('relatedDummy_a1')->shouldBeCalled(1);
$queryBuilderProphecy->addSelect('relatedDummy2_a2')->shouldBeCalled(1);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(3);

$queryBuilder = $queryBuilderProphecy->reveal();
$eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30);
Expand Down Expand Up @@ -864,6 +901,8 @@ private function doTestApplyToCollectionWithANonRedableButFetchEagerProperty(boo
$queryBuilderProphecy->innerJoin('o.relatedDummy2', 'relatedDummy2_a2')->shouldBeCalled(1);
$queryBuilderProphecy->addSelect('relatedDummy_a1')->shouldBeCalled(1);
$queryBuilderProphecy->addSelect('relatedDummy2_a2')->shouldBeCalled(1);
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(3);

$queryBuilder = $queryBuilderProphecy->reveal();
$eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30);
Expand Down Expand Up @@ -918,6 +957,8 @@ private function doTestApplyToCollectionWithARedableButNotFetchEagerProperty(boo
$queryBuilderProphecy->innerJoin('o.relatedDummy2', 'relatedDummy2_a2')->shouldNotBeCalled();
$queryBuilderProphecy->addSelect('relatedDummy_a1')->shouldNotBeCalled();
$queryBuilderProphecy->addSelect('relatedDummy2_a2')->shouldNotBeCalled();
$queryAbstractProphecy = $this->prophesize(AbstractQuery::class);
$queryBuilderProphecy->getQuery()->willReturn($queryAbstractProphecy)->shouldBeCalledTimes(1);

$queryBuilder = $queryBuilderProphecy->reveal();
$eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30);
Expand Down