Merge pull request #5235 from dlubitz/90/task/content-cache-feedback

TASK: Rename parent nodes to ancestor nodes and improve ancestor determination
neos · Sep 6, 2024 · 816cb92 · 816cb92
2 parents 5cf5d0e + 3b21399
commit 816cb92
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 29 deletions.
diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php
@@ -198,8 +198,11 @@ public function apply(EventInterface $event, EventEnvelope $eventEnvelope): void
             RootNodeAggregateWithNodeWasCreated::class => $this->whenRootNodeAggregateWithNodeWasCreated($event, $eventEnvelope),
             SubtreeWasTagged::class => $this->whenSubtreeWasTagged($event),
             SubtreeWasUntagged::class => $this->whenSubtreeWasUntagged($event),
-            WorkspaceWasDiscarded::class => null, // only needed for GraphProjectorCatchUpHookForCacheFlushing
-            WorkspaceWasPartiallyDiscarded::class => null, // only needed for GraphProjectorCatchUpHookForCacheFlushing
+            // the following two events are not actually handled, but we need to include them in {@see canHandle()} in order
+            // to trigger the catchup hooks for those (i.e. {@see GraphProjectorCatchUpHookForCacheFlushing}). This can
+            // be removed with https://github.com/neos/neos-development-collection/issues/4992
+            WorkspaceWasDiscarded::class => null,
+            WorkspaceWasPartiallyDiscarded::class => null,
             default => throw new \InvalidArgumentException(sprintf('Unsupported event %s', get_debug_type($event))),
         };
     }

diff --git a/Neos.Neos/Classes/Fusion/Cache/AssetChangeHandlerForCacheFlushing.php b/Neos.Neos/Classes/Fusion/Cache/AssetChangeHandlerForCacheFlushing.php
@@ -69,30 +69,30 @@ public function registerAssetChange(AssetInterface $asset): void
                     $workspaceName,
                     $nodeAggregate->nodeAggregateId,
                     $nodeAggregate->nodeTypeName,
-                    $this->determineParentNodeAggregateIds($contentRepository, $workspaceName, $nodeAggregate->nodeAggregateId, NodeAggregateIds::createEmpty()),
+                    $this->determineAncestorNodeAggregateIds($contentRepository, $workspaceName, $nodeAggregate->nodeAggregateId),
                 );
 
                 $this->contentCacheFlusher->flushNodeAggregate($flushNodeAggregateRequest, CacheFlushingStrategy::ON_SHUTDOWN);
             }
         }
     }
 
-    private function determineParentNodeAggregateIds(ContentRepository $contentRepository, WorkspaceName $workspaceName, NodeAggregateId $childNodeAggregateId, NodeAggregateIds $collectedParentNodeAggregateIds): NodeAggregateIds
+    private function determineAncestorNodeAggregateIds(ContentRepository $contentRepository, WorkspaceName $workspaceName, NodeAggregateId $childNodeAggregateId): NodeAggregateIds
     {
-        $parentNodeAggregates = $contentRepository->getContentGraph($workspaceName)->findParentNodeAggregates($childNodeAggregateId);
-        $parentNodeAggregateIds = NodeAggregateIds::fromArray(
-            array_map(static fn (NodeAggregate $parentNodeAggregate) => $parentNodeAggregate->nodeAggregateId, iterator_to_array($parentNodeAggregates))
-        );
+        $contentGraph = $contentRepository->getContentGraph($workspaceName);
+        $stack = iterator_to_array($contentGraph->findParentNodeAggregates($childNodeAggregateId));
+
+        $ancestorNodeAggregateIds = [];
+        while ($stack !== []) {
+            $nodeAggregate = array_shift($stack);
 
-        foreach ($parentNodeAggregateIds as $parentNodeAggregateId) {
             // Prevent infinite loops
-            if (!$collectedParentNodeAggregateIds->contain($parentNodeAggregateId)) {
-                $collectedParentNodeAggregateIds = $collectedParentNodeAggregateIds->merge(NodeAggregateIds::create($parentNodeAggregateId));
-                $collectedParentNodeAggregateIds = $this->determineParentNodeAggregateIds($contentRepository, $workspaceName, $parentNodeAggregateId, $collectedParentNodeAggregateIds);
+            if (!in_array($nodeAggregate->nodeAggregateId, $ancestorNodeAggregateIds, false)) {
+                $ancestorNodeAggregateIds[] = $nodeAggregate->nodeAggregateId;
+                array_push($stack, ...iterator_to_array($contentGraph->findParentNodeAggregates($nodeAggregate->nodeAggregateId)));
             }
         }
 
-
-        return $collectedParentNodeAggregateIds;
+        return NodeAggregateIds::fromArray($ancestorNodeAggregateIds);
     }
 }
diff --git a/Neos.Neos/Classes/Fusion/Cache/ContentCacheFlusher.php b/Neos.Neos/Classes/Fusion/Cache/ContentCacheFlusher.php
@@ -112,7 +112,7 @@ private function collectTagsForChangeOnNodeAggregate(
             $flushNodeAggregateRequest->nodeAggregateId,
         ), $tagsToFlush);
 
-        $parentNodeAggregateIds = $flushNodeAggregateRequest->parentNodeAggregateIds;
+        $parentNodeAggregateIds = $flushNodeAggregateRequest->ancestorNodeAggregateIds;
         foreach ($parentNodeAggregateIds as $parentNodeAggregateId) {
             $tagName = CacheTag::forDescendantOfNode($flushNodeAggregateRequest->contentRepositoryId, $workspaceNameToFlush, $parentNodeAggregateId);
             $tagsToFlush[$tagName->value] = sprintf(

diff --git a/Neos.Neos/Classes/Fusion/Cache/FlushNodeAggregateRequest.php b/Neos.Neos/Classes/Fusion/Cache/FlushNodeAggregateRequest.php
@@ -17,7 +17,7 @@ private function __construct(
         public WorkspaceName $workspaceName,
         public NodeAggregateId $nodeAggregateId,
         public NodeTypeName $nodeTypeName,
-        public NodeAggregateIds $parentNodeAggregateIds,
+        public NodeAggregateIds $ancestorNodeAggregateIds,
     ) {
     }
 
@@ -26,14 +26,14 @@ public static function create(
         WorkspaceName $workspaceName,
         NodeAggregateId $nodeAggregateId,
         NodeTypeName $nodeTypeName,
-        NodeAggregateIds $parentNodeAggregateIds
+        NodeAggregateIds $ancestorNodeAggregateIds
     ): self {
         return new self(
             $contentRepositoryId,
             $workspaceName,
             $nodeAggregateId,
             $nodeTypeName,
-            $parentNodeAggregateIds
+            $ancestorNodeAggregateIds
         );
     }
 }
diff --git a/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php b/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php
@@ -262,7 +262,7 @@ private function scheduleCacheFlushJobForNodeAggregate(
             $workspaceName,
             $nodeAggregate->nodeAggregateId,
             $nodeAggregate->nodeTypeName,
-            $this->determineParentNodeAggregateIds($workspaceName, $nodeAggregate->nodeAggregateId, NodeAggregateIds::createEmpty())
+            $this->determineAncestorNodeAggregateIds($workspaceName, $nodeAggregate->nodeAggregateId)
         );
     }
 
@@ -277,22 +277,23 @@ private function scheduleCacheFlushJobForWorkspaceName(
         );
     }
 
-    private function determineParentNodeAggregateIds(WorkspaceName $workspaceName, NodeAggregateId $childNodeAggregateId, NodeAggregateIds $collectedParentNodeAggregateIds): NodeAggregateIds
+    private function determineAncestorNodeAggregateIds(WorkspaceName $workspaceName, NodeAggregateId $childNodeAggregateId): NodeAggregateIds
     {
-        $parentNodeAggregates = $this->contentRepository->getContentGraph($workspaceName)->findParentNodeAggregates($childNodeAggregateId);
-        $parentNodeAggregateIds = NodeAggregateIds::fromArray(
-            array_map(static fn (NodeAggregate $parentNodeAggregate) => $parentNodeAggregate->nodeAggregateId, iterator_to_array($parentNodeAggregates))
-        );
+        $contentGraph = $this->contentRepository->getContentGraph($workspaceName);
+        $stack = iterator_to_array($contentGraph->findParentNodeAggregates($childNodeAggregateId));
+
+        $ancestorNodeAggregateIds = [];
+        while ($stack !== []) {
+            $nodeAggregate = array_shift($stack);
 
-        foreach ($parentNodeAggregateIds as $parentNodeAggregateId) {
             // Prevent infinite loops
-            if (!$collectedParentNodeAggregateIds->contain($parentNodeAggregateId)) {
-                $collectedParentNodeAggregateIds = $collectedParentNodeAggregateIds->merge(NodeAggregateIds::create($parentNodeAggregateId));
-                $collectedParentNodeAggregateIds = $this->determineParentNodeAggregateIds($workspaceName, $parentNodeAggregateId, $collectedParentNodeAggregateIds);
+            if (!in_array($nodeAggregate->nodeAggregateId, $ancestorNodeAggregateIds, false)) {
+                $ancestorNodeAggregateIds[] = $nodeAggregate->nodeAggregateId;
+                array_push($stack, ...iterator_to_array($contentGraph->findParentNodeAggregates($nodeAggregate->nodeAggregateId)));
             }
         }
 
-        return $collectedParentNodeAggregateIds;
+        return NodeAggregateIds::fromArray($ancestorNodeAggregateIds);
     }
 
     public function onBeforeBatchCompleted(): void