Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

!!! FEATURE: Remove legacy cache tag support #3639

Merged
merged 4 commits into from
Mar 24, 2022

Conversation

Sebobo
Copy link
Member

@Sebobo Sebobo commented Mar 18, 2022

What I did

In Neos 4.1 new Eel helpers were introduced to generate content cache tags in Fusion. Those make sure that the cache tags provided by the integrator are valid.
Until Neos 4.0 they needed to be written manually as string which was error prone.

But to support old cache tags that were lacking the workspace context the ContentCacheFlusher still cleared them.
This change removes that support. So this is breaking and projects need to be adjusted when upgrading and still using those old cache tags.

The advantage of removing them is that the number of cache tags to be flushed during publishing is lower. TODO: Add number.

This change requires #3631 to be merged first.

How I did it

Removed the legacy cache tag generation.

The change also include a new code migration version 20220318111600 to replace the most commonly used legacy cache tags with matching eel helper calls:

node = ${'Node_' + node.identifier}
descendants = ${'DescendantOf_' + node.identifier}
nodeType = 'NodeType_My.Vendor:Content.Foo'

@Sebobo
Copy link
Member Author

Sebobo commented Mar 18, 2022

Currently checking if I can provide a core migration for those old tags.

@Sebobo Sebobo self-assigned this Mar 18, 2022
@mficzel
Copy link
Member

mficzel commented Mar 21, 2022

Would love to get this into 8, how can i help?

@Sebobo Sebobo marked this pull request as ready for review March 21, 2022 08:16
@Sebobo
Copy link
Member Author

Sebobo commented Mar 21, 2022

@mficzel It's actually ready, but #3631 should be merged first because this PR is based on its changes.

@mficzel
Copy link
Member

mficzel commented Mar 23, 2022

@Sebobo am a bit confused as some of the changes look like they belonged to #3631 does this need a rebase?

@mficzel mficzel self-requested a review March 23, 2022 14:42
@Sebobo
Copy link
Member Author

Sebobo commented Mar 23, 2022

Yes, will quickly do it.

Sebobo added 2 commits March 23, 2022 15:45
With this change Neos stops flushing old style cache tags used in Neos < 4.1.

To prevent issues with caches that are not updated,
all content cache tags in Fusion should be defined with
the `Neos.Caching` Eel helper which exists since Neos 4.1.
Copy link
Member

@markusguenther markusguenther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me by reading

@Sebobo
Copy link
Member Author

Sebobo commented Mar 23, 2022

Hm I rebased and force pushed, but nothing is happening...

@Sebobo Sebobo force-pushed the feature/remove-legacy-cache-tag-support branch from 38ba59e to cc845a4 Compare March 23, 2022 14:55
@Sebobo
Copy link
Member Author

Sebobo commented Mar 23, 2022

Another amend and force pushed fixed it. Now only the new commits are visible.

Copy link
Member

@mficzel mficzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by reading

@kdambekalns kdambekalns reopened this Mar 23, 2022
@kdambekalns
Copy link
Member

Ah, I see…

GitHub Actions is experiencing degraded performance. We are continuing to investigate.

@kdambekalns kdambekalns enabled auto-merge March 23, 2022 15:43
@jonnitto
Copy link
Member

Try to trigger the check…

@jonnitto jonnitto closed this Mar 24, 2022
auto-merge was automatically disabled March 24, 2022 06:27

Pull request was closed

@jonnitto jonnitto reopened this Mar 24, 2022
@jonnitto jonnitto enabled auto-merge March 24, 2022 06:27
@jonnitto jonnitto merged commit fafd4aa into neos:master Mar 24, 2022
@Sebobo Sebobo deleted the feature/remove-legacy-cache-tag-support branch March 24, 2022 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants