feat: Redis caching layer with tests

[JoshdfG]

Closes #11

[DeFiVC]

PR Review: feat: Redis caching layer with tests

Author: JoshdfG (Josh_dfG) | Closes: #11 | State: OPEN | Mergeable: CONFLICTING (DIRTY)

Review Checklist

Item	Status
Linked Issue	✅ Closes #11 — matches requirements
Code Quality	✅ Well-implemented caching layer
CI Status	✅ Lint & Typecheck: SUCCESS, Test: SUCCESS
Merge Conflicts	❌ Has merge conflicts (must resolve)

Blocking Issues

❌ Merge Conflicts — src/modules/courses/course.service.ts has conflicts with main

The PR was created before PR #53 (the inArray fix) was merged. The conflict is in the listCourses method where the enrollment count query was updated.

To resolve, run these commands:

# Fetch latest main
git fetch origin main

# Rebase your branch onto main
git checkout caching_layer
git rebase origin/main

# Resolve the conflict in src/modules/courses/course.service.ts:
# - Keep the caching layer structure from your PR
# - Make sure the enrollment count query uses inArray(enrollments.courseId, courseIds)
#   instead of eq(enrollments.courseId, courseIds[0])

# After resolving, continue the rebase
git add src/modules/courses/course.service.ts
git rebase --continue

# Force push to update the PR
git push origin caching_layer --force-with-lease

The conflict is specifically in the listCourses method. Your PR has the caching layer wrapping the original code, but main now has the inArray fix from PR #53. You need to merge both changes:

1. Keep your caching structure (if (!cachedData) { ... })
2. Inside the cache miss block, use inArray(enrollments.courseId, courseIds) instead of eq(enrollments.courseId, courseIds[0])

Code Issues

1. cacheInvalidatePattern uses SCAN correctly — src/cache/index.ts:80-95
Good use of SCAN instead of KEYS * for production safety.

2. Cache TTLs match the issue spec — All TTL values align with the recommended strategy:

• Course listing: 30s ✅
• Course detail: 120s ✅
• User progress: 10s ✅
• Reward history: 30s ✅
• User profile: 300s ✅

3. Cache warming implementation — src/cache/warmer.ts
Good implementation with error handling and periodic warming every 5 minutes.

Minor Nits

• Makefile is minimal — Only has one target. Consider adding more dev targets if needed, or remove if not necessary for the PR scope.

What's Good

• Excellent implementation of cache-aside pattern with graceful degradation
• Proper cache invalidation on mutations (enroll, claim reward, mint credential)
• Cache metrics (hits/misses) are properly tracked with Prometheus counters
• Comprehensive test suite covering cache hits, misses, invalidation, and metrics
• Cache warming for course listings is a nice touch for startup performance
• The cacheInvalidatePattern using SCAN is production-safe

Decision: REQUEST CHANGES

The merge conflicts must be resolved before this PR can be merged. The code quality is excellent and the implementation matches the issue requirements.

[DeFiVC]

Re-review: feat: Redis caching layer with tests (#54)

Merge conflicts resolved — Contributor merged main into caching_layer (commit 313c39f). CI now passing.

Updated Checklist

Item	Status
Linked Issue	✅ Closes #11 — matches requirements
Code Quality	✅ Well-implemented caching layer
CI Status	✅ Lint & Typecheck: SUCCESS, Test: SUCCESS
Merge Conflicts	✅ Resolved — now MERGEABLE

What's Good

• Cache-aside pattern with proper TTLs per data type
• Graceful degradation on Redis failures
• cacheInvalidatePattern uses SCAN (production-safe)
• Proper invalidation on all mutation paths
• Comprehensive test suite

Minor Nits (non-blocking)

• src/cache/warmer.ts — setInterval not cleared on shutdown (minor resource leak)
• src/cache/index.ts:17-18 — CacheOptions.prefix defined but unused
• src/cache/index.ts:8-16 — Metrics use default register; verify alignment with existing prom-client setup

Decision: APPROVE — All blocking issues resolved. Code quality excellent.