fix: 整改 #8 审核问题 R1~R3 (#8)

- 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 并同步镜像
This commit is contained in:
ila
2026-09-11 22:46:23 +08:00
parent ec5ec2db35
commit 325849816e
12 changed files with 362 additions and 88 deletions
+54 -22
View File
@@ -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),
+122 -3
View File
@@ -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)
}
}
+1 -1
View File
@@ -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
}
}
+56 -22
View File
@@ -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) {
+34 -21
View File
@@ -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)
}