From f395ef615652cc509f6c0538d3d2f6a95b6f041f Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 23 Jun 2024 10:51:04 +0200 Subject: [PATCH 1/5] WIP 5150 flush content cache correctly --- ...phProjectorCatchUpHookForCacheFlushing.php | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php b/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php index c71a2965ca2..dddc9e7cac2 100644 --- a/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php +++ b/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php @@ -140,12 +140,7 @@ public function onBeforeEvent(EventInterface $eventInstance, EventEnvelope $even // cleared, leading to presumably duplicate nodes in the UI. || $eventInstance instanceof NodeAggregateWasMoved ) { - $workspace = $this->contentRepository->getWorkspaceFinder()->findOneByCurrentContentStreamId($eventInstance->getContentStreamId()); - if ($workspace === null) { - return; - } - // FIXME: EventInterface->workspaceName - $contentGraph = $this->contentRepository->getContentGraph($workspace->workspaceName); + $contentGraph = $this->contentRepository->getContentGraph($eventInstance->workspaceName); $nodeAggregate = $contentGraph->findNodeAggregateById( $eventInstance->getNodeAggregateId() ); @@ -157,7 +152,7 @@ public function onBeforeEvent(EventInterface $eventInstance, EventEnvelope $even assert($parentNodeAggregate instanceof NodeAggregate); $this->scheduleCacheFlushJobForNodeAggregate( $this->contentRepository, - $workspace->workspaceName, + $eventInstance->workspaceName, $parentNodeAggregate->nodeAggregateId ); } @@ -177,19 +172,20 @@ public function onAfterEvent(EventInterface $eventInstance, EventEnvelope $event !($eventInstance instanceof NodeAggregateWasRemoved) && $eventInstance instanceof EmbedsContentStreamAndNodeAggregateId ) { - $workspace = $this->contentRepository->getWorkspaceFinder()->findOneByCurrentContentStreamId($eventInstance->getContentStreamId()); - if ($workspace === null) { - return; - } - // FIXME: EventInterface->workspaceName - $nodeAggregate = $this->contentRepository->getContentGraph($workspace->workspaceName)->findNodeAggregateById( + // Hack, we don't declare the `workspaceName` as part of the `EmbedsContentStreamAndNodeAggregateId` events, + // but it will always exist! + // See https://github.com/neos/neos-development-collection/issues/5152 + /** @phpstan-ignore property.notFound */ + $workspaceName = $eventInstance->workspaceName; + + $nodeAggregate = $this->contentRepository->getContentGraph($workspaceName)->findNodeAggregateById( $eventInstance->getNodeAggregateId() ); if ($nodeAggregate) { $this->scheduleCacheFlushJobForNodeAggregate( $this->contentRepository, - $workspace->workspaceName, + $workspaceName, $nodeAggregate->nodeAggregateId ); } From 5b7d3c7e098dc92525e32eb0190c0edc4e168476 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 23 Jun 2024 11:28:45 +0200 Subject: [PATCH 2/5] WIP Fix `GraphProjectorCatchUpHookForCacheFlushing` to not crash if workspace doesnt exist anymore ``` Scenario: When a change is applied to the forked content stream AFTER the fork, it is not visible in the live content stream. # Features/ContentStreamForking/ForkContentStreamWithoutDimensions.feature:61 And the event NodePropertiesWereSet was published with payload: # Features/ContentStreamForking/ForkContentStreamWithoutDimensions.feature:66 Neos\ContentRepository\Core\SharedModel\Exception\WorkspaceDoesNotExist: The source workspace user does not exist ``` --- .../GraphProjectorCatchUpHookForCacheFlushing.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php b/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php index dddc9e7cac2..405ed04b649 100644 --- a/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php +++ b/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php @@ -19,6 +19,7 @@ use Neos\ContentRepository\Core\Feature\NodeRemoval\Event\NodeAggregateWasRemoved; use Neos\ContentRepository\Core\Projection\CatchUpHookInterface; use Neos\ContentRepository\Core\Projection\ContentGraph\NodeAggregate; +use Neos\ContentRepository\Core\SharedModel\Exception\WorkspaceDoesNotExist; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName; use Neos\EventStore\Model\EventEnvelope; @@ -140,7 +141,11 @@ public function onBeforeEvent(EventInterface $eventInstance, EventEnvelope $even // cleared, leading to presumably duplicate nodes in the UI. || $eventInstance instanceof NodeAggregateWasMoved ) { - $contentGraph = $this->contentRepository->getContentGraph($eventInstance->workspaceName); + try { + $contentGraph = $this->contentRepository->getContentGraph($eventInstance->workspaceName); + } catch (WorkspaceDoesNotExist) { + return; + } $nodeAggregate = $contentGraph->findNodeAggregateById( $eventInstance->getNodeAggregateId() ); @@ -178,7 +183,12 @@ public function onAfterEvent(EventInterface $eventInstance, EventEnvelope $event /** @phpstan-ignore property.notFound */ $workspaceName = $eventInstance->workspaceName; - $nodeAggregate = $this->contentRepository->getContentGraph($workspaceName)->findNodeAggregateById( + try { + $contentGraph = $this->contentRepository->getContentGraph($workspaceName); + } catch (WorkspaceDoesNotExist) { + return; + } + $nodeAggregate = $contentGraph->findNodeAggregateById( $eventInstance->getNodeAggregateId() ); From c55bd5df240551fc506b264ed311330311de85a5 Mon Sep 17 00:00:00 2001 From: Wilhelm Behncke Date: Fri, 21 Jun 2024 14:06:26 +0200 Subject: [PATCH 3/5] TASK: Add failing test for #5150 The test scenario consists of a document with a main content collection that initially contains two simple text content nodes. During the scenario, we switch to a user workspace and insert a third text node between the two initial nodes. The test then verifies, that fusion renders all three text nodes in order. Afterwards we discard the change we just made and check whether fusion now stops rendering the discarded node. Confirming #5150, with this commit, said verification fails. --- .../ContentCache/NodesInUserWorkspace.feature | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 Neos.Neos/Tests/Behavior/Features/ContentCache/NodesInUserWorkspace.feature diff --git a/Neos.Neos/Tests/Behavior/Features/ContentCache/NodesInUserWorkspace.feature b/Neos.Neos/Tests/Behavior/Features/ContentCache/NodesInUserWorkspace.feature new file mode 100644 index 00000000000..90463864f49 --- /dev/null +++ b/Neos.Neos/Tests/Behavior/Features/ContentCache/NodesInUserWorkspace.feature @@ -0,0 +1,143 @@ +@flowEntities +Feature: Tests for the ContentCacheFlusher and cache flushing when applied in user workspaces + + Background: + Given using no content dimensions + And using the following node types: + """yaml + 'Neos.ContentRepository:Root': {} + 'Neos.Neos:Sites': + superTypes: + 'Neos.ContentRepository:Root': true + 'Neos.Neos:Document': + properties: + title: + type: string + uriPathSegment: + type: string + 'Neos.Neos:ContentCollection': + constraints: + nodeTypes: + 'Neos.Neos:Document': false + '*': true + 'Neos.Neos:Content': + constraints: + nodeTypes: + '*': false + 'Neos.Neos:Site': + superTypes: + 'Neos.Neos:Document': true + 'Neos.Neos:Test.DocumentTypeWithMainCollection': + superTypes: + 'Neos.Neos:Document': true + childNodes: + main: + type: 'Neos.Neos:ContentCollection' + 'Neos.Neos:Test.TextNode': + superTypes: + 'Neos.Neos:Content': true + properties: + text: + type: string + """ + And using identifier "default", I define a content repository + And I am in content repository "default" + And I am user identified by "editor" + + When the command CreateRootWorkspace is executed with payload: + | Key | Value | + | workspaceName | "live" | + | newContentStreamId | "cs-identifier" | + And the command CreateWorkspace is executed with payload: + | Key | Value | + | workspaceName | "user-editor" | + | baseWorkspaceName | "live" | + | newContentStreamId | "user-editor-cs-identifier" | + And I am in workspace "live" and dimension space point {} + And the command CreateRootNodeAggregateWithNode is executed with payload: + | Key | Value | + | nodeAggregateId | "root" | + | nodeTypeName | "Neos.Neos:Sites" | + And the following CreateNodeAggregateWithNode commands are executed: + | nodeAggregateId | parentNodeAggregateId | nodeTypeName | initialPropertyValues | nodeName | tetheredDescendantNodeAggregateIds | + | site | root | Neos.Neos:Site | {} | site | {} | + | test-document-with-contents | site | Neos.Neos:Test.DocumentTypeWithMainCollection | {"uriPathSegment": "test-document-with-contents", "title": "Test document with contents"} | test-document-with-contents | {"main": "test-document-with-contents--main"} | + | text-node-start | test-document-with-contents--main | Neos.Neos:Test.TextNode | {"text": "Text Node at the start of the document"} | text-node-start | {} | + | text-node-end | test-document-with-contents--main | Neos.Neos:Test.TextNode | {"text": "Text Node at the end of the document"} | text-node-end | {} | + When the command RebaseWorkspace is executed with payload: + | Key | Value | + | workspaceName | "user-editor" | + And A site exists for node name "site" and domain "http://localhost" + And the sites configuration is: + """yaml + Neos: + Neos: + sites: + '*': + contentRepository: default + contentDimensions: + resolver: + factoryClassName: Neos\Neos\FrontendRouting\DimensionResolution\Resolver\NoopResolverFactory + """ + And the Fusion context node is "site" + And the Fusion context request URI is "http://localhost" + And I have the following Fusion setup: + """fusion + include: resource://Neos.Fusion/Private/Fusion/Root.fusion + include: resource://Neos.Neos/Private/Fusion/Root.fusion + + prototype(Neos.Neos:Test.TextNode) < prototype(Neos.Neos:ContentComponent) { + renderer = ${"[" + q(node).property("text") + "]"} + } + """ + + Scenario: ContentCache gets flushed when a node that was just created gets discarded + Given I have Fusion content cache enabled + And I am in workspace "user-editor" and dimension space point {} + And the Fusion context node is "test-document-with-contents" + And I execute the following Fusion code: + """fusion + test = Neos.Neos:ContentCollection { + nodePath = "main" + } + """ + Then I expect the following Fusion rendering result: + """ +
[Text Node at the start of the document][Text Node at the end of the document]
+ """ + + When the command CreateNodeAggregateWithNode is executed with payload: + | Key | Value | + | nodeAggregateId | "text-node-middle" | + | nodeTypeName | "Neos.Neos:Test.TextNode" | + | parentNodeAggregateId | "test-document-with-contents--main" | + | initialPropertyValues | {"text": "Text Node in the middle of the document"} | + | succeedingSiblingNodeAggregateId | "text-node-end" | + | nodeName | "text-node-middle" | + And I execute the following Fusion code: + """fusion + test = Neos.Neos:ContentCollection { + nodePath = "main" + } + """ + Then I expect the following Fusion rendering result: + """ +
[Text Node at the start of the document][Text Node in the middle of the document][Text Node at the end of the document]
+ """ + + When the command DiscardWorkspace is executed with payload: + | Key | Value | + | workspaceName | "user-editor" | + And I am in workspace "user-editor" and dimension space point {} + Then I expect node aggregate identifier "text-node-middle" to lead to no node + + When I execute the following Fusion code: + """fusion + test = Neos.Neos:ContentCollection { + nodePath = "main" + } + """ + Then I expect the following Fusion rendering result: + """ +
[Text Node at the start of the document][Text Node at the end of the document]
+ """ From ed09cf7f29a87dd9dad133ecfb79129321ff6ebc Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 23 Jun 2024 12:50:00 +0200 Subject: [PATCH 4/5] WIP add failing tests for content cache and `DiscardIndividualNodesFromWorkspace` --- .../ContentCache/NodesInUserWorkspace.feature | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/Neos.Neos/Tests/Behavior/Features/ContentCache/NodesInUserWorkspace.feature b/Neos.Neos/Tests/Behavior/Features/ContentCache/NodesInUserWorkspace.feature index 90463864f49..63db9d29952 100644 --- a/Neos.Neos/Tests/Behavior/Features/ContentCache/NodesInUserWorkspace.feature +++ b/Neos.Neos/Tests/Behavior/Features/ContentCache/NodesInUserWorkspace.feature @@ -141,3 +141,55 @@ Feature: Tests for the ContentCacheFlusher and cache flushing when applied in us """
[Text Node at the start of the document][Text Node at the end of the document]
""" + + Scenario: ContentCache gets flushed when a node that was just created gets explicitly discarded + Given I have Fusion content cache enabled + And I am in workspace "user-editor" and dimension space point {} + And the Fusion context node is "test-document-with-contents" + And I execute the following Fusion code: + """fusion + test = Neos.Neos:ContentCollection { + nodePath = "main" + } + """ + Then I expect the following Fusion rendering result: + """ +
[Text Node at the start of the document][Text Node at the end of the document]
+ """ + + When the command CreateNodeAggregateWithNode is executed with payload: + | Key | Value | + | nodeAggregateId | "text-node-middle" | + | nodeTypeName | "Neos.Neos:Test.TextNode" | + | parentNodeAggregateId | "test-document-with-contents--main" | + | initialPropertyValues | {"text": "Text Node in the middle of the document"} | + | succeedingSiblingNodeAggregateId | "text-node-end" | + | nodeName | "text-node-middle" | + And I execute the following Fusion code: + """fusion + test = Neos.Neos:ContentCollection { + nodePath = "main" + } + """ + Then I expect the following Fusion rendering result: + """ +
[Text Node at the start of the document][Text Node in the middle of the document][Text Node at the end of the document]
+ """ + + When the command DiscardIndividualNodesFromWorkspace is executed with payload: + | Key | Value | + | workspaceName | "user-editor" | + | nodesToDiscard | [{"dimensionSpacePoint": {}, "nodeAggregateId": "text-node-middle"}] | + And I am in workspace "user-editor" and dimension space point {} + Then I expect node aggregate identifier "text-node-middle" to lead to no node + + When I execute the following Fusion code: + """fusion + test = Neos.Neos:ContentCollection { + nodePath = "main" + } + """ + Then I expect the following Fusion rendering result: + """ +
[Text Node at the start of the document][Text Node at the end of the document]
+ """ From 7fabee1c029f025a4858561e68db8dae354e81b1 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 23 Jun 2024 12:51:29 +0200 Subject: [PATCH 5/5] WIP FIX neos Ui e2e edge case test Discarded node move changes are reflected correctly in the document tree Scenario #2: Moved nodes do not just disappear after discarding the move change --- .../src/DoctrineDbalContentGraphProjection.php | 5 +++++ .../GraphProjectorCatchUpHookForCacheFlushing.php | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php index d2b530d7333..433fd9f6322 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php @@ -43,6 +43,7 @@ use Neos\ContentRepository\Core\Feature\SubtreeTagging\Dto\SubtreeTags; use Neos\ContentRepository\Core\Feature\SubtreeTagging\Event\SubtreeWasTagged; use Neos\ContentRepository\Core\Feature\SubtreeTagging\Event\SubtreeWasUntagged; +use Neos\ContentRepository\Core\Feature\WorkspacePublication\Event\WorkspaceWasPartiallyDiscarded; use Neos\ContentRepository\Core\Infrastructure\DbalCheckpointStorage; use Neos\ContentRepository\Core\Infrastructure\DbalSchemaDiff; use Neos\ContentRepository\Core\NodeType\NodeTypeName; @@ -171,6 +172,7 @@ public function canHandle(EventInterface $event): bool RootNodeAggregateWithNodeWasCreated::class, SubtreeWasTagged::class, SubtreeWasUntagged::class, + WorkspaceWasPartiallyDiscarded::class ]); } @@ -195,6 +197,9 @@ public function apply(EventInterface $event, EventEnvelope $eventEnvelope): void RootNodeAggregateWithNodeWasCreated::class => $this->whenRootNodeAggregateWithNodeWasCreated($event, $eventEnvelope), SubtreeWasTagged::class => $this->whenSubtreeWasTagged($event), SubtreeWasUntagged::class => $this->whenSubtreeWasUntagged($event), + // workaround to allow the content graph catchuphook to react on this already now + // with https://github.com/neos/neos-development-collection/pull/5096 we will really handle the event here as well + WorkspaceWasPartiallyDiscarded::class => null, default => throw new \InvalidArgumentException(sprintf('Unsupported event %s', get_debug_type($event))), }; } diff --git a/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php b/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php index 405ed04b649..3f1595bb89c 100644 --- a/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php +++ b/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php @@ -17,6 +17,7 @@ use Neos\ContentRepository\Core\Feature\Common\EmbedsContentStreamAndNodeAggregateId; use Neos\ContentRepository\Core\Feature\NodeMove\Event\NodeAggregateWasMoved; use Neos\ContentRepository\Core\Feature\NodeRemoval\Event\NodeAggregateWasRemoved; +use Neos\ContentRepository\Core\Feature\WorkspacePublication\Event\WorkspaceWasPartiallyDiscarded; use Neos\ContentRepository\Core\Projection\CatchUpHookInterface; use Neos\ContentRepository\Core\Projection\ContentGraph\NodeAggregate; use Neos\ContentRepository\Core\SharedModel\Exception\WorkspaceDoesNotExist; @@ -134,6 +135,16 @@ public function onBeforeEvent(EventInterface $eventInstance, EventEnvelope $even return; } + if ($eventInstance instanceof WorkspaceWasPartiallyDiscarded) { + foreach ($eventInstance->discardedNodes as $discardedNode) { + $this->scheduleCacheFlushJobForNodeAggregate( + $this->contentRepository, + $eventInstance->workspaceName, + $discardedNode->nodeAggregateId + ); + } + } + if ( $eventInstance instanceof NodeAggregateWasRemoved // NOTE: when moving a node, we need to clear the cache not just after the move was completed,