-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[5.8] Fix PSR-16 TTL conformity #27217
[5.8] Fix PSR-16 TTL conformity #27217
Conversation
These changes to the Cache Repository interface and implementation will make our cache implementation compliant with the PSR-16 standard's TTL requirements. It contains some significant changes in how we handle and store cache values. The main change that was done is that now a TTL with a value of NULL will be considerd an attempt to store the item forever. In addition to this, any value with a TTL equal or lower than zero will be considerd an attempt to remove the item from the cache. The `put`, `putMany` and `add` methods were updated to reflect these changes. Before these changes a request like `Cache::put('foo', 'bar')` wouldn't do anything since a NULL TTL meant that the call was simply ignored. Now any request without a specified TTL will have the default TTL of NULL and thus the request is considered to attempting to store the item forever. For the `putMany` call a NULL TTL now means that every single item will be stored forever individually. As there isn't a `putManyForever` equivalent on the Repository or the PSR interface, there was no way to tell a cache store to store the given items in one go. For the `add` call, the behavior to ignore the call when a zero or less TTL was given is kept but when a NULL TTL is now passed, it's correctly passed to the cache store. This will ignore a custom `add` method on the cache store as any NULL TTL simply could be considered to re-store the item indefinitely. No changes were made to the cache stores themselves. Only the Repository was modified. This way, the cache stores keep their current behavior as theyaren't implementations of the PSR Simple Cache interface. All in all these changes will now make us conform with the PSR specs much better.
Thanks to the changes of the cache repository we can simplify this call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a quick look, this looks good 👍 I like that null
is checked both in repository and in each store implementation :)
Hey @asgrim, thanks for reviewing. Please note that the changes from the other PR (that wasn't merged) aren't in this one. |
Added integration tests for Memcached + tagged cache items. |
Resend (and a better implementation) for #27166
These changes to the Cache Repository interface and implementation will make our cache implementation compliant with the PSR-16 standard's TTL requirements. It contains some significant changes in how we handle and store cache values.
The main change that was done is that now a TTL with a value of NULL will be considerd an attempt to store the item forever. In addition to this, any value with a TTL equal or lower than zero will be considerd an attempt to remove the item from the cache. The
put
,putMany
andadd
methods were updated to reflect these changes.Before these changes a request like
Cache::put('foo', 'bar')
wouldn't do anything since a NULL TTL meant that the call was simply ignored. Now any request without a specified TTL will have the default TTL of NULL and thus the request is considered to attempting to store the item forever.For the
putMany
call a NULL TTL now means that every single item will be stored forever individually. As there isn't aputManyForever
equivalent on the Repository or the PSR interface, there was no way to tell a cache store to store the given items in one go.For the
add
call, the behavior to ignore the call when a zero or less TTL was given is kept but when a NULL TTL is now passed, it's correctly passed to theput
method. This will ignore a customadd
method on the cache store as any NULL TTL simply could be considered to re-store the item indefinitely.No changes were made to the cache stores themselves. Only the Repository was modified. This way, the cache stores keep their current behavior as they aren't implementations of the PSR Simple Cache interface.
All in all these changes will now make us conform with the PSR spec much better.
For reference: https://www.php-fig.org/psr/psr-16/
Fixes #27160
PS. As a proof of concept I was able to simplify a call in the Filesystem component.