fix(Nodes): Fixed node offspring resolution cache with new `NodeOffsp…

…ringResolverInterface` service
roadiz · Jun 17, 2024 · cf62690 · cf62690
1 parent 11a80e4
commit cf62690
Show file tree

Hide file tree

Showing 11 changed files with 160 additions and 32 deletions.
diff --git a/config/packages/roadiz_rozier.yaml b/config/packages/roadiz_rozier.yaml
@@ -62,7 +62,8 @@ roadiz_rozier:
                     name: 'deleted.nodes'
                     route: nodesHomeDeletedPage
                     icon: uk-icon-rz-deleted-nodes
-                    roles: ~
+                    roles:
+                        - EMPTY_TRASH
                 search_nodes:
                     name: 'search.nodes'
                     route: searchNodePage

diff --git a/lib/RoadizCoreBundle/src/EntityHandler/NodeHandler.php b/lib/RoadizCoreBundle/src/EntityHandler/NodeHandler.php
@@ -529,7 +529,7 @@ public function cleanRootNodesPositions(bool $setPositions = true): float
      * Return all node offspring id.
      *
      * @return array
-     * @deprecated Use NodeRepository::findAllOffspringIdByNode() instead.
+     * @deprecated Use NodeOffspringResolverInterface::getAllOffspringIds($chroot).
      */
     public function getAllOffspringId(): array
     {

diff --git a/lib/RoadizCoreBundle/src/EventSubscriber/CachedNodeOffspringEventSubscriber.php b/lib/RoadizCoreBundle/src/EventSubscriber/CachedNodeOffspringEventSubscriber.php
@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace RZ\Roadiz\CoreBundle\EventSubscriber;
+
+use RZ\Roadiz\CoreBundle\Event\FilterNodeEvent;
+use RZ\Roadiz\CoreBundle\Event\Node\NodeCreatedEvent;
+use RZ\Roadiz\CoreBundle\Event\Node\NodeDuplicatedEvent;
+use RZ\Roadiz\CoreBundle\Event\Node\NodeUpdatedEvent;
+use RZ\Roadiz\CoreBundle\Node\CachedNodeOffspringResolverInterface;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+class CachedNodeOffspringEventSubscriber implements EventSubscriberInterface
+{
+    public function __construct(private readonly CachedNodeOffspringResolverInterface $cachedNodeOffspringResolver)
+    {
+    }
+
+    public static function getSubscribedEvents(): array
+    {
+        return [
+            NodeCreatedEvent::class => 'invalidateNodeOffspringCache',
+            NodeDuplicatedEvent::class => 'invalidateNodeOffspringCache',
+            NodeUpdatedEvent::class => 'invalidateNodeOffspringCache',
+        ];
+    }
+
+    public function invalidateNodeOffspringCache(FilterNodeEvent $event): void
+    {
+        $this->cachedNodeOffspringResolver->purgeOffspringCache($event->getNode());
+    }
+}
diff --git a/lib/RoadizCoreBundle/src/Message/Handler/ApplyRealmNodeInheritanceMessageHandler.php b/lib/RoadizCoreBundle/src/Message/Handler/ApplyRealmNodeInheritanceMessageHandler.php
@@ -5,20 +5,21 @@
 namespace RZ\Roadiz\CoreBundle\Message\Handler;
 
 use Doctrine\Persistence\ManagerRegistry;
-use RZ\Roadiz\Core\Handlers\HandlerFactoryInterface;
 use RZ\Roadiz\CoreBundle\Entity\Node;
 use RZ\Roadiz\CoreBundle\Entity\Realm;
 use RZ\Roadiz\CoreBundle\Entity\RealmNode;
-use RZ\Roadiz\CoreBundle\EntityHandler\NodeHandler;
 use RZ\Roadiz\CoreBundle\Message\ApplyRealmNodeInheritanceMessage;
 use RZ\Roadiz\CoreBundle\Model\RealmInterface;
+use RZ\Roadiz\CoreBundle\Node\NodeOffspringResolverInterface;
 use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
 use Symfony\Component\Messenger\Handler\MessageHandlerInterface;
 
 final class ApplyRealmNodeInheritanceMessageHandler implements MessageHandlerInterface
 {
-    public function __construct(private readonly ManagerRegistry $managerRegistry)
-    {
+    public function __construct(
+        private readonly ManagerRegistry $managerRegistry,
+        private readonly NodeOffspringResolverInterface $nodeOffspringResolver
+    ) {
     }
 
     public function __invoke(ApplyRealmNodeInheritanceMessage $message): void
@@ -49,7 +50,7 @@ public function __invoke(ApplyRealmNodeInheritanceMessage $message): void
         }
 
         $nodeRepository = $this->managerRegistry->getRepository(Node::class);
-        $childrenIds = $nodeRepository->findAllOffspringIdByNode($node);
+        $childrenIds = $this->nodeOffspringResolver->getAllOffspringIds($node);
 
         foreach ($childrenIds as $childId) {
             /** @var Node|null $child */

diff --git a/lib/RoadizCoreBundle/src/Message/Handler/CleanRealmNodeInheritanceMessageHandler.php b/lib/RoadizCoreBundle/src/Message/Handler/CleanRealmNodeInheritanceMessageHandler.php
@@ -5,19 +5,20 @@
 namespace RZ\Roadiz\CoreBundle\Message\Handler;
 
 use Doctrine\Persistence\ManagerRegistry;
-use RZ\Roadiz\Core\Handlers\HandlerFactoryInterface;
 use RZ\Roadiz\CoreBundle\Entity\Node;
 use RZ\Roadiz\CoreBundle\Entity\Realm;
 use RZ\Roadiz\CoreBundle\Entity\RealmNode;
-use RZ\Roadiz\CoreBundle\EntityHandler\NodeHandler;
 use RZ\Roadiz\CoreBundle\Message\CleanRealmNodeInheritanceMessage;
+use RZ\Roadiz\CoreBundle\Node\NodeOffspringResolverInterface;
 use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
 use Symfony\Component\Messenger\Handler\MessageHandlerInterface;
 
 final class CleanRealmNodeInheritanceMessageHandler implements MessageHandlerInterface
 {
-    public function __construct(private readonly ManagerRegistry $managerRegistry)
-    {
+    public function __construct(
+        private readonly ManagerRegistry $managerRegistry,
+        private readonly NodeOffspringResolverInterface $nodeOffspringResolver
+    ) {
     }
 
     public function __invoke(CleanRealmNodeInheritanceMessage $message): void
@@ -35,8 +36,7 @@ public function __invoke(CleanRealmNodeInheritanceMessage $message): void
             throw new UnrecoverableMessageHandlingException('Realm does not exist');
         }
 
-        $nodeRepository = $this->managerRegistry->getRepository(Node::class);
-        $childrenIds = $nodeRepository->findAllOffspringIdByNode($node);
+        $childrenIds = $this->nodeOffspringResolver->getAllOffspringIds($node);
 
         $realmNodes = $this->managerRegistry
             ->getRepository(RealmNode::class)

diff --git a/lib/RoadizCoreBundle/src/Node/CachedNodeOffspringResolver.php b/lib/RoadizCoreBundle/src/Node/CachedNodeOffspringResolver.php
@@ -0,0 +1,68 @@
+<?php
+
+declare(strict_types=1);
+
+namespace RZ\Roadiz\CoreBundle\Node;
+
+use Doctrine\Persistence\ManagerRegistry;
+use Psr\Cache\CacheException;
+use Psr\Cache\CacheItemPoolInterface;
+use Psr\Cache\InvalidArgumentException;
+use RZ\Roadiz\CoreBundle\Entity\Node;
+use Symfony\Contracts\Cache\ItemInterface;
+use Symfony\Contracts\Cache\TagAwareCacheInterface;
+
+final class CachedNodeOffspringResolver implements CachedNodeOffspringResolverInterface
+{
+    public function __construct(
+        private readonly CacheItemPoolInterface $cache,
+        private readonly ManagerRegistry $managerRegistry
+    ) {
+    }
+
+    /**
+     * @inheritDoc
+     * @throws InvalidArgumentException
+     * @throws CacheException
+     */
+    public function getAllOffspringIds(Node $ancestor): array
+    {
+        $cacheItem = $this->cache->getItem(self::CACHE_PREFIX . $ancestor->getId());
+        if (!$cacheItem->isHit()) {
+            $nodeRepository = $this->managerRegistry->getRepository(Node::class);
+            $offspringIds = $nodeRepository->findAllOffspringIdByNode($ancestor);
+            $cacheItem->set($offspringIds);
+            $cacheItem->expiresAfter(300);
+            if ($cacheItem instanceof ItemInterface && $this->cache instanceof TagAwareCacheInterface) {
+                $cacheItem->tag(array_map(function (int $nodeId) {
+                    return self::CACHE_TAG_PREFIX . $nodeId;
+                }, $offspringIds));
+            }
+            $this->cache->save($cacheItem);
+        } else {
+            $offspringIds = $cacheItem->get();
+        }
+        return $offspringIds;
+    }
+
+    /**
+     * @throws InvalidArgumentException
+     */
+    public function purgeOffspringCache(Node $node): void
+    {
+        $this->cache->deleteItem(self::CACHE_PREFIX . $node->getId());
+        if ($this->cache instanceof TagAwareCacheInterface) {
+            /*
+             * If cache pool supports tags, we can invalidate all nodes at once.
+             */
+            $this->cache->invalidateTags([self::CACHE_TAG_PREFIX . $node->getId()]);
+        } else {
+            $ancestorsId = $this->managerRegistry
+                ->getRepository(Node::class)
+                ->findAllParentsIdByNode($node);
+            foreach ($ancestorsId as $ancestorId) {
+                $this->cache->deleteItem(self::CACHE_PREFIX . $ancestorId);
+            }
+        }
+    }
+}
diff --git a/lib/RoadizCoreBundle/src/Node/CachedNodeOffspringResolverInterface.php b/lib/RoadizCoreBundle/src/Node/CachedNodeOffspringResolverInterface.php
@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace RZ\Roadiz\CoreBundle\Node;
+
+use RZ\Roadiz\CoreBundle\Entity\Node;
+
+interface CachedNodeOffspringResolverInterface extends NodeOffspringResolverInterface
+{
+    public const CACHE_PREFIX = 'node_offspring_ids_';
+    public const CACHE_TAG_PREFIX = 'node_';
+    public function purgeOffspringCache(Node $node): void;
+}
diff --git a/lib/RoadizCoreBundle/src/Node/NodeOffspringResolverInterface.php b/lib/RoadizCoreBundle/src/Node/NodeOffspringResolverInterface.php
@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace RZ\Roadiz\CoreBundle\Node;
+
+use RZ\Roadiz\CoreBundle\Entity\Node;
+
+interface NodeOffspringResolverInterface
+{
+    /**
+     * @param Node $ancestor
+     * @return array<int>
+     */
+    public function getAllOffspringIds(Node $ancestor): array;
+}
diff --git a/lib/RoadizCoreBundle/src/Repository/NodeRepository.php b/lib/RoadizCoreBundle/src/Repository/NodeRepository.php
@@ -910,6 +910,7 @@ public function findByReverseNodeAndFieldAndTranslation(
     /**
      * @param Node $node
      * @return array<int>
+     * @internal Use NodeOffspringResolverInterface service instead
      */
     public function findAllOffspringIdByNode(Node $node): array
     {
@@ -969,6 +970,10 @@ public function findAllNodeParentsBy(
         );
     }
 
+    /**
+     * @param Node $node
+     * @return array<int|string>
+     */
     public function findAllParentsIdByNode(Node $node): array
     {
         $theParents = [];
@@ -979,7 +984,7 @@ public function findAllParentsIdByNode(Node $node): array
             $parent = $parent->getParent();
         }
 
-        return $theParents;
+        return array_filter($theParents);
     }
 
     /**

diff --git a/lib/RoadizCoreBundle/src/Security/Authorization/Voter/NodeVoter.php b/lib/RoadizCoreBundle/src/Security/Authorization/Voter/NodeVoter.php
@@ -4,10 +4,9 @@
 
 namespace RZ\Roadiz\CoreBundle\Security\Authorization\Voter;
 
-use Doctrine\Persistence\ManagerRegistry;
-use Psr\Cache\CacheItemPoolInterface;
 use RZ\Roadiz\CoreBundle\Entity\Node;
 use RZ\Roadiz\CoreBundle\Entity\NodesSources;
+use RZ\Roadiz\CoreBundle\Node\NodeOffspringResolverInterface;
 use RZ\Roadiz\CoreBundle\Security\Authorization\Chroot\NodeChrootResolver;
 use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
@@ -38,8 +37,7 @@ final class NodeVoter extends Voter
     public function __construct(
         private readonly NodeChrootResolver $chrootResolver,
         private readonly Security $security,
-        private readonly ManagerRegistry $managerRegistry,
-        private readonly CacheItemPoolInterface $cache
+        private readonly NodeOffspringResolverInterface $nodeOffspringResolver,
     ) {
     }
 
@@ -124,17 +122,7 @@ private function isNodeInsideUserChroot(Node $node, Node $chroot, bool $includeC
          * Test if node is inside user chroot using all Chroot node offspring ids
          * to be able to cache all results.
          */
-        $cacheItem = $this->cache->getItem('node_offspring_ids_' . $chroot->getId());
-        if (!$cacheItem->isHit()) {
-            $nodeRepository = $this->managerRegistry->getRepository(Node::class);
-            $offspringIds = $nodeRepository->findAllOffspringIdByNode($chroot);
-            $cacheItem->set($offspringIds);
-            $this->cache->save($cacheItem);
-        } else {
-            $offspringIds = $cacheItem->get();
-        }
-
-        return \in_array($node->getId(), $offspringIds, true);
+        return \in_array($node->getId(), $this->nodeOffspringResolver->getAllOffspringIds($chroot), true);
     }
 
     private function isGrantedWithUserChroot(Node $node, UserInterface $user, array|string $roles, bool $includeChroot): bool

diff --git a/lib/Rozier/src/Controllers/Nodes/NodesController.php b/lib/Rozier/src/Controllers/Nodes/NodesController.php
@@ -22,6 +22,7 @@
 use RZ\Roadiz\CoreBundle\Node\Exception\SameNodeUrlException;
 use RZ\Roadiz\CoreBundle\Node\NodeFactory;
 use RZ\Roadiz\CoreBundle\Node\NodeMover;
+use RZ\Roadiz\CoreBundle\Node\NodeOffspringResolverInterface;
 use RZ\Roadiz\CoreBundle\Node\UniqueNodeGenerator;
 use RZ\Roadiz\CoreBundle\Security\Authorization\Chroot\NodeChrootResolver;
 use RZ\Roadiz\CoreBundle\Security\Authorization\Voter\NodeVoter;
@@ -38,7 +39,7 @@
 use Themes\Rozier\Traits\NodesTrait;
 use Twig\Error\RuntimeError;
 
-class NodesController extends RozierApp
+final class NodesController extends RozierApp
 {
     use NodesTrait;
 
@@ -49,6 +50,7 @@ class NodesController extends RozierApp
      * @param HandlerFactoryInterface $handlerFactory
      * @param UniqueNodeGenerator $uniqueNodeGenerator
      * @param NodeFactory $nodeFactory
+     * @param NodeOffspringResolverInterface $nodeOffspringResolver
      * @param class-string<AbstractType> $nodeFormTypeClass
      * @param class-string<AbstractType> $addNodeFormTypeClass
      */
@@ -59,6 +61,7 @@ public function __construct(
         private readonly HandlerFactoryInterface $handlerFactory,
         private readonly UniqueNodeGenerator $uniqueNodeGenerator,
         private readonly NodeFactory $nodeFactory,
+        private readonly NodeOffspringResolverInterface $nodeOffspringResolver,
         private readonly string $nodeFormTypeClass,
         private readonly string $addNodeFormTypeClass
     ) {
@@ -569,8 +572,7 @@ public function emptyTrashAction(Request $request): Response
             /** @var Node|null $chroot */
             $chroot = $this->nodeChrootResolver->getChroot($this->getUser());
             if ($chroot !== null) {
-                $nodeRepository = $this->em()->getRepository(Node::class);
-                $criteria["parent"] = $nodeRepository->findAllOffspringIdByNode($chroot);
+                $criteria["parent"] = $this->nodeOffspringResolver->getAllOffspringIds($chroot);
             }
 
             $nodes = $this->em()