From 65a0c650be846c8b5b0bb0143d6f73fe729397e5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adam=20W=C3=B3js?= <adam@wojs.pl>
Date: Tue, 14 Feb 2023 10:03:55 +0100
Subject: [PATCH] IBX-4477: Improved copy subtree limit check performance

For more details see https://issues.ibexa.co/browse/IBX-4477 and https://github.com/ezsystems/ezplatform-admin-ui/pull/2085

* Skipped computing copy subtree limit if user doesn't have sufficient permissions

* Skipped computing copy subtree limit for Content Root

* Skipped computing copy subtree limit if location doesn't point to container

* Used dedicated method to compute subtree size

* [Tests] Aligned validator unit test with the changes

---------

Co-authored-by: Andrew Longosz <alongosz@users.noreply.github.com>
---
 src/lib/Menu/ContentRightSidebarBuilder.php   | 21 ++++++++++++-------
 .../Location/IsWithinCopySubtreeLimit.php     | 16 +++++++-------
 ...nIsWithinCopySubtreeLimitValidatorTest.php | 15 +++++++++++++
 3 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/src/lib/Menu/ContentRightSidebarBuilder.php b/src/lib/Menu/ContentRightSidebarBuilder.php
index 2aa87f6fa4..ba9fd7808c 100644
--- a/src/lib/Menu/ContentRightSidebarBuilder.php
+++ b/src/lib/Menu/ContentRightSidebarBuilder.php
@@ -401,15 +401,22 @@ private function canCopy(bool $hasCreatePermission): bool
 
     private function canCopySubtree(Location $location, bool $hasCreatePermission): bool
     {
-        $copyLimit = $this->configResolver->getParameter(
-            'subtree_operations.copy_subtree.limit'
-        );
+        if (!$hasCreatePermission) {
+            return false;
+        }
 
-        $canCopySubtree = (new IsWithinCopySubtreeLimit(
-            $copyLimit,
+        $isWithinCopySubtreeLimit = new IsWithinCopySubtreeLimit(
+            $this->getCopySubtreeLimit(),
             $this->locationService
-        ))->and((new IsRoot())->not())->isSatisfiedBy($location);
+        );
 
-        return $canCopySubtree && $hasCreatePermission;
+        return ((new IsRoot())->not())->and($isWithinCopySubtreeLimit)->isSatisfiedBy($location);
+    }
+
+    private function getCopySubtreeLimit(): int
+    {
+        return $this->configResolver->getParameter(
+            'subtree_operations.copy_subtree.limit'
+        );
     }
 }
diff --git a/src/lib/Specification/Location/IsWithinCopySubtreeLimit.php b/src/lib/Specification/Location/IsWithinCopySubtreeLimit.php
index 3e5751dbe2..c70c432874 100644
--- a/src/lib/Specification/Location/IsWithinCopySubtreeLimit.php
+++ b/src/lib/Specification/Location/IsWithinCopySubtreeLimit.php
@@ -9,8 +9,7 @@
 namespace EzSystems\EzPlatformAdminUi\Specification\Location;
 
 use eZ\Publish\API\Repository\LocationService;
-use eZ\Publish\API\Repository\Values\Content\Query\Criterion;
-use eZ\Publish\API\Repository\Values\Filter\Filter;
+use eZ\Publish\API\Repository\Values\Content\Location;
 use EzSystems\EzPlatformAdminUi\Specification\AbstractSpecification;
 
 /**
@@ -34,10 +33,6 @@ public function __construct(
 
     /**
      * @param \eZ\Publish\API\Repository\Values\Content\Location $item
-     *
-     * @return bool
-     *
-     * @throws \eZ\Publish\API\Repository\Exceptions\BadStateException
      */
     public function isSatisfiedBy($item): bool
     {
@@ -45,12 +40,15 @@ public function isSatisfiedBy($item): bool
             return true;
         }
 
-        if ($this->copyLimit === 0) {
+        if ($this->copyLimit === 0 || !$this->isContainer($item)) {
             return false;
         }
 
-        $filter = new Filter(new Criterion\Subtree($item->pathString));
+        return $this->copyLimit >= $this->locationService->getSubtreeSize($item);
+    }
 
-        return $this->copyLimit >= $this->locationService->count($filter);
+    private function isContainer(Location $location): bool
+    {
+        return $location->getContentInfo()->getContentType()->isContainer();
     }
 }
diff --git a/src/lib/Tests/Validator/Constraint/LocationIsWithinCopySubtreeLimitValidatorTest.php b/src/lib/Tests/Validator/Constraint/LocationIsWithinCopySubtreeLimitValidatorTest.php
index a6ad870ec4..8a559898ec 100644
--- a/src/lib/Tests/Validator/Constraint/LocationIsWithinCopySubtreeLimitValidatorTest.php
+++ b/src/lib/Tests/Validator/Constraint/LocationIsWithinCopySubtreeLimitValidatorTest.php
@@ -9,10 +9,13 @@
 namespace EzSystems\EzPlatformAdminUi\Tests\Validator\Constraint;
 
 use eZ\Publish\API\Repository\LocationService;
+use eZ\Publish\API\Repository\Values\Content\ContentInfo;
 use eZ\Publish\API\Repository\Values\Content\Location;
+use eZ\Publish\API\Repository\Values\ContentType\ContentType;
 use eZ\Publish\Core\MVC\ConfigResolverInterface;
 use EzSystems\EzPlatformAdminUi\Validator\Constraints\LocationIsWithinCopySubtreeLimit;
 use EzSystems\EzPlatformAdminUi\Validator\Constraints\LocationIsWithinCopySubtreeLimitValidator;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Validator\Context\ExecutionContextInterface;
 use Symfony\Component\Validator\Violation\ConstraintViolationBuilderInterface;
@@ -62,6 +65,8 @@ public function testValid(): void
             ->expects($this->never())
             ->method('addViolation');
 
+        $this->mockLocationContentContentTypeIsContainer($this->location);
+
         $this->validator->validate($this->location, new LocationIsWithinCopySubtreeLimit());
     }
 
@@ -87,4 +92,14 @@ public function testInvalid(): void
 
         $this->validator->validate($this->location, new LocationIsWithinCopySubtreeLimit());
     }
+
+    private function mockLocationContentContentTypeIsContainer(MockObject $location): void
+    {
+        $contentType = $this->createMock(ContentType::class);
+        $contentType->method('isContainer')->willReturn(true);
+        $contentInfo = $this->createMock(ContentInfo::class);
+        $contentInfo->method('getContentType')->willReturn($contentType);
+
+        $location->method('getContentInfo')->willReturn($contentInfo);
+    }
 }