TASK: Content cache adjustments

[nezaniel]

This adds an interface for other content caches than Neos' Fusion content cache.

Also flushes caches for deleted nodes, not just their parents, in case they or their types were used in CacheTags elsewhere

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

[kitsunet]

Isn't the point of this to be not internal in order for people to use this? I guess fine for a start but this is supposed to become API right?

[bwaidelich]

Sorry for the -1, but I'm really skeptical about this (as mentioned on Slack).

IMO we suffer from Interface-itis in the "old" Neos code base. I like interfaces but they should have a very clear role and in this case it is not evident to me.
i.e. What is an implementation of flushNodeAggregate supposed to do? (the name of this method is rather unfortunate IMO – not a new issue of this PR, but extracting it to an interface makes it more prominent)
Why are assets part of the deal and not other entities?

My main concern is, that this introduces another global point of replacement by using Objects.yaml – but changing the ContentCacheFlusherInterface implementation could break implementations that rely on the current content cache behavior..

Long story short:

I think it's a good idea to allow for 3rd parties to hook into the "content cache flushing" but IMO that should not be in the form of an adapter pattern but rather some kind of extension/hook/callback.
Probably the easiest solution would be some option for the GraphProjectorCatchUpHookForCacheFlushing that allows for registering custom code to be executed on a per CR basis.

Again, sorry for stepping on the brakes once again.. I'm happy to discuss this and come up with a solution together

[kitsunet]

Good points, maybe we need an idea of projected use cases? I can see a point of getting informed when content caches are flushed and a case of being informed when the projections changed, but both could be done differntly, but overwriting the cache flushing in itself? Thinking about it I agree this might be the "wrong" extension point.

I removed the -1 because I don't want to block this, but I would still suggest to change the implementation slightly and I'm happy to help with that

[nezaniel]

@bwaidelich yes, let's do this together... I have quite some ideas for improvements, including CacheTag VOs, Unit tests etc.

[kitsunet]

LGTM, but yes, an internal interface won't help much, so either way, if we want to merge this, I am fine, but I could also see room for something nicer.

[mhsdesign]

I think a interface like this to replace cache flushing is also bad practice, if we were to make this an extension point we would need some kind of chain where we can hook multiple flushers, as the ContentCacheFlusher is probably necessary.

I know that having an interface is more fancy but still, objects yaml exists and the ContentCacheFlusher is not final. So for your experimental usecase i would suggest to go down the road of extending the flusher and adding functionality and then for every place it is used use Objects.yaml to use your implementation as constructor argument.

That would make in my head much more sense at this stage. We as core devs have a little perk of being able to easily add what we want...

[mhsdesign]

still this part seems to be a bugfix that we want to preserve?

[dlubitz]

@nezaniel Is the interface still relevant?

flushes caches for deleted nodes, not just their parents

This is fixed by #5175

[nezaniel]

I think this can be closed, I've opted for AOP to solve this in the ComponentView for now.