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

ENH Migrate away from deprecated Symfony cache classes #10187

Closed

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented Dec 23, 2021

Issue silverstripe/silverstripe-fulltextsearch#302

Symfony *Cache classes (e.g. PhpFilesCache) were deprecated and their replacements are Symfony *Adapter classes (e.g. PhpFilesAdapter)

These *Adapter classes have support for 2 different cache styles

  • psr6 using Psr\SimpleCache\CacheInterface - this is the API that Silverstripe classes expect e.g. DefaultCacheFactory
  • "Cache contracts" which seems like the symfony preferred approach. However it's not compatible with all the existing Silverstripe classes

The solution I've proposed here is to create an adapter class SymfonyAdapterToPsr6Cache that implements the PSR6 CacheInterface and wraps the Symfony AbstractAdatper class

If we were to update Silverstripe classes to use "Cache contracts", due to semver we'd need to write entirely new classes and deprecate the old ones as there is plenty of API that expects a CacheInterface

Note: there are 2 different 'CacheInterface` with different APIs

  • Psr\SimpleCache\CacheInterface - The existing Psr6 interface
  • Symfony\Contracts\Cache\CacheInterface - The new Symfony Cache Contracts interface

The error message on the issues says use "Symfony\Component\Cache\Adapter\PhpFilesAdapter" and type-hint for "Symfony\Contracts\Cache\CacheInterface" instead. -- this is recommending the Cache Contracts interface with is different to what we're currently using.

TODO:

  • Test, I haven't tested anything
  • See if symfony 3 is still supported here, I did this with symfony/cache 4.4.34. May need to drop support for symfony 3
  • Implement $ttl param
  • Throw exceptions like the docblocks say I should - note the docblocks were copy pasted form CacheInterface, I'll probably delete most of them

@GuySartorelli
Copy link
Member

GuySartorelli commented Jul 7, 2022

Note: The documentation portion should not be merged here. I have started the process of migrating docs to a new repository so the docs will need to be removed from this PR and a new PR raised in the new repo instead.

@emteknetnz
Copy link
Member Author

Superseded by #10478

@emteknetnz emteknetnz closed this Sep 6, 2022
@GuySartorelli GuySartorelli deleted the pulls/4/symfonyadapter branch September 6, 2022 21:56
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.

2 participants