diff --git a/app/admin/models/sys_app.go b/app/admin/models/sys_app.go index 72008266..4e1c5aec 100644 --- a/app/admin/models/sys_app.go +++ b/app/admin/models/sys_app.go @@ -6,6 +6,23 @@ import ( "go-admin/common/models" ) +// The values sys_app.status takes. +// +// Three states rather than a single "installed", because an install that +// stopped partway has to be an observable row rather than the absence of one: +// the versions an app installs are separate migration files, and on MySQL a +// DDL statement commits the transaction around it - taking an outer +// transaction and every savepoint under it with it - so they cannot be +// wrapped in one. +// +// AppInstalling is also what a row reads as after the process was killed +// mid-install, which is why it is not treated as "installed" by anything. +const ( + AppInstalling = 1 + AppInstalled = 2 + AppFailed = 3 +) + // SysApp is the sys_app row model: one row per installed application (PRD // 008 F2). It deliberately does not embed models.ModelTime - see the design // doc (docs-prd/008-应用清单与安装器/数据库变更.md) §1.1 for why an diff --git a/app/admin/models/sys_menu.go b/app/admin/models/sys_menu.go index 1fdddc57..51adc8ec 100644 --- a/app/admin/models/sys_menu.go +++ b/app/admin/models/sys_menu.go @@ -44,7 +44,18 @@ type SysMenu struct { // unique index below: the database never treats two NULLs as equal, so // only rows that do carry a real code participate in the uniqueness // check at all. - SeedCode *string `json:"seedCode" gorm:"size:64;uniqueIndex:uk_sys_menu_app_seed_code_del;comment:raw MenuSpec.Code, null for rows not written through SeedMenus"` + // uk_sys_menu_app_seed_code_del is created by the migration, not from + // this tag, and deliberately: it covers (app_code, seed_code, + // deleted_at), and this struct cannot say so. A named uniqueIndex tag + // puts every field carrying that name into one index, and deleted_at + // comes from the shared ModelTime embed, which no single model can add a + // tag to. Naming it here anyway declared a unique index on seed_code + // alone under the same name - stricter than the real one, forbidding two + // applications from both having a "dir" node - and AutoMigrate on this + // model would have created that one first, after which the migration's + // HasIndex guard finds the name taken and leaves the wrong index in + // place. + SeedCode *string `json:"seedCode" gorm:"size:64;comment:raw MenuSpec.Code, null for rows not written through SeedMenus"` models.ControlBy models.ModelTime } diff --git a/app/admin/models/sys_menu_seed_code_test.go b/app/admin/models/sys_menu_seed_code_test.go new file mode 100644 index 00000000..aa8d125e --- /dev/null +++ b/app/admin/models/sys_menu_seed_code_test.go @@ -0,0 +1,49 @@ +package models + +import ( + "testing" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" +) + +func ptr(s string) *string { return &s } + +// uk_sys_menu_app_seed_code_del covers (app_code, seed_code, deleted_at) and +// is created by 1786700008000, not from a struct tag. It cannot come from a +// tag: a named uniqueIndex collects every field carrying that name, and +// deleted_at lives in the shared ModelTime embed that no single model can tag. +// +// Naming it on SeedCode alone anyway produced a unique index on seed_code by +// itself under the same name - stricter than the real one - and AutoMigrate +// here would create that one, after which the migration's HasIndex guard +// finds the name taken and leaves the wrong index in place. Nothing in +// production AutoMigrates this model (the initial table migration uses a +// frozen snapshot that has neither column), which is why this never showed up +// as a broken database; it showed up the first time a test built the schema +// from the live model and seeded two applications. +func TestSysMenuDeclaresNoSeedCodeIndexOfItsOwn(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{ + Logger: logger.Default.LogMode(logger.Silent), + }) + if err != nil { + t.Fatalf("open: %v", err) + } + if err := db.AutoMigrate(&SysMenu{}); err != nil { + t.Fatalf("automigrate: %v", err) + } + if db.Migrator().HasIndex(&SysMenu{}, "uk_sys_menu_app_seed_code_del") { + t.Error("AutoMigrate created uk_sys_menu_app_seed_code_del from a tag; " + + "the migration's HasIndex guard will now skip the composite index it should create") + } + + // Two applications, the same seed code. The real index allows it because + // app_code is part of the key; an index on seed_code alone does not. + for _, app := range []string{"order", "crm"} { + row := SysMenu{MenuName: app + "Dir", AppCode: app, SeedCode: ptr("dir")} + if err := db.Create(&row).Error; err != nil { + t.Fatalf("%s could not use the seed code \"dir\": %v", app, err) + } + } +} diff --git a/app/admin/service/seed.go b/app/admin/service/seed.go index e856566e..0e73a59a 100644 --- a/app/admin/service/seed.go +++ b/app/admin/service/seed.go @@ -5,6 +5,7 @@ import ( "fmt" "strconv" "strings" + "time" "gorm.io/gorm" @@ -76,7 +77,7 @@ func (adminSeeder) SeedMenus(tx *gorm.DB, appCode string, menus []seed.MenuSpec, if len(menuIDs) == 0 && len(apiRows) == 0 { return nil } - return grantToAdminRole(tx, menuIDs, apiRows) + return grantToAdminRole(tx, appCode, menuIDs, apiRows) } // seedApis writes one sys_api row per ApiSpec and returns them keyed by @@ -422,7 +423,7 @@ func pascalCase(s string) string { // framework migration sorts before every app-prefixed one - means that // should not happen in practice, but failing this call over it would be // worse than a menu with no grant yet. -func grantToAdminRole(tx *gorm.DB, menuIDs []int, apiRows map[string]models.SysApi) error { +func grantToAdminRole(tx *gorm.DB, appCode string, menuIDs []int, apiRows map[string]models.SysApi) error { var role models.SysRole if err := tx.Where("role_key = ?", adminRoleKey).First(&role).Error; err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { @@ -441,12 +442,53 @@ func grantToAdminRole(tx *gorm.DB, menuIDs []int, apiRows map[string]models.SysA } for _, a := range apiRows { - if err := tx.Exec( + res := tx.Exec( "INSERT INTO casbin_rule (ptype, v0, v1, v2, v3, v4, v5) SELECT 'p', ?, ?, ?, '', '', '' WHERE NOT EXISTS (SELECT 1 FROM casbin_rule WHERE ptype='p' AND v0=? AND v1=? AND v2=?)", role.RoleKey, a.Path, a.Action, role.RoleKey, a.Path, a.Action, - ).Error; err != nil { + ) + if res.Error != nil { + return res.Error + } + if res.RowsAffected == 0 { + // The policy was already there, so this install did not create + // it and it is not this app's to take away. Leaving it out of + // the ledger is what makes an uninstall report it instead of + // deleting it. + // + // The two ways this can be wrong are not equally bad, which is + // what settles it. Under-recording leaves a policy behind and + // the uninstall says so, because a policy naming an app's own + // path with no ledger entry is exactly what it lists as an + // orphan. Over-recording deletes a grant somebody else made, + // silently. Between a visible leftover and an invisible + // deletion of somebody's authorization, take the leftover. + continue + } + if err := recordGrant(tx, appCode, role.RoleKey, a.Path, a.Action); err != nil { return err } } return nil } + +// recordGrant writes down that this application's install created one casbin +// policy, keyed by the same tuple casbin_rule is unique on. +// +// A ledger rather than a column on casbin_rule, because casbin_rule is not +// this project's table: the gorm adapter's SavePolicy truncates it and writes +// it back from memory, which would drop any column added here, and +// SysRole.Update replaces a role's policy rows wholesale. The tuple survives +// both, because both rebuild it from the same sys_menu/sys_api data. +// +// Written with the same INSERT ... WHERE NOT EXISTS shape as the policy above +// rather than a plain insert: the ledger's unique index covers the tuple +// alone, so a duplicate would abort the whole seed instead of being the +// no-op it should be. +func recordGrant(tx *gorm.DB, appCode, roleKey, path, action string) error { + return tx.Exec( + "INSERT INTO sys_app_casbin_grant (app_code, ptype, v0, v1, v2, v3, v4, v5, created_at) "+ + "SELECT ?, 'p', ?, ?, ?, '', '', '', ? WHERE NOT EXISTS "+ + "(SELECT 1 FROM sys_app_casbin_grant WHERE ptype='p' AND v0=? AND v1=? AND v2=? AND v3='' AND v4='' AND v5='')", + appCode, roleKey, path, action, time.Now(), roleKey, path, action, + ).Error +} diff --git a/app/admin/service/seed_test.go b/app/admin/service/seed_test.go index 9fc02557..de220b75 100644 --- a/app/admin/service/seed_test.go +++ b/app/admin/service/seed_test.go @@ -32,7 +32,14 @@ func newSeedTestDB(t *testing.T) *gorm.DB { if err != nil { t.Fatalf("open: %v", err) } - if err := db.AutoMigrate(&models.SysMenu{}, &models.SysApi{}, &models.SysRole{}); err != nil { + // sys_app_casbin_grant is where grantToAdminRole records which policies + // this install created, so an uninstall can tell them from the ones + // somebody granted by hand. In a real database it is created by + // 1786700007000, which is a framework migration and therefore runs ahead + // of every application's - version strings sort bare digits before any + // app-prefixed one. + if err := db.AutoMigrate(&models.SysMenu{}, &models.SysApi{}, &models.SysRole{}, + &models.SysAppCasbinGrant{}); err != nil { t.Fatalf("automigrate: %v", err) } if err := db.Exec(`CREATE TABLE casbin_rule ( @@ -841,3 +848,138 @@ func TestSeedMenusPreservesAHandAddedBinding(t *testing.T) { t.Errorf("the seed's own binding count = %d, want 1 - it must survive the retry too", n) } } + +// Every policy grantToAdminRole creates has to be written down, or an +// uninstall has no way to tell this app's grants from a hand-made one and +// leaves all of them behind. +func TestSeedMenusRecordsTheGrantsItCreated(t *testing.T) { + db := newSeedTestDB(t) + role := seedAdminRole(t, db) + + apis := []seed.ApiSpec{ + {Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET", Handle: "apis.Order.GetPage-fm"}, + {Code: "create", Title: "Create order", Path: "/api/v1/order", Method: "POST", Handle: "apis.Order.Insert-fm"}, + } + if err := (adminSeeder{}).SeedMenus(db, "order", nil, apis); err != nil { + t.Fatalf("SeedMenus: %v", err) + } + + for _, a := range apis { + var n int64 + db.Model(&models.SysAppCasbinGrant{}). + Where("app_code = ? AND ptype = 'p' AND v0 = ? AND v1 = ? AND v2 = ?", + "order", role.RoleKey, a.Path, a.Method). + Count(&n) + if n != 1 { + t.Errorf("ledger rows for %s %s = %d, want 1", a.Method, a.Path, n) + } + } +} + +// A policy that was already there was not created by this install, so it is +// not this app's to take away later. Recording it would mean an uninstall +// deletes a grant somebody else made, and deletes it silently - the opposite +// mistake leaves a policy behind, which the uninstall reports. +func TestSeedMenusDoesNotClaimAPolicyItDidNotCreate(t *testing.T) { + db := newSeedTestDB(t) + role := seedAdminRole(t, db) + + if err := db.Exec( + "INSERT INTO casbin_rule (ptype, v0, v1, v2, v3, v4, v5) VALUES ('p', ?, '/api/v1/order', 'GET', '', '', '')", + role.RoleKey, + ).Error; err != nil { + t.Fatalf("pre-existing policy: %v", err) + } + + apis := []seed.ApiSpec{ + {Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET", Handle: "apis.Order.GetPage-fm"}, + {Code: "create", Title: "Create order", Path: "/api/v1/order", Method: "POST", Handle: "apis.Order.Insert-fm"}, + } + if err := (adminSeeder{}).SeedMenus(db, "order", nil, apis); err != nil { + t.Fatalf("SeedMenus: %v", err) + } + + var claimed int64 + db.Model(&models.SysAppCasbinGrant{}). + Where("v1 = ? AND v2 = ?", "/api/v1/order", "GET").Count(&claimed) + if claimed != 0 { + t.Errorf("the ledger claimed a policy that was already there (%d rows)", claimed) + } + // The one it did create is still recorded: the skip is per policy, not + // for the whole call. + var created int64 + db.Model(&models.SysAppCasbinGrant{}). + Where("v1 = ? AND v2 = ?", "/api/v1/order", "POST").Count(&created) + if created != 1 { + t.Errorf("ledger rows for the policy it did create = %d, want 1", created) + } + // And the pre-existing policy itself is untouched. + var policies int64 + db.Table("casbin_rule").Where("v1 = ? AND v2 = ?", "/api/v1/order", "GET").Count(&policies) + if policies != 1 { + t.Errorf("casbin_rule rows = %d, want the one that was already there", policies) + } +} + +// A migration that failed partway is re-run whole. The ledger must come out +// of a second run the same as the first, not with a duplicate or an error +// from its own unique index. +func TestSeedMenusLedgerSurvivesARetry(t *testing.T) { + db := newSeedTestDB(t) + seedAdminRole(t, db) + + apis := []seed.ApiSpec{ + {Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET", Handle: "apis.Order.GetPage-fm"}, + } + for i := 0; i < 2; i++ { + if err := (adminSeeder{}).SeedMenus(db, "order", nil, apis); err != nil { + t.Fatalf("SeedMenus run %d: %v", i+1, err) + } + } + + var n int64 + db.Model(&models.SysAppCasbinGrant{}).Count(&n) + if n != 1 { + t.Errorf("ledger has %d rows after two runs, want 1", n) + } +} + +// The ledger's own guard against a duplicate, which the plain retry above +// never reaches: there the policy still exists, so the insert is skipped +// before the ledger is touched at all. This is the case that does reach it - +// the policy row was removed while its ledger entry stayed, so the seed +// creates the policy again and writes a ledger entry that is already there. +// A plain insert would abort the whole seed on the ledger's unique index. +func TestSeedMenusLedgerToleratesAnEntryWhosePolicyWasRemoved(t *testing.T) { + db := newSeedTestDB(t) + seedAdminRole(t, db) + + apis := []seed.ApiSpec{ + {Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET", Handle: "apis.Order.GetPage-fm"}, + } + if err := (adminSeeder{}).SeedMenus(db, "order", nil, apis); err != nil { + t.Fatalf("first run: %v", err) + } + if err := db.Exec("DELETE FROM casbin_rule WHERE v1 = ? AND v2 = ?", "/api/v1/order", "GET").Error; err != nil { + t.Fatalf("removing the policy: %v", err) + } + var ledger int64 + db.Model(&models.SysAppCasbinGrant{}).Count(&ledger) + if ledger != 1 { + t.Fatalf("the ledger entry is gone, so this test is not set up: %d rows", ledger) + } + + if err := (adminSeeder{}).SeedMenus(db, "order", nil, apis); err != nil { + t.Fatalf("second run: %v", err) + } + + db.Model(&models.SysAppCasbinGrant{}).Count(&ledger) + if ledger != 1 { + t.Errorf("ledger has %d rows, want 1", ledger) + } + var policies int64 + db.Table("casbin_rule").Where("v1 = ? AND v2 = ?", "/api/v1/order", "GET").Count(&policies) + if policies != 1 { + t.Errorf("the policy was not put back: %d rows", policies) + } +} diff --git a/cmd/migrate/exit_test.go b/cmd/migrate/exit_test.go new file mode 100644 index 00000000..22680912 --- /dev/null +++ b/cmd/migrate/exit_test.go @@ -0,0 +1,45 @@ +package migrate + +import ( + "bytes" + "errors" + "strings" + "testing" +) + +// A deployment decides whether to start the new version on this command's +// exit code. Before this batch the only failure that produced one was a +// failing migration function, and it produced it by ending the process from +// inside the migration engine; moving that out would have taken the last +// reported failure with it. +func TestExitOnErrorEndsTheCommandNonZero(t *testing.T) { + var codes []int + osExit = func(c int) { codes = append(codes, c) } + t.Cleanup(func() { osExit = origExit }) + + var out bytes.Buffer + exitOnError(&out, errors.New("the tenant database is unreachable")) + + if len(codes) != 1 || codes[0] != 1 { + t.Errorf("exit codes = %v, want [1]", codes) + } + if !strings.Contains(out.String(), "the tenant database is unreachable") { + t.Errorf("the reason was not reported: %q", out.String()) + } +} + +func TestExitOnErrorLetsSuccessThrough(t *testing.T) { + var codes []int + osExit = func(c int) { codes = append(codes, c) } + t.Cleanup(func() { osExit = origExit }) + + var out bytes.Buffer + exitOnError(&out, nil) + + if len(codes) != 0 { + t.Errorf("a successful migration exited with %v", codes) + } + if out.Len() != 0 { + t.Errorf("a successful migration wrote %q", out.String()) + } +} diff --git a/cmd/migrate/install.go b/cmd/migrate/install.go new file mode 100644 index 00000000..5eeadd00 --- /dev/null +++ b/cmd/migrate/install.go @@ -0,0 +1,308 @@ +package migrate + +import ( + "errors" + "fmt" + "io" + "sort" + "strings" + "time" + + "github.com/go-admin-team/go-admin-core/v2/sdk/contract/app" + "gorm.io/gorm" + + adminmodels "go-admin/app/admin/models" + "go-admin/cmd/migrate/migration" +) + +// engine is the part of the migration engine the installer drives. +// +// An interface rather than *migration.Migration because the concrete type is +// a package-level singleton with no exported constructor, so a test that took +// it would be sharing one registry with every other test in the process. +type engine interface { + SetDb(*gorm.DB) + Status() ([]migration.StatusEntry, error) + MigrateApp(string) error +} + +// installReport is what an install did, for the command to print. +type installReport struct { + Code string + // Version is the manifest version this run recorded. + Version string + // Previous is the version sys_app held before this run, empty when this + // is the first install. + Previous string + // Applied lists the versions this run brought in, in the order they were + // applied. Empty on a no-op, and also empty on a run that only corrected + // sys_app - the difference is NoOp. + Applied []string + // NoOp says nothing was left to do: the app is recorded as installed, at + // this same version, with no migration outstanding. + NoOp bool +} + +// install brings one application up to the version its manifest declares. +// +// Three phases, each committing on its own. They are not one transaction and +// cannot be: an application's versions are separate migration files, and a +// DDL statement inside any of them commits the transaction around it on +// MySQL, which destroys an outer transaction and every savepoint taken from +// it. So this does not +// promise that a half-installed application cannot happen. It promises that +// one is visible when it does: phase A writes "installing" before anything +// that can fail, and phase C turns that into "installed" or "failed". +// +// What is left to apply comes from sys_migration, never from sys_app. +// sys_app is a derived view - a summary for a human, and the answer to "which +// version does this app think it is at". If it were the authority, then an +// operator who deleted sys_migration rows by hand would be told an app is +// installed while its schema is not, which is worse than not knowing. +func install(db *gorm.DB, eng engine, m app.Manifest) (installReport, error) { + code := migration.NormalizeAppCode(m.Code) + rep := installReport{Code: code, Version: m.Version} + if code == "" { + return rep, errors.New("the manifest declares no app code") + } + if code == migration.FrameworkAppCode { + // Installing the framework is what `migrate` is, and the framework + // has no manifest and no sys_app row. Saying so beats writing a row + // that nothing else in this batch expects to exist. + return rep, fmt.Errorf("%q is the framework's own migrations, not an application; run `migrate` for those", code) + } + if !db.Migrator().HasTable(&adminmodels.SysApp{}) { + return rep, errors.New("sys_app does not exist; run `migrate` first to bring the framework's own tables up to date") + } + + eng.SetDb(db) + + row, found, err := loadApp(db, code) + if err != nil { + return rep, err + } + // sameVersion is only meaningful when found; it stays false otherwise. + // The comparison happens here, before phase A, so an unparseable + // recorded version is refused while it is still readable rather than + // after being overwritten. + sameVersion := false + if found { + rep.Previous = row.Version + cmp, err := app.Compare(m.Version, row.Version) + if err != nil { + return rep, fmt.Errorf("comparing %s against the recorded %s: %w", m.Version, row.Version, err) + } + if cmp < 0 { + return rep, fmt.Errorf("%s is recorded at %s; installing %s would be a downgrade, which is not supported", + code, row.Version, m.Version) + } + sameVersion = cmp == 0 + } + + pending, err := pendingFor(eng, code) + if err != nil { + return rep, err + } + + // Nothing outstanding, recorded as installed, at this same version. All + // three, and the first one comes from sys_migration: a row that says + // installed while a migration of its has never run is exactly the case + // sys_app must not be believed about. AppInstalling is not installed - + // it is what a row reads as after the process was killed partway. + if found && sameVersion && row.Status == adminmodels.AppInstalled && len(pending) == 0 { + rep.NoOp = true + return rep, nil + } + + // Phase A: the attempt is on disk before anything that can fail. + now := time.Now() + if err := beginInstall(db, &row, m, code, found, now); err != nil { + return rep, err + } + + // Phase B: no atomicity across these, by the nature of the thing. + runErr := eng.MigrateApp(code) + + // Phase C. + if runErr != nil { + failed := "" + var vf *migration.VersionFailure + if errors.As(runErr, &vf) { + failed = vf.Version + } + if err := markFailed(db, code, failed, runErr, time.Now()); err != nil { + return rep, errors.Join(runErr, fmt.Errorf("recording the failure on sys_app: %w", err)) + } + return rep, runErr + } + if err := markInstalled(db, code, row.InstalledAt, time.Now()); err != nil { + return rep, err + } + rep.Applied = pending + return rep, nil +} + +// loadApp reads the sys_app row for code. A missing row is not an error: it +// is what a first install looks like. +func loadApp(db *gorm.DB, code string) (adminmodels.SysApp, bool, error) { + var row adminmodels.SysApp + err := db.Where("app_code = ?", code).First(&row).Error + if err == nil { + return row, true, nil + } + if errors.Is(err, gorm.ErrRecordNotFound) { + return adminmodels.SysApp{}, false, nil + } + return adminmodels.SysApp{}, false, fmt.Errorf("reading sys_app for %q: %w", code, err) +} + +// pendingFor is the authoritative answer to "what is left to apply", and it +// is recomputed every time rather than stored: what is registered in this +// process, minus what sys_migration says has run. sys_app.failed_version is a +// snapshot of what this returned once and may be stale by now; nothing may +// read it to decide this. +func pendingFor(eng engine, code string) ([]string, error) { + entries, err := eng.Status() + if err != nil { + return nil, err + } + var out []string + for _, e := range entries { + if e.AppCode == code && e.Registered && !e.Applied { + out = append(out, e.Version) + } + } + sort.Strings(out) + return out, nil +} + +// beginInstall is phase A. It refreshes every descriptive column from the +// manifest, because those are the manifest's to say and the row is only a +// copy, and it clears the two diagnostic columns so a stale failure from a +// previous attempt cannot be read as this one's. +func beginInstall(db *gorm.DB, row *adminmodels.SysApp, m app.Manifest, code string, found bool, now time.Time) error { + row.AppCode = code + row.Name = m.Name + row.Version = m.Version + row.Description = m.Description + row.Author = m.Author + row.Requires = strings.Join(m.Requires, ",") + row.Pricing = m.Pricing + row.License = m.License + row.Status = adminmodels.AppInstalling + row.FailedVersion = "" + row.LastError = "" + row.UpdatedAt = now + if !found { + if err := db.Create(row).Error; err != nil { + return fmt.Errorf("recording the install attempt for %q: %w", code, err) + } + return nil + } + if err := db.Save(row).Error; err != nil { + return fmt.Errorf("recording the install attempt for %q: %w", code, err) + } + return nil +} + +// markInstalled is the success half of phase C. installed_at is set once and +// never moved: an upgrade keeps the time of the first install, which is what +// the column is for. +// +// Computed here rather than with COALESCE so the statement is the same on all +// four drivers this repository supports. +func markInstalled(db *gorm.DB, code string, installedAt *time.Time, now time.Time) error { + updates := map[string]any{ + "status": adminmodels.AppInstalled, + "updated_at": now, + } + if installedAt == nil { + updates["installed_at"] = now + } + err := db.Model(&adminmodels.SysApp{}).Where("app_code = ?", code).Updates(updates).Error + if err != nil { + return fmt.Errorf("recording %q as installed: %w", code, err) + } + return nil +} + +// markFailed is the other half. Both columns it writes are diagnostic text +// for whoever reads the row; no code may branch on either one. +func markFailed(db *gorm.DB, code, failedVersion string, cause error, now time.Time) error { + updates := map[string]any{ + "status": adminmodels.AppFailed, + "failed_version": truncate(failedVersion, 64), + "last_error": truncate(cause.Error(), 255), + "updated_at": now, + } + return db.Model(&adminmodels.SysApp{}).Where("app_code = ?", code).Updates(updates).Error +} + +// truncate cuts s to at most n runes, not bytes: these columns are declared in +// characters, and a message that is partly Chinese would otherwise be cut in +// the middle of one and stored as an invalid sequence. +func truncate(s string, n int) string { + r := []rune(s) + if len(r) <= n { + return s + } + return string(r[:n]) +} + +// reportInstall prints what happened, and says that the data is in place but +// the code is not. +// +// That last sentence is not a pleasantry. Go links its applications at build +// time and Vite resolves its import globs at build time, so installing an +// application writes its menus, its APIs and its permissions and cannot make +// one line of its code run. An operator who is not told that sees the menus +// appear and reasonably concludes the thing is live. +func reportInstall(w io.Writer, rep installReport) { + if rep.NoOp { + fmt.Fprintf(w, "%s %s is already installed; nothing to do\n", rep.Code, rep.Version) + return + } + switch { + case rep.Previous == "": + fmt.Fprintf(w, "installed %s %s\n", rep.Code, rep.Version) + case rep.Previous == rep.Version: + fmt.Fprintf(w, "brought %s %s the rest of the way\n", rep.Code, rep.Version) + default: + fmt.Fprintf(w, "upgraded %s from %s to %s\n", rep.Code, rep.Previous, rep.Version) + } + if len(rep.Applied) > 0 { + fmt.Fprintf(w, "applied %d migration(s): %s\n", len(rep.Applied), strings.Join(rep.Applied, ", ")) + } else { + fmt.Fprintln(w, "no migration was outstanding; only sys_app was brought up to date") + } + fmt.Fprintln(w, "the database is up to date, but the application's code is not running yet:") + fmt.Fprintln(w, "rebuild and restart the server before expecting its routes to answer.") +} + +// manifestFor finds the manifest an application registered for code. +// +// A code nothing registered is an error naming what is registered, for the +// same reason exitUnlessAppRegistered exists: the alternative is telling an +// operator who typed `install ordr` that there was nothing to do. +func manifestFor(code string) (app.Manifest, error) { + want := migration.NormalizeAppCode(code) + all := app.Snapshot() + if m, ok := all[want]; ok { + return m, nil + } + codes := make([]string, 0, len(all)) + for c := range all { + codes = append(codes, c) + } + sort.Strings(codes) + if len(codes) == 0 { + // Worth its own sentence: no application is compiled into this + // binary at all, which is a different thing from having typed the + // wrong one of several. + return app.Manifest{}, fmt.Errorf( + "no application registers a manifest in this binary, so %q cannot be installed; "+ + "an application has to be compiled in before it can be installed", want) + } + return app.Manifest{}, fmt.Errorf("no application registers the code %q; registered: %s", + want, strings.Join(codes, ", ")) +} diff --git a/cmd/migrate/install_test.go b/cmd/migrate/install_test.go new file mode 100644 index 00000000..af7c64d8 --- /dev/null +++ b/cmd/migrate/install_test.go @@ -0,0 +1,468 @@ +package migrate + +import ( + "errors" + "strings" + "testing" + + "github.com/glebarez/sqlite" + "github.com/go-admin-team/go-admin-core/v2/sdk/contract/app" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + adminmodels "go-admin/app/admin/models" + "go-admin/cmd/migrate/migration" +) + +func newInstallDB(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(&adminmodels.SysApp{}); err != nil { + t.Fatalf("automigrate sys_app: %v", err) + } + return db +} + +// fakeEngine stands in for the migration engine. The real one is a +// package-level singleton with no exported constructor, so a test taking it +// would share one registry with every other test in this process. +type fakeEngine struct { + entries []migration.StatusEntry + // failWith, when set, is what MigrateApp returns instead of applying. + failWith error + calls []string +} + +func (f *fakeEngine) SetDb(*gorm.DB) {} + +func (f *fakeEngine) Status() ([]migration.StatusEntry, error) { + out := make([]migration.StatusEntry, len(f.entries)) + copy(out, f.entries) + return out, nil +} + +func (f *fakeEngine) MigrateApp(code string) error { + f.calls = append(f.calls, code) + if f.failWith != nil { + return f.failWith + } + for i := range f.entries { + if f.entries[i].AppCode == code && f.entries[i].Registered { + f.entries[i].Applied = true + } + } + return nil +} + +func orderManifest(version string) app.Manifest { + return app.Manifest{ + Code: "order", + Name: "Orders", + Version: version, + Description: "order management", + Author: "go-admin", + Requires: []string{"crm"}, + Pricing: "free", + License: "MIT", + } +} + +func loadRow(t *testing.T, db *gorm.DB, code string) adminmodels.SysApp { + t.Helper() + var row adminmodels.SysApp + if err := db.Where("app_code = ?", code).First(&row).Error; err != nil { + t.Fatalf("sys_app has no row for %q: %v", code, err) + } + return row +} + +// A1: a first install records the app, at the version the manifest declares, +// with every descriptive column copied from it. +func TestInstallRecordsAFirstInstall(t *testing.T) { + db := newInstallDB(t) + eng := &fakeEngine{entries: []migration.StatusEntry{ + {Version: "order-1786800001000", AppCode: "order", Registered: true}, + {Version: "order-1786800002000", AppCode: "order", Registered: true}, + {Version: "crm-1786800001000", AppCode: "crm", Registered: true}, + }} + + rep, err := install(db, eng, orderManifest("1.0.0")) + if err != nil { + t.Fatalf("install: %v", err) + } + if rep.NoOp { + t.Error("a first install reported nothing to do") + } + if got, want := len(rep.Applied), 2; got != want { + t.Errorf("applied %v, want %d versions", rep.Applied, want) + } + // Only this app's migrations, not every pending one in the process. + if len(eng.calls) != 1 || eng.calls[0] != "order" { + t.Errorf("MigrateApp calls = %v", eng.calls) + } + + row := loadRow(t, db, "order") + if row.Status != adminmodels.AppInstalled { + t.Errorf("status = %d, want installed", row.Status) + } + if row.Version != "1.0.0" { + t.Errorf("version = %q", row.Version) + } + if row.InstalledAt == nil { + t.Error("installed_at was not set") + } + if row.Name != "Orders" || row.Author != "go-admin" || row.Description != "order management" { + t.Errorf("descriptive columns not copied from the manifest: %+v", row) + } + if row.Requires != "crm" { + t.Errorf("requires = %q, want the manifest's list as CSV", row.Requires) + } + if row.Pricing != "free" || row.License != "MIT" { + t.Errorf("the reserved fields were not carried through: %+v", row) + } +} + +// A2: installing the same version again is a no-op, and says so. +func TestInstallIsANoOpAtTheSameVersion(t *testing.T) { + db := newInstallDB(t) + eng := &fakeEngine{entries: []migration.StatusEntry{ + {Version: "order-1786800001000", AppCode: "order", Registered: true}, + }} + if _, err := install(db, eng, orderManifest("1.0.0")); err != nil { + t.Fatalf("first install: %v", err) + } + before := loadRow(t, db, "order") + + rep, err := install(db, eng, orderManifest("1.0.0")) + if err != nil { + t.Fatalf("second install: %v", err) + } + if !rep.NoOp { + t.Error("installing the same version again was not reported as a no-op") + } + if len(eng.calls) != 1 { + t.Errorf("the engine was driven again: %v", eng.calls) + } + after := loadRow(t, db, "order") + if !after.UpdatedAt.Equal(before.UpdatedAt) { + t.Error("a no-op rewrote the row") + } + var n int64 + db.Model(&adminmodels.SysApp{}).Count(&n) + if n != 1 { + t.Errorf("sys_app has %d rows, want 1", n) + } +} + +// A no-op is only a no-op when nothing is outstanding. A row that says +// installed while a migration of its has never run is the case sys_app must +// not be believed over sys_migration. +func TestInstallRunsWhenTheRowSaysInstalledButAMigrationIsPending(t *testing.T) { + db := newInstallDB(t) + eng := &fakeEngine{entries: []migration.StatusEntry{ + {Version: "order-1786800001000", AppCode: "order", Registered: true, Applied: true}, + }} + if _, err := install(db, eng, orderManifest("1.0.0")); err != nil { + t.Fatalf("first install: %v", err) + } + + // A second version of the same app appears - the app was rebuilt with + // one more migration file, without its version changing. + eng.entries = append(eng.entries, migration.StatusEntry{ + Version: "order-1786800002000", AppCode: "order", Registered: true, + }) + + rep, err := install(db, eng, orderManifest("1.0.0")) + if err != nil { + t.Fatalf("install: %v", err) + } + if rep.NoOp { + t.Fatal("an outstanding migration was reported as nothing to do") + } + if len(rep.Applied) != 1 || rep.Applied[0] != "order-1786800002000" { + t.Errorf("applied = %v", rep.Applied) + } +} + +// A9: an upgrade is in place. installed_at is the first install's, not this +// one's. +func TestInstallUpgradesInPlaceAndKeepsTheFirstInstallTime(t *testing.T) { + db := newInstallDB(t) + eng := &fakeEngine{entries: []migration.StatusEntry{ + {Version: "order-1786800001000", AppCode: "order", Registered: true}, + }} + if _, err := install(db, eng, orderManifest("1.0.0")); err != nil { + t.Fatalf("first install: %v", err) + } + first := loadRow(t, db, "order") + if first.InstalledAt == nil { + t.Fatal("installed_at was not set by the first install") + } + + eng.entries = append(eng.entries, migration.StatusEntry{ + Version: "order-1786800002000", AppCode: "order", Registered: true, + }) + rep, err := install(db, eng, orderManifest("2.0.0")) + if err != nil { + t.Fatalf("upgrade: %v", err) + } + if rep.Previous != "1.0.0" { + t.Errorf("previous = %q, want 1.0.0", rep.Previous) + } + + row := loadRow(t, db, "order") + if row.Version != "2.0.0" { + t.Errorf("version = %q, want 2.0.0", row.Version) + } + if row.Status != adminmodels.AppInstalled { + t.Errorf("status = %d, want installed", row.Status) + } + if !row.InstalledAt.Equal(*first.InstalledAt) { + t.Errorf("installed_at moved from %v to %v; an upgrade keeps the first install's time", + first.InstalledAt, row.InstalledAt) + } +} + +// A10: a downgrade is refused, and refused before anything is written. +func TestInstallRefusesADowngrade(t *testing.T) { + db := newInstallDB(t) + eng := &fakeEngine{entries: []migration.StatusEntry{ + {Version: "order-1786800001000", AppCode: "order", Registered: true}, + }} + if _, err := install(db, eng, orderManifest("2.0.0")); err != nil { + t.Fatalf("first install: %v", err) + } + before := loadRow(t, db, "order") + + _, err := install(db, eng, orderManifest("1.0.0")) + if err == nil { + t.Fatal("a downgrade was accepted") + } + if !strings.Contains(err.Error(), "downgrade") { + t.Errorf("error = %q, it has to say what it refused", err) + } + after := loadRow(t, db, "order") + if after.Version != before.Version || after.Status != before.Status { + t.Errorf("the refused downgrade still wrote to the row: %+v -> %+v", before, after) + } +} + +// A5: a failing migration leaves a row that says so, and says where. +func TestInstallRecordsAFailure(t *testing.T) { + db := newInstallDB(t) + boom := errors.New("the seed hit a duplicate") + eng := &fakeEngine{ + entries: []migration.StatusEntry{ + {Version: "order-1786800001000", AppCode: "order", Registered: true}, + }, + failWith: &migration.VersionFailure{Version: "order-1786800001000", Err: boom}, + } + + _, err := install(db, eng, orderManifest("1.0.0")) + if err == nil { + t.Fatal("a failed install reported success") + } + if !errors.Is(err, boom) { + t.Errorf("the cause is not reachable: %v", err) + } + + row := loadRow(t, db, "order") + if row.Status != adminmodels.AppFailed { + t.Errorf("status = %d, want failed", row.Status) + } + if row.FailedVersion != "order-1786800001000" { + t.Errorf("failed_version = %q", row.FailedVersion) + } + if !strings.Contains(row.LastError, "duplicate") { + t.Errorf("last_error = %q", row.LastError) + } + if row.InstalledAt != nil { + t.Error("installed_at was set by an install that failed") + } +} + +// A failed install is retried by running it again - not by any special +// command, and without the previous attempt's diagnostics surviving into a +// row that now says installed. +func TestInstallResumesAfterAFailure(t *testing.T) { + db := newInstallDB(t) + eng := &fakeEngine{ + entries: []migration.StatusEntry{ + {Version: "order-1786800001000", AppCode: "order", Registered: true}, + }, + failWith: &migration.VersionFailure{Version: "order-1786800001000", Err: errors.New("boom")}, + } + if _, err := install(db, eng, orderManifest("1.0.0")); err == nil { + t.Fatal("the first attempt did not fail") + } + + eng.failWith = nil + rep, err := install(db, eng, orderManifest("1.0.0")) + if err != nil { + t.Fatalf("retry: %v", err) + } + if rep.NoOp { + t.Error("a failed row was treated as installed") + } + + row := loadRow(t, db, "order") + if row.Status != adminmodels.AppInstalled { + t.Errorf("status = %d, want installed", row.Status) + } + if row.FailedVersion != "" || row.LastError != "" { + t.Errorf("the previous failure survived onto a row that now says installed: %q / %q", + row.FailedVersion, row.LastError) + } + if row.InstalledAt == nil { + t.Error("installed_at was not set by the attempt that succeeded") + } +} + +// A row stuck at installing - the process was killed partway - is not +// installed, and must not be mistaken for it. +func TestInstallRetriesARowStuckAtInstalling(t *testing.T) { + db := newInstallDB(t) + if err := db.Create(&adminmodels.SysApp{ + AppCode: "order", Name: "Orders", Version: "1.0.0", + Status: adminmodels.AppInstalling, + }).Error; err != nil { + t.Fatalf("seed: %v", err) + } + eng := &fakeEngine{entries: []migration.StatusEntry{ + {Version: "order-1786800001000", AppCode: "order", Registered: true, Applied: true}, + }} + + rep, err := install(db, eng, orderManifest("1.0.0")) + if err != nil { + t.Fatalf("install: %v", err) + } + if rep.NoOp { + t.Fatal("a row stuck at installing was reported as already installed") + } + if row := loadRow(t, db, "order"); row.Status != adminmodels.AppInstalled { + t.Errorf("status = %d, want installed", row.Status) + } +} + +func TestInstallRejectsTheFrameworkCode(t *testing.T) { + db := newInstallDB(t) + m := orderManifest("1.0.0") + m.Code = migration.FrameworkAppCode + _, err := install(db, &fakeEngine{}, m) + if err == nil { + t.Fatal("the framework was installed as an application") + } + if !strings.Contains(err.Error(), "migrate") { + t.Errorf("error = %q, it should point at the command that does this", err) + } +} + +func TestInstallRefusesAnUnparseableRecordedVersion(t *testing.T) { + db := newInstallDB(t) + if err := db.Create(&adminmodels.SysApp{ + AppCode: "order", Name: "Orders", Version: "v1.0", Status: adminmodels.AppInstalled, + }).Error; err != nil { + t.Fatalf("seed: %v", err) + } + _, err := install(db, &fakeEngine{}, orderManifest("1.0.0")) + if err == nil { + t.Fatal("an unparseable recorded version was compared anyway") + } + row := loadRow(t, db, "order") + if row.Status != adminmodels.AppInstalled || row.Version != "v1.0" { + t.Errorf("the row was overwritten before the comparison failed: %+v", row) + } +} + +// A7: the report has to say the code is not running yet. Menus appearing is +// exactly what makes an operator think it is. +func TestReportInstallSaysTheCodeIsNotRunningYet(t *testing.T) { + var out strings.Builder + reportInstall(&out, installReport{Code: "order", Version: "1.0.0", Applied: []string{"order-1786800001000"}}) + got := out.String() + if !strings.Contains(got, "rebuild") || !strings.Contains(got, "restart") { + t.Errorf("the report does not say the binary has to be rebuilt: %q", got) + } + if !strings.Contains(got, "order-1786800001000") { + t.Errorf("the report does not name what it applied: %q", got) + } +} + +func TestReportInstallOnANoOp(t *testing.T) { + var out strings.Builder + reportInstall(&out, installReport{Code: "order", Version: "1.0.0", NoOp: true}) + if !strings.Contains(out.String(), "already installed") { + t.Errorf("output = %q", out.String()) + } +} + +// last_error is a varchar(255) declared in characters. A message that is +// partly Chinese would be cut mid-rune by a byte-wise truncation and stored +// as an invalid sequence. +func TestTruncateCutsRunesNotBytes(t *testing.T) { + s := strings.Repeat("迁", 300) + got := truncate(s, 255) + if n := len([]rune(got)); n != 255 { + t.Errorf("kept %d runes, want 255", n) + } + if !strings.HasPrefix(s, got) { + t.Error("truncation did not cut at a rune boundary") + } + if short := truncate("ok", 255); short != "ok" { + t.Errorf("a short message was altered: %q", short) + } +} + +func TestInstallNeedsSysApp(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.Fatalf("open sqlite: %v", err) + } + _, err = install(db, &fakeEngine{}, orderManifest("1.0.0")) + if err == nil { + t.Fatal("install ran against a database with no sys_app") + } + if !strings.Contains(err.Error(), "migrate") { + t.Errorf("error = %q, it should say what to run first", err) + } +} + +// The code written to sys_app and handed to the engine is the normalized one. +// A manifest whose Code was typed with different case or stray spaces has to +// land on the same identity migration.ForApp and seed.SeedMenus already use, +// or the row and the migrations it stands for are filed under two names. +func TestInstallNormalizesTheAppCode(t *testing.T) { + db := newInstallDB(t) + eng := &fakeEngine{entries: []migration.StatusEntry{ + {Version: "order-1786800001000", AppCode: "order", Registered: true}, + }} + m := orderManifest("1.0.0") + m.Code = " Order " + + rep, err := install(db, eng, m) + if err != nil { + t.Fatalf("install: %v", err) + } + if rep.Code != "order" { + t.Errorf("reported code = %q, want order", rep.Code) + } + if len(eng.calls) != 1 || eng.calls[0] != "order" { + t.Errorf("the engine was asked for %v, want [order]", eng.calls) + } + // The row has to be findable by the normalized code, which is what every + // other table in this batch is keyed by. + row := loadRow(t, db, "order") + if row.AppCode != "order" { + t.Errorf("app_code = %q", row.AppCode) + } + if len(rep.Applied) != 1 { + t.Errorf("applied = %v; the normalized code has to match what Status reports", rep.Applied) + } +} diff --git a/cmd/migrate/migration/init.go b/cmd/migrate/migration/init.go index 309d41c2..8f5070ab 100644 --- a/cmd/migrate/migration/init.go +++ b/cmd/migrate/migration/init.go @@ -284,12 +284,33 @@ func (e *Migration) Status() ([]StatusEntry, error) { } // 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) } +// across all apps. +func (e *Migration) Migrate() error { return 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)) } +func (e *Migration) MigrateApp(appCode string) error { return e.run(AppFilter(appCode)) } + +// VersionFailure names the migration that failed. +// +// The caller that needs this is an installer recording which version an +// install got stuck on. That is a diagnostic snapshot and nothing more: the +// authoritative answer to "where does a retry resume" is always recomputed +// by subtracting sys_migration's applied rows from what is registered, never +// read back from anywhere it was stored. Which is exactly why this carries +// the version rather than leaving the caller to infer it - inferring it +// would produce "what is pending now", a different question that happens to +// have the same answer most of the time. +type VersionFailure struct { + Version string + Err error +} + +func (e *VersionFailure) Error() string { + return fmt.Sprintf("migration %s failed: %v", e.Version, e.Err) +} + +func (e *VersionFailure) Unwrap() error { return e.Err } // NormalizeAppCode applies the same rule ForApp does, so a code typed on the // command line matches one written in an init(). @@ -332,7 +353,15 @@ func (e *Migration) AppCodes() []string { return out } -func (e *Migration) run(appCode string) { +// run applies the pending migrations selected by appCode. +// +// It reports failure instead of ending the process. It used to call +// log.Fatalf, which took the whole process down at the first failing +// migration - so a caller had nowhere to record what happened, and a test +// could not exercise a failing migration at all without killing the test +// binary. The exit now lives at the command layer, where the exit code is +// the command's business (see initDB in cmd/migrate/server.go). +func (e *Migration) run(appCode string) error { all := e.mergedEntries() versions := make([]string, 0, len(all)) entries := make(map[string]versionEntry, len(all)) @@ -347,10 +376,14 @@ func (e *Migration) run(appCode string) { // A mistyped --app would otherwise select nothing and report "no // migrations to apply", which reads exactly like "already up to date". + // + // The command layer rejects an unregistered code before any database + // work (exitUnlessAppRegistered), so on that path this is unreachable. + // It is reachable from an installer, which asks for one app by name and + // must not be told that installing an app nothing registered succeeded. if appCode != allApps && len(versions) == 0 { - log.Printf("no migrations are registered for app %q; registered: %s", + return fmt.Errorf("no migrations are registered for app %q; registered: %s", DisplayAppCode(appCode), strings.Join(e.AppCodes(), ", ")) - return } var err error @@ -359,7 +392,7 @@ func (e *Migration) run(appCode string) { for _, v := range versions { err = e.db.Table("sys_migration").Where("version = ?", v).Count(&count).Error if err != nil { - log.Fatalln(err) + return fmt.Errorf("checking whether migration %s was applied: %w", v, err) } if count > 0 { // Already applied. This used to print the bare count, so a mature @@ -369,7 +402,7 @@ func (e *Migration) run(appCode string) { } log.Printf("applying migration %s", v) if err = entries[v].fn(e.db.Debug(), v); err != nil { - log.Fatalf("migration %s failed: %v", v, err) + return &VersionFailure{Version: v, Err: err} } applied++ } @@ -378,6 +411,7 @@ func (e *Migration) run(appCode string) { } else { log.Printf("applied %d migration(s)", applied) } + return nil } // allApps is the sentinel run() takes to mean "do not filter". It is distinct diff --git a/cmd/migrate/migration/init_test.go b/cmd/migrate/migration/init_test.go index 13d8c6a6..e1287f8d 100644 --- a/cmd/migrate/migration/init_test.go +++ b/cmd/migrate/migration/init_test.go @@ -1,9 +1,7 @@ package migration import ( - "bytes" - "log" - "os" + "errors" "strings" "testing" "time" @@ -81,7 +79,9 @@ func TestForAppRecordsItsAppCode(t *testing.T) { m.ForApp("x").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { return recordFor(db, version, appCode) }) - m.Migrate() + if err := m.Migrate(); err != nil { + t.Fatalf("m.Migrate(): %v", err) + } rows := rowsByVersion(t, db) row, ok := rows["x-1786800001000"] @@ -104,7 +104,9 @@ func TestSetVersionStillRecordsTheFrameworkAsEmpty(t *testing.T) { m.SetVersion("1786700009000", func(db *gorm.DB, version string) error { return db.Create(&common.Migration{Version: version}).Error }) - m.Migrate() + if err := m.Migrate(); err != nil { + t.Fatalf("m.Migrate(): %v", err) + } rows := rowsByVersion(t, db) row, ok := rows["1786700009000"] @@ -136,7 +138,9 @@ func TestMigrateAppRunsOnlyThatApp(t *testing.T) { return recordFor(db, version, appCode) }) - m.MigrateApp("x") + if err := m.MigrateApp("x"); err != nil { + t.Fatalf("m.MigrateApp(\"x\"): %v", err) + } if !ran["x"] { t.Error("x did not run") @@ -167,7 +171,9 @@ func TestMigrateAppCoreSelectsTheFramework(t *testing.T) { return recordFor(db, version, appCode) }) - m.MigrateApp(FrameworkAppCode) + if err := m.MigrateApp(FrameworkAppCode); err != nil { + t.Fatalf("m.MigrateApp(FrameworkAppCode): %v", err) + } if !ran["core"] { t.Error("framework migration did not run") @@ -198,7 +204,9 @@ func TestMigrateRunsEveryApp(t *testing.T) { return recordFor(db, version, appCode) }) - m.Migrate() + if err := m.Migrate(); err != nil { + t.Fatalf("m.Migrate(): %v", err) + } // Namespacing puts every framework migration - bare digits - ahead of every // app migration, and orders apps by code rather than by whose timestamp @@ -232,7 +240,9 @@ func TestNamespacingKeepsTwoAppsWithTheSameTimestampApart(t *testing.T) { return recordFor(db, version, appCode) }) } - m.Migrate() + if err := m.Migrate(); err != nil { + t.Fatalf("m.Migrate(): %v", err) + } if ran != 2 { t.Errorf("ran %d migrations, want 2", ran) @@ -357,11 +367,11 @@ func TestFailedMigrationLeavesNoRecord(t *testing.T) { }) }) - // 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 { + // Driven through the scheduler, not by calling the registered function + // directly. That workaround was here because run() called log.Fatal and + // would have taken the test binary with it, which also meant nothing + // covered what the scheduler does with a failure. + if err := m.MigrateApp("crm"); err == nil { t.Fatal("migration reported success") } if rows := rowsByVersion(t, db); len(rows) != 0 { @@ -369,6 +379,49 @@ func TestFailedMigrationLeavesNoRecord(t *testing.T) { } } +// An installer records which version an attempt got stuck on. It gets that +// from the error rather than by asking the database what is still pending, +// which is a different question - see VersionFailure. +func TestRunReportsWhichVersionFailed(t *testing.T) { + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + // Two versions, and the first one succeeds: the failure has to name the + // one that actually failed, which a report that just names the app, or + // the first version it looked at, would get wrong. + m.ForApp("crm").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { + return recordFor(db, version, appCode) + }) + m.ForApp("crm").SetVersion("1786800002000", func(db *gorm.DB, version, appCode string) error { + return errTestMigrationFailed + }) + + err := m.MigrateApp("crm") + if err == nil { + t.Fatal("MigrateApp reported success") + } + var vf *VersionFailure + if !errors.As(err, &vf) { + t.Fatalf("error is %T, want *VersionFailure: %v", err, err) + } + if vf.Version != "crm-1786800002000" { + t.Errorf("failed version = %q, want crm-1786800002000", vf.Version) + } + if !errors.Is(err, errTestMigrationFailed) { + t.Errorf("the cause is not reachable through the wrapper: %v", err) + } + // The one that succeeded before it stays recorded: a retry must not run + // it again. + rows := rowsByVersion(t, db) + if _, ok := rows["crm-1786800001000"]; !ok { + t.Errorf("the migration that succeeded was not recorded: %v", rows) + } + if _, ok := rows["crm-1786800002000"]; ok { + t.Errorf("the migration that failed was recorded: %v", rows) + } +} + var errTestMigrationFailed = &testError{"boom"} type testError struct{ s string } @@ -389,17 +442,18 @@ func TestMigrateAppOnAnUnknownCodeSaysSo(t *testing.T) { 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()) + // Reported as an error rather than a log line, so an installer asking + // for one app by name cannot be told that installing an app nothing + // registered succeeded. + err := m.MigrateApp("crmm") + if err == nil { + t.Fatal("a typo reported success") } - if !strings.Contains(buf.String(), "registered: core, crm") { - t.Errorf("the message must list what is registered; got %q", buf.String()) + if !strings.Contains(err.Error(), `no migrations are registered for app "crmm"`) { + t.Errorf("error = %q", err) + } + if !strings.Contains(err.Error(), "registered: core, crm") { + t.Errorf("the message must list what is registered; got %q", err) } if rows := rowsByVersion(t, db); len(rows) != 0 { t.Errorf("a typo ran %v", rows) @@ -425,7 +479,9 @@ func TestMergedEntriesRunsAContractRegisteredAppMigration(t *testing.T) { return recordFor(db, version, appCode) }) - m.Migrate() + if err := m.Migrate(); err != nil { + t.Fatalf("m.Migrate(): %v", err) + } if !ran { t.Fatal("contract-registered migration did not run") @@ -466,7 +522,9 @@ func TestMergedEntriesStatusIncludesContractRegisteredMigrations(t *testing.T) { t.Fatalf("pending contract entry = %+v (ok=%v)", e, ok) } - m.Migrate() + if err := m.Migrate(); err != nil { + t.Fatalf("m.Migrate(): %v", err) + } entries, err = m.Status() if err != nil { @@ -522,7 +580,9 @@ func TestMergedEntriesMigrateAppRunsOnlyThatContractApp(t *testing.T) { return recordFor(db, version, appCode) }) - m.MigrateApp("order") + if err := m.MigrateApp("order"); err != nil { + t.Fatalf("m.MigrateApp(\"order\"): %v", err) + } if !ran["order"] { t.Error("order did not run") @@ -552,7 +612,9 @@ func TestMergedEntriesHostRegistrationWinsOnKeyCollision(t *testing.T) { return recordFor(db, version, appCode) }) - m.Migrate() + if err := m.Migrate(); err != nil { + t.Fatalf("m.Migrate(): %v", err) + } if !hostRan { t.Error("host registration did not run") diff --git a/cmd/migrate/server.go b/cmd/migrate/server.go index 951fedee..93222f97 100644 --- a/cmd/migrate/server.go +++ b/cmd/migrate/server.go @@ -3,6 +3,7 @@ package migrate import ( "bytes" "fmt" + "io" "os" "strconv" "strings" @@ -47,6 +48,31 @@ var ( runStatus() }, } + // Under migrate rather than under the existing `app` command, which + // already means "generate the skeleton of a new app" - a directory that + // does not exist yet, not an application already compiled into this + // binary. Installing an application is running its migrations, which is + // what this command is; --app, --domain and resolveDB are all already + // here, including the guard that refuses a mistyped code instead of + // reporting a successful no-op. + installCmd = &cobra.Command{ + Use: "install ", + Short: "Install one application: run its migrations and record it in sys_app", + Example: "go-admin migrate install order -c config/settings.yml", + Args: cobra.ExactArgs(1), + Run: func(cmd *cobra.Command, args []string) { + runInstall(args[0]) + }, + } + uninstallCmd = &cobra.Command{ + Use: "uninstall ", + Short: "Remove one application's menus, apis and permission grants; its own tables are left alone", + Example: "go-admin migrate uninstall order -c config/settings.yml", + Args: cobra.ExactArgs(1), + Run: func(cmd *cobra.Command, args []string) { + runUninstall(args[0]) + }, + } ) // fixme 在您看不见代码的时候运行迁移，我觉得是不安全的，所以编译后最好不要去执行迁移 @@ -64,6 +90,8 @@ func init() { StartCmd.Flags().BoolVar(&dryRun, "dry-run", false, "list what would be applied, in order, and write nothing") StartCmd.AddCommand(statusCmd) + StartCmd.AddCommand(installCmd) + StartCmd.AddCommand(uninstallCmd) } func run() { @@ -162,11 +190,9 @@ func migrateModel() error { } migration.Migrate.SetDb(db.Debug()) if appCode != "" { - migration.Migrate.MigrateApp(appCode) - return nil + return migration.Migrate.MigrateApp(appCode) } - migration.Migrate.Migrate() - return nil + return migration.Migrate.Migrate() } func initDB() { @@ -197,13 +223,40 @@ func initDB() { //4. 数据库迁移 fmt.Println("数据库迁移开始") - if err := migrateModel(); err != nil { - fmt.Println(err) - return - } + exitOnError(os.Stderr, migrateModel()) fmt.Println(`数据库基础数据初始化成功`) } +// exitOnError ends the command non-zero when the migration did not go through. +// +// A caller that migrates before starting a server decides whether to go ahead +// on the exit code alone - the deploy workflow does exactly that. Every path +// out of migrateModel used to return without one: an unreachable tenant +// database or a failed AutoMigrate printed a line and exited 0, so a +// deployment carried on onto a schema that had not been brought forward. A +// failing migration function was the only one reported, and only because it +// ended the process from inside the migration engine - which is the call this +// batch moved out here, so without this the last reported failure would have +// stopped being reported too. +// +// Split from the exit itself, the way appRegistrationError is split from +// exitUnlessAppRegistered, so what it decides can be tested without a +// subprocess. osExit is a variable for the same reason. +func exitOnError(w io.Writer, err error) { + if err == nil { + return + } + fmt.Fprintln(w, err) + osExit(1) +} + +// osExit is a variable so a test can watch the decision without ending the +// test binary; origExit is what it is put back to. +var ( + osExit = os.Exit + origExit = os.Exit +) + func runStatus() { config.Setup( file.NewSource(file.WithPath(configYml)), @@ -229,6 +282,54 @@ func runStatus() { ) } +func runInstall(code string) { + config.Setup( + file.NewSource(file.WithPath(configYml)), + func() { + database.Setup() + db, err := resolveDB() + if err != nil { + exitOnError(os.Stderr, err) + return + } + m, err := manifestFor(code) + if err != nil { + exitOnError(os.Stderr, err) + return + } + rep, err := install(db, migration.Migrate, m) + if err != nil { + exitOnError(os.Stderr, err) + return + } + reportInstall(os.Stdout, rep) + }, + ) +} + +func runUninstall(code string) { + config.Setup( + file.NewSource(file.WithPath(configYml)), + func() { + database.Setup() + db, err := resolveDB() + if err != nil { + exitOnError(os.Stderr, err) + return + } + // No manifest lookup. An application whose code has already been + // taken out of the binary registers nothing, and that is exactly + // when somebody needs to clear its rows out of the database. + rep, err := uninstall(db, code) + if err != nil { + exitOnError(os.Stderr, err) + return + } + reportUninstall(os.Stdout, rep) + }, + ) +} + func genFile() error { t1, err := template.ParseFiles("template/migrate.template") if err != nil { diff --git a/cmd/migrate/uninstall.go b/cmd/migrate/uninstall.go new file mode 100644 index 00000000..6d0a5710 --- /dev/null +++ b/cmd/migrate/uninstall.go @@ -0,0 +1,296 @@ +package migrate + +import ( + "errors" + "fmt" + "io" + "strings" + + "gorm.io/gorm" + + adminmodels "go-admin/app/admin/models" + "go-admin/cmd/migrate/migration" + commonmodels "go-admin/common/models" +) + +// policyKey is one casbin_rule row identified the way casbin_rule is unique: +// by its tuple, not by its id. Ids do not survive SysRole.Update, which +// removes a role's policies and adds them back. +type policyKey struct { + Ptype string `gorm:"column:ptype"` + V0 string `gorm:"column:v0"` + V1 string `gorm:"column:v1"` + V2 string `gorm:"column:v2"` + V3 string `gorm:"column:v3"` + V4 string `gorm:"column:v4"` + V5 string `gorm:"column:v5"` +} + +func (p policyKey) String() string { + return fmt.Sprintf("%s %s %s %s", p.Ptype, p.V0, p.V1, p.V2) +} + +// uninstallReport is what an uninstall removed, and what it deliberately did +// not. +type uninstallReport struct { + Code string + // Found says whether sys_app had a row. An application whose migrations + // were applied by plain `migrate` rather than by `install` has its menus + // and its permissions without ever having had one. + Found bool + Version string + Menus int64 + Apis int64 + Bindings int64 + RoleMenus int64 + Policies int64 + Migrations int64 + // Skipped are ledger entries whose casbin_rule row was not there any + // more: something this install created and something else removed. + Skipped []policyKey + // Orphans are policies naming this application's paths that no ledger + // entry claims - somebody granted this app's API to another role by + // hand. Reported, never deleted. + Orphans []policyKey +} + +// uninstall removes one application's menus, APIs and permission grants. +// +// It does not touch the application's own tables. Removing an order module +// is not the same decision as destroying the orders, and nothing here can +// tell the operator apart from someone who will reinstall tomorrow. +// +// One transaction, and this one really is one: every statement below is DML +// or a SELECT, so unlike an install there is no DDL to commit it out from +// under itself. Child rows go first, while the ids that identify them can +// still be read from the parents. +// +// A sys_app row is not required. `migrate` with no subcommand applies every +// registered migration, an application's included, so an application can +// have all of its data without ever having gone through the installer. +func uninstall(db *gorm.DB, code string) (uninstallReport, error) { + code = migration.NormalizeAppCode(code) + rep := uninstallReport{Code: code} + if code == "" { + return rep, errors.New("no app code given") + } + if code == migration.FrameworkAppCode { + return rep, fmt.Errorf("%q is the framework's own migrations; there is no uninstall for those", code) + } + + err := db.Transaction(func(tx *gorm.DB) error { + row, found, err := loadApp(tx, code) + if err != nil { + return err + } + rep.Found = found + if found { + rep.Version = row.Version + } + + // 1 and 2. Read before deleting: sys_api's rows are about to go, and + // step 5b needs their paths. + // + // Unscoped throughout. A row this application wrote that somebody + // soft-deleted from the UI is still this application's row, and + // leaving it behind would leave its join rows pointing at it. + var menuIDs []int + if err := tx.Unscoped().Model(&adminmodels.SysMenu{}). + Where("app_code = ?", code).Pluck("menu_id", &menuIDs).Error; err != nil { + return fmt.Errorf("reading this app's menus: %w", err) + } + var apiIDs []int + if err := tx.Unscoped().Model(&adminmodels.SysApi{}). + Where("app_code = ?", code).Pluck("id", &apiIDs).Error; err != nil { + return fmt.Errorf("reading this app's apis: %w", err) + } + var apiKeys []policyKey + if err := tx.Unscoped().Model(&adminmodels.SysApi{}). + Where("app_code = ?", code). + Select("path as v1, action as v2").Scan(&apiKeys).Error; err != nil { + return fmt.Errorf("reading this app's api paths: %w", err) + } + + // 3. The many2many rows behind SysMenu.SysApi. Either side is enough + // to make a row this application's. + // + // The guard is intent, not necessity: GORM renders IN with an empty + // slice as a condition matching nothing rather than the empty IN + // list raw SQL would reject, so removing it changes no behaviour + // today. It says out loud that an application with no menus, or no + // apis, is a normal thing to uninstall. + if len(menuIDs) > 0 || len(apiIDs) > 0 { + q := tx.Table("sys_menu_api_rule") + switch { + case len(menuIDs) > 0 && len(apiIDs) > 0: + q = q.Where("sys_menu_menu_id IN ? OR sys_api_id IN ?", menuIDs, apiIDs) + case len(menuIDs) > 0: + q = q.Where("sys_menu_menu_id IN ?", menuIDs) + default: + q = q.Where("sys_api_id IN ?", apiIDs) + } + res := q.Delete(nil) + if res.Error != nil { + return fmt.Errorf("removing menu/api bindings: %w", res.Error) + } + rep.Bindings = res.RowsAffected + } + + // 4. Role assignments. menu_id is a surrogate key, so a row here can + // only have come from a menu this application wrote - there is no + // "looks like it but is not". That is why this needs no ledger, and + // why a column on sys_role_menu would have been wrong: SysRole.Update + // deletes a role's rows and writes them back through GORM's + // many2many, which does not carry extra columns, so any such column + // would be silently blanked the first time somebody edits a role. + if len(menuIDs) > 0 { + res := tx.Table("sys_role_menu").Where("menu_id IN ?", menuIDs).Delete(nil) + if res.Error != nil { + return fmt.Errorf("removing role assignments: %w", res.Error) + } + rep.RoleMenus = res.RowsAffected + } + + // 5. Policies, by ledger, one at a time and by exact tuple. + var grants []adminmodels.SysAppCasbinGrant + if err := tx.Where("app_code = ?", code).Find(&grants).Error; err != nil { + return fmt.Errorf("reading the grant ledger: %w", err) + } + for _, g := range grants { + k := policyKey{Ptype: g.Ptype, V0: g.V0, V1: g.V1, V2: g.V2, V3: g.V3, V4: g.V4, V5: g.V5} + res := tx.Table("casbin_rule"). + Where("ptype = ? AND v0 = ? AND v1 = ? AND v2 = ? AND v3 = ? AND v4 = ? AND v5 = ?", + k.Ptype, k.V0, k.V1, k.V2, k.V3, k.V4, k.V5). + Delete(nil) + if res.Error != nil { + return fmt.Errorf("removing policy %s: %w", k, res.Error) + } + if res.RowsAffected == 0 { + // Something this install created is not there any more. Not + // an error: the uninstall's job was to remove it and it is + // gone. Reported because a policy this app created and did + // not remove means something else rewrote casbin_rule. + rep.Skipped = append(rep.Skipped, k) + continue + } + rep.Policies += res.RowsAffected + } + // The ledger's job ends here whether or not each row matched. Left + // behind it would only grow, and a reinstall writes its own entries. + if err := tx.Where("app_code = ?", code). + Delete(&adminmodels.SysAppCasbinGrant{}).Error; err != nil { + return fmt.Errorf("clearing the grant ledger: %w", err) + } + + // 5b. Read-only. By now every policy the ledger could speak for has + // been dealt with, so a policy still matching one of this app's paths + // is one the ledger never claimed - somebody granted this app's API + // to another role by hand. Business rule 3 says do not delete what + // is not ours; without this step nobody would ever learn it is + // there, pointing at an API that is about to stop existing. + orphans, err := findOrphanPolicies(tx, apiKeys) + if err != nil { + return err + } + rep.Orphans = orphans + + // 6 and 7. + res := tx.Unscoped().Where("app_code = ?", code).Delete(&adminmodels.SysApi{}) + if res.Error != nil { + return fmt.Errorf("removing this app's apis: %w", res.Error) + } + rep.Apis = res.RowsAffected + + res = tx.Unscoped().Where("app_code = ?", code).Delete(&adminmodels.SysMenu{}) + if res.Error != nil { + return fmt.Errorf("removing this app's menus: %w", res.Error) + } + rep.Menus = res.RowsAffected + + // 8. Without this a reinstall finds every version already applied, + // runs no migration, seeds nothing, and reports success. It is the + // easiest step to leave out, because a migration record does not + // look like the application's data. + res = tx.Where("app_code = ?", code).Delete(&commonmodels.Migration{}) + if res.Error != nil { + return fmt.Errorf("removing this app's migration records: %w", res.Error) + } + rep.Migrations = res.RowsAffected + + // 9. + if found { + if err := tx.Where("app_code = ?", code). + Delete(&adminmodels.SysApp{}).Error; err != nil { + return fmt.Errorf("removing the sys_app row: %w", err) + } + } + return nil + }) + if err != nil { + return uninstallReport{Code: code}, err + } + return rep, nil +} + +// findOrphanPolicies looks for policies naming any of this application's +// paths. +// +// Written as an OR chain rather than a row-value IN, which MySQL and modern +// SQLite accept and SQL Server does not; this repository supports all of +// them. Chunked because a driver's placeholder limit is reached long before +// an application runs out of endpoints. +func findOrphanPolicies(tx *gorm.DB, keys []policyKey) ([]policyKey, error) { + const chunk = 100 + var out []policyKey + for start := 0; start < len(keys); start += chunk { + end := start + chunk + if end > len(keys) { + end = len(keys) + } + clauses := make([]string, 0, end-start) + args := make([]any, 0, (end-start)*2) + for _, k := range keys[start:end] { + clauses = append(clauses, "(v1 = ? AND v2 = ?)") + args = append(args, k.V1, k.V2) + } + var found []policyKey + if err := tx.Table("casbin_rule"). + Where("ptype = ? AND ("+strings.Join(clauses, " OR ")+")", append([]any{"p"}, args...)...). + Scan(&found).Error; err != nil { + return nil, fmt.Errorf("looking for policies nothing claims: %w", err) + } + out = append(out, found...) + } + return out, nil +} + +// reportUninstall prints what went and what stayed. +// +// The two lists are separate because they mean different things: one is +// something this application created that had already gone, the other is +// somebody else's grant that is now pointing at nothing. Merged into one +// "could not remove" list, neither would be actionable. +func reportUninstall(w io.Writer, rep uninstallReport) { + if !rep.Found { + fmt.Fprintf(w, "%s had no sys_app row; removed what its migrations had written\n", rep.Code) + } else { + fmt.Fprintf(w, "uninstalled %s %s\n", rep.Code, rep.Version) + } + fmt.Fprintf(w, "removed: %d menu(s), %d api(s), %d binding(s), %d role assignment(s), %d policy(ies), %d migration record(s)\n", + rep.Menus, rep.Apis, rep.Bindings, rep.RoleMenus, rep.Policies, rep.Migrations) + fmt.Fprintln(w, "the application's own tables were not touched.") + + if len(rep.Skipped) > 0 { + fmt.Fprintf(w, "\n%d policy(ies) this install had created were already gone:\n", len(rep.Skipped)) + for _, k := range rep.Skipped { + fmt.Fprintf(w, " %s\n", k) + } + } + if len(rep.Orphans) > 0 { + fmt.Fprintf(w, "\n%d policy(ies) name this application's paths and were granted by somebody else, so they were left alone:\n", len(rep.Orphans)) + for _, k := range rep.Orphans { + fmt.Fprintf(w, " %s\n", k) + } + fmt.Fprintln(w, "they now point at APIs that no longer exist. Harmless to the running server, and yours to clear up.") + } +} diff --git a/cmd/migrate/uninstall_test.go b/cmd/migrate/uninstall_test.go new file mode 100644 index 00000000..2ae79df4 --- /dev/null +++ b/cmd/migrate/uninstall_test.go @@ -0,0 +1,504 @@ +package migrate + +import ( + "strconv" + "strings" + "testing" + "time" + + "github.com/glebarez/sqlite" + "github.com/go-admin-team/go-admin-core/v2/sdk/contract/seed" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + adminmodels "go-admin/app/admin/models" + _ "go-admin/app/admin/service" // registers the seeder SeedMenus dispatches to + "go-admin/cmd/migrate/migration" + commonmodels "go-admin/common/models" +) + +const adminRoleKey = "admin" + +// newUninstallDB builds every table an install writes to, plus one table +// standing in for the application's own data, which an uninstall must not +// touch. +func newUninstallDB(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( + &adminmodels.SysMenu{}, &adminmodels.SysApi{}, &adminmodels.SysRole{}, + &adminmodels.SysApp{}, &adminmodels.SysAppCasbinGrant{}, &commonmodels.Migration{}, + ); err != nil { + t.Fatalf("automigrate: %v", err) + } + // casbin_rule has no GORM model in this repository; the columns are the + // ones grantToAdminRole's INSERT addresses. + if err := db.Exec(`CREATE TABLE casbin_rule ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + ptype TEXT, v0 TEXT, v1 TEXT, v2 TEXT, v3 TEXT, v4 TEXT, v5 TEXT + )`).Error; err != nil { + t.Fatalf("create casbin_rule: %v", err) + } + if err := db.Exec(`CREATE TABLE app_order (id INTEGER PRIMARY KEY, note TEXT)`).Error; err != nil { + t.Fatalf("create app_order: %v", err) + } + if err := db.Exec(`INSERT INTO app_order (id, note) VALUES (1, 'a real order')`).Error; err != nil { + t.Fatalf("seed app_order: %v", err) + } + if err := db.Create(&adminmodels.SysRole{RoleName: "Administrator", RoleKey: adminRoleKey}).Error; err != nil { + t.Fatalf("seed admin role: %v", err) + } + return db +} + +// specsFor builds one application's menus and apis. The paths carry the app +// code, because two applications do not share an endpoint - and if a fixture +// let them, the second one's policies would already exist and its ledger +// would legitimately come out empty, which would make it a useless control. +func specsFor(code string) ([]seed.MenuSpec, []seed.ApiSpec) { + menus := []seed.MenuSpec{ + {Code: "dir", Kind: "M", Title: code + " example", Path: "/apps/" + code, Component: "Layout", Sort: 10}, + {Code: "list", Parent: "dir", Kind: "C", Title: code, Path: "list", Component: "apps/" + code + "/index", Sort: 1, ApiCodes: []string{"list"}}, + } + apis := []seed.ApiSpec{ + {Code: "list", Title: code + " list", Path: "/api/v1/" + code, Method: "GET", Handle: "apis." + code + ".GetPage-fm"}, + {Code: "create", Title: "create " + code, Path: "/api/v1/" + code, Method: "POST", Handle: "apis." + code + ".Insert-fm"}, + } + return menus, apis +} + +// seedApp runs the real seeding path, so what the uninstaller has to undo is +// what an install actually writes rather than a hand-built approximation. +func seedApp(t *testing.T, db *gorm.DB, code string) { + t.Helper() + menus, apis := specsFor(code) + if err := db.Transaction(func(tx *gorm.DB) error { + return seed.SeedMenus(tx, code, menus, apis) + }); err != nil { + t.Fatalf("seeding %q: %v", code, err) + } + if err := db.Create(&commonmodels.Migration{ + Version: code + "-1786800001000", AppCode: code, ApplyTime: time.Now(), + }).Error; err != nil { + t.Fatalf("recording the migration for %q: %v", code, err) + } + if err := db.Create(&adminmodels.SysApp{ + AppCode: code, Name: code, Version: "1.0.0", Status: adminmodels.AppInstalled, + }).Error; err != nil { + t.Fatalf("recording sys_app for %q: %v", code, err) + } +} + +func count(t *testing.T, db *gorm.DB, table, where string, args ...any) int64 { + t.Helper() + var n int64 + q := db.Table(table) + if where != "" { + q = q.Where(where, args...) + } + if err := q.Count(&n).Error; err != nil { + t.Fatalf("counting %s: %v", table, err) + } + return n +} + +// A3: everything the install wrote goes, and the application's own table does +// not. +func TestUninstallRemovesWhatWasSeededAndNothingElse(t *testing.T) { + db := newUninstallDB(t) + seedApp(t, db, "order") + + if count(t, db, "sys_menu", "app_code = ?", "order") == 0 { + t.Fatal("nothing was seeded, so this test proves nothing") + } + + rep, err := uninstall(db, "order") + if err != nil { + t.Fatalf("uninstall: %v", err) + } + if !rep.Found { + t.Error("the sys_app row was not found") + } + + for _, c := range []struct { + table, where string + args []any + }{ + {"sys_menu", "app_code = ?", []any{"order"}}, + {"sys_api", "app_code = ?", []any{"order"}}, + {"sys_menu_api_rule", "", nil}, + {"sys_role_menu", "", nil}, + {"casbin_rule", "v1 = ?", []any{"/api/v1/order"}}, + {"sys_app_casbin_grant", "app_code = ?", []any{"order"}}, + {"sys_migration", "app_code = ?", []any{"order"}}, + {"sys_app", "app_code = ?", []any{"order"}}, + } { + if n := count(t, db, c.table, c.where, c.args...); n != 0 { + t.Errorf("%s still has %d row(s)", c.table, n) + } + } + if n := count(t, db, "app_order", "", nil); n != 1 { + t.Errorf("app_order has %d row(s); the application's own data is not the uninstaller's to remove", n) + } + if len(rep.Skipped) != 0 || len(rep.Orphans) != 0 { + t.Errorf("a clean uninstall reported skipped=%v orphans=%v", rep.Skipped, rep.Orphans) + } + if rep.Menus == 0 || rep.Apis == 0 || rep.Policies == 0 || rep.Migrations == 0 { + t.Errorf("the report says nothing was removed: %+v", rep) + } +} + +// Uninstalling one application must not reach into another's rows. Every +// delete here is filtered, and a missing filter is invisible on a database +// with only one application in it. +func TestUninstallLeavesAnotherApplicationAlone(t *testing.T) { + db := newUninstallDB(t) + seedApp(t, db, "order") + seedApp(t, db, "crm") + + before := map[string]int64{ + "sys_menu": count(t, db, "sys_menu", "app_code = ?", "crm"), + "sys_api": count(t, db, "sys_api", "app_code = ?", "crm"), + "sys_app_casbin_grant": count(t, db, "sys_app_casbin_grant", "app_code = ?", "crm"), + "sys_migration": count(t, db, "sys_migration", "app_code = ?", "crm"), + "sys_app": count(t, db, "sys_app", "app_code = ?", "crm"), + } + for k, v := range before { + if v == 0 { + t.Fatalf("crm has no rows in %s, so this test proves nothing", k) + } + } + crmBindings := count(t, db, "sys_menu_api_rule", "", nil) + crmRoleMenus := count(t, db, "sys_role_menu", "", nil) + + if _, err := uninstall(db, "order"); err != nil { + t.Fatalf("uninstall: %v", err) + } + + for k, v := range before { + if n := count(t, db, k, "app_code = ?", "crm"); n != v { + t.Errorf("%s for crm went from %d to %d", k, v, n) + } + } + // crm's own bindings and role rows are half of each total, and must be + // exactly what is left. + if n := count(t, db, "sys_menu_api_rule", "", nil); n != crmBindings/2 { + t.Errorf("sys_menu_api_rule = %d, want %d (crm's half)", n, crmBindings/2) + } + if n := count(t, db, "sys_role_menu", "", nil); n != crmRoleMenus/2 { + t.Errorf("sys_role_menu = %d, want %d (crm's half)", n, crmRoleMenus/2) + } + // crm's policies name a different path, so they are untouched. + if n := count(t, db, "casbin_rule", "v1 = ?", "/api/v1/order"); n != 0 { + t.Errorf("order's policies survived: %d", n) + } +} + +// A6b: somebody granted this application's API to another role by hand. That +// grant is not in the ledger, is not this uninstall's to remove, and would +// otherwise vanish from view entirely. +func TestUninstallReportsAGrantSomebodyElseMade(t *testing.T) { + db := newUninstallDB(t) + seedApp(t, db, "order") + + if err := db.Exec( + "INSERT INTO casbin_rule (ptype, v0, v1, v2, v3, v4, v5) VALUES ('p', 'ops', '/api/v1/order', 'GET', '', '', '')", + ).Error; err != nil { + t.Fatalf("hand-made grant: %v", err) + } + + rep, err := uninstall(db, "order") + if err != nil { + t.Fatalf("uninstall: %v", err) + } + if n := count(t, db, "casbin_rule", "v0 = ?", "ops"); n != 1 { + t.Errorf("somebody else's grant was deleted (%d rows left)", n) + } + if len(rep.Orphans) != 1 { + t.Fatalf("orphans = %v, want the one hand-made grant", rep.Orphans) + } + if rep.Orphans[0].V0 != "ops" { + t.Errorf("orphan = %+v", rep.Orphans[0]) + } + // The admin grants it did own are gone. + if n := count(t, db, "casbin_rule", "v0 = ?", adminRoleKey); n != 0 { + t.Errorf("%d of this app's own policies survived", n) + } + + var out strings.Builder + reportUninstall(&out, rep) + if !strings.Contains(out.String(), "ops") || !strings.Contains(out.String(), "left alone") { + t.Errorf("the report does not say what was left behind: %q", out.String()) + } +} + +// A6a: a policy this install created is not there any more. Not an error - +// the uninstall wanted it gone and it is - but reported, because something +// else rewrote casbin_rule. +func TestUninstallReportsALedgerEntryWhosePolicyIsGone(t *testing.T) { + db := newUninstallDB(t) + seedApp(t, db, "order") + + if err := db.Exec("DELETE FROM casbin_rule WHERE v2 = 'POST'").Error; err != nil { + t.Fatalf("removing a policy: %v", err) + } + + rep, err := uninstall(db, "order") + if err != nil { + t.Fatalf("a missing policy made the uninstall fail: %v", err) + } + if len(rep.Skipped) != 1 { + t.Fatalf("skipped = %v, want the one that had gone", rep.Skipped) + } + if rep.Skipped[0].V2 != "POST" { + t.Errorf("skipped = %+v", rep.Skipped[0]) + } + if n := count(t, db, "sys_app_casbin_grant", "", nil); n != 0 { + t.Errorf("the ledger kept %d row(s); its job ends with the uninstall", n) + } + // It still committed: a skip is a reported branch, not a failure. + if n := count(t, db, "sys_menu", "app_code = ?", "order"); n != 0 { + t.Errorf("the transaction rolled back over a skip: sys_menu has %d row(s)", n) + } +} + +// G5/A4: without this the reinstall finds every version applied, runs no +// migration, seeds nothing, and reports success. +func TestUninstallClearsThisAppsMigrationRecordsOnly(t *testing.T) { + db := newUninstallDB(t) + seedApp(t, db, "order") + if err := db.Create(&commonmodels.Migration{ + Version: "1786700001000", AppCode: "", ApplyTime: time.Now(), + }).Error; err != nil { + t.Fatalf("framework migration row: %v", err) + } + + if _, err := uninstall(db, "order"); err != nil { + t.Fatalf("uninstall: %v", err) + } + if n := count(t, db, "sys_migration", "app_code = ?", "order"); n != 0 { + t.Errorf("sys_migration still has %d row(s) for order; a reinstall would seed nothing", n) + } + if n := count(t, db, "sys_migration", "app_code = ?", ""); n != 1 { + t.Errorf("the framework's own migration record was removed (%d left)", n) + } +} + +// A11: sys_role_menu is found by menu id, not by a column on it. A column +// would have been blanked the first time somebody edited a role, because +// SysRole.Update deletes the role's rows and writes them back through GORM's +// many2many, which does not carry extra columns. This reproduces that edit. +func TestUninstallSurvivesARoleMenuRewrite(t *testing.T) { + db := newUninstallDB(t) + seedApp(t, db, "order") + + var roleID int + if err := db.Model(&adminmodels.SysRole{}).Where("role_key = ?", adminRoleKey). + Pluck("role_id", &roleID).Error; err != nil { + t.Fatalf("reading the admin role: %v", err) + } + var menuIDs []int + if err := db.Model(&adminmodels.SysMenu{}).Where("app_code = ?", "order"). + Pluck("menu_id", &menuIDs).Error; err != nil { + t.Fatalf("reading menus: %v", err) + } + if len(menuIDs) == 0 { + t.Fatal("no menus were seeded") + } + // What SysRole.Update does: drop every row for the role, then write them + // back with nothing but the two keys. + if err := db.Exec("DELETE FROM sys_role_menu WHERE role_id = ?", roleID).Error; err != nil { + t.Fatalf("clearing role menus: %v", err) + } + for _, id := range menuIDs { + if err := db.Exec("INSERT INTO sys_role_menu (role_id, menu_id) VALUES (?, ?)", roleID, id).Error; err != nil { + t.Fatalf("rewriting role menus: %v", err) + } + } + + rep, err := uninstall(db, "order") + if err != nil { + t.Fatalf("uninstall: %v", err) + } + if rep.RoleMenus != int64(len(menuIDs)) { + t.Errorf("removed %d role assignment(s), want %d", rep.RoleMenus, len(menuIDs)) + } + if n := count(t, db, "sys_role_menu", "", nil); n != 0 { + t.Errorf("sys_role_menu still has %d row(s) after a role edit", n) + } +} + +// `migrate` with no subcommand applies every registered migration, an +// application's included, so an application can have all of its rows and +// never have had a sys_app row. Refusing to clean that up would leave the +// only case where nothing else can. +func TestUninstallWorksWithoutASysAppRow(t *testing.T) { + db := newUninstallDB(t) + seedApp(t, db, "order") + if err := db.Where("app_code = ?", "order").Delete(&adminmodels.SysApp{}).Error; err != nil { + t.Fatalf("removing the sys_app row: %v", err) + } + + rep, err := uninstall(db, "order") + if err != nil { + t.Fatalf("uninstall: %v", err) + } + if rep.Found { + t.Error("the report claims a sys_app row that was not there") + } + if n := count(t, db, "sys_menu", "app_code = ?", "order"); n != 0 { + t.Errorf("sys_menu still has %d row(s)", n) + } + var out strings.Builder + reportUninstall(&out, rep) + if !strings.Contains(out.String(), "no sys_app row") { + t.Errorf("the report does not say the row was missing: %q", out.String()) + } +} + +// One transaction, and it really is one: nothing here runs DDL, so unlike an +// install there is nothing to commit it out from under itself. +func TestUninstallRollsBackAsAWhole(t *testing.T) { + db := newUninstallDB(t) + seedApp(t, db, "order") + menusBefore := count(t, db, "sys_menu", "app_code = ?", "order") + policiesBefore := count(t, db, "casbin_rule", "", nil) + + // Step 8's table is gone, so the uninstall fails after it has already + // deleted menus, apis, bindings and policies. + if err := db.Migrator().DropTable(&commonmodels.Migration{}); err != nil { + t.Fatalf("dropping sys_migration: %v", err) + } + + if _, err := uninstall(db, "order"); err == nil { + t.Fatal("the uninstall reported success with sys_migration missing") + } + if n := count(t, db, "sys_menu", "app_code = ?", "order"); n != menusBefore { + t.Errorf("sys_menu = %d, want %d: the failed uninstall did not roll back", n, menusBefore) + } + if n := count(t, db, "casbin_rule", "", nil); n != policiesBefore { + t.Errorf("casbin_rule = %d, want %d: the failed uninstall did not roll back", n, policiesBefore) + } +} + +func TestUninstallRefusesTheFrameworkCode(t *testing.T) { + db := newUninstallDB(t) + if _, err := uninstall(db, migration.FrameworkAppCode); err == nil { + t.Fatal("the framework was uninstalled") + } +} + +// findOrphanPolicies chunks its OR chain because a driver runs out of +// placeholders long before an application runs out of endpoints. The +// boundary is where an off-by-one hides: a chunk size that drops the last +// element of each batch, or one that never advances, both leave policies +// unreported and nothing says so. +func TestFindOrphanPoliciesCoversEveryPathAcrossChunks(t *testing.T) { + db := newUninstallDB(t) + + // Deliberately not a multiple of the chunk size, so the last batch is + // short, and large enough to need three of them. + const n = 205 + keys := make([]policyKey, 0, n) + for i := 0; i < n; i++ { + path := "/api/v1/thing" + strconv.Itoa(i) + keys = append(keys, policyKey{V1: path, V2: "GET"}) + if err := db.Exec( + "INSERT INTO casbin_rule (ptype, v0, v1, v2, v3, v4, v5) VALUES ('p', 'ops', ?, 'GET', '', '', '')", + path, + ).Error; err != nil { + t.Fatalf("seeding policy %d: %v", i, err) + } + } + // One policy that must not match: a path no key names. + if err := db.Exec( + "INSERT INTO casbin_rule (ptype, v0, v1, v2, v3, v4, v5) VALUES ('p', 'ops', '/api/v1/elsewhere', 'GET', '', '', '')", + ).Error; err != nil { + t.Fatalf("seeding the control policy: %v", err) + } + + found, err := findOrphanPolicies(db, keys) + if err != nil { + t.Fatalf("findOrphanPolicies: %v", err) + } + if len(found) != n { + t.Fatalf("found %d policies, want %d", len(found), n) + } + seen := make(map[string]bool, len(found)) + for _, f := range found { + seen[f.V1] = true + if f.V1 == "/api/v1/elsewhere" { + t.Error("a path no key names was reported") + } + } + for _, k := range keys { + if !seen[k.V1] { + t.Errorf("%s was not reported", k.V1) + } + } +} + +// An application may register apis with no menus at all - endpoints another +// service calls - so either of the id lists an uninstall reads can be empty. +// The guard in front of the join-table delete turns out not to be what makes +// this work: GORM renders IN with an empty slice as a condition that matches +// nothing, rather than the empty IN list that would be a syntax error in raw +// SQL, and removing the guard leaves this test green. It stays as an explicit +// statement of intent rather than a reliance on that rendering. +func TestUninstallWithApisButNoMenus(t *testing.T) { + db := newUninstallDB(t) + apis := []seed.ApiSpec{ + {Code: "hook", Title: "Inbound hook", Path: "/api/v1/hook", Method: "POST", Handle: "hook.Receive"}, + } + if err := db.Transaction(func(tx *gorm.DB) error { + return seed.SeedMenus(tx, "hooks", nil, apis) + }); err != nil { + t.Fatalf("seeding: %v", err) + } + + rep, err := uninstall(db, "hooks") + if err != nil { + t.Fatalf("uninstall: %v", err) + } + if rep.Apis != 1 { + t.Errorf("removed %d api(s), want 1", rep.Apis) + } + if rep.Policies != 1 { + t.Errorf("removed %d policy(ies), want 1", rep.Policies) + } + if n := count(t, db, "casbin_rule", "", nil); n != 0 { + t.Errorf("casbin_rule has %d row(s)", n) + } +} + +// The mirror case: menus and no apis at all. +func TestUninstallWithMenusButNoApis(t *testing.T) { + db := newUninstallDB(t) + menus := []seed.MenuSpec{ + {Code: "dir", Kind: "M", Title: "Reports", Path: "/apps/reports", Component: "Layout", Sort: 10}, + } + if err := db.Transaction(func(tx *gorm.DB) error { + return seed.SeedMenus(tx, "reports", menus, nil) + }); err != nil { + t.Fatalf("seeding: %v", err) + } + + rep, err := uninstall(db, "reports") + if err != nil { + t.Fatalf("uninstall: %v", err) + } + if rep.Menus != 1 { + t.Errorf("removed %d menu(s), want 1", rep.Menus) + } + if n := count(t, db, "sys_menu", "app_code = ?", "reports"); n != 0 { + t.Errorf("sys_menu has %d row(s)", n) + } + if len(rep.Orphans) != 0 { + t.Errorf("an application with no apis reported orphans: %v", rep.Orphans) + } +}