#4041 bugfix: FilterEagerLoadingExtension foreign identifier handling (#4042)

* Issue #4041 bugfix: FilterEagerLoadingExtension foreign identifier handling

* test: add a unit test and use a foreign id for DummyCar

* chore: add changelog entry

Co-authored-by: Alan Poulain <[email protected]>

Author: Edvinas9

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## 2.6.3
 
 * Mercure: Do not use data in options when deleting (#4056)
+* Doctrine: Support for foreign identifiers
 
 ## 2.6.2
 

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

@@ -76,8 +76,16 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
         if (!$classMetadata->isIdentifierComposite) {
             $replacementAlias = $queryNameGenerator->generateJoinAlias($originAlias);
             $in = $this->getQueryBuilderWithNewAliases($queryBuilder, $queryNameGenerator, $originAlias, $replacementAlias);
-            $in->select($replacementAlias);
-            $queryBuilderClone->andWhere($queryBuilderClone->expr()->in($originAlias, $in->getDQL()));
+
+            if ($classMetadata->containsForeignIdentifier) {
+                $identifier = current($classMetadata->getIdentifier());
+                $in->select("IDENTITY($replacementAlias.$identifier)");
+                $queryBuilderClone->andWhere($queryBuilderClone->expr()->in("$originAlias.$identifier", $in->getDQL()));
+            } else {
+                $in->select($replacementAlias);
+                $queryBuilderClone->andWhere($queryBuilderClone->expr()->in($originAlias, $in->getDQL()));
+            }
+
             $changedWhereClause = true;
         } else {
             // Because Doctrine doesn't support WHERE ( foo, bar ) IN () (https://github.com/doctrine/doctrine2/issues/5238), we are building as many subqueries as they are identifiers

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

@@ -80,6 +80,11 @@ public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGenerator
         }
 
         if (null !== $this->order) {
+            // A foreign identifier cannot be used for ordering.
+            if ($classMetaData->containsForeignIdentifier) {
+                return;
+            }
+
             foreach ($identifiers as $identifier) {
                 $queryBuilder->addOrderBy("{$rootAlias}.{$identifier}", $this->order);
             }

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

@@ -559,6 +559,48 @@ public function testCompositeIdentifiersWithoutAssociation()
         $this->assertEquals($this->toDQLString($expected), $qb->getDQL());
     }
 
+    public function testCompositeIdentifiersWithForeignIdentifiers()
+    {
+        $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class);
+        $resourceMetadataFactoryProphecy->create(DummyCar::class)->willReturn(new ResourceMetadata(DummyCar::class));
+
+        $classMetadata = new ClassMetadataInfo(DummyCar::class);
+        $classMetadata->setIdentifier(['id']);
+        $classMetadata->containsForeignIdentifier = true;
+
+        $em = $this->prophesize(EntityManager::class);
+        $em->getExpressionBuilder()->shouldBeCalled()->willReturn(new Expr());
+        $em->getClassMetadata(DummyCar::class)->shouldBeCalled()->willReturn($classMetadata);
+
+        $qb = new QueryBuilder($em->reveal());
+
+        $qb->select('o')
+            ->from(DummyCar::class, 'o')
+            ->leftJoin('o.colors', 'colors')
+            ->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);
+        $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
+WHERE o.id IN(
+  SELECT IDENTITY(o_2.id) FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar o_2
+  LEFT JOIN o_2.colors colors_2
+  WHERE o_2.colors = :foo
+)
+SQL;
+
+        $this->assertEquals($this->toDQLString($expected), $qb->getDQL());
+    }
+
     private function toDQLString(string $dql): string
     {
         return preg_replace(['/\s+/', '/\(\s/', '/\s\)/'], [' ', '(', ')'], $dql);

tests/Fixtures/TestBundle/Entity/DummyCar.php

@@ -42,11 +42,10 @@
 class DummyCar
 {
     /**
-     * @var int The entity Id
+     * @var DummyCarIdentifier The entity Id
      *
      * @ORM\Id
-     * @ORM\GeneratedValue
-     * @ORM\Column(type="integer")
+     * @ORM\OneToOne(targetEntity="DummyCarIdentifier", cascade="persist")
      */
     private $id;
 
@@ -85,7 +84,7 @@ class DummyCar
      *
      * @ORM\ManyToMany(targetEntity="UuidIdentifierDummy", indexBy="uuid")
      * * @ORM\JoinTable(name="uuid_cars",
-     *     joinColumns={@ORM\JoinColumn(name="car_id", referencedColumnName="id")},
+     *     joinColumns={@ORM\JoinColumn(name="car_id", referencedColumnName="id_id")},
      *     inverseJoinColumns={@ORM\JoinColumn(name="uuid_uuid", referencedColumnName="uuid")}
      * )
      * @Serializer\Groups({"colors"})
@@ -127,6 +126,7 @@ class DummyCar
 
     public function __construct()
     {
+        $this->id = new DummyCarIdentifier();
         $this->colors = new ArrayCollection();
     }
 

tests/Fixtures/TestBundle/Entity/DummyCarColor.php

@@ -39,7 +39,7 @@ class DummyCarColor
      * @var DummyCar
      *
      * @ORM\ManyToOne(targetEntity="DummyCar", inversedBy="colors")
-     * @ORM\JoinColumn(nullable=false, onDelete="CASCADE")
+     * @ORM\JoinColumn(nullable=false, onDelete="CASCADE", referencedColumnName="id_id")
      * @Assert\NotBlank
      */
     private $car;

tests/Fixtures/TestBundle/Entity/DummyCarIdentifier.php

@@ -0,0 +1,34 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity
+ */
+class DummyCarIdentifier
+{
+    /**
+     * @ORM\Id
+     * @ORM\GeneratedValue
+     * @ORM\Column(type="integer")
+     */
+    private $id;
+
+    public function __toString()
+    {
+        return (string) $this->id;
+    }
+}

tests/Fixtures/TestBundle/Entity/DummyTravel.php

@@ -31,7 +31,7 @@ class DummyTravel
 
     /**
      * @ORM\ManyToOne(targetEntity="DummyCar")
-     * @ORM\JoinColumn(name="car_id", referencedColumnName="id")
+     * @ORM\JoinColumn(name="car_id", referencedColumnName="id_id")
      */
     public $car;