From 325849816ed86a1faba35e5b8cd55bc80914bcde Mon Sep 17 00:00:00 2001 From: QiuSW Date: Fri, 11 Sep 2026 22:46:23 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E6=95=B4=E6=94=B9=20#8=20=E5=AE=A1?= =?UTF-8?q?=E6=A0=B8=E9=97=AE=E9=A2=98=20R1=EF=BD=9ER3=20(#8)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - R2 并发同键作答:取得词条行锁后加锁复查答案键,stale 插入遇到唯一键冲突转为返回 已记录结果,不再返回 500;新增两个 goroutine 同键提交的集成用例 - R3 排期:只有新建或状态/等级实际变化才移动 due_at,编辑释义与例句保留原排期, 逾期词条不会被挤出当天队列 - R3 附带发现:保存未提及等级时保留已获得的等级,阅读器面板不再把 4 级词重置为 1 级 - R1 契约:作答响应 result 只取 applied/stale,另加 duplicate 标记,重放返回首次结果; 客户端按首次结果计数,本轮只解决卡片而没有新计分时显示完成页而不是空队列 - R4/R5:stale 与重放分别给出角色为 status 的提示,answerId 作用域注释与实现一致 - Wiki 更新 Business-Rules-and-Glossary、Architecture-and-Code-Map、 Local-Development-and-Verification 并同步镜像 --- docs/02-architecture-and-code-map.md | 6 +- docs/03-business-rules-and-glossary.md | 8 +- docs/04-local-development-and-verification.md | 22 ++- learner/src/__tests__/review.spec.ts | 38 +++++- learner/src/stores/review.ts | 35 ++++- learner/src/style.css | 1 + learner/src/views/ReviewView.vue | 4 +- server/app/lexgo/review.go | 76 ++++++++--- server/app/lexgo/review_test.go | 125 +++++++++++++++++- server/app/lexgo/router.go | 2 +- server/app/lexgo/terms.go | 78 ++++++++--- server/app/lexgo/terms_test.go | 55 +++++--- 12 files changed, 362 insertions(+), 88 deletions(-) diff --git a/docs/02-architecture-and-code-map.md b/docs/02-architecture-and-code-map.md index 2df66aa..1b293f2 100644 --- a/docs/02-architecture-and-code-map.md +++ b/docs/02-architecture-and-code-map.md @@ -2,8 +2,8 @@ generated: true (请先修改 Gitea Wiki,禁止直接编辑本文件) wiki_page: Architecture-and-Code-Map wiki_url: https://git.ilapage.cn/OPC/lexgo/wiki/Architecture-and-Code-Map.- -wiki_revision: 0d6fb43a2662e263fc6f2fb5426b9a8fafaa4868 -synchronized_at: 2026-09-11T12:54:05Z +wiki_revision: 8add5553222f7f5235c0b3fe45a0ad46d37bbd46 +synchronized_at: 2026-09-11T14:44:50Z # 架构与代码地图 @@ -267,7 +267,7 @@ schema v6 新增 `lexgo_term_reviews`(每个个人词条一行排期:`due_at | 接口 | 权限与输入/输出 | |---|---| | GET /api/v1/reviews/queue | 本人+当前语言;仅 `新词`/`学习中` 且 `due_at ≤ now`,按 `due_at, id` 排序、最多 50 条;返回 `{items, total}`,`total` 是全部到期数;不接受查询参数 | -| POST /api/v1/reviews/:termId/answers | `{answerId, grade: correct\|wrong\|again, expectedDueAt}`;应用成功 201,重放或过期 200;返回 `result`(applied/duplicate/stale)、前后状态/等级/到期时间、`requeued` 与词条新状态 | +| POST /api/v1/reviews/:termId/answers | `{answerId, grade: correct\|wrong\|again, expectedDueAt}`;应用成功 201,重放或过期 200;返回首次结果 `result`(applied/stale) 与 `duplicate` 标记、前后状态/等级/到期时间、`requeued` 与词条新状态;加锁后再次读取答案键,所以并发的同键提交也返回记录而不是报错 | 学习端新增 `/review` 路由与书库、阅读器顶栏的「到期复习」入口;`stores/review.ts` 维护队列、本轮计数、评分与重学,`ReviewCard.vue`/`ReviewView.vue` 呈现正面(词+挖空例句)、答案面(个人释义+例句+三个评分按钮)、完成页与空队列页。一次评分对应一个 `answerId`,失败重试复用同一个;换词后重新生成。切换账号或退出登录会清空队列、计数与当前卡片。 diff --git a/docs/03-business-rules-and-glossary.md b/docs/03-business-rules-and-glossary.md index 9327b3a..43c3e02 100644 --- a/docs/03-business-rules-and-glossary.md +++ b/docs/03-business-rules-and-glossary.md @@ -2,8 +2,8 @@ generated: true (请先修改 Gitea Wiki,禁止直接编辑本文件) wiki_page: Business-Rules-and-Glossary wiki_url: https://git.ilapage.cn/OPC/lexgo/wiki/Business-Rules-and-Glossary.- -wiki_revision: e59184a923910cee9283771154349397e7063fbd -synchronized_at: 2026-09-11T12:54:06Z +wiki_revision: 1d90013d883de9339865694e0bb0026f9edd3acc +synchronized_at: 2026-09-11T14:44:50Z # 业务规则与术语 @@ -181,13 +181,13 @@ exact优先;未命中再按WordNet异常表/词尾规则查候选,词性顺 |---|---|---|---|---|---|---|---| | 下次复习 | 1 天 | 2 天 | 4 天 | 7 天 | 15 天 | 30 天 | 60 天 | -**入队范围**:只有状态 `新词` 或 `学习中` 且 `due_at ≤ now` 的词条进入队列;`已知` 与 `忽略` 不入队。新保存的词立即到期(`due_at` = 保存时刻),保存后就能复习;手动把词条改为「学习中 level N」时下次复习为 `now + 间隔[N]`,避免刚标等级就被当成到期。已在队列中的词条被改成 `已知`/`忽略` 后再次作答会被拒绝(409),排队信息由服务端裁决而不是前端过滤。 +**入队范围**:只有状态 `新词` 或 `学习中` 且 `due_at ≤ now` 的词条进入队列;`已知` 与 `忽略` 不入队。新保存的词立即到期(`due_at` = 保存时刻),保存后就能复习;手动把词条改为「学习中 level N」时下次复习为 `now + 间隔[N]`,避免刚标等级就被当成到期。只有新建词条或状态/等级实际变化才移动复习时间:只修改释义、例句或原词形时保留原有排期,否则编辑文本会把逾期词条挤出当天队列。保存未提及等级时保留已获得的等级,只有进入 `学习中` 才从 1 开始,因此阅读器面板的保存不会把等级重置为 1。已在队列中的词条被改成 `已知`/`忽略` 后再次作答会被拒绝(409),排队信息由服务端裁决而不是前端过滤。 **三个评分动作**:`correct` 认识/答对 → 等级 +1(封顶 7)、按新等级排期、离开本轮;`wrong` 不认识/答错 → `学习中` 降一级(最低 1)、`新词` 保持 `新词`、`due_at = now` 立即回到本轮;`again` 再学一次 → 等级与状态不变、`due_at = now` 立即回到本轮。计数上 `correct_count` 只统计 `correct`,`wrong_count` 统计 `wrong` 与 `again`,`review_count` 统计全部已应用作答。词条的原文、个人释义与例句不因复习改变。 **时区与到期边界**:`due_at` 以 UTC 绝对时刻存储,到期判定是 `due_at ≤ now`,不引入本地日边界。理由:MVP 没有用户时区设置(属 X 系列边界),绝对时刻在多账号自托管下语义一致、没有夏令时陷阱;代价是复习时刻会随首次作答时间漂移,例如 22:00 答对的 1 天间隔词条要到次日 22:00 才到期。原型界面的「到期复习 N / M」指当前到期队列的位置与本轮卡片数,不是自然日统计。 -**幂等与并发**:客户端每次作答生成一个 `answerId`,服务端以 `UNIQUE(owner_id, answer_id)` 去重。同一个 `answerId` 再次提交返回首次结果(`result=duplicate`)且不改变等级、间隔和次数;同一词条在别处已经被推进(提交回传的 `expectedDueAt` 与服务端当前 `due_at` 不一致)时记为 `result=stale`,同样不改变任何状态。因此网络重发、双击、双标签页作答都只记账一次。每次尝试都会落一条 `lexgo_review_answers`(含 `result`),这是「只记账一次」的证据,也供 #13 统计使用。 +**幂等与并发**:客户端每次作答生成一个 `answerId`,服务端以 `UNIQUE(owner_id, answer_id)` 去重。同一个 `answerId` 再次提交返回首次结果并把 `duplicate` 标记为 true(`result` 仍是首次的 `applied` 或 `stale`),不改变等级、间隔和次数,因此客户端重试可以按首次结果计数;同一词条在别处已经被推进(提交回传的 `expectedDueAt` 与服务端当前 `due_at` 不一致)时记为 `result=stale`,同样不改变任何状态。因此网络重发、双击、双标签页作答都只记账一次。每次尝试都会落一条 `lexgo_review_answers`(含 `result`),这是「只记账一次」的证据,也供 #13 统计使用。 **归属与错误**:词条归属由服务端按会话裁决,`owner` 不接受客户端输入;他人词条与不存在的词条统一 404,未登录 401,未知评分/缺少 `expectedDueAt`/未知字段 400,词条已变成 `已知`/`忽略` 409。复习接口不写审计日志:答题属于私人学习内容。 diff --git a/docs/04-local-development-and-verification.md b/docs/04-local-development-and-verification.md index 8778409..dacbc51 100644 --- a/docs/04-local-development-and-verification.md +++ b/docs/04-local-development-and-verification.md @@ -2,8 +2,8 @@ generated: true (请先修改 Gitea Wiki,禁止直接编辑本文件) wiki_page: Local-Development-and-Verification wiki_url: https://git.ilapage.cn/OPC/lexgo/wiki/Local-Development-and-Verification.- -wiki_revision: e01464ad570bbd037e813985d78b137462bca241 -synchronized_at: 2026-09-11T12:54:06Z +wiki_revision: 1192d23a8198e981961adfc6066f0b3bc3b84058 +synchronized_at: 2026-09-11T14:44:50Z # 本地开发与验证 @@ -362,3 +362,21 @@ node --test spikes/english/view.test.mjs 运维记录:本次只有 lexgo-api(新二进制)与 lexgo-learner 被重启;学习端常驻 Vite 在长时间运行后一度对 `/src/style.css` 返回空样式表,导致 E2E 观察到 `white-space: normal`,重启后恢复,未改动代码或配置。其余实例保持 Running。 未验证:真实手机触屏详细证据与完整备份恢复演练仍属既有缺口(#14/#15);本单只用桌面浏览器窄屏检查,不当作真机结果。并发只覆盖到「双标签页同一词条」这一层,没有做多用户压测。 + +## #8 审核整改(R1~R3,2026-09-11) + +独立审核(Claude Code)只读审阅提交 `0328505`/`ec5ec2d`,指出三处影响验收标准第 2、3 条的问题。三点均先在 `lexgo_test_issue8` 写复现用例观察到失败,再修复并转绿。 + +| 问题 | 现象与根因 | 修复 | 回归用例 | +|---|---|---|---| +| R2 并发同键返回 500 | 两个同时到达、带同一 `answerId` 的请求都在加锁前查不到记录;后者进入 stale 分支插入相同 `answer_key`,触发唯一键冲突返回 500 | 取得词条行锁后再用加锁读复查一次答案键,命中直接返回首次结果;stale 插入遇到 1062 也转为返回记录 | `TestMySQLReviewConcurrentReplayOfOneAnswer`(两个 goroutine 同键提交,两个都 2xx、`review_count = 1`、只有一条答案记录) | +| R3 编辑文本会重排复习 | `saveTerm` 无条件调用 `syncTermReview`,编辑释义/例句也会把 `due_at` 重算,逾期词条被挤出当天队列 | 保存前加锁读取旧行,只有新建或状态/等级实际变化才移动 `due_at`;缺行时补建排期行 | `TestMySQLReviewEditKeepsSchedule`(逾期 3 级词只改释义:`due_at` 与队列不变;改等级则重排) | +| R3 附带发现:面板保存把等级重置为 1 | 阅读器面板只提交状态不提交等级,`termLevel` 对缺省等级一律返回 1,于是 4 级词改一个错字会掉到 1 级 | 保存未提及等级时保留已获得的等级;只有进入 `学习中` 才从 1 开始 | `TestMySQLReviewPanelSaveKeepsLevel`(4 级词面板式保存后仍为 4 级,且排期不变;退出再进入学习中则从 1 开始) | + +**答案契约随之明确**:作答响应 `result` 只取 `applied`/`stale`,另加 `duplicate` 布尔标记。重放返回首次结果并把 `duplicate` 置真,客户端因此可以按首次结果计数:网络把响应丢掉后点「重试提交」拿到 `duplicate=true` 的 `applied`,本轮计数正常增加,完成页不会退化成「今天没有到期词条」。已应用的作答返回 201,重放与 stale 返回 200(R1)。 + +**提示与注释(R4、R5)**:卡片因 `stale` 离开时页面显示 `role="status"` 提示「该词已在其他页面复习,本次未计分。」,重放且首次为 stale 时显示「该词已按上一次的评分记录,未重复计分。」;本轮只解决卡片而没有新计分时,完成页显示「本轮没有新的计分:N 个词条已在其他页面复习。」。`answerId` 的作用域注释改为「每张卡片一个,失败重试复用」,与 `answerIdFor` 的实现一致。 + +**流程记录(R6)**:评论 7769 的方案写的是在 `lexgo_terms` 上增加列,实际实现改为独立表 `lexgo_term_reviews`(加法迁移可重试、不对既有表做 ALTER)。该变更在实施评论 7776 与 Wiki 中说明了原因,但没有按「数据结构变化先更新工单」的要求在实施前追加变更评论;本页与上文契约按实际实现记录,方案评论中的「新增列均有默认值」以独立表为准。 + +整改后重跑:Go 单元与集成测试(专用库 `lexgo_test_issue8`)44 个顶层用例全部通过、0 跳过;学习端 73 项单测、类型检查、构建与 5 项 E2E 通过;管理端 31 项与 lint 通过;治理 56 项与严格检查通过;真实 API+MySQL 42 项检查通过(新增 4 项针对 R3 与重放契约);真实浏览器复核面板保存与复习闭环通过。截图 `.local/evidence/issue8-fixed-summary.png`。 diff --git a/learner/src/__tests__/review.spec.ts b/learner/src/__tests__/review.spec.ts index 3d6bc63..3d0edd6 100644 --- a/learner/src/__tests__/review.spec.ts +++ b/learner/src/__tests__/review.spec.ts @@ -69,7 +69,7 @@ describe('review store', () => { expect(review.finished).toBe(false) }) - it('treats a stale or replayed answer as the same action, never as a second review', async () => { + it('treats a stale answer as no new score but finishes the round instead of reporting an empty queue', async () => { vi.spyOn(globalThis, 'fetch').mockImplementation(async input => String(input).endsWith('/reviews/queue') ? ok({ items: [item()], total: 1 }) : ok(answer({ result: 'stale', requeued: false }))) const review = useReviewStore() await review.load() @@ -77,7 +77,33 @@ describe('review store', () => { expect(review.queue).toHaveLength(0) expect(review.answered).toBe(0) expect(review.correctCount).toBe(0) - expect(review.empty).toBe(true) + expect(review.resolved).toBe(1) + expect(review.empty).toBe(false) + expect(review.finished).toBe(true) + expect(review.notice).toContain('已在其他页面复习') + }) + + it('counts a replay of this client own answer after a lost response', async () => { + let sent = 0 + vi.spyOn(globalThis, 'fetch').mockImplementation(async input => { + if (String(input).endsWith('/reviews/queue')) return ok({ items: [item()], total: 1 }) + sent += 1 + // The first response never reaches the client, the retry reports the recorded answer. + if (sent === 1) throw new Error('网络中断') + return ok(answer({ result: 'applied', duplicate: true })) + }) + const review = useReviewStore() + await review.load() + await review.answer('correct') + expect(review.error).toContain('网络中断') + expect(review.answered).toBe(0) + await review.answer('correct') + expect(review.answered).toBe(1) + expect(review.correctCount).toBe(1) + expect(review.wordsReviewed).toBe(1) + expect(review.empty).toBe(false) + expect(review.finished).toBe(true) + expect(review.notice).toBe('') }) it('keeps the card and the same answer id when a submission fails, then retries once', async () => { @@ -203,6 +229,14 @@ describe('review page', () => { expect(view.text()).toContain('还有 2 个词条到期') }) + it('tells the learner when a card was already reviewed elsewhere', async () => { + const { view } = await open({ items: [item()], total: 1 }, () => ok(answer({ result: 'stale', duplicate: false }))) + await view.get('[data-testid="review-reveal"]').trigger('click'); await flushPromises() + await view.get('[data-testid="review-correct"]').trigger('click'); await flushPromises() + expect(view.get('[data-testid="review-notice"]').text()).toContain('已在其他页面复习') + expect(view.get('[data-testid="review-summary"]').text()).toContain('本轮没有新的计分') + }) + it('ends the round without submitting anything', async () => { const { view, router, fetchMock } = await open({ items: [item()], total: 1 }) await view.get('[data-testid="review-end"]').trigger('click'); await flushPromises() diff --git a/learner/src/stores/review.ts b/learner/src/stores/review.ts index 189ea58..7b7ea59 100644 --- a/learner/src/stores/review.ts +++ b/learner/src/stores/review.ts @@ -3,7 +3,9 @@ import { defineStore } from 'pinia' import { useSessionStore } from './session' export type ReviewGrade = 'correct' | 'wrong' | 'again' -export type ReviewResult = 'applied' | 'duplicate' | 'stale' +// The outcome of an answer: applied when the word moved, stale when another screen had +// already reviewed it. A replayed answer repeats the outcome it was given first. +export type ReviewResult = 'applied' | 'stale' export interface ReviewItem { id: number @@ -19,6 +21,7 @@ export interface ReviewItem { export interface ReviewAnswerResult { result: ReviewResult + duplicate: boolean grade: ReviewGrade requeued: boolean statusBefore: string @@ -67,19 +70,25 @@ export const useReviewStore = defineStore('review', () => { const wrongCount = ref(0) // Distinct words in this round: a requeued word is answered again but is one word. const wordsReviewed = ref(0) + // Cards this round took off the queue, counted for every outcome, and the reason a card + // left without a new score. + const resolved = ref(0) + const notice = ref('') const seen = new Set() // Words due beyond the fetched page, reported by the server for this round. const pending = ref(0) let started = ref(false) - // One answer id per card and grade: a retry of the same action keeps its key, so the - // server can answer it from the first outcome instead of counting twice. + // One answer id per card: a retry of a failed submission reuses its key, so the server + // answers the retry from the first outcome instead of counting the same action twice. let attempt: { itemId: number; answerId: string } | null = null let sequence = 0 let generation = 0 const current = computed(() => queue.value[0] ?? null) - const finished = computed(() => started.value && !loading.value && queue.value.length === 0 && answered.value > 0) - const empty = computed(() => started.value && !loading.value && queue.value.length === 0 && answered.value === 0) + // A round that resolved cards is finished even when every answer turned out to be a + // replay or a stale submission; only a round that never had a card is empty. + const finished = computed(() => started.value && !loading.value && queue.value.length === 0 && resolved.value > 0) + const empty = computed(() => started.value && !loading.value && queue.value.length === 0 && resolved.value === 0) watch(() => session.user?.id ?? null, (next, previous) => { if (next !== previous) reset() @@ -105,6 +114,8 @@ export const useReviewStore = defineStore('review', () => { correctCount.value = 0 wrongCount.value = 0 wordsReviewed.value = 0 + resolved.value = 0 + notice.value = '' pending.value = 0 started.value = false attempt = null @@ -118,6 +129,7 @@ export const useReviewStore = defineStore('review', () => { loading.value = true error.value = '' revealed.value = false + notice.value = '' attempt = null try { const result = await session.request('reviews/queue') @@ -133,6 +145,7 @@ export const useReviewStore = defineStore('review', () => { correctCount.value = 0 wrongCount.value = 0 wordsReviewed.value = 0 + resolved.value = 0 seen.clear() } started.value = true @@ -185,7 +198,11 @@ export const useReviewStore = defineStore('review', () => { queue.value = queue.value.filter(entry => entry.id !== item.id) attempt = null revealed.value = false + resolved.value += 1 if (result.result === 'applied') { + // A replay repeats the first outcome, and the first attempt may be this client's own + // submission whose response was lost, so it counts exactly like that attempt. + notice.value = '' answered.value += 1 if (!seen.has(item.id)) { seen.add(item.id) @@ -193,6 +210,10 @@ export const useReviewStore = defineStore('review', () => { } if (result.grade === 'correct') correctCount.value += 1 else wrongCount.value += 1 + } else { + notice.value = result.duplicate + ? '该词已按上一次的评分记录,未重复计分。' + : '该词已在其他页面复习,本次未计分。' } if (result.requeued) { const next = result.item ?? { ...item, level: result.levelAfter, status: result.statusAfter as ReviewItem['status'], dueAt: result.dueAtAfter } @@ -206,7 +227,7 @@ export const useReviewStore = defineStore('review', () => { } return { - queue, current, loading, error, busy, revealed, answered, correctCount, wrongCount, wordsReviewed, pending, - finished, empty, load, reveal, answer, continueRound, reset, + queue, current, loading, error, busy, revealed, answered, correctCount, wrongCount, wordsReviewed, resolved, + notice, pending, finished, empty, load, reveal, answer, continueRound, reset, } }) diff --git a/learner/src/style.css b/learner/src/style.css index 04a7eba..d7fe386 100644 --- a/learner/src/style.css +++ b/learner/src/style.css @@ -128,6 +128,7 @@ a.chapter-name:hover { color: #315c43; text-decoration: underline; } .lookup-saved { color: #2f6b45; font-size: 14px; margin: 12px 0 0; } .lookup-actions { display: flex; gap: 8px; margin-top: 14px; flex-wrap: wrap; } .review-page { max-width: 680px; } +.review-notice { margin: 10px 0 0; padding: 10px 14px; border: 1px solid #d9decf; border-radius: 8px; background: #fbf7ee; color: #6b5b3e; } .review-card, .review-summary { margin-top: 26px; padding: 28px; border: 1px solid #d9decf; border-radius: 14px; background: #fffdf8; } .review-summary h2 { margin-top: 0; font-family: Georgia, serif; font-size: 24px; } .review-word { margin: 12px 0; font-family: Georgia, serif; font-size: 34px; } diff --git a/learner/src/views/ReviewView.vue b/learner/src/views/ReviewView.vue index dd6e3b1..7d55e99 100644 --- a/learner/src/views/ReviewView.vue +++ b/learner/src/views/ReviewView.vue @@ -38,6 +38,7 @@ const requireItem = (value: ReviewItem | null): ReviewItem => value as ReviewIte

到期复习

+

{{ review.notice }}

正在加载…

{{ review.error }}

@@ -45,7 +46,8 @@ const requireItem = (value: ReviewItem | null): ReviewItem => value as ReviewIte

本次复习完成

-

复习了 {{ review.wordsReviewed }} 个词条 · 共 {{ review.answered }} 次作答

+

复习了 {{ review.wordsReviewed }} 个词条 · 共 {{ review.answered }} 次作答

+

本轮没有新的计分:{{ review.resolved }} 个词条已在其他页面复习。

答对 {{ review.correctCount }} · 答错或再学 {{ review.wrongCount }} · 已更新复习计划

还有 {{ review.pending }} 个词条到期。

diff --git a/server/app/lexgo/review.go b/server/app/lexgo/review.go index 0cb34d4..3253749 100644 --- a/server/app/lexgo/review.go +++ b/server/app/lexgo/review.go @@ -5,6 +5,7 @@ import ( "strconv" "time" + driver "github.com/go-sql-driver/mysql" "github.com/gin-gonic/gin" admin "go-admin/app/admin/models" "gorm.io/gorm" @@ -95,7 +96,11 @@ type ReviewAnswerInput struct { } type ReviewAnswerResult struct { - Result string `json:"result"` + // Result is the outcome of this answer: applied when the term moved, stale when another + // screen already advanced it. A replayed answer repeats the outcome it was given first. + Result string `json:"result"` + // Duplicate reports that this answer key was already recorded and nothing changed now. + Duplicate bool `json:"duplicate"` Grade string `json:"grade"` Requeued bool `json:"requeued"` StatusBefore string `json:"statusBefore"` @@ -206,18 +211,25 @@ func ReviewQueueFor(tx *gorm.DB, owner int, language string, now time.Time) (Rev // syncTermReview keeps the scheduling row in step with a saved term. A new word is due // immediately so saving it starts the loop; a word saved straight into a level waits for -// that level's interval, so a manual level is not silently re-queued today. -func syncTermReview(tx *gorm.DB, term Term, now time.Time) error { +// that level's interval, so a manual level is not silently re-queued today. When the caller +// does not ask for a reschedule, only a missing row is created and the existing date and +// counters stay untouched. +func syncTermReview(tx *gorm.DB, term Term, reschedule bool, now time.Time) error { due := stamp(now) if term.Status == termStatusLearning && term.Level >= 1 { due = nextDue(term.Level, now) } - row := TermReview{TermID: term.ID, OwnerID: term.OwnerID, Language: term.Language, DueAt: due} - // Counters are history and survive a status or level change. - return tx.Clauses(clause.OnConflict{ - Columns: []clause.Column{{Name: "term_id"}}, - DoUpdates: clause.Assignments(map[string]any{"due_at": row.DueAt}), - }).Create(&row).Error + var existing TermReview + err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Where("term_id = ?", term.ID).First(&existing).Error + switch { + case errors.Is(err, gorm.ErrRecordNotFound): + return tx.Create(&TermReview{TermID: term.ID, OwnerID: term.OwnerID, Language: term.Language, DueAt: due}).Error + case err != nil: + return err + case reschedule: + return tx.Model(&TermReview{}).Where("term_id = ?", term.ID).Update("due_at", due).Error + } + return nil } func lockedReview(tx *gorm.DB, owner int, term Term) (TermReview, error) { @@ -234,6 +246,15 @@ func lockedReview(tx *gorm.DB, owner int, term Term) (TermReview, error) { return review, err } +// lockedAnswerKey reads one recorded answer under a lock. It is called again after the term +// row is locked, because a locking read sees the newest committed row while the plain read +// before it may still see this transaction's snapshot. +func lockedAnswerKey(tx *gorm.DB, owner int, key string) (ReviewAnswer, error) { + var stored ReviewAnswer + err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Where("owner_id = ? AND answer_key = ?", owner, key).First(&stored).Error + return stored, err +} + // AnswerReview applies one grade to one owned term exactly once. A repeated answer key // returns the first outcome, and an answer whose due time no longer matches (another tab // already advanced the term) is recorded as stale without moving anything. @@ -248,15 +269,9 @@ func AnswerReview(tx *gorm.DB, owner int, termID int64, input ReviewAnswerInput, if err != nil { return ReviewAnswerResult{}, err } - var stored ReviewAnswer - err = tx.Where("owner_id = ? AND answer_key = ?", owner, key).First(&stored).Error - if err == nil { - if stored.TermID != termID { - return ReviewAnswerResult{}, failure(400, "请求编号已用于其他词条") - } - return replayedAnswer(tx, stored) - } - if !errors.Is(err, gorm.ErrRecordNotFound) { + if stored, err := lockedAnswerKey(tx, owner, key); err == nil { + return replayedAnswer(tx, stored, termID) + } else if !errors.Is(err, gorm.ErrRecordNotFound) { return ReviewAnswerResult{}, err } // Ownership first: another account's term and a missing term answer identically. @@ -266,6 +281,13 @@ func AnswerReview(tx *gorm.DB, owner int, termID int64, input ReviewAnswerInput, } else if err != nil { return ReviewAnswerResult{}, err } + // Simultaneous submissions of one answer id are serialized by the term lock above; the + // second one now sees the recorded answer and reports it instead of counting again. + if stored, err := lockedAnswerKey(tx, owner, key); err == nil { + return replayedAnswer(tx, stored, termID) + } else if !errors.Is(err, gorm.ErrRecordNotFound) { + return ReviewAnswerResult{}, err + } review, err := lockedReview(tx, owner, term) if err != nil { return ReviewAnswerResult{}, err @@ -315,7 +337,6 @@ func AnswerReview(tx *gorm.DB, owner int, termID int64, input ReviewAnswerInput, DueAtBefore: before.DueAt, DueAtAfter: next.DueAt, Item: reviewItem(term, review), }, nil } - // staleAnswer records that this attempt changed nothing because the term had already // moved on. It is a normal outcome of two open tabs, not an error. func staleAnswer(tx *gorm.DB, owner int, term Term, review TermReview, input ReviewAnswerInput, now time.Time) (ReviewAnswerResult, error) { @@ -329,6 +350,14 @@ func staleAnswer(tx *gorm.DB, owner int, term Term, review TermReview, input Rev DueAtBefore: review.DueAt, DueAtAfter: review.DueAt, CreatedAt: stamp(now), } if err := tx.Create(&record).Error; err != nil { + // Losing a race to another transaction that recorded this very answer id is not an + // error: report the outcome that was stored first. + var duplicate *driver.MySQLError + if errors.As(err, &duplicate) && duplicate.Number == 1062 { + if stored, readErr := lockedAnswerKey(tx, owner, key); readErr == nil { + return replayedAnswer(tx, stored, term.ID) + } + } return ReviewAnswerResult{}, err } return ReviewAnswerResult{ @@ -338,9 +367,12 @@ func staleAnswer(tx *gorm.DB, owner int, term Term, review TermReview, input Rev }, nil } -// replayedAnswer returns the outcome already recorded for this answer key. Nothing is +// replayedAnswer reports the outcome already recorded for this answer key. Nothing is // advanced a second time, so a resend cannot change counts or intervals. -func replayedAnswer(tx *gorm.DB, stored ReviewAnswer) (ReviewAnswerResult, error) { +func replayedAnswer(tx *gorm.DB, stored ReviewAnswer, termID int64) (ReviewAnswerResult, error) { + if stored.TermID != termID { + return ReviewAnswerResult{}, failure(400, "请求编号已用于其他词条") + } var term Term if err := tx.Where("id = ? AND owner_id = ?", stored.TermID, stored.OwnerID).First(&term).Error; err != nil { return ReviewAnswerResult{}, err @@ -350,7 +382,7 @@ func replayedAnswer(tx *gorm.DB, stored ReviewAnswer) (ReviewAnswerResult, error return ReviewAnswerResult{}, err } return ReviewAnswerResult{ - Result: "duplicate", Grade: stored.Grade, Requeued: stored.Requeued, + Result: stored.Result, Duplicate: true, Grade: stored.Grade, Requeued: stored.Requeued, StatusBefore: stored.StatusBefore, StatusAfter: stored.StatusAfter, LevelBefore: stored.LevelBefore, LevelAfter: stored.LevelAfter, DueAtBefore: stored.DueAtBefore, DueAtAfter: stored.DueAtAfter, Item: reviewItem(term, review), diff --git a/server/app/lexgo/review_test.go b/server/app/lexgo/review_test.go index 5c7776b..2bf1511 100644 --- a/server/app/lexgo/review_test.go +++ b/server/app/lexgo/review_test.go @@ -1,6 +1,7 @@ package lexgo import ( + "sync" "encoding/json" "fmt" "testing" @@ -272,16 +273,17 @@ func TestMySQLReviewAnswerIdempotencyAndStaleTabs(t *testing.T) { t.Fatalf("first tab: %d %#v", code, applied) } code, stale := answerReview(t, r, token, termID, answerBody("tab-two-answer-0002", reviewGradeCorrect, seen)) - if code != 200 || stale.Result != "stale" || stale.Requeued { + if code != 200 || stale.Result != "stale" || stale.Duplicate || stale.Requeued { t.Fatalf("second tab must not advance the term: %d %#v", code, stale) } if stale.LevelAfter != applied.LevelAfter || !stale.DueAtAfter.Equal(applied.DueAtAfter) { t.Fatalf("a stale answer changed state: %#v", stale) } - // A resend of the first answer returns the first outcome without another update. + // A resend of the first answer returns the first outcome, flagged as a replay, without + // another update. The client can therefore count it exactly like the original answer. code, duplicate := answerReview(t, r, token, termID, answerBody("tab-one-answer-0001", reviewGradeCorrect, seen)) - if code != 200 || duplicate.Result != "duplicate" { + if code != 200 || duplicate.Result != "applied" || !duplicate.Duplicate { t.Fatalf("resend: %d %#v", code, duplicate) } if duplicate.LevelAfter != applied.LevelAfter || !duplicate.DueAtAfter.Equal(applied.DueAtAfter) || duplicate.Item.ReviewCount != 1 { @@ -380,3 +382,120 @@ func TestMySQLReviewScheduleOnManualStatus(t *testing.T) { t.Fatalf("a new word is due at once: %#v", queue) } } + +// TestMySQLReviewConcurrentReplayOfOneAnswer reproduces the review finding that two +// simultaneous submissions of one answer id must both get the recorded outcome. +func TestMySQLReviewConcurrentReplayOfOneAnswer(t *testing.T) { + db := testDB(t) + r, clock, _, token, chapter := reviewFixture(t, db, "Dogs went home.") + dogs := saveWord(t, r, token, chapter.ID, 0, 4, termStatusNew, nil) + _, queue := reviewQueue(t, r, token) + seen := queue.Items[0].DueAt + body := answerBody("concurrent-answer-01", reviewGradeCorrect, seen) + + var wg sync.WaitGroup + codes := make(chan int, 2) + results := make(chan string, 2) + for i := 0; i < 2; i++ { + wg.Add(1) + go func() { + defer wg.Done() + code, data := callAPI(t, r, "POST", fmt.Sprintf("/api/v1/reviews/%d/answers", dogs.Term.ID), token, body) + var result ReviewAnswerResult + if data != nil { + json.Unmarshal(data, &result) + } + codes <- code + results <- result.Result + }() + } + wg.Wait() + close(codes) + close(results) + ok, bad := 0, 0 + for code := range codes { + if code == 200 || code == 201 { + ok++ + } else { + bad++ + t.Logf("status %d", code) + } + } + kinds := []string{} + for kind := range results { + kinds = append(kinds, kind) + } + if ok != 2 || bad != 0 { + t.Fatalf("both submissions must succeed: ok=%d bad=%d kinds=%v clock=%v", ok, bad, kinds, *clock) + } + review := termReviewRow(t, db, dogs.Term.ID) + if review.ReviewCount != 1 { + t.Fatalf("concurrent replay counted %d times", review.ReviewCount) + } + var attempts int64 + if err := db.Model(&ReviewAnswer{}).Where("term_id = ?", dogs.Term.ID).Count(&attempts).Error; err != nil || attempts != 1 { + t.Fatalf("concurrent replay logged %d attempts", attempts) + } +} + +// TestMySQLReviewEditKeepsSchedule reproduces the review finding that editing only the +// learner's own text must not move a review date (decision D2). +func TestMySQLReviewEditKeepsSchedule(t *testing.T) { + db := testDB(t) + r, clock, _, token, chapter := reviewFixture(t, db, "Dogs went home.") + level := 3 + learning := saveWord(t, r, token, chapter.ID, 0, 4, termStatusLearning, &level) + // Make the word overdue, as it would be after a missed day. + overdue := clock.AddDate(0, 0, -5) + if err := db.Model(&TermReview{}).Where("term_id = ?", learning.Term.ID).Update("due_at", overdue).Error; err != nil { + t.Fatal(err) + } + // Editing only the personal text must keep the schedule exactly as it was. + code, saved := saveTermAPI(t, r, token, map[string]any{"chapterId": chapter.ID, "start": 0, "end": 4, "definition": "改过的释义", "status": termStatusLearning, "level": 3}) + if code != 200 || saved.Term.Level != 3 { + t.Fatalf("text edit: %d %#v", code, saved) + } + if review := termReviewRow(t, db, learning.Term.ID); !review.DueAt.Equal(overdue) { + t.Fatalf("editing the text moved the due time: %v want %v", review.DueAt, overdue) + } + if _, queue := reviewQueue(t, r, token); queue.Total != 1 { + t.Fatalf("an overdue word must stay due after a text edit: %#v", queue) + } + // Changing the level is a scheduling action and does move the due time. + if code, _ := saveTermAPI(t, r, token, map[string]any{"chapterId": chapter.ID, "start": 0, "end": 4, "status": termStatusLearning, "level": 5}); code != 200 { + t.Fatal("level change failed") + } + if review := termReviewRow(t, db, learning.Term.ID); !review.DueAt.Equal(clock.AddDate(0, 0, 15)) { + t.Fatalf("a level change must reschedule: %v", review.DueAt) + } +} + +// TestMySQLReviewPanelSaveKeepsLevel reproduces the related defect found while checking the +// review: the reader panel saves a status without a level, and that must not reset a level +// the learner already earned. +func TestMySQLReviewPanelSaveKeepsLevel(t *testing.T) { + db := testDB(t) + r, clock, _, token, chapter := reviewFixture(t, db, "Dogs went home.") + level := 4 + learning := saveWord(t, r, token, chapter.ID, 0, 4, termStatusLearning, &level) + overdue := clock.AddDate(0, 0, -2) + if err := db.Model(&TermReview{}).Where("term_id = ?", learning.Term.ID).Update("due_at", overdue).Error; err != nil { + t.Fatal(err) + } + // This is exactly what the reader panel sends: status only, no level. + code, saved := saveTermAPI(t, r, token, map[string]any{"chapterId": chapter.ID, "start": 0, "end": 4, "definition": "改过的释义", "status": termStatusLearning}) + if code != 200 || saved.Term.Level != 4 { + t.Fatalf("a panel save must keep the learned level: %d %#v", code, saved) + } + if review := termReviewRow(t, db, learning.Term.ID); !review.DueAt.Equal(overdue) { + t.Fatalf("a panel save moved the due time: %v want %v", review.DueAt, overdue) + } + // Leaving learning and re-entering it is a real transition and starts at level 1. + if code, _ := saveTermAPI(t, r, token, map[string]any{"chapterId": chapter.ID, "start": 0, "end": 4, "status": termStatusNew}); code != 200 { + t.Fatal("reset to new failed") + } + code, again := saveTermAPI(t, r, token, map[string]any{"chapterId": chapter.ID, "start": 0, "end": 4, "status": termStatusLearning}) + if code != 200 || again.Term.Level != 1 { + t.Fatalf("re-entering learning starts at 1: %d %#v", code, again) + } +} diff --git a/server/app/lexgo/router.go b/server/app/lexgo/router.go index e9b515f..636fc64 100644 --- a/server/app/lexgo/router.go +++ b/server/app/lexgo/router.go @@ -143,7 +143,7 @@ func Router(db *gorm.DB, now func() time.Time) *gin.Engine { } // A replayed or stale answer changed nothing, so it is not a new answer. if c.Request.Method == "POST" && c.FullPath() == "/api/v1/reviews/:termId/answers" { - if answer, ok := data.(ReviewAnswerResult); ok && answer.Result == "applied" { + if answer, ok := data.(ReviewAnswerResult); ok && answer.Result == "applied" && !answer.Duplicate { status = 201 } } diff --git a/server/app/lexgo/terms.go b/server/app/lexgo/terms.go index 1d3bc91..45d7be3 100644 --- a/server/app/lexgo/terms.go +++ b/server/app/lexgo/terms.go @@ -100,21 +100,25 @@ func termView(t Term) TermView { return TermView{t.ID, t.Language, t.Term, t.OriginalForm, t.Definition, splitExamples(t.Examples), t.Status, t.Level, t.CreatedAt, t.UpdatedAt} } -// termLevel enforces the documented status/level boundary: only a learning entry -// carries a level, entering learning defaults to 1, and every other status must -// leave the level at 0. -func termLevel(status string, level *int) (int, error) { +// termLevel enforces the documented status/level boundary: only a learning entry carries a +// level, every other status must leave the level at 0, and entering learning starts at 1. +// A save that does not mention a level keeps the level the learner already earned, so +// editing a definition can never roll a word back to level 1. +func termLevel(status string, level *int, previous Term, exists bool) (int, error) { if !termStatuses[status] { return 0, failure(400, "词语状态无效") } if status == termStatusLearning { - if level == nil || *level == 0 { - return 1, nil + if level != nil && *level != 0 { + if *level < 1 || *level > termLevelMax { + return 0, failure(400, "学习等级须为 1~7") + } + return *level, nil } - if *level < 1 || *level > termLevelMax { - return 0, failure(400, "学习等级须为 1~7") + if exists && previous.Status == termStatusLearning && previous.Level >= 1 { + return previous.Level, nil } - return *level, nil + return 1, nil } if level != nil && *level != 0 { return 0, failure(400, "只有学习中的词语可以设置等级") @@ -193,6 +197,21 @@ func languageOf(tx *gorm.DB, owner int) (string, error) { return space.Language, nil } +// previousTerm reads the row this save is about to change under a lock, so the level and +// the review schedule can be compared with what the learner already had. +func previousTerm(tx *gorm.DB, owner int, language, term string) (Term, bool, error) { + var existing Term + err := tx.Clauses(clause.Locking{Strength: "UPDATE"}). + Where("owner_id = ? AND language = ? AND term = ?", owner, language, term).First(&existing).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return Term{}, false, nil + } + if err != nil { + return Term{}, false, err + } + return existing, true, nil +} + // saveTerm writes one identity with INSERT ... ON DUPLICATE KEY UPDATE: a repeated // save updates the same row instead of adding a second, conflicting record, and // two concurrent saves of the same word still leave exactly one. @@ -219,9 +238,11 @@ func saveTerm(tx *gorm.DB, owner int, language, word string, fields TermFields, if err := tx.Where("owner_id = ? AND language = ? AND term = ?", owner, language, row.Term).First(&stored).Error; err != nil { return TermSave{}, err } - // Saving a word also places it on the review schedule, so the loop from reading to - // reviewing needs no separate step. - if err := syncTermReview(tx, stored, now); err != nil { + // A saved word always owns a schedule row, but the date only moves for a new word or a + // real status/level change: editing a definition must not push a word out of today's + // queue (decision D2). + reschedule := !fields.Exists || fields.PreviousStatus != stored.Status || fields.PreviousLevel != stored.Level + if err := syncTermReview(tx, stored, reschedule, now); err != nil { return TermSave{}, err } return TermSave{termView(stored), insert.RowsAffected == 1}, nil @@ -268,12 +289,16 @@ func attachTerms(tx *gorm.DB, owner int, language string, tokens []TextToken) er return nil } -// TermFields carries the validated learner text and state into storage. +// TermFields carries the validated learner text and state into storage, together with the +// status and level the row had before this save. type TermFields struct { - Definition string - Examples string - Status string - Level int + Definition string + Examples string + Status string + Level int + PreviousStatus string + PreviousLevel int + Exists bool } type TermInput struct { @@ -306,7 +331,17 @@ func registerTermRoutes(v *gin.RouterGroup, protect func(bool, func(*gin.Context if err != nil { return nil, err } - level, err := termLevel(input.Status, input.Level) + language, err := languageOf(tx, u.UserId) + if err != nil { + return nil, err + } + // The previous row decides whether a missing level keeps the earned one and whether + // the review date may move at all. + previous, exists, err := previousTerm(tx, u.UserId, language, normalizeWord(word)) + if err != nil { + return nil, err + } + level, err := termLevel(input.Status, input.Level, previous, exists) if err != nil { return nil, err } @@ -314,11 +349,10 @@ func registerTermRoutes(v *gin.RouterGroup, protect func(bool, func(*gin.Context if err != nil { return nil, err } - language, err := languageOf(tx, u.UserId) - if err != nil { - return nil, err + fields := TermFields{ + Definition: definition, Examples: examples, Status: input.Status, Level: level, + PreviousStatus: previous.Status, PreviousLevel: previous.Level, Exists: exists, } - fields := TermFields{Definition: definition, Examples: examples, Status: input.Status, Level: level} return saveTerm(tx, u.UserId, language, word, fields, now()) })) v.GET("/terms/:id", protect(false, func(c *gin.Context, tx *gorm.DB, u admin.SysUser) (any, error) { diff --git a/server/app/lexgo/terms_test.go b/server/app/lexgo/terms_test.go index 326048a..8bd9aeb 100644 --- a/server/app/lexgo/terms_test.go +++ b/server/app/lexgo/terms_test.go @@ -14,31 +14,44 @@ import ( func TestTermLevelBoundary(t *testing.T) { level := func(v int) *int { return &v } + // The stored state the save is about to change: a level-4 learning word. + stored := Term{Status: termStatusLearning, Level: 4} cases := []struct { - status string - level *int - want int - ok bool + status string + level *int + previous Term + exists bool + want int + ok bool }{ - {termStatusNew, nil, 0, true}, - {termStatusNew, level(0), 0, true}, - {termStatusNew, level(1), 0, false}, - {termStatusKnown, level(0), 0, true}, - {termStatusKnown, level(3), 0, false}, - {termStatusIgnored, level(0), 0, true}, - {termStatusIgnored, level(-1), 0, false}, - {termStatusLearning, nil, 1, true}, - {termStatusLearning, level(0), 1, true}, - {termStatusLearning, level(1), 1, true}, - {termStatusLearning, level(7), 7, true}, - {termStatusLearning, level(8), 0, false}, - {termStatusLearning, level(-1), 0, false}, - {"", nil, 0, false}, - {"Learning", nil, 0, false}, - {"deleted", nil, 0, false}, + {termStatusNew, nil, Term{}, false, 0, true}, + {termStatusNew, level(0), Term{}, false, 0, true}, + {termStatusNew, level(1), Term{}, false, 0, false}, + {termStatusKnown, level(0), stored, true, 0, true}, + {termStatusKnown, level(3), stored, true, 0, false}, + {termStatusIgnored, level(0), stored, true, 0, true}, + {termStatusIgnored, level(-1), stored, true, 0, false}, + {termStatusLearning, nil, Term{}, false, 1, true}, + {termStatusLearning, level(0), Term{}, false, 1, true}, + {termStatusLearning, level(1), stored, true, 1, true}, + {termStatusLearning, level(7), stored, true, 7, true}, + {termStatusLearning, level(8), stored, true, 0, false}, + {termStatusLearning, level(-1), stored, true, 0, false}, + {"", nil, stored, true, 0, false}, + {"Learning", nil, stored, true, 0, false}, + {"deleted", nil, stored, true, 0, false}, } + // A save that does not mention a level keeps the earned one instead of resetting it. + cases = append(cases, struct { + status string + level *int + previous Term + exists bool + want int + ok bool + }{termStatusLearning, nil, stored, true, 4, true}) for _, tc := range cases { - got, err := termLevel(tc.status, tc.level) + got, err := termLevel(tc.status, tc.level, tc.previous, tc.exists) if tc.ok && (err != nil || got != tc.want) { t.Fatalf("%s/%v: got %d, %v; want %d", tc.status, tc.level, got, err, tc.want) }