[13.x] Add flushLocks() support to Cache stores#58907
Open
amirhshokri wants to merge 21 commits intolaravel:masterfrom
Open
[13.x] Add flushLocks() support to Cache stores#58907amirhshokri wants to merge 21 commits intolaravel:masterfrom
flushLocks() support to Cache stores#58907amirhshokri wants to merge 21 commits intolaravel:masterfrom
Conversation
Contributor
Author
|
Test failure is unrelated: 1) Illuminate\Tests\Http\HttpClientTest::testTheTransferStatsAreCustomizable
Illuminate\Http\Client\ConnectionException: cURL error 60: SSL certificate OpenSSL verify result: unable to get local issuer certificate (20) (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://example.com/ |
Member
|
I personally think this should only work if the lock connection / storage location is different than the default location. |
Contributor
Author
|
Thanks for the review @taylorotwell Good point — I agree with the isolation requirement. I'll refine the implementation to enforce that. |
Contributor
Author
|
Test failure is unrelated: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR introduces the ability to flush all locks from cache stores that support it, by adding a
FlushableLockinterface and exposingflushLocks()through theRepositoryandCachefacade.Motivation
There is recurring community demand for this feature (#36927) — particularly in local development scenarios where locks can become orphaned after a crashed request or a failed queue job, forcing developers to either wait for expiry or wipe the entire cache with
Cache::flush().Changes
New interface —
Illuminate\Contracts\Cache\FlushableLock:Stores that support flushing locks implement this interface alongside the existing
LockProviderinterface. This keeps the design non-breaking — stores that don't support it simply don't implement it.Implemented on:
ArrayStore,FileStore,DatabaseStore,RedisStoreRepository:
Added two methods:
flushLocks(): bool— delegates to the store if it implementsFlushableLock, otherwise throws aBadMethodCallExceptionconsistent with howtags()behaves on non-taggable stores.supportsFlushingLocks(): bool— allows callers to check support before callingflushLocks().Cachefacade:Added docblock annotations for both new methods:
@method static bool flushLocks() @method static bool supportsFlushingLocks()Usage
Design decisions
RedisStorealready extendsTaggableStore, so an abstract class approach would conflict with PHP's single inheritance. A standalone interface composes cleanly with the existing hierarchy.instanceofovermethod_exists— unlikesupportsTags()which predates a formal interface,supportsFlushingLocks()usesinstanceof FlushableLocksince the interface is explicitly defined.flushLocks()only removes lock keys, leaving regular cache entries untouched.Add
flushLocks()support to Cache storesWhat this PR does
This PR introduces the ability to flush all locks from cache stores that support it, by adding a
FlushableLockinterface and exposingflushLocks()through theRepositoryandCachefacade.Motivation
There is recurring community demand for this feature ([link to discussions]) — particularly in local development scenarios where locks can become orphaned after a crashed request or a failed queue job, forcing developers to either wait for expiry or wipe the entire cache with
Cache::flush().Changes
New interface —
Illuminate\Contracts\Cache\FlushableLockStores that support flushing locks implement this interface alongside the existing
LockProviderinterface. This keeps the design non-breaking — stores that don't support it simply don't implement it.Implemented on:
ArrayStore,FileStore,DatabaseStore,RedisStoreRepositoryAdded two methods:
flushLocks(): bool— delegates to the store if it implementsFlushableLock, otherwise throws aBadMethodCallExceptionconsistent with howtags()behaves on non-taggable stores.supportsFlushingLocks(): bool— allows callers to check support before callingflushLocks().CachefacadeAdded docblock annotations for both new methods:
@method static bool flushLocks() @method static bool supportsFlushingLocks()Usage
Design decisions
RedisStorealready extendsTaggableStore, so an abstract class approach would conflict with PHP's single inheritance. A standalone interface composes cleanly with the existing hierarchy.instanceofovermethod_exists— unlikesupportsTags()which predates a formal interface,supportsFlushingLocks()usesinstanceof FlushableLocksince the interface is explicitly defined.flushLocks()only removes lock keys, leaving regular cache entries untouched.Update
Following Taylor's feedback that flushing locks should only be allowed when the lock store is provably isolated from the cache store, a
hasSeparateLockStore(): boolmethod has been added to theFlushableLockinterface. Each store defines what isolation means in its own context:lockConnectionis a different connection name thanconnectionlockTableis a different table thantabletrue, locks live in$locks, cache in$storagelockDirectoryis explicitly set and differs fromdirectoryflushLocks()on each store now guards against non-isolated stores by throwing aRuntimeExceptionbefore proceeding.