diff --git a/.claude/rules/server-cache.md b/.claude/rules/server-cache.md new file mode 100644 index 000000000..f52599379 --- /dev/null +++ b/.claude/rules/server-cache.md @@ -0,0 +1,98 @@ +--- +description: Server-side caching rules +paths: + - 'src/lib/clients/cache.ts' + - 'src/lib/server/**/cache.ts' + - 'src/features/**/server/cache.ts' +--- + +# Server-Side Cache + +## Core Pattern: `Cache.getOrFetch()` + +Use `cache.getOrFetch(key, fetchFn)` for all get-or-fetch operations. Never inline the get/set/if pattern manually — it duplicates logic that `getOrFetch` already provides. + +```typescript +export function getCachedFoo(fetchFn: () => Promise): Promise { + return fooCache.getOrFetch(KEY, fetchFn); +} +``` + +## Domain Cache Module Structure + +Place cache modules at `server/cache.ts` within each domain: + +| Domain | Path | +| ------- | ------------------------------------- | +| Shared | `src/lib/server/{domain}/cache.ts` | +| Feature | `src/features/{name}/server/cache.ts` | + +Each module exports: + +- `getCached*()` — thin wrapper around `cache.getOrFetch()` +- `invalidate*Caches()` — clears all related cache instances +- `dispose*Caches()` — disposes all related cache instances (for test cleanup) + +## Invalidation Rules + +- Call `invalidate*Caches()` immediately after the DB write (`create`, `update`, `delete`) succeeds — not before, not in a `finally` block. +- Group related caches in a single invalidation function (e.g., `invalidateTaskCaches()` clears both `tasksCache` and `mergedTasksCache`). +- Never invalidate from route handlers — invalidation belongs in the service layer, co-located with the write operation. + +## TTL Guidelines + +| Data characteristics | TTL | +| ------------------------------ | ---------- | +| Rarely changes (tasks, grades) | 1 hour | +| Moderately changes (votes) | 10 minutes | + +Adjust based on production metrics after deployment. + +## Testing + +Core cache behavior (hit, miss, TTL, error propagation) is tested on `Cache.getOrFetch()` in `src/lib/clients/cache.test.ts`. Domain cache tests cover only domain-specific concerns: + +- **Wiring**: wrapper delegates correctly and returns cached value on subsequent calls +- **Key isolation**: different parameters produce different cache keys (e.g., `buildPlacementKey`) +- **Invalidation grouping**: `invalidate*()` clears all related caches + +Do not duplicate TTL or error propagation tests in domain cache test files. + +Always call `afterAll(() => dispose*Caches())` to prevent timer leaks. Isolate tests with `beforeEach(() => invalidate*Caches())`. + +## Type Constraint + +`Cache` — never use `null` or `undefined` as `T`. The cache uses `=== undefined` to detect misses; storing `undefined` would silently bypass the cache on every call. + +## Stampede Prevention + +`Cache.getOrFetch()` shares a single in-flight `Promise` across concurrent callers for the same key. Do not implement manual dedup on top of it. + +## Invalidation Audit + +When adding a new DB write function, audit every write path in the same domain for missing `invalidate*Caches()` calls. Initial implementation of `upsertVoteGradeTables()` missed this and served stale data for up to 10 minutes. + +## Service Layer Integration + +Services call `getCached*()` with a `fetchFn` that performs the DB query. The service does not import `Cache` directly. + +```typescript +// In service file +import { getCachedTasksMap } from '$lib/server/tasks/cache'; + +export async function getTasksByTaskId(): Promise> { + return getCachedTasksMap(async () => { + const tasks = await db.task.findMany(); + return new Map(tasks.map((task) => [task.task_id, task])); + }); +} +``` + +Mock cache in service tests to bypass caching: + +```typescript +vi.mock('$lib/server/tasks/cache', () => ({ + getCachedTasksMap: (fetchFn: () => Promise) => fetchFn(), + invalidateTaskCaches: vi.fn(), +})); +``` diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 044a12378..75577c16d 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -78,6 +78,24 @@ const mockFindUnique = (data) => db.task.findUnique.mockResolvedValue(data); const mockFindMany = (data) => db.task.findMany.mockResolvedValue(data); ``` +### Cache Module Tests + +Prevent timer leaks and test isolation: + +```typescript +afterAll(() => disposeDomainCaches()); +beforeEach(() => invalidateDomainCaches()); +``` + +Mock cache modules in service tests so caching is bypassed: + +```typescript +vi.mock('$lib/server/tasks/cache', () => ({ + getCachedTasksMap: (fetchFn: () => Promise) => fetchFn(), + invalidateTaskCaches: vi.fn(), +})); +``` + ### HTTP Mocking (Nock) Extract setup into helpers, declare once at describe scope: diff --git a/src/features/votes/server/cache.test.ts b/src/features/votes/server/cache.test.ts new file mode 100644 index 000000000..adf2a92f3 --- /dev/null +++ b/src/features/votes/server/cache.test.ts @@ -0,0 +1,55 @@ +import { describe, test, expect, vi, beforeEach, afterEach, afterAll } from 'vitest'; + +import type { VotedGradeStatistics } from '@prisma/client'; +import { TaskGrade } from '$lib/types/task'; +import { getCachedVoteStats, invalidateVoteCaches, disposeVoteCaches } from './cache'; + +const makeStats = (): Map => + new Map([ + [ + 'abc408_d', + { + id: '1', + taskId: 'abc408_d', + grade: TaskGrade.Q1, + isExperimental: false, + createdAt: new Date('2026-01-01'), + updatedAt: new Date('2026-01-01'), + } as unknown as VotedGradeStatistics, + ], + ]); +const mockStatsFn = () => vi.fn().mockResolvedValue(makeStats()); + +afterAll(() => disposeVoteCaches()); + +describe('getCachedVoteStats', () => { + beforeEach(() => invalidateVoteCaches()); + afterEach(() => vi.restoreAllMocks()); + + test('delegates to cache and returns fetched value', async () => { + const fetchFn = mockStatsFn(); + const result = await getCachedVoteStats(fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + expect(result.get('abc408_d')?.grade).toBe(TaskGrade.Q1); + }); + + test('returns cached value on subsequent calls', async () => { + const fetchFn = mockStatsFn(); + await getCachedVoteStats(fetchFn); + await getCachedVoteStats(fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); +}); + +describe('invalidateVoteCaches', () => { + beforeEach(() => invalidateVoteCaches()); + afterEach(() => vi.restoreAllMocks()); + + test('clears vote stats cache', async () => { + const fetchFn = mockStatsFn(); + await getCachedVoteStats(fetchFn); + invalidateVoteCaches(); + await getCachedVoteStats(fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/features/votes/server/cache.ts b/src/features/votes/server/cache.ts new file mode 100644 index 000000000..1b82831fe --- /dev/null +++ b/src/features/votes/server/cache.ts @@ -0,0 +1,21 @@ +import { Cache } from '$lib/clients/cache'; +import type { VotedGradeStatistics } from '@prisma/client'; + +const VOTE_STATS_TTL_MS = 10 * 60 * 1000; +const KEY = 'vote_grade_statistics'; + +const cache = new Cache>(VOTE_STATS_TTL_MS); + +export function getCachedVoteStats( + fetchFn: () => Promise>, +): Promise> { + return cache.getOrFetch(KEY, fetchFn); +} + +export function invalidateVoteCaches(): void { + cache.delete(KEY); +} + +export function disposeVoteCaches(): void { + cache.dispose(); +} diff --git a/src/features/votes/services/vote_grade.test.ts b/src/features/votes/services/vote_grade.test.ts index f792fb8a0..397c965c3 100644 --- a/src/features/votes/services/vote_grade.test.ts +++ b/src/features/votes/services/vote_grade.test.ts @@ -4,6 +4,10 @@ import { TaskGrade } from '@prisma/client'; import { getVoteGrade, upsertVoteGradeTables } from './vote_grade'; +vi.mock('$features/votes/server/cache', () => ({ + invalidateVoteCaches: vi.fn(), +})); + vi.mock('$lib/server/database', () => ({ default: { voteGrade: { @@ -26,6 +30,7 @@ vi.mock('$lib/server/database', () => ({ })); import prisma from '$lib/server/database'; +import { invalidateVoteCaches } from '$features/votes/server/cache'; beforeEach(() => { vi.clearAllMocks(); @@ -220,4 +225,26 @@ describe('upsertVoteGradeTables', () => { }), ); }); + + test('invalidates vote caches after successful write', async () => { + const tx = setupTransaction(); + tx.voteGrade.findUnique.mockResolvedValue(null); + tx.voteGrade.upsert.mockResolvedValue({}); + tx.votedGradeCounter.upsert.mockResolvedValue({}); + tx.votedGradeCounter.findMany.mockResolvedValue([{ grade: TaskGrade.Q5, count: 1 }]); + tx.task.findUnique.mockResolvedValue({ grade: TaskGrade.Q3 }); + + await upsertVoteGradeTables('user-1', 'abc001_a', TaskGrade.Q5); + + expect(invalidateVoteCaches).toHaveBeenCalledTimes(1); + }); + + test('does not invalidate vote caches when grade is unchanged (early return)', async () => { + const tx = setupTransaction(); + tx.voteGrade.findUnique.mockResolvedValue({ grade: TaskGrade.Q5 }); + + await upsertVoteGradeTables('user-1', 'abc001_a', TaskGrade.Q5); + + expect(invalidateVoteCaches).not.toHaveBeenCalled(); + }); }); diff --git a/src/features/votes/services/vote_grade.ts b/src/features/votes/services/vote_grade.ts index aac78a565..dedea47e9 100644 --- a/src/features/votes/services/vote_grade.ts +++ b/src/features/votes/services/vote_grade.ts @@ -7,6 +7,7 @@ import { MIN_VOTES_FOR_STATISTICS, MIN_VOTES_FOR_PROVISIONAL_GRADE, } from '$features/votes/constants/statistics'; +import { invalidateVoteCaches } from '$features/votes/server/cache'; export async function getVoteGrade(userId: string, taskId: string): Promise { const voteRecord = await prisma.voteGrade.findUnique({ @@ -32,14 +33,13 @@ export async function upsertVoteGradeTables( taskId: string, grade: TaskGrade, ): Promise<{ success: true }> { - await prisma.$transaction(async (tx) => { + const isUpdated = await prisma.$transaction(async (tx) => { const existing = await tx.voteGrade.findUnique({ where: { userId_taskId: { userId, taskId } }, }); - // 冪等性: 既に同じグレードなら何もしない if (existing?.grade === grade) { - return; + return false; } if (existing) { @@ -63,11 +63,18 @@ export async function upsertVoteGradeTables( ? MIN_VOTES_FOR_PROVISIONAL_GRADE : MIN_VOTES_FOR_STATISTICS; if (total < minVotes) { - return; + return true; } await updateVoteStatistics(tx, taskId, latestCounters, minVotes); + + return true; }); + + if (isUpdated) { + invalidateVoteCaches(); + } + return { success: true }; } diff --git a/src/features/votes/services/vote_statistics.test.ts b/src/features/votes/services/vote_statistics.test.ts index 89d32000d..22c6b51dd 100644 --- a/src/features/votes/services/vote_statistics.test.ts +++ b/src/features/votes/services/vote_statistics.test.ts @@ -30,6 +30,10 @@ vi.mock('$lib/server/database', () => ({ }, })); +vi.mock('$features/votes/server/cache', () => ({ + getCachedVoteStats: (fetchFn: () => Promise) => fetchFn(), +})); + import prisma from '$lib/server/database'; beforeEach(() => { diff --git a/src/features/votes/services/vote_statistics.ts b/src/features/votes/services/vote_statistics.ts index e84483c50..83898ca62 100644 --- a/src/features/votes/services/vote_statistics.ts +++ b/src/features/votes/services/vote_statistics.ts @@ -1,4 +1,6 @@ import { default as prisma } from '$lib/server/database'; +import { getCachedVoteStats } from '$features/votes/server/cache'; + import type { VotedGradeStatistics, VotedGradeCounter, TaskGrade } from '@prisma/client'; /** A task row enriched with estimated grade and total vote count. */ @@ -15,13 +17,10 @@ export type TaskWithVoteInfo = { }; export async function getVoteGradeStatistics(): Promise> { - const allStats = await prisma.votedGradeStatistics.findMany(); - const gradesMap = new Map(); - - allStats.forEach((stat) => { - gradesMap.set(stat.taskId, stat); + return getCachedVoteStats(async () => { + const allStats = await prisma.votedGradeStatistics.findMany(); + return new Map(allStats.map((stat) => [stat.taskId, stat])); }); - return gradesMap; } export async function getVoteGradeStatisticsForTaskIds( diff --git a/src/features/workbooks/server/cache.test.ts b/src/features/workbooks/server/cache.test.ts new file mode 100644 index 000000000..ed58b8fe2 --- /dev/null +++ b/src/features/workbooks/server/cache.test.ts @@ -0,0 +1,60 @@ +import { describe, test, expect, vi, beforeEach, afterEach, afterAll } from 'vitest'; + +import { + getCachedWorkbooksByPlacement, + getCachedWorkbooksByUser, + invalidateWorkbookCaches, + disposeWorkbookCaches, +} from './cache'; + +const mockFetchFn = () => vi.fn().mockResolvedValue([]); + +afterAll(() => disposeWorkbookCaches()); + +describe('getCachedWorkbooksByPlacement', () => { + beforeEach(() => invalidateWorkbookCaches()); + afterEach(() => vi.restoreAllMocks()); + + test('returns cached value on subsequent calls', async () => { + const fetchFn = mockFetchFn(); + await getCachedWorkbooksByPlacement('SOLUTION:SEARCH_SIMULATION:false', fetchFn); + await getCachedWorkbooksByPlacement('SOLUTION:SEARCH_SIMULATION:false', fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + + test('misses cache when key differs', async () => { + const fetchFn = mockFetchFn(); + await getCachedWorkbooksByPlacement('SOLUTION:SEARCH_SIMULATION:false', fetchFn); + await getCachedWorkbooksByPlacement('SOLUTION:DYNAMIC_PROGRAMMING:false', fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(2); + }); +}); + +describe('getCachedWorkbooksByUser', () => { + beforeEach(() => invalidateWorkbookCaches()); + afterEach(() => vi.restoreAllMocks()); + + test('returns cached value on subsequent calls', async () => { + const fetchFn = mockFetchFn(); + await getCachedWorkbooksByUser(fetchFn); + await getCachedWorkbooksByUser(fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); +}); + +describe('invalidateWorkbookCaches', () => { + beforeEach(() => invalidateWorkbookCaches()); + afterEach(() => vi.restoreAllMocks()); + + test('clears both placement and user caches', async () => { + const placementFn = mockFetchFn(); + const userFn = mockFetchFn(); + await getCachedWorkbooksByPlacement('SOLUTION:SEARCH_SIMULATION:false', placementFn); + await getCachedWorkbooksByUser(userFn); + invalidateWorkbookCaches(); + await getCachedWorkbooksByPlacement('SOLUTION:SEARCH_SIMULATION:false', placementFn); + await getCachedWorkbooksByUser(userFn); + expect(placementFn).toHaveBeenCalledTimes(2); + expect(userFn).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/features/workbooks/server/cache.ts b/src/features/workbooks/server/cache.ts new file mode 100644 index 000000000..50bf5d546 --- /dev/null +++ b/src/features/workbooks/server/cache.ts @@ -0,0 +1,30 @@ +import { Cache, DEFAULT_CACHE_TTL } from '$lib/clients/cache'; +import type { WorkbooksWithAuthors } from '$features/workbooks/types/workbook'; + +const BY_USER_KEY = 'workbooks_by_user'; + +const placementCache = new Cache(DEFAULT_CACHE_TTL, 100); +const byUserCache = new Cache(DEFAULT_CACHE_TTL); + +export function getCachedWorkbooksByPlacement( + key: string, + fetchFn: () => Promise, +): Promise { + return placementCache.getOrFetch(key, fetchFn); +} + +export function getCachedWorkbooksByUser( + fetchFn: () => Promise, +): Promise { + return byUserCache.getOrFetch(BY_USER_KEY, fetchFn); +} + +export function invalidateWorkbookCaches(): void { + placementCache.clear(); + byUserCache.delete(BY_USER_KEY); +} + +export function disposeWorkbookCaches(): void { + placementCache.dispose(); + byUserCache.dispose(); +} diff --git a/src/features/workbooks/services/workbooks.test.ts b/src/features/workbooks/services/workbooks.test.ts index fe110f3ee..eebdc1f41 100644 --- a/src/features/workbooks/services/workbooks.test.ts +++ b/src/features/workbooks/services/workbooks.test.ts @@ -40,6 +40,12 @@ vi.mock('$lib/services/users', () => ({ getUserById: vi.fn(), })); +vi.mock('$features/workbooks/server/cache', () => ({ + getCachedWorkbooksByPlacement: (_key: string, fetchFn: () => Promise) => fetchFn(), + getCachedWorkbooksByUser: (fetchFn: () => Promise) => fetchFn(), + invalidateWorkbookCaches: vi.fn(), +})); + import prisma from '$lib/server/database'; import * as usersCrud from '$lib/services/users'; diff --git a/src/features/workbooks/services/workbooks.ts b/src/features/workbooks/services/workbooks.ts index d3c056cbc..801f5085d 100644 --- a/src/features/workbooks/services/workbooks.ts +++ b/src/features/workbooks/services/workbooks.ts @@ -21,8 +21,15 @@ import { } from '$features/workbooks/services/workbook_tasks'; import * as userCrud from '$lib/services/users'; +import { + getCachedWorkbooksByPlacement, + getCachedWorkbooksByUser, + invalidateWorkbookCaches, +} from '$features/workbooks/server/cache'; + import { sanitizeUrl } from '$lib/utils/url'; import { parseWorkBookId, parseWorkBookUrlSlug } from '$features/workbooks/utils/workbook'; +import { buildPlacementKey } from '$features/workbooks/utils/workbooks'; export async function getWorkBooks(): Promise { const workbooks = await db.workBook.findMany({ @@ -72,28 +79,32 @@ export async function getWorkbooksByPlacement( query: PlacementQuery, includeUnpublished = false, ): Promise { - const placementFilter = buildPlacementFilter(query); + const cacheKey = buildPlacementKey(query, includeUnpublished); - const workbooks = await db.workBook.findMany({ - where: { - workBookType: query.workBookType, - ...(includeUnpublished ? {} : { isPublished: true }), - placement: placementFilter, - }, - orderBy: { - placement: { priority: 'asc' }, - }, - include: { - user: { - select: { username: true }, + return getCachedWorkbooksByPlacement(cacheKey, async () => { + const placementFilter = buildPlacementFilter(query); + + const workbooks = await db.workBook.findMany({ + where: { + workBookType: query.workBookType, + ...(includeUnpublished ? {} : { isPublished: true }), + placement: placementFilter, }, - workBookTasks: { - orderBy: { priority: 'asc' }, + orderBy: { + placement: { priority: 'asc' }, }, - }, - }); + include: { + user: { + select: { username: true }, + }, + workBookTasks: { + orderBy: { priority: 'asc' }, + }, + }, + }); - return mapWithAuthorName(workbooks); + return mapWithAuthorName(workbooks); + }); } /** @@ -101,20 +112,22 @@ export async function getWorkbooksByPlacement( * Intended for admin-only display on the workbooks list page. */ export async function getWorkBooksCreatedByUsers(): Promise { - const workbooks = await db.workBook.findMany({ - where: { workBookType: WorkBookTypeConst.CREATED_BY_USER }, - orderBy: { id: 'asc' }, - include: { - user: { - select: { username: true }, - }, - workBookTasks: { - orderBy: { priority: 'asc' }, + return getCachedWorkbooksByUser(async () => { + const workbooks = await db.workBook.findMany({ + where: { workBookType: WorkBookTypeConst.CREATED_BY_USER }, + orderBy: { id: 'asc' }, + include: { + user: { + select: { username: true }, + }, + workBookTasks: { + orderBy: { priority: 'asc' }, + }, }, - }, - }); + }); - return mapWithAuthorName(workbooks); + return mapWithAuthorName(workbooks); + }); } /** @@ -268,7 +281,7 @@ export async function createWorkBook(workBook: Omit): Promise { @@ -304,6 +317,8 @@ export async function updateWorkBook(workBookId: number, workBook: WorkBook): Pr }, }), ]); + + invalidateWorkbookCaches(); } catch (error) { console.error( `Failed to update WorkBook with id ${workBookId} and title ${workBook.title}:`, @@ -323,6 +338,8 @@ export async function deleteWorkBook(workBookId: number): Promise { id: workBookId, }, }); + + invalidateWorkbookCaches(); } // ---- Private helpers ---- diff --git a/src/features/workbooks/utils/workbooks.test.ts b/src/features/workbooks/utils/workbooks.test.ts index c1e39a3fc..24e8fff41 100644 --- a/src/features/workbooks/utils/workbooks.test.ts +++ b/src/features/workbooks/utils/workbooks.test.ts @@ -3,8 +3,11 @@ import { describe, expect, test } from 'vitest'; import { Roles } from '$lib/types/user'; import { TaskGrade, type Task, type TaskResult } from '$lib/types/task'; import { type WorkbookList, WorkBookType } from '$features/workbooks/types/workbook'; +import { SolutionCategory } from '$features/workbooks/types/workbook_placement'; +import type { PlacementQuery } from '$features/workbooks/types/workbook_placement'; import { + buildPlacementKey, canViewWorkBook, getUrlSlugFrom, getWorkBooksByType, @@ -457,3 +460,31 @@ describe('Workbooks', () => { }); }); }); + +describe('buildPlacementKey', () => { + test('returns CURRICULUM key with taskGrade and includeUnpublished', () => { + const query: PlacementQuery = { + workBookType: WorkBookType.CURRICULUM, + taskGrade: TaskGrade.Q7, + }; + expect(buildPlacementKey(query, false)).toBe('CURRICULUM:Q7:false'); + expect(buildPlacementKey(query, true)).toBe('CURRICULUM:Q7:true'); + }); + + test('returns SOLUTION key with solutionCategory and includeUnpublished', () => { + const query: PlacementQuery = { + workBookType: WorkBookType.SOLUTION, + solutionCategory: SolutionCategory.SEARCH_SIMULATION, + }; + expect(buildPlacementKey(query, false)).toBe('SOLUTION:SEARCH_SIMULATION:false'); + expect(buildPlacementKey(query, true)).toBe('SOLUTION:SEARCH_SIMULATION:true'); + }); + + test('returns SOLUTION key with null solutionCategory', () => { + const query: PlacementQuery = { + workBookType: WorkBookType.SOLUTION, + solutionCategory: null, + }; + expect(buildPlacementKey(query, false)).toBe('SOLUTION:null:false'); + }); +}); diff --git a/src/features/workbooks/utils/workbooks.ts b/src/features/workbooks/utils/workbooks.ts index e8bc9ce24..a813a5e4c 100644 --- a/src/features/workbooks/utils/workbooks.ts +++ b/src/features/workbooks/utils/workbooks.ts @@ -13,10 +13,20 @@ import type { WorkBookTaskBase, WorkBookType, } from '$features/workbooks/types/workbook'; +import { WorkBookType as WorkBookTypeConst } from '$features/workbooks/types/workbook'; +import type { PlacementQuery } from '$features/workbooks/types/workbook_placement'; import { isAdmin, canRead } from '$lib/utils/authorship'; import { calcGradeMode } from '$lib/utils/task'; +export function buildPlacementKey(query: PlacementQuery, includeUnpublished: boolean): string { + if (query.workBookType === WorkBookTypeConst.CURRICULUM) { + return `CURRICULUM:${query.taskGrade}:${includeUnpublished}`; + } + + return `SOLUTION:${query.solutionCategory}:${includeUnpublished}`; +} + /** Returns true when the user can view the workbook: admins always can; others only when published. */ export function canViewWorkBook(role: Roles, isPublished: boolean) { return isAdmin(role) || isPublished; diff --git a/src/lib/clients/cache.test.ts b/src/lib/clients/cache.test.ts index 5b0c65d0a..cd0334c81 100644 --- a/src/lib/clients/cache.test.ts +++ b/src/lib/clients/cache.test.ts @@ -154,21 +154,17 @@ describe('Cache', () => { describe('edge cases', () => { test('expects to handle different value types', () => { - const cache = new Cache(); + const cache = new Cache(); - // Save various types of values. cache.set('string', 'test'); cache.set('number', 123); cache.set('boolean', true); - cache.set('null', null); cache.set('object', { a: 1, b: 2 }); cache.set('array', [1, 2, 3]); - // Validate the values are stored correctly. expect(cache.get('string')).toBe('test'); expect(cache.get('number')).toBe(123); - expect(cache.get('boolean')).toBeTruthy(); - expect(cache.get('null')).toBe(null); + expect(cache.get('boolean')).toBe(true); expect(cache.get('object')).toEqual({ a: 1, b: 2 }); expect(cache.get('array')).toEqual([1, 2, 3]); }); @@ -343,4 +339,142 @@ describe('Cache', () => { expect(cache.size).toBe(0); }); }); + + describe('getOrFetch', () => { + describe('successful case', () => { + test('calls fetchFn and caches result on first invocation', async () => { + const cache = new Cache(); + const fetchFn = vi.fn().mockResolvedValue('fetched'); + const result = await cache.getOrFetch('key', fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + expect(result).toBe('fetched'); + }); + + test('returns cached value without calling fetchFn on subsequent calls', async () => { + const cache = new Cache(); + const fetchFn = vi.fn().mockResolvedValue('fetched'); + await cache.getOrFetch('key', fetchFn); + await cache.getOrFetch('key', fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + }); + + describe('error cases', () => { + test('propagates fetchFn error without caching', async () => { + const cache = new Cache(); + const fetchFn = vi.fn().mockRejectedValue(new Error('fetch failed')); + await expect(cache.getOrFetch('key', fetchFn)).rejects.toThrow('fetch failed'); + expect(cache.size).toBe(0); + }); + + test('retries fetchFn after a previous failure', async () => { + const cache = new Cache(); + const fetchFn = vi + .fn() + .mockRejectedValueOnce(new Error('fetch failed')) + .mockResolvedValue('retried'); + await expect(cache.getOrFetch('key', fetchFn)).rejects.toThrow('fetch failed'); + const result = await cache.getOrFetch('key', fetchFn); + expect(result).toBe('retried'); + expect(fetchFn).toHaveBeenCalledTimes(2); + }); + }); + + describe('boundary cases', () => { + test('caches empty Map as valid value', async () => { + const cache = new Cache>(); + const fetchFn = vi.fn().mockResolvedValue(new Map()); + await cache.getOrFetch('key', fetchFn); + await cache.getOrFetch('key', fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + + test('returns cached value just before TTL expires', async () => { + const TTL = 1000; + const cache = new Cache(TTL); + const fetchFn = vi.fn().mockResolvedValue('fetched'); + await cache.getOrFetch('key', fetchFn); + vi.advanceTimersByTime(TTL - 1); + await cache.getOrFetch('key', fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + + test('calls fetchFn again after TTL expires', async () => { + const TTL = 1000; + const cache = new Cache(TTL); + const fetchFn = vi.fn().mockResolvedValue('fetched'); + await cache.getOrFetch('key', fetchFn); + vi.advanceTimersByTime(TTL + 1); + await cache.getOrFetch('key', fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(2); + }); + }); + + describe('stampede prevention', () => { + test('shares a single fetchFn call across concurrent requests for the same key', async () => { + const cache = new Cache(); + let resolvePromise: (value: string) => void; + const fetchFn = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + resolvePromise = resolve; + }), + ); + + const promise1 = cache.getOrFetch('key', fetchFn); + const promise2 = cache.getOrFetch('key', fetchFn); + + resolvePromise!('shared'); + const [result1, result2] = await Promise.all([promise1, promise2]); + + expect(fetchFn).toHaveBeenCalledTimes(1); + expect(result1).toBe('shared'); + expect(result2).toBe('shared'); + }); + + test('cleans up inflight entry after fetchFn resolves', async () => { + const cache = new Cache(); + const fetchFn = vi.fn().mockResolvedValue('done'); + await cache.getOrFetch('key', fetchFn); + + const fetchFn2 = vi.fn().mockResolvedValue('done2'); + vi.advanceTimersByTime(cache['timeToLive'] + 1); + const result = await cache.getOrFetch('key', fetchFn2); + + expect(fetchFn2).toHaveBeenCalledTimes(1); + expect(result).toBe('done2'); + }); + + test('cleans up inflight entry and propagates error to all waiters on fetchFn failure', async () => { + const cache = new Cache(); + const fetchFn = vi.fn().mockRejectedValue(new Error('boom')); + + const results = await Promise.allSettled([ + cache.getOrFetch('key', fetchFn), + cache.getOrFetch('key', fetchFn), + ]); + + expect(fetchFn).toHaveBeenCalledTimes(1); + expect(results[0]).toEqual(expect.objectContaining({ status: 'rejected' })); + expect(results[1]).toEqual(expect.objectContaining({ status: 'rejected' })); + expect(cache.size).toBe(0); + }); + + test('does not share inflight between different keys', async () => { + const cache = new Cache(); + const fetchFnA = vi.fn().mockResolvedValue('a'); + const fetchFnB = vi.fn().mockResolvedValue('b'); + + const [resultA, resultB] = await Promise.all([ + cache.getOrFetch('keyA', fetchFnA), + cache.getOrFetch('keyB', fetchFnB), + ]); + + expect(fetchFnA).toHaveBeenCalledTimes(1); + expect(fetchFnB).toHaveBeenCalledTimes(1); + expect(resultA).toBe('a'); + expect(resultB).toBe('b'); + }); + }); + }); }); diff --git a/src/lib/clients/cache.ts b/src/lib/clients/cache.ts index cafb54935..528eee3f5 100644 --- a/src/lib/clients/cache.ts +++ b/src/lib/clients/cache.ts @@ -1,20 +1,8 @@ -/** - * A generic cache class that stores data with a timestamp and provides methods to set, get, and delete cache entries. - * The cache automatically removes the oldest entry when the maximum cache size is reached. - * Entries are also automatically invalidated and removed if they exceed a specified time-to-live (TTL). - * - * @template T - The type of data to be stored in the cache. - */ -export class Cache { +export class Cache { private cache: Map> = new Map(); + private inflight = new Map>(); private cleanupInterval: NodeJS.Timeout; - /** - * Constructs an instance of the class with the specified cache time-to-live (TTL) and maximum cache size. - * - * @param timeToLive - The time-to-live for the cache entries, in milliseconds. Defaults to `CACHE_TTL`. - * @param maxSize - The maximum number of entries the cache can hold. Defaults to `MAX_CACHE_SIZE`. - */ constructor( private readonly timeToLive: number = DEFAULT_CACHE_TTL, private readonly maxSize: number = DEFAULT_MAX_CACHE_SIZE, @@ -29,22 +17,10 @@ export class Cache { this.cleanupInterval = setInterval(() => this.cleanup(), this.timeToLive); } - /** - * Gets the size of the cache. - * - * @returns {number} The number of items in the cache. - */ get size(): number { return this.cache.size; } - /** - * Retrieves the health status of the cache. - * - * @returns An object containing the size of the cache and the timestamp of the oldest entry. - * @property {number} size - The number of entries in the cache. - * @property {number} oldestEntry - The timestamp of the oldest entry in the cache. - */ get health(): { size: number; oldestEntry: number } { if (this.cache.size === 0) { return { size: 0, oldestEntry: 0 }; @@ -56,21 +32,13 @@ export class Cache { return { size: this.cache.size, oldestEntry }; } - /** - * Sets a new entry in the cache with the specified key and data. - * If the cache size exceeds the maximum limit, the oldest entry is removed. - * - * @param key - The key associated with the data to be cached. - * @param data - The data to be cached. - * - * @throws {Error} If the key is empty, not a string, or longer than 255 characters. - */ + /** @throws {Error} If the key is empty, not a string, or longer than 255 characters. */ set(key: string, data: T): void { if (!key || typeof key !== 'string' || key.length > 255) { throw new Error('Invalid cache key'); } - // Note: Remove existing entry first to avoid counting it twice. + // Remove existing entry first to avoid counting it twice. this.cache.delete(key); if (this.cache.size >= this.maxSize) { @@ -84,12 +52,6 @@ export class Cache { this.cache.set(key, { data, timestamp: Date.now() }); } - /** - * Checks if a key exists in the cache without removing expired entries. - * - * @param key - The key to check. - * @returns True if the key exists in the cache, false otherwise. - */ has(key: string): boolean { const entry = this.cache.get(key); @@ -105,12 +67,6 @@ export class Cache { return true; } - /** - * Retrieves an entry from the cache. - * - * @param key - The key associated with the cache entry. - * @returns The cached data if it exists and is not expired, otherwise `undefined`. - */ get(key: string): T | undefined { const entry = this.cache.get(key); @@ -126,37 +82,48 @@ export class Cache { return entry.data; } - /** - * Disposes of resources used by the cache instance. - * - * This method clears the interval used for cleanup and clears the cache. - * It should be called when the cache instance is no longer needed to prevent memory leaks. - */ + async getOrFetch(key: string, fetchFn: () => Promise): Promise { + const cached = this.get(key); + + if (cached !== undefined) { + return cached; + } + + const pending = this.inflight.get(key); + + if (pending) { + return pending; + } + + const promise = fetchFn() + .then((result) => { + this.set(key, result); + return result; + }) + .finally(() => { + this.inflight.delete(key); + }); + + this.inflight.set(key, promise); + + return promise; + } + + /** Call when the cache instance is no longer needed to prevent timer leaks. */ dispose(): void { clearInterval(this.cleanupInterval); this.cache.clear(); + this.inflight.clear(); } - /** - * Clears all entries from the cache. - */ clear(): void { this.cache.clear(); } - /** - * Deletes an entry from the cache. - * - * @param key - The key of the entry to delete. - */ delete(key: string): void { this.cache.delete(key); } - /** - * Removes expired entries from the cache. - * This method is called periodically by the cleanup interval. - */ private cleanup(): void { const now = Date.now(); @@ -167,11 +134,6 @@ export class Cache { } } - /** - * Finds the key of the oldest entry in the cache based on timestamp. - * - * @returns The key of the oldest entry, or undefined if the cache is empty. - */ private findOldestEntry(): string | undefined { let oldestKey: string | undefined; let oldestTime = Infinity; @@ -187,48 +149,19 @@ export class Cache { } } -/** - * Represents a cache entry with data and a timestamp. - * - * @template T - The type of the cached data. - * @property {T} data - The cached data. - * @property {number} timestamp - The timestamp when the data was cached. - */ type CacheEntry = { data: T; timestamp: number; }; -/** - * The time-to-live (TTL) for the cache, specified in milliseconds. - * This value represents 1 hour. - */ -const DEFAULT_CACHE_TTL = 60 * 60 * 1000; // 1 hour in milliseconds -/** - * The default maximum number of entries the cache can hold. - * This value represents 50 entries. - */ +export const DEFAULT_CACHE_TTL = 60 * 60 * 1000; const DEFAULT_MAX_CACHE_SIZE = 50; -/** - * Configuration options for caching. - * - * @property {number} [timeToLive] - The duration (in milliseconds) for which a cache entry should remain valid. - * @property {number} [maxSize] - The maximum number of entries that the cache can hold. - */ - export interface CacheConfig { timeToLive?: number; maxSize?: number; } -/** - * Configuration for the API client's caching behavior. - * - * @interface ApiClientConfig - * @property {CacheConfig} contestCache - Configuration for contest-related data caching. - * @property {CacheConfig} taskCache - Configuration for task-related data caching. - */ export interface ApiClientConfig { contestCache: CacheConfig; taskCache: CacheConfig; diff --git a/src/lib/clients/cache_strategy.ts b/src/lib/clients/cache_strategy.ts index 71fef98b5..220343f74 100644 --- a/src/lib/clients/cache_strategy.ts +++ b/src/lib/clients/cache_strategy.ts @@ -3,79 +3,23 @@ import { Cache } from '$lib/clients/cache'; import type { ContestsForImport } from '$lib/types/contest'; import type { TasksForImport } from '$lib/types/task'; -/** - * A strategy for caching contest and task data. - * Separates the caching logic from the data fetching concerns. - */ export class ContestTaskCache { - /** - * Constructs a cache strategy with the specified contest and task caches. - * @param contestCache - Cache for storing contest import data - * @param taskCache - Cache for storing task import data - */ constructor( private readonly contestCache: Cache, private readonly taskCache: Cache, ) {} - /** - * Retrieves data from cache if available, otherwise fetches it using the provided function. - * - * @template T - The type of data being cached and returned - * @param {string} key - The unique identifier for the cached data - * @param {() => Promise} fetchFunction - Function that returns a Promise resolving to data of type T - * @param {Cache} cache - Cache object with get and set methods for type T - * @returns {Promise} - The cached data or newly fetched data - * - * @example - * const result = await cacheInstance.getCachedOrFetch( - * 'contests-123', - * () => api.fetchContests(), - * contestCache - * ); - */ - async getCachedOrFetch( - key: string, - fetchFunction: () => Promise, - cache: Cache, - ): Promise { - const cachedData = cache.get(key); - - if (cachedData) { - console.log(`Using cached data for ${key}`); - return cachedData; - } - - console.log(`Cache miss for ${key}, fetching...`); - - try { - const contestTasks = await fetchFunction(); - cache.set(key, contestTasks); - - return contestTasks; - } catch (error) { - console.error(`Failed to fetch contests and/or tasks for ${key}:`, error); - return [] as unknown as T; - } - } - - /** - * Gets contests from cache or fetches them. - */ async getCachedOrFetchContests( key: string, fetchFunction: () => Promise, ): Promise { - return this.getCachedOrFetch(key, fetchFunction, this.contestCache); + return this.contestCache.getOrFetch(key, fetchFunction); } - /** - * Gets tasks from cache or fetches them. - */ async getCachedOrFetchTasks( key: string, fetchFunction: () => Promise, ): Promise { - return this.getCachedOrFetch(key, fetchFunction, this.taskCache); + return this.taskCache.getOrFetch(key, fetchFunction); } } diff --git a/src/lib/server/tasks/cache.test.ts b/src/lib/server/tasks/cache.test.ts new file mode 100644 index 000000000..5c687df88 --- /dev/null +++ b/src/lib/server/tasks/cache.test.ts @@ -0,0 +1,62 @@ +import { describe, test, expect, vi, beforeEach, afterEach, afterAll } from 'vitest'; + +import type { Task } from '$lib/types/task'; +import { + getCachedTasksMap, + getCachedMergedTasksMap, + invalidateTaskCaches, + disposeTaskCaches, +} from './cache'; + +const taskEntry = new Map([['abc422_a', { task_id: 'abc422_a' } as unknown as Task]]); +const mockFetchFn = (data: Map = new Map()) => vi.fn().mockResolvedValue(data); + +afterAll(() => disposeTaskCaches()); + +describe('getCachedTasksMap', () => { + beforeEach(() => invalidateTaskCaches()); + afterEach(() => vi.restoreAllMocks()); + + test('delegates to cache and returns fetched value', async () => { + const fetchFn = mockFetchFn(taskEntry); + const result = await getCachedTasksMap(fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + expect(result.get('abc422_a')?.task_id).toBe('abc422_a'); + }); + + test('returns cached value on subsequent calls', async () => { + const fetchFn = mockFetchFn(taskEntry); + await getCachedTasksMap(fetchFn); + await getCachedTasksMap(fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); +}); + +describe('getCachedMergedTasksMap', () => { + beforeEach(() => invalidateTaskCaches()); + afterEach(() => vi.restoreAllMocks()); + + test('returns cached value on subsequent calls', async () => { + const fetchFn = mockFetchFn(); + await getCachedMergedTasksMap(fetchFn); + await getCachedMergedTasksMap(fetchFn); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); +}); + +describe('invalidateTaskCaches', () => { + beforeEach(() => invalidateTaskCaches()); + afterEach(() => vi.restoreAllMocks()); + + test('clears both tasks and mergedTasks caches', async () => { + const tasksFn = mockFetchFn(); + const mergedFn = mockFetchFn(); + await getCachedTasksMap(tasksFn); + await getCachedMergedTasksMap(mergedFn); + invalidateTaskCaches(); + await getCachedTasksMap(tasksFn); + await getCachedMergedTasksMap(mergedFn); + expect(tasksFn).toHaveBeenCalledTimes(2); + expect(mergedFn).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/lib/server/tasks/cache.ts b/src/lib/server/tasks/cache.ts new file mode 100644 index 000000000..ef676a749 --- /dev/null +++ b/src/lib/server/tasks/cache.ts @@ -0,0 +1,32 @@ +import type { Task } from '$lib/types/task'; +import type { TaskMapByContestTaskPair } from '$lib/types/contest_task_pair'; + +import { Cache, DEFAULT_CACHE_TTL } from '$lib/clients/cache'; + +const TASK_MAP_KEY = 'tasks_by_task_id'; +const MERGED_KEY = 'merged_tasks_map'; + +const tasksCache = new Cache>(DEFAULT_CACHE_TTL); +const mergedTasksCache = new Cache(DEFAULT_CACHE_TTL); + +export function getCachedTasksMap( + fetchFn: () => Promise>, +): Promise> { + return tasksCache.getOrFetch(TASK_MAP_KEY, fetchFn); +} + +export function getCachedMergedTasksMap( + fetchFn: () => Promise, +): Promise { + return mergedTasksCache.getOrFetch(MERGED_KEY, fetchFn); +} + +export function invalidateTaskCaches(): void { + tasksCache.delete(TASK_MAP_KEY); + mergedTasksCache.delete(MERGED_KEY); +} + +export function disposeTaskCaches(): void { + tasksCache.dispose(); + mergedTasksCache.dispose(); +} diff --git a/src/lib/services/tasks.ts b/src/lib/services/tasks.ts index dfbdcde5b..e4ddc99df 100644 --- a/src/lib/services/tasks.ts +++ b/src/lib/services/tasks.ts @@ -11,6 +11,11 @@ import type { TaskMapByContestTaskPair, } from '$lib/types/contest_task_pair'; +import { + getCachedTasksMap, + getCachedMergedTasksMap, + invalidateTaskCaches, +} from '$lib/server/tasks/cache'; import { classifyContest } from '$lib/utils/contest'; import { createContestTaskPairKey } from '$lib/utils/contest_task_pair'; @@ -40,16 +45,26 @@ export async function getTasks(): Promise { * const mergedTasksMap = await getMergedTasksMap(filteredTasks); */ export async function getMergedTasksMap(tasks?: Tasks): Promise { - const tasksToMerge = tasks ?? (await getTasks()); - const contestTaskPairs = await getContestTaskPairs(); + if (tasks !== undefined) { + const contestTaskPairs = await getContestTaskPairs(); + return buildMergedMap(tasks, contestTaskPairs); + } + + return getCachedMergedTasksMap(async () => { + const [allTasks, contestTaskPairs] = await Promise.all([getTasks(), getContestTaskPairs()]); + return buildMergedMap(allTasks, contestTaskPairs); + }); +} +function buildMergedMap( + tasks: Tasks, + contestTaskPairs: ContestTaskPair[], +): TaskMapByContestTaskPair { const baseTaskMap = new Map( - tasksToMerge.map((task) => [createContestTaskPairKey(task.contest_id, task.task_id), task]), + tasks.map((task) => [createContestTaskPairKey(task.contest_id, task.task_id), task]), ); - // Unique task_id in database - const taskMap = new Map(tasksToMerge.map((task) => [task.task_id, task])); + const taskMap = new Map(tasks.map((task) => [task.task_id, task])); - // Filter task(s) only the same task_id but different contest_id const additionalTaskMap = contestTaskPairs .filter((pair) => !baseTaskMap.has(createContestTaskPairKey(pair.contestId, pair.taskId))) .flatMap((pair) => { @@ -125,14 +140,10 @@ export async function getTasksWithSelectedTaskIds( } export async function getTasksByTaskId(): Promise> { - const tasks = await db.task.findMany(); - const tasksMap = new Map(); - - (await tasks).map((task) => { - tasksMap.set(task.task_id, task); + return getCachedTasksMap(async () => { + const tasks = await db.task.findMany(); + return new Map(tasks.map((task) => [task.task_id, task])); }); - - return tasksMap; } export async function getTask(task_id: string): Promise { @@ -177,6 +188,7 @@ export async function createTask( }, }); + invalidateTaskCaches(); console.log(task); } @@ -197,6 +209,7 @@ export async function updateTask(task_id: string, task_grade: TaskGrade): Promis }, }); + invalidateTaskCaches(); console.log(task); } catch (error) { if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2025') { diff --git a/src/test/lib/services/tasks.test.ts b/src/test/lib/services/tasks.test.ts index b7fb6140f..62427f247 100644 --- a/src/test/lib/services/tasks.test.ts +++ b/src/test/lib/services/tasks.test.ts @@ -13,6 +13,12 @@ vi.mock('$lib/server/database', () => ({ }, })); +vi.mock('$lib/server/tasks/cache', () => ({ + getCachedTasksMap: (fetchFn: () => Promise) => fetchFn(), + getCachedMergedTasksMap: (fetchFn: () => Promise) => fetchFn(), + invalidateTaskCaches: vi.fn(), +})); + import db from '$lib/server/database'; describe('updateTask', () => {