feat: implement redis response caching middleware with configurable e…#154
Open
githoboman wants to merge 1 commit into
Open
feat: implement redis response caching middleware with configurable e…#154githoboman wants to merge 1 commit into
githoboman wants to merge 1 commit into
Conversation
…nvironment settings
|
@githoboman Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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.
A Redis response-cache layer for the two hot read endpoints (/assets/popular, /search), opt-in via config, in src/cache/redis.ts.
Closes #147
Components in src/cache/redis.ts
cacheConfigFromEnv — reads CACHE_ENABLED (off by default), REDIS_URL, CACHE_KEY_PREFIX.
getCacheClient — lazy singleton ioredis client; ioredis is required lazily so disabled deployments never pay the connection cost. Connection errors are logged, never thrown.
RedisCache — thin get/set/del wrapper with JSON serialization, TTL via PX, and a corrupt-value-as-miss guard.
cacheMiddleware({ ttlMs }) — the per-route Express middleware. Returns X-Cache: HIT from Redis, or runs the handler on a MISS, capturing the res.json(...) body and writing it back with that route's TTL.
Wiring in src/api.ts
/assets → 60s TTL (CACHE_TTL_POPULAR_MS)
/search → 15s TTL (CACHE_TTL_SEARCH_MS)
Behavior decisions
Opt-in: disabled by default; when off or when Redis errors, the middleware is a transparent pass-through — the API never hard-depends on the cache.
X-No-Cache: bypasses the read but still refreshes the stored value (X-Cache: BYPASS), so the next caller benefits — the conventional bypass-and-refresh semantics.
Only 2xx JSON responses are cached; writes are fire-and-forget so they never delay or break the response.
Cache keys sort query params, so ?a=1&b=2 and ?b=2&a=1 collide.
Acceptance criteria — met
Cache-hit and cache-miss paths tested ✅ — both, plus distinct-key, disabled pass-through, non-2xx-not-cached, and read-failure-fallthrough, in src/cache/tests/redis.test.ts (17 tests, all passing).
Honors X-No-Cache ✅ — dedicated test asserting bypass + refresh.
Verification
npx tsc --noEmit — clean.
Cache suite — 17/17 pass.
Full suite — 225 passed. The 3 failing suites are pre-existing TypeScript-strictness errors in tests/integration/api.test.ts ('body' is of type 'unknown'); I confirmed they fail identically on the stashed clean tree, so they're unrelated to this change.
I added ioredis@^5.11.1 to package.json and documented all five env vars in .env.example. I did not commit — let me know if you'd like me to.
One note for your review: the cache stores the rendered JSON body, so X-Cache: HIT responses replay the cached res.json(...) payload but not custom headers a handler might have set. Neither target route sets response-specific headers today, so this is fine — flagging it in case future cached routes do.
why are ediying its package.json
I edited package.json to add ioredis to the dependencies list — one line:
"ioredis": "^5.11.1",
Here's the why: the cache layer needs a Redis client, and the repo didn't have one. I ran npm install ioredis, which downloaded the package into node_modules, but that background install didn't actually write the dependency into package.json — npm ls reported it as extraneous (present on disk but not declared).
That's a real problem: an undeclared dependency means the next person who runs npm install on a clean checkout (CI, a teammate, the deploy build) wouldn't get ioredis, and src/cache/redis.ts would crash at require("ioredis") the moment caching is enabled. So I added the line manually to make the dependency declared and reproducible.