feat(backend): implement comprehensive multi-level caching layer (Iss…#252
feat(backend): implement comprehensive multi-level caching layer (Iss…#252Topmatrixmor2014 wants to merge 4 commits into
Conversation
nexoraorg#169) ## Overview Implements a full caching layer for the backend with multi-level caching (L1: in-memory node-cache, L2: Redis), cache tags, statistics monitoring, cache warming, Prisma query cache middleware, and fallback support. ## Changes ### New Files - backend/src/config/cache.ts — Central cache configuration with env-var overrides, tiered strategy configs (memory/redis), TTL presets, monitoring & warming settings - backend/src/utils/cacheUtils.ts — Utility functions for tag key building, key hashing (DJB2), retry with exponential backoff, safe serialization/parsing ### Modified Files - backend/src/types/cache.ts — Added interfaces: CacheGetOptions, CacheSetOptions, CacheStats, CacheServiceWithStats, CacheServiceWithTags, CacheServiceWithWarming, MultiLevelCacheService, PrismaCacheMiddlewareOptions - backend/src/services/cacheService.ts — Major enhancement: * MemoryCacheService: L1 in-memory cache using node-cache with tag index * RedisCacheService: L2 Redis cache with tag storage via SETs and pipelines * MultiLevelCacheService: Orchestrates L1+L2 with: - L1 first, L2 fallback with automatic L1 population - Graceful degradation when Redis is unavailable (fallbackEnabled) - Cache statistics tracking (hits/misses/latency per tier) - Tag-based group invalidation (invalidateByTags) - Cache warming with batched concurrent loading and retries - Configurable monitoring with periodic stats logging * createPrismaCacheMiddleware: Prisma middleware for automatic query result caching with model/operation filtering and write-invalidation via tags * Backward-compatible singleton export (cache, redisCache, memoryCache) - backend/src/middleware/cache.ts — Enhanced middleware: * cacheMiddleware: Added skipRead/skipWrite callbacks, cache tags support, configurable TTL via CacheTTL presets, graceful error fallthrough * invalidateCacheKeys: Dynamic key-based invalidation middleware * invalidateCacheByTags: Tag-based group invalidation middleware * allowCacheBypass: Helper for X-Cache-Bypass / Cache-Control: no-cache * cacheStatsHandler: Express route handler exposing cache statistics * invalidateCache: Deprecated legacy wrapper (backward compatible) - backend/src/utils/cacheKeys.ts — Comprehensive key patterns for: accounts, credit scores, fraud alerts, transactions, analytics, health/metrics, users/sessions, API keys, feature flags ### Test Updates - backend/src/services/__tests__/cacheService.test.ts — Expanded from 10 to 33 tests: * MemoryCacheService: get/set, TTL, del, exists, incrBy, expire * RedisCacheService: get/set, del, exists, incrBy, expire, tag operations * MultiLevelCacheService: L1 hit, L2 fallback, full miss, skipCache/skipMemory, multi-level set with tags, stats tracking (hits/misses/sets/deletes), tag invalidation, incrBy with memory sync ### Dependencies - Added node-cache for in-memory (L1) caching ## Acceptance Criteria - [x] Response caching works via cacheMiddleware with X-Cache HIT/MISS headers - [x] Database queries are cacheable via Prisma middleware - [x] Cache invalidation works correctly (key-based and tag-based) - [x] Cache statistics are available (hits, misses, hit rate, latency per tier) - [x] Performance is improved via two-tier caching (L1 memory + L2 Redis) - [x] Cache fallback works (graceful degradation when Redis is down) - [x] Multi-level caching works (L1 population on L2 hit) - [x] Monitoring is in place (configurable stats logging) Closes nexoraorg#169
|
Warning Review limit reached
Next review available in: 56 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 (4)
📝 WalkthroughWalkthroughIntroduces configurable memory and Redis caching with shared key utilities, tag invalidation, warming, statistics, response middleware, Prisma query caching, fallback behavior, monitoring lifecycle wiring, package validation scripts, and updated blockchain CI workflows. ChangesCaching layer
Development and CI tooling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CacheMiddleware
participant MultiLevelCacheService
participant MemoryCacheService
participant RedisCacheService
Client->>CacheMiddleware: request
CacheMiddleware->>MultiLevelCacheService: get(cache key)
MultiLevelCacheService->>MemoryCacheService: read L1
MemoryCacheService-->>MultiLevelCacheService: hit or miss
MultiLevelCacheService->>RedisCacheService: read L2 on L1 miss
RedisCacheService-->>MultiLevelCacheService: cached value or miss
MultiLevelCacheService-->>CacheMiddleware: response value
CacheMiddleware-->>Client: cached or loaded response
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (1)
backend/src/services/__tests__/cacheService.test.ts (1)
146-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact Redis tag commands.
These tests still pass if the wrong tag key is used,
SADD/EXPIREis omitted, or Redis member keys are not deleted. Capture the pipeline mock and assert exactsadd,expire, anddelarguments, including overwrite and mixed-TTL cases.Also applies to: 205-218, 321-335, 373-390
🤖 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 `@backend/src/services/__tests__/cacheService.test.ts` around lines 146 - 157, Strengthen the tag-storage tests around cacheService.set and the related cases by capturing the pipeline mock and asserting exact sadd, expire, and del commands and arguments. Verify the expected Redis tag key, member keys, and TTL behavior, including overwrite and mixed-TTL scenarios, so the tests fail when commands are missing or use incorrect keys or values.
🤖 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 `@backend/src/middleware/cache.ts`:
- Around line 138-140: Restrict allowCacheBypass to authenticated administrative
or internal requests before honoring x-cache-bypass, and remove the
user-controlled cache-control: no-cache trigger unless an equivalent
authorization and rate-limiting policy is already enforced. Preserve normal
cached behavior for unauthenticated requests.
- Around line 93-106: Update backend/src/middleware/cache.ts lines 93-106 in
invalidateCacheKeys to register invalidation on the response finish event,
perform it only for 2xx–3xx status codes, handle asynchronous errors without
blocking completion, and call next() immediately; apply the same finish-event
and successful-status gating to invalidateCacheByTags at lines 118-128.
- Around line 158-160: Update the cache statistics response formatting around
hitRatePercent to handle a NaN stats.hitRate, defaulting it to 0 before
multiplying and formatting so zero-operation responses render as "0.00%" instead
of "NaN%".
- Around line 54-58: Update the cache-write condition in the res.json
interception around shouldSkipWrite so responses are cached only when the HTTP
status is successful (2xx). Preserve the existing skipWrite callback behavior,
and prevent 400/500 error responses from reaching the CacheSetOptions/write
path.
In `@backend/src/services/cacheService.ts`:
- Around line 407-417: Remove the process-local memory synchronization from
incrBy when Redis is enabled, specifically the memory.set call under
cacheConfig.strategy.memory.enabled. Return the Redis incrBy result directly so
counter reads remain Redis-backed and cannot be served from stale L1 snapshots.
- Around line 539-556: Update the cache warming flow around the batch
Promise.allSettled processing to count only non-null loader results that are
successfully passed to set, excluding fulfilled null results. Accumulate this
stored-value count across batches and use it for the completion marker’s
keysLoaded field instead of keys.length, while preserving the existing batch
success warning.
- Around line 646-654: Update the cache-key construction around hashKey,
serializeValue, and cacheKey to replace the 32-bit hash with SHA-256 over a
canonical serialization of args. Preserve the existing Prisma model/action key
structure and cache lookup behavior, ensuring distinct argument sets produce
reliably distinct keys.
- Around line 124-138: The cache encoding used by get and set is not round-trip
safe for raw strings and Prisma runtime values. Update serializeValue and
safeJsonParse, and the L1/L2 cache paths in get and set, to use one versioned
type-tagged representation that preserves strings, null, dates, buffers, and
BigInt values without parse ambiguity; ensure both layers read and write through
this shared serializer.
- Around line 307-315: Update the Redis read and L1 hydration logic in the cache
lookup method around redisValue and this.memory.set to fetch the remaining Redis
TTL alongside the value, then cap the memory cache defaultTtlSeconds to that
remaining TTL. Preserve the existing memory.enabled guard and avoid populating
L1 with a lifetime longer than the Redis entry’s remaining expiration.
- Around line 366-383: Update cache invalidation methods del, expire, and tag
invalidation so Redis failures are not treated as successful invalidations.
Retry failed Redis operations, then tombstone or bypass the affected keys/tags;
in strict mode, propagate the failure so dependent Prisma writes do not return
success. Preserve the existing successful memory and Redis invalidation
behavior.
- Around line 51-71: Update CacheService methods set, del, and addTags to
maintain a reverse key-to-tags index alongside tagIndex. Remove prior tag
memberships when overwriting a key, and clean both indexes when keys are deleted
or expire in memory or Redis; ensure Redis tag membership expiration is per
key/member or explicitly cleaned up rather than relying on one TTL for the
entire tag SET, and make addTags apply expiration consistently.
- Around line 660-663: The cache write in the Prisma caching flow must account
for relation-aware queries. Update the tagging around cacheInstance.set to add
tags derived from included or nested-selected relations, or apply the broader
Prisma invalidation tag used by writes, so related model mutations evict cached
results. Add coverage for a query using include or nested select and verify a
related write invalidates it.
In `@backend/src/utils/cacheUtils.ts`:
- Around line 11-21: Replace the 32-bit DJB2 implementation in hashKey with
SHA-256 using createHash from crypto, and add the import at the top of the file.
Hash the raw input with SHA-256 and return a deterministic encoded digest string
suitable for cache keys.
- Around line 81-89: Update serializeValue to remove the typeof value ===
'string' bypass, ensuring all non-null values—including strings—are serialized
with JSON.stringify before cache storage. Preserve the existing null and
undefined handling so deserialization retains the original value types.
---
Nitpick comments:
In `@backend/src/services/__tests__/cacheService.test.ts`:
- Around line 146-157: Strengthen the tag-storage tests around cacheService.set
and the related cases by capturing the pipeline mock and asserting exact sadd,
expire, and del commands and arguments. Verify the expected Redis tag key,
member keys, and TTL behavior, including overwrite and mixed-TTL scenarios, so
the tests fail when commands are missing or use incorrect keys or values.
🪄 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
Run ID: 830522ea-4caa-4bf0-98b2-960224ca69a6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
backend/package.jsonbackend/src/config/cache.tsbackend/src/middleware/cache.tsbackend/src/services/__tests__/cacheService.test.tsbackend/src/services/cacheService.tsbackend/src/types/cache.tsbackend/src/utils/cacheKeys.tsbackend/src/utils/cacheUtils.ts
| // Optionally skip cache write | ||
| const shouldSkipWrite = options.skipWrite?.(req, body as T) ?? false; | ||
|
|
||
| if (!shouldSkipWrite) { | ||
| const setOptions: CacheSetOptions = { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Prevent caching of error responses.
The middleware intercepts res.json and unconditionally caches the payload. If a route handler encounters an error and sends a 400 or 500 status code, that error response will be cached for the default TTL, turning a temporary failure into a prolonged outage for that route.
Ensure that only successful responses (e.g., 2xx status codes) are written to the cache.
🐛 Proposed fix
- // Optionally skip cache write
- const shouldSkipWrite = options.skipWrite?.(req, body as T) ?? false;
-
- if (!shouldSkipWrite) {
+ // Optionally skip cache write (and ensure we only cache successful responses)
+ const shouldSkipWrite = options.skipWrite?.(req, body as T) ?? false;
+ const isSuccess = res.statusCode >= 200 && res.statusCode < 300;
+
+ if (!shouldSkipWrite && isSuccess) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Optionally skip cache write | |
| const shouldSkipWrite = options.skipWrite?.(req, body as T) ?? false; | |
| if (!shouldSkipWrite) { | |
| const setOptions: CacheSetOptions = { | |
| // Optionally skip cache write (and ensure we only cache successful responses) | |
| const shouldSkipWrite = options.skipWrite?.(req, body as T) ?? false; | |
| const isSuccess = res.statusCode >= 200 && res.statusCode < 300; | |
| if (!shouldSkipWrite && isSuccess) { | |
| const setOptions: CacheSetOptions = { |
🤖 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 `@backend/src/middleware/cache.ts` around lines 54 - 58, Update the cache-write
condition in the res.json interception around shouldSkipWrite so responses are
cached only when the HTTP status is successful (2xx). Preserve the existing
skipWrite callback behavior, and prevent 400/500 error responses from reaching
the CacheSetOptions/write path.
| export function invalidateCacheKeys(keyProvider: (req: Request) => string[]) { | ||
| return async (_req: Request, _res: Response, next: NextFunction) => { | ||
| try { | ||
| await Promise.all(keys.map((k) => cache.del(k))); | ||
| const keys = keyProvider(_req); | ||
| if (keys.length > 0) { | ||
| await Promise.all(keys.map((k) => cache.del(k))); | ||
| } | ||
| next(); | ||
| } catch (err) { | ||
| next(err); | ||
| console.warn('[cache:invalidate] error', err); | ||
| next(); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Invalidation middleware race condition and execution order flaw.
Both invalidation middlewares execute immediately and block the middleware chain rather than waiting for the request to succeed. If placed before the route handler, they invalidate the cache before the database is updated (creating a race condition where concurrent reads load and cache stale DB data) and run even if the handler ultimately fails. If placed after the handler, they will likely never execute because standard Express handlers terminate the request without calling next().
Delay the invalidation until the request successfully completes by hooking into the response finish event.
backend/src/middleware/cache.ts#L93-L106: Wrap the key invalidation logic insideres.on('finish', () => { if (res.statusCode >= 200 && res.statusCode < 400) { ... } })and callnext()immediately outside it.backend/src/middleware/cache.ts#L118-L128: Apply the sameres.on('finish')event wrapper to the tag invalidation logic.
💡 Proposed fix for both middlewares
export function invalidateCacheKeys(keyProvider: (req: Request) => string[]) {
return (_req: Request, res: Response, next: NextFunction) => {
res.on('finish', () => {
// Only invalidate if the request was successful
if (res.statusCode >= 200 && res.statusCode < 400) {
try {
const keys = keyProvider(_req);
if (keys.length > 0) {
Promise.all(keys.map((k) => cache.del(k))).catch((err) =>
console.warn('[cache:invalidate] error', err)
);
}
} catch (err) {
console.warn('[cache:invalidate] error', err);
}
}
});
next();
};
}
export function invalidateCacheByTags(tags: string[]) {
return (_req: Request, res: Response, next: NextFunction) => {
res.on('finish', () => {
// Only invalidate if the request was successful
if (res.statusCode >= 200 && res.statusCode < 400) {
cache.invalidateByTags(tags).catch((err) =>
console.warn('[cache:invalidate:tags] error', err)
);
}
});
next();
};
}📍 Affects 1 file
backend/src/middleware/cache.ts#L93-L106(this comment)backend/src/middleware/cache.ts#L118-L128
🤖 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 `@backend/src/middleware/cache.ts` around lines 93 - 106, Update
backend/src/middleware/cache.ts lines 93-106 in invalidateCacheKeys to register
invalidation on the response finish event, perform it only for 2xx–3xx status
codes, handle asynchronous errors without blocking completion, and call next()
immediately; apply the same finish-event and successful-status gating to
invalidateCacheByTags at lines 118-128.
| export function allowCacheBypass(req: Request): boolean { | ||
| return req.headers['x-cache-bypass'] === 'true' || req.headers['cache-control'] === 'no-cache'; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unauthenticated cache bypass exposes a DoS vector.
Exposing cache bypass capabilities based solely on user-provided headers allows any client to trivially bypass the L1/L2 caching layers, leading to a potential Denial of Service (DoS) against the database.
Consider restricting this bypass capability strictly to authenticated administrative/internal requests, or ensure strict rate-limiting is applied to bypassed requests. If this is intended for public endpoints, it is highly recommended to remove the cache-control: no-cache trigger.
🤖 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 `@backend/src/middleware/cache.ts` around lines 138 - 140, Restrict
allowCacheBypass to authenticated administrative or internal requests before
honoring x-cache-bypass, and remove the user-controlled cache-control: no-cache
trigger unless an equivalent authorization and rate-limiting policy is already
enforced. Preserve normal cached behavior for unauthenticated requests.
| ...stats, | ||
| hitRatePercent: (stats.hitRate * 100).toFixed(2) + '%', | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent NaN rendering in cache statistics.
If there have been no cache operations, stats.hitRate may evaluate to NaN (0 / 0), resulting in a "NaN%" string in the JSON response. Defaulting to 0 prevents this formatting error.
🐛 Proposed fix
data: {
...stats,
- hitRatePercent: (stats.hitRate * 100).toFixed(2) + '%',
+ hitRatePercent: ((stats.hitRate || 0) * 100).toFixed(2) + '%',
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ...stats, | |
| hitRatePercent: (stats.hitRate * 100).toFixed(2) + '%', | |
| }, | |
| ...stats, | |
| hitRatePercent: ((stats.hitRate || 0) * 100).toFixed(2) + '%', | |
| }, |
🤖 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 `@backend/src/middleware/cache.ts` around lines 158 - 160, Update the cache
statistics response formatting around hitRatePercent to handle a NaN
stats.hitRate, defaulting it to 0 before multiplying and formatting so
zero-operation responses render as "0.00%" instead of "NaN%".
| async set<T = CacheValue>(key: string, value: T, options?: CacheSetOptions): Promise<void> { | ||
| const ttl = options?.ttlSeconds ?? cacheConfig.strategy.memory.defaultTtlSeconds; | ||
| const success = this.store.set(key, value, ttl); | ||
|
|
||
| if (success && options?.tags && options.tags.length > 0) { | ||
| for (const tag of options.tags) { | ||
| if (!this.tagIndex.has(tag)) { | ||
| this.tagIndex.set(tag, new Set()); | ||
| } | ||
| this.tagIndex.get(tag)!.add(key); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| async del(key: string): Promise<void> { | ||
| this.store.del(key); | ||
| // Clean up tag index | ||
| for (const [, keys] of this.tagIndex) { | ||
| keys.delete(key); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep tag indexes synchronized with key lifecycle.
Overwrites never remove prior tags, memory expiry leaves stale memberships, Redis deletion leaves SET members, and one member’s TTL controls the entire Redis tag SET. addTags can also create non-expiring SETs. This causes both missed and false invalidations.
Maintain a reverse key-to-tags index and clean it on overwrite, deletion, and expiry. Use per-member expiry or explicit lifecycle cleanup instead of a single TTL per tag SET.
Also applies to: 140-155, 491-517
🤖 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 `@backend/src/services/cacheService.ts` around lines 51 - 71, Update
CacheService methods set, del, and addTags to maintain a reverse key-to-tags
index alongside tagIndex. Remove prior tag memberships when overwriting a key,
and clean both indexes when keys are deleted or expire in memory or Redis;
ensure Redis tag membership expiration is per key/member or explicitly cleaned
up rather than relying on one TTL for the entire tag SET, and make addTags apply
expiration consistently.
| const results = await Promise.allSettled( | ||
| batch.map(async (key) => { | ||
| const value = await withRetry(() => loader(key), { maxRetries: 2 }); | ||
| if (value !== null) { | ||
| await this.set(key, value); | ||
| } | ||
| }) | ||
| ); | ||
|
|
||
| const succeeded = results.filter((r) => r.status === 'fulfilled').length; | ||
| if (succeeded < batch.length) { | ||
| console.warn(`[cache] Warm batch: ${succeeded}/${batch.length} keys loaded`); | ||
| } | ||
| } | ||
|
|
||
| // Mark warming complete | ||
| const warmingKey = buildWarmingKey('status'); | ||
| await this.set(warmingKey, { completed: true, timestamp: Date.now(), keysLoaded: keys.length }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Record the number of values actually warmed.
A fulfilled loader returning null counts as successful, while the completion marker always reports keys.length. Track and persist only values successfully stored.
🤖 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 `@backend/src/services/cacheService.ts` around lines 539 - 556, Update the
cache warming flow around the batch Promise.allSettled processing to count only
non-null loader results that are successfully passed to set, excluding fulfilled
null results. Accumulate this stored-value count across batches and use it for
the completion marker’s keysLoaded field instead of keys.length, while
preserving the existing batch success warning.
| // Build cache key from model, action, and hashed args | ||
| // Hashing prevents extremely long keys for complex queries | ||
| const argsHash = hashKey(serializeValue(args)); | ||
| const cacheKey = `prisma:${model}:${action}:${argsHash}`; | ||
|
|
||
| // Check cache | ||
| const cached = await cacheInstance.get(cacheKey); | ||
| if (cached !== null) { | ||
| return cached; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Replace the 32-bit cache-key hash.
Distinct Prisma arguments can collide and return another query’s cached result, potentially crossing user boundaries. Use SHA-256 over a canonical serialization instead of the current 32-bit hash.
🤖 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 `@backend/src/services/cacheService.ts` around lines 646 - 654, Update the
cache-key construction around hashKey, serializeValue, and cacheKey to replace
the 32-bit hash with SHA-256 over a canonical serialization of args. Preserve
the existing Prisma model/action key structure and cache lookup behavior,
ensuring distinct argument sets produce reliably distinct keys.
| await cacheInstance.set(cacheKey, result, { | ||
| ttlSeconds, | ||
| tags: [`prisma:${model}`, `prisma:${model}:${action}`], | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== outline cacheService ==\n'
ast-grep outline backend/src/services/cacheService.ts --view expanded || true
printf '\n== relevant ranges ==\n'
sed -n '600,740p' backend/src/services/cacheService.ts
printf '\n== search prisma tag usage ==\n'
rg -n "prisma:|tags: \[|invalidate|cacheInstance\.set|cacheInstance\.del|cacheInstance\.clear" backend/src -SRepository: nexoraorg/chenaikit
Length of output: 10441
🏁 Script executed:
set -euo pipefail
printf '\n== cacheService invalidateByTags/addTags implementation ==\n'
sed -n '430,525p' backend/src/services/cacheService.ts
printf '\n== cacheService tests around tagging/invalidation ==\n'
sed -n '120,420p' backend/src/services/__tests__/cacheService.test.ts
printf '\n== search for Prisma include/select cache tests ==\n'
rg -n "include|select|prisma-cache|createPrismaCacheMiddleware|invalidateByTags\\(\\[\\`prisma:" backend/src/services/__tests__ backend/src -SRepository: nexoraorg/chenaikit
Length of output: 12321
🏁 Script executed:
set -euo pipefail
printf '\n== createPrismaCacheMiddleware usages ==\n'
rg -n "createPrismaCacheMiddleware\\(" backend/src -S
printf '\n== prisma middleware / cache config types ==\n'
sed -n '1,220p' backend/src/types/cache.ts
printf '\n== precise cacheService line range ==\n'
sed -n '620,670p' backend/src/services/cacheService.tsRepository: nexoraorg/chenaikit
Length of output: 4388
Invalidate relation-aware Prisma cache entries. Cached User reads with include/nested select are still tagged only as prisma:User, so a Post write won’t evict them and they can return stale relation data. Add relation-derived tags or invalidate a broader Prisma tag on writes, and cover a query with include/nested select in tests.
🤖 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 `@backend/src/services/cacheService.ts` around lines 660 - 663, The cache write
in the Prisma caching flow must account for relation-aware queries. Update the
tagging around cacheInstance.set to add tags derived from included or
nested-selected relations, or apply the broader Prisma invalidation tag used by
writes, so related model mutations evict cached results. Add coverage for a
query using include or nested select and verify a related write invalidates it.
| /** | ||
| * Compute a safe hash for a cache key to keep keys within reasonable length. | ||
| * Uses a simple DJB2 hash for performance. | ||
| */ | ||
| export function hashKey(raw: string): string { | ||
| let hash = 5381; | ||
| for (let i = 0; i < raw.length; i++) { | ||
| hash = ((hash << 5) + hash + raw.charCodeAt(i)) | 0; | ||
| } | ||
| return Math.abs(hash).toString(36); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Replace DJB2 hash with a collision-resistant algorithm.
The DJB2 hash produces a 32-bit integer, which has a high probability of collisions for similar inputs. Because hashKey is used to hash serialized Prisma query arguments (like skip and take), hash collisions will cause distinct database queries to produce the same cache key, leading to incorrect cached results being returned (cache poisoning). Use a cryptographic hash like SHA-256 instead.
Add the crypto import at the top of the file:
import { createHash } from 'crypto';🔒️ Proposed fix to use SHA-256
/**
* Compute a safe hash for a cache key to keep keys within reasonable length.
- * Uses a simple DJB2 hash for performance.
*/
export function hashKey(raw: string): string {
- let hash = 5381;
- for (let i = 0; i < raw.length; i++) {
- hash = ((hash << 5) + hash + raw.charCodeAt(i)) | 0;
- }
- return Math.abs(hash).toString(36);
+ return createHash('sha256').update(raw).digest('hex');
}🤖 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 `@backend/src/utils/cacheUtils.ts` around lines 11 - 21, Replace the 32-bit
DJB2 implementation in hashKey with SHA-256 using createHash from crypto, and
add the import at the top of the file. Hash the raw input with SHA-256 and
return a deterministic encoded digest string suitable for cache keys.
| /** | ||
| * Serialize a value for cache storage. | ||
| */ | ||
| export function serializeValue(value: unknown): string { | ||
| if (value === null || value === undefined) return 'null'; | ||
| if (typeof value === 'string') return value; | ||
| return JSON.stringify(value); | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Remove the string bypass to prevent data integrity issues.
Bypassing JSON.stringify for strings causes severe type corruption upon deserialization. If a string containing numeric digits (e.g., "123") or booleans (e.g., "true") is cached, it is stored exactly as "123". When retrieved, safeJsonParse executes JSON.parse("123") and incorrectly coerces it into a number 123 (or boolean true) instead of the original string. This will cause downstream type errors when the application expects a string.
🐛 Proposed fix
/**
* Serialize a value for cache storage.
*/
export function serializeValue(value: unknown): string {
- if (value === null || value === undefined) return 'null';
- if (typeof value === 'string') return value;
- return JSON.stringify(value);
+ return JSON.stringify(value) ?? 'null';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Serialize a value for cache storage. | |
| */ | |
| export function serializeValue(value: unknown): string { | |
| if (value === null || value === undefined) return 'null'; | |
| if (typeof value === 'string') return value; | |
| return JSON.stringify(value); | |
| } | |
| /** | |
| * Serialize a value for cache storage. | |
| */ | |
| export function serializeValue(value: unknown): string { | |
| return JSON.stringify(value) ?? 'null'; | |
| } |
🤖 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 `@backend/src/utils/cacheUtils.ts` around lines 81 - 89, Update serializeValue
to remove the typeof value === 'string' bypass, ensuring all non-null
values—including strings—are serialized with JSON.stringify before cache
storage. Preserve the existing null and undefined handling so deserialization
retains the original value types.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
18-18: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPrevent credential persistence in CI environments.
By default, the
actions/checkoutstep persists the GitHub auth token in the local git configuration. If subsequent steps upload workspace files or.gitcontents as artifacts, the token could be inadvertently exposed. It is a security best practice to disable this behavior when git write access is not needed.
.github/workflows/test.yml#L18-L18: Addwith: persist-credentials: falseto theactions/checkoutstep..github/workflows/test.yml#L68-L68: Addwith: persist-credentials: falseto theactions/checkoutstep.🛡️ Proposed fixes
For
.github/workflows/test.yml#L18-L18:- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: falseFor
.github/workflows/test.yml#L68-L68:- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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 @.github/workflows/test.yml at line 18, Disable credential persistence for both actions/checkout steps in .github/workflows/test.yml at lines 18-18 and 68-68 by adding the persist-credentials: false setting under each checkout step.Source: Linters/SAST tools
🤖 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 `@backend/src/index.ts`:
- Line 141: Update the initialization flow around cache.startMonitoring() so
monitoring starts regardless of ensureRedisConnection() success or Redis
availability. Invoke startMonitoring independently before the Redis connection
attempt or guarantee it through a finally block, while preserving the existing
Redis fallback behavior.
- Line 20: Ensure dotenv configuration runs before the cache service is imported
or initialized, so cacheConfig reads the intended environment values instead of
module-scope defaults. Update the startup flow around the cache import and
dotenv.config(), or make cacheConfig lazy while preserving existing cache
behavior.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Line 18: Disable credential persistence for both actions/checkout steps in
.github/workflows/test.yml at lines 18-18 and 68-68 by adding the
persist-credentials: false setting under each checkout step.
🪄 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
Run ID: 5edf26b2-fa59-4f0d-9f55-6544bdf35ab1
📒 Files selected for processing (5)
.github/workflows/backend.yml.github/workflows/test.ymlbackend/src/index.tsbackend/src/services/cacheService.tsjest.config.js
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/services/cacheService.ts
| import { validateEnvironment, initializeMonitoring, shutdownMonitoring } from './config/monitoring'; | ||
| import { UserPayload } from './types/auth'; | ||
| import { ensureRedisConnection } from './config/redis'; | ||
| import { cache } from './services/cacheService'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'cacheConfig|process\.env|new MultiLevelCacheService|dotenv\.config' \
backend/src/config/cache.ts \
backend/src/services/cacheService.ts \
backend/src/index.tsRepository: nexoraorg/chenaikit
Length of output: 6248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- backend/src/index.ts (lines 1-140) ---'
sed -n '1,140p' backend/src/index.ts | cat -n
echo
echo '--- backend/src/config/cache.ts (lines 1-140) ---'
sed -n '1,140p' backend/src/config/cache.ts | cat -n
echo
echo '--- backend/src/services/cacheService.ts (lines 1-120, 200-260, 280-330) ---'
{
sed -n '1,120p' backend/src/services/cacheService.ts
echo '---'
sed -n '200,260p' backend/src/services/cacheService.ts
echo '---'
sed -n '280,330p' backend/src/services/cacheService.ts
} | cat -nRepository: nexoraorg/chenaikit
Length of output: 19134
Load dotenv before importing the cache service
backend/src/config/cache.ts reads process.env at module scope, but backend/src/index.ts imports cache before dotenv.config() runs. That freezes .env-backed cache TTL/Redis/monitoring settings at defaults; move env loading ahead of the cache import or make cacheConfig lazy.
🤖 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 `@backend/src/index.ts` at line 20, Ensure dotenv configuration runs before the
cache service is imported or initialized, so cacheConfig reads the intended
environment values instead of module-scope defaults. Update the startup flow
around the cache import and dotenv.config(), or make cacheConfig lazy while
preserving existing cache behavior.
|
|
||
| try { | ||
| await ensureRedisConnection(); | ||
| cache.startMonitoring(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Start cache monitoring even when Redis is unavailable.
startMonitoring() only manages local statistics, so gating it on ensureRedisConnection() disables monitoring during the L1-only fallback mode. Start monitoring independently of Redis readiness, such as before the connection attempt or in a finally 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 `@backend/src/index.ts` at line 141, Update the initialization flow around
cache.startMonitoring() so monitoring starts regardless of
ensureRedisConnection() success or Redis availability. Invoke startMonitoring
independently before the Redis connection attempt or guarantee it through a
finally block, while preserving the existing Redis fallback behavior.
|
@Topmatrixmor2014 fix ci |
|
@Topmatrixmor2014 resolve conflict so i can merge |
…hain.yml, missing type-check scripts, and blockchain test/build scripts
Signed-off-by: Topmatrix Mor <Topmatrixmor2014@gmail.com>
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 `@backend/package.json`:
- Around line 20-21: Update the package.json lint script to quote the
src/**/*.ts glob, ensuring the shell passes it unexpanded to ESLint for
consistent cross-platform file discovery.
🪄 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: 12f87f69-424c-4a06-bd40-b640943b796c
📒 Files selected for processing (7)
.github/workflows/backend.yml.github/workflows/blockchain.ymlbackend/package.jsonfrontend/package.jsonpackage.jsonpackages/cli/package.jsonpackages/core/package.json
| "lint": "eslint src/**/*.ts", | ||
| "type-check": "tsc --noEmit" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json scripts and ESLint config/package existence hints =="
if [ -f backend/package.json ]; then
cat -n backend/package.json | sed -n '1,90p'
fi
echo
echo "== relevant ESLint config files under backend =="
git ls-files backend | grep -E '(^|/)(eslint|\.eslintrc|eslint\.config\.(js|mjs|cjs|ts)|\.eslintignore)$|/package\.json$' | sort
echo
echo "== shell-expansion behavior probe from repo root (simulates npm script /bin/sh invocation) =="
python3 - <<'PY'
import os, subprocess, tempfile
root=os.getcwd()
os.chdir("backend") if os.path.exists("backend/package.json") else os.chdir(root)
print("cwd:", os.getcwd(), "exists:", os.path.exists("package.json"))
files=os.popen("find src -type f -name '*.ts' 2>/dev/null | sort").read().splitlines()
print("tracked TypeScript files:", len(files))
for script in ("eslint src/**/*.ts", r'npm run lint --silent -- src/**/*.ts'):
shell=os.environ.get("SHELL","/bin/sh")
proc=subprocess.run([shell,"-c",script], check=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8')
print("\ncommand: ", script, " shell:", shell)
print("exit:", proc.returncode, "stdout_len:", len(proc.stdout), "stderr_line:", (proc.stderr.splitlines()[0] if proc.stderr.strip() else "")[:160])
PY
echo
echo "== exact string matches in package scripts for lint/type-check =="
python3 - <<'PY'
import json, pathlib
p=pathlib.Path("backend/package.json")
if p.exists():
data=json.loads(p.read_text())
for k,v in data.get("scripts",{}).items():
if "lint" in k or "type-check" in k or k=="lint" or k=="test":
print(k, "=", v)
PYRepository: nexoraorg/chenaikit
Length of output: 4577
🌐 Web query:
npm scripts shell globs starstar pattern quotes npm run lint
💡 Result:
When using glob patterns—especially the double-star "globstar" pattern—in npm scripts, you should always wrap the pattern in double quotes (e.g., "src//.js") [1][2]. The core reason for this requirement is to ensure consistency across different operating systems and shell environments [1]. 1. The Problem: Shell vs. Tool Expansion When you run a command directly in your terminal, your shell (like bash or zsh) often performs "glob expansion" before the command is even executed [1][3][4]. For instance, it replaces src//.js with a list of all matching files. However, npm scripts are executed by a sub-shell (by default, /bin/sh on POSIX and cmd.exe on Windows) [5][6][7]. - If the shell does not support globstar (like older versions of bash or cmd.exe), it may fail to expand the pattern correctly, causing the tool (like ESLint or Mocha) to receive a literal string "/.js" instead of the expected file list [8][1][9][3]. - By quoting the pattern, you prevent the shell from attempting to expand it [1][10][4]. Instead, the tool itself receives the raw glob pattern and uses its own internal library (typically node-glob) to perform the expansion, ensuring identical behavior regardless of the host shell [1][9][11]. 2. Why Double Quotes? While single quotes work in many POSIX shells, they are not universally compatible [10]. Windows command shells (cmd.exe) do not treat single quotes as string delimiters [1][2][10]. Using double quotes ensures that the argument is passed correctly across both Unix-like systems and Windows environments [1][2][10]. 3. Best Practices - Always quote your globs: Use "pattern//*.js" within your package.json scripts [1]. - Rely on the tool: Prefer tools that handle their own glob expansion (most modern JS tooling like ESLint, Mocha, and Jest do this) [1][9][11]. - Cross-platform tools: For operations like file deletion or copying, consider cross-platform wrappers like shx, which are designed to handle these shell-related pitfalls automatically [10]. Example of a corrected lint script: "scripts": { "lint": "eslint "src//.js"" } If you encounter issues where a tool does not support glob expansion internally, you may need to configure the npm script shell using the script-shell setting (e.g., npm config set script-shell=zsh), though this is generally discouraged as it sacrifices cross-platform compatibility [9][3][7].
Citations:
- 1: https://medium.com/@jakubsynowiec/you-should-always-quote-your-globs-in-npm-scripts-621887a2a784
- 2: Glob arg behaves differently if ran from npm script eslint/eslint#5796
- 3: https://stackoverflow.com/questions/72262156/using-glob-in-yarn-npm-script
- 4: https://github.com/kayahr/npm-utils
- 5: https://docs.npmjs.com/cli/v11/commands/npm-run-script/
- 6: https://docs.npmjs.com/cli/v9/commands/npm-run-script/?v=true
- 7: https://exploringjs.com/nodejs-shell-scripting/ch_package-scripts.html
- 8:
npm run scriptfile glob not respecting**npm/npm#10481 - 9: https://gist.github.com/khalidx/70f14efa145da4e8d6c4ffb9cb9911bb
- 10: https://github.com/shelljs/shx/
- 11: https://eslint.org/docs/latest/use/command-line-interface
Quote the ESLint glob for cross-platform lint coverage.
eslint src/**/*.ts is expanded by the shell before ESLint sees it; different shells can produce different or incomplete file lists. Pass the pattern unexpanded so ESLint expands it consistently.
Proposed fix
- "lint": "eslint src/**/*.ts",
+ "lint": "eslint \"src/**/*.ts\"",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "lint": "eslint src/**/*.ts", | |
| "type-check": "tsc --noEmit" | |
| "lint": "eslint \"src/**/*.ts\"", | |
| "type-check": "tsc --noEmit" |
🤖 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 `@backend/package.json` around lines 20 - 21, Update the package.json lint
script to quote the src/**/*.ts glob, ensuring the shell passes it unexpanded to
ESLint for consistent cross-platform file discovery.
2 participants
…ue #169)
Overview
Implements a full caching layer for the backend with multi-level caching (L1: in-memory node-cache, L2: Redis), cache tags, statistics monitoring, cache warming, Prisma query cache middleware, and fallback support.
Changes
New Files
Modified Files
backend/src/types/cache.ts — Added interfaces: CacheGetOptions, CacheSetOptions, CacheStats, CacheServiceWithStats, CacheServiceWithTags, CacheServiceWithWarming, MultiLevelCacheService, PrismaCacheMiddlewareOptions
backend/src/services/cacheService.ts — Major enhancement:
backend/src/middleware/cache.ts — Enhanced middleware:
backend/src/utils/cacheKeys.ts — Comprehensive key patterns for: accounts, credit scores, fraud alerts, transactions, analytics, health/metrics, users/sessions, API keys, feature flags
Test Updates
Dependencies
Acceptance Criteria
Closes #169
Closes #169
Summary by CodeRabbit
type-checkscripts and backend lint/type-check; refreshed CI action versions and Rust setup.