Skip to content

Commit bcf4c32

Browse files
mikadamczykmikadamczyk
committed
IBX-10841: Refactored location filtering logic
1 parent 78f9b7b commit bcf4c32

22 files changed

+570
-43
lines changed

‎src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BaseLocationCriterionQueryBuilder.php‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder;
1212
use Ibexa\Contracts\Core\Repository\Values\Filter\CriterionQueryBuilder;
1313
use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringCriterion;
14+
use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway;
1415

1516
/**
1617
* @internal for internal use by Repository Filtering
@@ -21,10 +22,35 @@ public function buildQueryConstraint(
2122
FilteringQueryBuilder $queryBuilder,
2223
FilteringCriterion $criterion
2324
): ?string {
24-
$queryBuilder->joinAllLocations();
25+
if ($this->isLocationFilteringContext($queryBuilder)) {
26+
return null;
27+
}
28+
29+
$expressionBuilder = $queryBuilder->expr();
30+
$queryBuilder->joinOnce(
31+
'content',
32+
LocationGateway::CONTENT_TREE_TABLE,
33+
'location',
34+
(string)$expressionBuilder->andX(
35+
'content.id = location.contentobject_id',
36+
'location.node_id = location.main_node_id'
37+
)
38+
);
2539

2640
return null;
2741
}
42+
43+
private function isLocationFilteringContext(FilteringQueryBuilder $queryBuilder): bool
44+
{
45+
$fromParts = $queryBuilder->getQueryPart('from');
46+
foreach ($fromParts as $fromPart) {
47+
if (($fromPart['alias'] ?? null) === 'location') {
48+
return true;
49+
}
50+
}
51+
52+
return false;
53+
}
2854
}
2955

3056
class_alias(BaseLocationCriterionQueryBuilder::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BaseLocationCriterionQueryBuilder');

‎src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
use Doctrine\DBAL\DBALException;
1414
use Doctrine\DBAL\FetchMode;
1515
use Doctrine\DBAL\Platforms\AbstractPlatform;
16-
use Doctrine\DBAL\Query\QueryBuilder;
1716
use Ibexa\Contracts\Core\Persistence\Filter\CriterionVisitor;
1817
use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder;
1918
use Ibexa\Contracts\Core\Persistence\Filter\SortClauseVisitor;
@@ -23,7 +22,6 @@
2322
use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway;
2423
use Ibexa\Core\Persistence\Legacy\Filter\Gateway\Gateway;
2524
use function iterator_to_array;
26-
use function sprintf;
2725
use Traversable;
2826

2927
/**
@@ -110,14 +108,12 @@ public function find(
110108
$names = $this->bulkFetchVersionNames(clone $query);
111109
$fieldValues = $this->bulkFetchFieldValues(clone $query);
112110

113-
// wrap query to avoid duplicate entries for multiple Locations
114-
$wrappedQuery = $this->wrapMainQuery($query);
115-
$wrappedQuery->setFirstResult($offset);
111+
$query->setFirstResult($offset);
116112
if ($limit > 0) {
117-
$wrappedQuery->setMaxResults($limit);
113+
$query->setMaxResults($limit);
118114
}
119115

120-
$resultStatement = $wrappedQuery->execute();
116+
$resultStatement = $query->execute();
121117
while (false !== ($row = $resultStatement->fetch(FetchMode::ASSOCIATIVE))) {
122118
$contentId = (int)$row['content_id'];
123119
$versionNo = (int)$row['content_version_no'];
@@ -274,21 +270,6 @@ private function getColumns(): Traversable
274270
yield "{$columnName} AS {$columnAlias}";
275271
}
276272
}
277-
278-
/**
279-
* Wrap query to avoid duplicate entries for multiple Locations.
280-
*/
281-
private function wrapMainQuery(FilteringQueryBuilder $query): QueryBuilder
282-
{
283-
$wrappedQuery = $this->connection->createQueryBuilder();
284-
$wrappedQuery
285-
->select(array_keys(self::COLUMN_MAP))
286-
->distinct()
287-
->from(sprintf('(%s)', $query->getSQL()), 'wrapped')
288-
->setParameters($query->getParameters(), $query->getParameterTypes());
289-
290-
return $wrappedQuery;
291-
}
292273
}
293274

294275
class_alias(DoctrineGateway::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\Gateway\Content\Doctrine\DoctrineGateway');

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/DateModifiedSortClauseQueryBuilder.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public function buildQuery(
2525
FilteringSortClause $sortClause
2626
): void {
2727
/** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
28-
$queryBuilder->addOrderBy('content.modified', $sortClause->direction);
28+
$queryBuilder->addOrderBy('content_modified', $sortClause->direction);
2929
}
3030
}
3131

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/DatePublishedSortClauseQueryBuilder.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public function buildQuery(
2525
FilteringSortClause $sortClause
2626
): void {
2727
/** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
28-
$queryBuilder->addOrderBy('content.published', $sortClause->direction);
28+
$queryBuilder->addOrderBy('content_published', $sortClause->direction);
2929
}
3030
}
3131

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/IdSortClauseQueryBuilder.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public function buildQuery(
2525
FilteringSortClause $sortClause
2626
): void {
2727
/** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
28-
$queryBuilder->addOrderBy('content.id', $sortClause->direction);
28+
$queryBuilder->addOrderBy('content_id', $sortClause->direction);
2929
}
3030
}
3131

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/NameSortClauseQueryBuilder.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public function buildQuery(
2525
FilteringSortClause $sortClause
2626
): void {
2727
/** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
28-
$queryBuilder->addOrderBy('content.name', $sortClause->direction);
28+
$queryBuilder->addOrderBy('content_name', $sortClause->direction);
2929
}
3030
}
3131

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/SectionIdentifierSortClauseQueryBuilder.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ public function buildQuery(
2525
FilteringQueryBuilder $queryBuilder,
2626
FilteringSortClause $sortClause
2727
): void {
28+
$sortAlias = 'ibexa_filter_sort_section_identifier';
2829
$queryBuilder
29-
->addSelect('section.identifier')
30+
->addSelect(sprintf('section.identifier AS %s', $sortAlias))
3031
->joinOnce(
3132
'content',
3233
SectionGateway::CONTENT_SECTION_TABLE,
@@ -35,7 +36,7 @@ public function buildQuery(
3536
);
3637

3738
/** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
38-
$queryBuilder->addOrderBy('section.identifier', $sortClause->direction);
39+
$queryBuilder->addOrderBy($sortAlias, $sortClause->direction);
3940
}
4041
}
4142

‎src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/SectionNameSortClauseQueryBuilder.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ public function buildQuery(
2525
FilteringQueryBuilder $queryBuilder,
2626
FilteringSortClause $sortClause
2727
): void {
28+
$sortAlias = 'ibexa_filter_sort_section_name';
2829
$queryBuilder
29-
->addSelect('section.name')
30+
->addSelect(sprintf('section.name AS %s', $sortAlias))
3031
->joinOnce(
3132
'content',
3233
SectionGateway::CONTENT_SECTION_TABLE,
@@ -35,7 +36,7 @@ public function buildQuery(
3536
);
3637

3738
/** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
38-
$queryBuilder->addOrderBy('section.name', $sortClause->direction);
39+
$queryBuilder->addOrderBy($sortAlias, $sortClause->direction);
3940
}
4041
}
4142

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

Lines changed: 89 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,110 @@
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+
private const SORT_FIELD_ALIAS_PREFIX = 'ibexa_filter_sort_';
23+
2024
public function buildQuery(
2125
FilteringQueryBuilder $queryBuilder,
2226
FilteringSortClause $sortClause
2327
): void {
24-
$sort = $this->getSortingExpression();
25-
$queryBuilder
26-
->addSelect($this->getSortingExpression())
27-
->joinAllLocations();
28+
$locationContext = $this->prepareLocationContext($queryBuilder);
29+
$locationAlias = $locationContext['alias'];
30+
31+
$sort = $this->getSortingExpressionForAlias($locationAlias);
32+
$sortAlias = $this->getSortFieldAlias($sort);
33+
$queryBuilder->addSelect(sprintf('%s AS %s', $sort, $sortAlias));
34+
35+
if ($locationContext['needsMainLocationJoin']) {
36+
$this->joinMainLocationOnly($queryBuilder, $locationAlias);
37+
}
2838

2939
/** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
30-
$queryBuilder->addOrderBy($sort, $sortClause->direction);
40+
$queryBuilder->addOrderBy($sortAlias, $sortClause->direction);
3141
}
3242

43+
/**
44+
* @return array{alias: string, needsMainLocationJoin: bool}
45+
*/
46+
private function prepareLocationContext(FilteringQueryBuilder $queryBuilder): array
47+
{
48+
if ($this->isLocationFilteringContext($queryBuilder)) {
49+
$queryBuilder->joinAllLocations();
50+
51+
return [
52+
'alias' => 'location',
53+
'needsMainLocationJoin' => false,
54+
];
55+
}
56+
57+
return [
58+
'alias' => self::CONTENT_SORT_LOCATION_ALIAS,
59+
'needsMainLocationJoin' => true,
60+
];
61+
}
62+
63+
private function isLocationFilteringContext(FilteringQueryBuilder $queryBuilder): bool
64+
{
65+
$fromParts = $queryBuilder->getQueryPart('from');
66+
foreach ($fromParts as $fromPart) {
67+
if (($fromPart['alias'] ?? null) === 'location') {
68+
return true;
69+
}
70+
}
71+
72+
return false;
73+
}
74+
75+
private function joinMainLocationOnly(FilteringQueryBuilder $queryBuilder, string $alias): void
76+
{
77+
$queryBuilder->joinOnce(
78+
'content',
79+
LocationGateway::CONTENT_TREE_TABLE,
80+
$alias,
81+
(string)$queryBuilder->expr()->andX(
82+
sprintf('content.id = %s.contentobject_id', $alias),
83+
sprintf('%s.node_id = %s.main_node_id', $alias, $alias)
84+
)
85+
);
86+
}
87+
88+
/**
89+
* Legacy entry point: implementations are expected to override this.
90+
*/
3391
abstract protected function getSortingExpression(): string;
92+
93+
/**
94+
* Optional alias-aware override; default falls back to legacy expression with alias swap.
95+
*/
96+
protected function getSortingExpressionForAlias(string $locationAlias): string
97+
{
98+
$expression = $this->getSortingExpression();
99+
100+
if ($locationAlias === 'location') {
101+
return $expression;
102+
}
103+
104+
$replaced = preg_replace('/\\blocation\\./', sprintf('%s.', $locationAlias), $expression);
105+
106+
return $replaced ?? $expression;
107+
}
108+
109+
protected function getSortFieldAlias(string $sortExpression): string
110+
{
111+
return self::SORT_FIELD_ALIAS_PREFIX . $this->getSortFieldName($sortExpression);
112+
}
113+
114+
protected function getSortFieldName(string $sortExpression): string
115+
{
116+
return str_replace('.', '_', $sortExpression);
117+
}
34118
}
35119

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

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@ protected function getSortingExpression(): string
2525
{
2626
return 'location.depth';
2727
}
28+
29+
protected function getSortingExpressionForAlias(string $locationAlias): string
30+
{
31+
return sprintf('%s.depth', $locationAlias);
32+
}
33+
34+
protected function getSortFieldName(string $sortExpression): string
35+
{
36+
return 'location_depth';
37+
}
2838
}
2939

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

0 commit comments

Comments
 (0)