From db0cfc4aa738acad310836247ef6ad166884035d Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Tue, 16 Jun 2026 13:28:36 +0000 Subject: [PATCH 1/3] fix(task-results): skip getAnswers full-scan for anonymous users getAnswers(undefined) drops the WHERE filter and full-scans taskAnswer. Guard the call in createTaskResults / getTaskResultsOnlyResultExists so anonymous users skip the DB round-trip, and make userId types honest (string | undefined), including TaskResult.user_id. Co-Authored-By: Claude Opus 4.8 --- .../skip-getanswers-for-anonymous/plan.md | 126 ++++++++++++++++++ src/lib/services/task_results.ts | 20 +-- src/lib/types/task.ts | 3 +- src/test/lib/services/task_results.test.ts | 55 +++++++- 4 files changed, 193 insertions(+), 11 deletions(-) create mode 100644 docs/dev-notes/2026-06-16/skip-getanswers-for-anonymous/plan.md diff --git a/docs/dev-notes/2026-06-16/skip-getanswers-for-anonymous/plan.md b/docs/dev-notes/2026-06-16/skip-getanswers-for-anonymous/plan.md new file mode 100644 index 000000000..1a2588928 --- /dev/null +++ b/docs/dev-notes/2026-06-16/skip-getanswers-for-anonymous/plan.md @@ -0,0 +1,126 @@ +# getAnswers(undefined) 全行スキャン除去 + +## 背景と結論 + +匿名ユーザーが `/problems` を開くと、[problems/+page.server.ts:57](src/routes/problems/+page.server.ts#L57) が +`getTaskResults(session?.user.userId)` に `undefined` を渡す。内部の +[createTaskResults](src/lib/services/task_results.ts#L218) が **`isLoggedIn` 判定の前に** +`getAnswers(undefined)` を無条件で呼ぶ（line 219 が line 220 より先）。 + +`getAnswers` は [answers.ts:9-21](src/lib/services/answers.ts#L9) で +`where: { user_id: undefined }` を Prisma に渡す。Prisma は `undefined` を WHERE 句から +除外するため、フィルタが無視され **`taskAnswer` テーブルを全件 SELECT** する。 +`isLoggedIn = userId !== undefined` ガードが結果の使用を防ぐためデータ漏洩はないが、 +不要な全行スキャンが毎回発生し、Vercel Function Duration を押し上げる。 + +Phase 3 の CDN キャッシュにより匿名ヒット時は関数自体が起動しないが、キャッシュミス時 +（初回・TTL 失効後）には発生し続ける。これを根絶する。 + +同じアンチパターンが [getTaskResultsOnlyResultExists](src/lib/services/task_results.ts#L137) +（line 137 で無条件 `getAnswers(userId)`）にも存在する。現状は workbooks load が匿名で +早期 return するため実害ゼロだが、ガードが外れた瞬間に同じ全行スキャンが再発する潜在バグ +なので、今回まとめて防御的に修正する。 + +参考: [getTaskResultsByTaskId](src/lib/services/task_results.ts#L182) は既に +`userId ? ... : []` で正しくガード済み（修正不要・パターンの手本）。 + +## 設計の根拠（確定済み） + +1. ガード位置: **`createTaskResults` 内**（`getAnswers` 自体の契約は変えない。 + 既存 `getAnswersWithSelectedTaskIds` の「呼び出し側でガード」パターンと一貫） +2. スコープ: `getTaskResultsOnlyResultExists` も**防御的に修正** +3. 型: `userId` を **`string | undefined` に正直化**（現状 `userId !== undefined` 判定と + `userId: string` 型が矛盾している＝型が嘘をついている） + +### 却下した代替案 + +- `getAnswers` 内ガード → 低レベル CRUD が undefined を黙って受理し他バグを隠す懸念 +- 呼び出し側（+page.server.ts）早期 return → problems は匿名でも一覧描画するため表示影響の + 確認が必要で、対象経路を網羅できない + +## フェーズ + +単一レイヤー（services）・小規模のため 1 フェーズ。 + +### Phase 1: createTaskResults / getTaskResultsOnlyResultExists の匿名ガード + +#### 変更対象: [src/lib/services/task_results.ts](src/lib/services/task_results.ts) + +**`createTaskResults`（line 218-226）** — `getAnswers` 呼び出しを `isLoggedIn` でガード: + +```typescript +async function createTaskResults(tasks: Tasks, userId: string | undefined): Promise { + const isLoggedIn = userId !== undefined; + // Skip the DB round-trip for anonymous users: getAnswers(undefined) drops the + // WHERE filter and full-scans taskAnswer. + const answers = isLoggedIn ? await answer_crud.getAnswers(userId) : new Map(); + + return tasks.map((task: Task) => { + const answer = isLoggedIn ? answers.get(task.task_id) : null; + return mergeTaskAndAnswer(task, userId, answer); + }); +} +``` + +**`getTaskResults`（line 35）** — 型を正直化（`userId: string` → `userId: string | undefined`）。 +本体ロジックは不変。 + +**`getTaskResultsOnlyResultExists`（line 129-161）** — line 137 を同様にガード: + +```typescript +const tasks = await getTasks(); +const answers = userId !== undefined ? await answer_crud.getAnswers(userId) : new Map(); +``` + +注: 型は `userId: string | undefined` に広げる。`with_map` 分岐や `tasksHasAnswer` +フィルタは空 Map でそのまま正しく「該当なし」に評価されるため、後続ロジックは不変。 + +#### 呼び出し側の型確認（変更最小） + +- [problems/+page.server.ts:57](src/routes/problems/+page.server.ts#L57), + [:46](src/routes/problems/+page.server.ts#L46) — 既に `session?.user.userId` + （`string | undefined`）を渡しており、型を広げる方向なので**変更不要**。 +- [workbooks/+page.server.ts:92](src/routes/workbooks/+page.server.ts#L92) — `loggedInUser.id` + （`string`）を渡す。広げた型に対し代入互換なので**変更不要**。 +- `mergeTaskAndAnswer` / `createDefaultTaskResult` の `userId` 引数も `string | undefined` を + 受けられるか確認。`pnpm check` で検出する。 + +## テスト（TDD: 先に書く） + +ファイル: [src/test/lib/services/task_results.test.ts](src/test/lib/services/task_results.test.ts) + +`getAnswers` は既に `vi.mock('$lib/services/answers', ...)`（line 321）でモック済み。 +spy 参照のため `import { getAnswers } from '$lib/services/answers'` を追加し、`beforeEach` で +モックをリセットする。 + +追加するケース: + +1. `describe('getTaskResults')` 配下、`describe('when anonymous (userId is undefined)')`: + - `test('does not call getAnswers')` → `getTaskResults(undefined)` 後に + `expect(getAnswers).not.toHaveBeenCalled()` + - `test('returns default (未挑戦) results for all tasks')` → 全 `taskResult.is_ac === false`, + `status_name === 'ns'`（既存「no answers」ケースのアサーション流用） +2. `getTaskResultsOnlyResultExists` のテストが既存にあれば同様に + `test('does not call getAnswers when userId is undefined')` を追加。無ければ最小の + describe を新設し、`getTasks` モックありで `getTaskResultsOnlyResultExists(undefined, false)` + が空配列を返し `getAnswers` 未呼び出しであることをアサート。 + +既存の `getTaskResults('user_123')` 系テスト（ログイン時）はガード追加後も +`isLoggedIn === true` 経路を通るため不変で PASS する想定。 + +## 検証 + +```bash +pnpm test:unit # 追加テスト含め task_results 関連が PASS +pnpm check # 型の正直化（string | undefined）で型エラーが出ないこと +pnpm lint +``` + +手動確認（任意）: ローカル DB に対し匿名で `/problems` をロードし、Prisma クエリログに +`taskAnswer` の `findMany`（WHERE なし全件）が出ないことを確認。 + +## 完了後 + +このバグは [sveltekit-caching/plan.md](docs/dev-notes/2026-06-13/sveltekit-caching/plan.md#L95) +の Phase 3 残タスク。修正完了後、同 plan の該当チェックボックスを `[x]` に更新する。 +小規模バグ修正のため refactor cycle / session-close は対象外（AGENTS.md のホットフィックス扱い）。 diff --git a/src/lib/services/task_results.ts b/src/lib/services/task_results.ts index 4bbac7fa4..cce4582d5 100644 --- a/src/lib/services/task_results.ts +++ b/src/lib/services/task_results.ts @@ -32,7 +32,7 @@ import { NOT_FOUND } from '$lib/constants/http-response-status-codes'; const statusById = await getSubmissionStatusMapWithId(); const statusByName = await getSubmissionStatusMapWithName(); -export async function getTaskResults(userId: string): Promise { +export async function getTaskResults(userId: string | undefined): Promise { // 問題と特定のユーザの回答状況を使ってデータを結合 // 計算量: 問題数をN、特定のユーザの解答数をMとすると、O(N + M)になるはず。 const mergedTasksMap = await getMergedTasksMap(); @@ -127,14 +127,16 @@ async function transferAnswers( // with_mapをtrueにすると、taskIdを使って各TaskResultにO(1)でアクセスできる。 // Why : データ総量を抑えるため。 export async function getTaskResultsOnlyResultExists( - userId: string, + userId: string | undefined, with_map: boolean = false, ): Promise> { const taskResultsMap: Map = new Map(); // TODO: answerの降順にしたい const tasks = await getTasks(); - const answers = await answer_crud.getAnswers(userId); + // Skip the DB round-trip for anonymous users: getAnswers(undefined) drops the + // WHERE filter and full-scans taskAnswer. + const answers = userId !== undefined ? await answer_crud.getAnswers(userId) : new Map(); const tasksHasAnswer = tasks.filter((task) => answers.has(task.task_id)); const taskResultsWithAnswer = tasksHasAnswer.map((task: Task) => { const taskResult = createDefaultTaskResult(userId, task); @@ -215,9 +217,11 @@ export async function getTaskResultsByTaskId( * @param userId - User ID for creating TaskResults * @returns Promise - Array of TaskResult objects */ -async function createTaskResults(tasks: Tasks, userId: string): Promise { - const answers = await answer_crud.getAnswers(userId); +async function createTaskResults(tasks: Tasks, userId: string | undefined): Promise { const isLoggedIn = userId !== undefined; + // Skip the DB round-trip for anonymous users: getAnswers(undefined) drops the + // WHERE filter and full-scans taskAnswer. + const answers = isLoggedIn ? await answer_crud.getAnswers(userId) : new Map(); return tasks.map((task: Task) => { const answer = isLoggedIn ? answers.get(task.task_id) : null; @@ -236,7 +240,7 @@ async function createTaskResults(tasks: Tasks, userId: string): Promise { const tagIds = tagIds_string.split(','); const taskIdByTagIds = await db.taskTag.groupBy({ diff --git a/src/lib/types/task.ts b/src/lib/types/task.ts index 3f09ee387..9a2e93715 100644 --- a/src/lib/types/task.ts +++ b/src/lib/types/task.ts @@ -62,7 +62,8 @@ export type TaskGrades = TaskGrade[]; export const taskGradeValues = Object.values(TaskGrade); export interface TaskResult extends Task { - user_id: string; + // Anonymous (logged-out) results carry no user; user_id is undefined for them. + user_id: string | undefined; status_name: string; status_id: string; submission_status_image_path: string; diff --git a/src/test/lib/services/task_results.test.ts b/src/test/lib/services/task_results.test.ts index 42d02a79a..23aca1c3d 100644 --- a/src/test/lib/services/task_results.test.ts +++ b/src/test/lib/services/task_results.test.ts @@ -12,8 +12,11 @@ import { describe, test, expect, vi, beforeEach } from 'vitest'; -import { getTaskResults } from '$lib/services/task_results'; -import type { TaskResult, TaskResults } from '$lib/types/task'; +import { getTasks } from '$lib/services/tasks'; +import { getAnswers } from '$lib/services/answers'; +import { getTaskResults, getTaskResultsOnlyResultExists } from '$lib/services/task_results'; + +import type { TaskResult, TaskResults, Tasks } from '$lib/types/task'; import { MOCK_TASKS_DATA, @@ -381,6 +384,27 @@ describe('getTaskResults', () => { }); }); + describe('when anonymous (userId is undefined)', () => { + beforeEach(async () => { + mockAnswersForTest = MOCK_ANSWERS_WITH_ANSWERS; + vi.mocked(getAnswers).mockClear(); + taskResults = await getTaskResults(undefined); + }); + + test('does not call getAnswers (skips the full-scan DB round-trip)', () => { + expect(getAnswers).not.toHaveBeenCalled(); + }); + + test('returns default (未挑戦) results for all tasks', () => { + expect(taskResults.length).toBeGreaterThan(0); + taskResults.forEach((taskResult: TaskResult) => { + expect(taskResult.is_ac).toBe(false); + expect(taskResult.status_name).toBe('ns'); + expect(taskResult.submission_status_label_name).toBe('未挑戦'); + }); + }); + }); + describe('when answers exist', () => { beforeEach(async () => { mockAnswersForTest = MOCK_ANSWERS_WITH_ANSWERS; @@ -411,6 +435,33 @@ describe('getTaskResults', () => { }); }); +describe('getTaskResultsOnlyResultExists', () => { + describe('when anonymous (userId is undefined)', () => { + beforeEach(() => { + mockAnswersForTest = MOCK_ANSWERS_WITH_ANSWERS; + vi.mocked(getAnswers).mockClear(); + vi.mocked(getTasks).mockResolvedValue([ + { + id: '1', + contest_id: 'abc101', + task_id: 'arc099_a', + contest_type: 'ABC', + task_table_index: 'C', + title: 'Minimization', + grade: 'Q3', + }, + ] as unknown as Tasks); + }); + + test('does not call getAnswers and returns an empty array', async () => { + const taskResults = await getTaskResultsOnlyResultExists(undefined, false); + + expect(getAnswers).not.toHaveBeenCalled(); + expect(taskResults).toEqual([]); + }); + }); +}); + describe('mergeTaskAndAnswer', () => { createMergedTaskResults( 'when no answers exist', From e482684ded512cb6cc5ec7be93d95ae3d94c616e Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Tue, 16 Jun 2026 13:33:33 +0000 Subject: [PATCH 2/3] docs: remove completed plan for skip-getanswers-for-anonymous Co-Authored-By: Claude Opus 4.7 --- .../skip-getanswers-for-anonymous/plan.md | 126 ------------------ 1 file changed, 126 deletions(-) delete mode 100644 docs/dev-notes/2026-06-16/skip-getanswers-for-anonymous/plan.md diff --git a/docs/dev-notes/2026-06-16/skip-getanswers-for-anonymous/plan.md b/docs/dev-notes/2026-06-16/skip-getanswers-for-anonymous/plan.md deleted file mode 100644 index 1a2588928..000000000 --- a/docs/dev-notes/2026-06-16/skip-getanswers-for-anonymous/plan.md +++ /dev/null @@ -1,126 +0,0 @@ -# getAnswers(undefined) 全行スキャン除去 - -## 背景と結論 - -匿名ユーザーが `/problems` を開くと、[problems/+page.server.ts:57](src/routes/problems/+page.server.ts#L57) が -`getTaskResults(session?.user.userId)` に `undefined` を渡す。内部の -[createTaskResults](src/lib/services/task_results.ts#L218) が **`isLoggedIn` 判定の前に** -`getAnswers(undefined)` を無条件で呼ぶ（line 219 が line 220 より先）。 - -`getAnswers` は [answers.ts:9-21](src/lib/services/answers.ts#L9) で -`where: { user_id: undefined }` を Prisma に渡す。Prisma は `undefined` を WHERE 句から -除外するため、フィルタが無視され **`taskAnswer` テーブルを全件 SELECT** する。 -`isLoggedIn = userId !== undefined` ガードが結果の使用を防ぐためデータ漏洩はないが、 -不要な全行スキャンが毎回発生し、Vercel Function Duration を押し上げる。 - -Phase 3 の CDN キャッシュにより匿名ヒット時は関数自体が起動しないが、キャッシュミス時 -（初回・TTL 失効後）には発生し続ける。これを根絶する。 - -同じアンチパターンが [getTaskResultsOnlyResultExists](src/lib/services/task_results.ts#L137) -（line 137 で無条件 `getAnswers(userId)`）にも存在する。現状は workbooks load が匿名で -早期 return するため実害ゼロだが、ガードが外れた瞬間に同じ全行スキャンが再発する潜在バグ -なので、今回まとめて防御的に修正する。 - -参考: [getTaskResultsByTaskId](src/lib/services/task_results.ts#L182) は既に -`userId ? ... : []` で正しくガード済み（修正不要・パターンの手本）。 - -## 設計の根拠（確定済み） - -1. ガード位置: **`createTaskResults` 内**（`getAnswers` 自体の契約は変えない。 - 既存 `getAnswersWithSelectedTaskIds` の「呼び出し側でガード」パターンと一貫） -2. スコープ: `getTaskResultsOnlyResultExists` も**防御的に修正** -3. 型: `userId` を **`string | undefined` に正直化**（現状 `userId !== undefined` 判定と - `userId: string` 型が矛盾している＝型が嘘をついている） - -### 却下した代替案 - -- `getAnswers` 内ガード → 低レベル CRUD が undefined を黙って受理し他バグを隠す懸念 -- 呼び出し側（+page.server.ts）早期 return → problems は匿名でも一覧描画するため表示影響の - 確認が必要で、対象経路を網羅できない - -## フェーズ - -単一レイヤー（services）・小規模のため 1 フェーズ。 - -### Phase 1: createTaskResults / getTaskResultsOnlyResultExists の匿名ガード - -#### 変更対象: [src/lib/services/task_results.ts](src/lib/services/task_results.ts) - -**`createTaskResults`（line 218-226）** — `getAnswers` 呼び出しを `isLoggedIn` でガード: - -```typescript -async function createTaskResults(tasks: Tasks, userId: string | undefined): Promise { - const isLoggedIn = userId !== undefined; - // Skip the DB round-trip for anonymous users: getAnswers(undefined) drops the - // WHERE filter and full-scans taskAnswer. - const answers = isLoggedIn ? await answer_crud.getAnswers(userId) : new Map(); - - return tasks.map((task: Task) => { - const answer = isLoggedIn ? answers.get(task.task_id) : null; - return mergeTaskAndAnswer(task, userId, answer); - }); -} -``` - -**`getTaskResults`（line 35）** — 型を正直化（`userId: string` → `userId: string | undefined`）。 -本体ロジックは不変。 - -**`getTaskResultsOnlyResultExists`（line 129-161）** — line 137 を同様にガード: - -```typescript -const tasks = await getTasks(); -const answers = userId !== undefined ? await answer_crud.getAnswers(userId) : new Map(); -``` - -注: 型は `userId: string | undefined` に広げる。`with_map` 分岐や `tasksHasAnswer` -フィルタは空 Map でそのまま正しく「該当なし」に評価されるため、後続ロジックは不変。 - -#### 呼び出し側の型確認（変更最小） - -- [problems/+page.server.ts:57](src/routes/problems/+page.server.ts#L57), - [:46](src/routes/problems/+page.server.ts#L46) — 既に `session?.user.userId` - （`string | undefined`）を渡しており、型を広げる方向なので**変更不要**。 -- [workbooks/+page.server.ts:92](src/routes/workbooks/+page.server.ts#L92) — `loggedInUser.id` - （`string`）を渡す。広げた型に対し代入互換なので**変更不要**。 -- `mergeTaskAndAnswer` / `createDefaultTaskResult` の `userId` 引数も `string | undefined` を - 受けられるか確認。`pnpm check` で検出する。 - -## テスト（TDD: 先に書く） - -ファイル: [src/test/lib/services/task_results.test.ts](src/test/lib/services/task_results.test.ts) - -`getAnswers` は既に `vi.mock('$lib/services/answers', ...)`（line 321）でモック済み。 -spy 参照のため `import { getAnswers } from '$lib/services/answers'` を追加し、`beforeEach` で -モックをリセットする。 - -追加するケース: - -1. `describe('getTaskResults')` 配下、`describe('when anonymous (userId is undefined)')`: - - `test('does not call getAnswers')` → `getTaskResults(undefined)` 後に - `expect(getAnswers).not.toHaveBeenCalled()` - - `test('returns default (未挑戦) results for all tasks')` → 全 `taskResult.is_ac === false`, - `status_name === 'ns'`（既存「no answers」ケースのアサーション流用） -2. `getTaskResultsOnlyResultExists` のテストが既存にあれば同様に - `test('does not call getAnswers when userId is undefined')` を追加。無ければ最小の - describe を新設し、`getTasks` モックありで `getTaskResultsOnlyResultExists(undefined, false)` - が空配列を返し `getAnswers` 未呼び出しであることをアサート。 - -既存の `getTaskResults('user_123')` 系テスト（ログイン時）はガード追加後も -`isLoggedIn === true` 経路を通るため不変で PASS する想定。 - -## 検証 - -```bash -pnpm test:unit # 追加テスト含め task_results 関連が PASS -pnpm check # 型の正直化（string | undefined）で型エラーが出ないこと -pnpm lint -``` - -手動確認（任意）: ローカル DB に対し匿名で `/problems` をロードし、Prisma クエリログに -`taskAnswer` の `findMany`（WHERE なし全件）が出ないことを確認。 - -## 完了後 - -このバグは [sveltekit-caching/plan.md](docs/dev-notes/2026-06-13/sveltekit-caching/plan.md#L95) -の Phase 3 残タスク。修正完了後、同 plan の該当チェックボックスを `[x]` に更新する。 -小規模バグ修正のため refactor cycle / session-close は対象外（AGENTS.md のホットフィックス扱い）。 From 10e8a040876f00b899878cb7517541ffc731ad42 Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Tue, 16 Jun 2026 22:59:20 +0000 Subject: [PATCH 3/3] docs(types): add TSDoc to TaskResult clarifying anonymous user_id semantics Co-Authored-By: Claude Opus 4.7 --- src/lib/types/task.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lib/types/task.ts b/src/lib/types/task.ts index 9a2e93715..d32dc6ea3 100644 --- a/src/lib/types/task.ts +++ b/src/lib/types/task.ts @@ -61,8 +61,13 @@ export type TaskGrades = TaskGrade[]; export const taskGradeValues = Object.values(TaskGrade); +/** + * A user's submission result for a single task, extending {@link Task} with status metadata. + * + * Used for both authenticated and anonymous (logged-out) views. + */ export interface TaskResult extends Task { - // Anonymous (logged-out) results carry no user; user_id is undefined for them. + /** Owner of this result; `undefined` for anonymous (logged-out) results. */ user_id: string | undefined; status_name: string; status_id: string;