diff --git a/features/doctrine/eager_loading.feature b/features/doctrine/eager_loading.feature index 06ce1baf36f..27ab9804abe 100644 --- a/features/doctrine/eager_loading.feature +++ b/features/doctrine/eager_loading.feature @@ -11,11 +11,12 @@ Feature: Eager Loading Then the response status code should be 200 And the DQL should be equal to: """ - SELECT o, thirdLevel_a1, relatedToDummyFriend_a2, dummyFriend_a3 + SELECT o, thirdLevel_a1, fourthLevel_a2, relatedToDummyFriend_a3, dummyFriend_a4 FROM ApiPlatform\Tests\Fixtures\TestBundle\Entity\RelatedDummy o LEFT JOIN o.thirdLevel thirdLevel_a1 - LEFT JOIN o.relatedToDummyFriend relatedToDummyFriend_a2 - LEFT JOIN relatedToDummyFriend_a2.dummyFriend dummyFriend_a3 + LEFT JOIN thirdLevel_a1.fourthLevel fourthLevel_a2 + LEFT JOIN o.relatedToDummyFriend relatedToDummyFriend_a3 + LEFT JOIN relatedToDummyFriend_a3.dummyFriend dummyFriend_a4 WHERE o.id = :id_id """ @@ -45,11 +46,12 @@ Feature: Eager Loading Then the response status code should be 200 And the DQL should be equal to: """ - SELECT o, thirdLevel_a4, relatedToDummyFriend_a1, dummyFriend_a5 + SELECT o, thirdLevel_a4, fourthLevel_a5, relatedToDummyFriend_a1, dummyFriend_a6 FROM ApiPlatform\Tests\Fixtures\TestBundle\Entity\RelatedDummy o INNER JOIN o.relatedToDummyFriend relatedToDummyFriend_a1 LEFT JOIN o.thirdLevel thirdLevel_a4 - INNER JOIN relatedToDummyFriend_a1.dummyFriend dummyFriend_a5 + LEFT JOIN thirdLevel_a4.fourthLevel fourthLevel_a5 + INNER JOIN relatedToDummyFriend_a1.dummyFriend dummyFriend_a6 WHERE o IN( SELECT o_a2 FROM ApiPlatform\Tests\Fixtures\TestBundle\Entity\RelatedDummy o_a2 @@ -81,11 +83,12 @@ Feature: Eager Loading Then the response status code should be 200 And the DQL should be equal to: """ - SELECT o, thirdLevel_a3, relatedToDummyFriend_a4, dummyFriend_a5 + SELECT o, thirdLevel_a3, fourthLevel_a4, relatedToDummyFriend_a5, dummyFriend_a6 FROM ApiPlatform\Tests\Fixtures\TestBundle\Entity\RelatedDummy o LEFT JOIN o.thirdLevel thirdLevel_a3 - LEFT JOIN o.relatedToDummyFriend relatedToDummyFriend_a4 - LEFT JOIN relatedToDummyFriend_a4.dummyFriend dummyFriend_a5 + LEFT JOIN thirdLevel_a3.fourthLevel fourthLevel_a4 + LEFT JOIN o.relatedToDummyFriend relatedToDummyFriend_a5 + LEFT JOIN relatedToDummyFriend_a5.dummyFriend dummyFriend_a6 WHERE o.id IN ( SELECT related_dummy_a1.id FROM ApiPlatform\Tests\Fixtures\TestBundle\Entity\RelatedDummy related_dummy_a1 diff --git a/src/Core/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php b/src/Core/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php index da05190e554..9476f7b087f 100644 --- a/src/Core/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php +++ b/src/Core/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php @@ -28,8 +28,10 @@ use ApiPlatform\Exception\RuntimeException; use ApiPlatform\Metadata\ApiProperty; use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; +use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\ClassMetadataInfo; use Doctrine\ORM\Query\Expr\Join; +use Doctrine\ORM\Query\Expr\Select; use Doctrine\ORM\QueryBuilder; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface; @@ -167,7 +169,7 @@ private function apply(bool $collection, QueryBuilder $queryBuilder, QueryNameGe * * @throws RuntimeException when the max number of joins has been reached */ - private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, bool $forceEager, bool $fetchPartial, string $parentAlias, array $options = [], array $normalizationContext = [], bool $wasLeftJoin = false, int &$joinCount = 0, int $currentDepth = null) + private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, bool $forceEager, bool $fetchPartial, string $parentAlias, array $options = [], array $normalizationContext = [], bool $wasLeftJoin = false, int &$joinCount = 0, int $currentDepth = null, string $parentAssociation = null) { if ($joinCount > $this->maxJoins) { throw new RuntimeException('The total number of joined relations has exceeded the specified maximum. Raise the limit if necessary with the "api_platform.eager_loading.max_joins" configuration key (https://api-platform.com/docs/core/performance/#eager-loading), or limit the maximum serialization depth using the "enable_max_depth" option of the Symfony serializer (https://symfony.com/doc/current/components/serializer.html#handling-serialization-depth).'); @@ -227,8 +229,17 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt continue; } - $isNotReadableLink = false === $propertyMetadata->isReadableLink(); - if (null === $fetchEager && (false === $propertyMetadata->isReadable() || ((null === $inAttributes && $isNotReadableLink) || (false === $inAttributes)))) { + if (true !== $fetchEager && (false === $propertyMetadata->isReadable() || false === $inAttributes)) { + continue; + } + + // Avoid joining back to the parent that we just came from, but only on *ToOne relations + if ( + null !== $parentAssociation && + isset($mapping['inversedBy']) && + $mapping['inversedBy'] === $parentAssociation && + $mapping['type'] & ClassMetadata::TO_ONE + ) { continue; } @@ -254,16 +265,16 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt continue; } } else { - $queryBuilder->addSelect($associationAlias); + $this->addSelectOnce($queryBuilder, $associationAlias); } - // Avoid recursive joins + // Avoid recursive joins for self-referencing relations if ($mapping['targetEntity'] === $resourceClass) { - // Avoid joining the same association twice (see #1959) - if (!\in_array($associationAlias, $queryBuilder->getAllAliases(), true)) { - $queryBuilder->addSelect($associationAlias); - } + continue; + } + // Only join the relation's relations recursively if it's a readableLink + if (true !== $fetchEager && (true !== $propertyMetadata->isReadableLink())) { continue; } @@ -276,7 +287,7 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt } } - $this->joinRelations($queryBuilder, $queryNameGenerator, $mapping['targetEntity'], $forceEager, $fetchPartial, $associationAlias, $options, $childNormalizationContext, $isLeftJoin, $joinCount, $currentDepth); + $this->joinRelations($queryBuilder, $queryNameGenerator, $mapping['targetEntity'], $forceEager, $fetchPartial, $associationAlias, $options, $childNormalizationContext, $isLeftJoin, $joinCount, $currentDepth, $association); } } @@ -286,7 +297,7 @@ private function addSelect(QueryBuilder $queryBuilder, string $entity, string $a $entityManager = $queryBuilder->getEntityManager(); $targetClassMetadata = $entityManager->getClassMetadata($entity); if (!empty($targetClassMetadata->subClasses)) { - $queryBuilder->addSelect($associationAlias); + $this->addSelectOnce($queryBuilder, $associationAlias); return; } @@ -325,6 +336,17 @@ private function addSelect(QueryBuilder $queryBuilder, string $entity, string $a $queryBuilder->addSelect(sprintf('partial %s.{%s}', $associationAlias, implode(',', $select))); } + private function addSelectOnce(QueryBuilder $queryBuilder, string $alias) + { + $existingSelects = array_reduce($queryBuilder->getDQLPart('select') ?? [], function ($existing, $dqlSelect) { + return ($dqlSelect instanceof Select) ? array_merge($existing, $dqlSelect->getParts()) : $existing; + }, []); + + if (!\in_array($alias, $existingSelects, true)) { + $queryBuilder->addSelect($alias); + } + } + /** * Gets the serializer context. * diff --git a/tests/Core/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php b/tests/Core/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php index d6d9dfa152f..4326c63ab34 100644 --- a/tests/Core/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php +++ b/tests/Core/Bridge/Doctrine/Orm/Extension/EagerLoadingExtensionTest.php @@ -236,6 +236,7 @@ public function testApplyToItem() $queryBuilderProphecy->addSelect('partial relatedDummy4_a5.{id}')->shouldBeCalledTimes(1); $queryBuilderProphecy->addSelect('singleInheritanceRelation_a6')->shouldBeCalledTimes(1); $queryBuilderProphecy->getDQLPart('join')->willReturn([]); + $queryBuilderProphecy->getDQLPart('select')->willReturn([]); $queryBuilder = $queryBuilderProphecy->reveal(); $orderExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30, false, null, null, true); @@ -895,6 +896,7 @@ public function testApplyToCollectionNoPartial() $queryBuilderProphecy->addSelect('relatedDummy_a1')->shouldBeCalledTimes(1); $queryBuilderProphecy->addSelect('relatedDummy2_a2')->shouldBeCalledTimes(1); $queryBuilderProphecy->getDQLPart('join')->willReturn([]); + $queryBuilderProphecy->getDQLPart('select')->willReturn([]); $queryBuilder = $queryBuilderProphecy->reveal(); $eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30); @@ -960,6 +962,7 @@ private function doTestApplyToCollectionWithANonReadableButFetchEagerProperty(bo $queryBuilderProphecy->addSelect('relatedDummy_a1')->shouldBeCalledTimes(1); $queryBuilderProphecy->addSelect('relatedDummy2_a2')->shouldBeCalledTimes(1); $queryBuilderProphecy->getDQLPart('join')->willReturn([]); + $queryBuilderProphecy->getDQLPart('select')->willReturn([]); $queryBuilder = $queryBuilderProphecy->reveal(); $eagerExtensionTest = new EagerLoadingExtension($propertyNameCollectionFactoryProphecy->reveal(), $propertyMetadataFactoryProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), 30); @@ -1004,6 +1007,7 @@ public function testApplyToCollectionWithExistingJoin(string $joinType): void new Join($joinType, 'o.relatedDummy', 'existing_join_alias'), ], ]); + $queryBuilderProphecy->getDQLPart('select')->willReturn([]); $queryBuilderProphecy->addSelect('existing_join_alias')->shouldBeCalledTimes(1); $queryBuilder = $queryBuilderProphecy->reveal();