-
Notifications
You must be signed in to change notification settings - Fork 10.9k
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
[11.x] Add remaining
method to get the expiration timestamp of a cache key
#52017
base: 11.x
Are you sure you want to change the base?
Conversation
…atical operations on it if need be
… is only a draft PR
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.
You might need to leave out of this PR all changes made to the Contract, and add those to a different PR targeting master
instead of 11.x
since any changes made to a Contract are considered breaking changes.
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.
Yes, I have restored the Contract to its original state to reduce overcomplication. I originally just wanted this to be a change to FileStore
for testing purposes and suggestions/discussion and got a little carried away, haha.
What is wrong with two cache keys, one containing the data, the other the ttl? |
Maybe it's better to return \DateIntrval object? We will be able to convert it to what format we want |
I like it. But I feel like this is only necessary on protocols such as Memcached and Redis. Storing 2 keys for FileStore and DatabaseStore seems unnecessary to me as we can easily retrieve the expiration with these protocols for example, and would also make the Retrieving an item from the cache could also have an optional parameter which enables the developer to also retrieve the remaining TTL for the given item in an array, such as: [
'data' => ...,
'ttl' => 1720099976
] So we could have 2 options to get the remaining TTL: [$data, $ttl] = cache('my_data', ttl: true); $ttl = cache()->remaining('my_data'); This is just quick drafting off the top of my head, so will test some implementations. |
remaining
method to get the expiration timestamp of a cache keyremaining
method to get the expiration timestamp of a cache key
@Markshall please rebase this PR and mark it as ready if you need a new review. |
Can we please fix tests on the local machine before pushing it? |
Was having trouble replicating the tests on my local machine. I don't have memcached PHP extension installed so it's skipping the tests and trying to install it is proving impossible. Windows is saying the extension isn't found in my ext dir even though it clearly is there. |
Synopsis
I'm running a large(ish) query and caching the data for a set period of time, and wanting to display to the user how long is remaining until the data is refreshed in the cache. To test this, I manually modified the
FileStore
andDatabaseStore
classes within my project'svendor/
directory to add a method that will return the expiry time as a human readable string (or optionally the UNIX timestamp).This is only a draft PR because protocols such as Memcached do not provide an efficient API to retrieve the expiry time of a cached item, unless you store the expiry time alongside the actual data being cached, but that would likely be a breaking change within the Laravel codebase if this was to happen. I have also not provided methods for other stores such as
RedisStore
orDynamoDbStore
etc. as I was quickly testing solutions before drafting this PR.I am open to seeing suggestions as to what should happen if this were to be a feature within the Cache system in Laravel. For example, as mentioned, protocols such as Memcached do not provide a way to retrieve the expiry time, so should the method exist, but return null? Or should it not exist at all?
Creating the method, but returning null, would mean you don't get an error when calling via the cache helper function:
Not having the method would of course mean you receive an error.
Proposed usage
Get the expiry time as a human readable string
Get the UNIX timestamp