From 154f2ef74fb602495ade79f7a7099f9c5fe21c13 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Wed, 18 Jun 2025 10:00:49 +0100 Subject: [PATCH 1/2] IBX-10174 Add limit option to getSubtreeSize --- eZ/Publish/API/Repository/LocationService.php | 5 ++- .../Repository/Tests/LocationServiceTest.php | 37 +++++++++++++++++++ .../Persistence/Cache/LocationHandler.php | 4 +- .../Legacy/Content/Location/Gateway.php | 2 +- .../Location/Gateway/DoctrineDatabase.php | 21 +++++++++-- .../Location/Gateway/ExceptionConversion.php | 4 +- .../Legacy/Content/Location/Handler.php | 4 +- .../Core/Repository/LocationService.php | 5 ++- .../SiteAccessAware/LocationService.php | 4 +- .../Persistence/Content/Location/Handler.php | 2 +- .../Decorator/LocationServiceDecorator.php | 4 +- 11 files changed, 73 insertions(+), 19 deletions(-) diff --git a/eZ/Publish/API/Repository/LocationService.php b/eZ/Publish/API/Repository/LocationService.php index e8af6dbc4b..24295d9ea7 100644 --- a/eZ/Publish/API/Repository/LocationService.php +++ b/eZ/Publish/API/Repository/LocationService.php @@ -133,8 +133,11 @@ public function getLocationChildCount(Location $location): int; * Return the subtree size of a given location. * * Warning! This method is not permission aware by design. + * + * @param \eZ\Publish\API\Repository\Values\Content\Location $location + * @param int|null $limit Optional limit to the number of locations to count. (Can be used to limit the number of locations counted in large subtrees.) */ - 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. diff --git a/eZ/Publish/API/Repository/Tests/LocationServiceTest.php b/eZ/Publish/API/Repository/Tests/LocationServiceTest.php index 153f768ba7..b996f0be9b 100644 --- a/eZ/Publish/API/Repository/Tests/LocationServiceTest.php +++ b/eZ/Publish/API/Repository/Tests/LocationServiceTest.php @@ -3552,6 +3552,43 @@ 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::assertSame(1, $locationService->getSubtreeSize($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 testGetSubtreeSizeWithInvalidLimitHasNoEffect(): Location + { + $repository = $this->getRepository(); + $locationService = $repository->getLocationService(); + + $folder = $this->createFolder(['eng-GB' => 'Parent Folder'], 2); + $location = $folder->getVersionInfo()->getContentInfo()->getMainLocation(); + self::assertSame(1, $locationService->getSubtreeSize($location)); + + for ($i = 1; $i <= 10; $i++) { + $this->createFolder(['eng-GB' => 'Child ' . $i], $location->id); + } + + + self::assertSame(11, $locationService->getSubtreeSize($location, -2)); + + return $location; + } + /** * Loads properties from all locations in the $location's subtree. * diff --git a/eZ/Publish/Core/Persistence/Cache/LocationHandler.php b/eZ/Publish/Core/Persistence/Cache/LocationHandler.php index d34c03d3b0..d1f7686a4f 100644 --- a/eZ/Publish/Core/Persistence/Cache/LocationHandler.php +++ b/eZ/Publish/Core/Persistence/Cache/LocationHandler.php @@ -259,13 +259,13 @@ 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, ]); - return $this->persistenceHandler->locationHandler()->getSubtreeSize($path); + return $this->persistenceHandler->locationHandler()->getSubtreeSize($path, $limit); } /** diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway.php b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway.php index d49a1f0286..a3a0e70e42 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway.php @@ -111,7 +111,7 @@ abstract public function loadParentLocationsDataForDraftContent(int $contentId): */ abstract public function getSubtreeContent(int $sourceId, bool $onlyIds = false): 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/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php index 495947b5fa..6b33d3bab3 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php @@ -237,18 +237,31 @@ public function getSubtreeContent(int $sourceId, bool $onlyIds = false): array : $results; } - public function getSubtreeSize(string $path): int + public function getSubtreeSize(string $path, ?int $limit = null): int { - $query = $this->createNodeQueryBuilder([$this->dbPlatform->getCountExpression('node_id')]); + $useLimit = $limit !== null && $limit > 0; + + $query = $this->createNodeQueryBuilder([$useLimit ? 'node_id': $this->dbPlatform->getCountExpression('node_id')]); $query->andWhere( - $query->expr()->like( + $query->expr()->like( 't.path_string', $query->createPositionalParameter( $path . '%', - ) + ), ) ); + if ($useLimit) { + $query->setMaxResults($limit); + $outerQuery = $this + ->connection + ->createQueryBuilder() + ->select($this->dbPlatform->getCountExpression( '*')) + ->from('(' . $query->getSQL() . ')', 't') + ->setParameters($query->getParameters()); + return (int) $outerQuery->execute()->fetchOne(); + } + return (int) $query->execute()->fetchOne(); } diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/ExceptionConversion.php b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/ExceptionConversion.php index ed1acf56e2..57c90007c3 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/ExceptionConversion.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/ExceptionConversion.php @@ -106,10 +106,10 @@ public function getSubtreeContent(int $sourceId, bool $onlyIds = false): 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/eZ/Publish/Core/Persistence/Legacy/Content/Location/Handler.php b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Handler.php index b125238675..93f51964a1 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Handler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Handler.php @@ -346,9 +346,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/eZ/Publish/Core/Repository/LocationService.php b/eZ/Publish/Core/Repository/LocationService.php index 8fa272dd9c..9509400a70 100644 --- a/eZ/Publish/Core/Repository/LocationService.php +++ b/eZ/Publish/Core/Repository/LocationService.php @@ -379,10 +379,11 @@ public function getLocationChildCount(APILocation $location): int return $this->count($filter); } - public function getSubtreeSize(APILocation $location): int + public function getSubtreeSize(APILocation $location, ?int $limit = null): int { return $this->persistenceHandler->locationHandler()->getSubtreeSize( - $location->getPathString() + $location->getPathString(), + $limit ); } diff --git a/eZ/Publish/Core/Repository/SiteAccessAware/LocationService.php b/eZ/Publish/Core/Repository/SiteAccessAware/LocationService.php index 1a5700d2f1..132f9d2558 100644 --- a/eZ/Publish/Core/Repository/SiteAccessAware/LocationService.php +++ b/eZ/Publish/Core/Repository/SiteAccessAware/LocationService.php @@ -109,9 +109,9 @@ public function getLocationChildCount(Location $location): int return $this->service->getLocationChildCount($location); } - 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 diff --git a/eZ/Publish/SPI/Persistence/Content/Location/Handler.php b/eZ/Publish/SPI/Persistence/Content/Location/Handler.php index ff36c20df7..fc69b27adf 100644 --- a/eZ/Publish/SPI/Persistence/Content/Location/Handler.php +++ b/eZ/Publish/SPI/Persistence/Content/Location/Handler.php @@ -110,7 +110,7 @@ public function loadParentLocationsForDraftContent($contentId); */ public function copySubtree($sourceId, $destinationParentId); - public function getSubtreeSize(string $path): int; + public function getSubtreeSize(string $path, ?int $limit = null): int; /** * Moves location identified by $sourceId into new parent identified by $destinationParentId. diff --git a/eZ/Publish/SPI/Repository/Decorator/LocationServiceDecorator.php b/eZ/Publish/SPI/Repository/Decorator/LocationServiceDecorator.php index 31885e27b9..6debd1f067 100644 --- a/eZ/Publish/SPI/Repository/Decorator/LocationServiceDecorator.php +++ b/eZ/Publish/SPI/Repository/Decorator/LocationServiceDecorator.php @@ -87,9 +87,9 @@ public function getLocationChildCount(Location $location): int return $this->innerService->getLocationChildCount($location); } - 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( From 8eebd254582ec7c4f2167a15cba8436b2b77e4d4 Mon Sep 17 00:00:00 2001 From: Ryan Lee Date: Wed, 18 Jun 2025 10:05:16 +0100 Subject: [PATCH 2/2] IBX-10174 Run linter --- eZ/Publish/API/Repository/LocationService.php | 2 +- eZ/Publish/API/Repository/Tests/LocationServiceTest.php | 7 +++---- .../Legacy/Content/Location/Gateway/DoctrineDatabase.php | 9 +++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/eZ/Publish/API/Repository/LocationService.php b/eZ/Publish/API/Repository/LocationService.php index 24295d9ea7..cc1c858374 100644 --- a/eZ/Publish/API/Repository/LocationService.php +++ b/eZ/Publish/API/Repository/LocationService.php @@ -133,7 +133,7 @@ public function getLocationChildCount(Location $location): int; * Return the subtree size of a given location. * * Warning! This method is not permission aware by design. - * + * * @param \eZ\Publish\API\Repository\Values\Content\Location $location * @param int|null $limit Optional limit to the number of locations to count. (Can be used to limit the number of locations counted in large subtrees.) */ diff --git a/eZ/Publish/API/Repository/Tests/LocationServiceTest.php b/eZ/Publish/API/Repository/Tests/LocationServiceTest.php index b996f0be9b..a0d51747b3 100644 --- a/eZ/Publish/API/Repository/Tests/LocationServiceTest.php +++ b/eZ/Publish/API/Repository/Tests/LocationServiceTest.php @@ -3561,7 +3561,7 @@ public function testGetSubtreeSizeWithLimit(): Location $location = $folder->getVersionInfo()->getContentInfo()->getMainLocation(); self::assertSame(1, $locationService->getSubtreeSize($location)); - for ($i = 1; $i <= 10; $i++) { + for ($i = 1; $i <= 10; ++$i) { $this->createFolder(['eng-GB' => 'Child ' . $i], $location->id); } @@ -3570,7 +3570,7 @@ public function testGetSubtreeSizeWithLimit(): Location return $location; } - public function testGetSubtreeSizeWithInvalidLimitHasNoEffect(): Location + public function testGetSubtreeSizeWithInvalidLimitHasNoEffect(): Location { $repository = $this->getRepository(); $locationService = $repository->getLocationService(); @@ -3579,11 +3579,10 @@ public function testGetSubtreeSizeWithInvalidLimitHasNoEffect(): Location $location = $folder->getVersionInfo()->getContentInfo()->getMainLocation(); self::assertSame(1, $locationService->getSubtreeSize($location)); - for ($i = 1; $i <= 10; $i++) { + for ($i = 1; $i <= 10; ++$i) { $this->createFolder(['eng-GB' => 'Child ' . $i], $location->id); } - self::assertSame(11, $locationService->getSubtreeSize($location, -2)); return $location; diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php index 6b33d3bab3..179e4c71f7 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php @@ -241,9 +241,9 @@ public function getSubtreeSize(string $path, ?int $limit = null): int { $useLimit = $limit !== null && $limit > 0; - $query = $this->createNodeQueryBuilder([$useLimit ? 'node_id': $this->dbPlatform->getCountExpression('node_id')]); + $query = $this->createNodeQueryBuilder([$useLimit ? 'node_id' : $this->dbPlatform->getCountExpression('node_id')]); $query->andWhere( - $query->expr()->like( + $query->expr()->like( 't.path_string', $query->createPositionalParameter( $path . '%', @@ -256,9 +256,10 @@ public function getSubtreeSize(string $path, ?int $limit = null): int $outerQuery = $this ->connection ->createQueryBuilder() - ->select($this->dbPlatform->getCountExpression( '*')) - ->from('(' . $query->getSQL() . ')', 't') + ->select($this->dbPlatform->getCountExpression('*')) + ->from('(' . $query->getSQL() . ')', 't') ->setParameters($query->getParameters()); + return (int) $outerQuery->execute()->fetchOne(); }