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: Extract LowLevelFrontendInterface and implement PSR-6 and PSR-16 as cache frontends to be used via Caches.yaml #3239

Open
wants to merge 1 commit into
base: 9.0
Choose a base branch
from

Conversation

mficzel
Copy link
Member

@mficzel mficzel commented Nov 18, 2023

While Neos had methods to create PSR caches those were seldomly used as they worked quite differently to other caches that were configured via Caches.yaml.

This change introduces implementations of PSR-6 Cache and PSR-16 SimpleCache that can be used as normal flow cache-frontends. To make this possible the low level functionality of object instantiation and garbage collection and flushing was extracted into a LowLevelFrontendInterface while the FrontenInterface on to defines the interaction with the caches.

Vendor_Package_SimpleCache:
  frontend: Neos\Cache\Frontend\Psr\SimpleCache\SimpleCacheFrontend
  backend: Neos\Cache\Backend\FileBackend

Vendor_Package_Cache:
  frontend: Neos\Cache\Frontend\Psr\Cache\CacheItemPoolFrontend
  backend: Neos\Cache\Backend\FileBackend

Tho help to ensuring which cache interface you are working with the method getCache of the CacheManager now got an optional second argument that allows to specify the classname of the expected frontend implementation.

The old methods to create PSR caches are deprecated and marked for removal with Neos Flow 10.

Upgrade instructions

If Caches are configured via Caches.yaml and injected via Objects.yaml no changes are needed.

If the CacheManager is used to obtain certain caches it is important to note that the the method getCache now returns LowLevelFrontendInterface instead of FrontendInterface. It will still return the configured cache implementation from Caches.yaml but you may have to add the expected className as second argument to convince your static analysis.

Review instructions

A secondary non-official goal of this pr is allowing to implement other cache interfaces like the following as flow cache and be managed by the cache manager.

    public function resolve(string $identifier, \Closure $updateClosure, array $tags = [], ?int $lifeTime = null, ?int $gracePeriod = null, ?int $lockPeriod = null): mixed

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

@github-actions github-actions bot added the 9.0 label Nov 18, 2023
@mficzel mficzel changed the title !!! FEATURE: Extract LowLevelFrontendInterface and implement PSR-6 and PSR-16 cache frontends !!! FEATURE: Extract LowLevelFrontendInterface and implement PSR-6 and PSR-16 as cache frontends to be used via Caches.yaml Nov 18, 2023
@mficzel mficzel force-pushed the feature/extractLowLevelCacheFrontend branch from 6109af0 to ec13193 Compare November 18, 2023 20:50
@kitsunet
Copy link
Member

Beautiful thanks for preparing this!

@mficzel mficzel force-pushed the feature/extractLowLevelCacheFrontend branch 2 times, most recently from c8d8e1f to e309627 Compare November 18, 2023 21:25
@mficzel mficzel marked this pull request as ready for review November 18, 2023 22:19
@mficzel
Copy link
Member Author

mficzel commented Nov 18, 2023

@kitsunet @kdambekalns after stumbling over the limitations of our cache interface again i dug out the old idea of having a more basic cache interface we recntly discussed.

Alongside making the use of PSR Caches as easy as any other cache this would allow us to implement caches like https://github.com/sitegeist/Sitegeist.StaleMate directly as a cache frontend which i consider quite valuable.

@mficzel mficzel force-pushed the feature/extractLowLevelCacheFrontend branch from e309627 to 0f91ed0 Compare November 22, 2023 08:29
…PSR-16 cache frontends

While Neos had methods to create PSR caches those were seldomly used as they worked quite differently to other caches that were configured via `Caches.yaml`.

This change introduces implementations of PSR-6 Cache and PSR-16 SimpleCache that can be used as normal flow cache-frontends. To make this possible the low level functionality of object instantiation and garbage collection and flushing was extracted into a `LowLevelFrontendInterface` while the `FrontenInterface` on to defines the interaction with the caches.

The old methods for creating psr caches are deprecated and marked for removal with Neos Flow 10.
@mficzel mficzel force-pushed the feature/extractLowLevelCacheFrontend branch from 0f91ed0 to d590baa Compare November 22, 2023 08:34
Copy link
Member

@kitsunet kitsunet left a comment

Choose a reason for hiding this comment

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

Thanks for taking care, would love to get this into 9.0, as written, I think composing our Frontend with a lowlevel one inside might be nicer in the long run :)

But I probably wouldn't block it if that takes too long.

@kitsunet
Copy link
Member

Had a look as promised, my suggestion about composition was 🤦 I just misread what was going on. I don#t think it's necessary, but overall I find the code a bit convoluted now, I wrote up a whole lot here about conceptually splitting the interfaces but however I turn it, there is no beautiful solution with different needs pulling and pushing at this code. Maybe the LowLevelInterface could have a different name ala CacheMaintenanceInterface to put it apart from the FrontendInterface, but that's the best I can come up with...

Copy link
Member

@kitsunet kitsunet left a comment

Choose a reason for hiding this comment

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

Giving this a preliminary +1 even though comment above applies.

@mficzel
Copy link
Member Author

mficzel commented Feb 16, 2024

@kitsunet for the psr-frontends it would work to have them as a wrapper around the current frontends. For flexibility i would like to be able to implement frontends with a totally different interface like the closures the symfony cache uses.

For that (+ async cache population) i tried the wrapper approach here https://github.com/sitegeist/Sitegeist.StaleMate but this is even more convoluted. That is why i ended here.

I did not know how to describe this properly in the pr description. Will add another line to the review instructions.

@mhsdesign
Copy link
Member

If you like I would also try to look in depth into this and give you a full review. But that may take some time and I might have some questions :P (i don't know what to make out of the lowlevel naming). Maybe we can discuss this at the sprint or next Friday?
I hope you're okay with delaying it a bit longer;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Under Review 👀
Development

Successfully merging this pull request may close these issues.

None yet

3 participants