From c92495c5b97502d8401c8db46a6857e707cfdad9 Mon Sep 17 00:00:00 2001 From: shanshanzhong147 Date: Thu, 9 Jul 2026 08:56:19 -0700 Subject: [PATCH] =?UTF-8?q?=E4=BF=AE=E5=A4=8D(#3):=20=E6=8A=BD=E5=A5=96=20?= =?UTF-8?q?EligibilitySnapshot.UnmetReasons=20=E7=A9=BA=E5=AD=97=E7=AC=A6?= =?UTF-8?q?=E4=B8=B2=E8=BF=9D=E5=8F=8D=20MySQL=20JSON=20=E6=A0=A1=E9=AA=8C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes HIF-3 (Stage 1 P0 from QA smoke) F4 (P0): EligibilitySnapshot.UnmetReasons=\"\" → MySQL error 3140 (Invalid JSON text: The document is empty) - 修法：方式 A 对称守卫 UnmetReasons=\"[]\" (与 PrizeSnapshot.Config 的 \"{}\" 守卫模式一致) - Stage 1 只有 passed=true 分支进 insertSnapshots，语义正确 F5 (P2 顺手): EvaluatedAt 显式 time.Now() 避免 GORM zero time 触 sql_mode STRICT 回归护栏：TestInsertSnapshots_UnmetReasonsIsValidJSON 用 sqlmock per-arg matcher (unmetReasonsNotEmpty + evaluatedAtNotZero)，退化时 t.Fatalf 立即抛出 CI 全绿；2 files, +108/-0；无 DB 变更 架构师侧透明：这是 PR C review 时漏检查——PrizeSnapshot.Config 的空串守卫看到了，但没同步检查 EligibilitySnapshot.UnmetReasons。single-code-path 的错觉是审查盲点，Stage 2 起会 sweep 所有 JSON/字符串写库字段。 --- internal/logic/lottery/draw/service.go | 8 ++ internal/logic/lottery/draw/service_test.go | 100 ++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/internal/logic/lottery/draw/service.go b/internal/logic/lottery/draw/service.go index e2b55e2..3d2d85d 100644 --- a/internal/logic/lottery/draw/service.go +++ b/internal/logic/lottery/draw/service.go @@ -383,6 +383,14 @@ func (s *Service) insertSnapshots(ctx context.Context, tx *gorm.DB, draw *lotter UserId: draw.UserId, ActivityId: draw.ActivityId, Passed: passedEligibility, + // UnmetReasons 是 JSON 列，MySQL 拒绝空字符串（error 3140）—— + // Stage 1 只有 passed=true 进 insertSnapshots，语义上"没有未过项"， + // 用 "[]" 与 PrizeSnapshot.Config 的 "{}" 守卫对称。 + // Stage 2 若开始持久化 passed=false 的失败评估，再改成真正的 marshal。 + UnmetReasons: "[]", + // 显式 time.Now()：GORM 遇 zero time 有时会传 '0000-00-00 00:00:00'， + // 触 sql_mode STRICT。不依赖 DB DEFAULT CURRENT_TIMESTAMP。 + EvaluatedAt: time.Now(), } return tx.WithContext(ctx).Create(&es).Error } diff --git a/internal/logic/lottery/draw/service_test.go b/internal/logic/lottery/draw/service_test.go index 35ddce6..965da8b 100644 --- a/internal/logic/lottery/draw/service_test.go +++ b/internal/logic/lottery/draw/service_test.go @@ -3,6 +3,7 @@ package draw import ( "context" "database/sql" + "database/sql/driver" "encoding/json" "errors" "fmt" @@ -511,3 +512,102 @@ func errAsCode(err error) (uint32, bool) { } return 0, false } + +// TestInsertSnapshots_UnmetReasonsIsValidJSON is the F4 regression guard. +// +// Before PR E, insertSnapshots created lottery_eligibility_snapshot rows with +// UnmetReasons="" — MySQL error 3140 rejects empty strings on JSON columns, +// so every real /draw request 100% failed the tx commit even though sqlmock +// (which does no JSON validation) was happy. This test snapshots the exact +// INSERT arg values and asserts: +// 1. UnmetReasons must never be "" (it should be "[]") +// 2. EvaluatedAt must not be the zero time.Time (STRICT sql_mode rejects +// '0000-00-00 00:00:00' on DATETIME NOT NULL) +// +// sqlmock cannot catch the JSON validity itself — only real MySQL can — but +// it can catch the two upstream bugs that let bad values through the Go +// layer. This is a defense-in-depth check. +func TestInsertSnapshots_UnmetReasonsIsValidJSON(t *testing.T) { + db, mock, cleanup := newTestDB(t) + defer cleanup() + + expectRunningActivity(mock, 100) + expectPrizePool(mock, 100, lottery.Prize{ + Id: 1, ActivityId: 100, Slot: 0, Type: lottery.PrizeTypeNone, Name: "谢谢参与", Config: "{}", Weight: 100, + }) + + mock.ExpectBegin() + expectExistingDrawEmpty(mock) + mock.ExpectExec("INSERT INTO `lottery_draw`"). + WillReturnResult(sqlmock.NewResult(555, 1)) + mock.ExpectExec("INSERT INTO `lottery_prize_snapshot`"). + WillReturnResult(sqlmock.NewResult(1, 1)) + + // Field order matches EligibilitySnapshot struct-tag order under GORM: + // draw_id, user_id, activity_id, passed, unmet_reasons, evaluated_at. + // We assert UnmetReasons=="[]" (never "") and EvaluatedAt is non-zero. + mock.ExpectExec("INSERT INTO `lottery_eligibility_snapshot`"). + WithArgs( + sqlmock.AnyArg(), // draw_id + sqlmock.AnyArg(), // user_id + sqlmock.AnyArg(), // activity_id + sqlmock.AnyArg(), // passed + unmetReasonsNotEmpty{t}, // MUST be "[]" + evaluatedAtNotZero{t}, // MUST be non-zero time + ). + WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec("UPDATE `lottery_draw`"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + + svc := NewService(Deps{ + DB: db, + Enabled: true, + Chance: &fakeChance{consumeRemaining: 2}, + Evaluator: &fakeEvaluator{passed: true}, + Picker: &fakePicker{idx: 0}, + Registry: &stubRegistry{handlers: map[string]lottery.PrizeHandler{}}, + ContextBuilder: fakeContextBuilder{}, + }) + _, err := svc.Draw(context.Background(), Request{UserId: 1, ActivityId: 100, ClientNonce: "f4-regression"}) + if err != nil { + t.Fatalf("Draw: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +// unmetReasonsNotEmpty is a per-arg matcher: the value MUST be the string +// "[]"; the empty string is the exact F4 regression we are guarding against. +type unmetReasonsNotEmpty struct{ t *testing.T } + +func (m unmetReasonsNotEmpty) Match(v driver.Value) bool { + s, ok := v.(string) + if !ok { + m.t.Fatalf("F4 guard: expected string for UnmetReasons, got %T (%v)", v, v) + } + if s == "" { + m.t.Fatalf("F4 regression: UnmetReasons must not be empty string (MySQL error 3140)") + } + if s != "[]" { + m.t.Fatalf("F4 guard: expected UnmetReasons==%q, got %q", "[]", s) + } + return true +} + +// evaluatedAtNotZero is a per-arg matcher: the value MUST be a non-zero +// time.Time; the zero time is the F5 regression that STRICT sql_mode rejects +// as '0000-00-00 00:00:00'. +type evaluatedAtNotZero struct{ t *testing.T } + +func (m evaluatedAtNotZero) Match(v driver.Value) bool { + tv, ok := v.(time.Time) + if !ok { + m.t.Fatalf("F5 guard: expected time.Time for EvaluatedAt, got %T (%v)", v, v) + } + if tv.IsZero() { + m.t.Fatalf("F5 regression: EvaluatedAt must not be zero time") + } + return true +}