feat(cache): harden lifecycle safety and complete Laravel parity - #455
Merged
Conversation
Format cache expiration timestamps into the ten-byte header reserved by the file-store format, including dates beyond the current Unix timestamp width. Cover put, add, and refresh behavior so payload offsets remain stable and long-lived entries cannot corrupt their serialized values.
Add the current Laravel-compatible filesystem-disk cache store with checked reads and writes, fixed-width expiration metadata, pruning, and path-prefix support. Exercise successful operations and native filesystem failure contracts through a reusable in-memory filesystem fixture.
Register the storage driver, declare the nullable limiter and secure serialization settings, and pass serialization policy into Swoole stores. Replace ambiguous container array access and dead call-site defaults with canonical typed resolution while preserving manager aliases, test spies, and the framework's merged configuration behavior.
Add closure TTLs, rememberWithWarmth, and incomplete-class handling across the repository contract, implementation, manager-facing facade metadata, and test cleanup. Keep nullable values and raw-readable wrappers intact, namespace flexible-cache ownership by the resolved item key, and release Redis-backed reads before user callbacks execute.
Make callback-scoped locks attempt release on every acquired path without allowing a release error to replace the callback's original failure. Cover successful callbacks, callback failures, release failures, and their combined precedence so lock ownership remains deterministic.
Apply exhaustive release and earliest-failure precedence to both concurrency-limiter callback APIs after a lease has been acquired. Retain existing false-result behavior and verify the callback/release failure matrix without adding logging, retry, or lifecycle machinery.
Compare a newly created counter with the requested increment amount rather than the hard-coded value one before installing its decay window. Add regression coverage for custom increments so their first write cannot accidentally remain permanent.
Require every failover backend to support lock cleanup before starting, then flush all stores exhaustively while preserving false results and the earliest exception. Report separate-lock cleanup consistently and cover unsupported capabilities, partial failures, and successful cleanup across every configured store.
Expose truthful lock-flush behavior from memoized stores when their backing store supports it and make the null store a real no-op lock provider. Cover capability propagation and cleanup results so failover preflight cannot accept vacuous or incomplete implementations.
Compensate completed stack writes in reverse order and continue forced forget and flush operations after false results or exceptions. Preserve the earliest failure, aggregate unsuccessful cleanup truthfully, and retain blocked object payloads during cache promotion.
Represent only forever values and permanent framework metadata with pseudo-permanent Swoole expirations while retaining real TTLs and verifying permanent lock ownership. Apply the configured unserialization policy to public Swoole cache values without restricting counters or internal metadata, with integration coverage for cached authentication models.
Retain every native timer ID, roll back earlier registrations when later startup fails, and clear the exact owned timers during teardown. Exercise partial creation, callback failure reporting, and deterministic cleanup without introducing a separate registry or retry path.
Emit the established repository event sequence for custom all-mode and any-mode reads, writes, failures, deletes, and flushes with truthful store names and tags. Keep atomic any-mode additions event-free, preserve plain-key behavior, and ensure incomplete cached objects are handled once at the owning read boundary.
Remove the six hidden remember operation implementations that held one pooled Redis connection across arbitrary user callbacks and child-coroutine work. Use ordinary short read and write checkouts on misses, retain explicit withPinnedConnection for callers that intentionally need one lease, and delete the obsolete operation caches, exception, tests, and constructor dependency.
Inject the Redis factory already owned by RedisStore and borrow connections only for each scan page or deletion chunk instead of retaining a lease across lazy iteration. Follow PhpRedis SafeScan cursor semantics through empty nonterminal pages, avoid redundant terminal requests, bound per-tag deduplication, and remove lease-held sleeps from pruning.
Teach both clustered and Lua-backed any-mode add paths to represent a null TTL as truly permanent rather than assigning an arbitrary one-year expiration. Keep positive TTL behavior atomic, align permanent tag metadata with the shared maximum-expiry contract, and verify the result through unit and real Redis coverage.
Read required cache and Redis command configuration from the merged framework config instead of repeating fallback values at each call site. Retain nullable detection where absence is meaningful and cover the doctor command's declared configuration contract.
Document the storage driver, closure TTLs, remember warmth reporting, incomplete-class handling, serialization policy, rate-limiter store selection, and tagged event behavior. Explain the supported Swoole, failover, stack, and Redis-specific boundaries in the same task-first style as the surrounding framework documentation.
Cross-reference the cache serialization allowlist from authentication and Sanctum guidance so cached framework models remain an explicit application choice. Clarify that native PhpRedis serializers deserialize before Cache can enforce its class policy, allowing applications to configure the relevant connection boundary deliberately.
Exercise every lazy RedisStore cache through its real warming accessor and verify that each boot-time configuration mutator clears the complete cache set. Derive the authoritative set from nullable private cache properties so newly introduced slots fail loudly until their invalidation behavior is covered. Keep setLockConnection outside the matrix because lock connections are resolved per call and are never captured by a cached instance.
|
Warning Review limit reached
Next review available in: 15 minutes 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 (3)
📝 WalkthroughWalkthroughThis PR adds filesystem-backed caching, serializable-class controls, warm-cache retrieval, lock-flush support, Redis tagged-cache updates, improved scan and timer cleanup behavior, revised Redis dependency injection, and extensive unit, integration, and documentation coverage. ChangesCache contracts and storage
Redis tagged caching
Documentation
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 substantially expands and hardens the Cache package.
Confidence Score: 4/5The PR appears safe to merge, with the previously reported nested incomplete-class diagnostic limitation still outstanding. The registered unserializable-class callback is still reached only when the cache value itself is an incomplete class; nested incomplete classes remain undetected, but no blocking failure remains. Files Needing Attention: src/cache/src/Repository.php
|
| Filename | Overview |
|---|---|
| src/cache/src/Repository.php | Adds warm-cache reporting, closure TTL support, incomplete-class handling, and namespace-aware flexible-cache coordination. |
| src/cache/src/StorageStore.php | Introduces the filesystem-abstraction-backed cache store and its serialization and expiration behavior. |
| src/cache/src/RedisStore.php | Refactors Redis checkout ownership and invalidation of lazily derived store configuration. |
| src/cache/src/SwooleStore.php | Updates serialization boundaries and preserves truly permanent Swoole-backed values. |
| src/cache/src/StackStore.php | Makes layered-store operations and cleanup exhaustive while retaining the earliest failure. |
| src/cache/src/FailoverStore.php | Revises failover execution and cleanup behavior to preserve primary failures. |
| src/cache/src/Redis/AllTaggedCache.php | Updates all-mode tagged-cache operations, event behavior, and Redis resource ownership. |
| src/cache/src/Redis/AnyTaggedCache.php | Updates any-mode tagged-cache operations, permanent additions, and event behavior. |
| src/cache/src/RateLimiter.php | Corrects custom-amount limiter expiration installation. |
| src/cache/src/SwooleTimer.php | Makes timer startup transactional and failure-safe. |
Reviews (2): Last reviewed commit: "docs(audit): complete cache lifecycle wo..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/cache/src/Redis/AllTaggedCache.php (1)
51-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEvent emission is now asymmetric between
add()'s two branches.The null-TTL path delegates to
forever()and therefore emitsWritingKey+KeyWritten/KeyWriteFailed, while the TTL path (Lines 62-67) still writes without emitting anything. Listeners will see writes for someadd()calls only. Consider emitting the same events around the TTL branch for parity.🤖 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/Redis/AllTaggedCache.php` around lines 51 - 76, Update AllTaggedCache::add so the TTL branch emits the same WritingKey and KeyWritten/KeyWriteFailed events as the null-TTL branch delegated through forever(). Wrap the allTagOps()->add()->execute call with the existing cache event flow, preserving its current key, value, TTL, tags, and boolean result behavior.tests/Cache/Redis/AllTaggedCacheTest.php (1)
313-342: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHardcoded
nullscan cursor is inconsistent with this file's own convention.Both mocks assert
zScan(..., null, '*', 1000), but this same file'stestFlushDispatchesTheRepositoryEventsWithStoreNameAndTags(line 1283) correctly usesPhpRedis::initialScanCursor()for the identical assertion.PhpRedis::initialScanCursor()returns null for phpredis versions >= 6.1.0 and 0 otherwise. Hardcodingnulldefeats that abstraction and will fail these two tests on any phpredis install where the initial cursor is0.🔧 Proposed fix
+use Hypervel\Redis\PhpRedis; ... $connection->shouldReceive('zScan') ->once() - ->with('prefix:_all:tag:people:entries', null, '*', 1000) + ->with('prefix:_all:tag:people:entries', PhpRedis::initialScanCursor(), '*', 1000) ->andReturnUsing(function ($key, &$cursor) {(apply to both
testFlushandtestClearFlushesTaggedItems;PhpRedisis already imported at line 22)Also applies to: 347-372
🤖 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/Redis/AllTaggedCacheTest.php` around lines 313 - 342, Update the zScan expectations in testFlush and testClearFlushesTaggedItems to use PhpRedis::initialScanCursor() instead of hardcoded null, matching the file’s existing convention while preserving the remaining arguments and assertions.
🧹 Nitpick comments (3)
src/cache/src/Redis/Operations/AnyTag/Forever.php (1)
195-195: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: pass
StoreContext::MAX_EXPIRYas an ARGV instead of duplicating the literal in three Lua scripts. The comments now document the coupling, but the value is still hardcoded in three places and a change to the constant would silently diverge.
src/cache/src/Redis/Operations/AnyTag/Forever.php#L195-L195: appendStoreContext::MAX_EXPIRYto$argsinexecuteUsingLua()and read it as an ARGV instoreForeverWithTagsScript().src/cache/src/Redis/Operations/AnyTag/Decrement.php#L175-L175: same change inexecuteUsingLua()/decrementWithTagsScript()(note the tag ARGV offset shifts).src/cache/src/Redis/Operations/AnyTag/Increment.php#L175-L175: same change inexecuteUsingLua()/incrementWithTagsScript().🤖 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/Redis/Operations/AnyTag/Forever.php` at line 195, Pass StoreContext::MAX_EXPIRY through the Lua arguments instead of duplicating the literal: update executeUsingLua() and storeForeverWithTagsScript() in src/cache/src/Redis/Operations/AnyTag/Forever.php#L195-L195, executeUsingLua() and decrementWithTagsScript() in src/cache/src/Redis/Operations/AnyTag/Decrement.php#L175-L175 (adjust tag ARGV offsets), and executeUsingLua() and incrementWithTagsScript() in src/cache/src/Redis/Operations/AnyTag/Increment.php#L175-L175 to read the appended expiry value from ARGV.src/cache/src/Limiters/ConcurrencyLimiter.php (1)
84-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate release-preserving-exception logic across two files.
Both
ConcurrencyLimiter::block()andConcurrencyLimiterBuilder::then()implement the identical "preserve callback failure, swallow release failure" try/catch/rethrow pattern independently. Extracting a shared helper (e.g. a small trait method or static helper taking theLease/Lockand callback) would keep both call sites in sync if this semantics ever needs adjustment (e.g., adding logging for the swallowed release exception).
src/cache/src/Limiters/ConcurrencyLimiter.php#L84-L107: extract the try/catch/rethrow block fromblock()into a shared helper.src/cache/src/Limiters/ConcurrencyLimiterBuilder.php#L109-L136: reuse the same helper fromthen()instead of duplicating the block.🤖 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/Limiters/ConcurrencyLimiter.php` around lines 84 - 107, Extract the shared callback-execution and lease/lock-release exception handling from ConcurrencyLimiter::block() into a reusable helper, then update ConcurrencyLimiterBuilder::then() to call that helper instead of duplicating the try/catch/rethrow logic. Apply the shared change at src/cache/src/Limiters/ConcurrencyLimiter.php lines 84-107 and reuse it at src/cache/src/Limiters/ConcurrencyLimiterBuilder.php lines 109-136, preserving callback failures as primary while swallowing release failures.src/cache/src/StackTaggedCache.php (1)
51-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated write+event pattern from
put()/forever().Both methods repeat the same shape: emit
WritingKey, perform the tagged write, then branch intoKeyWritten/KeyWriteFailedwith nearly identical event construction. A small protected helper (e.g.writeTaggedRecord(string $key, array $record, mixed $value, ?int $seconds = null): bool) that performs the write and emits the three events would remove the duplication and keep both call sites in sync if the event contract changes again.♻️ Sketch of the extraction
+ protected function writeTaggedRecord(string $key, array $record, mixed $value, ?int $seconds = null): bool + { + $this->event(WritingKey::class, fn (): WritingKey => new WritingKey($this->getName(), $key, NullSentinel::unwrap($value), $seconds)); + + $result = $this->store->putRecordTagged($this->tags->getNames(), $key, $record); + + $this->event( + $result ? KeyWritten::class : KeyWriteFailed::class, + fn () => $result + ? new KeyWritten($this->getName(), $key, NullSentinel::unwrap($value), $seconds) + : new KeyWriteFailed($this->getName(), $key, NullSentinel::unwrap($value), $seconds) + ); + + return $result; + }Also applies to: 97-106, 111-136
🤖 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/StackTaggedCache.php` around lines 51 - 92, Extract the duplicated tagged-write and event flow from put() and forever() into a protected helper such as writeTaggedRecord(). Have it emit WritingKey, call putRecordTagged(), then emit KeyWritten or KeyWriteFailed and return the result; update both methods to delegate to this helper while preserving their existing TTL and record behavior.
🤖 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/cache/src/Redis/Operations/AllTag/GetEntries.php`:
- Around line 31-34: Move the $seen initialization outside the foreach ($tagIds
as $tagId) loop in the all-mode entry retrieval flow, while retaining it across
all tag scans. Ensure duplicate keys shared by multiple tag hashes are yielded
only once, without changing the existing per-tag cursor scanning behavior.
In `@src/cache/src/StorageStore.php`:
- Around line 45-55: Update StorageStore::__construct to reject an empty or
whitespace-only $directory after trimming it, throwing an appropriate framework
or PHP exception before assigning the normalized path. Preserve valid normalized
directory values and ensure the related flush() path cannot operate with an
empty storage directory.
- Around line 79-85: Update StorageStore::add to use an atomic backend
conditional-create primitive instead of calling get() before put(). Ensure the
operation succeeds only when the key does not already exist, including when its
stored value is null, and preserves the existing TTL and boolean contract across
local and remote-disk backends.
---
Outside diff comments:
In `@src/cache/src/Redis/AllTaggedCache.php`:
- Around line 51-76: Update AllTaggedCache::add so the TTL branch emits the same
WritingKey and KeyWritten/KeyWriteFailed events as the null-TTL branch delegated
through forever(). Wrap the allTagOps()->add()->execute call with the existing
cache event flow, preserving its current key, value, TTL, tags, and boolean
result behavior.
In `@tests/Cache/Redis/AllTaggedCacheTest.php`:
- Around line 313-342: Update the zScan expectations in testFlush and
testClearFlushesTaggedItems to use PhpRedis::initialScanCursor() instead of
hardcoded null, matching the file’s existing convention while preserving the
remaining arguments and assertions.
---
Nitpick comments:
In `@src/cache/src/Limiters/ConcurrencyLimiter.php`:
- Around line 84-107: Extract the shared callback-execution and
lease/lock-release exception handling from ConcurrencyLimiter::block() into a
reusable helper, then update ConcurrencyLimiterBuilder::then() to call that
helper instead of duplicating the try/catch/rethrow logic. Apply the shared
change at src/cache/src/Limiters/ConcurrencyLimiter.php lines 84-107 and reuse
it at src/cache/src/Limiters/ConcurrencyLimiterBuilder.php lines 109-136,
preserving callback failures as primary while swallowing release failures.
In `@src/cache/src/Redis/Operations/AnyTag/Forever.php`:
- Line 195: Pass StoreContext::MAX_EXPIRY through the Lua arguments instead of
duplicating the literal: update executeUsingLua() and
storeForeverWithTagsScript() in
src/cache/src/Redis/Operations/AnyTag/Forever.php#L195-L195, executeUsingLua()
and decrementWithTagsScript() in
src/cache/src/Redis/Operations/AnyTag/Decrement.php#L175-L175 (adjust tag ARGV
offsets), and executeUsingLua() and incrementWithTagsScript() in
src/cache/src/Redis/Operations/AnyTag/Increment.php#L175-L175 to read the
appended expiry value from ARGV.
In `@src/cache/src/StackTaggedCache.php`:
- Around line 51-92: Extract the duplicated tagged-write and event flow from
put() and forever() into a protected helper such as writeTaggedRecord(). Have it
emit WritingKey, call putRecordTagged(), then emit KeyWritten or KeyWriteFailed
and return the result; update both methods to delegate to this helper while
preserving their existing TTL and record behavior.
🪄 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: 37ddaafa-0c97-4796-93ce-e49d57354cf9
📒 Files selected for processing (99)
src/auth/README.mdsrc/boost/docs/authentication.mdsrc/boost/docs/cache.mdsrc/boost/docs/redis.mdsrc/boost/docs/sanctum.mdsrc/cache/src/AnyModeTaggedCache.phpsrc/cache/src/CacheManager.phpsrc/cache/src/CacheServiceProvider.phpsrc/cache/src/FailoverStore.phpsrc/cache/src/FileStore.phpsrc/cache/src/Limiters/ConcurrencyLimiter.phpsrc/cache/src/Limiters/ConcurrencyLimiterBuilder.phpsrc/cache/src/Listeners/BaseListener.phpsrc/cache/src/Listeners/CreateSwooleTimers.phpsrc/cache/src/Lock.phpsrc/cache/src/MemoizedStore.phpsrc/cache/src/NullStore.phpsrc/cache/src/RateLimiter.phpsrc/cache/src/Redis/AllTaggedCache.phpsrc/cache/src/Redis/AnyTaggedCache.phpsrc/cache/src/Redis/Console/BenchmarkCommand.phpsrc/cache/src/Redis/Console/Concerns/DetectsRedisStore.phpsrc/cache/src/Redis/Console/DoctorCommand.phpsrc/cache/src/Redis/Exceptions/RedisCacheException.phpsrc/cache/src/Redis/Operations/AllTag/Flush.phpsrc/cache/src/Redis/Operations/AllTag/GetEntries.phpsrc/cache/src/Redis/Operations/AllTag/Prune.phpsrc/cache/src/Redis/Operations/AllTag/Remember.phpsrc/cache/src/Redis/Operations/AllTag/RememberForever.phpsrc/cache/src/Redis/Operations/AllTagOperations.phpsrc/cache/src/Redis/Operations/AnyTag/Add.phpsrc/cache/src/Redis/Operations/AnyTag/Decrement.phpsrc/cache/src/Redis/Operations/AnyTag/Forever.phpsrc/cache/src/Redis/Operations/AnyTag/GetTaggedKeys.phpsrc/cache/src/Redis/Operations/AnyTag/Increment.phpsrc/cache/src/Redis/Operations/AnyTag/Prune.phpsrc/cache/src/Redis/Operations/AnyTag/Remember.phpsrc/cache/src/Redis/Operations/AnyTag/RememberForever.phpsrc/cache/src/Redis/Operations/AnyTagOperations.phpsrc/cache/src/Redis/Operations/Remember.phpsrc/cache/src/Redis/Operations/RememberForever.phpsrc/cache/src/Redis/Support/StoreContext.phpsrc/cache/src/RedisStore.phpsrc/cache/src/Repository.phpsrc/cache/src/StackStore.phpsrc/cache/src/StackTaggedCache.phpsrc/cache/src/StorageStore.phpsrc/cache/src/SwooleLock.phpsrc/cache/src/SwooleStore.phpsrc/cache/src/SwooleTimer.phpsrc/contracts/src/Cache/Repository.phpsrc/foundation/config/cache.phpsrc/support/src/Facades/Cache.phptests/Cache/CacheFailoverStoreTest.phptests/Cache/CacheFileStoreTest.phptests/Cache/CacheLockTest.phptests/Cache/CacheManagerTest.phptests/Cache/CacheMemoizedStoreTest.phptests/Cache/CacheNullStoreTest.phptests/Cache/CacheRateLimiterTest.phptests/Cache/CacheRedisStoreTest.phptests/Cache/CacheRepositoryTest.phptests/Cache/CacheSpyMemoTest.phptests/Cache/CacheStackStoreTagsTest.phptests/Cache/CacheStackStoreTest.phptests/Cache/CacheStorageStoreTest.phptests/Cache/CacheSwooleStoreIntervalTest.phptests/Cache/CacheSwooleStoreTest.phptests/Cache/ConcurrencyLimiterTest.phptests/Cache/CreateSwooleTimersTest.phptests/Cache/Fixtures/ArrayFilesystem.phptests/Cache/Redis/AllTaggedCacheTest.phptests/Cache/Redis/AnyTaggedCacheTest.phptests/Cache/Redis/Console/DoctorCommandTest.phptests/Cache/Redis/Operations/AllTag/FlushTest.phptests/Cache/Redis/Operations/AllTag/GetEntriesTest.phptests/Cache/Redis/Operations/AllTag/PruneTest.phptests/Cache/Redis/Operations/AllTag/RememberForeverTest.phptests/Cache/Redis/Operations/AllTag/RememberTest.phptests/Cache/Redis/Operations/AllTagOperationsTest.phptests/Cache/Redis/Operations/AnyTag/AddTest.phptests/Cache/Redis/Operations/AnyTag/DecrementTest.phptests/Cache/Redis/Operations/AnyTag/ForeverTest.phptests/Cache/Redis/Operations/AnyTag/GetTaggedKeysTest.phptests/Cache/Redis/Operations/AnyTag/IncrementTest.phptests/Cache/Redis/Operations/AnyTag/PruneTest.phptests/Cache/Redis/Operations/AnyTag/RememberForeverTest.phptests/Cache/Redis/Operations/AnyTag/RememberTest.phptests/Cache/Redis/Operations/AnyTagOperationsTest.phptests/Cache/Redis/Operations/RememberForeverTest.phptests/Cache/Redis/Operations/RememberTest.phptests/Cache/Redis/RedisCacheTestCase.phptests/Cache/Redis/RedisStoreTest.phptests/Cache/Redis/Support/StoreContextTest.phptests/Integration/Auth/EloquentUserProviderCacheTagsTest.phptests/Integration/Cache/Redis/BasicOperationsIntegrationTest.phptests/Integration/Cache/Redis/ClusterFallbackIntegrationTest.phptests/Integration/Cache/Redis/ConnectionPinningIntegrationTest.phptests/Testbench/DefaultConfigurationTest.php
💤 Files with no reviewable changes (15)
- src/cache/src/Redis/Operations/RememberForever.php
- src/cache/src/Redis/Operations/AllTag/RememberForever.php
- src/cache/src/Redis/Operations/AllTag/Remember.php
- src/cache/src/Redis/Operations/AnyTag/Remember.php
- src/cache/src/Redis/Exceptions/RedisCacheException.php
- tests/Cache/Redis/Operations/RememberTest.php
- src/cache/src/Redis/Operations/Remember.php
- tests/Cache/Redis/Operations/RememberForeverTest.php
- src/cache/src/Redis/Operations/AnyTag/RememberForever.php
- tests/Cache/Redis/Operations/AnyTag/RememberTest.php
- tests/Cache/Redis/Operations/AllTag/RememberTest.php
- src/cache/src/Redis/Operations/AllTagOperations.php
- tests/Cache/Redis/Operations/AllTag/RememberForeverTest.php
- tests/Cache/Redis/Operations/AnyTag/RememberForeverTest.php
- src/cache/src/Redis/Operations/AnyTagOperations.php
Collaborator
Author
|
@greptile-apps I traced the nested Expanding this would create a new contract rather than fix a missed branch. It would require traversal rules for arrays, object visibility, cycles, and potentially user-defined iterables, adding work to every affected cache read without a documented or demonstrated need. The current top-level behavior is source-faithful and covered, so no change is needed. |
Collaborator
Author
|
@coderabbitai I checked the complete review, including the outside-diff findings, nitpicks, and coverage warning. The three inline findings have individual replies on their threads. The remaining dispositions are:
No source or test changes are warranted by these items. |
|
Tip For best results, initiate chat on the files or code changes.
🐇💭 ✏️ Learnings added
|
|
That's a thorough trace and a solid justification. The upstream contract is explicitly top-level, the recursive expansion would require new traversal, cycle, and visibility rules with no demonstrated need, and adding that work to every affected cache read isn't a net win. The current behavior is correct as-is. |
Update the displayed Hypervel documentation URL to match the existing hypervel.org target. This removes the stale hypervel.com label so the package README presents one accurate canonical documentation address.
Record the completed Cache audit, including its final architecture, verified findings, accepted corrections, rejected speculative machinery, regression coverage, validation, and Laravel-facing result. Mark Cache complete in the package checklist, route the next audit cycle to Session, and add the cache serialization finding to the cross-package revalidation index for its remaining consumers.
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 brings the Cache package up to the current supported Laravel surface while tightening the parts that need different ownership rules in a long-lived, pooled Swoole runtime.
The user-facing additions include the Storage cache driver,
rememberWithWarmth(), closure-based TTLs, incomplete-class handling, current facade metadata, and the corresponding configuration and documentation. The port keeps Hypervel's existing nullable values, Swoole store, failover and stack stores, Redis tag modes, transforms, SafeScan behavior, and explicit pinned-connection API.The rest of the change fixes lifecycle and correctness issues that only become expensive or unsafe once a worker serves many requests:
Redis-backed cache work no longer holds a pooled connection across arbitrary application callbacks. Hits still use one checkout. Misses use short read and write checkouts around the callback rather than retaining a lease while user code runs, and no additional Redis command is introduced. Explicit
withPinnedConnection()remains available for callers that intentionally need one held connection.Redis tag maintenance now follows PhpRedis SafeScan cursor semantics, continues through sparse pages, borrows connections per page or chunk, avoids nested checkouts, and does not retain complete tag result sets. Permanent any-mode additions use the existing atomic paths without inventing a new event contract.
The object serialization boundary is explicit and secure by default. Cache declares the allowed serializable classes, applies that policy to Swoole user payloads without restricting framework-owned metadata, and documents the native PhpRedis serializer boundary: extension-level serializers deserialize before Cache can enforce its own class policy.
Finally, this removes the superseded Redis remember-operation hierarchy, unused operation containers and exception types, repeated factory resolution, and drift-prone call-site configuration defaults. An exhaustive regression now warms every lazy
RedisStorecache and verifies that each boot-time configuration mutator invalidates the complete set, so a future cache slot cannot silently retain stale derived state.Why
The previous implementation mixed Laravel's request-lifetime assumptions with pooled connections and worker-lifetime state. That created several failure modes:
These fixes move each responsibility to the layer that owns it. They do not add a retry loop, registry, state machine, compatibility path, or hot-path synchronization.
Compatibility
Current supported Laravel Cache APIs, facade metadata, configuration behavior, and documentation are preserved. Hypervel-specific behavior remains where the runtime requires it:
cache.limiterkey is declared explicitly throughCACHE_LIMITER;add()remains event-free, matching its established atomic contract.The secure serialization default may require applications that intentionally cache objects to opt those classes in. The documentation shows that configuration and calls out the separate native PhpRedis serializer boundary.
Performance
The hot-path changes are bounded:
RedisStoreinstead of repeatedly resolving it.The change removes traffic-proportional connection retention without adding locks, polling, retries, or unbounded worker state.
Testing
Validation covers:
RedisStorelazy-cache invalidation;Summary by CodeRabbit
New Features
Bug Fixes
Documentation