Uh oh!
There was an error while loading. Please reload this page.
fix: Redis invalidate not working when there is only 1 entry - #26
Conversation
📝 WalkthroughWalkthroughRedis tag-based and global invalidation now delete cache keys when scans return a single matching key. A Redis test verifies that invalidating one tagged cached entry changes its subsequent status to ChangesRedis invalidation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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 |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/providers/redis.ts (1)
63-69: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAwait the Redis deletions before resolving invalidation.
invalidate()andinvalidateAll()resolve when the scan stream ends, not when the asyncDELcalls insidedatahandlers finish. That can report success while keys are still being deleted, and anyDELrejection is dropped.
src/providers/redis.ts#L53-L72: await the per-tag delete promises before resolving.src/providers/redis.ts#L76-L91: do the same for the global scan.🤖 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/providers/redis.ts` around lines 63 - 69, Update invalidate() at src/providers/redis.ts:53-72 and invalidateAll() at src/providers/redis.ts:76-91 so Redis DEL promises from scan data handlers are collected and awaited before resolving on stream end; propagate any deletion rejection instead of dropping it.
🤖 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/redis.test.ts`:
- Around line 1210-1228: Update the cache invalidation test around
extDb.user.exists and extDb.$cache.invalidate to deterministically wait for the
first read’s asynchronous cache.set/tag registration to complete before
invalidating. Use the existing test setup or cache synchronization mechanism
rather than relying on timing, then preserve the final miss assertion.
---
Outside diff comments:
In `@src/providers/redis.ts`:
- Around line 63-69: Update invalidate() at src/providers/redis.ts:53-72 and
invalidateAll() at src/providers/redis.ts:76-91 so Redis DEL promises from scan
data handlers are collected and awaited before resolving on stream end;
propagate any deletion rejection instead of dropping it.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e6abba89-856e-4c0c-88e8-dca819644c80
📒 Files selected for processing (2)
src/providers/redis.tstests/redis.test.ts
| await extDb.user.exists({ | ||
| cache: { | ||
| tags: ['user1'], | ||
| ttl: 60, | ||
| }, | ||
| }) | ||
| await extDb.$cache.invalidate({ | ||
| tags: ['user1'], | ||
| }) | ||
| await extDb.user.exists({ | ||
| cache: { | ||
| tags: ['user1'], | ||
| ttl: 60, | ||
| }, | ||
| }) | ||
| expect(extDb.$cache.status).toBe('miss') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Synchronize the seed write before invalidating.
The first cached read schedules cache.set asynchronously in src/plugin.ts without awaiting it. Calling invalidate immediately can scan before the single entry or its tag membership exists, allowing the later read to hit for reasons unrelated to this fix. Add a deterministic synchronization step or await the cache write through the test setup.
🤖 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/redis.test.ts` around lines 1210 - 1228, Update the cache invalidation
test around extDb.user.exists and extDb.$cache.invalidate to deterministically
wait for the first read’s asynchronous cache.set/tag registration to complete
before invalidating. Use the existing test setup or cache synchronization
mechanism rather than relying on timing, then preserve the final miss assertion.
Summary by CodeRabbit
Bug Fixes
Tests