From 68780a845c2fa3bde20c8771bc386afd271b6315 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Tue, 1 Sep 2026 17:45:40 +0800 Subject: [PATCH] =?UTF-8?q?feat=E2=9C=A8:=20register=20migrations=20under?= =?UTF-8?q?=20an=20app=20code=20with=20ForApp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/migrate/migration/init.go | 271 +++++++++++++++++++- cmd/migrate/migration/init_test.go | 390 +++++++++++++++++++++++++++++ 2 files changed, 649 insertions(+), 12 deletions(-) create mode 100644 cmd/migrate/migration/init_test.go diff --git a/cmd/migrate/migration/init.go b/cmd/migrate/migration/init.go index 28f48e0d..ac58b582 100644 --- a/cmd/migrate/migration/init.go +++ b/cmd/migrate/migration/init.go @@ -1,21 +1,37 @@ package migration import ( + "fmt" "log" "path/filepath" "sort" + "strings" "sync" + "time" "gorm.io/gorm" + + common "go-admin/common/models" ) -var Migrate = &Migration{ - version: make(map[string]func(db *gorm.DB, version string) error), +var Migrate = newMigration() + +func newMigration() *Migration { + return &Migration{version: make(map[string]versionEntry)} +} + +// versionEntry is one registered migration plus the app it belongs to. The +// empty app code means the framework itself, which is also what the +// sys_migration.app_code column defaults to, so history written before this +// field existed reads back correctly with no backfill. +type versionEntry struct { + appCode string + fn func(db *gorm.DB, version string) error } type Migration struct { db *gorm.DB - version map[string]func(db *gorm.DB, version string) error + version map[string]versionEntry mutex sync.Mutex } @@ -27,20 +43,247 @@ func (e *Migration) SetDb(db *gorm.DB) { e.db = db } +// SetVersion registers a migration owned by the framework. Signature and +// behaviour are unchanged: every existing call site in version/*.go keeps +// compiling and keeps writing common.Migration{Version: version} with no app +// code, which is the correct meaning of "framework". func (e *Migration) SetVersion(k string, f func(db *gorm.DB, version string) error) { - e.mutex.Lock() - defer e.mutex.Unlock() - e.version[k] = f + e.setVersion(k, "", f) } -func (e *Migration) Migrate() { - versions := make([]string, 0) - for k := range e.version { +func (e *Migration) setVersion(k, appCode string, f func(db *gorm.DB, version string) error) { + e.mutex.Lock() + defer e.mutex.Unlock() + e.version[k] = versionEntry{appCode: appCode, fn: f} +} + +// AppMigrationFunc is the signature of a migration registered through ForApp. +// +// It receives appCode explicitly because the migration - not the framework - +// writes its own completion row, normally as the last statement inside its own +// transaction. That is what makes "the schema change and the record of it +// commit together" true, and the framework cannot insert the row on the +// migration's behalf without giving that up. Handing the code to the function +// is what stops an app's migrations from silently recording themselves as the +// framework's. +type AppMigrationFunc func(db *gorm.DB, version, appCode string) error + +// AppRegistrar is a per-app view over a registry. +type AppRegistrar struct { + m *Migration + appCode string +} + +// FrameworkAppCode is the name migrate status prints for migrations that belong +// to the framework rather than to an app, and the name --app accepts to select +// them. The stored app code for those is the empty string; this is only the +// spelling humans use. It is reserved - ForApp rejects it - so that every group +// heading status prints is also a value --app understands. +const FrameworkAppCode = "core" + +// ForApp returns a registrar that records migrations under code. +// +// The code is lower-cased: sys_migration.version sorts as ASCII, so mixed case +// would order MyApp before crm for no reason a reader could guess, and the two +// spellings would group as two different apps in migrate status. +// +// An empty or reserved code panics rather than falling back to the framework. +// Registration happens in init(), so this fires the first time the binary runs +// anywhere, which is the point: an app whose migrations quietly file themselves +// under the framework is exactly the class of silent failure this work is meant +// to remove. Framework migrations call Migrate.SetVersion directly. +func ForApp(code string) *AppRegistrar { return Migrate.ForApp(code) } + +// ForApp is the same on an explicit registry, which is what tests use. +func (e *Migration) ForApp(code string) *AppRegistrar { + code = NormalizeAppCode(code) + switch code { + case "": + panic("migration.ForApp: empty app code; framework migrations use Migrate.SetVersion") + case FrameworkAppCode: + panic("migration.ForApp: app code " + FrameworkAppCode + " is reserved for the framework") + } + return &AppRegistrar{m: e, appCode: code} +} + +// AppCode reports the code this registrar files migrations under, after +// normalisation. +func (r *AppRegistrar) AppCode() string { return r.appCode } + +// SetVersion registers an app-owned migration under k, which is the bare +// timestamp taken from the file name exactly as framework migrations do. +// +// What reaches sys_migration.version is the namespaced form; the version string +// handed to f is that same namespaced string, so a migration that writes +// common.Migration{Version: version, AppCode: appCode} records the key the +// registry will look for next time. +func (r *AppRegistrar) SetVersion(k string, f AppMigrationFunc) { + key := namespacedKey(r.appCode, k) + r.m.setVersion(key, r.appCode, func(db *gorm.DB, version string) error { + return f(db, version, r.appCode) + }) +} + +// namespacedKey scopes k to appCode so two apps cannot collide on the +// sys_migration.version primary key by minting the same millisecond timestamp. +// Framework migrations (appCode == "") stay bare, matching every version string +// already in production. +func namespacedKey(appCode, k string) string { + if appCode == "" { + return k + } + return appCode + "-" + k +} + +// StatusEntry is one row of migrate status. +type StatusEntry struct { + AppCode string + Version string + Registered bool + Applied bool + ApplyTime *time.Time +} + +// Status merges the in-process registry with sys_migration, so it reports all +// three shapes at once: registered but not applied, registered and applied, and +// applied while nothing registers it any more - a row left behind by a +// migration file that was deleted, or by an app that was uninstalled. +// +// It only reads. Nothing here creates or alters a table, which is what lets +// both `status` and `--dry-run` run against a database without touching it. +func (e *Migration) Status() ([]StatusEntry, error) { + if e.db == nil { + return nil, fmt.Errorf("migration: no database configured") + } + + e.mutex.Lock() + registered := make(map[string]string, len(e.version)) + for k, v := range e.version { + registered[k] = v.appCode + } + e.mutex.Unlock() + + applied := make(map[string]common.Migration) + // A database that has never been migrated has no sys_migration table. + // Reporting everything as pending is the honest answer there; erroring out + // would make status useless in exactly the case it is most wanted. + if e.db.Migrator().HasTable(&common.Migration{}) { + var rows []common.Migration + if err := e.db.Find(&rows).Error; err != nil { + return nil, err + } + for _, r := range rows { + applied[r.Version] = r + } + } + + versions := make(map[string]struct{}, len(registered)+len(applied)) + for k := range registered { + versions[k] = struct{}{} + } + for k := range applied { + versions[k] = struct{}{} + } + list := make([]string, 0, len(versions)) + for k := range versions { + list = append(list, k) + } + sort.Strings(list) + + out := make([]StatusEntry, 0, len(list)) + for _, v := range list { + entry := StatusEntry{Version: v} + if code, ok := registered[v]; ok { + entry.Registered = true + entry.AppCode = code + } + if row, ok := applied[v]; ok { + entry.Applied = true + t := row.ApplyTime + entry.ApplyTime = &t + if !entry.Registered { + // Nothing registers this version any more, so the database is + // the only source left for what it belonged to. + entry.AppCode = row.AppCode + } + } + out = append(out, entry) + } + return out, nil +} + +// Migrate applies every registered migration that has not been applied yet, +// across all apps. Existing callers are unaffected. +func (e *Migration) Migrate() { e.run(allApps) } + +// MigrateApp applies only the migrations registered under appCode. Pass +// FrameworkAppCode for the framework's own migrations. +func (e *Migration) MigrateApp(appCode string) { e.run(AppFilter(appCode)) } + +// NormalizeAppCode applies the same rule ForApp does, so a code typed on the +// command line matches one written in an init(). +func NormalizeAppCode(code string) string { + return strings.ToLower(strings.TrimSpace(code)) +} + +// AppFilter turns a code as typed into the code stored in the registry, so +// "core" selects the framework's migrations, whose stored code is empty. +func AppFilter(code string) string { + code = NormalizeAppCode(code) + if code == FrameworkAppCode { + return "" + } + return code +} + +// DisplayAppCode is the inverse: what to print for a stored code. +func DisplayAppCode(code string) string { + if code == "" { + return FrameworkAppCode + } + return code +} + +// AppCodes lists the app codes with at least one registered migration, framework +// included under its display name, sorted. +func (e *Migration) AppCodes() []string { + e.mutex.Lock() + seen := map[string]struct{}{} + for _, v := range e.version { + seen[DisplayAppCode(v.appCode)] = struct{}{} + } + e.mutex.Unlock() + + out := make([]string, 0, len(seen)) + for code := range seen { + out = append(out, code) + } + sort.Strings(out) + return out +} + +func (e *Migration) run(appCode string) { + e.mutex.Lock() + versions := make([]string, 0, len(e.version)) + entries := make(map[string]versionEntry, len(e.version)) + for k, v := range e.version { + if appCode != allApps && v.appCode != appCode { + continue + } versions = append(versions, k) + entries[k] = v } - if !sort.StringsAreSorted(versions) { - sort.Strings(versions) + e.mutex.Unlock() + sort.Strings(versions) + + // A mistyped --app would otherwise select nothing and report "no + // migrations to apply", which reads exactly like "already up to date". + if appCode != allApps && len(versions) == 0 { + log.Printf("no migrations are registered for app %q; registered: %s", + DisplayAppCode(appCode), strings.Join(e.AppCodes(), ", ")) + return } + var err error var count int64 applied := 0 @@ -56,7 +299,7 @@ func (e *Migration) Migrate() { continue } log.Printf("applying migration %s", v) - if err = (e.version[v])(e.db.Debug(), v); err != nil { + if err = entries[v].fn(e.db.Debug(), v); err != nil { log.Fatalf("migration %s failed: %v", v, err) } applied++ @@ -68,6 +311,10 @@ func (e *Migration) Migrate() { } } +// allApps is the sentinel run() takes to mean "do not filter". It is distinct +// from the empty app code, which selects the framework's own migrations. +const allApps = "\x00all" + func GetFilename(s string) string { s = filepath.Base(s) return s[:13] diff --git a/cmd/migrate/migration/init_test.go b/cmd/migrate/migration/init_test.go new file mode 100644 index 00000000..ec59a8d6 --- /dev/null +++ b/cmd/migrate/migration/init_test.go @@ -0,0 +1,390 @@ +package migration + +import ( + "bytes" + "log" + "os" + "strings" + "testing" + "time" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + common "go-admin/common/models" +) + +func newTestDB(t *testing.T) *gorm.DB { + t.Helper() + db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{ + Logger: logger.Default.LogMode(logger.Silent), + }) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + if err = db.AutoMigrate(&common.Migration{}); err != nil { + t.Fatalf("automigrate: %v", err) + } + return db +} + +// recordFor is what an app's migration is expected to do: write its own +// completion row, with the version it was handed and the app code it was told +// it belongs to. +func recordFor(db *gorm.DB, version, appCode string) error { + return db.Create(&common.Migration{Version: version, AppCode: appCode}).Error +} + +func rowsByVersion(t *testing.T, db *gorm.DB) map[string]common.Migration { + t.Helper() + var rows []common.Migration + if err := db.Find(&rows).Error; err != nil { + t.Fatalf("read sys_migration: %v", err) + } + out := make(map[string]common.Migration, len(rows)) + for _, r := range rows { + out[r.Version] = r + } + return out +} + +// Acceptance 9: a migration registered through ForApp("x") lands in +// sys_migration with app_code "x". +// +// The registry cannot write that row for the migration, because the row is the +// migration's own last statement inside its own transaction. So the only thing +// that can make this true is handing the code to the function - which is why +// AppMigrationFunc takes three parameters. +func TestForAppRecordsItsAppCode(t *testing.T) { + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + m.ForApp("x").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { + return recordFor(db, version, appCode) + }) + m.Migrate() + + rows := rowsByVersion(t, db) + row, ok := rows["x-1786800001000"] + if !ok { + t.Fatalf("no row for x-1786800001000; got %v", rows) + } + if row.AppCode != "x" { + t.Errorf("app_code = %q, want %q", row.AppCode, "x") + } +} + +// The framework path is untouched: same signature, and an empty app code, which +// is what the column defaults to and what every row written before this field +// existed reads back as. +func TestSetVersionStillRecordsTheFrameworkAsEmpty(t *testing.T) { + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + m.SetVersion("1786700009000", func(db *gorm.DB, version string) error { + return db.Create(&common.Migration{Version: version}).Error + }) + m.Migrate() + + rows := rowsByVersion(t, db) + row, ok := rows["1786700009000"] + if !ok { + t.Fatalf("no row for 1786700009000; got %v", rows) + } + if row.AppCode != "" { + t.Errorf("app_code = %q, want empty (framework)", row.AppCode) + } +} + +// Acceptance 12: --app x runs x's migrations and touches nothing else. +func TestMigrateAppRunsOnlyThatApp(t *testing.T) { + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + ran := map[string]bool{} + m.SetVersion("1786700009000", func(db *gorm.DB, version string) error { + ran["core"] = true + return db.Create(&common.Migration{Version: version}).Error + }) + m.ForApp("x").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { + ran["x"] = true + return recordFor(db, version, appCode) + }) + m.ForApp("y").SetVersion("1786800002000", func(db *gorm.DB, version, appCode string) error { + ran["y"] = true + return recordFor(db, version, appCode) + }) + + m.MigrateApp("x") + + if !ran["x"] { + t.Error("x did not run") + } + if ran["y"] || ran["core"] { + t.Errorf("MigrateApp(x) also ran %v", ran) + } + rows := rowsByVersion(t, db) + if len(rows) != 1 { + t.Fatalf("sys_migration has %d rows, want 1: %v", len(rows), rows) + } +} + +// "core" is what status prints for the framework, so --app core has to select +// it. The stored code is the empty string; AppFilter is the translation. +func TestMigrateAppCoreSelectsTheFramework(t *testing.T) { + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + ran := map[string]bool{} + m.SetVersion("1786700009000", func(db *gorm.DB, version string) error { + ran["core"] = true + return db.Create(&common.Migration{Version: version}).Error + }) + m.ForApp("x").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { + ran["x"] = true + return recordFor(db, version, appCode) + }) + + m.MigrateApp(FrameworkAppCode) + + if !ran["core"] { + t.Error("framework migration did not run") + } + if ran["x"] { + t.Error("--app core also ran x") + } +} + +// Zero-argument Migrate keeps meaning "everything", which is what every +// existing caller relies on. +func TestMigrateRunsEveryApp(t *testing.T) { + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + var order []string + m.SetVersion("1786700009000", func(db *gorm.DB, version string) error { + order = append(order, version) + return db.Create(&common.Migration{Version: version}).Error + }) + m.ForApp("bbb").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { + order = append(order, version) + return recordFor(db, version, appCode) + }) + m.ForApp("aaa").SetVersion("1786800002000", func(db *gorm.DB, version, appCode string) error { + order = append(order, version) + return recordFor(db, version, appCode) + }) + + m.Migrate() + + // Namespacing puts every framework migration - bare digits - ahead of every + // app migration, and orders apps by code rather than by whose timestamp + // happened to be smaller. aaa's file is the newer of the two and still runs + // first. Cross-app order is not promised, but this is the order, and it is + // the one to notice changed. + want := []string{"1786700009000", "aaa-1786800002000", "bbb-1786800001000"} + if len(order) != len(want) { + t.Fatalf("ran %v, want %v", order, want) + } + for i := range want { + if order[i] != want[i] { + t.Fatalf("ran %v, want %v", order, want) + } + } +} + +// Two apps minting the same millisecond timestamp used to mean one of them was +// read as already applied and silently skipped. The namespace prefix is what +// makes that impossible without changing the primary key. +func TestNamespacingKeepsTwoAppsWithTheSameTimestampApart(t *testing.T) { + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + const sameTimestamp = "1786800001000" + ran := 0 + for _, app := range []string{"crm", "oms"} { + m.ForApp(app).SetVersion(sameTimestamp, func(db *gorm.DB, version, appCode string) error { + ran++ + return recordFor(db, version, appCode) + }) + } + m.Migrate() + + if ran != 2 { + t.Errorf("ran %d migrations, want 2", ran) + } + rows := rowsByVersion(t, db) + for _, want := range []string{"crm-" + sameTimestamp, "oms-" + sameTimestamp} { + if _, ok := rows[want]; !ok { + t.Errorf("missing %s; got %v", want, rows) + } + } +} + +func TestNamespacedKeyLeavesFrameworkVersionsBare(t *testing.T) { + if got := namespacedKey("", "1786700009000"); got != "1786700009000" { + t.Errorf("framework version was rewritten to %q", got) + } + if got := namespacedKey("crm", "1786800001000"); got != "crm-1786800001000" { + t.Errorf("namespacedKey = %q", got) + } +} + +// An app code differing only in case would group as two apps in status and sort +// before every lower-case one, for no reason a reader could guess. +func TestForAppNormalisesTheCode(t *testing.T) { + m := newMigration() + if got := m.ForApp(" CRM ").AppCode(); got != "crm" { + t.Errorf("AppCode = %q, want crm", got) + } +} + +func TestForAppRejectsReservedCodes(t *testing.T) { + for _, code := range []string{"", " ", FrameworkAppCode, "CORE"} { + t.Run("code="+code, func(t *testing.T) { + defer func() { + if recover() == nil { + t.Errorf("ForApp(%q) did not panic", code) + } + }() + newMigration().ForApp(code) + }) + } +} + +func TestStatusReportsPendingAppliedAndOrphaned(t *testing.T) { + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + applied := time.Date(2026, 8, 25, 14, 3, 11, 0, time.UTC) + if err := db.Create(&common.Migration{Version: "1786700009000", ApplyTime: applied}).Error; err != nil { + t.Fatal(err) + } + // Recorded, but nothing registers it any more. + if err := db.Create(&common.Migration{Version: "gone-1786800000000", ApplyTime: applied, AppCode: "gone"}).Error; err != nil { + t.Fatal(err) + } + m.SetVersion("1786700009000", func(db *gorm.DB, version string) error { return nil }) + m.ForApp("crm").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { return nil }) + + entries, err := m.Status() + if err != nil { + t.Fatal(err) + } + byVersion := map[string]StatusEntry{} + for _, e := range entries { + byVersion[e.Version] = e + } + + if e := byVersion["1786700009000"]; !e.Applied || !e.Registered || e.AppCode != "" { + t.Errorf("framework entry = %+v", e) + } else if e.ApplyTime == nil || !e.ApplyTime.Equal(applied) { + t.Errorf("framework apply time = %v, want %v", e.ApplyTime, applied) + } + if e := byVersion["crm-1786800001000"]; e.Applied || !e.Registered || e.AppCode != "crm" { + t.Errorf("crm entry = %+v", e) + } + if e := byVersion["gone-1786800000000"]; !e.Applied || e.Registered || e.AppCode != "gone" { + t.Errorf("orphaned entry = %+v", e) + } +} + +// Acceptance 11 rests on this: status and --dry-run both go through Status, and +// Status must not create the table it reads. +func TestStatusDoesNotCreateItsTable(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{ + Logger: logger.Default.LogMode(logger.Silent), + }) + if err != nil { + t.Fatal(err) + } + m := newMigration() + m.SetDb(db) + m.ForApp("crm").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { return nil }) + + entries, err := m.Status() + if err != nil { + t.Fatalf("Status on a database with no sys_migration: %v", err) + } + if len(entries) != 1 || entries[0].Applied { + t.Errorf("entries = %+v, want one pending", entries) + } + if db.Migrator().HasTable(&common.Migration{}) { + t.Error("Status created sys_migration; it must only read") + } +} + +// The completion row is the migration's own last statement, inside its own +// transaction. A migration that fails must leave no record of having run, or +// the next run skips it and the schema stays half-changed with nothing to say +// so. +func TestFailedMigrationLeavesNoRecord(t *testing.T) { + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + m.ForApp("crm").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { + return db.Transaction(func(tx *gorm.DB) error { + if err := recordFor(tx, version, appCode); err != nil { + return err + } + return errTestMigrationFailed + }) + }) + + // run() calls log.Fatal on failure, which would take the test binary with + // it, so drive the registered function directly - the point here is the + // transaction boundary, not the scheduler. + entry := m.version["crm-1786800001000"] + if err := entry.fn(db, "crm-1786800001000"); err == nil { + t.Fatal("migration reported success") + } + if rows := rowsByVersion(t, db); len(rows) != 0 { + t.Errorf("sys_migration has %v after a failed migration", rows) + } +} + +var errTestMigrationFailed = &testError{"boom"} + +type testError struct{ s string } + +func (e *testError) Error() string { return e.s } + +// A mistyped --app used to select nothing and print "no migrations to apply", +// which reads as "already up to date" - the command reports success and does +// nothing, which is the failure mode this whole batch exists to remove. +func TestMigrateAppOnAnUnknownCodeSaysSo(t *testing.T) { + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + m.SetVersion("1786700009000", func(db *gorm.DB, version string) error { + return db.Create(&common.Migration{Version: version}).Error + }) + m.ForApp("crm").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { + return recordFor(db, version, appCode) + }) + + var buf bytes.Buffer + log.SetOutput(&buf) + t.Cleanup(func() { log.SetOutput(os.Stderr) }) + + m.MigrateApp("crmm") + + if !strings.Contains(buf.String(), `no migrations are registered for app "crmm"`) { + t.Errorf("output = %q", buf.String()) + } + if !strings.Contains(buf.String(), "registered: core, crm") { + t.Errorf("the message must list what is registered; got %q", buf.String()) + } + if rows := rowsByVersion(t, db); len(rows) != 0 { + t.Errorf("a typo ran %v", rows) + } +}