[Lock] Expose an expiringDate and isExpired method in Lock #22543
Conversation
09c696f to
5990f0a
Compare
src/Symfony/Component/Lock/Key.php
Outdated
| */ | ||
| public function reduceExpiringDate(\DateTimeImmutable $expiringDate) | ||
| { | ||
| if (null === $this->expiringDate || $this->expiringDate > $expiringDate) { |
There was a problem hiding this comment.
Why not throw an exception when $this->expiringDate <= $expiringDate?
There was a problem hiding this comment.
I would do that in a method named setExpiringDate. But I didn't want to backport such logic in this class. It purpose it to move foward the expiration date regardless the previous value.
There was a problem hiding this comment.
The method name is not clear to me. reduce means that it's going to expire earlier than before, right. But you say that the purpose is to "move forward".
There was a problem hiding this comment.
When several store are combined, they can define different expiration date, the goal is to take the shorter one. cf if (.. && $this->expiringDate > $expiringDate) { set value}
Indeed, reducing a date does not really make sens. What's about reduceLifeLength or advanceExpiringDate?
There was a problem hiding this comment.
It's common to use lifetime or ttl when talking about storage (session, cache, authorizations, ...). Could we use the same wording here? (reduceLifetime could do the job then)
There was a problem hiding this comment.
Finally replaced reduceExpiringDate(\Datetime $newExpiringDate) by reduceLifetime(float $ttl)
d318ee4 to
d5ae790
Compare
d5ae790 to
7d69adc
Compare
|
What's the status of this PR? I'd like to merge/close all PRs about the lock component ASAP to avoid having to revert the component for 3.4. |
|
@fabpot PR waiting for review/approval. As far as I know, nothing to add from my side. |
|
Thank you @jderusse. |
…Lock (jderusse) This PR was squashed before being merged into the 3.4 branch (closes #22543). Discussion ---------- [Lock] Expose an expiringDate and isExpired method in Lock | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22452 | License | MIT | Doc PR | NA This PR store the expiration date of the lock in the key and exposes public method to let the user know the state of the Lock expiration. Commits ------- 2794308 [Lock] Expose an expiringDate and isExpired method in Lock
| } | ||
|
|
||
| /** | ||
| * @return \DateTimeImmutable |
| { | ||
| $newExpiringDate = \DateTimeImmutable::createFromFormat('U.u', (string) (microtime(true) + $ttl)); | ||
|
|
||
| if (null === $this->expiringDate || $newExpiringDate < $this->expiringDate) { |
There was a problem hiding this comment.
I am not sure if it really is expected behaviour that the method silently ignores any argument if it is no smaller than the existing expiration date.
There was a problem hiding this comment.
IMO it's not an issue if a store ask for a TTL greater than the current one.
This PR store the expiration date of the lock in the key and exposes public method to let the user know the state of the Lock expiration.