Add stack cache tags#419
Merged
Merged
Conversation
Record the reviewed implementation plan for stack cache tag support and the tag architecture refactor. The plan captures the agreed design decisions, rejected alternatives, implementation phases, verification expectations, and comment guidance used for this PR. Keeping it in the repo gives future maintainers the rationale behind the larger cache changes rather than only the final code shape.
Introduce a slim abstract TagSet that only owns the store and tag names, then move namespace-based behavior into NamespacedTagSet and the generic random-id machinery into VersionedTagSet. Redis all-mode tags now extend the namespaced branch directly, while Redis any-mode tags extend the slim base without inherited namespace methods to neutralize. This makes the tag mode distinction structural instead of relying on overrides that disable inherited behavior. Update the Redis tag set tests to assert the new shape and remove coverage for methods that no longer exist on any-mode tag sets.
Make TaggedCache an abstract mode-neutral base, add NamespacedTaggedCache for all-mode semantics, and add AnyModeTaggedCache for write/index/flush-only semantics. Wire Redis all-mode and any-mode tagged caches onto those branches, remove the inherited-then-neutralized any-mode behavior, and keep optimized Redis remember paths while normalizing enum keys at their public boundaries. Add Redis touch operations that keep key TTLs and tag metadata in sync, and add any-mode plain forget cleanup so deleting a tagged key removes reverse-index and tag hash membership. This prevents stale tag metadata from deleting a later unrelated value at the same key. Update Redis store routing, doctor namespaced-key handling, and the affected unit/integration tests for clear scope, touch score synchronization, any-mode metadata cleanup, zero-TTL semantics, and enum-key regressions.
Replace method_exists-based tag support detection with a TaggableStore capability probe so stores whose tag support depends on configuration or composition can report their real state. Add a supportsFlushingLocks probe to CanFlushLocks for the same reason: composite stores can implement the lock-flush contract while delegating to a lower layer that may not actually support flushing locks. Update repository and event tests so mocks describe the new capability surface explicitly instead of relying on structural shortcuts.
Teach StackStore to validate taggable layer composition and expose any-mode tagged caches when the composition is safe. Non-taggable microcache layers may sit above the first taggable layer, while every layer from that point down must be an any-mode taggable store to avoid flushed-value resurrection. Add StackTagSet and StackTaggedCache so tagged stack writes record indexes in taggable layers while reads continue through the normal plain-key stack path. Stack tagged writes preserve the existing record format, rollback behavior, and proxy TTL clamping, including the taggable-layer path where tagged writes bypass the proxy. Delegate stack locks and lock flushing to the bottom layer so lock-backed cache helpers work when the bottom store is lock-capable without ever caching lock state in upper tiers. Cover composition validation, tagged write mechanics, remember hits and misses, TTL clamps, lock delegation, empty-stack validation, and Redis integration behavior for flush staleness and no-resurrection guarantees.
Make the JWT blacklist storage mode-aware. All-mode stores keep the existing tag-scoped read/delete behavior, while any-mode stores write through tags but read and delete direct prefixed keys because any-mode tags are invalidation indexes only. Keep the provider gate on Repository::supportsTags(), which is now composition-aware, so valid stack stores are accepted and invalid or non-taggable stores fail before blacklist storage is used. Add tests for all-mode behavior, any-mode direct-key isolation, disabled blacklist construction over unsupported stores, and valid stack acceptance.
Update the Eloquent user provider cache tag gate to check store type, tag support, and tag mode in that order. This lets invalid stack compositions report unsupported tags without treating getTagMode() as a probe. Valid any-mode stacks now satisfy the same auth-cache tag contract as any-mode Redis stores, while all-mode and non-taggable stores continue to be rejected with explicit errors. Extend auth cache tests for valid stack acceptance, invalid stack rejection without getTagMode(), and the updated mock capability surface.
Describe stack tag support as an any-mode-only composition and document the safe layer shape for microcache stacks. Call out why non-taggable layers below the taggable region are rejected, how bounded staleness works for non-taggable upper layers, and how direct stack writes and touch operations relate to tag metadata. Also clarify the any-mode Redis metadata contract for tagged writes, plain forget, finite touch, plain rewrites, and tagged touch failures.
Update authentication docs and config comments to describe valid any-mode cache stacks as supported stores for auth cache tags. Clarify that untagged auth caching still validates the outer stack store while tagged auth caching validates the stack tag composition, and preserve the node-local staleness caveat for stack microcaches.
Document that JWT blacklist storage supports taggable all-mode and any-mode stores while any-mode uses direct prefixed blacklist keys under the hood. Add the security caveat for cache stacks and node-local tiers: revoked tokens can remain visible to another node until that node's local cache entry expires, so strict revocation should use a fully shared store or a very short upper-tier TTL.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates cache tagging, Redis tag operations, stack-store delegation, lock-flush capability detection, and Auth/JWT validation to distinguish any-mode tag support from general taggable stores. It also adds matching documentation and tests for the new behavior. ChangesCache Tagging and Stack Store Overhaul
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant RedisStore
participant AnyTagOperations
participant Touch
participant Redis
RedisStore->>AnyTagOperations: touch()
AnyTagOperations->>Touch: execute(key, seconds)
Touch->>Redis: evalWithShaCache(touchWithTagsScript)
Redis-->>Touch: success boolean
Touch-->>RedisStore: bool
sequenceDiagram
participant Client
participant StackTaggedCache
participant StackStore
participant UpperLayer
participant TaggableLayer
Client->>StackTaggedCache: put(key, value, ttl)
StackTaggedCache->>StackStore: putRecordTagged(tags, key, record)
StackStore->>UpperLayer: putToStore(record)
StackStore->>TaggableLayer: putToStoreTagged(tags, record)
TaggableLayer-->>StackStore: success
StackStore-->>StackTaggedCache: bool
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Pull request overview
This PR extends Hypervel’s cache tagging system to support any-mode tags across stack cache stores, enabling a short-lived L1 cache layer in front of Redis while preserving tagged invalidation semantics. It also refactors tag internals into mode-specific abstractions and tightens Redis tagged-cache lifecycle behavior (touch/forget/clear semantics, TTL consistency, enum key normalization).
Changes:
- Refactor tag architecture into mode-specific TagSet/TaggedCache variants (namespaced vs any-mode), and update Redis tagged-cache operations accordingly.
- Add stack-store tag support (any-mode only) plus stack lock delegation to the bottom layer, with capability probes reflecting true support.
- Update framework consumers (Auth + JWT) and add/expand unit + integration tests and docs covering the new semantics.
Reviewed changes
Copilot reviewed 58 out of 59 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/JWT/Storage/TaggedCacheTest.php | Expands JWT tagged-cache tests to cover all-mode vs any-mode behavior and direct-key prefixing semantics. |
| tests/JWT/JWTServiceProviderTest.php | Updates blacklist storage validation to accept any-mode (including valid stacks) and improves error messaging. |
| tests/Integration/Cache/Redis/TtlHandlingIntegrationTest.php | Adds integration coverage for all-mode touch score updates and any-mode plain touch metadata synchronization. |
| tests/Integration/Cache/Redis/TagConsistencyIntegrationTest.php | Updates any-mode forget expectations to verify immediate tag membership cleanup and reuse safety. |
| tests/Integration/Cache/Redis/StackTaggedCacheIntegrationTest.php | New integration tests validating stack tagged writes, bounded L1 staleness after flush, and composition safety. |
| tests/Integration/Cache/Redis/PruneIntegrationTest.php | Adjusts prune tests to reflect plain any-mode forget cleanup and simulates orphan creation via direct Redis deletes. |
| tests/Integration/Cache/Redis/HashLifecycleIntegrationTest.php | Updates orphan-field tests to delete outside cache-aware paths to preserve the new forget cleanup behavior. |
| tests/Cache/Redis/RedisStoreTest.php | Adds unit tests ensuring RedisStore routes touch/forget through the correct all/any-mode paths. |
| tests/Cache/Redis/AnyTagSetTest.php | Removes tests for API methods no longer applicable after TagSet refactor. |
| tests/Cache/Redis/AnyTaggedCacheTest.php | Adds negative API tests (PSR + ArrayAccess) for any-mode tagged caches, and validates new zero-TTL semantics + enum normalization. |
| tests/Cache/Redis/AllTagSetTest.php | Adds coverage for reset/flush walking all tags under the refactored TagSet API. |
| tests/Cache/Redis/AllTaggedCacheTest.php | Adds clear(), touch(), putMany(0 TTL), and enum normalization tests for all-mode Redis tagged caches. |
| tests/Cache/FunnelUnsupportedStoresTest.php | Removes the “StackStore doesn’t implement LockProvider” assertion since StackStore now delegates locks. |
| tests/Cache/CacheTaggedCacheTest.php | Adds clear() semantics test ensuring tagged clear does not wipe plain keys in the same store. |
| tests/Cache/CacheStackStoreTest.php | Adds constructor validation test requiring at least one stack layer. |
| tests/Cache/CacheStackStoreTagsTest.php | New unit tests for stack tag composition validation and tagged write behavior across layers. |
| tests/Cache/CacheStackStoreLocksTest.php | New unit tests for stack lock delegation, and lock flushing capability behavior. |
| tests/Cache/CacheRepositoryTest.php | Updates Repository behavior tests for supportsTags/lock flushing probes and taggable-store “supportsTags() false” handling. |
| tests/Cache/CacheRepositoryEnumTest.php | Updates enum tag tests to reflect supportsTags probing behavior. |
| tests/Cache/CacheEventsTest.php | Updates lock flushing event test to account for supportsFlushingLocks probing. |
| tests/Auth/AuthEloquentUserProviderCacheTest.php | Adds acceptance for any-mode stack tags and improves rejection messages/behavior for invalid taggable stacks. |
| src/testbench/hypervel/config/auth.php | Updates auth cache config docs to mention stack tag composition validation when tags are enabled. |
| src/jwt/src/Storage/TaggedCache.php | Adds direct-key mode for any-mode stores (with prefix isolation) while keeping tagged writes for invalidation indexing. |
| src/jwt/src/JWTServiceProvider.php | Improves JWT blacklist “requires tags” exception message to acknowledge both tag modes. |
| src/jwt/config/jwt.php | Documents bounded blacklist visibility with node-local stack tiers. |
| src/foundation/config/auth.php | Mirrors testbench auth config doc updates regarding stack tag validation. |
| src/contracts/src/Cache/CanFlushLocks.php | Adds supportsFlushingLocks() probe to represent real lock flushing capability (esp. composite stores). |
| src/cache/src/VersionedTagSet.php | Introduces versioned/namespaced tag set for stores without native tag indexes. |
| src/cache/src/TagSet.php | Refactors TagSet into an abstract base with mode-specific subclasses owning semantics. |
| src/cache/src/TaggedCache.php | Refactors base tagged cache, adds PSR clear() scoped to tags. |
| src/cache/src/TaggableStore.php | Updates default tags() to return a namespaced tagged cache with a versioned tag set; introduces supportsTags(). |
| src/cache/src/StackTagSet.php | New tag set delegating flush across taggable stack layers (any-mode). |
| src/cache/src/StackTaggedCache.php | New any-mode tagged cache implementation for StackStore with tagged writes + plain reads/remember semantics. |
| src/cache/src/StackStoreProxy.php | Adds accessors for underlying store + ttl override to support taggable layer introspection. |
| src/cache/src/StackStore.php | Adds tag validation/composition rules, taggable-layer traversal, and lock delegation + lock flushing support. |
| src/cache/src/Repository.php | Updates supportsTags/supportsFlushingLocks probes and tag operation gating based on TaggableStore/supportsTags(). |
| src/cache/src/RedisStore.php | Routes touch/forget through any-mode metadata-aware operations; introduces Touch op and supportsFlushingLocks probe. |
| src/cache/src/Redis/Operations/Touch.php | New all-mode plain expire wrapper operation. |
| src/cache/src/Redis/Operations/AnyTagOperations.php | Adds any-mode Touch + Forget operations and caches them. |
| src/cache/src/Redis/Operations/AnyTag/Touch.php | Implements metadata-aware any-mode touch (key + reverse index + tag fields + registry TTL coherence). |
| src/cache/src/Redis/Operations/AnyTag/Forget.php | Implements metadata-aware any-mode forget (key + tag membership + reverse index cleanup). |
| src/cache/src/Redis/Operations/AllTagOperations.php | Adds all-mode tagged Touch operation and caches it. |
| src/cache/src/Redis/Operations/AllTag/Touch.php | Implements all-mode tagged touch updating both key expiry and tag ZSET scores coherently. |
| src/cache/src/Redis/Console/Doctor/DoctorContext.php | Updates namespaced key derivation to use NamespacedTaggedCache explicitly. |
| src/cache/src/Redis/AnyTagSet.php | Simplifies any-mode tag set behavior and aligns reset/flush with any-mode semantics. |
| src/cache/src/Redis/AnyTaggedCache.php | Rehomes any-mode behavior onto AnyModeTaggedCache base; updates TTL<=0 semantics and enum key normalization. |
| src/cache/src/Redis/AllTagSet.php | Updates AllTagSet to use NamespacedTagSet base and implement reset/flush. |
| src/cache/src/Redis/AllTaggedCache.php | Refactors all-mode tagged cache onto NamespacedTaggedCache; adds touch and adjusts TTL<=0 behavior. |
| src/cache/src/NamespacedTagSet.php | New base class for tag sets that namespace item keys (getNamespace/tagIds contract). |
| src/cache/src/NamespacedTaggedCache.php | New tagged cache for namespaced semantics (taggedItemKey + itemKey override). |
| src/cache/src/FileStore.php | Adds supportsFlushingLocks probe to match new CanFlushLocks interface. |
| src/cache/src/DatabaseStore.php | Adds supportsFlushingLocks probe to match new CanFlushLocks interface. |
| src/cache/src/AnyModeTaggedCache.php | New base class enforcing any-mode contract: tags are write/flush only; read/delete/touch operations throw. |
| src/cache/src/AbstractArrayStore.php | Adds supportsFlushingLocks probe to match new CanFlushLocks interface. |
| src/boost/docs/jwt.md | Documents JWT blacklist requirements and bounded stale window with stack/L1 tiers. |
| src/boost/docs/cache.md | Updates cache docs to describe stack tags, any-mode forget/touch behavior, and stack lock delegation. |
| src/boost/docs/authentication.md | Updates auth docs to include any-mode stack tag support and composition validation. |
| src/auth/src/EloquentUserProvider.php | Updates auth cache tag requirements and expands rationale to include stack any-mode tag support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR adds any-mode tag support to
Confidence Score: 5/5Safe to merge. Tag-composition validation is lazily computed and memoized; new Redis operations handle both Lua and cluster paths consistently; correctness fixes in AnyTaggedCache/AllTaggedCache are straightforward. The core invariants are structurally enforced by the class hierarchy and tested. The two findings are design trade-offs that do not break existing behaviour or introduce data loss. src/cache/src/StackTaggedCache.php — add() is non-atomic; worth a follow-up if callers rely on single-winner semantics. Important Files Changed
Reviews (4): Last reviewed commit: "Harden Redis hash expiration doctor prob..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The stack tagged cache Redis integration test creates its file-store scratch directory after the Redis integration base setup runs. That base setup can skip before returning when Redis or any-mode tag support is unavailable in a CI job, but PHPUnit still invokes tearDown(). Make the temp directory property nullable and only delete it when setup reached the directory creation step. The test still uses ParallelTesting::tempDir(), and real test paths assert the directory has been initialized before creating the file store.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/cache/src/Redis/Operations/AllTag/Touch.php (1)
73-88: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMisleading comment about phpredis OPT_PREFIX for eval() keys.
The comment states static tag keys go in
KEYS"so phpredis applies OPT_PREFIX," but phpredis/predis do not apply the client's key prefix option toeval/evalShaKEYSarguments (this is a well-documented phpredis limitation) — that's precisely why the code must manually build$this->context->prefix() . $keyand$this->context->prefix() . $tagIdhere. The code itself is correct, but the comment's stated rationale is backwards and could mislead a future maintainer into "simplifying" this by removing the manual prefix concatenation, which would break key resolution.📝 Suggested comment fix
- // Static tag ZSET keys belong in KEYS so phpredis applies - // OPT_PREFIX; ARGV-built keys are only for dynamic Lua paths. + // phpredis/predis do not apply the client prefix option to eval()'s + // KEYS array, so static tag ZSET keys must be manually prefixed here + // before being placed in KEYS; ARGV-built keys are only for dynamic Lua paths.🤖 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/AllTag/Touch.php` around lines 73 - 88, The logic in Touch::evalWithShaCache key construction is correct, but the comment above the $keys array gives the wrong rationale for manually prefixing keys. Update the comment near the $this->context->prefix() concatenation to state that eval/evalSha KEY arguments do not get client prefixing applied, so the prefix must be added explicitly for the static tag and tagId keys. Keep the code in Touch unchanged and only fix the misleading explanation so future edits do not remove the manual prefixing.src/cache/src/StackStore.php (2)
208-273: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared tag-composition guard.
tags(),getTagMode(), andtaggableLayers()each repeat the identicaltagCompositionError()-check-and-throw block. Extracting a small guard method would keep the three call sites in sync and shorten each method.♻️ Proposed refactor
+ /** + * `@throws` NotSupportedException when the layer composition cannot support tags + */ + private function assertTagCompositionValid(): void + { + if (! is_null($error = $this->tagCompositionError())) { + throw new NotSupportedException($error); + } + } + public function tags(mixed $names): TaggedCache { - if (! is_null($error = $this->tagCompositionError())) { - throw new NotSupportedException($error); - } + $this->assertTagCompositionValid(); return new StackTaggedCache($this, new StackTagSet($this, is_array($names) ? $names : func_get_args())); } @@ public function getTagMode(): TagMode { - if (! is_null($error = $this->tagCompositionError())) { - throw new NotSupportedException($error); - } + $this->assertTagCompositionValid(); return TagMode::Any; } @@ public function taggableLayers(): array { - if (! is_null($error = $this->tagCompositionError())) { - throw new NotSupportedException($error); - } + $this->assertTagCompositionValid(); $layers = [];🤖 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/StackStore.php` around lines 208 - 273, Extract the repeated tag-composition error check in StackStore by adding a small shared guard method that wraps tagCompositionError() and throws NotSupportedException when needed. Update tags(), getTagMode(), and taggableLayers() to call that helper instead of duplicating the same if-not-null block so the behavior stays identical and the three methods remain in sync.
169-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce duplicated bottom-layer lock-flush check.
flushLocks()re-implements the sameinstanceof CanFlushLocks && supportsFlushingLocks()check already encapsulated insupportsFlushingLocks(), callingbottomStore()twice. Consider delegating to avoid the two implementations drifting apart later.♻️ Proposed refactor
public function flushLocks(): bool { - $store = $this->bottomStore(); - - if (! $store instanceof CanFlushLocks || ! $store->supportsFlushingLocks()) { + if (! $this->supportsFlushingLocks()) { throw new NotSupportedException(sprintf( 'The stack\'s bottom layer [%s] does not support flushing locks.', - $store::class + $this->bottomStore()::class )); } - return $store->flushLocks(); + return $this->bottomStore()->flushLocks(); }🤖 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/StackStore.php` around lines 169 - 196, Reduce the duplicated bottom-layer lock-flush capability check in StackStore by making flushLocks() delegate to supportsFlushingLocks() instead of repeating the instanceof CanFlushLocks and supportsFlushingLocks() logic. Keep bottomStore() lookup and the NotSupportedException behavior in flushLocks(), but centralize the eligibility check through supportsFlushingLocks() so the two methods cannot drift apart. Use the existing StackStore methods supportsFlushingLocks() and flushLocks() as the main touchpoints.
🤖 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/AnyTag/Touch.php`:
- Line 62: The any-mode tag touch path in Touch::touch now relies on Redis HASH
expiry commands, so document that this feature requires Redis 7.4+ or add a
runtime capability guard before calling hexpire/httl. Update the feature
description near Touch and any related any-tag cache docs/config checks to
mention the minimum Redis version and fail or skip gracefully when the server
does not support these commands.
---
Nitpick comments:
In `@src/cache/src/Redis/Operations/AllTag/Touch.php`:
- Around line 73-88: The logic in Touch::evalWithShaCache key construction is
correct, but the comment above the $keys array gives the wrong rationale for
manually prefixing keys. Update the comment near the $this->context->prefix()
concatenation to state that eval/evalSha KEY arguments do not get client
prefixing applied, so the prefix must be added explicitly for the static tag and
tagId keys. Keep the code in Touch unchanged and only fix the misleading
explanation so future edits do not remove the manual prefixing.
In `@src/cache/src/StackStore.php`:
- Around line 208-273: Extract the repeated tag-composition error check in
StackStore by adding a small shared guard method that wraps
tagCompositionError() and throws NotSupportedException when needed. Update
tags(), getTagMode(), and taggableLayers() to call that helper instead of
duplicating the same if-not-null block so the behavior stays identical and the
three methods remain in sync.
- Around line 169-196: Reduce the duplicated bottom-layer lock-flush capability
check in StackStore by making flushLocks() delegate to supportsFlushingLocks()
instead of repeating the instanceof CanFlushLocks and supportsFlushingLocks()
logic. Keep bottomStore() lookup and the NotSupportedException behavior in
flushLocks(), but centralize the eligibility check through
supportsFlushingLocks() so the two methods cannot drift apart. Use the existing
StackStore methods supportsFlushingLocks() and flushLocks() as the main
touchpoints.
🪄 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: 5ef6df04-31a3-454f-8775-2ea2a08875e9
📒 Files selected for processing (59)
docs/plans/2026-07-06-stack-cache-tags-and-tag-architecture-refactor.mdsrc/auth/src/EloquentUserProvider.phpsrc/boost/docs/authentication.mdsrc/boost/docs/cache.mdsrc/boost/docs/jwt.mdsrc/cache/src/AbstractArrayStore.phpsrc/cache/src/AnyModeTaggedCache.phpsrc/cache/src/DatabaseStore.phpsrc/cache/src/FileStore.phpsrc/cache/src/NamespacedTagSet.phpsrc/cache/src/NamespacedTaggedCache.phpsrc/cache/src/Redis/AllTagSet.phpsrc/cache/src/Redis/AllTaggedCache.phpsrc/cache/src/Redis/AnyTagSet.phpsrc/cache/src/Redis/AnyTaggedCache.phpsrc/cache/src/Redis/Console/Doctor/DoctorContext.phpsrc/cache/src/Redis/Operations/AllTag/Touch.phpsrc/cache/src/Redis/Operations/AllTagOperations.phpsrc/cache/src/Redis/Operations/AnyTag/Forget.phpsrc/cache/src/Redis/Operations/AnyTag/Touch.phpsrc/cache/src/Redis/Operations/AnyTagOperations.phpsrc/cache/src/Redis/Operations/Touch.phpsrc/cache/src/RedisStore.phpsrc/cache/src/Repository.phpsrc/cache/src/StackStore.phpsrc/cache/src/StackStoreProxy.phpsrc/cache/src/StackTagSet.phpsrc/cache/src/StackTaggedCache.phpsrc/cache/src/TagSet.phpsrc/cache/src/TaggableStore.phpsrc/cache/src/TaggedCache.phpsrc/cache/src/VersionedTagSet.phpsrc/contracts/src/Cache/CanFlushLocks.phpsrc/foundation/config/auth.phpsrc/jwt/config/jwt.phpsrc/jwt/src/JWTServiceProvider.phpsrc/jwt/src/Storage/TaggedCache.phpsrc/testbench/hypervel/config/auth.phptests/Auth/AuthEloquentUserProviderCacheTest.phptests/Cache/CacheEventsTest.phptests/Cache/CacheRepositoryEnumTest.phptests/Cache/CacheRepositoryTest.phptests/Cache/CacheStackStoreLocksTest.phptests/Cache/CacheStackStoreTagsTest.phptests/Cache/CacheStackStoreTest.phptests/Cache/CacheTaggedCacheTest.phptests/Cache/FunnelUnsupportedStoresTest.phptests/Cache/Redis/AllTagSetTest.phptests/Cache/Redis/AllTaggedCacheTest.phptests/Cache/Redis/AnyTagSetTest.phptests/Cache/Redis/AnyTaggedCacheTest.phptests/Cache/Redis/RedisStoreTest.phptests/Integration/Cache/Redis/HashLifecycleIntegrationTest.phptests/Integration/Cache/Redis/PruneIntegrationTest.phptests/Integration/Cache/Redis/StackTaggedCacheIntegrationTest.phptests/Integration/Cache/Redis/TagConsistencyIntegrationTest.phptests/Integration/Cache/Redis/TtlHandlingIntegrationTest.phptests/JWT/JWTServiceProviderTest.phptests/JWT/Storage/TaggedCacheTest.php
💤 Files with no reviewable changes (2)
- tests/Cache/FunnelUnsupportedStoresTest.php
- tests/Cache/Redis/AnyTagSetTest.php
Make cache capability probes match the operations they guard. Redis, file, and database stores now report lock flushing support only when lock storage is separate, while their direct flushLocks guards remain in place for store-level calls. Let conditional taggable stores own their detailed tag support failures by removing Repository::tags()' supportsTags preflight. Non-taggable stores still get the generic BadMethodCallException, but invalid StackStore compositions now preserve their NotSupportedException message. Clarify the TaggableStore contract for conditional tag support, extract the StackStore tag-composition guard, and tighten Redis doctor/docs wording so any-mode requirements are tied to hash-field expiration commands including HSETEX and HEXPIRE. Add regression coverage for preserved StackStore tag errors, honest lock-flush probes, repository/store exception boundaries, and the updated enum tag expectations.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cache/src/Redis/Console/Doctor/Checks/HexpireCheck.php (1)
14-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMessage mentions HSETEX but the probe only tests HEXPIRE.
The doc comment and fix instructions now reference both
HSETEXandHEXPIRE, butrun()(Lines 45-58) only exerciseshexpire(). If a server supports one but not the other, the check name ('HEXPIRE Command', Line 31) and message could be misleading about which command actually failed.Also applies to: 70-70
🤖 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/Console/Doctor/Checks/HexpireCheck.php` around lines 14 - 15, The HexpireCheck probe is describing both HSETEX and HEXPIRE even though run() only tests HEXPIRE, so align the check text and messaging with the actual command being exercised. Update the doc comment and the check name/message in HexpireCheck, and if HSETEX is intentionally part of the requirement then extend run() to probe it explicitly; otherwise keep the references limited to HEXPIRE so the reported failure matches the real failing command.
🤖 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.
Nitpick comments:
In `@src/cache/src/Redis/Console/Doctor/Checks/HexpireCheck.php`:
- Around line 14-15: The HexpireCheck probe is describing both HSETEX and
HEXPIRE even though run() only tests HEXPIRE, so align the check text and
messaging with the actual command being exercised. Update the doc comment and
the check name/message in HexpireCheck, and if HSETEX is intentionally part of
the requirement then extend run() to probe it explicitly; otherwise keep the
references limited to HEXPIRE so the reported failure matches the real failing
command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b074166-1ff6-4348-a1e5-8d90c24a0bc4
📒 Files selected for processing (15)
src/boost/docs/cache.mdsrc/cache/src/DatabaseStore.phpsrc/cache/src/FileStore.phpsrc/cache/src/Redis/Console/Doctor/Checks/HexpireCheck.phpsrc/cache/src/Redis/Console/Doctor/Checks/RedisVersionCheck.phpsrc/cache/src/RedisStore.phpsrc/cache/src/Repository.phpsrc/cache/src/StackStore.phpsrc/cache/src/TaggableStore.phptests/Cache/CacheDatabaseStoreTest.phptests/Cache/CacheFileStoreTest.phptests/Cache/CacheRedisStoreTest.phptests/Cache/CacheRepositoryTest.phptests/Integration/Cache/Redis/StackTaggedCacheIntegrationTest.phptests/Integration/Cache/RedisStoreTest.php
✅ Files skipped from review due to trivial changes (1)
- src/cache/src/Redis/Console/Doctor/Checks/RedisVersionCheck.php
🚧 Files skipped from review as they are similar to previous changes (7)
- src/cache/src/FileStore.php
- tests/Integration/Cache/Redis/StackTaggedCacheIntegrationTest.php
- src/cache/src/TaggableStore.php
- src/cache/src/Repository.php
- src/cache/src/RedisStore.php
- src/cache/src/StackStore.php
- src/boost/docs/cache.md
Collaborator
Author
|
@coderabbitai responding to the first-round review-body notes here, separate from the newer HexpireCheck nitpick.
|
|
Tip For best results, initiate chat on the files or code changes.
Thanks for the detailed rationale on each. |
Repository::touch() now normalizes enum keys before probing the store and reads the existing value through getRaw(). This keeps cached null sentinels distinct from misses and avoids passing enum instances through the string-only item-key path. All-mode Redis tagged touch now uses the same raw read before updating TTL metadata. Finite touches update the key and tag scores for cached-null values, while touch(key, null) re-stores the raw sentinel through the tagged forever path instead of writing a literal null. The regression coverage pins the plain repository enum/sentinel behavior, the all-mode tagged unit path, and the Redis integration case where a nullable tagged value is touched, survives stale pruning, and remains stored as the sentinel.
Rename the old HEXPIRE-only doctor check to HashFieldExpirationCheck so the name matches the capability any-mode tags actually require. The environment check now probes both HSETEX and HEXPIRE in any tag mode. HSETEX is used by tagged writes and HEXPIRE is used when plain touch extends any-mode tag metadata, so checking only HEXPIRE could report a store as ready when writes would still fail. The tests cover all-mode skipping, successful any-mode probing, and the throwing failure path with the existing fix instructions.
The doctor probe now treats false returns from either HSETEX or HEXPIRE as a failed capability check instead of relying only on exceptions. This matches phpredis behavior where server-side command errors may surface as false plus connection error state rather than Throwable. Without this, cache:redis-doctor could report any-mode tag support as available on servers that do not actually support the required hash-field expiration commands. The regression test covers the silent false-return path and verifies that the existing upgrade/switch-mode fix instructions are still reported.
Collaborator
Author
|
@greptileai @coderabbitai follow-up on the remaining review-body items. Greptile:
CodeRabbit:
Validation after the fixes: focused doctor tests green, focused Horizon test green after the transient teardown failure, and |
|
Your plan includes PR reviews subject to rate limits. More reviews will be available in 58 minutes. |
|
All three items are confirmed resolved:
The PR looks good to merge from my side. |
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.
Summary
This PR adds any-mode cache tag support to stack cache stores, allowing a short-lived local cache layer to sit in front of Redis while still preserving tagged invalidation semantics.
For more information, see: docs/plans/2026-07-06-stack-cache-tags-and-tag-architecture-refactor.md
The implementation also refactors the tag architecture so each tag mode has abstractions that match how it actually works. All-mode remains namespace/version based. Any-mode remains index based. Stack stores expose tags only when the composed store chain can honestly support any-mode tags.
What Changed
The cache tag hierarchy is split into mode-specific classes:
TagSetis now the mode-neutral base.NamespacedTagSetandVersionedTagSetmodel namespace/version semantics.AnyModeTaggedCachemodels key-addressed any-mode semantics.Stack cache stores now support tags and locks:
Redis tagged cache lifecycle behavior is tightened:
clear()no longer risks flushing an entire backing store.forget()removes reverse-index and tag membership entries.touch()keeps key TTLs and tag index TTLs coherent.touch()updates both the item TTL and tag sorted-set scores.put()/putMany()with non-positive TTLs delete plain keys instead of leaving inconsistent state.remember()andrememberForever()normalize enum keys before use.Framework consumers were updated to use the new capabilities:
Design Notes
Plain cache writes remain plain.
put()andforever()do not inspect tag indexes just because a key may have previously been tagged. Tag membership is maintained by the tagged APIs; plain deletion cleans up indexes because otherwise it leaves stale membership behind.Stack tag support intentionally requires any-mode tags. Namespace/version based all-mode tags are not safe to compose behind a fast L1 cache because invalidation changes namespaces instead of directly addressing keys.
Verification
composer fix./vendor/bin/phpunit --no-progress tests/Cache/CacheStackStoreTagsTest.php./vendor/bin/phpunit --no-progress tests/Cache/Redis/AnyTaggedCacheTest.php./vendor/bin/phpunit --no-progress tests/Cache/Redis/AllTaggedCacheTest.phpSummary by CodeRabbit
New Features
Bug Fixes
Documentation