Skip to content

BUGFIX: Use ContentCacheFlusher Aspect for banning varnish pages#56

Merged
daniellienert merged 3 commits intoFlowpack:4.0from
paxuclus:bugfix/contentcacheflusher-aspect
Aug 17, 2021
Merged

BUGFIX: Use ContentCacheFlusher Aspect for banning varnish pages#56
daniellienert merged 3 commits intoFlowpack:4.0from
paxuclus:bugfix/contentcacheflusher-aspect

Conversation

@paxuclus
Copy link
Copy Markdown
Collaborator

@paxuclus paxuclus commented Feb 22, 2021

The cache tags of the Neos ContentCacheFlusher are now used instead
of the manually generated ones from the ContentCacheFlusherService

Resolves #55

todo:

  • Mark ContentCacheFlusher as deprecated

The cache tags of the Neos ContentCacheFlusher are now used instead
of the manually generated ones from the ContentCacheFlusherService

Resolves Flowpack#55
@paxuclus
Copy link
Copy Markdown
Collaborator Author

@daniellienert can you please open a 4.0 branch from b431dbb so I can target this against that? Since the package was moved to the flowpack namespace I don't have maintain permissions anymore.

@paxuclus paxuclus marked this pull request as draft February 22, 2021 14:21
@paxuclus paxuclus changed the base branch from master to 4.0 February 22, 2021 21:21
@paxuclus paxuclus marked this pull request as ready for review February 22, 2021 21:22
@paxuclus paxuclus self-assigned this Feb 23, 2021
@paxuclus paxuclus marked this pull request as draft February 23, 2021 07:52
@paxuclus paxuclus marked this pull request as ready for review March 7, 2021 16:18
@paxuclus paxuclus requested a review from daniellienert March 7, 2021 16:18
@@ -3,6 +3,8 @@

namespace MOC\Varnish\Aspects;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Namespace needs to be adjusted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@daniellienert the package is still in the MOC\Varnish namespace in 4.0. I'd like to release a 4.1.1 with this bugfix since Neos 5.X is still supported.

This patch will need to be upmerged to master later for Neos 7.X compatibility.

What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You are so right ... ☺️

@daniellienert daniellienert merged commit 6e8bd8f into Flowpack:4.0 Aug 17, 2021
@dlubitz
Copy link
Copy Markdown
Contributor

dlubitz commented Jan 8, 2025

@paxuclus @daniellienert I try to prepare a Neos 9 compatible version of this package and stubled over this PR. While I tried to remove the deprecated Flowpack\Varnish\Service\ContentCacheFlusherService I noticed yet another usage of this service:

public function purgeCacheAction(Node $node): void
{
$service = new ContentCacheFlusherService();
$service->flushForNode($node);
$this->view->assign('value', true);
}

Not sure if your change made this feature obsolete or if it is still needed to invalidate individual nodes in the backend module?

image

I know it's quite a long time, but maybe you remember what was the plan here? Thank you

@paxuclus
Copy link
Copy Markdown
Collaborator Author

paxuclus commented Jan 8, 2025

Hi @dlubitz,

I'm pretty sure the ContentCacheFlusherService is still needed for the backend module to work as expected. I guess the deprecation annotation is invalid then.

@dlubitz
Copy link
Copy Markdown
Contributor

dlubitz commented Jan 8, 2025

I see, thank you for the quick reply.

@daniellienert
Copy link
Copy Markdown
Contributor

Had a quick look, I also found the usage which @dlubitz mentions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Changing an Asset does not flush pages that reference it

3 participants