Add limiter leases and harden lock refresh behavior#428
Merged
Conversation
Record the agreed design for exposing concurrency limiter leases, hardening refreshable locks, unifying limiter timeout exceptions, and adding Redis Cluster hash-tag safety. The plan captures the source audit, consensus decisions, implementation order, test coverage, and documentation updates so the following commits have a clear design reference.
Introduce the Lease and RefreshableLease capability contracts for public concurrency limiter slots, plus a single Contracts\Limiters\LimiterTimeoutException shared by cache funnels, redis funnels, and redis throttles. Remove the older backend-specific timeout exception classes so generic limiter code can catch one exception regardless of which limiter implementation is used.
Add explicit Hypervel\Support\now imports to cache and Sanctum files that call the helper unqualified. This keeps standalone package installs from depending on the global foundation helper while preserving the same Date-backed behavior.
Add isLocked() to the lock contract and provide base refresh() and getRemainingLifetime() fallbacks on the abstract cache lock. Update RefreshableLock documentation to describe the unified refresh semantics: refresh(null) re-applies the acquisition duration as interpreted by the driver, while explicit non-positive TTLs are rejected.
Make RedisLock refresh(null) verify current ownership for permanent locks and reject explicit non-positive refresh TTLs with the shared message. Tighten acquisition checks to the actual Redis wrapper return types, using set(...) === true and setnx(...) === 1, and keep phpredis serializer-sensitive refresh arguments split so TTL conversion stays numeric. Extend Redis lock coverage across unit and integration tests, including serializer/compression refresh paths and ownership-sensitive permanent refresh behavior.
Treat expired array lock records as absent at the owner lookup boundary so ownership, release, refresh, and isLocked() all agree. Simplify release through the shared ownership check, keep the refresh read-modify-write non-yielding for WorkerArrayStore, and add boundary coverage for remaining lifetime and expired records.
Filter expired rows when reading database lock owners and require an unexpired matching owner in the refresh UPDATE itself. Rework database refresh(null) to re-extend the same default timeout used for non-positive acquisition durations, since TTL-less database rows need a crash-safety expiration. Return the nullable database connection name honestly and cover expired ownership, refresh, lifetime, and isLocked() behavior in the database lock tests.
Add owner-checked FileStore refresh and lifetime inspection using file locks, make FileLock implement RefreshableLock, and route acquisition through the narrowed FileStore accessor. Use a named permanent timestamp sentinel, return null for missing, expired, or permanent lifetimes, and preserve the file driver's atomicity with exclusive refresh writes and shared lifetime reads. Also fix FileStore contention handling to catch the filesystem LockTimeoutException actually thrown by LockableFile, and keep permission checks strict.
Make NoLock report isLocked() as false while keeping refresh(null) aligned with the shared ownership-check semantics for permanent locks. Add tests for refresh, lifetime, and locked-state behavior so generic lock code sees the same API shape even when the backend deliberately performs no locking.
Add RedisConnection::hasHashTag() so cluster-sensitive code can detect existing Redis Cluster hash tags before wrapping keys. Cover normal, empty, missing, and already-tagged key forms to match the Redis Cluster rule that only non-empty brace contents count as a hash tag.
Add the Redis concurrency lease implementation and make Redis::funnel() expose acquire() through the limiter and builder. Route then() and block() through the same lease acquisition path, release callback-scoped leases in finally blocks, keep fire-and-forget block() behavior, and use millisecond deadlines with a fakeable time source. Hash-tag Redis funnel slot keys on cluster connections so the multi-key Lua acquire script stays on one Redis Cluster slot, while leaving pre-tagged names unchanged. Cover lease acquire, release, refresh, lifetime, timeout, callback exception handling, fire-and-forget behavior, and cluster key tagging in unit and integration tests.
Move Redis throttle timeout handling onto the shared Contracts\Limiters\LimiterTimeoutException and update the builder/tests to match. Use the same millisecond-precise, fakeable deadline loop style as the funnel limiters so block() does not oversleep the configured timeout.
Add cache-backed concurrency lease implementations and expose acquire() from Cache::funnel() through both the limiter and builder. Return RefreshableLease instances when the claimed slot is backed by a RefreshableLock, preserve non-refreshable lock providers through the base Lease contract, and keep then()/block() as callback-scoped sugar over the same acquisition path. Update the Redis cache fast path to construct locks with the acquisition TTL instead of restoring them as permanent locks, so refresh(null) re-extends the lease as intended. Hash-tag Redis-backed cache funnel slots for cluster connections, keep prefix round-tripping intact, and cover lease lifecycle, TTL reclaim, refresh, serializer, prefix, and cluster behavior across unit and integration tests.
Add cluster-safe Redis queue storage key resolution that wraps untagged queue names in Redis Cluster hash tags while preserving logical queue names in payloads and public getQueue() output. Cache the cluster-connection check on the queue instance and apply the storage-key resolver to queue, delayed, reserved, and notify keys used by multi-key queue operations. Cover clustered, non-clustered, and pre-tagged queue names so Redis Queue Lua operations stay same-slot on cluster connections without changing non-cluster behavior.
Move the previously skipped database cache store and database lock integration tests out of Todo into the active Integration/Database suite. Drop the stale skip wrappers, update namespaces, and keep the tests aligned with the current database lock/store APIs, including owner checks, connection names, refresh behavior, and expired-row handling.
Update cache docs with funnel lease acquisition, refreshable lease heartbeats, leaked-lease TTL reclaim, isLocked(), and the refined lock refresh semantics across drivers. Add Redis funnel and throttle documentation, including the new lease API and automatic Redis Cluster hash-tagging for funnel and queue storage keys. Record Hypervel's intentional Laravel differences in the cache and redis package READMEs, including unified limiter timeout exceptions, refresh capability interfaces, explicit non-positive refresh rejection, and lease acquisition APIs.
|
Warning Review limit reached
Next review available in: 10 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds limiter lease contracts and implementations, reworks cache and Redis concurrency limiters to return leases, hardens lock refresh and lifetime behavior across drivers, adds Redis Cluster hash-tag safety for limiter and queue keys, and updates related docs and tests. ChangesLimiter leases, lock hardening, and cluster safety
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR turns the Redis and cache concurrency limiters into first-class primitives by adding an explicit
Confidence Score: 5/5PR is safe to merge. All lock and lease paths have been verified for correctness, ownership semantics are consistent across drivers, and the cluster tagging logic handles edge cases correctly. The core timeout comparison is intentional and correct. Lock ownership checks are symmetric across acquire/release/refresh paths on every driver. The Lua return-value contract for acquireConcurrencySlot is preserved: prefixed KEYS go in, unprefixed name-plus-index comes back, and store->lock() re-applies the prefix exactly once. No logic regressions were found. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "Harden throwable-path test assertions" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/cache/src/ArrayLock.php (1)
98-110: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider avoiding double record fetch in
refresh().
refresh()fetches the lock record on line 108, then callsisOwnedByCurrentProcess()on line 110, which fetches the record again viagetCurrentOwner(). The already-fetched$recordcould be used for both the existence and ownership checks, avoiding the redundant store lookup.♻️ Proposed refactor
$record = $this->store->getLockRecord($this->name); - if ($record === null || ! $this->isOwnedByCurrentProcess()) { + if ($record === null) { + return false; + } + + $expiresAt = $record['expiresAt']; + if ($expiresAt !== null && ! $expiresAt->isFuture()) { + return false; + } + + if ($record['owner'] !== $this->owner) { return false; }This inlines the expiry and ownership checks from
getCurrentOwner()using the already-fetched$record, eliminating the second store lookup. The trade-off is duplicating the expiry-check logic, so the current approach is also acceptable if you prefer DRY over the minor efficiency gain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cache/src/ArrayLock.php` around lines 98 - 110, In refresh() of ArrayLock, avoid fetching the lock record twice: the existing $record from getLockRecord() should be reused for both the existence check and the ownership check instead of calling isOwnedByCurrentProcess(), which triggers another store lookup via getCurrentOwner(). Update the refresh flow to derive ownership from the already-fetched record, keeping the same expiry behavior while removing the redundant access.tests/Cache/CacheDatabaseLockTest.php (1)
141-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
: voidreturn types to new test methods.As per coding guidelines, test methods should use
: voidreturn types. The four new test methods (testExpiredLockIsNotLockedOrOwned,testRefreshOnDefaultTimeoutLockReappliesDefaultTimeout,testRefreshReturnsFalseWhenExpired,testGetConnectionNameCanReturnNull) are missing this return type.♻️ Proposed fix
- public function testExpiredLockIsNotLockedOrOwned() + public function testExpiredLockIsNotLockedOrOwned(): void- public function testRefreshOnDefaultTimeoutLockReappliesDefaultTimeout() + public function testRefreshOnDefaultTimeoutLockReappliesDefaultTimeout(): void- public function testRefreshReturnsFalseWhenExpired() + public function testRefreshReturnsFalseWhenExpired(): void- public function testGetConnectionNameCanReturnNull() + public function testGetConnectionNameCanReturnNull(): voidAlso applies to: 236-236, 254-254, 334-334
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Cache/CacheDatabaseLockTest.php` at line 141, Add explicit : void return types to the new test methods in CacheDatabaseLockTest, including testExpiredLockIsNotLockedOrOwned, testRefreshOnDefaultTimeoutLockReappliesDefaultTimeout, testRefreshReturnsFalseWhenExpired, and testGetConnectionNameCanReturnNull. Update each method signature in the test class so it follows the project’s test method convention without changing the test logic.Source: Coding guidelines
tests/Cache/CacheArrayStoreTest.php (1)
229-245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
: voidreturn types to new test methods.As per coding guidelines, test methods should use
: voidreturn types. All four new test methods (testExpiredLockIsNotLockedOrOwned,testExpiredLockCannotBeReleasedByOldOwner,testRefreshReturnsFalseWhenExpired,testRefreshOnPermanentLockReturnsFalseWhenNotOwned) are missing this return type.♻️ Proposed fix
- public function testExpiredLockIsNotLockedOrOwned() + public function testExpiredLockIsNotLockedOrOwned(): void- public function testExpiredLockCannotBeReleasedByOldOwner() + public function testExpiredLockCannotBeReleasedByOldOwner(): void- public function testRefreshReturnsFalseWhenExpired() + public function testRefreshReturnsFalseWhenExpired(): void- public function testRefreshOnPermanentLockReturnsFalseWhenNotOwned() + public function testRefreshOnPermanentLockReturnsFalseWhenNotOwned(): voidAlso applies to: 289-300, 521-532, 544-552
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Cache/CacheArrayStoreTest.php` around lines 229 - 245, Add the missing : void return type to each new test method in CacheArrayStoreTest, including testExpiredLockIsNotLockedOrOwned, testExpiredLockCannotBeReleasedByOldOwner, testRefreshReturnsFalseWhenExpired, and testRefreshOnPermanentLockReturnsFalseWhenNotOwned. Update the method signatures only, keeping the existing test logic unchanged, and ensure all newly added PHPUnit test methods in this class follow the project’s void-return convention.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/boost/docs/cache.md`:
- Around line 868-897: The lease handling example in the cache docs should stop
assuming refresh always succeeds. Update the loop around
Cache::funnel(...)->acquire() so the code checks the result of
RefreshableLease::refresh() before proceeding to the next chunk, and if it
returns false, stop processing or reacquire the lease before continuing. Keep
the guidance tied to RefreshableLease, refresh(), and the explicit release() in
the try/finally flow.
In `@src/boost/docs/redis.md`:
- Around line 402-458: The Redis funnel lease example ignores the boolean result
from Lease::refresh(), so the loop can continue after the slot is no longer
owned. Update the sample around Redis::funnel(), acquire(), and refresh() to
check the return value before calling runStep() for the next iteration, and
either break/abort or reacquire the lease when refresh() fails.
In `@tests/Cache/CacheNoLockTest.php`:
- Around line 78-84: The new test method testIsLockedAlwaysReturnsFalse should
follow the test coding guidelines by declaring a void return type. Update the
method signature in CacheNoLockTest so the test method explicitly returns void,
matching the style used across tests/**/*.php.
In `@tests/Cache/CacheRedisLockTest.php`:
- Around line 144-191: Add explicit : void return types to the four new test
methods in CacheRedisLockTest:
testRefreshOnPermanentLockReturnsTrueWhenStillOwned,
testRefreshOnPermanentLockReturnsFalseWhenNotOwned,
testIsLockedReturnsTrueWhenLockExists, and
testIsLockedReturnsFalseWhenLockDoesNotExist. Keep the existing assertions and
mock expectations unchanged; only update the method signatures to match the test
convention used in tests/**/*.php.
In `@tests/Integration/Cache/RedisCacheFunnelTest.php`:
- Line 92: The new test method testFastPathLeaseRefreshReappliesReleaseAfterTtl
should declare a void return type to match the test coding guidelines. Update
the method signature in RedisCacheFunnelTest so this test explicitly returns
void, consistent with the other test methods in the class.
---
Nitpick comments:
In `@src/cache/src/ArrayLock.php`:
- Around line 98-110: In refresh() of ArrayLock, avoid fetching the lock record
twice: the existing $record from getLockRecord() should be reused for both the
existence check and the ownership check instead of calling
isOwnedByCurrentProcess(), which triggers another store lookup via
getCurrentOwner(). Update the refresh flow to derive ownership from the
already-fetched record, keeping the same expiry behavior while removing the
redundant access.
In `@tests/Cache/CacheArrayStoreTest.php`:
- Around line 229-245: Add the missing : void return type to each new test
method in CacheArrayStoreTest, including testExpiredLockIsNotLockedOrOwned,
testExpiredLockCannotBeReleasedByOldOwner, testRefreshReturnsFalseWhenExpired,
and testRefreshOnPermanentLockReturnsFalseWhenNotOwned. Update the method
signatures only, keeping the existing test logic unchanged, and ensure all newly
added PHPUnit test methods in this class follow the project’s void-return
convention.
In `@tests/Cache/CacheDatabaseLockTest.php`:
- Line 141: Add explicit : void return types to the new test methods in
CacheDatabaseLockTest, including testExpiredLockIsNotLockedOrOwned,
testRefreshOnDefaultTimeoutLockReappliesDefaultTimeout,
testRefreshReturnsFalseWhenExpired, and testGetConnectionNameCanReturnNull.
Update each method signature in the test class so it follows the project’s test
method convention without changing the test logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e78576ab-f9b4-4c40-a629-6923e34f6404
📒 Files selected for processing (66)
docs/plans/2026-07-09-limiter-leases-and-lock-hardening.mdsrc/boost/docs/cache.mdsrc/boost/docs/queues.mdsrc/boost/docs/redis.mdsrc/cache/README.mdsrc/cache/src/AbstractArrayStore.phpsrc/cache/src/ArrayLock.phpsrc/cache/src/DatabaseLock.phpsrc/cache/src/FileLock.phpsrc/cache/src/FileStore.phpsrc/cache/src/Limiters/ConcurrencyLease.phpsrc/cache/src/Limiters/ConcurrencyLimiter.phpsrc/cache/src/Limiters/ConcurrencyLimiterBuilder.phpsrc/cache/src/Limiters/RedisConcurrencyLimiter.phpsrc/cache/src/Limiters/RefreshableConcurrencyLease.phpsrc/cache/src/Lock.phpsrc/cache/src/NoLock.phpsrc/cache/src/Redis/Operations/AllTag/Add.phpsrc/cache/src/Redis/Operations/AllTag/AddEntry.phpsrc/cache/src/Redis/Operations/AllTag/FlushStale.phpsrc/cache/src/Redis/Operations/AllTag/Put.phpsrc/cache/src/Redis/Operations/AllTag/PutMany.phpsrc/cache/src/Redis/Operations/AllTag/Remember.phpsrc/cache/src/Redis/Operations/AllTag/Touch.phpsrc/cache/src/RedisLock.phpsrc/contracts/src/Cache/Lock.phpsrc/contracts/src/Cache/RefreshableLock.phpsrc/contracts/src/Limiters/Lease.phpsrc/contracts/src/Limiters/LimiterTimeoutException.phpsrc/contracts/src/Limiters/RefreshableLease.phpsrc/contracts/src/Redis/LimiterTimeoutException.phpsrc/queue/src/RedisQueue.phpsrc/redis/README.mdsrc/redis/src/Limiters/ConcurrencyLease.phpsrc/redis/src/Limiters/ConcurrencyLimiter.phpsrc/redis/src/Limiters/ConcurrencyLimiterBuilder.phpsrc/redis/src/Limiters/DurationLimiter.phpsrc/redis/src/Limiters/DurationLimiterBuilder.phpsrc/redis/src/RedisConnection.phpsrc/sanctum/src/Console/Commands/PruneExpired.phpsrc/sanctum/src/PersonalAccessToken.phpsrc/sanctum/src/SanctumGuard.phptests/Cache/CacheArrayStoreTest.phptests/Cache/CacheDatabaseLockTest.phptests/Cache/CacheFileStoreTest.phptests/Cache/CacheNoLockTest.phptests/Cache/CacheRedisLockTest.phptests/Cache/ConcurrencyLimiterTest.phptests/Cache/RedisLockTest.phptests/Integration/Cache/CacheFunnelTestCase.phptests/Integration/Cache/FileCacheLockTest.phptests/Integration/Cache/PhpRedisCacheFunnelTest.phptests/Integration/Cache/PhpRedisCacheLockTest.phptests/Integration/Cache/RedisCacheFunnelTest.phptests/Integration/Cache/RedisCacheLockTest.phptests/Integration/Database/DatabaseCacheStoreTest.phptests/Integration/Database/DatabaseLockTest.phptests/Integration/Database/Todo/DatabaseLockTest.phptests/Integration/Redis/ConcurrencyLimiterIntegrationTest.phptests/Integration/Redis/DurationLimiterIntegrationTest.phptests/Queue/QueueRedisQueueTest.phptests/Redis/ConcurrencyLimiterBuilderTest.phptests/Redis/ConcurrencyLimiterTest.phptests/Redis/DurationLimiterBuilderTest.phptests/Redis/DurationLimiterTest.phptests/Redis/RedisConnectionTest.php
💤 Files with no reviewable changes (2)
- src/contracts/src/Redis/LimiterTimeoutException.php
- tests/Integration/Database/Todo/DatabaseLockTest.php
Move the custom Redis connection helper into InteractsWithRedis so integration tests can create serializer, prefix, and pool-size variants through one shared test concern. Keep the more general max-connections option from RedisProxyIntegrationTest and make createRedisConnectionWithPrefix delegate to the same helper, removing the duplicate private helper from the proxy test.
Run permanent concurrency-lease ownership checks through RedisProxy::withoutSerializationOrCompression so Lua-written raw owner ids compare correctly on serializer-enabled phpredis connections. Add an integration regression with PhpRedis::SERIALIZER_PHP and document the remaining Redis transformed-command return-type audit in docs/todo.md.
Capture the current timestamp once in DatabaseLock::getRemainingLifetime, query only unexpired rows, and calculate the remaining lifetime from that same timestamp. Update the cache lock tests to assert the expiration predicate and model expired locks as absent from the live-row query, matching the source behavior.
Register cleanup immediately after defineCacheRoutes writes a testbench route file so failures during route:cache or cache-file verification cannot leave route files behind for the worker lifetime. Skip cleanup during the intentional reloadApplication cycle, re-register it in the finally block after reload teardown resets callbacks, and always delete tracked route files even when cache-path cleanup cannot use a Hypervel application instance. Add regression coverage for the failure path and a RouteCacheCommandTest precondition that reports leaked testbench route filenames directly if another leak ever appears.
Delete the upstream placeholder testNativeValidationVsRfcCompliant method because its data set is empty and PHPUnit flags it as risky for performing zero assertions. Leave a REMOVED marker at the upstream-matching location so future Laravel test syncs do not mechanically re-port the no-op method.
Update the refreshable lock and funnel lease examples so copied code checks the boolean result from refresh(). The examples now throw when a lock or lease has been lost instead of continuing work outside the limiter. This matches the RefreshableLock and RefreshableLease contracts, where false means the owner no longer holds the resource or the resource expired. The snippets keep the framework documentation's class-code style, including explicit RuntimeException imports.
Finish the return-type cleanup across the test files touched by the limiter lease and lock hardening work. Every PHPUnit test method in the eight affected files now declares : void, matching the components testing standard instead of only fixing the individual methods flagged by CodeRabbit. Also replace the RedisProxy fully qualified return type in DurationLimiterIntegrationTest with an import while touching that file, keeping imports consistent with the components guide. Focused test runs passed for each edited file, and composer fix passed before this commit.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/Testbench/Concerns/DefineCacheRoutesTest.php`:
- Around line 129-149: The exception assertion in
testDefineCacheRoutesCleansRouteFileWhenRouteCacheFails is being swallowed
because the fail() call sits inside the catch(Throwable) block. Move the
“expected failure” check out of the catch by using expectException() or a
separate flag/after-try assertion so defineCacheRoutes() must actually throw
before the cleanup assertions run; keep the cleanup verification in the same
test method using the existing testbenchRouteFiles and
callBeforeApplicationDestroyedCallbacks symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 792f89f3-ba16-43fd-a902-f1dca654264f
📒 Files selected for processing (19)
docs/todo.mdsrc/boost/docs/cache.mdsrc/boost/docs/redis.mdsrc/cache/src/DatabaseLock.phpsrc/foundation/src/Testing/Concerns/InteractsWithRedis.phpsrc/redis/src/Limiters/ConcurrencyLease.phpsrc/testbench/src/Concerns/HandlesRoutes.phptests/Cache/CacheArrayStoreTest.phptests/Cache/CacheDatabaseLockTest.phptests/Cache/CacheNoLockTest.phptests/Cache/CacheRedisLockTest.phptests/Integration/Cache/PhpRedisCacheFunnelTest.phptests/Integration/Cache/RedisCacheFunnelTest.phptests/Integration/Foundation/Console/RouteCacheCommandTest.phptests/Integration/Redis/ConcurrencyLimiterIntegrationTest.phptests/Integration/Redis/DurationLimiterIntegrationTest.phptests/Integration/Redis/RedisProxyIntegrationTest.phptests/Testbench/Concerns/DefineCacheRoutesTest.phptests/Validation/ValidationEmailRuleTest.php
💤 Files with no reviewable changes (1)
- tests/Integration/Redis/RedisProxyIntegrationTest.php
🚧 Files skipped from review as they are similar to previous changes (10)
- src/redis/src/Limiters/ConcurrencyLease.php
- tests/Cache/CacheNoLockTest.php
- src/boost/docs/redis.md
- tests/Integration/Cache/PhpRedisCacheFunnelTest.php
- src/boost/docs/cache.md
- tests/Cache/CacheRedisLockTest.php
- tests/Integration/Cache/RedisCacheFunnelTest.php
- tests/Integration/Redis/ConcurrencyLimiterIntegrationTest.php
- tests/Cache/CacheDatabaseLockTest.php
- tests/Cache/CacheArrayStoreTest.php
Capture expected throwables outside catch blocks in tests that need to keep asserting cleanup or job state after the exception path. This prevents PHPUnit assertion failures from being swallowed by broad Throwable catches, so the tests now fail if the exercised code stops throwing while still preserving the post-exception assertions. Also add explicit void return types to the touched queue middleware tests to match the repository test style.
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
1 participant
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.
Summary
This PR turns the concurrency limiter into a more full-featured primitive: acquire a slot, hold it for as long as the caller owns the work, and release it explicitly when done.
The existing callback API remains. It is now sugar over the public lease path. This gives long-lived workers, connection pools, streamed jobs, and multi-step operations a first-class way to hold bounded capacity without wrapping everything in one closure.
While doing that, this PR also hardens the lock refresh surface. Hypervel already had refreshable locks, and Laravel has since added similar APIs. The merged result keeps Laravel's
refresh()naming where it fits, but preserves Hypervel's stronger semantics: explicit non-positive refresh TTLs throw, permanent refreshes verify ownership, and database locks re-extend their crash-safety timeout rather than pretending a TTL-less backend can hold a truly permanent lock safely.Finally, this ports Redis Cluster hash-tag safety to the multi-key Redis paths touched by this work: Redis funnels, Redis-backed cache funnels, and Redis queues.
For more detailes, see: docs/plans/2026-07-09-limiter-leases-and-lock-hardening.md
What Changed
Limiter leases
Hypervel\Contracts\Limiters\Lease.Hypervel\Contracts\Limiters\RefreshableLease.Hypervel\Contracts\Limiters\LimiterTimeoutException.Redis::funnel(...)->acquire().Cache::funnel(...)->acquire().then()andblock()behavior, but routed callback-scoped execution through the same lease acquisition path.refresh(null)actually re-extends that TTL.Lock refresh hardening
isLocked()to the cache lock contract.refresh()andgetRemainingLifetime()fallbacks to the abstract cache lock.RefreshableLocksemantics and docs.Redis Cluster safety
RedisConnection::hasHashTag().Package hardening
Hypervel\Support\nowimports where standalone packages were relying on the global foundation helper.Todoand into the active database integration suite.Design Notes
The important API choice is that leases are capability-based, just like refreshable locks. Code that only needs release works against
Lease. Code that can heartbeat a held slot checks forRefreshableLease. There is noisRefreshable()flag and no method that advertises support only to throw later.The Redis paths stay efficient. Funnel acquisition still uses the existing Lua slot claim. Refresh and release are owner-checked atomic Lua operations. Cluster tagging is computed once per limiter or queue instance, not inside the hot script path.
The non-Redis cache funnel path still uses the cache lock provider abstraction. That means each driver gets the best behavior its backend can support: Redis uses atomic Lua, database uses guarded SQL updates, file uses filesystem locks, and array/worker-array stays in-memory and non-yielding for coroutine safety.
The database driver is intentionally different from native-TTL drivers. A database row cannot expire itself. Non-positive acquisition durations therefore map to the default timeout, and
refresh(null)re-applies that same safety timeout. That is the clean behavior for a TTL-less backend because it keeps crash recovery intact.Testing
Validated with
composer fix, which ran:php-cs-fixerphpstantest:parallelSummary by CodeRabbit
Summary by CodeRabbit
New Features
acquire()-based slot acquisition with explicit lease release/refresh.isLocked()support for locks.Bug Fixes
Documentation
Tests