(SP: 3) [Backend] Redis Cache for Quiz Questions#293
(SP: 3) [Backend] Redis Cache for Quiz Questions#293ViktorSvertoka merged 10 commits intodevelopfrom
Conversation
Release v0.5.4
- Create types/quiz.ts with shared quiz domain types - Add getOrCreateQuestionsCache() to cache questions per quiz/locale - Integrate cache into getQuizQuestions() with DB fallback
- Cache empty quiz results to prevent repeated DB queries - Add try/catch around redis.set() for graceful degradation - Redis failures no longer break requests when DB data is available
- Remove /api/quiz/[slug] route (unused, exposed isCorrect) - Remove related test file quiz-slug-route.test.ts
Add error handling to redis.get() calls in: - getOrCreateQuizAnswersCache - getCorrectAnswer - getOrCreateQuestionsCache
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds locale-aware Redis caching for quiz questions: new shared types for quiz questions/answers, a getOrCreateQuestionsCache(quizId, locale) function that assembles and caches questions with answers, and integration of that cache into getQuizQuestions() to prefer cached results before querying the DB. It also removes an API route and its tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant App as getQuizQuestions()
participant Cache as getOrCreateQuestionsCache()
participant Redis
participant DB
Client->>App: getQuizQuestions(quizId, locale)
App->>Cache: getOrCreateQuestionsCache(quizId, locale)
Cache->>Redis: GET quiz:questions:{quizId}:{locale}
alt Cache Hit
Redis-->>Cache: cached questions with answers
Cache-->>App: return cached data
else Cache Miss
Cache->>DB: query localized questions, contents, and answers
DB-->>Cache: question & answer rows
Cache->>Cache: assemble QuizQuestionWithAnswers[]
Cache->>Redis: SET quiz:questions:{quizId}:{locale}
Cache-->>App: return assembled data
end
App-->>Client: quiz questions with answers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/app/layout.tsx (1)
1-7:⚠️ Potential issue | 🟡 MinorDuplicate
globals.cssimport.
./globals.cssis imported on both Line 1 and Line 7. Remove one of the duplicates.Proposed fix
import './globals.css'; import type { Metadata } from 'next'; import { Geist, Geist_Mono } from 'next/font/google'; import { Analytics } from '@vercel/analytics/next'; import { SpeedInsights } from '@vercel/speed-insights/next'; -import './globals.css';frontend/lib/quiz/quiz-answers-redis.ts (1)
70-76:⚠️ Potential issue | 🟠 MajorSame unhandled
redis.setconcern ingetOrCreateQuizAnswersCache.Line 76 has the same issue: if
redis.setthrows, the caller gets an error even though the answers were successfully queried. The same try/catch pattern should be applied here for resilience.🛡️ Proposed fix
- await redis.set(key, cacheData); - return true; + try { + await redis.set(key, cacheData); + } catch (err) { + console.warn('Failed to cache quiz answers in Redis', err); + } + return true;
🤖 Fix all issues with AI agents
In `@frontend/lib/quiz/quiz-answers-redis.ts`:
- Around line 145-147: The function currently returns early on an empty
questionsData (if (questionsData.length === 0) return [];) which bypasses Redis
and causes repeated DB hits; instead, write an empty result into Redis before
returning — use the same cache key generation used elsewhere in this module (the
code path that creates the Redis key for this quiz ID) and set an appropriate
TTL (either the normal TTL or a shorter negative-cache TTL) so subsequent calls
hit the cache; update the early-return branch to call the module's Redis write
method (e.g., the existing redis.set/client.set wrapper) with JSON-serialized []
and then return [].
- Around line 188-195: The redis.set call can throw and should not break the
request flow; wrap the await redis.set(key, cacheData) in a try/catch so Redis
failures degrade gracefully: keep the cacheData creation (QuizQuestionsCache)
and successful return path unchanged, but catch any error from redis.set, log it
(using the existing logger if available or console.warn) with context including
key/quizId/locale, and do not rethrow the error so callers still receive the
DB-fetched data.
🧹 Nitpick comments (2)
frontend/types/quiz.ts (1)
1-7: Consider narrowingexplanation: anyto a more specific type.Using
anyforexplanationbypasses type checking entirely and can silently propagate unexpected data shapes. If the DB column stores JSON, considerRecord<string, unknown> | nullor a dedicated interface. If it's always a string, usestring | null.♻️ Suggested type narrowing
export interface QuizQuestion { id: string; displayOrder: number; difficulty: string | null; questionText: string | null; - explanation: any; + explanation: Record<string, unknown> | string | null; }frontend/lib/quiz/quiz-answers-redis.ts (1)
126-186: Duplicated DB query logic withgetQuizQuestionsinfrontend/db/queries/quiz.ts(lines 187–244).The questions + answers fetch-and-assemble logic is nearly identical in both locations. If the schema or query shape changes, both copies must be updated in lockstep. Consider extracting a shared helper (e.g.,
fetchQuestionsFromDb(quizId, locale)) used by both the cache-populating path and the DB fallback.
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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)
frontend/lib/quiz/quiz-answers-redis.ts (1)
76-76:⚠️ Potential issue | 🟠 Major
redis.setnot wrapped in try/catch — same risk fixed ingetOrCreateQuestionsCache.The new function at Lines 206-210 correctly wraps
redis.setin try/catch (per prior review feedback), but this older call site has the identical risk: a Redis write failure will propagate the exception and break the request despite having valid DB data.🛡️ Proposed fix
- await redis.set(key, cacheData); - return true; + try { + await redis.set(key, cacheData); + } catch (e) { + console.warn('Redis cache write failed:', e); + } + return true;
🤖 Fix all issues with AI agents
In `@frontend/lib/quiz/quiz-answers-redis.ts`:
- Around line 189-197: The API response currently exposes answer correctness in
frontend/app/api/quiz/[slug]/route.ts where answers are built using
q.answers.map; remove the isCorrect property from that mapping so the returned
objects only include id and text (or answerText) and do not expose correctness
to the client, leaving correctness checks to the verify endpoint
(frontend/app/api/quiz/verify-answer/route.ts); update the mapping in the
handler that constructs answers (the q.answers.map callback) to omit isCorrect.
🧹 Nitpick comments (3)
frontend/lib/quiz/quiz-answers-redis.ts (3)
1-1: Minor formatting: missing space after comma.
eq,inArray→eq, inArray
145-157: Inconsistent indentation in the empty-results block.Line 145 (
if (questionsData.length === 0)) has extra leading whitespace compared to the rest of the function body. This looks like a copy-paste artifact.🧹 Fix indentation
- if (questionsData.length === 0) { - const cacheData: QuizQuestionsCache = { - quizId, - locale, - questions: [], - cachedAt: Date.now(), - }; - try { - await redis.set(key, cacheData); - } catch (e) { - console.warn('Redis cache write failed:', e); - } - return []; - } + if (questionsData.length === 0) { + const cacheData: QuizQuestionsCache = { + quizId, + locale, + questions: [], + cachedAt: Date.now(), + }; + try { + await redis.set(key, cacheData); + } catch (e) { + console.warn('Redis cache write failed:', e); + } + return []; + }
206-210: Indentation inside try block is off.Line 207 (
await redis.set(key, cacheData)) is not indented relative to its enclosingtryblock.🧹 Fix indentation
try { - await redis.set(key, cacheData); + await redis.set(key, cacheData); } catch (e) {
| const questions: QuizQuestionWithAnswers[] = questionsData.map(q => ({ | ||
| ...q, | ||
| answers: (answersByQuestion.get(q.id) || []).map(a => ({ | ||
| id: a.id, | ||
| displayOrder: a.displayOrder, | ||
| isCorrect: a.isCorrect, | ||
| answerText: a.answerText, | ||
| })), | ||
| })); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how getOrCreateQuestionsCache results are used and whether isCorrect is stripped
rg -n -C10 'getOrCreateQuestionsCache' --type=tsRepository: DevLoversTeam/devlovers.net
Length of output: 3613
🏁 Script executed:
#!/bin/bash
# Check if isCorrect appears in any API route or page component response
rg -n 'isCorrect' --type=ts -g '!**/quiz-answers-redis.ts'Repository: DevLoversTeam/devlovers.net
Length of output: 9115
🏁 Script executed:
#!/bin/bash
# Look for getQuizQuestions function in db/queries/quiz.ts
find . -name "quiz.ts" -path "*/db/queries/*" -type fRepository: DevLoversTeam/devlovers.net
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Check the full getQuizQuestions function implementation
sed -n '179,250p' frontend/db/queries/quiz.tsRepository: DevLoversTeam/devlovers.net
Length of output: 2197
🏁 Script executed:
#!/bin/bash
# Check the API route at frontend/app/api/quiz/[slug]/route.ts
cat frontend/app/api/quiz/[slug]/route.tsRepository: DevLoversTeam/devlovers.net
Length of output: 1470
🏁 Script executed:
#!/bin/bash
# Check how QuizQuestion component receives and uses isCorrect
head -100 frontend/components/quiz/QuizQuestion.tsxRepository: DevLoversTeam/devlovers.net
Length of output: 3468
Remove isCorrect from the quiz API response before sending to the client.
The API route at frontend/app/api/quiz/[slug]/route.ts (line 29) includes isCorrect in the JSON response, exposing all correct answers to the client. Users can inspect the network response to discover correct answers before answering the quiz, defeating the purpose of the quiz.
Location: frontend/app/api/quiz/[slug]/route.ts lines 20-29
answers: q.answers.map(a => ({
id: a.id,
text: a.answerText,
isCorrect: a.isCorrect,
})),
Remove the isCorrect field from the answers mapping in the API response. The isCorrect verification should only be returned by the frontend/app/api/quiz/verify-answer/route.ts endpoint after the user submits an answer, not in the initial questions fetch.
🤖 Prompt for AI Agents
In `@frontend/lib/quiz/quiz-answers-redis.ts` around lines 189 - 197, The API
response currently exposes answer correctness in
frontend/app/api/quiz/[slug]/route.ts where answers are built using
q.answers.map; remove the isCorrect property from that mapping so the returned
objects only include id and text (or answerText) and do not expose correctness
to the client, leaving correctness checks to the verify endpoint
(frontend/app/api/quiz/verify-answer/route.ts); update the mapping in the
handler that constructs answers (the q.answers.map callback) to omit isCorrect.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/lib/quiz/quiz-answers-redis.ts`:
- Around line 115-129: Wrap the redis.get call in getOrCreateQuestionsCache in a
try/catch so any exception (deserialization, timeout, etc.) is swallowed and the
function returns null (preserving the graceful-fallback contract); specifically,
catch errors around the call to redis.get<QuizQuestionsCache>(key) inside
getOrCreateQuestionsCache and on error log/debug as needed but return null.
Apply the same pattern to the redis.get usages in getOrCreateQuizAnswersCache
and getCorrectAnswer so those functions also return null on redis.get failures
instead of letting exceptions propagate.
🧹 Nitpick comments (3)
frontend/lib/quiz/quiz-answers-redis.ts (3)
1-1: Nit: missing space after comma.-import { and, eq,inArray } from 'drizzle-orm'; +import { and, eq, inArray } from 'drizzle-orm';
150-163: Inconsistent indentation inside the empty-result branch.Line 150 has 4 spaces of leading indentation for the
ifstatement, while the rest of the function body uses 2 spaces. Similarly, thetry/catchblock within (lines 157–161) is at the correct level but theifline itself is off. This likely happened during a fixup commit.- if (questionsData.length === 0) { + if (questionsData.length === 0) {
211-215:await redis.setis under-indented inside thetryblock.Line 212 should be indented one level deeper than the
tryon line 211.try { - await redis.set(key, cacheData); + await redis.set(key, cacheData); } catch (e) {
2 participants
#Closes #291
Summary
types/quiz.tsfor quiz domain typesgetQuizQuestions()with fallback to DB if Redis unavailableTest plan
Summary by CodeRabbit
New Features
Chores
Tests