Is not correctly generated sql when changed/switched sqlFilter parame…

…ters CachedPersisterContext::$selectJoinSql should be clear or regenerated when sqlFilter changed The problem reproduce when in use fetch=EAGER and use additional sql filter on this property
doctrine · greg0ire · Nov 23, 2024 · Oct 22, 2024 · Oct 23, 2024 · Oct 25, 2024
commit 439b4dacf415b743b0d40d65c490a4123759c520
diff --git a/src/Persisters/Entity/BasicEntityPersister.php b/src/Persisters/Entity/BasicEntityPersister.php
@@ -199,6 +199,9 @@ class BasicEntityPersister implements EntityPersister
     /** @var CachedPersisterContext */
     private $noLimitsContext;
 
+    /** @var ?string */
+    private $filterHash = null;
+
     /**
      * Initializes a new <tt>BasicEntityPersister</tt> that uses the given EntityManager
      * and persists instances of the class described by the given ClassMetadata descriptor.
@@ -1271,7 +1274,7 @@ final protected function getOrderBySQL(array $orderBy, string $baseTableAlias):
      */
     protected function getSelectColumnsSQL()
     {
-        if ($this->currentPersisterContext->selectColumnListSql !== null) {
+        if ($this->currentPersisterContext->selectColumnListSql !== null && $this->filterHash === $this->em->getFilters()->getHash()) {
             return $this->currentPersisterContext->selectColumnListSql;
         }
 
@@ -1378,6 +1381,7 @@ protected function getSelectColumnsSQL()
         }
 
         $this->currentPersisterContext->selectColumnListSql = implode(', ', $columnList);
+        $this->filterHash                                   = $this->em->getFilters()->getHash();
 
         return $this->currentPersisterContext->selectColumnListSql;
     }

diff --git a/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/ChangeFiltersTest.php b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/ChangeFiltersTest.php
@@ -0,0 +1,142 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilter;
+
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+use function sprintf;
+use function str_replace;
+
+final class ChangeFiltersTest extends OrmFunctionalTestCase
+{
+    private const COMPANY_A = 'A';
+    private const COMPANY_B = 'B';
+
+    public function setUp(): void
+    {
+        parent::setUp();
+
+        $this->setUpEntitySchema([
+            Order::class,
+            User::class,
+        ]);
+    }
+
+    /**
+     * @return non-empty-array<"companyA"|"companyB", array{orderId: int, userId: int}>
+     */
+    private function prepareData(): array
+    {
+        $user1  = new User(self::COMPANY_A);
+        $order1 = new Order($user1);
+        $user2  = new User(self::COMPANY_B);
+        $order2 = new Order($user2);
+
+        $this->_em->persist($user1);
+        $this->_em->persist($order1);
+        $this->_em->persist($user2);
+        $this->_em->persist($order2);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        return [
+            'companyA' => ['orderId' => $order1->id, 'userId' => $user1->id],
+            'companyB' => ['orderId' => $order2->id, 'userId' => $user2->id],
+        ];
+    }
+
+    public function testUseEnableDisableFilter(): void
+    {
+        $this->_em->getConfiguration()->addFilter(CompanySQLFilter::class, CompanySQLFilter::class);
+        $this->_em->getFilters()->enable(CompanySQLFilter::class)->setParameter('company', self::COMPANY_A);
+
+        ['companyA' => $companyA, 'companyB' => $companyB] = $this->prepareData();
+
+        $order1 = $this->_em->find(Order::class, $companyA['orderId']);
+
+        self::assertNotNull($order1->user, $this->generateMessage('Order1->User1 not found'));
+        self::assertEquals($companyA['userId'], $order1->user->id, $this->generateMessage('Order1->User1 != User1'));
+
+        $this->_em->getFilters()->disable(CompanySQLFilter::class);
+        $this->_em->getFilters()->enable(CompanySQLFilter::class)->setParameter('company', self::COMPANY_B);
+
+        $order2 = $this->_em->find(Order::class, $companyB['orderId']);
+
+        self::assertNotNull($order2->user, $this->generateMessage('Order2->User2 not found'));
+        self::assertEquals($companyB['userId'], $order2->user->id, $this->generateMessage('Order2->User2 != User2'));
+    }
+
+    public function testUseChangeFilterParameters(): void
+    {
+        $this->_em->getConfiguration()->addFilter(CompanySQLFilter::class, CompanySQLFilter::class);
+        $filter = $this->_em->getFilters()->enable(CompanySQLFilter::class);
+
+        ['companyA' => $companyA, 'companyB' => $companyB] = $this->prepareData();
+
+        $filter->setParameter('company', self::COMPANY_A);
+
+        $order1 = $this->_em->find(Order::class, $companyA['orderId']);
+
+        self::assertNotNull($order1->user, $this->generateMessage('Order1->User1 not found'));
+        self::assertEquals($companyA['userId'], $order1->user->id, $this->generateMessage('Order1->User1 != User1'));
+
+        $filter->setParameter('company', self::COMPANY_B);
+
+        $order2 = $this->_em->find(Order::class, $companyB['orderId']);
+
+        self::assertNotNull($order2->user, $this->generateMessage('Order2->User2 not found'));
+        self::assertEquals($companyB['userId'], $order2->user->id, $this->generateMessage('Order2->User2 != User2'));
+    }
+
+    public function testUseQueryBuilder(): void
+    {
+        $this->_em->getConfiguration()->addFilter(CompanySQLFilter::class, CompanySQLFilter::class);
+        $filter = $this->_em->getFilters()->enable(CompanySQLFilter::class);
+
+        ['companyA' => $companyA, 'companyB' => $companyB] = $this->prepareData();
+
+        $getOrderByIdCache = function (int $orderId): ?Order {
+            return $this->_em->createQueryBuilder()
+                ->select('orderMaster, user')
+                ->from(Order::class, 'orderMaster')
+                ->innerJoin('orderMaster.user', 'user')
+                ->where('orderMaster.id = :orderId')
+                ->setParameter('orderId', $orderId)
+                ->setCacheable(true)
+                ->getQuery()
+                ->setQueryCacheLifetime(10)
+                ->getOneOrNullResult();
+        };
+
+        $filter->setParameter('company', self::COMPANY_A);
+
+        $order = $getOrderByIdCache($companyB['orderId']);
+        self::assertNull($order);
+
+        $order = $getOrderByIdCache($companyA['orderId']);
+
+        self::assertInstanceOf(Order::class, $order);
+        self::assertInstanceOf(User::class, $order->user);
+        self::assertEquals($companyA['userId'], $order->user->id);
+
+        $filter->setParameter('company', self::COMPANY_B);
+
+        $order = $getOrderByIdCache($companyA['orderId']);
+        self::assertNull($order);
+
+        $order = $getOrderByIdCache($companyB['orderId']);
+
+        self::assertInstanceOf(Order::class, $order);
+        self::assertInstanceOf(User::class, $order->user);
+        self::assertEquals($companyB['userId'], $order->user->id);
+    }
+
+    private function generateMessage(string $message): string
+    {
+        $log = $this->getLastLoggedQuery();
+
+        return sprintf("%s\nSQL: %s", $message, str_replace(['?'], (array) $log['params'], $log['sql']));
+    }
+}
diff --git a/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/CompanySQLFilter.php b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/CompanySQLFilter.php
@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilter;
+
+use Doctrine\ORM\Mapping\ClassMetadata;
+use Doctrine\ORM\Query\Filter\SQLFilter;
+
+use function sprintf;
+
+class CompanySQLFilter extends SQLFilter
+{
+    public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias): string
+    {
+        if ($targetEntity->getName() === User::class) {
+            return sprintf('%s.%s = %s', $targetTableAlias, $targetEntity->fieldMappings['company']['fieldName'], $this->getParameter('company'));
+        }
+
+        if ($targetEntity->getName() === Order::class) {
+            return sprintf('%s.%s = %s', $targetTableAlias, $targetEntity->fieldMappings['company']['fieldName'], $this->getParameter('company'));
+        }
+
+        return '';
+    }
+}
diff --git a/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/Order.php b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/Order.php
@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilter;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity
+ * @ORM\Table(name="Order_Master")
+ */
+class Order
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue(strategy="AUTO")
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * @ORM\Column(type="string")
+     *
+     * @var string
+     */
+    public $company;
+
+    /**
+     * @ORM\ManyToOne(targetEntity="User", fetch="EAGER")
+     *
+     * @var User
+     */
+    public $user;
+
+    public function __construct(User $user)
+    {
+        $this->user    = $user;
+        $this->company = $user->company;
+    }
+}
diff --git a/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/User.php b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/User.php
@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilter;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity
+ * @ORM\Table(name="User_Master")
+ */
+class User
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue(strategy="AUTO")
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * @ORM\Column(type="string")
+     *
+     * @var string
+     */
+    public $company;
+
+    public function __construct(string $company)
+    {
+        $this->company = $company;
+    }
+}