BUG: Discarded node remains in cache if UI was reloaded between creation and discard #5150

grebaldi · 2024-06-19T12:23:08Z

I've encountered some weird occasions in which the content cache becomes stale after discarding.

For example:

If I create a content node and then immediately discard it, the node remains cached:

Peek.2024-06-19.14-15.Discard.Bug.-.Behavior.without.Property.Changes.mp4

If I create a content node, then edit its properties and immediately discard it, the node vanishes from cache (which is what should always happen):

Peek.2024-06-19.14-11.Discard.Bug.-.Desired.Behavior.mp4

Now, weirdly, if I create a content node, then edit its properties, then reload the UI and discard it, the node remains cached:

Peek.2024-06-19.14-13.Discard.Bug.-.Behavior.after.Reload.mp4

kitsunet · 2024-06-19T12:47:07Z

Is this an UI bug? or rather a backend bug? I guess the middle "correct" behavior is due to the change flushing the cache correctly and discarding it before rebuilding the cache entry will lead to it being gone from the cache.

grebaldi · 2024-06-19T13:01:40Z

@kitsunet

Is this an UI bug? or rather a backend bug?

Not sure. The fact that it happens when no PropertiesWereSet event occurs speaks for the backend. The fact that it also happens after reloading the UI speaks for the UI.

I guess the middle "correct" behavior is due to the change flushing the cache correctly and discarding it before rebuilding the cache entry will lead to it being gone from the cache.

Plausible, indeed. This would leave us with the backend being the culprit.

In theory, if I were to change a property in the inspector that causes a reloadIfChanged, I should then also see a stale cache entry after discard (because the node was re-rendered in between). I'll test this to see if your hypothesis holds.

EDIT: It does not :( But this could also be due to differences between regular fusion rendering and out of band rendering. So, your explanation is not ruled out.

EDIT 2: Found it. It's a regression introduced with #5083, so definitely a backend thing. I'll move the issue.

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.

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 neos#5150, with this commit, said verification fails.

mhsdesign · 2024-06-23T13:49:36Z

We do rely hard core on the fact that we create a new cache entry if we discard / publish or do any thing. Thats why we dont flush these affected nodes. Because we just render the stuff and let the cache entry get garbage collected if at all :D

before (after deleting Text Node in the middle of the document and rerendering:

  entries => array(2)
    string "70d69c77a4e9fef0fc86521f2682d7df" (32) => string "<div class="neos-contentcollection">[Text Node at the start of the document][Text Node in the middle of the document][Text Node at the end of the document]</div>" (161)
    string "73dff7036302d0d063a22b3e1190e0c3" (32) => string "<div class="neos-contentcollection">[Text Node at the start of the document][Text Node at the end of the document]</div>" (120)
  tagsAndEntries => array(4)
    string "DescendantOf_user-editor_default_test-document-with-contents--main" (66) => array(2)
      string "70d69c77a4e9fef0fc86521f2682d7df" (32) => true
      string "73dff7036302d0d063a22b3e1190e0c3" (32) => true
    string "DescendantOf_default_test-document-with-contents--main" (54) => array(2)
      string "70d69c77a4e9fef0fc86521f2682d7df" (32) => true
      string "73dff7036302d0d063a22b3e1190e0c3" (32) => true
    string "Node_user-editor_default_test-document-with-contents--main" (58) => array(2)
      string "70d69c77a4e9fef0fc86521f2682d7df" (32) => true
      string "73dff7036302d0d063a22b3e1190e0c3" (32) => true
    string "Node_default_test-document-with-contents--main" (46) => array(2)
      string "70d69c77a4e9fef0fc86521f2682d7df" (32) => true
      string "73dff7036302d0d063a22b3e1190e0c3" (32) => true

now

  entries => array(1)
    string "9f4f3ec0a51728a02c7a0db6cd33d87a" (32) => string "<div class="neos-contentcollection">[Text Node at the start of the document][Text Node in the middle of the document][Text Node at the end of the document]</div>" (161)
  tagsAndEntries => array(4)
    string "DescendantOf_user-editor_default_test-document-with-contents--main" (66) => array(1)
      string "9f4f3ec0a51728a02c7a0db6cd33d87a" (32) => true
    string "DescendantOf_default_test-document-with-contents--main" (54) => array(1)
      string "9f4f3ec0a51728a02c7a0db6cd33d87a" (32) => true
    string "Node_user-editor_default_test-document-with-contents--main" (58) => array(1)
      string "9f4f3ec0a51728a02c7a0db6cd33d87a" (32) => true
    string "Node_default_test-document-with-contents--main" (46) => array(1)
      string "9f4f3ec0a51728a02c7a0db6cd33d87a" (32) => true

The problem is even with the changes here #5163 where we check the nodes that were discarded partially, if they were deleted we will never be able to calculate their parents and thus flush correctly the DescendantOf_default_test-document-with-contents--main tag.

These are the maximum information that are available. Everything that is local to the text-node-middle node. But no nodetypes or parents.

array(4)
 string "Node_user-editor_default_text-node-middle" (41) => string "which were tagged with "Node_user-editor_default_text-node-middle" because that identifier has changed." (103)
 string "DynamicNodeTag_user-editor_default_text-node-middle" (51) => string "which were tagged with "DynamicNodeTag_user-editor_default_text-node-middle" because that identifier has changed." (113)
 string "DescendantOf_user-editor_default_text-node-middle" (49) => string "which were tagged with "DescendantOf_user-editor_default_text-node-middle" because node "Node_user-editor_default_text-node-middle" has changed." (144)
 string "Everything" (10) => string "which were tagged with "Everything"." (36)

Places were we manifest this behaviour:

neos-development-collection/Neos.Neos/Classes/Fusion/Cache/ContentCacheFlusher.php

Lines 99 to 102 in 2b4890c

    
           if (!$nodeAggregate) { 
        
               // Node Aggregate was removed in the meantime, so no need to clear caches on this one anymore. 
        
               return []; 
        
           }

by not handling WorkspaceWasPartiallyDiscarded or WorkspaceWasDiscarded at all

Ideas to solve this:

flush all tags that contain the workspace that is published, deleted, etc
see how Neos 8.3 does it as there are no changing content streams as well to rely on
use content stream ids forever??? BUGFIX: Fix content cache for discard, revert #5083 for now #5163

P.S. i deobfuscated my cache tags locally to not use sha1 for debugging so they are understandable, see #5164

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 neos#5150, with this commit, said verification fails.

… nodes" Discarding individual nodes works through a separate command that still needs to be considered by the recently introduced cache invalidation mechanism in `WorkspaceProjectorCatchUpHookForCacheFlushing`. This commit introduces a test that illustrates the issue.

grebaldi added Bug 9.0 labels Jun 19, 2024

grebaldi transferred this issue from neos/neos-ui Jun 19, 2024

grebaldi mentioned this issue Jun 21, 2024

WIP: BUGFIX: Flush content cache on discard #5155

Closed

This was referenced Jun 23, 2024

TASK: adjust to overhauled node uri building neos/neos-ui#3802

Merged

WIP 5150 flush content cache correctly #5160

Closed

mhsdesign mentioned this issue Jun 23, 2024

BUGFIX: Fix content cache for discard, revert #5083 for now #5163

Merged

6 tasks

mhsdesign closed this as completed in #5163 Jun 23, 2024

dlubitz mentioned this issue Jul 5, 2024

Overhaul ContentCacheFlusher #5175

Closed

grebaldi added a commit to grebaldi/neos-development-collection that referenced this issue Jul 8, 2024

WIP: Modify last test introduced for neos#5150

05cedb8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Discarded node remains in cache if UI was reloaded between creation and discard #5150

BUG: Discarded node remains in cache if UI was reloaded between creation and discard #5150

grebaldi commented Jun 19, 2024

kitsunet commented Jun 19, 2024

grebaldi commented Jun 19, 2024 •

edited

Loading

mhsdesign commented Jun 23, 2024 •

edited

Loading

BUG: Discarded node remains in cache if UI was reloaded between creation and discard #5150

BUG: Discarded node remains in cache if UI was reloaded between creation and discard #5150

Comments

grebaldi commented Jun 19, 2024

kitsunet commented Jun 19, 2024

grebaldi commented Jun 19, 2024 • edited Loading

mhsdesign commented Jun 23, 2024 • edited Loading

grebaldi commented Jun 19, 2024 •

edited

Loading

mhsdesign commented Jun 23, 2024 •

edited

Loading