diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 6e152f3d77..98ab313e7a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -16,22 +16,17 @@ jobs: php: - '8.1' steps: - - uses: actions/checkout@v5 - - - name: Setup PHP Action - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php }} - coverage: none - extensions: 'pdo_sqlite, gd' - tools: cs2pr + - uses: actions/checkout@v5 - - uses: ramsey/composer-install@v3 - with: - dependency-versions: "highest" + - uses: ibexa/gh-workflows/actions/composer-install@main + with: + gh-client-id: ${{ secrets.AUTOMATION_CLIENT_ID }} + gh-client-secret: ${{ secrets.AUTOMATION_CLIENT_SECRET }} + satis-network-key: ${{ secrets.SATIS_NETWORK_KEY }} + satis-network-token: ${{ secrets.SATIS_NETWORK_TOKEN }} - - name: Run code style check - run: composer run-script check-cs -- --format=checkstyle | cs2pr + - name: Run code style check + run: composer run-script check-cs -- --format=checkstyle | cs2pr tests: name: Unit tests & SQLite integration tests @@ -47,38 +42,33 @@ jobs: - '8.1' steps: - - uses: actions/checkout@v5 - - - name: Setup PHP Action - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php }} - coverage: none - extensions: pdo_sqlite, gd - tools: cs2pr + - uses: actions/checkout@v5 - - uses: ramsey/composer-install@v3 - with: - dependency-versions: "highest" + - uses: ibexa/gh-workflows/actions/composer-install@main + with: + gh-client-id: ${{ secrets.AUTOMATION_CLIENT_ID }} + gh-client-secret: ${{ secrets.AUTOMATION_CLIENT_SECRET }} + satis-network-key: ${{ secrets.SATIS_NETWORK_KEY }} + satis-network-token: ${{ secrets.SATIS_NETWORK_TOKEN }} - - name: Setup problem matchers for PHPUnit - run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" + - name: Setup problem matchers for PHPUnit + run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" - - name: Run PHPStan analysis - run: composer run-script phpstan + - name: Run PHPStan analysis + run: composer run-script phpstan - - name: Run unit test suite - run: composer run-script unit + - name: Run unit test suite + run: composer run-script unit - - name: Run integration test suite - run: composer run-script integration + - name: Run integration test suite + run: composer run-script integration integration-tests-postgres: name: PostgreSQL integration tests needs: tests services: postgres: - image: postgres:14 + image: ${{ matrix.image }} ports: - 5432 env: @@ -100,21 +90,19 @@ jobs: - '7.4' - '8.0' - '8.1' + image: + - 'postgres:14' + - 'postgres:18' steps: - uses: actions/checkout@v5 - - name: Setup PHP Action - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php }} - coverage: none - extensions: pdo_pgsql, gd - tools: cs2pr - - - uses: ramsey/composer-install@v3 + - uses: ibexa/gh-workflows/actions/composer-install@main with: - dependency-versions: "highest" + gh-client-id: ${{ secrets.AUTOMATION_CLIENT_ID }} + gh-client-secret: ${{ secrets.AUTOMATION_CLIENT_SECRET }} + satis-network-key: ${{ secrets.SATIS_NETWORK_KEY }} + satis-network-token: ${{ secrets.SATIS_NETWORK_TOKEN }} - name: Setup problem matchers for PHPUnit run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" @@ -126,12 +114,12 @@ jobs: # Required by old repository tests DATABASE: "pgsql://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/testdb" - integration-tests-mysql-80: - name: MySQL integration tests (8.0) + integration-tests-mysql: + name: MySQL integration tests needs: tests services: mysql: - image: mysql:8.0 + image: ${{ matrix.image }} ports: - 3306/tcp env: @@ -155,75 +143,19 @@ jobs: - '7.4' - '8.0' - '8.1' + image: + - 'mysql:8.0' + - 'mysql:8.4' steps: - uses: actions/checkout@v5 - - name: Setup PHP Action - uses: shivammathur/setup-php@v2 + - uses: ibexa/gh-workflows/actions/composer-install@main with: - php-version: ${{ matrix.php }} - coverage: none - extensions: pdo_mysql, gd, redis - tools: cs2pr - - - uses: ramsey/composer-install@v3 - with: - dependency-versions: "highest" - - - name: Setup problem matchers for PHPUnit - run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" - - - name: Run integration test suite vs MySQL - run: composer run-script integration - env: - DATABASE_URL: "mysql://mysql:mysql@127.0.0.1:${{ job.services.mysql.ports[3306] }}/testdb" - DATABASE: "mysql://mysql:mysql@127.0.0.1:${{ job.services.mysql.ports[3306] }}/testdb" - - integration-tests-mysql-84: - name: MySQL integration tests (8.4) - needs: tests - services: - mysql: - image: mysql:8.4 - ports: - - 3306/tcp - env: - MYSQL_RANDOM_ROOT_PASSWORD: true - MYSQL_USER: mysql - MYSQL_PASSWORD: mysql - MYSQL_DATABASE: testdb - options: >- - --health-cmd="mysqladmin ping" - --health-interval=10s - --health-timeout=5s - --health-retries=5 - --tmpfs=/var/lib/mysql - runs-on: "ubuntu-24.04" - timeout-minutes: 60 - - strategy: - fail-fast: false - matrix: - php: - - '7.4' - - '8.0' - - '8.1' - - steps: - - uses: actions/checkout@v5 - - - name: Setup PHP Action - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php }} - coverage: none - extensions: pdo_mysql, gd, redis - tools: cs2pr - - - uses: ramsey/composer-install@v3 - with: - dependency-versions: "highest" + gh-client-id: ${{ secrets.AUTOMATION_CLIENT_ID }} + gh-client-secret: ${{ secrets.AUTOMATION_CLIENT_SECRET }} + satis-network-key: ${{ secrets.SATIS_NETWORK_KEY }} + satis-network-token: ${{ secrets.SATIS_NETWORK_TOKEN }} - name: Setup problem matchers for PHPUnit run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" @@ -265,28 +197,28 @@ jobs: - '8.0' - '8.1' steps: - - uses: actions/checkout@v5 - with: - fetch-depth: 0 + - uses: actions/checkout@v5 + with: + fetch-depth: 0 - - name: Setup PHP Action - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php }} - coverage: none + - name: Setup PHP Action + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + coverage: none - - name: Add solr dependency - run: | - VERSION=$(jq -r '.extra | ."branch-alias" | ."dev-main"' < composer.json) - composer require --no-update "ibexa/solr:$VERSION" + - name: Add solr dependency + run: | + VERSION=$(jq -r '.extra | ."branch-alias" | ."dev-main"' < composer.json) + composer require --no-update "ibexa/solr:$VERSION" - - uses: ramsey/composer-install@v3 - with: - dependency-versions: "highest" + - uses: ramsey/composer-install@v3 + with: + dependency-versions: "highest" - - name: Run integration test suite - run: composer test-integration-solr - env: + - name: Run integration test suite + run: composer test-integration-solr + env: CUSTOM_CACHE_POOL: singleredis CACHE_HOST: 127.0.0.1 CORES_SETUP: single diff --git a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BaseLocationCriterionQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BaseLocationCriterionQueryBuilder.php index b5189a4792..b613e8265e 100644 --- a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BaseLocationCriterionQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BaseLocationCriterionQueryBuilder.php @@ -11,6 +11,7 @@ use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder; use Ibexa\Contracts\Core\Repository\Values\Filter\CriterionQueryBuilder; use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringCriterion; +use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway; /** * @internal for internal use by Repository Filtering @@ -21,10 +22,35 @@ public function buildQueryConstraint( FilteringQueryBuilder $queryBuilder, FilteringCriterion $criterion ): ?string { - $queryBuilder->joinAllLocations(); + if ($this->isLocationFilteringContext($queryBuilder)) { + return null; + } + + $expressionBuilder = $queryBuilder->expr(); + $queryBuilder->joinOnce( + 'content', + LocationGateway::CONTENT_TREE_TABLE, + 'location', + (string)$expressionBuilder->andX( + 'content.id = location.contentobject_id', + 'location.node_id = location.main_node_id' + ) + ); return null; } + + private function isLocationFilteringContext(FilteringQueryBuilder $queryBuilder): bool + { + $fromParts = $queryBuilder->getQueryPart('from'); + foreach ($fromParts as $fromPart) { + if (($fromPart['alias'] ?? null) === 'location') { + return true; + } + } + + return false; + } } class_alias(BaseLocationCriterionQueryBuilder::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BaseLocationCriterionQueryBuilder'); diff --git a/src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php b/src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php index 18a2424afc..3d82b5c0b9 100644 --- a/src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php +++ b/src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php @@ -13,7 +13,6 @@ use Doctrine\DBAL\DBALException; use Doctrine\DBAL\FetchMode; use Doctrine\DBAL\Platforms\AbstractPlatform; -use Doctrine\DBAL\Query\QueryBuilder; use Ibexa\Contracts\Core\Persistence\Filter\CriterionVisitor; use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder; use Ibexa\Contracts\Core\Persistence\Filter\SortClauseVisitor; @@ -23,7 +22,6 @@ use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway; use Ibexa\Core\Persistence\Legacy\Filter\Gateway\Gateway; use function iterator_to_array; -use function sprintf; use Traversable; /** @@ -110,14 +108,12 @@ public function find( $names = $this->bulkFetchVersionNames(clone $query); $fieldValues = $this->bulkFetchFieldValues(clone $query); - // wrap query to avoid duplicate entries for multiple Locations - $wrappedQuery = $this->wrapMainQuery($query); - $wrappedQuery->setFirstResult($offset); + $query->setFirstResult($offset); if ($limit > 0) { - $wrappedQuery->setMaxResults($limit); + $query->setMaxResults($limit); } - $resultStatement = $wrappedQuery->execute(); + $resultStatement = $query->execute(); while (false !== ($row = $resultStatement->fetch(FetchMode::ASSOCIATIVE))) { $contentId = (int)$row['content_id']; $versionNo = (int)$row['content_version_no']; @@ -274,21 +270,6 @@ private function getColumns(): Traversable yield "{$columnName} AS {$columnAlias}"; } } - - /** - * Wrap query to avoid duplicate entries for multiple Locations. - */ - private function wrapMainQuery(FilteringQueryBuilder $query): QueryBuilder - { - $wrappedQuery = $this->connection->createQueryBuilder(); - $wrappedQuery - ->select(array_keys(self::COLUMN_MAP)) - ->distinct() - ->from(sprintf('(%s)', $query->getSQL()), 'wrapped') - ->setParameters($query->getParameters(), $query->getParameterTypes()); - - return $wrappedQuery; - } } class_alias(DoctrineGateway::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\Gateway\Content\Doctrine\DoctrineGateway'); diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/DateModifiedSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/DateModifiedSortClauseQueryBuilder.php index 0ebd294116..2ffb92d9ea 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/DateModifiedSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/DateModifiedSortClauseQueryBuilder.php @@ -25,7 +25,7 @@ public function buildQuery( FilteringSortClause $sortClause ): void { /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */ - $queryBuilder->addOrderBy('content.modified', $sortClause->direction); + $queryBuilder->addOrderBy('content_modified', $sortClause->direction); } } diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/DatePublishedSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/DatePublishedSortClauseQueryBuilder.php index e6ae81cea4..1f95446350 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/DatePublishedSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/DatePublishedSortClauseQueryBuilder.php @@ -25,7 +25,7 @@ public function buildQuery( FilteringSortClause $sortClause ): void { /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */ - $queryBuilder->addOrderBy('content.published', $sortClause->direction); + $queryBuilder->addOrderBy('content_published', $sortClause->direction); } } diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/IdSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/IdSortClauseQueryBuilder.php index 2beae9b636..ab3325bcbd 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/IdSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/IdSortClauseQueryBuilder.php @@ -25,7 +25,7 @@ public function buildQuery( FilteringSortClause $sortClause ): void { /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */ - $queryBuilder->addOrderBy('content.id', $sortClause->direction); + $queryBuilder->addOrderBy('content_id', $sortClause->direction); } } diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/NameSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/NameSortClauseQueryBuilder.php index 51946da0bc..a165d0b7a3 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/NameSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/NameSortClauseQueryBuilder.php @@ -25,7 +25,7 @@ public function buildQuery( FilteringSortClause $sortClause ): void { /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */ - $queryBuilder->addOrderBy('content.name', $sortClause->direction); + $queryBuilder->addOrderBy('content_name', $sortClause->direction); } } diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/SectionIdentifierSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/SectionIdentifierSortClauseQueryBuilder.php index c9bfd8a6ab..6a219b0d98 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/SectionIdentifierSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/SectionIdentifierSortClauseQueryBuilder.php @@ -25,8 +25,9 @@ public function buildQuery( FilteringQueryBuilder $queryBuilder, FilteringSortClause $sortClause ): void { + $sortAlias = 'ibexa_filter_sort_section_identifier'; $queryBuilder - ->addSelect('section.identifier') + ->addSelect(sprintf('section.identifier AS %s', $sortAlias)) ->joinOnce( 'content', SectionGateway::CONTENT_SECTION_TABLE, @@ -35,7 +36,7 @@ public function buildQuery( ); /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */ - $queryBuilder->addOrderBy('section.identifier', $sortClause->direction); + $queryBuilder->addOrderBy($sortAlias, $sortClause->direction); } } diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/SectionNameSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/SectionNameSortClauseQueryBuilder.php index a2c95562d8..9c1dbc0e68 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/SectionNameSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Content/SectionNameSortClauseQueryBuilder.php @@ -25,8 +25,9 @@ public function buildQuery( FilteringQueryBuilder $queryBuilder, FilteringSortClause $sortClause ): void { + $sortAlias = 'ibexa_filter_sort_section_name'; $queryBuilder - ->addSelect('section.name') + ->addSelect(sprintf('section.name AS %s', $sortAlias)) ->joinOnce( 'content', SectionGateway::CONTENT_SECTION_TABLE, @@ -35,7 +36,7 @@ public function buildQuery( ); /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */ - $queryBuilder->addOrderBy('section.name', $sortClause->direction); + $queryBuilder->addOrderBy($sortAlias, $sortClause->direction); } } diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/BaseLocationSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/BaseLocationSortClauseQueryBuilder.php index 25f09c0596..50ad085341 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/BaseLocationSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/BaseLocationSortClauseQueryBuilder.php @@ -11,26 +11,107 @@ use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder; use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringSortClause; use Ibexa\Contracts\Core\Repository\Values\Filter\SortClauseQueryBuilder; +use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway; /** * @internal */ abstract class BaseLocationSortClauseQueryBuilder implements SortClauseQueryBuilder { + private const CONTENT_SORT_LOCATION_ALIAS = 'ibexa_sort_location'; + private const SORT_FIELD_ALIAS_PREFIX = 'ibexa_filter_sort_'; + + private string $locationAlias = self::CONTENT_SORT_LOCATION_ALIAS; + + private bool $needsMainLocationJoin = true; + public function buildQuery( FilteringQueryBuilder $queryBuilder, FilteringSortClause $sortClause ): void { - $sort = $this->getSortingExpression(); - $queryBuilder - ->addSelect($this->getSortingExpression()) - ->joinAllLocations(); + $this->prepareLocationAlias($queryBuilder); + + $sort = $this->getSortingExpressionForAlias($this->locationAlias); + $sortAlias = $this->getSortFieldAlias($sort); + $queryBuilder->addSelect(sprintf('%s AS %s', $sort, $sortAlias)); + + if ($this->needsMainLocationJoin) { + $this->joinMainLocationOnly($queryBuilder, $this->locationAlias); + } /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */ - $queryBuilder->addOrderBy($sort, $sortClause->direction); + $queryBuilder->addOrderBy($sortAlias, $sortClause->direction); + } + + private function prepareLocationAlias(FilteringQueryBuilder $queryBuilder): void + { + if ($this->isLocationFilteringContext($queryBuilder)) { + $queryBuilder->joinAllLocations(); + $this->locationAlias = 'location'; + $this->needsMainLocationJoin = false; + + return; + } + + $this->locationAlias = self::CONTENT_SORT_LOCATION_ALIAS; + $this->needsMainLocationJoin = true; + } + + private function isLocationFilteringContext(FilteringQueryBuilder $queryBuilder): bool + { + $fromParts = $queryBuilder->getQueryPart('from'); + foreach ($fromParts as $fromPart) { + if (($fromPart['alias'] ?? null) === 'location') { + return true; + } + } + + return false; } + private function joinMainLocationOnly(FilteringQueryBuilder $queryBuilder, string $alias): void + { + $queryBuilder->joinOnce( + 'content', + LocationGateway::CONTENT_TREE_TABLE, + $alias, + (string)$queryBuilder->expr()->andX( + sprintf('content.id = %s.contentobject_id', $alias), + sprintf('%s.node_id = %s.main_node_id', $alias, $alias) + ) + ); + } + + /** + * Legacy entry point: implementations are expected to override this. + */ abstract protected function getSortingExpression(): string; + + /** + * Optional alias-aware override; default falls back to legacy expression with alias swap. + */ + protected function getSortingExpressionForAlias(string $locationAlias): string + { + $expression = $this->getSortingExpression(); + + if ($locationAlias === 'location') { + return $expression; + } + + $replaced = preg_replace('/\\blocation\\./', sprintf('%s.', $locationAlias), $expression); + + return $replaced ?? $expression; + } + + protected function getSortFieldAlias(string $sortExpression): string + { + return self::SORT_FIELD_ALIAS_PREFIX . $this->getSortFieldName($sortExpression); + } + + protected function getSortFieldName(string $sortExpression): string + { + return str_replace('.', '_', $sortExpression); + } } class_alias(BaseLocationSortClauseQueryBuilder::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Location\BaseLocationSortClauseQueryBuilder'); diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/DepthQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/DepthQueryBuilder.php index 2f862abc37..945da63a48 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/DepthQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/DepthQueryBuilder.php @@ -25,6 +25,16 @@ protected function getSortingExpression(): string { return 'location.depth'; } + + protected function getSortingExpressionForAlias(string $locationAlias): string + { + return sprintf('%s.depth', $locationAlias); + } + + protected function getSortFieldName(string $sortExpression): string + { + return 'location_depth'; + } } class_alias(DepthQueryBuilder::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Location\DepthQueryBuilder'); diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/IdQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/IdQueryBuilder.php index a9651bddb5..589537ea9c 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/IdQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/IdQueryBuilder.php @@ -25,6 +25,16 @@ protected function getSortingExpression(): string { return 'location.node_id'; } + + protected function getSortingExpressionForAlias(string $locationAlias): string + { + return sprintf('%s.node_id', $locationAlias); + } + + protected function getSortFieldName(string $sortExpression): string + { + return 'location_id'; + } } class_alias(IdQueryBuilder::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Location\IdQueryBuilder'); diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/PathQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/PathQueryBuilder.php index 039951e0c0..75b946f605 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/PathQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/PathQueryBuilder.php @@ -22,6 +22,16 @@ protected function getSortingExpression(): string { return 'location.path_string'; } + + protected function getSortingExpressionForAlias(string $locationAlias): string + { + return sprintf('%s.path_string', $locationAlias); + } + + protected function getSortFieldName(string $sortExpression): string + { + return 'location_path'; + } } class_alias(PathQueryBuilder::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Location\PathQueryBuilder'); diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/PriorityQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/PriorityQueryBuilder.php index 543f64d816..c6f07c32b4 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/PriorityQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/PriorityQueryBuilder.php @@ -22,6 +22,16 @@ protected function getSortingExpression(): string { return 'location.priority'; } + + protected function getSortingExpressionForAlias(string $locationAlias): string + { + return sprintf('%s.priority', $locationAlias); + } + + protected function getSortFieldName(string $sortExpression): string + { + return 'location_priority'; + } } class_alias(PriorityQueryBuilder::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Location\PriorityQueryBuilder'); diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/VisibilityQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/VisibilityQueryBuilder.php index 3877761668..89fed7f41d 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/VisibilityQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/VisibilityQueryBuilder.php @@ -22,6 +22,16 @@ protected function getSortingExpression(): string { return 'location.is_invisible'; } + + protected function getSortingExpressionForAlias(string $locationAlias): string + { + return sprintf('%s.is_invisible', $locationAlias); + } + + protected function getSortFieldName(string $sortExpression): string + { + return 'location_visibility'; + } } class_alias(VisibilityQueryBuilder::class, 'eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Location\VisibilityQueryBuilder'); diff --git a/tests/integration/Core/Repository/Filtering/BaseRepositoryFilteringTestCase.php b/tests/integration/Core/Repository/Filtering/BaseRepositoryFilteringTestCase.php index 6118035a9a..9c6f929641 100644 --- a/tests/integration/Core/Repository/Filtering/BaseRepositoryFilteringTestCase.php +++ b/tests/integration/Core/Repository/Filtering/BaseRepositoryFilteringTestCase.php @@ -75,7 +75,7 @@ public function testFind(callable $filterFactory): void protected function setUp(): void { parent::setUp(); - $this->contentProvider = new TestContentProvider($this->getRepository(false), $this); + $this->contentProvider = new TestContentProvider($this->getRepository(true), $this); } /** @@ -268,7 +268,7 @@ public function getCriteriaForInitialData(): iterable yield 'Sibling IN 2, 1]' => new Criterion\Sibling(2, 1); yield 'Subtree=/1/2/' => new Criterion\Subtree('/1/2/'); yield 'UserEmail=admin@link.invalid' => new Criterion\UserEmail('admin@link.invalid'); - yield 'UserEmail=admin@*' => new Criterion\UserEmail('*@link.invalid', Criterion\Operator::LIKE); + yield 'UserEmail=admin@*' => new Criterion\UserEmail('admin@*', Criterion\Operator::LIKE); yield 'UserId=14' => new Criterion\UserId(14); yield 'UserLogin=admin' => new Criterion\UserLogin('admin'); yield 'UserLogin=a*' => new Criterion\UserLogin('a*', Criterion\Operator::LIKE); diff --git a/tests/integration/Core/Repository/Filtering/ContentFilteringTest.php b/tests/integration/Core/Repository/Filtering/ContentFilteringTest.php index 5a26a80163..52ef1722c9 100644 --- a/tests/integration/Core/Repository/Filtering/ContentFilteringTest.php +++ b/tests/integration/Core/Repository/Filtering/ContentFilteringTest.php @@ -10,8 +10,10 @@ use function array_map; use function count; +use Ibexa\Contracts\Core\Repository\LocationService; use Ibexa\Contracts\Core\Repository\Values\Content\Content; use Ibexa\Contracts\Core\Repository\Values\Content\ContentList; +use Ibexa\Contracts\Core\Repository\Values\Content\Location; use Ibexa\Contracts\Core\Repository\Values\Content\Query; use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause; @@ -22,6 +24,7 @@ use Ibexa\Contracts\Core\Repository\Values\ObjectState\ObjectStateGroupCreateStruct; use Ibexa\Core\FieldType\Keyword; use Ibexa\Tests\Core\Repository\Filtering\TestContentProvider; +use function iterator_to_array; use IteratorAggregate; use function sprintf; @@ -81,6 +84,148 @@ public function testFindWithLocationSortClauses(): void ); } + public function testLocationSortClausesUseMainLocationDuringContentFiltering(): void + { + $repository = $this->getRepository(false); + $locationService = $repository->getLocationService(); + $contentService = $repository->getContentService(); + + $shallowParent = $this->createFolder( + ['eng-GB' => 'Shallow Parent'], + 2 + ); + $referenceContent = $this->createFolder( + ['eng-GB' => 'Reference folder'], + $shallowParent->getContentInfo()->getMainLocationId() + ); + $deepParent = $this->createFolder( + ['eng-GB' => 'Deep Parent'], + $referenceContent->getContentInfo()->getMainLocationId() + ); + $contentWithAdditionalLocation = $this->createFolder( + ['eng-GB' => 'Folder with extra location'], + $deepParent->getContentInfo()->getMainLocationId() + ); + $locationService->createLocation( + $contentWithAdditionalLocation->contentInfo, + $locationService->newLocationCreateStruct(2) + ); + + $mainLocation = $this->loadMainLocation($locationService, $contentWithAdditionalLocation); + $nonMainLocations = []; + foreach ($locationService->loadLocations($contentWithAdditionalLocation->contentInfo) as $location) { + if ($location->id !== $contentWithAdditionalLocation->getContentInfo()->getMainLocationId()) { + $nonMainLocations[] = $location; + } + } + self::assertNotEmpty($nonMainLocations); + $nonMainLocation = $nonMainLocations[0]; + $referenceLocation = $this->loadMainLocation($locationService, $referenceContent); + + self::assertLessThan($referenceLocation->depth, $nonMainLocation->depth); + self::assertLessThan($mainLocation->depth, $referenceLocation->depth); + + $filter = (new Filter()) + ->withCriterion( + new Criterion\ContentId( + [ + $contentWithAdditionalLocation->id, + $referenceContent->id, + ] + ) + ) + ->withSortClause(new SortClause\Location\Depth(Query::SORT_ASC)); + + $contentList = $contentService->find($filter); + + self::assertSame( + [$referenceContent->id, $contentWithAdditionalLocation->id], + array_map( + static function (Content $content): int { + return $content->id; + }, + iterator_to_array($contentList) + ) + ); + } + + public function testLocationSortClausesStayDeterministicWithComplexCriteria(): void + { + $repository = $this->getRepository(false); + $locationService = $repository->getLocationService(); + $contentService = $repository->getContentService(); + + $shallowParent = $this->createFolder( + ['eng-GB' => 'Complex Root'], + 2 + ); + $referenceContent = $this->createFolder( + ['eng-GB' => 'Ref folder'], + $shallowParent->getContentInfo()->getMainLocationId() + ); + $middleContent = $this->createFolder( + ['eng-GB' => 'Middle folder'], + $referenceContent->getContentInfo()->getMainLocationId() + ); + $deepParent = $this->createFolder( + ['eng-GB' => 'Deep intermediate'], + $middleContent->getContentInfo()->getMainLocationId() + ); + $contentWithAdditionalLocation = $this->createFolder( + ['eng-GB' => 'Folder with randomizing location'], + $deepParent->getContentInfo()->getMainLocationId() + ); + $locationService->createLocation( + $contentWithAdditionalLocation->contentInfo, + $locationService->newLocationCreateStruct(2) + ); + + $mainLocation = $this->loadMainLocation($locationService, $contentWithAdditionalLocation); + $nonMainLocations = []; + foreach ($locationService->loadLocations($contentWithAdditionalLocation->contentInfo) as $location) { + if ($location->id !== $contentWithAdditionalLocation->getContentInfo()->getMainLocationId()) { + $nonMainLocations[] = $location; + } + } + self::assertNotEmpty($nonMainLocations); + $nonMainLocation = $nonMainLocations[0]; + self::assertNotEquals($mainLocation->depth, $nonMainLocation->depth); + + $shallowParentLocation = $this->loadMainLocation($locationService, $shallowParent); + + $filter = (new Filter()) + ->withCriterion(new Criterion\Subtree($shallowParentLocation->pathString)) + ->andWithCriterion(new Criterion\ContentTypeIdentifier('folder')) + ->andWithCriterion( + new Criterion\ContentId( + [ + $referenceContent->id, + $middleContent->id, + $contentWithAdditionalLocation->id, + ] + ) + ) + ->withSortClause(new SortClause\Location\Depth(Query::SORT_ASC)) + ->withSortClause(new SortClause\ContentId(Query::SORT_ASC)) + ->withLimit(10); + + $contentList = $contentService->find($filter); + + self::assertSame( + [ + $referenceContent->id, + $middleContent->id, + $contentWithAdditionalLocation->id, + ], + array_map( + static function (Content $content): int { + return $content->id; + }, + iterator_to_array($contentList) + ) + ); + } + /** * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException * @throws \Exception @@ -158,7 +303,7 @@ static function (Content $parentFolder): Filter { private function createMultiplePagesOfContentItems(int $pageSize, int $noOfPages): int { $parentFolder = $this->createFolder(['eng-GB' => 'Parent Folder'], 2); - $parentFolderMainLocationId = $parentFolder->contentInfo->mainLocationId; + $parentFolderMainLocationId = $parentFolder->getContentInfo()->getMainLocationId(); $noOfItems = $pageSize * $noOfPages; for ($itemNo = 1; $itemNo <= $noOfItems; ++$itemNo) { @@ -256,7 +401,7 @@ public function testFindContentUsingLocationCriterion( public function getDataForTestFindContentWithLocationCriterion(): iterable { - yield 'Content items with secondary Location, sorted by Content ID' => [ + yield 'Content items with secondary Location ignored in content filtering, sorted by Content ID' => [ static function (Content $parentFolder): Filter { return (new Filter()) ->withCriterion( @@ -266,7 +411,7 @@ static function (Content $parentFolder): Filter { ) ->withSortClause(new SortClause\ContentId(Query::SORT_ASC)); }, - [TestContentProvider::CONTENT_REMOTE_IDS['folder2']], + [], ]; yield 'Folders with Location, sorted by Content ID' => [ @@ -278,7 +423,7 @@ static function (Content $parentFolder): Filter { ) ) ->andWithCriterion( - new Criterion\ParentLocationId($parentFolder->contentInfo->mainLocationId) + new Criterion\ParentLocationId($parentFolder->getContentInfo()->getMainLocationId()) ) ->andWithCriterion( new Criterion\ContentTypeIdentifier('folder') @@ -300,12 +445,12 @@ protected function assertFoundContentItemsByRemoteIds( foreach ($list as $content) { /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Content $content */ self::assertContainsEquals( - $content->contentInfo->remoteId, + $content->getContentInfo()->remoteId, $expectedContentRemoteIds, sprintf( 'Content %d (%s) was not supposed to be found', $content->id, - $content->contentInfo->remoteId + $content->getContentInfo()->remoteId ) ); } @@ -447,6 +592,14 @@ private function buildSearchQueryFromFilter(Filter $filter): Query ] ); } + + private function loadMainLocation(LocationService $locationService, Content $content): Location + { + $mainLocationId = $content->getContentInfo()->getMainLocationId(); + self::assertNotNull($mainLocationId); + + return $locationService->loadLocation($mainLocationId); + } } class_alias(ContentFilteringTest::class, 'eZ\Publish\API\Repository\Tests\Filtering\ContentFilteringTest'); diff --git a/tests/integration/Core/Repository/Filtering/Fixtures/LegacyLocationSortClause.php b/tests/integration/Core/Repository/Filtering/Fixtures/LegacyLocationSortClause.php new file mode 100644 index 0000000000..94c5176f52 --- /dev/null +++ b/tests/integration/Core/Repository/Filtering/Fixtures/LegacyLocationSortClause.php @@ -0,0 +1,20 @@ +createFolderWithRemoteId('legacy-parent', 'Legacy Parent'); + $folder1 = $this->createFolderWithRemoteId( + 'legacy-folder-1', + 'Legacy Folder 1', + (int)$parentFolder->getContentInfo()->getMainLocationId() + ); + $folder2 = $this->createFolderWithRemoteId( + 'legacy-folder-2', + 'Legacy Folder 2', + (int)$parentFolder->getContentInfo()->getMainLocationId() + ); + + $filter = (new Filter()) + ->withCriterion( + new Criterion\ParentLocationId((int)$parentFolder->getContentInfo()->getMainLocationId()) + ) + ->andWithCriterion( + new Criterion\ContentTypeIdentifier('folder') + ) + ->withSortClause(new LegacyLocationSortClause(Query::SORT_ASC)); + + $contentService = self::getContentService(); + $list = $contentService->find($filter, []); + + self::assertCount(2, $list); + $remoteIds = array_map( + static fn ($content): string => $content->getContentInfo()->remoteId, + iterator_to_array($list) + ); + self::assertSame( + [ + $folder1->getContentInfo()->remoteId, + $folder2->getContentInfo()->remoteId, + ], + $remoteIds + ); + } + + protected static function getKernelClass(): string + { + return LegacyTestKernel::class; + } + + private function createFolderWithRemoteId( + string $remoteId, + string $name, + int $parentLocationId = self::CONTENT_TREE_ROOT_ID + ): Content { + $contentService = self::getContentService(); + $contentTypeService = self::getContentTypeService(); + $locationService = self::getLocationService(); + + /** @var \Ibexa\Contracts\Core\Repository\Values\ContentType\ContentType $folderType */ + $folderType = $contentTypeService->loadContentTypeByIdentifier('folder'); + $createStruct = $contentService->newContentCreateStruct($folderType, 'eng-GB'); + $createStruct->setField('name', $name, 'eng-GB'); + $createStruct->remoteId = $remoteId; + + $locationCreateStruct = $locationService->newLocationCreateStruct($parentLocationId); + + $draft = $contentService->createContent( + $createStruct, + [$locationCreateStruct] + ); + + return $contentService->publishVersion($draft->versionInfo); + } +} diff --git a/tests/integration/Core/Repository/Filtering/LegacyTestKernel.php b/tests/integration/Core/Repository/Filtering/LegacyTestKernel.php new file mode 100644 index 0000000000..17f1a93366 --- /dev/null +++ b/tests/integration/Core/Repository/Filtering/LegacyTestKernel.php @@ -0,0 +1,22 @@ +load(__DIR__ . '/Resources/config/services/legacy_sort_clause.yaml'); + } +} diff --git a/tests/integration/Core/Repository/Filtering/Resources/config/services/legacy_sort_clause.yaml b/tests/integration/Core/Repository/Filtering/Resources/config/services/legacy_sort_clause.yaml new file mode 100644 index 0000000000..ea83b5876e --- /dev/null +++ b/tests/integration/Core/Repository/Filtering/Resources/config/services/legacy_sort_clause.yaml @@ -0,0 +1,4 @@ +services: + Ibexa\Tests\Integration\Core\Repository\Filtering\Fixtures\LegacyLocationSortQueryBuilder: + tags: + - { name: ibexa.filter.sort_clause.query.builder } diff --git a/tests/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/BaseLocationSortClauseQueryBuilderTest.php b/tests/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/BaseLocationSortClauseQueryBuilderTest.php new file mode 100644 index 0000000000..1ab851fee3 --- /dev/null +++ b/tests/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/BaseLocationSortClauseQueryBuilderTest.php @@ -0,0 +1,62 @@ + 'sqlite:///:memory:']); + $queryBuilder = new FilteringQueryBuilder($connection); + + $sortClause = new class() implements FilteringSortClause { + public string $direction = Query::SORT_ASC; + }; + + $builder = new class() extends BaseLocationSortClauseQueryBuilder { + protected function getSortingExpression(): string + { + return 'location.depth'; + } + + public function accepts(FilteringSortClause $sortClause): bool + { + return true; + } + }; + + $builder->buildQuery($queryBuilder, $sortClause); + + self::assertSame( + ['ibexa_sort_location.depth AS ibexa_filter_sort_ibexa_sort_location_depth'], + $queryBuilder->getQueryPart('select') + ); + + $joins = $queryBuilder->getQueryPart('join'); + self::assertArrayHasKey('content', $joins); + self::assertCount(1, $joins['content']); + self::assertSame(LocationGateway::CONTENT_TREE_TABLE, $joins['content'][0]['joinTable']); + self::assertSame('ibexa_sort_location', $joins['content'][0]['joinAlias']); + self::assertSame( + '(content.id = ibexa_sort_location.contentobject_id) AND (ibexa_sort_location.node_id = ibexa_sort_location.main_node_id)', + (string)$joins['content'][0]['joinCondition'] + ); + + $orderBy = $queryBuilder->getQueryPart('orderBy'); + self::assertSame(['ibexa_filter_sort_ibexa_sort_location_depth ASC'], $orderBy); + } +}