feat(@cacheable/node-cache): enhance NodeCacheStore with missing features#1643
Conversation
…etTtl, flushAll, events, and useClones Add missing functionality to NodeCacheStore to bring it closer to parity with NodeCache: - keys() and has() for key inspection with expiration-aware checks - getTtl() to inspect a key's expiration timestamp - flushAll() that clears data, resets all stats, and emits a "flush" event - Events: "set", "del", "expired", "flush" emitted on corresponding operations - useClones option for deep-cloning values via structuredClone on get/set - checkperiod option for interval-based expired item detection - deleteOnExpire option to control automatic deletion of expired items - close() and getIntervalId() for interval lifecycle management Also fixes several bugs found during code review: - setTtl() falsy-zero: `if (item)` replaced with `if (item !== undefined)` - mdel() unconditional stats: now delegates to del() for proper guarding - startInterval() timer leak: calls stopInterval() before creating new timer - set() overwrite double-counting: adjusts stats for existing key before re-adding - checkData() unhandled rejection: wraps async call in .catch() with error emission - handleExpired() stats underflow: guards decrements behind key existence check - checkData() Map mutation: snapshots entries before iterating https://claude.ai/code/session_01R74MUq37AtZmGxxoKPiHDt
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the NodeCacheStore with support for cloning cached items, active background expiration checks via intervals, event emissions for cache operations, and several new utility methods like keys(), has(), getTtl(), and flushAll(). Feedback focuses on critical concurrency issues, such as a race condition in handleExpired that could lead to silent data loss, and another race condition in set that can cause double-counting of stats. Additionally, suggestions are provided to filter out expired keys in the keys() method and to parallelize sequential operations in mget, mdel, and mset using Promise.all for better performance.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1643 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 26 26
Lines 2497 2608 +111
Branches 554 579 +25
==========================================
+ Hits 2497 2608 +111 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98bdccf604
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…atures Document new NodeCacheStore options (useClones, checkperiod, deleteOnExpire), methods (keys, has, getTtl, flushAll, close, getStats, flushStats), and events (set, del, expired, flush, flush_stats) with usage examples and option tables. https://claude.ai/code/session_01R74MUq37AtZmGxxoKPiHDt
- Add race condition guard in handleExpired with double isExpired check before and after async Keyv get to prevent deleting refreshed keys - Move _keys.add before async operations in set() to prevent concurrent set calls from both seeing isOverwrite=false - Emit expiration timestamp in set event to match NodeCache contract https://claude.ai/code/session_01R74MUq37AtZmGxxoKPiHDt
Parallelize batch operations for better performance with remote Keyv backends. Each individual set/get/del call adds to _keys synchronously before its first await, so unique keys (the common case) are safe from race conditions. Stats increments are atomic within each post-await microtask block. https://claude.ai/code/session_01R74MUq37AtZmGxxoKPiHDt
2 participants
Summary
Bug fixes found during code review
if (item)treated cached values of0,"",false,nullas non-existent; fixed toif (item !== undefined)del()for proper guardingstopInterval()firstvoid this.checkData()swallowed promise rejections; now wraps in.catch()with error emission_ttlswhile deleting entries; now snapshots entries before iteratingTest plan
https://claude.ai/code/session_01R74MUq37AtZmGxxoKPiHDt
Generated by Claude Code