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

CacheItem for PSR-6 needs to start expiring after calling CacheItemInterface#expiresAfter #199

Merged
merged 2 commits into from Aug 10, 2022

Conversation

boesing
Copy link
Member

@boesing boesing commented Jul 10, 2022

Q A
Documentation no
Bugfix kinda
BC Break maybe
New Feature no

Description

This might introduce some kind of BC break.
Prior this change, cache items did not start to expire after calling CacheItemInterface#expiresAfter.
Therefore, CacheItemInterface instances only started to expire after calling one of the following methods:

  • CacheItemPoolInterface#save
  • CacheItemPoolInterface#commit

With one of the integration tests we are using for each of our cache adapters, we found out that this is not how cache items should work.

Since symfony/cache and doctrine/cache do use these integration tests as well, I'd expect that this is a common use-case and thus want to align with these assumptions.


Do we want to introduce this slight BC break? I have a few use-cases in mind where one might want to synchronize expiration times of multiple cache items but I would assume that this is not fully breaking a project since whenever one is working with time, microseconds might be different and thus CacheItemPoolInterface#getItem might return items which are not hit even tho they should be there from a TTL PoV.

… `CacheItemInterface#expiresAfter`

More details can be found in laminas/laminas-cache-storage-adapter-test#31

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing added Bug Something isn't working Enhancement labels Jul 10, 2022
@boesing boesing added this to the 3.2.0 milestone Jul 10, 2022
@boesing
Copy link
Member Author

boesing commented Jul 10, 2022

Since this change introduces some kind of BC break, I targeted 3.2.x (minor) rather than 3.1.x (patch).

composer.json Outdated Show resolved Hide resolved
src/Psr/CacheItemPool/CacheItem.php Show resolved Hide resolved
…torage-adapter-test` to get rid of unstable `cache/integration-tests`

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@Ocramius Ocramius self-assigned this Aug 10, 2022
@Ocramius
Copy link
Member

🚢 the BC concern is very minimal here 👍

@Ocramius Ocramius merged commit 8c5f5f4 into laminas:3.2.x Aug 10, 2022
@boesing boesing deleted the bugfix/psr-6-expires-at branch August 10, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants