From 7a4c9b97d02b3b96eeea1dafbde3414c0427a2ba Mon Sep 17 00:00:00 2001 From: wangjia <809946525@qq.com> Date: Fri, 10 Jul 2026 14:06:01 +0800 Subject: [PATCH] =?UTF-8?q?fix(server):=20migrate=20down=20=E5=85=BC?= =?UTF-8?q?=E5=AE=B9=20codes=20=E5=BA=93=E8=87=AA=E5=BB=BA=E8=A1=A8(?= =?UTF-8?q?=E5=85=88=20DROP=20=E5=86=8D=E6=94=B9=E5=90=8D=E5=9B=9E)+=20?= =?UTF-8?q?=E7=9C=9F=E5=AE=9E=E6=8E=A5=E7=BA=BF=E5=9B=9E=E7=8E=AF=E6=B5=8B?= =?UTF-8?q?=E8=AF=95(#codes-lib)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer 复现:cmd/migrate up 接线 ApplyCodesLibMigrations 后,codes 库自建的 codes/codes_batches/codes_audit_log/codes_schema_migrations 四张表不受 golang-migrate 追踪;000020 的 down 脚本做 legacy_codes -> codes 改名回时, 撞上库自建的同名 codes 表,报 "table already exists" 硬失败。 000020 的 sqlite/mysql down 脚本改名前先 DROP TABLE IF EXISTS 掉库自建四张表, 并注明:down = 完整回滚「rename + 库建表」组合,库表数据随 down 销毁属预期 (权威数据仍在 legacy_* 表,回填是后续任务,回填落地后 down 语义需重新审视)。 新增 TestCodesLibMigrateRoundTrip 走真实接线路径(MigrateUp -> ApplyCodesLibMigrations -> 单步撤销 000020 断言 legacy 复原/库表清空 -> 撤销回去 -> 再跑 ApplyCodesLibMigrations 验幂等 -> 全量 store.MigrateDown 即 cmd/migrate down 的真实调用路径,确认不再硬失败)。先用 git stash 掉修复验证 测试能精确复现 reviewer 报的 "table already exists" 报错(RED),再恢复修复 转绿(GREEN)。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013nMthbVEmQquxBRKb9Fj8u --- .../internal/store/codes_lib_migrate_test.go | 120 ++++++++++++++++++ .../000020_codes_lib_legacy_rename.down.sql | 9 ++ .../000020_codes_lib_legacy_rename.down.sql | 9 ++ 3 files changed, 138 insertions(+) diff --git a/server/internal/store/codes_lib_migrate_test.go b/server/internal/store/codes_lib_migrate_test.go index 1a6e786..4476512 100644 --- a/server/internal/store/codes_lib_migrate_test.go +++ b/server/internal/store/codes_lib_migrate_test.go @@ -2,10 +2,16 @@ package store_test import ( "context" + "database/sql" "testing" + "github.com/golang-migrate/migrate/v4" + migratesqlite "github.com/golang-migrate/migrate/v4/database/sqlite" + "github.com/golang-migrate/migrate/v4/source/iofs" + "github.com/wangjia/pangolin/server/internal/config" "github.com/wangjia/pangolin/server/internal/store" + "github.com/wangjia/pangolin/server/migrations" ) // TestApplyCodesLibMigrations verifies the shared codes library's tables are @@ -39,3 +45,117 @@ func TestApplyCodesLibMigrations(t *testing.T) { } } } + +// newSQLiteStepper builds a golang-migrate instance identical to the one +// internal/store.MigrateUp/MigrateDown use internally (same embedded FS, +// same sqlite driver), except it exposes Steps() so the test can land +// exactly on the 000020 boundary instead of only being able to invoke a +// full up-to-latest / down-to-zero round trip like the public API does. +// Mirrors internal/store/migrate.go's newMigrator — kept in the test +// because that constructor is unexported. +func newSQLiteStepper(t *testing.T, database *sql.DB) *migrate.Migrate { + t.Helper() + src, err := iofs.New(migrations.SQLiteFS, "sqlite") + if err != nil { + t.Fatalf("iofs source: %v", err) + } + drv, err := migratesqlite.WithInstance(database, &migratesqlite.Config{}) + if err != nil { + t.Fatalf("sqlite driver: %v", err) + } + m, err := migrate.NewWithInstance("iofs", src, "sqlite", drv) + if err != nil { + t.Fatalf("new migrator: %v", err) + } + return m +} + +// TestCodesLibMigrateRoundTrip exercises the real wired path end-to-end: +// MigrateUp -> ApplyCodesLibMigrations -> step 000020 back down -> assert +// legacy restored/lib gone -> step back up -> ApplyCodesLibMigrations again +// -> finally the full `store.MigrateDown` (== cmd/migrate's "down" command) +// all the way to version 0, which is the exact call the reviewer reported +// as failing hard. +// +// Reproduces the reviewer-found collision: 000020's down does +// legacy_codes -> codes renames, but the codes-lib's own `codes` table +// (created by ApplyCodesLibMigrations, untracked by golang-migrate) is +// still sitting there, so the rename used to fail with "table already +// exists" (sqlite) / "ALTER TABLE ... table already exists" (mysql +// RENAME TABLE semantics). 000020's down script must DROP the lib-owned +// tables before renaming legacy_* back. +func TestCodesLibMigrateRoundTrip(t *testing.T) { + ctx := context.Background() + db, err := store.Open(&config.Config{Driver: "sqlite", DSN: ":memory:"}) + if err != nil { + t.Fatalf("open: %v", err) + } + defer db.Close() + + // 1. Up + wire in the codes-lib's own tables (the real cmd/migrate up path). + if err := store.MigrateUp(db, "sqlite"); err != nil { + t.Fatalf("MigrateUp: %v", err) + } + if err := store.ApplyCodesLibMigrations(ctx, db, "sqlite"); err != nil { + t.Fatalf("ApplyCodesLibMigrations: %v", err) + } + + // 2. Step exactly 000020 back down (19 <- 20). This is the precise seam + // the reviewer's repro hit: the lib's `codes` table collides with the + // name 000020's down script renames legacy_codes back to. + m := newSQLiteStepper(t, db) + if err := m.Steps(-1); err != nil { + t.Fatalf("step 000020 down: %v (this is the reviewer-reported collision — "+ + "000020 down must DROP the codes-lib tables before renaming legacy_* back)", err) + } + + // 3. Legacy tables restored to their original pangolin names/shape; + // lib-owned tables gone. + for _, tbl := range []string{"codes", "code_batches"} { + var name string + if err := db.QueryRow( + `SELECT name FROM sqlite_master WHERE type='table' AND name=?`, tbl, + ).Scan(&name); err != nil { + t.Errorf("legacy table %q not restored: %v", tbl, err) + } + } + // Original pangolin `codes` shape (plan_id/duration_days), not the + // codes-lib shape (entitlement_kind/entitlement_payload). + if _, err := db.Exec(`SELECT plan_id, duration_days, redeemed_by FROM codes LIMIT 0`); err != nil { + t.Errorf("codes table not restored to pangolin shape: %v", err) + } + for _, tbl := range []string{"codes_batches", "codes_audit_log", "codes_schema_migrations"} { + var name string + err := db.QueryRow( + `SELECT name FROM sqlite_master WHERE type='table' AND name=?`, tbl, + ).Scan(&name) + if err == nil { + t.Errorf("lib table %q should have been dropped by the 000020 down step, still present", tbl) + } else if err != sql.ErrNoRows { + t.Errorf("checking lib table %q gone: %v", tbl, err) + } + } + + // 4. Step 000020 back up + reapply the lib migrations: idempotent, must + // stay green. + if err := m.Steps(1); err != nil { + t.Fatalf("step 000020 up (2nd round-trip): %v", err) + } + if err := store.ApplyCodesLibMigrations(ctx, db, "sqlite"); err != nil { + t.Fatalf("ApplyCodesLibMigrations (2nd round-trip): %v", err) + } + + // 5. Finally, the exact call the reviewer reported as failing hard: + // `cmd/migrate down` == store.MigrateDown, a full rollback to version 0. + // Must succeed cleanly (no "table already exists"). + if err := store.MigrateDown(db, "sqlite"); err != nil { + t.Fatalf("store.MigrateDown (full, == cmd/migrate down): %v", err) + } + v, _, err := store.MigrateVersion(db, "sqlite") + if err != nil { + t.Fatalf("MigrateVersion after full down: %v", err) + } + if v != 0 { + t.Errorf("version after full MigrateDown = %d, want 0", v) + } +} diff --git a/server/migrations/mysql/000020_codes_lib_legacy_rename.down.sql b/server/migrations/mysql/000020_codes_lib_legacy_rename.down.sql index f31d8f0..1dc327c 100644 --- a/server/migrations/mysql/000020_codes_lib_legacy_rename.down.sql +++ b/server/migrations/mysql/000020_codes_lib_legacy_rename.down.sql @@ -1,2 +1,11 @@ +-- down = 完整回滚「rename + 库建表」组合;库表数据随 down 销毁属预期(此时权威数据仍在 +-- legacy_* 表——回填是后续任务,回填落地后 down 语义需在该任务重新审视并注明)。 +-- 先把 codes 库(github.com/wangjia/codes)自建的表全部清掉,给下面的 RENAME 让位 +-- (up 时是库先建表、这里 down 顺序相反:先拆库表再把 legacy_* 改回原名)。 +DROP TABLE IF EXISTS codes_schema_migrations; +DROP TABLE IF EXISTS codes_audit_log; +DROP TABLE IF EXISTS codes; +DROP TABLE IF EXISTS codes_batches; + RENAME TABLE legacy_code_batches TO code_batches; RENAME TABLE legacy_codes TO codes; diff --git a/server/migrations/sqlite/000020_codes_lib_legacy_rename.down.sql b/server/migrations/sqlite/000020_codes_lib_legacy_rename.down.sql index cc0b779..a9aba68 100644 --- a/server/migrations/sqlite/000020_codes_lib_legacy_rename.down.sql +++ b/server/migrations/sqlite/000020_codes_lib_legacy_rename.down.sql @@ -1,3 +1,12 @@ +-- down = 完整回滚「rename + 库建表」组合;库表数据随 down 销毁属预期(此时权威数据仍在 +-- legacy_* 表——回填是后续任务,回填落地后 down 语义需在该任务重新审视并注明)。 +-- 先把 codes 库(github.com/wangjia/codes)自建的表全部清掉,给下面的 RENAME 让位 +-- (up 时是库先建表、这里 down 顺序相反:先拆库表再把 legacy_* 改回原名)。 +DROP TABLE IF EXISTS codes_schema_migrations; +DROP TABLE IF EXISTS codes_audit_log; +DROP TABLE IF EXISTS codes; +DROP TABLE IF EXISTS codes_batches; + DROP INDEX idx_legacy_codes_status; ALTER TABLE legacy_code_batches RENAME TO code_batches; ALTER TABLE legacy_codes RENAME TO codes;