Merge pull request #2793 from antograssiot/join-condition-replacement

[2.4] Fixed join condition replacement.

Author: soyuka

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

@@ -149,10 +149,11 @@ private function getQueryBuilderWithNewAliases(QueryBuilder $queryBuilder, Query
             }
             $alias = substr($joinString, 0, $pos);
             $association = substr($joinString, $pos + 1);
-            $condition = str_replace($aliases, $replacements, $joinPart->getCondition());
-            $newAlias = QueryBuilderHelper::addJoinOnce($queryBuilderClone, $queryNameGenerator, $alias, $association, $joinPart->getJoinType(), $joinPart->getConditionType(), $condition, $originAlias);
+            $newAlias = $queryNameGenerator->generateJoinAlias($association);
             $aliases[] = "{$joinPart->getAlias()}.";
             $replacements[] = "$newAlias.";
+            $condition = str_replace($aliases, $replacements, $joinPart->getCondition());
+            QueryBuilderHelper::addJoinOnce($queryBuilderClone, $queryNameGenerator, $alias, $association, $joinPart->getJoinType(), $joinPart->getConditionType(), $condition, $originAlias, $newAlias);
         }
 
         $queryBuilderClone->add('where', str_replace($aliases, $replacements, (string) $wherePart));

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

@@ -31,15 +31,15 @@ private function __construct()
     /**
      * Adds a join to the QueryBuilder if none exists.
      */
-    public static function addJoinOnce(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $association, string $joinType = null, string $conditionType = null, string $condition = null, string $originAlias = null): string
+    public static function addJoinOnce(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $association, string $joinType = null, string $conditionType = null, string $condition = null, string $originAlias = null, string $newAlias = null): string
     {
         $join = self::getExistingJoin($queryBuilder, $alias, $association, $originAlias);
 
         if (null !== $join) {
             return $join->getAlias();
         }
 
-        $associationAlias = $queryNameGenerator->generateJoinAlias($association);
+        $associationAlias = $newAlias ?? $queryNameGenerator->generateJoinAlias($association);
         $query = "$alias.$association";
 
         if (Join::LEFT_JOIN === $joinType || QueryChecker::hasLeftJoin($queryBuilder)) {

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

@@ -211,6 +211,47 @@ public function testApplyCollectionWithManualJoin()
         $this->assertEquals($this->toDQLString($expected), $qb->getDQL());
     }
 
+    public function testApplyCollectionCorrectlyReplacesJoinCondition()
+    {
+        $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
+        $resourceMetadataFactoryProphecy->create(DummyCar::class)->willReturn(new ResourceMetadata(DummyCar::class));
+
+        $resourceNameCollectionFactoryProphecy = $this->prophesize(ResourceNameCollectionFactoryInterface::class);
+        $resourceNameCollectionFactoryProphecy->create()->willReturn(new ResourceNameCollection([DummyTravel::class]));
+
+        $em = $this->prophesize(EntityManager::class);
+        $em->getExpressionBuilder()->shouldBeCalled()->willReturn(new Expr());
+        $em->getClassMetadata(DummyCar::class)->shouldBeCalled()->willReturn(new ClassMetadataInfo(DummyCar::class));
+
+        $qb = new QueryBuilder($em->reveal());
+
+        $qb->select('o')
+            ->from(DummyCar::class, 'o')
+            ->leftJoin('o.colors', 'colors', 'ON', 'o.id = colors.car AND colors.id IN (1,2,3)')
+            ->where('o.colors = :foo')
+            ->setParameter('foo', 1);
+
+        $queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
+        $queryNameGenerator->generateJoinAlias('colors')->shouldBeCalled()->willReturn('colors_2');
+        $queryNameGenerator->generateJoinAlias('o')->shouldBeCalled()->willReturn('o_2');
+
+        $filterEagerLoadingExtension = new FilterEagerLoadingExtension($resourceMetadataFactoryProphecy->reveal(), true, new ResourceClassResolver($resourceNameCollectionFactoryProphecy->reveal()));
+        $filterEagerLoadingExtension->applyToCollection($qb, $queryNameGenerator->reveal(), DummyCar::class, 'get');
+
+        $expected = <<<'SQL'
+SELECT o
+FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar o
+LEFT JOIN o.colors colors ON o.id = colors.car AND colors.id IN (1,2,3)
+WHERE o IN(
+  SELECT o_2 FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar o_2
+  LEFT JOIN o_2.colors colors_2 ON o_2.id = colors_2.car AND colors_2.id IN (1,2,3)
+  WHERE o_2.colors = :foo
+)
+SQL;
+
+        $this->assertEquals($this->toDQLString($expected), $qb->getDQL());
+    }
+
     /**
      * https://github.com/api-platform/core/issues/1021.
      */

tests/Bridge/Doctrine/Orm/Util/QueryBuilderHelperTest.php

@@ -22,6 +22,7 @@
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\QueryBuilder;
 use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
 
 class QueryBuilderHelperTest extends TestCase
 {
@@ -53,6 +54,33 @@ public function testAddJoinOnce(?string $originAliasForJoinOnce, string $expecte
             $queryBuilder->getDQLPart('join')[$originAliasForJoinOnce ?? 'f'][0]->getAlias());
     }
 
+    /**
+     * @dataProvider provideAddJoinOnce
+     */
+    public function testAddJoinOnceWithSpecifiedNewAlias()
+    {
+        $queryBuilder = new QueryBuilder($this->prophesize(EntityManagerInterface::class)->reveal());
+        $queryBuilder->from('foo', 'f');
+
+        $queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
+        $queryNameGenerator->generateJoinAlias(Argument::any())->shouldNotbeCalled();
+
+        QueryBuilderHelper::addJoinOnce(
+            $queryBuilder,
+            $queryNameGenerator->reveal(),
+            'f',
+            'bar',
+            null,
+            null,
+            null,
+            null,
+            'f_8'
+        );
+
+        $this->assertSame('f_8',
+            $queryBuilder->getDQLPart('join')['f'][0]->getAlias());
+    }
+
     public function testGetEntityClassByAliasWithJoinByAssociation(): void
     {
         $dummyMetadata = new ClassMetadata(Dummy::class);