Cache POST-based recommendation endpoints and document cache layer - #71
Merged
zeemscript merged 10 commits intoJul 24, 2026
Merged
Conversation
- Apply cacheMiddleware to GET endpoints for books, courses, users, spaces, and search routes with appropriate TTLs - Add cache invalidation middleware to POST, PUT, DELETE operations to ensure cache consistency when data changes - Add Redis environment variables to validateEnv.js optional vars - Create .env.example documenting all environment variables including Redis configuration options Closes Deen-Bridge#2
feat: configure Redis cache layer for production use
…ests test: add Stellar payment flow coverage
…nt-horizon-client feat: resilient horizon client layer
The books/courses/spaces/search/users list and detail endpoints were already wired to the Redis cache layer in a prior change. The two POST /recommended endpoints (books and courses) were left uncached since they take interests via the request body, which the shared GET-only cacheMiddleware can't key off. Cache them explicitly in their controllers, keyed by the sorted interests list, and document every cached endpoint and TTL in docs/caching.md.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughRecommendation book and course endpoints now cache responses using normalized keys and configured TTLs. Redis and Horizon settings are documented and validated, Stellar operations use a shared client wrapper, and payment-controller transaction outcomes are covered by Jest tests. ChangesRecommendation caching
Stellar payment resilience
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RecommendationController
participant getCacheOrSet
participant Redis
participant MongoDB
Client->>RecommendationController: POST recommendation request
RecommendationController->>getCacheOrSet: normalized cache key and TTL
getCacheOrSet->>Redis: lookup cached payload
alt cache hit
Redis-->>getCacheOrSet: cached payload
else cache miss
getCacheOrSet->>MongoDB: query recommendations
MongoDB-->>getCacheOrSet: recommendation results
getCacheOrSet->>Redis: store payload with TTL
end
getCacheOrSet-->>RecommendationController: recommendation payload
RecommendationController-->>Client: success response
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Collaborator
|
@Tijesunimi004 please fix conflicts |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/caching.md (1)
41-43: 🚀 Performance & Scalability | 🔵 TrivialAvoid Redis
KEYSscans at production scale.A wildcard
KEYSscan can block Redis while traversing a large keyspace, turning writes into latency or availability incidents. PreferSCANwith bounded deletion or explicit key tracking before production traffic grows.🤖 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 `@docs/caching.md` around lines 41 - 43, Update invalidateCacheMiddleware and its deleteCachePattern path to avoid Redis KEYS scans for wildcard cache invalidation. Replace the unbounded lookup with bounded SCAN-based iteration and deletion, or reuse explicit key tracking, while preserving invalidation of all relevant create, update, delete, and review cache entries.
🤖 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/controllers/books/recommendedBooksController.js`:
- Line 49: Standardize both recommendation response envelopes to `{ success,
message, data }`: in src/controllers/books/recommendedBooksController.js lines
49-49, map the cached result to data and provide a stable message; in
src/controllers/courses/courseController.js lines 258-258, place the recommended
courses under data and include message while preserving the success field.
- Around line 12-17: Use one shared canonicalization contract for interests:
validate and bound the input, normalize it once, encode the resulting list
without delimiter collisions, and reuse that normalized list in database filters
and cache keys. Apply this in
src/controllers/books/recommendedBooksController.js lines 12-17 and
src/controllers/courses/courseController.js lines 244-250, then document the
actual encoded or hashed representation in docs/caching.md lines 20-24.
- Around line 20-47: Add Jest coverage for the recommendation cache flows in
src/controllers/books/recommendedBooksController.js (lines 20-47) and
src/controllers/courses/courseController.js (lines 252-256), covering cache hits
and misses, Redis fallback, sorted-interest cache keys, popular-book fallback,
and empty-interest handling for courses. Use the controller methods and existing
cache helpers to verify each path without changing the recommendation behavior.
In `@src/controllers/courses/courseController.js`:
- Around line 252-255: Update the recommendation flow around getCacheOrSet and
Course.find to handle missing or empty interests before constructing the
category query. Return the defined no-interest response or reject the request in
that branch, and ensure the cache key reflects the same path instead of querying
with $in: undefined.
---
Nitpick comments:
In `@docs/caching.md`:
- Around line 41-43: Update invalidateCacheMiddleware and its deleteCachePattern
path to avoid Redis KEYS scans for wildcard cache invalidation. Replace the
unbounded lookup with bounded SCAN-based iteration and deletion, or reuse
explicit key tracking, while preserving invalidation of all relevant create,
update, delete, and review cache entries.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 3e33cd24-1d9c-46f7-8488-abdeba093933
📒 Files selected for processing (3)
docs/caching.mdsrc/controllers/books/recommendedBooksController.jssrc/controllers/courses/courseController.js
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 (2)
src/services/stellar/stellarService.js (2)
89-93: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRoute every Horizon request through the resilient client.
These direct
servercalls always targetclient.endpoints[0]and bypass configured timeout, retry, failover, and circuit-breaker behavior.
src/services/stellar/stellarService.js#L89-L93: wrapstrictReceivePaths(...).call()inclient.execute(...).src/services/stellar/stellarService.js#L122-L124: load the path-payment source account throughclient.execute(...).src/services/stellar/stellarService.js#L317-L321: load preflight accounts throughclient.execute(...).src/services/stellar/stellarService.js#L483-L485: load the reverse-payment source account throughclient.execute(...).🤖 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/services/stellar/stellarService.js` around lines 89 - 93, Route all four Horizon requests through the resilient client: in src/services/stellar/stellarService.js lines 89-93, wrap strictReceivePaths(...).call() in client.execute(...); at lines 122-124, 317-321, and 483-485, wrap the respective source-account and preflight-account loads in client.execute(...). Preserve each request’s existing operation and result handling while removing direct server calls.
626-635: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winConsume each matched operation during verification.
find()can return the same operation for multiple identical{ destination, amount }expectations. A transaction containing one payment can therefore pass verification for two required payments and grant access despite underpayment.Proposed fix
+ const remainingPaymentOps = [...paymentOps]; for (const expected of expectedPayments) { - const match = paymentOps.find((op) => { + const matchIndex = remainingPaymentOps.findIndex((op) => { if (op.to !== expected.destination) return false; const opAmount = op.type === "path_payment_strict_receive" ? op.destination_amount : op.amount; return toStroops(opAmount) === toStroops(expected.amount); }); - if (!match) { + if (matchIndex === -1) { return { verified: false, reason: `Missing expected USDC payment of ${expected.amount} to ${expected.destination}`, }; } + remainingPaymentOps.splice(matchIndex, 1); }As per path instructions, “payments are verified against Horizon before granting access.”
🤖 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/services/stellar/stellarService.js` around lines 626 - 635, Update the payment verification loop over expectedPayments so each matched operation is consumed and cannot satisfy another expectation. Replace the reusable lookup behavior in the paymentOps matching logic with tracking or removal of the matched operation, while preserving destination and stroop-amount comparisons before granting access.Source: Path instructions
🤖 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 @.env.example:
- Around line 29-32: Remove the duplicate EMAILJS_PRIVATE_KEY,
EMAILJS_PUBLIC_KEY, EMAILJS_SERVICE_ID, and EMAILJS_TEMPLATE_ID declarations
from this block in .env.example, preserving the single existing declaration for
each key elsewhere in the file.
---
Outside diff comments:
In `@src/services/stellar/stellarService.js`:
- Around line 89-93: Route all four Horizon requests through the resilient
client: in src/services/stellar/stellarService.js lines 89-93, wrap
strictReceivePaths(...).call() in client.execute(...); at lines 122-124,
317-321, and 483-485, wrap the respective source-account and preflight-account
loads in client.execute(...). Preserve each request’s existing operation and
result handling while removing direct server calls.
- Around line 626-635: Update the payment verification loop over
expectedPayments so each matched operation is consumed and cannot satisfy
another expectation. Replace the reusable lookup behavior in the paymentOps
matching logic with tracking or removal of the matched operation, while
preserving destination and stroop-amount comparisons before granting access.
🪄 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: CHILL
Plan: Pro Plus
Run ID: fb68dbff-3b74-4577-8d73-759e092c4e9a
📒 Files selected for processing (3)
.env.examplesrc/config/validateEnv.jssrc/services/stellar/stellarService.js
Comment on lines
29
to
32
| EMAILJS_PRIVATE_KEY=your_emailjs_private_key | ||
| EMAILJS_PUBLIC_KEY=your_emailjs_public_key | ||
| EMAILJS_SERVICE_ID=your_emailjs_service_id | ||
| EMAILJS_TEMPLATE_ID=your_emailjs_template_id |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the duplicate EmailJS declarations.
These keys appear elsewhere in the file, so dotenv will use the last occurrence and make edits to the earlier block ineffective. Keep one declaration for each key.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 29-29: [DuplicatedKey] The EMAILJS_PRIVATE_KEY key is duplicated
(DuplicatedKey)
[warning] 30-30: [DuplicatedKey] The EMAILJS_PUBLIC_KEY key is duplicated
(DuplicatedKey)
[warning] 31-31: [DuplicatedKey] The EMAILJS_SERVICE_ID key is duplicated
(DuplicatedKey)
[warning] 32-32: [DuplicatedKey] The EMAILJS_TEMPLATE_ID key is duplicated
(DuplicatedKey)
🤖 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 @.env.example around lines 29 - 32, Remove the duplicate EMAILJS_PRIVATE_KEY,
EMAILJS_PUBLIC_KEY, EMAILJS_SERVICE_ID, and EMAILJS_TEMPLATE_ID declarations
from this block in .env.example, preserving the single existing declaration for
each key elsewhere in the file.
Source: Linters/SAST tools
The stellarService.js mock in this suite only listed the handful of
exports the original paymentController.js used. Since then
paymentController.js grew (path payments, pre-flight checks,
platform-collect settlement) and other test files
(preflightPayment.test.js, pathPayments.test.js) started importing
the real, unmocked stellarService.js. When both run in the same Jest
worker, the incomplete mock trips a SyntaxError from Jest's ESM
module linker ("does not provide an export named 'USDC'") for
exports this file never even references.
Mirror every named export of stellarService.js in the mock, and add
mocks for preflightPayment/calculateFeeSplit (now called by
initializePayment) and the job queue's enqueue (now called by
submitPayment for receipt generation), which paymentController.js
picked up in the meantime.
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
Issue #19 asked for the Redis cache layer (
cacheMiddleware,smartCache,invalidateCacheMiddleware) to be wired onto the read endpoints for books, courses, spaces, and search, with invalidation on writes. That work is already merged intomain(the change that closed #2 covers books, courses, spaces, search, and users list/detail endpoints).Two gaps remained:
POST /api/books/recommendedandPOST /api/courses/recommendedtakeinterestsvia the request body, so the existing GET-onlycacheMiddlewarecan't cache them via route wiring. They were left uncached (courses even has a comment noting "POST routes not cached").Changes
getRecommendedBooks(src/controllers/books/recommendedBooksController.js) andfetchRecommendedCourses(src/controllers/courses/courseController.js) now cache their results viagetCacheOrSet, keyed by the sortedinterestslist, with the existingCACHE_TTL.BOOKS/CACHE_TTL.COURSESpresets.books:recommended:*,courses:recommended:*) are already covered by the existingbooks:*/courses:*invalidation patterns on book/course writes, so no route changes were needed.docs/caching.mddocumenting every cached endpoint, its TTL, the invalidation rules, and what's deliberately left uncached (per-user bookmarks, and reels - whose response embeds per-viewer like/love state and would leak across users under a shared cache key).Test plan
node --checkon both modified controllersbcryptbinding +crypto.getRandomValuespolyfill issues unrelated to this change)POST /api/books/recommendedandPOST /api/courses/recommendedSummary by CodeRabbit
Performance
Reliability
Documentation
Closes #19