-
Notifications
You must be signed in to change notification settings - Fork 11.3k
[5.7] Honor PSR-16 SimpleCache interface #26700
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
Conversation
Methods `put`, `set`, `putMany`, `setMultiple`, and `forever` now return boolean values. Fixes issue laravel#26674
2029051
to
90c9569
Compare
You've changed the docblocks from btw, pozdrav iz Trikodera 😄 |
What are we accomplishing for users (and what percentage of users) for using PSR-16 at all? Do you have a particular use case in mind? |
@taylorotwell I'm using some framework-agnostic packages which depend on PSR-16 on a couple of Laravel projects and having the ability to inject the Illuminate Cache Repository there is a great thing. Since we already claim to implement that interface I don't see the harm in actually fixing this so that we are compliant with it, especially since it's not a huge change. |
@taylorotwell In my case, I'm working on a project where we use the jeremykendall/php-domain-parser package. It has a dependency for any PSR-16 compatible cache library. As reported in issue #26674, caching with Laravel's cache component doesn't work because the package strictly expects the return values for certain methods to be boolean (as defined in the PSR-16 contract). What I had to do is create a wrapper around the cache component and basically fix the return values to make it work, which kind of defeats the purpose of having a cache component that tries to be PSR-16 complaint. With this PR we're aiming to make the cache component fully compliant with PSR-16, so there are no surprises, and workarounds have to be made for packages that expect strict return types. I can't say exactly what percentage of users will benefit from this, but I can say that the adoption of PSR-16 is growing. When |
@X-Coder264 Good catch, thanks. Fixed it with last changes. I figured since ps, pozdrav s ex3k strane 👋 😃 |
Thanks for the info. |
Do we actually implement the PSR-16 interfaces anywhere? |
I think the only PR regarding psr-16 was #20194. Laravel contract extends the PSR-16 |
There's a lot of changes here to the extent I would probably prefer this be sent to master branch. |
Cache repository methods
set
andsetMultiple
, defined in the SimpleCache interface do not return any value, but they should according to PSR-16. This PR aims to solve this and fix issue #26674.Cache stores Apc, Array, Database, File, Memcached, Null, and Redis are affected. Their respective unit tests were updated to check for return values.