Skip to content

Commit 8df9df8

Browse files
mikadamczykmikadamczyk
committed
Refactored location sorting logic in query builders and enhance filtering tests
1 parent 6fe6167 commit 8df9df8

File tree

8 files changed

+221
-16
lines changed

8 files changed

+221
-16
lines changed

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/BaseLocationSortClauseQueryBuilder.php‎

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,76 @@
1111
use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder;
1212
use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringSortClause;
1313
use Ibexa\Contracts\Core\Repository\Values\Filter\SortClauseQueryBuilder;
14+
use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway;
1415

1516
/**
1617
* @internal
1718
*/
1819
abstract class BaseLocationSortClauseQueryBuilder implements SortClauseQueryBuilder
1920
{
21+
private const CONTENT_SORT_LOCATION_ALIAS = 'ibexa_sort_location';
22+
23+
private string $locationAlias = self::CONTENT_SORT_LOCATION_ALIAS;
24+
25+
private bool $needsMainLocationJoin = true;
26+
2027
public function buildQuery(
2128
FilteringQueryBuilder $queryBuilder,
2229
FilteringSortClause $sortClause
2330
): void {
24-
$sort = $this->getSortingExpression();
25-
$queryBuilder
26-
->addSelect($this->getSortingExpression())
27-
->joinAllLocations();
31+
$this->prepareLocationAlias($queryBuilder);
32+
33+
$sort = $this->getSortingExpression($this->locationAlias);
34+
$queryBuilder->addSelect($sort);
35+
36+
if ($this->needsMainLocationJoin) {
37+
$this->joinMainLocationOnly($queryBuilder, $this->locationAlias);
38+
}
2839

2940
/** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
3041
$queryBuilder->addOrderBy($sort, $sortClause->direction);
3142
}
3243

33-
abstract protected function getSortingExpression(): string;
44+
private function prepareLocationAlias(FilteringQueryBuilder $queryBuilder): void
45+
{
46+
if ($this->isLocationFilteringContext($queryBuilder)) {
47+
$queryBuilder->joinAllLocations();
48+
$this->locationAlias = 'location';
49+
$this->needsMainLocationJoin = false;
50+
51+
return;
52+
}
53+
54+
$this->locationAlias = self::CONTENT_SORT_LOCATION_ALIAS;
55+
$this->needsMainLocationJoin = true;
56+
}
57+
58+
private function isLocationFilteringContext(FilteringQueryBuilder $queryBuilder): bool
59+
{
60+
$fromParts = $queryBuilder->getQueryPart('from');
61+
foreach ($fromParts as $fromPart) {
62+
if (($fromPart['alias'] ?? null) === 'location') {
63+
return true;
64+
}
65+
}
66+
67+
return false;
68+
}
69+
70+
private function joinMainLocationOnly(FilteringQueryBuilder $queryBuilder, string $alias): void
71+
{
72+
$queryBuilder->joinOnce(
73+
'content',
74+
LocationGateway::CONTENT_TREE_TABLE,
75+
$alias,
76+
(string)$queryBuilder->expr()->andX(
77+
sprintf('content.id = %s.contentobject_id', $alias),
78+
sprintf('%s.node_id = %s.main_node_id', $alias, $alias)
79+
)
80+
);
81+
}
82+
83+
abstract protected function getSortingExpression(string $locationAlias): string;
3484
}
3585

3686
class_alias(BaseLocationSortClauseQueryBuilder::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Location\BaseLocationSortClauseQueryBuilder');

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/DepthQueryBuilder.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ public function accepts(FilteringSortClause $sortClause): bool
2121
return $sortClause instanceof Location\Depth;
2222
}
2323

24-
protected function getSortingExpression(): string
24+
protected function getSortingExpression(string $locationAlias): string
2525
{
26-
return 'location.depth';
26+
return sprintf('%s.depth', $locationAlias);
2727
}
2828
}
2929

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/IdQueryBuilder.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ public function accepts(FilteringSortClause $sortClause): bool
2121
return $sortClause instanceof Location\Id;
2222
}
2323

24-
protected function getSortingExpression(): string
24+
protected function getSortingExpression(string $locationAlias): string
2525
{
26-
return 'location.node_id';
26+
return sprintf('%s.node_id', $locationAlias);
2727
}
2828
}
2929

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/PathQueryBuilder.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ public function accepts(FilteringSortClause $sortClause): bool
1818
return $sortClause instanceof Location\Path;
1919
}
2020

21-
protected function getSortingExpression(): string
21+
protected function getSortingExpression(string $locationAlias): string
2222
{
23-
return 'location.path_string';
23+
return sprintf('%s.path_string', $locationAlias);
2424
}
2525
}
2626

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/PriorityQueryBuilder.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ public function accepts(FilteringSortClause $sortClause): bool
1818
return $sortClause instanceof Location\Priority;
1919
}
2020

21-
protected function getSortingExpression(): string
21+
protected function getSortingExpression(string $locationAlias): string
2222
{
23-
return 'location.priority';
23+
return sprintf('%s.priority', $locationAlias);
2424
}
2525
}
2626

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/VisibilityQueryBuilder.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ public function accepts(FilteringSortClause $sortClause): bool
1818
return $sortClause instanceof Location\Visibility;
1919
}
2020

21-
protected function getSortingExpression(): string
21+
protected function getSortingExpression(string $locationAlias): string
2222
{
23-
return 'location.is_invisible';
23+
return sprintf('%s.is_invisible', $locationAlias);
2424
}
2525
}
2626

‎tests/integration/Core/Repository/Filtering/BaseRepositoryFilteringTestCase.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public function testFind(callable $filterFactory): void
7575
protected function setUp(): void
7676
{
7777
parent::setUp();
78-
$this->contentProvider = new TestContentProvider($this->getRepository(false), $this);
78+
$this->contentProvider = new TestContentProvider($this->getRepository(true), $this);
7979
}
8080

8181
/**

‎tests/integration/Core/Repository/Filtering/ContentFilteringTest.php‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@
1010

1111
use function array_map;
1212
use function count;
13+
use Ibexa\Contracts\Core\Repository\LocationService;
1314
use Ibexa\Contracts\Core\Repository\Values\Content\Content;
1415
use Ibexa\Contracts\Core\Repository\Values\Content\ContentList;
16+
use Ibexa\Contracts\Core\Repository\Values\Content\Location;
1517
use Ibexa\Contracts\Core\Repository\Values\Content\Query;
1618
use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion;
1719
use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause;
@@ -22,6 +24,7 @@
2224
use Ibexa\Contracts\Core\Repository\Values\ObjectState\ObjectStateGroupCreateStruct;
2325
use Ibexa\Core\FieldType\Keyword;
2426
use Ibexa\Tests\Core\Repository\Filtering\TestContentProvider;
27+
use function iterator_to_array;
2528
use IteratorAggregate;
2629
use function sprintf;
2730

@@ -81,6 +84,150 @@ public function testFindWithLocationSortClauses(): void
8184
);
8285
}
8386

87+
public function testLocationSortClausesUseMainLocationDuringContentFiltering(): void
88+
{
89+
$repository = $this->getRepository(false);
90+
$locationService = $repository->getLocationService();
91+
$contentService = $repository->getContentService();
92+
93+
$shallowParent = $this->createFolder(
94+
['eng-GB' => 'Shallow Parent'],
95+
2
96+
);
97+
$referenceContent = $this->createFolder(
98+
['eng-GB' => 'Reference folder'],
99+
$shallowParent->contentInfo->mainLocationId
100+
);
101+
$deepParent = $this->createFolder(
102+
['eng-GB' => 'Deep Parent'],
103+
$referenceContent->contentInfo->mainLocationId
104+
);
105+
$contentWithAdditionalLocation = $this->createFolder(
106+
['eng-GB' => 'Folder with extra location'],
107+
$deepParent->contentInfo->mainLocationId
108+
);
109+
$locationService->createLocation(
110+
$contentWithAdditionalLocation->contentInfo,
111+
$locationService->newLocationCreateStruct(2)
112+
);
113+
114+
$mainLocation = $this->loadMainLocation($locationService, $contentWithAdditionalLocation);
115+
$nonMainLocations = [];
116+
foreach ($locationService->loadLocations($contentWithAdditionalLocation->contentInfo) as $location) {
117+
if ($location->id !== $contentWithAdditionalLocation->contentInfo->mainLocationId) {
118+
$nonMainLocations[] = $location;
119+
}
120+
}
121+
self::assertNotEmpty($nonMainLocations);
122+
$nonMainLocation = $nonMainLocations[0];
123+
$referenceLocation = $this->loadMainLocation($locationService, $referenceContent);
124+
125+
self::assertLessThan($referenceLocation->depth, $nonMainLocation->depth);
126+
self::assertLessThan($mainLocation->depth, $referenceLocation->depth);
127+
128+
$filter = (new Filter())
129+
->withCriterion(
130+
new Criterion\ContentId(
131+
[
132+
$contentWithAdditionalLocation->id,
133+
$referenceContent->id,
134+
]
135+
)
136+
)
137+
->withSortClause(new SortClause\Location\Depth(Query::SORT_ASC));
138+
139+
$contentList = $contentService->find($filter);
140+
141+
self::assertSame(
142+
[$referenceContent->id, $contentWithAdditionalLocation->id],
143+
array_map(
144+
static function (Content $content): int {
145+
return $content->id;
146+
},
147+
iterator_to_array($contentList)
148+
)
149+
);
150+
}
151+
152+
public function testLocationSortClausesStayDeterministicWithComplexCriteria(): void
153+
{
154+
$repository = $this->getRepository(false);
155+
$locationService = $repository->getLocationService();
156+
$contentService = $repository->getContentService();
157+
158+
$shallowParent = $this->createFolder(
159+
['eng-GB' => 'Complex Root'],
160+
2
161+
);
162+
$referenceContent = $this->createFolder(
163+
['eng-GB' => 'Ref folder'],
164+
$shallowParent->contentInfo->mainLocationId
165+
);
166+
$middleContent = $this->createFolder(
167+
['eng-GB' => 'Middle folder'],
168+
$referenceContent->contentInfo->mainLocationId
169+
);
170+
$deepParent = $this->createFolder(
171+
['eng-GB' => 'Deep intermediate'],
172+
$middleContent->contentInfo->mainLocationId
173+
);
174+
$contentWithAdditionalLocation = $this->createFolder(
175+
['eng-GB' => 'Folder with randomizing location'],
176+
$deepParent->contentInfo->mainLocationId
177+
);
178+
$locationService->createLocation(
179+
$contentWithAdditionalLocation->contentInfo,
180+
$locationService->newLocationCreateStruct(2)
181+
);
182+
183+
$referenceLocation = $this->loadMainLocation($locationService, $referenceContent);
184+
$middleLocation = $this->loadMainLocation($locationService, $middleContent);
185+
$mainLocation = $this->loadMainLocation($locationService, $contentWithAdditionalLocation);
186+
$nonMainLocations = [];
187+
foreach ($locationService->loadLocations($contentWithAdditionalLocation->contentInfo) as $location) {
188+
if ($location->id !== $contentWithAdditionalLocation->contentInfo->mainLocationId) {
189+
$nonMainLocations[] = $location;
190+
}
191+
}
192+
self::assertNotEmpty($nonMainLocations);
193+
$nonMainLocation = $nonMainLocations[0];
194+
self::assertNotEquals($mainLocation->depth, $nonMainLocation->depth);
195+
196+
$shallowParentLocation = $this->loadMainLocation($locationService, $shallowParent);
197+
198+
$filter = (new Filter())
199+
->withCriterion(new Criterion\Subtree($shallowParentLocation->pathString))
200+
->andWithCriterion(new Criterion\ContentTypeIdentifier('folder'))
201+
->andWithCriterion(
202+
new Criterion\ContentId(
203+
[
204+
$referenceContent->id,
205+
$middleContent->id,
206+
$contentWithAdditionalLocation->id,
207+
]
208+
)
209+
)
210+
->withSortClause(new SortClause\Location\Depth(Query::SORT_ASC))
211+
->withSortClause(new SortClause\ContentId(Query::SORT_ASC))
212+
->withLimit(10);
213+
214+
$contentList = $contentService->find($filter);
215+
216+
self::assertSame(
217+
[
218+
$referenceContent->id,
219+
$middleContent->id,
220+
$contentWithAdditionalLocation->id,
221+
],
222+
array_map(
223+
static function (Content $content): int {
224+
return $content->id;
225+
},
226+
iterator_to_array($contentList)
227+
)
228+
);
229+
}
230+
84231
/**
85232
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
86233
* @throws \Exception
@@ -447,6 +594,14 @@ private function buildSearchQueryFromFilter(Filter $filter): Query
447594
]
448595
);
449596
}
597+
598+
private function loadMainLocation(LocationService $locationService, Content $content): Location
599+
{
600+
$mainLocationId = $content->contentInfo->mainLocationId;
601+
self::assertNotNull($mainLocationId);
602+
603+
return $locationService->loadLocation($mainLocationId);
604+
}
450605
}
451606

452607
class_alias(ContentFilteringTest::class, 'eZ\Publish\API\Repository\Tests\Filtering\ContentFilteringTest');

0 commit comments

Comments
 (0)