Skip to content

Commit f616985

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

File tree

7 files changed

+222
-15
lines changed

7 files changed

+222
-15
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/ContentFilteringTest.php‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@
88

99
namespace Ibexa\Tests\Integration\Core\Repository\Filtering;
1010

11+
use function array_filter;
1112
use function array_map;
13+
use function array_values;
1214
use function count;
15+
use Ibexa\Contracts\Core\Repository\LocationService;
1316
use Ibexa\Contracts\Core\Repository\Values\Content\Content;
1417
use Ibexa\Contracts\Core\Repository\Values\Content\ContentList;
18+
use Ibexa\Contracts\Core\Repository\Values\Content\Location;
1519
use Ibexa\Contracts\Core\Repository\Values\Content\Query;
1620
use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion;
1721
use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause;
@@ -22,6 +26,7 @@
2226
use Ibexa\Contracts\Core\Repository\Values\ObjectState\ObjectStateGroupCreateStruct;
2327
use Ibexa\Core\FieldType\Keyword;
2428
use Ibexa\Tests\Core\Repository\Filtering\TestContentProvider;
29+
use function iterator_to_array;
2530
use IteratorAggregate;
2631
use function sprintf;
2732

@@ -81,6 +86,150 @@ public function testFindWithLocationSortClauses(): void
8186
);
8287
}
8388

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

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

0 commit comments

Comments
 (0)