Skip to content

Commit 3ca9529

Browse files
authored
Merge pull request #11694 from dbannik/Bug-join-sql-when-change-sqlFilter-parameters
BUG: When changing SQLFilter parameter, resulting SQL query is not generated correctly
2 parents 182469b + 439b4da commit 3ca9529

File tree

5 files changed

+251
-1
lines changed

5 files changed

+251
-1
lines changed

src/Persisters/Entity/BasicEntityPersister.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ class BasicEntityPersister implements EntityPersister
199199
/** @var CachedPersisterContext */
200200
private $noLimitsContext;
201201

202+
/** @var ?string */
203+
private $filterHash = null;
204+
202205
/**
203206
* Initializes a new <tt>BasicEntityPersister</tt> that uses the given EntityManager
204207
* and persists instances of the class described by the given ClassMetadata descriptor.
@@ -1271,7 +1274,7 @@ final protected function getOrderBySQL(array $orderBy, string $baseTableAlias):
12711274
*/
12721275
protected function getSelectColumnsSQL()
12731276
{
1274-
if ($this->currentPersisterContext->selectColumnListSql !== null) {
1277+
if ($this->currentPersisterContext->selectColumnListSql !== null && $this->filterHash === $this->em->getFilters()->getHash()) {
12751278
return $this->currentPersisterContext->selectColumnListSql;
12761279
}
12771280

@@ -1378,6 +1381,7 @@ protected function getSelectColumnsSQL()
13781381
}
13791382

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

13821386
return $this->currentPersisterContext->selectColumnListSql;
13831387
}
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilter;
6+
7+
use Doctrine\Tests\OrmFunctionalTestCase;
8+
9+
use function sprintf;
10+
use function str_replace;
11+
12+
final class ChangeFiltersTest extends OrmFunctionalTestCase
13+
{
14+
private const COMPANY_A = 'A';
15+
private const COMPANY_B = 'B';
16+
17+
public function setUp(): void
18+
{
19+
parent::setUp();
20+
21+
$this->setUpEntitySchema([
22+
Order::class,
23+
User::class,
24+
]);
25+
}
26+
27+
/**
28+
* @return non-empty-array<"companyA"|"companyB", array{orderId: int, userId: int}>
29+
*/
30+
private function prepareData(): array
31+
{
32+
$user1 = new User(self::COMPANY_A);
33+
$order1 = new Order($user1);
34+
$user2 = new User(self::COMPANY_B);
35+
$order2 = new Order($user2);
36+
37+
$this->_em->persist($user1);
38+
$this->_em->persist($order1);
39+
$this->_em->persist($user2);
40+
$this->_em->persist($order2);
41+
$this->_em->flush();
42+
$this->_em->clear();
43+
44+
return [
45+
'companyA' => ['orderId' => $order1->id, 'userId' => $user1->id],
46+
'companyB' => ['orderId' => $order2->id, 'userId' => $user2->id],
47+
];
48+
}
49+
50+
public function testUseEnableDisableFilter(): void
51+
{
52+
$this->_em->getConfiguration()->addFilter(CompanySQLFilter::class, CompanySQLFilter::class);
53+
$this->_em->getFilters()->enable(CompanySQLFilter::class)->setParameter('company', self::COMPANY_A);
54+
55+
['companyA' => $companyA, 'companyB' => $companyB] = $this->prepareData();
56+
57+
$order1 = $this->_em->find(Order::class, $companyA['orderId']);
58+
59+
self::assertNotNull($order1->user, $this->generateMessage('Order1->User1 not found'));
60+
self::assertEquals($companyA['userId'], $order1->user->id, $this->generateMessage('Order1->User1 != User1'));
61+
62+
$this->_em->getFilters()->disable(CompanySQLFilter::class);
63+
$this->_em->getFilters()->enable(CompanySQLFilter::class)->setParameter('company', self::COMPANY_B);
64+
65+
$order2 = $this->_em->find(Order::class, $companyB['orderId']);
66+
67+
self::assertNotNull($order2->user, $this->generateMessage('Order2->User2 not found'));
68+
self::assertEquals($companyB['userId'], $order2->user->id, $this->generateMessage('Order2->User2 != User2'));
69+
}
70+
71+
public function testUseChangeFilterParameters(): void
72+
{
73+
$this->_em->getConfiguration()->addFilter(CompanySQLFilter::class, CompanySQLFilter::class);
74+
$filter = $this->_em->getFilters()->enable(CompanySQLFilter::class);
75+
76+
['companyA' => $companyA, 'companyB' => $companyB] = $this->prepareData();
77+
78+
$filter->setParameter('company', self::COMPANY_A);
79+
80+
$order1 = $this->_em->find(Order::class, $companyA['orderId']);
81+
82+
self::assertNotNull($order1->user, $this->generateMessage('Order1->User1 not found'));
83+
self::assertEquals($companyA['userId'], $order1->user->id, $this->generateMessage('Order1->User1 != User1'));
84+
85+
$filter->setParameter('company', self::COMPANY_B);
86+
87+
$order2 = $this->_em->find(Order::class, $companyB['orderId']);
88+
89+
self::assertNotNull($order2->user, $this->generateMessage('Order2->User2 not found'));
90+
self::assertEquals($companyB['userId'], $order2->user->id, $this->generateMessage('Order2->User2 != User2'));
91+
}
92+
93+
public function testUseQueryBuilder(): void
94+
{
95+
$this->_em->getConfiguration()->addFilter(CompanySQLFilter::class, CompanySQLFilter::class);
96+
$filter = $this->_em->getFilters()->enable(CompanySQLFilter::class);
97+
98+
['companyA' => $companyA, 'companyB' => $companyB] = $this->prepareData();
99+
100+
$getOrderByIdCache = function (int $orderId): ?Order {
101+
return $this->_em->createQueryBuilder()
102+
->select('orderMaster, user')
103+
->from(Order::class, 'orderMaster')
104+
->innerJoin('orderMaster.user', 'user')
105+
->where('orderMaster.id = :orderId')
106+
->setParameter('orderId', $orderId)
107+
->setCacheable(true)
108+
->getQuery()
109+
->setQueryCacheLifetime(10)
110+
->getOneOrNullResult();
111+
};
112+
113+
$filter->setParameter('company', self::COMPANY_A);
114+
115+
$order = $getOrderByIdCache($companyB['orderId']);
116+
self::assertNull($order);
117+
118+
$order = $getOrderByIdCache($companyA['orderId']);
119+
120+
self::assertInstanceOf(Order::class, $order);
121+
self::assertInstanceOf(User::class, $order->user);
122+
self::assertEquals($companyA['userId'], $order->user->id);
123+
124+
$filter->setParameter('company', self::COMPANY_B);
125+
126+
$order = $getOrderByIdCache($companyA['orderId']);
127+
self::assertNull($order);
128+
129+
$order = $getOrderByIdCache($companyB['orderId']);
130+
131+
self::assertInstanceOf(Order::class, $order);
132+
self::assertInstanceOf(User::class, $order->user);
133+
self::assertEquals($companyB['userId'], $order->user->id);
134+
}
135+
136+
private function generateMessage(string $message): string
137+
{
138+
$log = $this->getLastLoggedQuery();
139+
140+
return sprintf("%s\nSQL: %s", $message, str_replace(['?'], (array) $log['params'], $log['sql']));
141+
}
142+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilter;
6+
7+
use Doctrine\ORM\Mapping\ClassMetadata;
8+
use Doctrine\ORM\Query\Filter\SQLFilter;
9+
10+
use function sprintf;
11+
12+
class CompanySQLFilter extends SQLFilter
13+
{
14+
public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias): string
15+
{
16+
if ($targetEntity->getName() === User::class) {
17+
return sprintf('%s.%s = %s', $targetTableAlias, $targetEntity->fieldMappings['company']['fieldName'], $this->getParameter('company'));
18+
}
19+
20+
if ($targetEntity->getName() === Order::class) {
21+
return sprintf('%s.%s = %s', $targetTableAlias, $targetEntity->fieldMappings['company']['fieldName'], $this->getParameter('company'));
22+
}
23+
24+
return '';
25+
}
26+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilter;
6+
7+
use Doctrine\ORM\Mapping as ORM;
8+
9+
/**
10+
* @ORM\Entity
11+
* @ORM\Table(name="Order_Master")
12+
*/
13+
class Order
14+
{
15+
/**
16+
* @ORM\Id
17+
* @ORM\Column(type="integer")
18+
* @ORM\GeneratedValue(strategy="AUTO")
19+
*
20+
* @var int
21+
*/
22+
public $id;
23+
24+
/**
25+
* @ORM\Column(type="string")
26+
*
27+
* @var string
28+
*/
29+
public $company;
30+
31+
/**
32+
* @ORM\ManyToOne(targetEntity="User", fetch="EAGER")
33+
*
34+
* @var User
35+
*/
36+
public $user;
37+
38+
public function __construct(User $user)
39+
{
40+
$this->user = $user;
41+
$this->company = $user->company;
42+
}
43+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Functional\Ticket\SwitchContextWithFilter;
6+
7+
use Doctrine\ORM\Mapping as ORM;
8+
9+
/**
10+
* @ORM\Entity
11+
* @ORM\Table(name="User_Master")
12+
*/
13+
class User
14+
{
15+
/**
16+
* @ORM\Id
17+
* @ORM\Column(type="integer")
18+
* @ORM\GeneratedValue(strategy="AUTO")
19+
*
20+
* @var int
21+
*/
22+
public $id;
23+
24+
/**
25+
* @ORM\Column(type="string")
26+
*
27+
* @var string
28+
*/
29+
public $company;
30+
31+
public function __construct(string $company)
32+
{
33+
$this->company = $company;
34+
}
35+
}

0 commit comments

Comments
 (0)