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

Add cache invalidation for snippet areas #7177

Merged

Conversation

Prokyonn
Copy link
Member

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
License MIT

What's in this PR?

Invalidate snippet area cache, when the area snippet is modified/removed

Why?

Cache invalidation does not work properly for default area snippets that were updated

@@ -74,6 +83,8 @@ public function loadByArea($area, $webspaceKey = null, $locale = null)
$locale = $this->requestAnalyzer->getCurrentLocalization()->getLocale();
}

$this->snippetAreaReferenceStore->add($area);
Copy link
Member

Choose a reason for hiding this comment

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

This could let to invalidation of the whole website when a global area snippet is used. Lets discuss this with @chirimoya on thursday.

Copy link
Member Author

Choose a reason for hiding this comment

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

In summary of our discussion, we will introduce a new parameter to the area configuration in the snippet template, which will determine whether the snippet-area should be excluded from the cache invalidation. By default, the snippet-area will always be invalidated. The only exception is for the automatically created area, they should not be invalidated, otherwise the old behaviour would be changed.

The decision of whether the invalidation should take place will be integrated into the CacheInvalidationSubscriber. Moreover, the snippet-area will always be included in the reference store. The benefit of this is, that we only have to check in one class if the snippet-area should be invalidated.

Copy link
Member

@alexander-schranz alexander-schranz left a comment

Choose a reason for hiding this comment

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

We should target 2.4 for this. as discussed we add a new feature flags for snippets here

@Prokyonn Prokyonn force-pushed the enhancement/snippet-cache-invalidation branch from 39057f3 to 05cc7de Compare October 18, 2023 07:08
@alexander-schranz alexander-schranz changed the base branch from 2.5 to 2.6 February 5, 2024 16:39
@alexander-schranz alexander-schranz added the Feature New functionality not yet included in Sulu label Feb 5, 2024
@alexander-schranz alexander-schranz added this to the Release 2.6 milestone Feb 5, 2024
@Prokyonn Prokyonn force-pushed the enhancement/snippet-cache-invalidation branch from 05cc7de to 5aca841 Compare February 12, 2024 08:19
@wachterjohannes
Copy link
Member

@Prokyonn please rebase to see if the tests turn green and @alexander-schranz really target that to 2.4?

@alexander-schranz
Copy link
Member

2.6 is correct target branch for this.

@Prokyonn
Copy link
Member Author

@Prokyonn please rebase to see if the tests turn green and @alexander-schranz really target that to 2.4?

@wachterjohannes php 8.0 did fail because of the readonly keyword 😅

last week we discussed it and decided that it will be part of 2.6 and not backported to 2.4

Prokyonn and others added 2 commits February 12, 2024 16:44
Co-authored-by: Alexander Schranz <alexander@sulu.io>
Copy link
Member

@alexander-schranz alexander-schranz left a comment

Choose a reason for hiding this comment

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

Should fix the phpstan issues

@alexander-schranz alexander-schranz merged commit 7a43253 into sulu:2.6 Feb 13, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New functionality not yet included in Sulu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants