Skip to content

Commit

Permalink
Is not correctly generated sql when changed/switched sqlFilter parame…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
dbannik committed Oct 23, 2024
1 parent 182469b commit 439b4da
Show file tree
Hide file tree
Showing 5 changed files with 251 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/Persisters/Entity/BasicEntityPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -1378,6 +1381,7 @@ protected function getSelectColumnsSQL()
}

$this->currentPersisterContext->selectColumnListSql = implode(', ', $columnList);
$this->filterHash = $this->em->getFilters()->getHash();

return $this->currentPersisterContext->selectColumnListSql;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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']));
}
}
Original file line number Diff line number Diff line change
@@ -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 '';
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
35 changes: 35 additions & 0 deletions tests/Tests/ORM/Functional/Ticket/SwitchContextWithFilter/User.php
Original file line number Diff line number Diff line change
@@ -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;
}
}

0 comments on commit 439b4da

Please sign in to comment.