From 439b4dacf415b743b0d40d65c490a4123759c520 Mon Sep 17 00:00:00 2001 From: Dzmitry Bannik Date: Tue, 22 Oct 2024 22:30:39 +0300 Subject: [PATCH] Is not correctly generated sql when changed/switched sqlFilter parameters 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 --- .../Entity/BasicEntityPersister.php | 6 +- .../ChangeFiltersTest.php | 142 ++++++++++++++++++ .../CompanySQLFilter.php | 26 ++++ .../Ticket/SwitchContextWithFilter/Order.php | 43 ++++++ .../Ticket/SwitchContextWithFilter/User.php | 35 +++++ 5 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/ChangeFiltersTest.php create mode 100644 tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/CompanySQLFilter.php create mode 100644 tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/Order.php create mode 100644 tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/User.php diff --git a/src/Persisters/Entity/BasicEntityPersister.php b/src/Persisters/Entity/BasicEntityPersister.php index 5ca00cb007..9bd8afd3cc 100644 --- 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 BasicEntityPersister 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 new file mode 100644 index 0000000000..7ce97442b2 --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/ChangeFiltersTest.php @@ -0,0 +1,142 @@ +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 new file mode 100644 index 0000000000..e65188334a --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/CompanySQLFilter.php @@ -0,0 +1,26 @@ +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 new file mode 100644 index 0000000000..a6d86dca8a --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/Order.php @@ -0,0 +1,43 @@ +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 new file mode 100644 index 0000000000..294bfdf87a --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/User.php @@ -0,0 +1,35 @@ +company = $company; + } +}