From 57afc29ef8b6132b39cb07b5e5c007912aafdc9a Mon Sep 17 00:00:00 2001 From: abluchet Date: Mon, 28 Nov 2016 18:40:30 +0100 Subject: [PATCH 1/2] Revert "Remove partial select" This reverts commit 8efed0bf3dfe7afd6bbed2d1ca83b60c436c2a60. --- .../Orm/Extension/EagerLoadingExtension.php | 33 +++- .../Bundle/Resources/config/doctrine_orm.xml | 1 + .../Extension/EagerLoadingExtensionTest.php | 178 +++++++++++++++--- 3 files changed, 182 insertions(+), 30 deletions(-) diff --git a/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php b/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php index 8102c8e3821..8da4ac22261 100644 --- a/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php +++ b/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php @@ -16,6 +16,7 @@ use ApiPlatform\Core\Exception\ResourceClassNotFoundException; use ApiPlatform\Core\Exception\RuntimeException; use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface; +use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use Doctrine\ORM\Mapping\ClassMetadataInfo; use Doctrine\ORM\QueryBuilder; @@ -35,8 +36,9 @@ final class EagerLoadingExtension implements QueryCollectionExtensionInterface, private $maxJoins; private $forceEager; - public function __construct(PropertyMetadataFactoryInterface $propertyMetadataFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, int $maxJoins = 30, bool $forceEager = true) + public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, int $maxJoins = 30, bool $forceEager = true) { + $this->propertyNameCollectionFactory = $propertyNameCollectionFactory; $this->propertyMetadataFactory = $propertyMetadataFactory; $this->resourceMetadataFactory = $resourceMetadataFactory; $this->maxJoins = $maxJoins; @@ -143,10 +145,39 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt $queryBuilder->{$method}(sprintf('%s.%s', $parentAlias, $association), $associationAlias); ++$joinCount; + try { + $this->addSelect($queryBuilder, $mapping['targetEntity'], $associationAlias, $propertyMetadataOptions); + } catch (ResourceClassNotFoundException $resourceClassNotFoundException) { + continue; + } + $this->joinRelations($queryBuilder, $queryNameGenerator, $mapping['targetEntity'], $forceEager, $associationAlias, $propertyMetadataOptions, $method === 'leftJoin', $joinCount); } } + private function addSelect(QueryBuilder $queryBuilder, string $entity, string $associationAlias, array $propertyMetadataOptions) + { + $select = []; + $entityManager = $queryBuilder->getEntityManager(); + $targetClassMetadata = $entityManager->getClassMetadata($entity); + + foreach ($this->propertyNameCollectionFactory->create($entity) as $property) { + $propertyMetadata = $this->propertyMetadataFactory->create($entity, $property, $propertyMetadataOptions); + + if (true === $propertyMetadata->isIdentifier()) { + $select[] = $property; + continue; + } + + //the field test allows to add methods to a Resource which do not reflect real database fields + if (true === $targetClassMetadata->hasField($property) && true === $propertyMetadata->isReadable()) { + $select[] = $property; + } + } + + $queryBuilder->addSelect(sprintf('partial %s.{%s}', $associationAlias, implode(',', $select))); + } + /** * Gets serializer groups if available, if not it returns the $options array. * diff --git a/src/Bridge/Symfony/Bundle/Resources/config/doctrine_orm.xml b/src/Bridge/Symfony/Bundle/Resources/config/doctrine_orm.xml index 097fe018738..ca72c60149c 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/doctrine_orm.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/doctrine_orm.xml @@ -88,6 +88,7 @@ + %api_platform.eager_loading.max_joins% diff --git a/tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php b/tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php index 26d1dc108a3..93461f3a16b 100644 --- a/tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php +++ b/tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php @@ -16,7 +16,9 @@ use ApiPlatform\Core\Exception\PropertyNotFoundException; use ApiPlatform\Core\Exception\ResourceClassNotFoundException; use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface; +use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface; use ApiPlatform\Core\Metadata\Property\PropertyMetadata; +use ApiPlatform\Core\Metadata\Property\PropertyNameCollection; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy; @@ -38,13 +40,35 @@ public function testApplyToCollection() $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata()); + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + + $relatedNameCollection = new PropertyNameCollection(['id', 'name', 'notindatabase', 'notreadable']); + + $propertyNameCollectionFactoryProphecy->create(RelatedDummy::class)->willReturn($relatedNameCollection)->shouldBeCalled(); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $relationPropertyMetadata = new PropertyMetadata(); $relationPropertyMetadata = $relationPropertyMetadata->withReadableLink(true); - $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy2', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); + $idPropertyMetadata = new PropertyMetadata(); + $idPropertyMetadata = $idPropertyMetadata->withIdentifier(true); + $namePropertyMetadata = new PropertyMetadata(); + $namePropertyMetadata = $namePropertyMetadata->withReadable(true); + $notInDatabasePropertyMetadata = new PropertyMetadata(); + $notInDatabasePropertyMetadata = $notInDatabasePropertyMetadata->withReadable(true); + $notReadablePropertyMetadata = new PropertyMetadata(); + $notReadablePropertyMetadata = $notReadablePropertyMetadata->withReadable(false); + + $propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'id', [])->willReturn($idPropertyMetadata)->shouldBeCalled(); + $propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'name', [])->willReturn($namePropertyMetadata)->shouldBeCalled(); + $propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'notindatabase', [])->willReturn($notInDatabasePropertyMetadata)->shouldBeCalled(); + $propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'notreadable', [])->willReturn($notReadablePropertyMetadata)->shouldBeCalled(); + + $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); + $classMetadataProphecy = $this->prophesize(ClassMetadata::class); $classMetadataProphecy->associationMappings = [ 'relatedDummy' => ['fetch' => 3, 'joinColumns' => [['nullable' => true]], 'targetEntity' => RelatedDummy::class], @@ -52,20 +76,30 @@ public function testApplyToCollection() ]; $relatedClassMetadataProphecy = $this->prophesize(ClassMetadata::class); + + foreach ($relatedNameCollection as $property) { + if ($property !== 'id') { + $relatedClassMetadataProphecy->hasField($property)->willReturn($property !== 'notindatabase')->shouldBeCalled(); + } + } + $relatedClassMetadataProphecy->associationMappings = []; $emProphecy = $this->prophesize(EntityManager::class); $emProphecy->getClassMetadata(Dummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal()); $emProphecy->getClassMetadata(RelatedDummy::class)->shouldBeCalled()->willReturn($relatedClassMetadataProphecy->reveal()); - $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); + $queryBuilderProphecy->leftJoin('o.relatedDummy', 'relatedDummy_a1')->shouldBeCalled(1); $queryBuilderProphecy->innerJoin('o.relatedDummy2', 'relatedDummy2_a2')->shouldBeCalled(1); + $queryBuilderProphecy->addSelect('partial relatedDummy_a1.{id,name}')->shouldBeCalled(1); + $queryBuilderProphecy->addSelect('partial relatedDummy2_a2.{id,name}')->shouldBeCalled(1); - $eagerExtensionTest = new EagerLoadingExtension($propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); - $eagerExtensionTest->applyToCollection($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class); + $queryBuilder = $queryBuilderProphecy->reveal(); + $eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); + $eagerExtensionTest->applyToCollection($queryBuilder, new QueryNameGenerator(), Dummy::class); } public function testApplyToItem() @@ -73,16 +107,40 @@ public function testApplyToItem() $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata()); + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + + $relatedNameCollection = new PropertyNameCollection(['id', 'name', 'notindatabase', 'notreadable', 'relation']); + + $propertyNameCollectionFactoryProphecy->create(RelatedDummy::class)->willReturn($relatedNameCollection)->shouldBeCalled(); + $propertyNameCollectionFactoryProphecy->create(UnknownDummy::class)->willReturn(new PropertyNameCollection(['id']))->shouldBeCalled(); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $relationPropertyMetadata = new PropertyMetadata(); $relationPropertyMetadata = $relationPropertyMetadata->withReadableLink(true); - $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy2', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy3', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy4', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy5', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); + + $idPropertyMetadata = new PropertyMetadata(); + $idPropertyMetadata = $idPropertyMetadata->withIdentifier(true); + $namePropertyMetadata = new PropertyMetadata(); + $namePropertyMetadata = $namePropertyMetadata->withReadable(true); + $notInDatabasePropertyMetadata = new PropertyMetadata(); + $notInDatabasePropertyMetadata = $notInDatabasePropertyMetadata->withReadable(true); + $notReadablePropertyMetadata = new PropertyMetadata(); + $notReadablePropertyMetadata = $notReadablePropertyMetadata->withReadable(false); + + $propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'id', [])->willReturn($idPropertyMetadata)->shouldBeCalled(); + $propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'name', [])->willReturn($namePropertyMetadata)->shouldBeCalled(); + $propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'notindatabase', [])->willReturn($notInDatabasePropertyMetadata)->shouldBeCalled(); + $propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'notreadable', [])->willReturn($notReadablePropertyMetadata)->shouldBeCalled(); $propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'relation', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); + $propertyMetadataFactoryProphecy->create(UnknownDummy::class, 'id', [])->willReturn($idPropertyMetadata)->shouldBeCalled(); + + $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); $classMetadataProphecy = $this->prophesize(ClassMetadata::class); $classMetadataProphecy->associationMappings = [ @@ -94,6 +152,13 @@ public function testApplyToItem() ]; $relatedClassMetadataProphecy = $this->prophesize(ClassMetadata::class); + + foreach ($relatedNameCollection as $property) { + if ($property !== 'id') { + $relatedClassMetadataProphecy->hasField($property)->willReturn($property !== 'notindatabase')->shouldBeCalled(); + } + } + $relatedClassMetadataProphecy->associationMappings = [ 'relation' => ['fetch' => 3, 'joinColumns' => [['nullable' => false]], 'targetEntity' => UnknownDummy::class], ]; @@ -106,7 +171,6 @@ public function testApplyToItem() $emProphecy->getClassMetadata(RelatedDummy::class)->shouldBeCalled()->willReturn($relatedClassMetadataProphecy->reveal()); $emProphecy->getClassMetadata(UnknownDummy::class)->shouldBeCalled()->willReturn($unknownClassMetadataProphecy->reveal()); - $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); $queryBuilderProphecy->leftJoin('o.relatedDummy', 'relatedDummy_a1')->shouldBeCalled(1); @@ -114,9 +178,16 @@ public function testApplyToItem() $queryBuilderProphecy->innerJoin('o.relatedDummy2', 'relatedDummy2_a3')->shouldBeCalled(1); $queryBuilderProphecy->innerJoin('o.relatedDummy3', 'relatedDummy3_a4')->shouldBeCalled(1); $queryBuilderProphecy->leftJoin('o.relatedDummy4', 'relatedDummy4_a5')->shouldBeCalled(1); + $queryBuilderProphecy->addSelect('partial relatedDummy_a1.{id,name}')->shouldBeCalled(1); + $queryBuilderProphecy->addSelect('partial relation_a2.{id}')->shouldBeCalled(1); + $queryBuilderProphecy->addSelect('partial relatedDummy2_a3.{id}')->shouldBeCalled(1); + $queryBuilderProphecy->addSelect('partial relatedDummy3_a4.{id}')->shouldBeCalled(1); + $queryBuilderProphecy->addSelect('partial relatedDummy4_a5.{id}')->shouldBeCalled(1); - $orderExtensionTest = new EagerLoadingExtension($propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); - $orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, []); + $queryBuilder = $queryBuilderProphecy->reveal(); + $orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); + + $orderExtensionTest->applyToItem($queryBuilder, new QueryNameGenerator(), Dummy::class, []); } public function testCreateItemWithOperationName() @@ -124,6 +195,8 @@ public function testCreateItemWithOperationName() $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata()); + $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $propertyMetadataFactoryProphecy->create(Dummy::class, 'foo', ['item_operation_name' => 'item_operation'])->shouldBeCalled()->willReturn(new PropertyMetadata()); @@ -134,12 +207,10 @@ public function testCreateItemWithOperationName() $emProphecy = $this->prophesize(EntityManager::class); $emProphecy->getClassMetadata(Dummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal()); - - $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); - $orderExtensionTest = new EagerLoadingExtension($propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); + $orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); $orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, [], 'item_operation'); } @@ -148,6 +219,8 @@ public function testCreateCollectionWithOperationName() $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata()); + $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $propertyMetadataFactoryProphecy->create(Dummy::class, 'foo', ['collection_operation_name' => 'collection_operation'])->shouldBeCalled()->willReturn(new PropertyMetadata()); @@ -158,12 +231,10 @@ public function testCreateCollectionWithOperationName() $emProphecy = $this->prophesize(EntityManager::class); $emProphecy->getClassMetadata(Dummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal()); - - $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); - $eagerExtensionTest = new EagerLoadingExtension($propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); + $eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); $eagerExtensionTest->applyToCollection($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, 'collection_operation'); } @@ -173,7 +244,7 @@ public function testDenormalizeItemWithCorrectResourceClass() //Dummy is the correct class for the denormalization context serialization groups, and we're fetching RelatedDummy $resourceMetadataFactoryProphecy->create(RelatedDummy::class)->willReturn(new ResourceMetadata())->shouldBeCalled(); $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata())->shouldBeCalled(); - + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $classMetadataProphecy = $this->prophesize(ClassMetadata::class); @@ -181,12 +252,11 @@ public function testDenormalizeItemWithCorrectResourceClass() $emProphecy = $this->prophesize(EntityManager::class); $emProphecy->getClassMetadata(RelatedDummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal()); - $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); - $eagerExtensionTest = new EagerLoadingExtension($propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); + $eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); $eagerExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), RelatedDummy::class, ['id' => 1], 'item_operation', ['resource_class' => Dummy::class]); } @@ -196,7 +266,7 @@ public function testDenormalizeItemWithExistingGroups() //groups exist from the context, we don't need to compute them again $resourceMetadataFactoryProphecy->create(RelatedDummy::class)->willReturn(new ResourceMetadata())->shouldBeCalled(); $resourceMetadataFactoryProphecy->create(Dummy::class)->shouldNotBeCalled(); - + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $classMetadataProphecy = $this->prophesize(ClassMetadata::class); @@ -204,12 +274,11 @@ public function testDenormalizeItemWithExistingGroups() $emProphecy = $this->prophesize(EntityManager::class); $emProphecy->getClassMetadata(RelatedDummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal()); - $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); - $eagerExtensionTest = new EagerLoadingExtension($propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); + $eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); $eagerExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), RelatedDummy::class, ['id' => 1], 'item_operation', ['groups' => 'some_groups']); } @@ -222,14 +291,23 @@ public function testMaxDepthReached() $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata()); + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + + $relatedNameCollection = new PropertyNameCollection(['dummy']); + $dummyNameCollection = new PropertyNameCollection(['relatedDummy']); + + $propertyNameCollectionFactoryProphecy->create(RelatedDummy::class)->willReturn($relatedNameCollection)->shouldBeCalled(); + $propertyNameCollectionFactoryProphecy->create(Dummy::class)->willReturn($dummyNameCollection)->shouldBeCalled(); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $relationPropertyMetadata = new PropertyMetadata(); $relationPropertyMetadata = $relationPropertyMetadata->withReadableLink(true); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); + $relatedPropertyMetadata = new PropertyMetadata(); $relatedPropertyMetadata = $relatedPropertyMetadata->withReadableLink(true); - $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); - $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); $propertyMetadataFactoryProphecy->create(RelatedDummy::class, 'dummy', [])->willReturn($relatedPropertyMetadata)->shouldBeCalled(); $classMetadataProphecy = $this->prophesize(ClassMetadata::class); @@ -249,9 +327,11 @@ public function testMaxDepthReached() $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); + $queryBuilderProphecy->innerJoin(Argument::type('string'), Argument::type('string'))->shouldBeCalled(); + $queryBuilderProphecy->addSelect(Argument::type('string'))->shouldBeCalled(); - $eagerExtensionTest = new EagerLoadingExtension($propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); + $eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false); $eagerExtensionTest->applyToCollection($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class); } @@ -260,12 +340,21 @@ public function testForceEager() $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata()); + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(UnknownDummy::class)->willReturn(new PropertyNameCollection(['id']))->shouldBeCalled(); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $relationPropertyMetadata = new PropertyMetadata(); $relationPropertyMetadata = $relationPropertyMetadata->withReadableLink(true); - $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $idPropertyMetadata = new PropertyMetadata(); + $idPropertyMetadata = $idPropertyMetadata->withIdentifier(true); + + $propertyMetadataFactoryProphecy->create(UnknownDummy::class, 'id', [])->willReturn($idPropertyMetadata)->shouldBeCalled(); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relation', [])->willReturn($relationPropertyMetadata)->shouldBeCalled(); + $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); + $classMetadataProphecy = $this->prophesize(ClassMetadata::class); $classMetadataProphecy->associationMappings = [ 'relation' => ['fetch' => 2, 'targetEntity' => UnknownDummy::class, 'joinColumns' => [['nullable' => false]]], @@ -278,12 +367,13 @@ public function testForceEager() $emProphecy->getClassMetadata(Dummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal()); $emProphecy->getClassMetadata(UnknownDummy::class)->shouldBeCalled()->willReturn($unknownClassMetadataProphecy->reveal()); - $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); + $queryBuilderProphecy->innerJoin('o.relation', 'relation_a1')->shouldBeCalled(1); + $queryBuilderProphecy->addSelect('partial relation_a1.{id}')->shouldBeCalled(1); + $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); - $queryBuilderProphecy->innerJoin('o.relation', 'relation_a1')->shouldBeCalled(1); - $orderExtensionTest = new EagerLoadingExtension($propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true); + $orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true); $orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, []); } @@ -292,6 +382,8 @@ public function testResourceClassNotFoundException() $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata()); + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relation', [])->willThrow(new ResourceClassNotFoundException()); @@ -305,7 +397,7 @@ public function testResourceClassNotFoundException() $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); - $orderExtensionTest = new EagerLoadingExtension($propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true); + $orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true); $orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, []); } @@ -314,6 +406,8 @@ public function testPropertyNotFoundException() $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata()); + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relation', [])->willThrow(new PropertyNotFoundException()); @@ -326,11 +420,37 @@ public function testPropertyNotFoundException() $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); + + $orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true); + $orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, []); + } + + public function testResourceClassNotFoundExceptionPropertyNameCollection() + { + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); + $resourceMetadataFactoryProphecy->create(Dummy::class)->willReturn(new ResourceMetadata()); + + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(UnknownDummy::class)->willThrow(new ResourceClassNotFoundException()); + + $relationPropertyMetadata = new PropertyMetadata(); + $relationPropertyMetadata = $relationPropertyMetadata->withReadableLink(true); + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'relation', [])->willReturn($relationPropertyMetadata); + + $classMetadataProphecy = $this->prophesize(ClassMetadata::class); + $classMetadataProphecy->associationMappings = [ + 'relation' => ['fetch' => 2, 'targetEntity' => UnknownDummy::class, 'joinColumns' => [['nullable' => false]]], + ]; + $emProphecy = $this->prophesize(EntityManager::class); + $emProphecy->getClassMetadata(Dummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal()); + $emProphecy->getClassMetadata(UnknownDummy::class)->shouldBeCalled()->willReturn($classMetadataProphecy->reveal()); $queryBuilderProphecy = $this->prophesize(QueryBuilder::class); $queryBuilderProphecy->getRootAliases()->willReturn(['o']); $queryBuilderProphecy->getEntityManager()->willReturn($emProphecy); + $queryBuilderProphecy->innerJoin('o.relation', 'relation_a1')->shouldBeCalled(1); - $orderExtensionTest = new EagerLoadingExtension($propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true); + $orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, true); $orderExtensionTest->applyToItem($queryBuilderProphecy->reveal(), new QueryNameGenerator(), Dummy::class, []); } } From c91849592705481b67d29624603c8e904bb3811e Mon Sep 17 00:00:00 2001 From: abluchet Date: Tue, 29 Nov 2016 10:52:42 +0100 Subject: [PATCH 2/2] fix ManyToMany should be leftJoin --- src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php | 4 ++-- .../Doctrine/Orm/Extension/EagerLoadingExtensionTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php b/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php index 8da4ac22261..b7e9c9b3078 100644 --- a/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php +++ b/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php @@ -134,8 +134,8 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt continue; } - $joinColumns = $mapping['joinColumns'] ?? $mapping['joinTable']['joinColumns'] ?? null; - if (false !== $wasLeftJoin || !isset($joinColumns[0]['nullable']) || false !== $joinColumns[0]['nullable']) { + $isNullable = $mapping['joinColumns'][0]['nullable'] ?? true; + if (false !== $wasLeftJoin || true === $isNullable) { $method = 'leftJoin'; } else { $method = 'innerJoin'; diff --git a/tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php b/tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php index 93461f3a16b..131835328bd 100644 --- a/tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php +++ b/tests/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php @@ -176,7 +176,7 @@ public function testApplyToItem() $queryBuilderProphecy->leftJoin('o.relatedDummy', 'relatedDummy_a1')->shouldBeCalled(1); $queryBuilderProphecy->leftJoin('relatedDummy_a1.relation', 'relation_a2')->shouldBeCalled(1); $queryBuilderProphecy->innerJoin('o.relatedDummy2', 'relatedDummy2_a3')->shouldBeCalled(1); - $queryBuilderProphecy->innerJoin('o.relatedDummy3', 'relatedDummy3_a4')->shouldBeCalled(1); + $queryBuilderProphecy->leftJoin('o.relatedDummy3', 'relatedDummy3_a4')->shouldBeCalled(1); $queryBuilderProphecy->leftJoin('o.relatedDummy4', 'relatedDummy4_a5')->shouldBeCalled(1); $queryBuilderProphecy->addSelect('partial relatedDummy_a1.{id,name}')->shouldBeCalled(1); $queryBuilderProphecy->addSelect('partial relation_a2.{id}')->shouldBeCalled(1);