Fix duplicated eager loading joins

Author: julienfalque

phpstan.neon.dist

@@ -109,6 +109,10 @@ parameters:
 		-
 			message: '#Cannot assign new offset to string\.#'
 			path: src/JsonSchema/SchemaFactory.php
+		# https://github.com/phpstan/phpstan-doctrine/pull/135
+		-
+			message: '#^Method ApiPlatform\\Core\\Tests\\Bridge\\Doctrine\\Orm\\EntityManager::getRepository\(\) should return Doctrine\\Persistence\\ObjectRepository<T> but returns Doctrine\\Persistence\\ObjectRepository<object>\.$#'
+			path: tests/Bridge/Doctrine/Orm/CollectionDataProviderIntegrationTest.php
 
 		# Expected, due to optional interfaces
 		- '#Method ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Extension\\QueryCollectionExtensionInterface::applyToCollection\(\) invoked with 5 parameters, 3-4 required\.#'

src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php

@@ -14,6 +14,7 @@
 namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Extension;
 
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\EagerLoadingTrait;
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
 use ApiPlatform\Core\Exception\PropertyNotFoundException;
@@ -24,6 +25,7 @@
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface;
 use Doctrine\ORM\Mapping\ClassMetadataInfo;
+use Doctrine\ORM\Query\Expr\Join;
 use Doctrine\ORM\QueryBuilder;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface;
@@ -190,16 +192,20 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt
                 continue;
             }
 
-            $isNullable = $mapping['joinColumns'][0]['nullable'] ?? true;
-            if (false !== $wasLeftJoin || true === $isNullable) {
-                $method = 'leftJoin';
+            $existingJoin = QueryBuilderHelper::getExistingJoin($queryBuilder, $parentAlias, $association);
+
+            if (null !== $existingJoin) {
+                $associationAlias = $existingJoin->getAlias();
+                $isLeftJoin = Join::LEFT_JOIN === $existingJoin->getJoinType();
             } else {
-                $method = 'innerJoin';
-            }
+                $isNullable = $mapping['joinColumns'][0]['nullable'] ?? true;
+                $isLeftJoin = false !== $wasLeftJoin || true === $isNullable;
+                $method = $isLeftJoin ? 'leftJoin' : 'innerJoin';
 
-            $associationAlias = $queryNameGenerator->generateJoinAlias($association);
-            $queryBuilder->{$method}(sprintf('%s.%s', $parentAlias, $association), $associationAlias);
-            ++$joinCount;
+                $associationAlias = $queryNameGenerator->generateJoinAlias($association);
+                $queryBuilder->{$method}(sprintf('%s.%s', $parentAlias, $association), $associationAlias);
+                ++$joinCount;
+            }
 
             if (true === $fetchPartial) {
                 try {
@@ -230,7 +236,7 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt
                 }
             }
 
-            $this->joinRelations($queryBuilder, $queryNameGenerator, $mapping['targetEntity'], $forceEager, $fetchPartial, $associationAlias, $options, $normalizationContext, 'leftJoin' === $method, $joinCount, $currentDepth);
+            $this->joinRelations($queryBuilder, $queryNameGenerator, $mapping['targetEntity'], $forceEager, $fetchPartial, $associationAlias, $options, $normalizationContext, $isLeftJoin, $joinCount, $currentDepth);
         }
     }
 

src/Bridge/Doctrine/Orm/Util/QueryBuilderHelper.php

@@ -160,7 +160,7 @@ public static function traverseJoins(string $alias, QueryBuilder $queryBuilder,
     /**
      * Gets the existing join from QueryBuilder DQL parts.
      */
-    private static function getExistingJoin(QueryBuilder $queryBuilder, string $alias, string $association, string $originAlias = null): ?Join
+    public static function getExistingJoin(QueryBuilder $queryBuilder, string $alias, string $association, string $originAlias = null): ?Join
     {
         $parts = $queryBuilder->getDQLPart('join');
         $rootAlias = $originAlias ?? $queryBuilder->getRootAliases()[0];

src/Bridge/Symfony/Bundle/Resources/config/doctrine_orm.xml

@@ -131,7 +131,7 @@
             <argument type="service" id="serializer.mapping.class_metadata_factory" />
 
             <tag name="api_platform.doctrine.orm.query_extension.item" priority="-8" />
-            <tag name="api_platform.doctrine.orm.query_extension.collection" priority="-8" />
+            <tag name="api_platform.doctrine.orm.query_extension.collection" priority="-18" />
         </service>
         <service id="ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\EagerLoadingExtension" alias="api_platform.doctrine.orm.query_extension.eager_loading" />