From 2238c9711b5100694f99b6b01a04ecc7a2a2d1c8 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Sat, 22 Nov 2025 00:14:51 +0000 Subject: [PATCH 1/4] IBX-10186 Add subtree limit queries --- phpstan-baseline.neon | 182 ++++++++++++++++-- .../Repository/FutureContentService.php | 23 +++ .../Repository/FutureLocationService.php | 34 ++++ .../Persistence/Content/Location/Handler.php | 2 +- .../Persistence/Filter/Content/Handler.php | 2 +- .../Persistence/Filter/Location/Handler.php | 2 +- src/contracts/Repository/ContentService.php | 5 +- .../Decorator/ContentServiceDecorator.php | 4 +- .../Decorator/LocationServiceDecorator.php | 12 +- src/contracts/Repository/LocationService.php | 11 +- src/lib/Persistence/Cache/LocationHandler.php | 5 +- .../Legacy/Content/Location/Gateway.php | 2 +- .../Location/Gateway/DoctrineDatabase.php | 16 +- .../Location/Gateway/ExceptionConversion.php | 4 +- .../Legacy/Content/Location/Handler.php | 4 +- .../Content/Doctrine/DoctrineGateway.php | 15 +- .../Legacy/Filter/Gateway/Gateway.php | 4 +- .../Location/Doctrine/DoctrineGateway.php | 15 +- .../Handler/ContentFilteringHandler.php | 4 +- .../Handler/LocationFilteringHandler.php | 4 +- .../Filter/Query/LimitedCountQueryBuilder.php | 71 +++++++ src/lib/Repository/ContentService.php | 4 +- src/lib/Repository/LocationService.php | 16 +- .../SiteAccessAware/ContentService.php | 5 +- .../SiteAccessAware/LocationService.php | 16 +- .../Resources/settings/repository/inner.yml | 2 + .../storage_engines/legacy/filter.yaml | 4 + .../storage_engines/legacy/location.yml | 1 + .../Core/Repository/LocationServiceTest.php | 64 ++++++ .../Location/Gateway/DoctrineDatabaseTest.php | 3 +- .../Gateway/DoctrineDatabaseTrashTest.php | 3 +- .../Content/UrlAlias/UrlAliasHandlerTest.php | 3 +- .../Query/LimitedCountQueryBuilderTest.php | 73 +++++++ tests/lib/Persistence/Legacy/TestCase.php | 6 + .../LocationServiceDecoratorTest.php | 2 +- 35 files changed, 556 insertions(+), 67 deletions(-) create mode 100644 src/contracts/Persistence/Content/Future/Repository/FutureContentService.php create mode 100644 src/contracts/Persistence/Content/Future/Repository/FutureLocationService.php create mode 100644 src/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilder.php create mode 100644 tests/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilderTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 6aff62b7aa..c589efd171 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -4680,6 +4680,12 @@ parameters: count: 1 path: src/contracts/Persistence/Content/VersionInfo.php + - + message: '#^PHPDoc tag @param references unknown parameter\: \$limit$#' + identifier: parameter.notFound + count: 1 + path: src/contracts/Persistence/Filter/Content/Handler.php + - message: '#^Method Ibexa\\Contracts\\Core\\Persistence\\Filter\\Doctrine\\FilteringQueryBuilder\:\:buildOperatorBasedCriterionConstraint\(\) has parameter \$criterionValue with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue @@ -4710,6 +4716,24 @@ parameters: count: 1 path: src/contracts/Persistence/Filter/LazyListIterator.php + - + message: '#^Method Ibexa\\Contracts\\Core\\Persistence\\Handler\:\:beginTransaction\(\) has no return type specified\.$#' + identifier: missingType.return + count: 1 + path: src/contracts/Persistence/Handler.php + + - + message: '#^Method Ibexa\\Contracts\\Core\\Persistence\\Handler\:\:commit\(\) has no return type specified\.$#' + identifier: missingType.return + count: 1 + path: src/contracts/Persistence/Handler.php + + - + message: '#^Method Ibexa\\Contracts\\Core\\Persistence\\Handler\:\:rollback\(\) has no return type specified\.$#' + identifier: missingType.return + count: 1 + path: src/contracts/Persistence/Handler.php + - message: '#^Property Ibexa\\Contracts\\Core\\Persistence\\Notification\\CreateStruct\:\:\$data type has no value type specified in iterable type array\.$#' identifier: missingType.iterableValue @@ -4848,6 +4872,18 @@ parameters: count: 1 path: src/contracts/Repository/ContentService.php + - + message: '#^PHPDoc tag @param references unknown parameter\: \$limit$#' + identifier: parameter.notFound + count: 1 + path: src/contracts/Repository/ContentService.php + + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\ContentService\:\:count\(\) invoked with 3 parameters, 1\-2 required\.$#' + identifier: arguments.count + count: 1 + path: src/contracts/Repository/Decorator/ContentServiceDecorator.php + - message: '#^Method Ibexa\\Contracts\\Core\\Repository\\Decorator\\ContentServiceDecorator\:\:validate\(\) has parameter \$context with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue @@ -4860,12 +4896,42 @@ parameters: count: 1 path: src/contracts/Repository/Decorator/ContentServiceDecorator.php + - + message: '#^PHPDoc tag @param references unknown parameter\: \$limit$#' + identifier: parameter.notFound + count: 1 + path: src/contracts/Repository/Decorator/ContentServiceDecorator.php + - message: '#^Method Ibexa\\Contracts\\Core\\Repository\\Decorator\\LocationServiceDecorator\:\:loadLocationList\(\) has parameter \$locationIds with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue count: 1 path: src/contracts/Repository/Decorator/LocationServiceDecorator.php + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:count\(\) invoked with 3 parameters, 1\-2 required\.$#' + identifier: arguments.count + count: 1 + path: src/contracts/Repository/Decorator/LocationServiceDecorator.php + + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:getLocationChildCount\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 1 + path: src/contracts/Repository/Decorator/LocationServiceDecorator.php + + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:getSubtreeSize\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 1 + path: src/contracts/Repository/Decorator/LocationServiceDecorator.php + + - + message: '#^PHPDoc tag @param references unknown parameter\: \$limit$#' + identifier: parameter.notFound + count: 3 + path: src/contracts/Repository/Decorator/LocationServiceDecorator.php + - message: '#^Method Ibexa\\Contracts\\Core\\Repository\\Decorator\\SearchServiceDecorator\:\:suggest\(\) has no return type specified\.$#' identifier: missingType.return @@ -5400,6 +5466,12 @@ parameters: count: 1 path: src/contracts/Repository/LocationService.php + - + message: '#^PHPDoc tag @param references unknown parameter\: \$limit$#' + identifier: parameter.notFound + count: 3 + path: src/contracts/Repository/LocationService.php + - message: '#^PHPDoc tag @param for parameter \$objectStateGroupId with type mixed is not subtype of native type int\.$#' identifier: parameter.phpDocType @@ -10350,6 +10422,12 @@ parameters: count: 1 path: src/lib/Persistence/Cache/LocationHandler.php + - + message: '#^Method Ibexa\\Contracts\\Core\\Persistence\\Content\\Location\\Handler\:\:getSubtreeSize\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 1 + path: src/lib/Persistence/Cache/LocationHandler.php + - message: '#^Method Ibexa\\Core\\Persistence\\Cache\\LocationHandler\:\:changeMainLocation\(\) has no return type specified\.$#' identifier: missingType.return @@ -16650,6 +16728,12 @@ parameters: count: 1 path: src/lib/Repository/ContentService.php + - + message: '#^Method Ibexa\\Contracts\\Core\\Persistence\\Filter\\Content\\Handler\:\:count\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 1 + path: src/lib/Repository/ContentService.php + - message: '#^Method Ibexa\\Core\\Repository\\ContentService\:\:__construct\(\) has parameter \$settings with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue @@ -16704,6 +16788,12 @@ parameters: count: 1 path: src/lib/Repository/ContentService.php + - + message: '#^PHPDoc tag @param has invalid value \(\\Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Language\|null if not set the draft is created with the initialLanguage code of the source version or if not present with the main language\.\)\: Unexpected token "if", expected variable at offset 565 on line 9$#' + identifier: phpDoc.parseError + count: 1 + path: src/lib/Repository/ContentService.php + - message: '#^PHPDoc tag @var has invalid value \(\$content \\Ibexa\\Core\\Repository\\Values\\Content\\Content\)\: Unexpected token "\$content", expected type at offset 9 on line 1$#' identifier: phpDoc.parseError @@ -16914,12 +17004,24 @@ parameters: count: 1 path: src/lib/Repository/LocationService.php + - + message: '#^Method Ibexa\\Contracts\\Core\\Persistence\\Content\\Location\\Handler\:\:getSubtreeSize\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 1 + path: src/lib/Repository/LocationService.php + - message: '#^Method Ibexa\\Contracts\\Core\\Persistence\\Content\\UrlAlias\\Handler\:\:publishUrlAliasForLocation\(\) invoked with 6 parameters, 4\-5 required\.$#' identifier: arguments.count count: 1 path: src/lib/Repository/LocationService.php + - + message: '#^Method Ibexa\\Contracts\\Core\\Persistence\\Filter\\Location\\Handler\:\:count\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 1 + path: src/lib/Repository/LocationService.php + - message: '#^Method Ibexa\\Core\\Repository\\LocationService\:\:__construct\(\) has parameter \$settings with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue @@ -16932,6 +17034,12 @@ parameters: count: 1 path: src/lib/Repository/LocationService.php + - + message: '#^PHPDoc tag @param references unknown parameter\: \$limit$#' + identifier: parameter.notFound + count: 2 + path: src/lib/Repository/LocationService.php + - message: '#^Property Ibexa\\Core\\Repository\\LocationService\:\:\$repository \(Ibexa\\Core\\Repository\\Repository\) does not accept Ibexa\\Contracts\\Core\\Repository\\Repository\.$#' identifier: assign.propertyType @@ -17814,6 +17922,12 @@ parameters: count: 1 path: src/lib/Repository/SettingService.php + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\ContentService\:\:count\(\) invoked with 3 parameters, 1\-2 required\.$#' + identifier: arguments.count + count: 1 + path: src/lib/Repository/SiteAccessAware/ContentService.php + - message: '#^Method Ibexa\\Core\\Repository\\SiteAccessAware\\ContentService\:\:validate\(\) has parameter \$context with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue @@ -17826,6 +17940,12 @@ parameters: count: 1 path: src/lib/Repository/SiteAccessAware/ContentService.php + - + message: '#^PHPDoc tag @param references unknown parameter\: \$limit$#' + identifier: parameter.notFound + count: 1 + path: src/lib/Repository/SiteAccessAware/ContentService.php + - message: '#^Method Ibexa\\Core\\Repository\\SiteAccessAware\\Language\\AbstractLanguageResolver\:\:getPrioritizedLanguages\(\) has parameter \$forcedLanguages with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue @@ -17844,12 +17964,36 @@ parameters: count: 1 path: src/lib/Repository/SiteAccessAware/Language/LanguageResolver.php + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:count\(\) invoked with 3 parameters, 1\-2 required\.$#' + identifier: arguments.count + count: 1 + path: src/lib/Repository/SiteAccessAware/LocationService.php + + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:getLocationChildCount\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 1 + path: src/lib/Repository/SiteAccessAware/LocationService.php + + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:getSubtreeSize\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 1 + path: src/lib/Repository/SiteAccessAware/LocationService.php + - message: '#^Method Ibexa\\Core\\Repository\\SiteAccessAware\\LocationService\:\:loadLocationList\(\) has parameter \$locationIds with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue count: 1 path: src/lib/Repository/SiteAccessAware/LocationService.php + - + message: '#^PHPDoc tag @param references unknown parameter\: \$limit$#' + identifier: parameter.notFound + count: 2 + path: src/lib/Repository/SiteAccessAware/LocationService.php + - message: '#^Property Ibexa\\Core\\Repository\\SiteAccessAware\\Repository\:\:\$notificationService \(Ibexa\\Core\\Repository\\NotificationService\) does not accept Ibexa\\Core\\Repository\\SiteAccessAware\\NotificationService\.$#' identifier: assign.propertyType @@ -32433,7 +32577,7 @@ parameters: - message: '#^Cannot access property \$id on Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Location\|null\.$#' identifier: property.nonObject - count: 11 + count: 9 path: tests/integration/Core/Repository/LocationServiceTest.php - @@ -32442,6 +32586,18 @@ parameters: count: 2 path: tests/integration/Core/Repository/LocationServiceTest.php + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:getLocationChildCount\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 1 + path: tests/integration/Core/Repository/LocationServiceTest.php + + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:getSubtreeSize\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 2 + path: tests/integration/Core/Repository/LocationServiceTest.php + - message: '#^Method Ibexa\\Tests\\Integration\\Core\\Repository\\LocationServiceTest\:\:assertAliasesBeforeCopy\(\) has no return type specified\.$#' identifier: missingType.return @@ -32640,12 +32796,6 @@ parameters: count: 1 path: tests/integration/Core/Repository/LocationServiceTest.php - - - message: '#^Method Ibexa\\Tests\\Integration\\Core\\Repository\\LocationServiceTest\:\:testGetSubtreeSize\(\) should return Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Location but returns Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Location\|null\.$#' - identifier: return.type - count: 1 - path: tests/integration/Core/Repository/LocationServiceTest.php - - message: '#^Method Ibexa\\Tests\\Integration\\Core\\Repository\\LocationServiceTest\:\:testHideLocation\(\) has no return type specified\.$#' identifier: missingType.return @@ -32898,12 +33048,6 @@ parameters: count: 1 path: tests/integration/Core/Repository/LocationServiceTest.php - - - message: '#^Parameter \#1 \$location of method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:getSubtreeSize\(\) expects Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Location, Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Location\|null given\.$#' - identifier: argument.type - count: 2 - path: tests/integration/Core/Repository/LocationServiceTest.php - - message: '#^Parameter \#1 \$location of method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:moveSubtree\(\) expects Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Location, Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Location\|null given\.$#' identifier: argument.type @@ -52770,6 +52914,12 @@ parameters: count: 1 path: tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/LogicalOperatorQueryBuilderQueryBuilderTest.php + - + message: '#^Parameter \#3 \$limit of method Ibexa\\Core\\Persistence\\Legacy\\Filter\\Query\\LimitedCountQueryBuilder\:\:wrap\(\) expects int\<1, max\>\|null, 0 given\.$#' + identifier: argument.type + count: 1 + path: tests/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilderTest.php + - message: '#^Call to an undefined method Ibexa\\Contracts\\Core\\Container\:\:get\(\)\.$#' identifier: method.notFound @@ -54456,6 +54606,12 @@ parameters: count: 1 path: tests/lib/Repository/Decorator/LanguageServiceDecoratorTest.php + - + message: '#^Method Ibexa\\Contracts\\Core\\Repository\\LocationService\:\:getLocationChildCount\(\) invoked with 2 parameters, 1 required\.$#' + identifier: arguments.count + count: 1 + path: tests/lib/Repository/Decorator/LocationServiceDecoratorTest.php + - message: '#^Method Ibexa\\Tests\\Core\\Repository\\Decorator\\LocationServiceDecoratorTest\:\:testCopySubtreeDecorator\(\) has no return type specified\.$#' identifier: missingType.return diff --git a/src/contracts/Persistence/Content/Future/Repository/FutureContentService.php b/src/contracts/Persistence/Content/Future/Repository/FutureContentService.php new file mode 100644 index 0000000000..20e2a14707 --- /dev/null +++ b/src/contracts/Persistence/Content/Future/Repository/FutureContentService.php @@ -0,0 +1,23 @@ + + * */ - public function count(Filter $filter, ?array $languages = null): int; + public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int; } diff --git a/src/contracts/Repository/Decorator/ContentServiceDecorator.php b/src/contracts/Repository/Decorator/ContentServiceDecorator.php index c9d6c65038..760091598d 100644 --- a/src/contracts/Repository/Decorator/ContentServiceDecorator.php +++ b/src/contracts/Repository/Decorator/ContentServiceDecorator.php @@ -284,8 +284,8 @@ public function find(Filter $filter, ?array $languages = null): ContentList return $this->innerService->find($filter, $languages); } - public function count(Filter $filter, ?array $languages = null): int + public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int { - return $this->innerService->count($filter, $languages); + return $this->innerService->count($filter, $languages, $limit); } } diff --git a/src/contracts/Repository/Decorator/LocationServiceDecorator.php b/src/contracts/Repository/Decorator/LocationServiceDecorator.php index 9cdf51d075..259779aebb 100644 --- a/src/contracts/Repository/Decorator/LocationServiceDecorator.php +++ b/src/contracts/Repository/Decorator/LocationServiceDecorator.php @@ -81,14 +81,14 @@ public function loadParentLocationsForDraftContent( return $this->innerService->loadParentLocationsForDraftContent($versionInfo, $prioritizedLanguages); } - public function getLocationChildCount(Location $location): int + public function getLocationChildCount(Location $location, ?int $limit = null): int { - return $this->innerService->getLocationChildCount($location); + return $this->innerService->getLocationChildCount($location, $limit); } - public function getSubtreeSize(Location $location): int + public function getSubtreeSize(Location $location, ?int $limit = null): int { - return $this->innerService->getSubtreeSize($location); + return $this->innerService->getSubtreeSize($location, $limit); } public function createLocation( @@ -159,8 +159,8 @@ public function find(Filter $filter, ?array $languages = null): LocationList return $this->innerService->find($filter, $languages); } - public function count(Filter $filter, ?array $languages = null): int + public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int { - return $this->innerService->count($filter, $languages); + return $this->innerService->count($filter, $languages, $limit); } } diff --git a/src/contracts/Repository/LocationService.php b/src/contracts/Repository/LocationService.php index 79136f9841..deb40da35d 100644 --- a/src/contracts/Repository/LocationService.php +++ b/src/contracts/Repository/LocationService.php @@ -123,17 +123,20 @@ public function loadParentLocationsForDraftContent(VersionInfo $versionInfo, ?ar * Returns the number of children which are readable by the current user of a location object. * * @param \Ibexa\Contracts\Core\Repository\Values\Content\Location $location + * @param int|null $limit If set, the count will be limited to first $limit items found. * * @return int */ - public function getLocationChildCount(Location $location): int; + public function getLocationChildCount(Location $location, ?int $limit = null): int; /** * Return the subtree size of a given location. * * Warning! This method is not permission aware by design. + * + * @param int|null $limit */ - public function getSubtreeSize(Location $location): int; + public function getSubtreeSize(Location $location, ?int $limit = null): int; /** * Creates the new $location in the content repository for the given content. @@ -280,6 +283,8 @@ public function find(Filter $filter, ?array $languages = null): LocationList; * @param array|null $languages a list of language codes to be added as additional constraints. * If skipped, by default, unless SiteAccessAware layer has been disabled, languages set * for a SiteAccess in a current context will be used. + * @param int|null $limit If set, the count will be limited to first $limit items found. + * In some cases it can significantly speed up a count operation for more complex filters. */ - public function count(Filter $filter, ?array $languages = null): int; + public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int; } diff --git a/src/lib/Persistence/Cache/LocationHandler.php b/src/lib/Persistence/Cache/LocationHandler.php index 52091c2eb7..da55c46387 100644 --- a/src/lib/Persistence/Cache/LocationHandler.php +++ b/src/lib/Persistence/Cache/LocationHandler.php @@ -257,13 +257,14 @@ public function copySubtree($sourceId, $destinationParentId, $newOwnerId = null) return $this->persistenceHandler->locationHandler()->copySubtree($sourceId, $destinationParentId, $newOwnerId); } - public function getSubtreeSize(string $path): int + public function getSubtreeSize(string $path, ?int $limit = null): int { $this->logger->logCall(__METHOD__, [ 'path' => $path, + 'limit' => $limit, ]); - return $this->persistenceHandler->locationHandler()->getSubtreeSize($path); + return $this->persistenceHandler->locationHandler()->getSubtreeSize($path, $limit); } /** diff --git a/src/lib/Persistence/Legacy/Content/Location/Gateway.php b/src/lib/Persistence/Legacy/Content/Location/Gateway.php index 5616d64f2f..87dcfbe5cc 100644 --- a/src/lib/Persistence/Legacy/Content/Location/Gateway.php +++ b/src/lib/Persistence/Legacy/Content/Location/Gateway.php @@ -128,7 +128,7 @@ abstract public function getSubtreeNodeIdToContentIdMap(int $sourceId): array; */ abstract public function getSubtreeChildrenDraftContentIds(int $sourceId): array; - abstract public function getSubtreeSize(string $path): int; + abstract public function getSubtreeSize(string $path, ?int $limit = null): int; /** * Returns data for the first level children of the location identified by given $locationId. diff --git a/src/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php index bfdfab4534..fbc47b203b 100644 --- a/src/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php +++ b/src/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php @@ -24,6 +24,7 @@ use Ibexa\Core\Persistence\Legacy\Content\Gateway as ContentGateway; use Ibexa\Core\Persistence\Legacy\Content\Language\MaskGenerator; use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway; +use Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder; use Ibexa\Core\Search\Legacy\Content\Common\Gateway\CriteriaConverter; use Ibexa\Core\Search\Legacy\Content\Common\Gateway\SortClauseConverter; use RuntimeException; @@ -40,11 +41,13 @@ final class DoctrineDatabase extends Gateway { public const string NODE_ASSIGNMENT_TABLE = 'ibexa_node_assignment'; + public function __construct( private readonly Connection $connection, private readonly MaskGenerator $languageMaskGenerator, private readonly CriteriaConverter $trashCriteriaConverter, - private readonly SortClauseConverter $trashSortClauseConverter + private readonly SortClauseConverter $trashSortClauseConverter, + private readonly LimitedCountQueryBuilder $limitedCountQueryBuilder ) { } @@ -249,7 +252,10 @@ public function getSubtreeChildrenDraftContentIds(int $sourceId): array return $query->executeQuery()->fetchFirstColumn(); } - public function getSubtreeSize(string $path): int + /** + * @phpstan-param positive-int $limit + */ + public function getSubtreeSize(string $path, ?int $limit = null): int { $query = $this->createNodeQueryBuilder(['COUNT(node_id)']); $query->andWhere( @@ -261,6 +267,12 @@ public function getSubtreeSize(string $path): int ) ); + $query = $this->limitedCountQueryBuilder->wrap( + $query, + 't.node_id', + $limit + ); + return (int) $query->executeQuery()->fetchOne(); } diff --git a/src/lib/Persistence/Legacy/Content/Location/Gateway/ExceptionConversion.php b/src/lib/Persistence/Legacy/Content/Location/Gateway/ExceptionConversion.php index 4208552cc7..f16d1fe069 100644 --- a/src/lib/Persistence/Legacy/Content/Location/Gateway/ExceptionConversion.php +++ b/src/lib/Persistence/Legacy/Content/Location/Gateway/ExceptionConversion.php @@ -128,10 +128,10 @@ public function getSubtreeChildrenDraftContentIds(int $sourceId): array } } - public function getSubtreeSize(string $path): int + public function getSubtreeSize(string $path, ?int $limit = null): int { try { - return $this->innerGateway->getSubtreeSize($path); + return $this->innerGateway->getSubtreeSize($path, $limit); } catch (DBALException | PDOException $e) { throw DatabaseException::wrap($e); } diff --git a/src/lib/Persistence/Legacy/Content/Location/Handler.php b/src/lib/Persistence/Legacy/Content/Location/Handler.php index 078f42daa0..8459ae94da 100644 --- a/src/lib/Persistence/Legacy/Content/Location/Handler.php +++ b/src/lib/Persistence/Legacy/Content/Location/Handler.php @@ -327,9 +327,9 @@ public function copySubtree($sourceId, $destinationParentId, $newOwnerId = null) return $copiedSubtreeRootLocation; } - public function getSubtreeSize(string $path): int + public function getSubtreeSize(string $path, ?int $limit = null): int { - return $this->locationGateway->getSubtreeSize($path); + return $this->locationGateway->getSubtreeSize($path, $limit); } /** 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 6221a55268..35bafb4b37 100644 --- a/src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php +++ b/src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php @@ -19,6 +19,7 @@ use Ibexa\Core\Persistence\Legacy\Content\Gateway as ContentGateway; use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway; use Ibexa\Core\Persistence\Legacy\Filter\Gateway\Gateway; +use Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder; use function iterator_to_array; use function sprintf; use Traversable; @@ -59,17 +60,27 @@ final class DoctrineGateway implements Gateway public function __construct( private readonly Connection $connection, private readonly CriterionVisitor $criterionVisitor, - private readonly SortClauseVisitor $sortClauseVisitor + private readonly SortClauseVisitor $sortClauseVisitor, + private readonly LimitedCountQueryBuilder $limitedCountQueryBuilder ) { } - public function count(FilteringCriterion $criterion): int + /** + * @phpstan-param positive-int $limit + */ + public function count(FilteringCriterion $criterion, ?int $limit = null): int { $query = $this->buildQuery( ['COUNT(DISTINCT content.id)'], $criterion ); + $query = $this->limitedCountQueryBuilder->wrap( + $query, + 'content.id', + $limit + ); + return (int)$query->executeQuery()->fetch(FetchMode::COLUMN); } diff --git a/src/lib/Persistence/Legacy/Filter/Gateway/Gateway.php b/src/lib/Persistence/Legacy/Filter/Gateway/Gateway.php index c76a604349..e3deea4310 100644 --- a/src/lib/Persistence/Legacy/Filter/Gateway/Gateway.php +++ b/src/lib/Persistence/Legacy/Filter/Gateway/Gateway.php @@ -18,9 +18,9 @@ interface Gateway { /** - * Return number of matched rows for the given Criteria (a total count w/o pagination constraints). + * Return number of matched rows for the given Criteria (a total count w/o pagination constraints, Unless a limit is passed). */ - public function count(FilteringCriterion $criterion): int; + public function count(FilteringCriterion $criterion, ?int $limit = null): int; /** * Return iterator for raw Repository data for the given Query result filtered by the given Criteria, diff --git a/src/lib/Persistence/Legacy/Filter/Gateway/Location/Doctrine/DoctrineGateway.php b/src/lib/Persistence/Legacy/Filter/Gateway/Location/Doctrine/DoctrineGateway.php index 49557211b8..0ca9a66ba5 100644 --- a/src/lib/Persistence/Legacy/Filter/Gateway/Location/Doctrine/DoctrineGateway.php +++ b/src/lib/Persistence/Legacy/Filter/Gateway/Location/Doctrine/DoctrineGateway.php @@ -17,6 +17,7 @@ use Ibexa\Core\Persistence\Legacy\Content\Gateway as ContentGateway; use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway; use Ibexa\Core\Persistence\Legacy\Filter\Gateway\Gateway; +use Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder; /** * @internal for internal use by Legacy Storage @@ -26,16 +27,26 @@ public function __construct( private Connection $connection, private CriterionVisitor $criterionVisitor, - private SortClauseVisitor $sortClauseVisitor + private SortClauseVisitor $sortClauseVisitor, + private LimitedCountQueryBuilder $limitedCountQueryBuilder ) { } - public function count(FilteringCriterion $criterion): int + /** + * @phpstan-param positive-int $limit + */ + public function count(FilteringCriterion $criterion, ?int $limit = null): int { $query = $this->buildQuery($criterion); $query->select('COUNT(DISTINCT location.node_id)'); + $query = $this->limitedCountQueryBuilder->wrap( + $query, + 'location.node_id', + $limit + ); + return (int)$query->executeQuery()->fetch(FetchMode::COLUMN); } diff --git a/src/lib/Persistence/Legacy/Filter/Handler/ContentFilteringHandler.php b/src/lib/Persistence/Legacy/Filter/Handler/ContentFilteringHandler.php index be975019d2..ef6118f295 100644 --- a/src/lib/Persistence/Legacy/Filter/Handler/ContentFilteringHandler.php +++ b/src/lib/Persistence/Legacy/Filter/Handler/ContentFilteringHandler.php @@ -73,8 +73,8 @@ function (array $row): ContentItem { return $list; } - public function count(Filter $filter): int + public function count(Filter $filter, ?int $limit = null): int { - return $this->gateway->count($filter->getCriterion()); + return $this->gateway->count($filter->getCriterion(), $limit); } } diff --git a/src/lib/Persistence/Legacy/Filter/Handler/LocationFilteringHandler.php b/src/lib/Persistence/Legacy/Filter/Handler/LocationFilteringHandler.php index da28228501..dd03786797 100644 --- a/src/lib/Persistence/Legacy/Filter/Handler/LocationFilteringHandler.php +++ b/src/lib/Persistence/Legacy/Filter/Handler/LocationFilteringHandler.php @@ -69,8 +69,8 @@ function (array $row): LocationWithContentInfo { return $list; } - public function count(Filter $filter): int + public function count(Filter $filter, ?int $limit = null): int { - return $this->gateway->count($filter->getCriterion()); + return $this->gateway->count($filter->getCriterion(), $limit); } } diff --git a/src/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilder.php new file mode 100644 index 0000000000..28e24f164a --- /dev/null +++ b/src/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilder.php @@ -0,0 +1,71 @@ +connection = $connection; + } + + /** + * Takes a QueryBuilder and wraps it in a count query with a limit if a limit is provided. + * This performs the following transformation to the passed query. + * SELECT DISTINCT COUNT(DISTINCT someField) FROM XXX WHERE YYY; + * To + * SELECT COUNT(*) FROM (SELECT DISTINCT someField FROM XXX WHERE YYY LIMIT N) AS csub;. + * + * @phpstan-param positive-int $limit + * + * @throws \Ibexa\Core\Base\Exceptions\InvalidArgumentException + * @throws \Doctrine\DBAL\Exception + */ + public function wrap( + QueryBuilder $queryBuilder, + string $countableField, + ?int $limit = null + ): QueryBuilder { + if ($limit === null) { + return $queryBuilder; + } + + if ($limit <= 0) { + throw new InvalidArgumentException('$limit', 'Limit must be greater than 0'); + } + + $querySql = $queryBuilder + ->select($countableField) + ->setMaxResults($limit) + ->getSQL(); + + $countQuery = $this->connection->createQueryBuilder(); + + return $countQuery + ->select( + 'COUNT(*)' + ) + ->from('(' . $querySql . ')', 'csub') + ->setParameters($queryBuilder->getParameters(), $queryBuilder->getParameterTypes()); + } +} diff --git a/src/lib/Repository/ContentService.php b/src/lib/Repository/ContentService.php index 6c6065f7f3..37efab7f59 100644 --- a/src/lib/Repository/ContentService.php +++ b/src/lib/Repository/ContentService.php @@ -2693,7 +2693,7 @@ public function find(Filter $filter, ?array $languages = null): ContentList return new ContentList($contentItemsIterator->getTotalCount(), $contentItems); } - public function count(Filter $filter, ?array $languages = null): int + public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int { $filter = clone $filter; if (!empty($languages)) { @@ -2713,6 +2713,6 @@ public function count(Filter $filter, ?array $languages = null): int $filter->andWithCriterion($permissionCriterion); } - return $this->contentFilteringHandler->count($filter); + return $this->contentFilteringHandler->count($filter, $limit); } } diff --git a/src/lib/Repository/LocationService.php b/src/lib/Repository/LocationService.php index 6dcc99fa2a..24dbfad2ce 100644 --- a/src/lib/Repository/LocationService.php +++ b/src/lib/Repository/LocationService.php @@ -363,17 +363,21 @@ public function loadParentLocationsForDraftContent(VersionInfo $versionInfo, ?ar /** * Returns the number of children which are readable by the current user of a Location object. */ - public function getLocationChildCount(APILocation $location): int + public function getLocationChildCount(APILocation $location, ?int $limit = null): int { $filter = $this->buildLocationChildrenFilter($location); - return $this->count($filter); + return $this->count($filter, null, $limit); } - public function getSubtreeSize(APILocation $location): int + /** + * @param int|null $limit + */ + public function getSubtreeSize(APILocation $location, ?int $limit = null): int { return $this->persistenceHandler->locationHandler()->getSubtreeSize( - $location->getPathString() + $location->getPathString(), + $limit ); } @@ -931,7 +935,7 @@ public function find(Filter $filter, ?array $languages = null): LocationList ); } - public function count(Filter $filter, ?array $languages = null): int + public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int { $filter = clone $filter; if (!empty($languages)) { @@ -951,7 +955,7 @@ public function count(Filter $filter, ?array $languages = null): int $filter->andWithCriterion($permissionCriterion); } - return $this->locationFilteringHandler->count($filter); + return $this->locationFilteringHandler->count($filter, $limit); } /** diff --git a/src/lib/Repository/SiteAccessAware/ContentService.php b/src/lib/Repository/SiteAccessAware/ContentService.php index 1155d2fe9e..67f6d3c6f3 100644 --- a/src/lib/Repository/SiteAccessAware/ContentService.php +++ b/src/lib/Repository/SiteAccessAware/ContentService.php @@ -293,11 +293,12 @@ public function find(Filter $filter, ?array $languages = null): ContentList ); } - public function count(Filter $filter, ?array $languages = null): int + public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int { return $this->service->count( $filter, - $this->languageResolver->getPrioritizedLanguages($languages) + $this->languageResolver->getPrioritizedLanguages($languages), + $limit ); } } diff --git a/src/lib/Repository/SiteAccessAware/LocationService.php b/src/lib/Repository/SiteAccessAware/LocationService.php index 3dc1d8ddae..d7e3e5fad0 100644 --- a/src/lib/Repository/SiteAccessAware/LocationService.php +++ b/src/lib/Repository/SiteAccessAware/LocationService.php @@ -104,14 +104,14 @@ public function loadParentLocationsForDraftContent(VersionInfo $versionInfo, ?ar ); } - public function getLocationChildCount(Location $location): int + public function getLocationChildCount(Location $location, ?int $limit = null): int { - return $this->service->getLocationChildCount($location); + return $this->service->getLocationChildCount($location, $limit); } - public function getSubtreeSize(Location $location): int + public function getSubtreeSize(Location $location, ?int $limit = null): int { - return $this->service->getSubtreeSize($location); + return $this->service->getSubtreeSize($location, $limit); } public function createLocation(ContentInfo $contentInfo, LocationCreateStruct $locationCreateStruct): Location @@ -192,11 +192,15 @@ public function find(Filter $filter, ?array $languages = null): LocationList ); } - public function count(Filter $filter, ?array $languages = null): int + /** + * @param int|null $limit + */ + public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int { return $this->service->count( $filter, - $this->languageResolver->getPrioritizedLanguages($languages) + $this->languageResolver->getPrioritizedLanguages($languages), + $limit ); } } diff --git a/src/lib/Resources/settings/repository/inner.yml b/src/lib/Resources/settings/repository/inner.yml index 2c10e30ac5..3d79114c80 100644 --- a/src/lib/Resources/settings/repository/inner.yml +++ b/src/lib/Resources/settings/repository/inner.yml @@ -105,6 +105,8 @@ services: class: Ibexa\Core\Repository\LocationService factory: ['@Ibexa\Core\Repository\Repository', getLocationService] lazy: true + tags: + - { name: 'proxy', interface: 'Ibexa\Contracts\Core\Future\Repository\FutureLocationService' } Ibexa\Core\Repository\LanguageService: class: Ibexa\Core\Repository\LanguageService diff --git a/src/lib/Resources/settings/storage_engines/legacy/filter.yaml b/src/lib/Resources/settings/storage_engines/legacy/filter.yaml index 4ac9426423..54022002cb 100644 --- a/src/lib/Resources/settings/storage_engines/legacy/filter.yaml +++ b/src/lib/Resources/settings/storage_engines/legacy/filter.yaml @@ -56,3 +56,7 @@ services: arguments: $gateway: '@Ibexa\Core\Persistence\Legacy\Filter\Gateway\Location\Doctrine\DoctrineGateway' $locationMapper: '@Ibexa\Core\Persistence\Legacy\Content\Location\Mapper' + + Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder: + arguments: + $connection: '@ibexa.persistence.connection' \ No newline at end of file diff --git a/src/lib/Resources/settings/storage_engines/legacy/location.yml b/src/lib/Resources/settings/storage_engines/legacy/location.yml index 3b0ba0cde5..ee3a7a813a 100644 --- a/src/lib/Resources/settings/storage_engines/legacy/location.yml +++ b/src/lib/Resources/settings/storage_engines/legacy/location.yml @@ -6,6 +6,7 @@ services: - '@Ibexa\Core\Persistence\Legacy\Content\Language\MaskGenerator' - '@ibexa.core.trash.search.legacy.gateway.criteria_converter' - '@ibexa.core.trash.search.legacy.gateway.sort_clause_converter' + - '@Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder' Ibexa\Core\Persistence\Legacy\Content\Location\Gateway\ExceptionConversion: class: Ibexa\Core\Persistence\Legacy\Content\Location\Gateway\ExceptionConversion diff --git a/tests/integration/Core/Repository/LocationServiceTest.php b/tests/integration/Core/Repository/LocationServiceTest.php index 172b4b05df..f081a7a39e 100644 --- a/tests/integration/Core/Repository/LocationServiceTest.php +++ b/tests/integration/Core/Repository/LocationServiceTest.php @@ -1143,6 +1143,26 @@ public function testGetLocationChildCount() ); } + /** + * Test for the getLocationChildCount() method with a limitation on the number of children. + * + * @covers \Ibexa\Contracts\Core\Repository\LocationService::getLocationChildCount() + * @depends testLoadLocation + */ + public function testGetLocationChildCountWithLimitation(): void + { + // $locationId is the ID of an existing location + $locationService = $this->getRepository()->getLocationService(); + $location = $locationService->loadLocation($this->generateId('location', 5)); + $this->assertSame( + 2, + $locationService->getLocationChildCount( + $location, + 2 + ) + ); + } + /** * Test for the loadLocationChildren() method. * @@ -3604,6 +3624,9 @@ public function testGetSubtreeSize(): Location $folder = $this->createFolder(['eng-GB' => 'Parent Folder'], 2); $location = $folder->getVersionInfo()->getContentInfo()->getMainLocation(); + self::assertNotNull($location); + + // phpstan-ignore-next-line self::assertSame(1, $locationService->getSubtreeSize($location)); $this->createFolder(['eng-GB' => 'Child 1'], $location->id); @@ -3614,6 +3637,47 @@ public function testGetSubtreeSize(): Location return $location; } + public function testGetSubtreeSizeWithLimit(): Location + { + $repository = $this->getRepository(); + $locationService = $repository->getLocationService(); + + $folder = $this->createFolder(['eng-GB' => 'Parent Folder'], 2); + $location = $folder->getVersionInfo()->getContentInfo()->getMainLocation(); + self::assertNotNull($location); + + for ($i = 1; $i <= 10; ++$i) { + $this->createFolder(['eng-GB' => 'Child ' . $i], $location->id); + } + + self::assertSame(3, $locationService->getSubtreeSize($location, 3)); + + return $location; + } + + public function testGetSubtreeSizeWithInvalidLimitThrowsExpectedError(): Location + { + $repository = $this->getRepository(); + $locationService = $repository->getLocationService(); + + $folder = $this->createFolder(['eng-GB' => 'Parent Folder'], 2); + $location = $folder->getVersionInfo()->getContentInfo()->getMainLocation(); + self::assertNotNull($location); + + self::assertSame(1, $locationService->getSubtreeSize($location)); + + for ($i = 1; $i <= 10; ++$i) { + $this->createFolder(['eng-GB' => 'Child ' . $i], $location->id); + } + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessageMatches('/Limit must be greater than 0/'); + + self::assertSame(3, $locationService->getSubtreeSize($location, -42)); + + return $location; + } + /** * Loads properties from all locations in the $location's subtree. * diff --git a/tests/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabaseTest.php b/tests/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabaseTest.php index e05e15bbcb..a45774ba49 100644 --- a/tests/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabaseTest.php +++ b/tests/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabaseTest.php @@ -30,7 +30,8 @@ protected function getLocationGateway() $this->getDatabaseConnection(), $this->getLanguageMaskGenerator(), $this->getTrashCriteriaConverterDependency(), - $this->getTrashSortClauseConverterDependency() + $this->getTrashSortClauseConverterDependency(), + $this->getLimitedCountQueryBuilderDependency() ); } diff --git a/tests/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabaseTrashTest.php b/tests/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabaseTrashTest.php index 0aa8113c3f..e73158b309 100644 --- a/tests/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabaseTrashTest.php +++ b/tests/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabaseTrashTest.php @@ -26,7 +26,8 @@ protected function getLocationGateway() $this->getDatabaseConnection(), $this->getLanguageMaskGenerator(), $this->getTrashCriteriaConverterDependency(), - $this->getTrashSortClauseConverterDependency() + $this->getTrashSortClauseConverterDependency(), + $this->getLimitedCountQueryBuilderDependency(), ); } diff --git a/tests/lib/Persistence/Legacy/Content/UrlAlias/UrlAliasHandlerTest.php b/tests/lib/Persistence/Legacy/Content/UrlAlias/UrlAliasHandlerTest.php index 2be4fcd1d2..216cf15741 100644 --- a/tests/lib/Persistence/Legacy/Content/UrlAlias/UrlAliasHandlerTest.php +++ b/tests/lib/Persistence/Legacy/Content/UrlAlias/UrlAliasHandlerTest.php @@ -5447,7 +5447,8 @@ protected function getLocationGateway() $this->getDatabaseConnection(), $this->getLanguageMaskGenerator(), $this->getTrashCriteriaConverterDependency(), - $this->getTrashSortClauseConverterDependency() + $this->getTrashSortClauseConverterDependency(), + $this->getLimitedCountQueryBuilderDependency() ); } diff --git a/tests/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilderTest.php b/tests/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilderTest.php new file mode 100644 index 0000000000..0ae64145db --- /dev/null +++ b/tests/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilderTest.php @@ -0,0 +1,73 @@ +limitedCountQueryBuilder = new LimitedCountQueryBuilder($this->getDatabaseConnection()); + } + + /** + * @covers \Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder::wrap + */ + public function testWrapThrowsExceptionOnZeroLimit(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessageMatches('/Limit must be greater than 0/'); + + $qb = $this->getDatabaseConnection()->createQueryBuilder(); + + $this->limitedCountQueryBuilder->wrap($qb, 'someField', 0); + } + + /** + * @covers \Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder::wrap + */ + public function testWrapDoesNotChangeQueryBuilderIfLimitIsNull(): void + { + $qb = $this->getDatabaseConnection()->createQueryBuilder(); + $qb->select('DISTINCT someField') + ->from('someTable') + ->where('someCondition = :condition') + ->setParameter('condition', 'value'); + + $wrappedQueryBuilder = $this->limitedCountQueryBuilder->wrap($qb, 'someField', null); + + // The original query should remain unchanged + $this->assertEquals($qb->getSQL(), $wrappedQueryBuilder->getSQL()); + } + + /** + * @covers \Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder::wrap + */ + public function testWrapWrapsQueryBuilderCorrectly(): void + { + $qb = $this->getDatabaseConnection()->createQueryBuilder(); + $qb->select('DISTINCT someField') + ->from('someTable') + ->where('someCondition = :condition') + ->setParameter('condition', 'value'); + + $wrappedQueryBuilder = $this->limitedCountQueryBuilder->wrap($qb, 'someField', 10); + + $expectedSql = 'SELECT COUNT(*) FROM (SELECT someField FROM someTable WHERE someCondition = :condition LIMIT 10) csub'; + $this->assertEquals($expectedSql, $wrappedQueryBuilder->getSQL()); + } +} diff --git a/tests/lib/Persistence/Legacy/TestCase.php b/tests/lib/Persistence/Legacy/TestCase.php index f3e006b230..3e942177d4 100644 --- a/tests/lib/Persistence/Legacy/TestCase.php +++ b/tests/lib/Persistence/Legacy/TestCase.php @@ -17,6 +17,7 @@ use Ibexa\Contracts\Core\Test\Persistence\Fixture\FixtureImporter; use Ibexa\Contracts\Core\Test\Persistence\Fixture\YamlFixture; use Ibexa\Contracts\Core\Test\Repository\SetupFactory\Legacy; +use Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder; use Ibexa\Core\Persistence\Legacy\SharedGateway; use Ibexa\Core\Search\Legacy\Content; use Ibexa\Core\Search\Legacy\Content\Common\Gateway\CriteriaConverter; @@ -347,4 +348,9 @@ protected function getTrashSortClauseConverterDependency(): SortClauseConverter ] ); } + + protected function getLimitedCountQueryBuilderDependency(): LimitedCountQueryBuilder + { + return new LimitedCountQueryBuilder($this->getDatabaseConnection()); + } } diff --git a/tests/lib/Repository/Decorator/LocationServiceDecoratorTest.php b/tests/lib/Repository/Decorator/LocationServiceDecoratorTest.php index 906c9575ac..cd175b1402 100644 --- a/tests/lib/Repository/Decorator/LocationServiceDecoratorTest.php +++ b/tests/lib/Repository/Decorator/LocationServiceDecoratorTest.php @@ -151,7 +151,7 @@ public function testGetLocationChildCountDecorator() $serviceMock = $this->createServiceMock(); $decoratedService = $this->createDecorator($serviceMock); - $parameters = [$this->createMock(Location::class)]; + $parameters = [$this->createMock(Location::class), 8]; $serviceMock->expects(self::once())->method('getLocationChildCount')->with(...$parameters); From f3b6f80afa07b658ca0253e9edf0224ed57d4480 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Sat, 22 Nov 2025 14:25:08 +0000 Subject: [PATCH 2/4] IBX-10186 Extract interface and fix docs --- .../Filter/Query/CountQueryBuilder.php | 16 ++++++++++++++++ .../Location/Gateway/DoctrineDatabase.php | 6 +++--- .../Content/Doctrine/DoctrineGateway.php | 10 +++++----- .../Location/Doctrine/DoctrineGateway.php | 6 +++--- .../Filter/Query/LimitedCountQueryBuilder.php | 16 +++++----------- .../storage_engines/legacy/filter.yaml | 5 ++++- .../Core/Repository/LocationServiceTest.php | 2 -- .../Query/LimitedCountQueryBuilderTest.php | 19 +++++++------------ 8 files changed, 43 insertions(+), 37 deletions(-) create mode 100644 src/contracts/Persistence/Filter/Query/CountQueryBuilder.php diff --git a/src/contracts/Persistence/Filter/Query/CountQueryBuilder.php b/src/contracts/Persistence/Filter/Query/CountQueryBuilder.php new file mode 100644 index 0000000000..d7997ed19c --- /dev/null +++ b/src/contracts/Persistence/Filter/Query/CountQueryBuilder.php @@ -0,0 +1,16 @@ +limitedCountQueryBuilder->wrap( + $query = $this->countQueryBuilder->wrap( $query, 't.node_id', $limit 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 35bafb4b37..a80fa94330 100644 --- a/src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php +++ b/src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php @@ -8,21 +8,21 @@ namespace Ibexa\Core\Persistence\Legacy\Filter\Gateway\Content\Doctrine; -use function array_filter; use Doctrine\DBAL\Connection; use Doctrine\DBAL\FetchMode; 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\Query\CountQueryBuilder; use Ibexa\Contracts\Core\Persistence\Filter\SortClauseVisitor; use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringCriterion; use Ibexa\Core\Persistence\Legacy\Content\Gateway as ContentGateway; use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway; use Ibexa\Core\Persistence\Legacy\Filter\Gateway\Gateway; -use Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder; +use Traversable; +use function array_filter; use function iterator_to_array; use function sprintf; -use Traversable; /** * @internal for internal use by Legacy Storage @@ -61,7 +61,7 @@ public function __construct( private readonly Connection $connection, private readonly CriterionVisitor $criterionVisitor, private readonly SortClauseVisitor $sortClauseVisitor, - private readonly LimitedCountQueryBuilder $limitedCountQueryBuilder + private readonly CountQueryBuilder $countQueryBuilder ) { } @@ -75,7 +75,7 @@ public function count(FilteringCriterion $criterion, ?int $limit = null): int $criterion ); - $query = $this->limitedCountQueryBuilder->wrap( + $query = $this->countQueryBuilder->wrap( $query, 'content.id', $limit diff --git a/src/lib/Persistence/Legacy/Filter/Gateway/Location/Doctrine/DoctrineGateway.php b/src/lib/Persistence/Legacy/Filter/Gateway/Location/Doctrine/DoctrineGateway.php index 0ca9a66ba5..84ad3d38dd 100644 --- a/src/lib/Persistence/Legacy/Filter/Gateway/Location/Doctrine/DoctrineGateway.php +++ b/src/lib/Persistence/Legacy/Filter/Gateway/Location/Doctrine/DoctrineGateway.php @@ -12,12 +12,12 @@ use Doctrine\DBAL\FetchMode; use Ibexa\Contracts\Core\Persistence\Filter\CriterionVisitor; use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder; +use Ibexa\Contracts\Core\Persistence\Filter\Query\CountQueryBuilder; use Ibexa\Contracts\Core\Persistence\Filter\SortClauseVisitor; use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringCriterion; use Ibexa\Core\Persistence\Legacy\Content\Gateway as ContentGateway; use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway; use Ibexa\Core\Persistence\Legacy\Filter\Gateway\Gateway; -use Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder; /** * @internal for internal use by Legacy Storage @@ -28,7 +28,7 @@ public function __construct( private Connection $connection, private CriterionVisitor $criterionVisitor, private SortClauseVisitor $sortClauseVisitor, - private LimitedCountQueryBuilder $limitedCountQueryBuilder + private CountQueryBuilder $countQueryBuilder ) { } @@ -41,7 +41,7 @@ public function count(FilteringCriterion $criterion, ?int $limit = null): int $query->select('COUNT(DISTINCT location.node_id)'); - $query = $this->limitedCountQueryBuilder->wrap( + $query = $this->countQueryBuilder->wrap( $query, 'location.node_id', $limit diff --git a/src/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilder.php index 28e24f164a..9b337ab06e 100644 --- a/src/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilder.php @@ -10,23 +10,18 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Query\QueryBuilder; +use Ibexa\Contracts\Core\Persistence\Filter\Query\CountQueryBuilder; use Ibexa\Core\Base\Exceptions\InvalidArgumentException; /** - * Limited Count trait. Used to allow for proper limiting of count queries + * Limited Count query builder. Used to allow for proper limiting of count queries * when using Doctrine DBAL QueryBuilder. */ -final class LimitedCountQueryBuilder +final readonly class LimitedCountQueryBuilder implements CountQueryBuilder { - /** - * @var \Doctrine\DBAL\Connection - */ - private $connection; - public function __construct( - Connection $connection + private readonly Connection $connection ) { - $this->connection = $connection; } /** @@ -39,7 +34,6 @@ public function __construct( * @phpstan-param positive-int $limit * * @throws \Ibexa\Core\Base\Exceptions\InvalidArgumentException - * @throws \Doctrine\DBAL\Exception */ public function wrap( QueryBuilder $queryBuilder, @@ -63,7 +57,7 @@ public function wrap( return $countQuery ->select( - 'COUNT(*)' + 'COUNT(1)' ) ->from('(' . $querySql . ')', 'csub') ->setParameters($queryBuilder->getParameters(), $queryBuilder->getParameterTypes()); diff --git a/src/lib/Resources/settings/storage_engines/legacy/filter.yaml b/src/lib/Resources/settings/storage_engines/legacy/filter.yaml index 54022002cb..5b23edfb25 100644 --- a/src/lib/Resources/settings/storage_engines/legacy/filter.yaml +++ b/src/lib/Resources/settings/storage_engines/legacy/filter.yaml @@ -22,6 +22,9 @@ services: Ibexa\Contracts\Core\Persistence\Filter\SortClauseVisitor: alias: Ibexa\Core\Persistence\Legacy\Filter\SortClauseVisitor + + Ibexa\Contracts\Core\Persistence\Filter\Query\CountQueryBuilder: + alias: Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder # implementations: Ibexa\Core\Persistence\Legacy\Filter\Gateway\Content\Mapper\DoctrineGatewayDataMapper: @@ -59,4 +62,4 @@ services: Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder: arguments: - $connection: '@ibexa.persistence.connection' \ No newline at end of file + $connection: '@ibexa.persistence.connection' diff --git a/tests/integration/Core/Repository/LocationServiceTest.php b/tests/integration/Core/Repository/LocationServiceTest.php index f081a7a39e..0b11f34f91 100644 --- a/tests/integration/Core/Repository/LocationServiceTest.php +++ b/tests/integration/Core/Repository/LocationServiceTest.php @@ -1127,8 +1127,6 @@ public function testLoadParentLocationsForDraftContentThrowsBadStateException(Co * Test for the getLocationChildCount() method. * * @covers \Ibexa\Contracts\Core\Repository\LocationService::getLocationChildCount() - * - * @depends testLoadLocation */ public function testGetLocationChildCount() { diff --git a/tests/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilderTest.php b/tests/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilderTest.php index 0ae64145db..76b3ca8c63 100644 --- a/tests/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilderTest.php +++ b/tests/lib/Persistence/Legacy/Filter/Query/LimitedCountQueryBuilderTest.php @@ -15,7 +15,7 @@ /** * @covers \Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder */ -class LimitedCountQueryBuilderTest extends TestCase +final class LimitedCountQueryBuilderTest extends TestCase { private LimitedCountQueryBuilder $limitedCountQueryBuilder; @@ -24,16 +24,13 @@ protected function setUp(): void $this->limitedCountQueryBuilder = new LimitedCountQueryBuilder($this->getDatabaseConnection()); } - /** - * @covers \Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder::wrap - */ public function testWrapThrowsExceptionOnZeroLimit(): void { + $qb = $this->getDatabaseConnection()->createQueryBuilder(); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessageMatches('/Limit must be greater than 0/'); - $qb = $this->getDatabaseConnection()->createQueryBuilder(); - $this->limitedCountQueryBuilder->wrap($qb, 'someField', 0); } @@ -51,12 +48,9 @@ public function testWrapDoesNotChangeQueryBuilderIfLimitIsNull(): void $wrappedQueryBuilder = $this->limitedCountQueryBuilder->wrap($qb, 'someField', null); // The original query should remain unchanged - $this->assertEquals($qb->getSQL(), $wrappedQueryBuilder->getSQL()); + self::assertEquals($qb->getSQL(), $wrappedQueryBuilder->getSQL()); } - /** - * @covers \Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder::wrap - */ public function testWrapWrapsQueryBuilderCorrectly(): void { $qb = $this->getDatabaseConnection()->createQueryBuilder(); @@ -67,7 +61,8 @@ public function testWrapWrapsQueryBuilderCorrectly(): void $wrappedQueryBuilder = $this->limitedCountQueryBuilder->wrap($qb, 'someField', 10); - $expectedSql = 'SELECT COUNT(*) FROM (SELECT someField FROM someTable WHERE someCondition = :condition LIMIT 10) csub'; - $this->assertEquals($expectedSql, $wrappedQueryBuilder->getSQL()); + $expectedSql = 'SELECT COUNT(1) FROM (SELECT someField FROM someTable WHERE someCondition = :condition LIMIT 10) csub'; + self::assertEquals($expectedSql, $wrappedQueryBuilder->getSQL()); + self::assertEquals($qb->getParameters(), $wrappedQueryBuilder->getParameters()); } } From ebd54d5b4b75c802b4c6e5b17dd3d2e6a57a73cb Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Sat, 22 Nov 2025 19:28:38 +0000 Subject: [PATCH 3/4] IBX-10186 Fix contract against di --- src/lib/Resources/settings/storage_engines/legacy/location.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Resources/settings/storage_engines/legacy/location.yml b/src/lib/Resources/settings/storage_engines/legacy/location.yml index ee3a7a813a..d6e86033d5 100644 --- a/src/lib/Resources/settings/storage_engines/legacy/location.yml +++ b/src/lib/Resources/settings/storage_engines/legacy/location.yml @@ -6,7 +6,7 @@ services: - '@Ibexa\Core\Persistence\Legacy\Content\Language\MaskGenerator' - '@ibexa.core.trash.search.legacy.gateway.criteria_converter' - '@ibexa.core.trash.search.legacy.gateway.sort_clause_converter' - - '@Ibexa\Core\Persistence\Legacy\Filter\Query\LimitedCountQueryBuilder' + - '@Ibexa\Contracts\Core\Persistence\Filter\Query\CountQueryBuilder' Ibexa\Core\Persistence\Legacy\Content\Location\Gateway\ExceptionConversion: class: Ibexa\Core\Persistence\Legacy\Content\Location\Gateway\ExceptionConversion From a64b39ac5e39b7b9cdcaccd4dad1a5aaa59be01e Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Sat, 22 Nov 2025 19:33:29 +0000 Subject: [PATCH 4/4] IBX-10186 Run CS --- src/contracts/Persistence/Filter/Query/CountQueryBuilder.php | 4 ++-- src/contracts/Repository/ContentService.php | 2 +- .../Legacy/Content/Location/Gateway/DoctrineDatabase.php | 1 - .../Filter/Gateway/Content/Doctrine/DoctrineGateway.php | 4 ++-- tests/integration/Core/Repository/LocationServiceTest.php | 1 + 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/contracts/Persistence/Filter/Query/CountQueryBuilder.php b/src/contracts/Persistence/Filter/Query/CountQueryBuilder.php index d7997ed19c..7c8fbf4df4 100644 --- a/src/contracts/Persistence/Filter/Query/CountQueryBuilder.php +++ b/src/contracts/Persistence/Filter/Query/CountQueryBuilder.php @@ -7,10 +7,10 @@ declare(strict_types=1); namespace Ibexa\Contracts\Core\Persistence\Filter\Query; -use Doctrine\DBAL\Query\QueryBuilder; +use Doctrine\DBAL\Query\QueryBuilder; interface CountQueryBuilder { public function wrap(QueryBuilder $queryBuilder, string $countableField, ?int $limit = null): QueryBuilder; -} \ No newline at end of file +} diff --git a/src/contracts/Repository/ContentService.php b/src/contracts/Repository/ContentService.php index 8647a3169b..c029a9b695 100644 --- a/src/contracts/Repository/ContentService.php +++ b/src/contracts/Repository/ContentService.php @@ -528,11 +528,11 @@ public function find(Filter $filter, ?array $languages = null): ContentList; * @phpstan-param TFilteringLanguageFilter|null $languages $languages A list of language codes to be added as additional constraints. * If skipped, by default, unless SiteAccessAware layer has been disabled, languages set * for a SiteAccess in a current context will be used. + * * @param int|null $limit If set, the count will be limited to first $limit items found. * In some cases it can significantly speed up a count operation for more complex filters. * * @phpstan-return int<0, max> - * */ public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int; } diff --git a/src/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php index f6c28a500b..8891c71043 100644 --- a/src/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php +++ b/src/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php @@ -41,7 +41,6 @@ final class DoctrineDatabase extends Gateway { public const string NODE_ASSIGNMENT_TABLE = 'ibexa_node_assignment'; - public function __construct( private readonly Connection $connection, private readonly MaskGenerator $languageMaskGenerator, 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 a80fa94330..69f7ecc630 100644 --- a/src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php +++ b/src/lib/Persistence/Legacy/Filter/Gateway/Content/Doctrine/DoctrineGateway.php @@ -8,6 +8,7 @@ namespace Ibexa\Core\Persistence\Legacy\Filter\Gateway\Content\Doctrine; +use function array_filter; use Doctrine\DBAL\Connection; use Doctrine\DBAL\FetchMode; use Doctrine\DBAL\Query\QueryBuilder; @@ -19,10 +20,9 @@ use Ibexa\Core\Persistence\Legacy\Content\Gateway as ContentGateway; use Ibexa\Core\Persistence\Legacy\Content\Location\Gateway as LocationGateway; use Ibexa\Core\Persistence\Legacy\Filter\Gateway\Gateway; -use Traversable; -use function array_filter; use function iterator_to_array; use function sprintf; +use Traversable; /** * @internal for internal use by Legacy Storage diff --git a/tests/integration/Core/Repository/LocationServiceTest.php b/tests/integration/Core/Repository/LocationServiceTest.php index 0b11f34f91..76b38051f4 100644 --- a/tests/integration/Core/Repository/LocationServiceTest.php +++ b/tests/integration/Core/Repository/LocationServiceTest.php @@ -1145,6 +1145,7 @@ public function testGetLocationChildCount() * Test for the getLocationChildCount() method with a limitation on the number of children. * * @covers \Ibexa\Contracts\Core\Repository\LocationService::getLocationChildCount() + * * @depends testLoadLocation */ public function testGetLocationChildCountWithLimitation(): void