Skip to content

Commit ffda414

Browse files
authored
fix(doctrine): FilterEagerLoadingExtension breaks DQL if property name endings matches with query builder aliases (#4592)
* Issue #4591 bugfix: FilterEagerLoadingExtension breaks DQL if property name endings matches with query builder aliases * test: edit a unit test * test: replace EmbeddableDummy class with DummyCarInfo
1 parent 1e8d2ae commit ffda414

File tree

5 files changed

+65
-7
lines changed

5 files changed

+65
-7
lines changed

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,14 @@ private function getQueryBuilderWithNewAliases(QueryBuilder $queryBuilder, Query
141141
// Change join aliases
142142
foreach ($joinParts[$originAlias] as $joinPart) {
143143
/** @var Join $joinPart */
144-
$joinString = str_replace($aliases, $replacements, $joinPart->getJoin());
144+
$joinString = preg_replace($this->buildReplacePatterns($aliases), $replacements, $joinPart->getJoin());
145145
$pos = strpos($joinString, '.');
146146
if (false === $pos) {
147147
if (null !== $joinPart->getCondition() && null !== $this->resourceClassResolver && $this->resourceClassResolver->isResourceClass($joinString)) {
148148
$newAlias = $queryNameGenerator->generateJoinAlias($joinPart->getAlias());
149149
$aliases[] = "{$joinPart->getAlias()}.";
150150
$replacements[] = "$newAlias.";
151-
$condition = str_replace($aliases, $replacements, $joinPart->getCondition());
151+
$condition = preg_replace($this->buildReplacePatterns($aliases), $replacements, $joinPart->getCondition());
152152
$join = new Join($joinPart->getJoinType(), $joinPart->getJoin(), $newAlias, $joinPart->getConditionType(), $condition);
153153
/* @phpstan-ignore-next-line */
154154
$queryBuilderClone->add('join', [$replacement => $join], true);
@@ -161,12 +161,19 @@ private function getQueryBuilderWithNewAliases(QueryBuilder $queryBuilder, Query
161161
$newAlias = $queryNameGenerator->generateJoinAlias($association);
162162
$aliases[] = "{$joinPart->getAlias()}.";
163163
$replacements[] = "$newAlias.";
164-
$condition = str_replace($aliases, $replacements, $joinPart->getCondition() ?? '');
164+
$condition = preg_replace($this->buildReplacePatterns($aliases), $replacements, $joinPart->getCondition() ?? '');
165165
QueryBuilderHelper::addJoinOnce($queryBuilderClone, $queryNameGenerator, $alias, $association, $joinPart->getJoinType(), $joinPart->getConditionType(), $condition, $originAlias, $newAlias);
166166
}
167167

168-
$queryBuilderClone->add('where', str_replace($aliases, $replacements, (string) $wherePart));
168+
$queryBuilderClone->add('where', preg_replace($this->buildReplacePatterns($aliases), $replacements, (string) $wherePart));
169169

170170
return $queryBuilderClone;
171171
}
172+
173+
private function buildReplacePatterns(array $aliases): array
174+
{
175+
return array_map(static function (string $alias): string {
176+
return '/\b'.preg_quote($alias, '/').'/';
177+
}, $aliases);
178+
}
172179
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,9 @@ public function testApplyCollectionCorrectlyReplacesJoinCondition()
232232
->from(DummyCar::class, 'o')
233233
->leftJoin('o.colors', 'colors', 'ON', 'o.id = colors.car AND colors.id IN (1,2,3)')
234234
->where('o.colors = :foo')
235-
->setParameter('foo', 1);
235+
->andWhere('o.info.name = :bar')
236+
->setParameter('foo', 1)
237+
->setParameter('bar', 'a');
236238

237239
$queryNameGenerator = $this->prophesize(QueryNameGeneratorInterface::class);
238240
$queryNameGenerator->generateJoinAlias('colors')->shouldBeCalled()->willReturn('colors_2');
@@ -248,7 +250,7 @@ public function testApplyCollectionCorrectlyReplacesJoinCondition()
248250
WHERE o IN(
249251
SELECT o_2 FROM ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar o_2
250252
LEFT JOIN o_2.colors colors_2 ON o_2.id = colors_2.car AND colors_2.id IN (1,2,3)
251-
WHERE o_2.colors = :foo
253+
WHERE o_2.colors = :foo AND o_2.info.name = :bar
252254
)
253255
SQL;
254256

tests/Fixtures/TestBundle/Entity/DummyCar.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,18 @@ class DummyCar
124124
*/
125125
private $brand = 'DummyBrand';
126126

127+
/**
128+
* @var DummyCarInfo
129+
*
130+
* @ORM\Embedded(class="DummyCarInfo")
131+
*/
132+
private $info;
133+
127134
public function __construct()
128135
{
129136
$this->id = new DummyCarIdentifier();
130137
$this->colors = new ArrayCollection();
138+
$this->info = new DummyCarInfo();
131139
}
132140

133141
public function getId()
@@ -219,4 +227,14 @@ public function setBrand(string $brand): void
219227
{
220228
$this->brand = $brand;
221229
}
230+
231+
public function getInfo(): DummyCarInfo
232+
{
233+
return $this->info;
234+
}
235+
236+
public function setInfo(DummyCarInfo $info): void
237+
{
238+
$this->info = $info;
239+
}
222240
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity;
15+
16+
use Doctrine\ORM\Mapping as ORM;
17+
18+
/**
19+
* @author Sergey Balasov <[email protected]>
20+
*
21+
* @ORM\Embeddable
22+
*/
23+
class DummyCarInfo
24+
{
25+
/**
26+
* @var string
27+
*
28+
* @ORM\Column(nullable=true)
29+
*/
30+
public $name;
31+
}

tests/Util/AnnotationFilterExtractorTraitTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function testReadAnnotations()
4141

4242
$this->assertEquals($this->extractor->getFilters($reflectionClass), [
4343
'annotated_api_platform_core_tests_fixtures_test_bundle_entity_dummy_car_api_platform_core_bridge_doctrine_orm_filter_date_filter' => [
44-
['properties' => ['id' => 'exclude_null', 'colors' => 'exclude_null', 'name' => 'exclude_null', 'canSell' => 'exclude_null', 'availableAt' => 'exclude_null', 'brand' => 'exclude_null', 'secondColors' => 'exclude_null', 'thirdColors' => 'exclude_null', 'uuid' => 'exclude_null']],
44+
['properties' => ['id' => 'exclude_null', 'colors' => 'exclude_null', 'name' => 'exclude_null', 'canSell' => 'exclude_null', 'availableAt' => 'exclude_null', 'brand' => 'exclude_null', 'secondColors' => 'exclude_null', 'thirdColors' => 'exclude_null', 'uuid' => 'exclude_null', 'info' => 'exclude_null']],
4545
DateFilter::class,
4646
],
4747
'annotated_api_platform_core_tests_fixtures_test_bundle_entity_dummy_car_api_platform_core_bridge_doctrine_orm_filter_boolean_filter' => [

0 commit comments

Comments
 (0)