diff --git a/app/admin/models/sys_app.go b/app/admin/models/sys_app.go new file mode 100644 index 00000000..72008266 --- /dev/null +++ b/app/admin/models/sys_app.go @@ -0,0 +1,89 @@ +package models + +import ( + "time" + + "go-admin/common/models" +) + +// 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 +// installed-app registry does not need the millisecond soft-delete marker +// every other sys_* table follows. Uninstalling an app deletes its row +// outright; a later reinstall creates a fresh one. +type SysApp struct { + models.Model // Id int, primary key, autoincrement + + // AppCode is the app.Manifest.Code / migration.ForApp / seed.SeedMenus + // identity, already lower-cased by migration.NormalizeAppCode before + // anything reaches this table. Unique: row existence alone answers G2 + // ("is app X installed"). + AppCode string `json:"appCode" gorm:"type:varchar(64);not null;uniqueIndex:uk_sys_app_app_code;comment:app code"` + + Name string `json:"name" gorm:"size:128;not null;comment:display name, from Manifest.Name"` + // Version is the version this row currently reflects - attempted or + // confirmed, disambiguated by Status. It does not drive which + // migrations run next; sys_migration's per-version rows do that (see + // design doc §1.5's resume flow). This field is descriptive, refreshed + // from the manifest on every install/upgrade/resume attempt. + Version string `json:"version" gorm:"size:32;not null;comment:version this row currently reflects, see Status"` + Description string `json:"description" gorm:"size:255;not null;default:'';comment:from Manifest.Description"` + Author string `json:"author" gorm:"size:128;not null;default:'';comment:from Manifest.Author"` + + // Requires is a comma-separated list of app codes this app declared as + // dependencies (Manifest.Requires). Stored as plain VARCHAR CSV, not + // JSON - see design doc §1.3 for why. F8 (P1) is what validates and + // orders on this; this batch only stores what the manifest declared. + Requires string `json:"requires" gorm:"size:255;not null;default:'';comment:declared dependency app codes, comma separated"` + + // Pricing/License are reserved passthrough fields (PRD 003; PRD 008 + // open question 1). This batch stores whatever the manifest carries and + // does not interpret either one. + Pricing string `json:"pricing" gorm:"size:64;not null;default:'';comment:reserved, not interpreted by this batch"` + License string `json:"license" gorm:"size:64;not null;default:'';comment:reserved, not interpreted by this batch"` + + // Status: 1=installing 2=installed 3=failed. Three states, not a + // single "1=installed", because a partial, stuck install has to be an + // observable row rather than "the row doesn't exist yet" - see design + // doc §1.5 for why cross-migration-file atomicity is not available on + // MySQL (implicit commit on DDL). + Status int `json:"status" gorm:"size:4;not null;default:1;comment:1=installing 2=installed 3=failed"` + + // FailedVersion and LastError are DIAGNOSTIC TEXT ONLY - what a human + // looking at this row is told about the last failure, nothing more. No + // code anywhere may read either one to decide what to do next. + // + // The question "where should a resume pick up" has exactly one + // authoritative answer, and it is not these two columns: subtract + // sys_migration's applied rows for this app_code from what the app's + // own compiled-in code has registered (migration.Snapshot()/ForApp - + // the same set F7's `migrate status` already walks). That answer can + // never go stale, because it is not stored anywhere to go stale - it is + // recomputed from sys_migration every time it is asked. FailedVersion + // is a snapshot of what that computation returned at the moment of + // failure, kept only so an operator does not have to go find the + // process's logs; if it and a fresh recomputation from sys_migration + // ever disagree, sys_migration is right and this column is stale, by + // definition, and nothing should ever notice or care except a human + // reading the row. + FailedVersion string `json:"failedVersion" gorm:"size:64;not null;default:'';comment:diagnostic snapshot only, not a judgment basis; meaningful only when status=3"` + LastError string `json:"lastError" gorm:"size:255;not null;default:'';comment:diagnostic text only, not a judgment basis; meaningful only when status=3"` + + // InstalledAt is when this app first reached status=installed - set + // once, never moved by a later upgrade (see design doc §1.4). Nullable, + // unlike every other column here: a row can exist before it has a + // value (a fresh install starts at status=installing). This is not the + // deleted_at problem 1786700003000_soft_delete_marker.go fixed - that + // column sat inside a unique index, where NULL <> NULL let two live + // rows coexist under the same key. InstalledAt is in no index at all, + // so nullability here opens no such hole. + InstalledAt *time.Time `json:"installedAt" gorm:"comment:first successful install time; null until status first reaches installed"` + UpdatedAt time.Time `json:"updatedAt" gorm:"comment:last updated time"` + + models.ControlBy // CreateBy/UpdateBy: which operator triggered the attempt +} + +func (*SysApp) TableName() string { + return "sys_app" +} diff --git a/app/admin/models/sys_app_casbin_grant.go b/app/admin/models/sys_app_casbin_grant.go new file mode 100644 index 00000000..b375d15f --- /dev/null +++ b/app/admin/models/sys_app_casbin_grant.go @@ -0,0 +1,43 @@ +package models + +import "time" + +// SysAppCasbinGrant is a ledger of casbin_rule rows an app install created, +// keyed by the exact natural key casbin_rule itself is unique on. It exists +// because casbin_rule is not a table this project owns (see design doc +// docs-prd/008-应用清单与安装器/数据库变更.md §2.2): we cannot add an +// app_code column to it without that column being silently zeroed the first +// time anything calls the gorm-adapter's SavePolicy/SavePolicyCtx. Recording +// the natural key here, instead of a foreign key into casbin_rule, is also +// what survives SysRole.Update's RemoveFilteredPolicy+re-add cycle for a +// role's policies (app/admin/service/sys_role.go): that cycle replaces the +// underlying row (a new auto-increment ID) but reproduces the same +// (ptype,v0,v1,v2) tuple from the same sys_menu/sys_api data, so a +// natural-key match here still finds it. What it does not survive is the +// role being renamed, or the tuple being rebuilt from a completely different +// source (a future SavePolicy call from outside this seeder) - in both cases +// the match legitimately fails, and business rule 3 says the uninstaller +// should report and skip, not delete something else that happens to look +// the same. +type SysAppCasbinGrant struct { + Id int `json:"id" gorm:"primaryKey;autoIncrement"` + + AppCode string `json:"appCode" gorm:"type:varchar(64);not null;index:idx_sys_app_casbin_grant_app_code;comment:app code that created this grant"` + + // Column widths mirror gorm-adapter's own CasbinRule struct exactly, so + // a value that fits into casbin_rule always fits here, and the unique + // index below matches the one createTable() puts on casbin_rule itself. + Ptype string `json:"ptype" gorm:"size:100;not null;uniqueIndex:uk_sys_app_casbin_grant_rule;comment:casbin ptype, 'p' today"` + V0 string `json:"v0" gorm:"size:100;not null;default:'';uniqueIndex:uk_sys_app_casbin_grant_rule;comment:role_key at grant time"` + V1 string `json:"v1" gorm:"size:100;not null;default:'';uniqueIndex:uk_sys_app_casbin_grant_rule;comment:api path"` + V2 string `json:"v2" gorm:"size:100;not null;default:'';uniqueIndex:uk_sys_app_casbin_grant_rule;comment:http method"` + V3 string `json:"v3" gorm:"size:100;not null;default:'';uniqueIndex:uk_sys_app_casbin_grant_rule;comment:unused today"` + V4 string `json:"v4" gorm:"size:100;not null;default:'';uniqueIndex:uk_sys_app_casbin_grant_rule;comment:unused today"` + V5 string `json:"v5" gorm:"size:100;not null;default:'';uniqueIndex:uk_sys_app_casbin_grant_rule;comment:unused today"` + + CreatedAt time.Time `json:"createdAt" gorm:"comment:when this grant was recorded"` +} + +func (*SysAppCasbinGrant) TableName() string { + return "sys_app_casbin_grant" +} diff --git a/app/admin/models/sys_menu.go b/app/admin/models/sys_menu.go index 2f4baf5c..1fdddc57 100644 --- a/app/admin/models/sys_menu.go +++ b/app/admin/models/sys_menu.go @@ -32,6 +32,19 @@ type SysMenu struct { // AutoMigrate adding this column to an existing table leaves every // pre-existing row reading back as "" rather than NULL. AppCode string `json:"appCode" gorm:"type:varchar(64);not null;default:'';index:idx_sys_menu_app_code;comment:AppCode"` + // SeedCode is the raw seed.MenuSpec.Code this row was created from, kept + // so seedMenuTree can ask "did I already write this node" without + // relying on MenuName's PascalCase concatenation, which is not + // injective (see design doc §1.6). Nullable, unlike AppCode: every row + // seed.SeedMenus writes sets a real value, but every pre-existing row - + // the host's own hand-placed menus, and every app-seeded row written + // before this column existed - has none, and there is no way to + // backfill one that means anything. NULL is what lets an unbounded + // number of those coexist under the same app_code without tripping the + // 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"` models.ControlBy models.ModelTime } diff --git a/app/admin/service/seed.go b/app/admin/service/seed.go index 0dc4562a..e856566e 100644 --- a/app/admin/service/seed.go +++ b/app/admin/service/seed.go @@ -90,6 +90,23 @@ func (adminSeeder) SeedMenus(tx *gorm.DB, appCode string, menus []seed.MenuSpec, // application's ids in the module cache. Never accepting a caller-chosen id // here removes the collision this Seeder has no way to detect instead of // trying to detect it after the fact. +// +// The natural key is (app_code, path, action) - the same three columns +// 1786700002000_remove_refresh_token_api.go already used to identify a +// single API by hand, and the ones 1786700008000_seed_natural_keys.go put a +// unique index on. Before inserting, this looks for a live row (deleted_at +// = 0, applied automatically by the soft-delete plugin on every query +// against models.SysApi) already holding that key and reuses it instead of +// inserting a second one - see the design doc §1.6: a migration retried +// after a partial failure previously re-ran this as a bare tx.Create and +// produced duplicate rows on the demo site. +// +// Unlike seedMenuTree's reuse branch, this one has nothing left to repair +// after finding an existing row: models.SysApi carries no association +// (nothing like SysMenu's many2many SysApi field) and this function writes +// nothing beyond the row itself - no second statement comparable to +// seedMenuTree's paths UPDATE follows tx.Create below. An interrupted retry +// can therefore only ever find this row complete or not find it at all. func seedApis(tx *gorm.DB, appCode string, apis []seed.ApiSpec) (map[string]models.SysApi, error) { seen := make(map[string]bool, len(apis)) rows := make(map[string]models.SysApi, len(apis)) @@ -102,6 +119,19 @@ func seedApis(tx *gorm.DB, appCode string, apis []seed.ApiSpec) (map[string]mode } seen[a.Code] = true + var existing models.SysApi + err := tx.Where("app_code = ? AND path = ? AND action = ?", appCode, a.Path, a.Method). + First(&existing).Error + switch { + case err == nil: + rows[a.Code] = existing + continue + case errors.Is(err, gorm.ErrRecordNotFound): + // Not seen yet; fall through to insert it. + default: + return nil, fmt.Errorf("api %q: checking for an existing row: %w", a.Code, err) + } + row := models.SysApi{ Handle: a.Handle, Title: a.Title, @@ -153,6 +183,12 @@ func seedMenuTree(tx *gorm.DB, appCode string, specs []seed.MenuSpec, apiRows ma continue } + // Resolved before the idempotency check below, whether or not + // this spec's own row turns out to already exist: repairing an + // existing-but-incomplete row's paths needs the parent's + // already-resolved Paths exactly as much as creating a fresh + // row does (see repairExistingMenu), so both have to wait for + // it the same way. var parentRow models.SysMenu if s.Parent != "" { parent, ok := created[s.Parent] @@ -165,6 +201,32 @@ func seedMenuTree(tx *gorm.DB, appCode string, specs []seed.MenuSpec, apiRows ma parentRow = parent } + // Idempotency check: does this node already have a row, from + // an earlier, possibly-interrupted attempt? The natural key is + // (app_code, seed_code) - menu_name's PascalCase concatenation + // is not injective and cannot be used for this (see menuName's + // doc comment and the design doc §1.6). Only a live row counts; + // the soft-delete plugin scopes deleted_at = 0 automatically on + // every query against models.SysMenu. + var existing models.SysMenu + err := tx.Where("app_code = ? AND seed_code = ?", appCode, s.Code).First(&existing).Error + switch { + case err == nil: + row, err := repairExistingMenu(tx, existing, s, parentRow, apiRows) + if err != nil { + return nil, fmt.Errorf("%q: repairing an existing row: %w", s.Code, err) + } + created[s.Code] = row + ids = append(ids, row.MenuId) + progressed = true + continue + case errors.Is(err, gorm.ErrRecordNotFound): + // Not written yet; fall through to create it below. + default: + return nil, fmt.Errorf("%q: checking for an existing row: %w", s.Code, err) + } + + seedCode := s.Code row := models.SysMenu{ MenuName: menuName(appCode, s.Code), Title: s.Title, @@ -179,9 +241,10 @@ func seedMenuTree(tx *gorm.DB, appCode string, specs []seed.MenuSpec, apiRows ma // 1786700001000_demo_menu.go seeds its own menu with. A // freshly installed application's menu should not need an // administrator to first find and unhide it. - Visible: "0", - IsFrame: "1", - AppCode: appCode, + Visible: "0", + IsFrame: "1", + AppCode: appCode, + SeedCode: &seedCode, } for _, code := range s.ApiCodes { api, ok := apiRows[code] @@ -205,11 +268,7 @@ func seedMenuTree(tx *gorm.DB, appCode string, specs []seed.MenuSpec, apiRows ma // two-step create-then-update 1786700001000_demo_menu.go's // hand-assigned ids let it do in one literal, sequenced here // instead. - if s.Parent == "" { - row.Paths = "/0/" + strconv.Itoa(row.MenuId) - } else { - row.Paths = parentRow.Paths + "/" + strconv.Itoa(row.MenuId) - } + row.Paths = expectedPaths(row.MenuId, s.Parent, parentRow) if err := tx.Model(&models.SysMenu{}).Where("menu_id = ?", row.MenuId). Update("paths", row.Paths).Error; err != nil { return nil, fmt.Errorf("%q: writing paths: %w", s.Code, err) @@ -226,6 +285,91 @@ func seedMenuTree(tx *gorm.DB, appCode string, specs []seed.MenuSpec, apiRows ma return ids, nil } +// expectedPaths is the materialized path a fresh insert of menuID under +// parent (or at the root, if parent is "") computes - factored out so +// repairExistingMenu can ask the same question about a row it did not just +// create. +func expectedPaths(menuID int, parent string, parentRow models.SysMenu) string { + if parent == "" { + return "/0/" + strconv.Itoa(menuID) + } + return parentRow.Paths + "/" + strconv.Itoa(menuID) +} + +// repairExistingMenu brings a row seedMenuTree's idempotency check found up +// to what a fresh insert of the same spec would have produced. +// +// A row can be found and still be incomplete: tx.Create's own association +// write (the sys_menu_api_rule bindings from row.SysApi) and the paths +// UPDATE that follows it are each their own statement, and design doc §1.5 +// establishes that nothing after the first DDL in a migration function can +// be rolled back together - a process interrupted between the row insert +// and either of those two steps leaves exactly this row: present, findable +// by its natural key, but missing what makes it a working menu entry. A +// retry that only checked "does the row exist" and stopped there would +// report success while the sys_menu_api_rule binding stays missing (the +// api is granted to no one) or paths stays empty (a materialized-path +// break that orphans the rest of the subtree from the root) - as silent as +// the duplicate-row defect the idempotency check itself was written to +// close. +// +// Both checks are read-before-write, so a row that is already complete - +// the ordinary case on every retry after the first successful one - causes +// no writes at all: existing.Paths already equals what expectedPaths +// computes, and the sys_menu_api_rule INSERT is itself guarded by +// WHERE NOT EXISTS, the same idempotent-insert shape grantToAdminRole +// already uses for sys_role_menu/casbin_rule. Never DELETEs an existing +// binding to rebuild it - that is the FullSaveAssociations mistake +// sys_role.go's SysRole.Update makes for sys_role_menu/casbin_rule +// (app/admin/service/sys_role.go:148-153), the exact pattern this design +// went out of its way to avoid for the tables that do use it. +// +// Insert-only cuts both ways, deliberately. A binding an administrator +// added by hand through the menu management UI, for an api never in +// s.ApiCodes at all, is never touched by this loop and survives every +// later retry (TestSeedMenusPreservesAHandAddedBinding is the reproduction +// case for the opposite mistake: delete-then-reinsert wipes it silently, +// the same shape as sys_role_menu/casbin_rule getting zeroed by a role +// edit, just with this code as the actor instead of the victim). The +// converse case - a MenuSpec that used to list an ApiCode and no longer +// does - is not handled here either, and that half is intentional rather +// than an oversight: this loop only ever adds rows for codes the *current* +// call's ApiCodes names, so a binding for a code an earlier version +// granted and the current one dropped is left in place, stale. Reconciling +// that is deleting something, which needs the same certainty about +// ownership uninstall's design (see design doc §5) already requires - +// this function has no way to tell "stale, from an older version of this +// same app" apart from "hand-added, for a reason", and business rule 3 +// ("uninstall deletes only what it can attribute with certainty") applies +// here just as much as it does there. Reconciling stale seed-driven +// bindings, if it is ever wanted, belongs in the upgrade path with that +// same ownership check - not silently inside every retry of every install. +func repairExistingMenu(tx *gorm.DB, existing models.SysMenu, s seed.MenuSpec, parentRow models.SysMenu, apiRows map[string]models.SysApi) (models.SysMenu, error) { + want := expectedPaths(existing.MenuId, s.Parent, parentRow) + if existing.Paths != want { + if err := tx.Model(&models.SysMenu{}).Where("menu_id = ?", existing.MenuId). + Update("paths", want).Error; err != nil { + return models.SysMenu{}, fmt.Errorf("repairing paths: %w", err) + } + existing.Paths = want + } + + for _, code := range s.ApiCodes { + api, ok := apiRows[code] + if !ok { + return models.SysMenu{}, fmt.Errorf("ApiCodes references %q, which is not an ApiSpec.Code in this call", code) + } + if err := tx.Exec( + "INSERT INTO sys_menu_api_rule (sys_menu_menu_id, sys_api_id) SELECT ?, ? WHERE NOT EXISTS (SELECT 1 FROM sys_menu_api_rule WHERE sys_menu_menu_id = ? AND sys_api_id = ?)", + existing.MenuId, api.Id, existing.MenuId, api.Id, + ).Error; err != nil { + return models.SysMenu{}, fmt.Errorf("binding %q: %w", code, err) + } + } + + return existing, nil +} + // validateMenuSpec rejects the malformed input tools/checksilent's // menu-sort-overflow and Kind-adjacent checks would catch for an in-tree // seed but cannot for a third-party application's - see menuSortRange's doc diff --git a/app/admin/service/seed_test.go b/app/admin/service/seed_test.go index fe7a9845..9fc02557 100644 --- a/app/admin/service/seed_test.go +++ b/app/admin/service/seed_test.go @@ -1,13 +1,17 @@ package service import ( + "context" "errors" "strconv" "strings" + "sync" "testing" + "time" "github.com/glebarez/sqlite" "gorm.io/gorm" + gormlogger "gorm.io/gorm/logger" contractmodels "github.com/go-admin-team/go-admin-core/v2/sdk/contract/models" "github.com/go-admin-team/go-admin-core/v2/sdk/contract/seed" @@ -289,3 +293,551 @@ func TestSeedMenusWithNothingRegisteredWritesNothing(t *testing.T) { } } } + +// newSeedTestDB's AutoMigrate builds a unique index on seed_code alone, +// because SysMenu.SeedCode is the only field in the struct carrying the +// uk_sys_menu_app_seed_code_del tag - app_code already carries a different, +// non-unique index name of its own, and the embedded ModelTime's +// DeletedAt (aliased from go-admin-core) cannot be given a third one. The +// real migration (cmd/migrate/migration/version/1786700008000_seed_natural_keys.go) +// never lets AutoMigrate touch this table for exactly that reason: it +// builds the composite (app_code, seed_code, deleted_at) index by hand +// instead. Reproduce that by hand here too, so a test that seeds two rows +// sharing a seed_code under different deleted_at values sees what a real +// install would, not gorm's narrower default. +func useCompositeSeedCodeIndex(t *testing.T, db *gorm.DB) { + t.Helper() + if db.Migrator().HasIndex(&models.SysMenu{}, "uk_sys_menu_app_seed_code_del") { + if err := db.Migrator().DropIndex(&models.SysMenu{}, "uk_sys_menu_app_seed_code_del"); err != nil { + t.Fatalf("drop the single-column seed_code index: %v", err) + } + } + if err := db.Exec( + "CREATE UNIQUE INDEX uk_sys_menu_app_seed_code_del ON sys_menu (app_code, seed_code, deleted_at)", + ).Error; err != nil { + t.Fatalf("create the composite seed_code index: %v", err) + } +} + +// A retried migration - one that failed partway through and is run again, +// or simply run twice by mistake - must not create a second sys_api or +// sys_menu row for the same (appCode, natural key). This is the defect the +// demo site hit in production: duplicate sys_menu/casbin_rule rows from a +// bare tx.Create on a natural key nothing was checking. +func TestSeedMenusIsIdempotentAcrossARetry(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + menus := []seed.MenuSpec{ + {Code: "dir", Kind: contractmodels.Directory, Title: "Order", Sort: 10}, + {Code: "list", Parent: "dir", Kind: contractmodels.Menu, Title: "Orders", Sort: 1, ApiCodes: []string{"list"}}, + } + apis := []seed.ApiSpec{ + {Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET"}, + } + + run := func() { + t.Helper() + if err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", menus, apis) + }); err != nil { + t.Fatalf("SeedMenus: %v", err) + } + } + run() + firstMenuIDs := allMenuIDs(t, db, "order") + firstApiIDs := allApiIDs(t, db, "order") + + run() // the retry + + if got := allMenuIDs(t, db, "order"); !sameIDs(got, firstMenuIDs) { + t.Errorf("sys_menu ids after retry = %v, want unchanged %v (a second call inserted new rows)", got, firstMenuIDs) + } + if got := allApiIDs(t, db, "order"); !sameIDs(got, firstApiIDs) { + t.Errorf("sys_api ids after retry = %v, want unchanged %v (a second call inserted new rows)", got, firstApiIDs) + } + + assertRowCount(t, db, "sys_api", 1) + assertRowCount(t, db, "sys_menu", 2) + assertRowCount(t, db, "sys_menu_api_rule", 1) + assertRowCount(t, db, "sys_role_menu", 2) + assertRowCount(t, db, "casbin_rule", 1) +} + +// Only a live row counts as "already written". A row a prior, unrelated +// soft-delete already retired must not be reused - seedApis/seedMenuTree +// have to insert a fresh one under the same natural key, the same way the +// unique indexes 1786700008000_seed_natural_keys.go builds only bind live +// rows. +func TestSeedMenusOnlyReusesLiveRows(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + menus := []seed.MenuSpec{{Code: "dir", Kind: contractmodels.Directory, Title: "Order", Sort: 10}} + apis := []seed.ApiSpec{{Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET"}} + + if err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", menus, apis) + }); err != nil { + t.Fatalf("SeedMenus: %v", err) + } + + // Soft-delete both rows this first call wrote, as if an operator (or an + // earlier uninstall) had retired them, independently of this migration + // ever running again. + if err := db.Exec("UPDATE sys_menu SET deleted_at = 1").Error; err != nil { + t.Fatalf("soft-delete sys_menu: %v", err) + } + if err := db.Exec("UPDATE sys_api SET deleted_at = 1").Error; err != nil { + t.Fatalf("soft-delete sys_api: %v", err) + } + + if err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", menus, apis) + }); err != nil { + t.Fatalf("SeedMenus after soft-delete: %v", err) + } + + // Two rows total: the soft-deleted original, plus a fresh one - not the + // dead row resurrected in place, and not left with zero live rows. + assertRowCount(t, db, "sys_menu", 2) + assertRowCount(t, db, "sys_api", 2) + + var liveMenus, liveApis int64 + db.Model(&models.SysMenu{}).Where("app_code = ?", "order").Count(&liveMenus) + db.Model(&models.SysApi{}).Where("app_code = ?", "order").Count(&liveApis) + if liveMenus != 1 { + t.Errorf("live sys_menu rows = %d, want 1", liveMenus) + } + if liveApis != 1 { + t.Errorf("live sys_api rows = %d, want 1", liveApis) + } +} + +// app_code is part of the natural key, not a descriptive column alongside +// it. Two applications that happen to register an identical (path, action) +// or seed_code must each get their own row - reusing one app's row for +// another's install would make an uninstall of the first delete a row the +// second considers its own. +func TestSeedMenusScopesTheNaturalKeyByAppCode(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + menus := []seed.MenuSpec{{Code: "dir", Kind: contractmodels.Directory, Title: "Dir", Sort: 10}} + apis := []seed.ApiSpec{{Code: "list", Title: "Shared endpoint", Path: "/api/v1/shared", Method: "GET"}} + + for _, appCode := range []string{"order", "billing"} { + if err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, appCode, menus, apis) + }); err != nil { + t.Fatalf("SeedMenus(%q): %v", appCode, err) + } + } + + var apiRows []models.SysApi + if err := db.Where("path = ? AND action = ?", "/api/v1/shared", "GET"). + Order("app_code").Find(&apiRows).Error; err != nil { + t.Fatalf("read sys_api: %v", err) + } + if len(apiRows) != 2 { + t.Fatalf("sys_api has %d row(s) for the shared (path, action), want 2 - one per app", len(apiRows)) + } + if apiRows[0].AppCode != "billing" || apiRows[1].AppCode != "order" { + t.Errorf("sys_api app_codes = [%s %s], want [billing order]", apiRows[0].AppCode, apiRows[1].AppCode) + } + + var menuRows []models.SysMenu + if err := db.Where("seed_code = ?", "dir").Order("app_code").Find(&menuRows).Error; err != nil { + t.Fatalf("read sys_menu: %v", err) + } + if len(menuRows) != 2 { + t.Fatalf("sys_menu has %d row(s) for the shared seed_code, want 2 - one per app", len(menuRows)) + } + if menuRows[0].AppCode != "billing" || menuRows[1].AppCode != "order" { + t.Errorf("sys_menu app_codes = [%s %s], want [billing order]", menuRows[0].AppCode, menuRows[1].AppCode) + } +} + +func allMenuIDs(t *testing.T, db *gorm.DB, appCode string) []int { + t.Helper() + var rows []models.SysMenu + if err := db.Where("app_code = ?", appCode).Order("menu_id").Find(&rows).Error; err != nil { + t.Fatalf("read sys_menu: %v", err) + } + ids := make([]int, len(rows)) + for i, r := range rows { + ids[i] = r.MenuId + } + return ids +} + +func allApiIDs(t *testing.T, db *gorm.DB, appCode string) []int { + t.Helper() + var rows []models.SysApi + if err := db.Where("app_code = ?", appCode).Order("id").Find(&rows).Error; err != nil { + t.Fatalf("read sys_api: %v", err) + } + ids := make([]int, len(rows)) + for i, r := range rows { + ids[i] = r.Id + } + return ids +} + +func sameIDs(a, b []int) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func assertRowCount(t *testing.T, db *gorm.DB, table string, want int64) { + t.Helper() + var n int64 + if err := db.Table(table).Count(&n).Error; err != nil { + t.Fatalf("count %s: %v", table, err) + } + if n != want { + t.Errorf("%s has %d row(s), want %d", table, n, want) + } +} + +// A retried migration does not just risk inserting a second copy of a row +// it already wrote (that gap is closed above) - the reuse path itself has +// to leave the row in the same state a fresh insert would have. Before +// this defect was fixed, the reuse branch (seed.go's "case err == nil") +// stopped at reusing the row's id and skipped everything a fresh insert +// does afterwards: the sys_menu_api_rule binding gorm's association save +// writes as part of Create, and the paths UPDATE that follows Create as a +// separate statement. A row a prior attempt inserted but did not finish - +// exactly the shape design doc §1.5 says a non-transactional retry can +// leave behind - would be "found" and then left broken forever, with the +// migration reporting success. +// +// existingHalfWrittenMenu inserts a sys_menu row the way seedMenuTree's own +// tx.Create leaves one when interrupted immediately afterwards: the row +// exists with its natural key, but paths was never computed and no +// sys_menu_api_rule binding was ever written for it - Create's association +// save and the paths UPDATE are each a separate statement from the row +// insert itself. +func existingHalfWrittenMenu(t *testing.T, db *gorm.DB, appCode, seedCode string, parentID int) models.SysMenu { + t.Helper() + code := seedCode + row := models.SysMenu{ + MenuName: menuName(appCode, seedCode), + AppCode: appCode, + SeedCode: &code, + ParentId: parentID, + Visible: "0", + IsFrame: "1", + // Paths deliberately left "" - never computed, the same as a row + // whose Create succeeded but whose follow-up paths UPDATE never ran. + } + if err := db.Create(&row).Error; err != nil { + t.Fatalf("seed half-written menu %q: %v", seedCode, err) + } + return row +} + +func bindingCount(t *testing.T, db *gorm.DB, menuID, apiID int) int64 { + t.Helper() + var n int64 + if err := db.Table("sys_menu_api_rule"). + Where("sys_menu_menu_id = ? AND sys_api_id = ?", menuID, apiID).Count(&n).Error; err != nil { + t.Fatalf("count sys_menu_api_rule: %v", err) + } + return n +} + +// TestSeedMenusRepairsAnIncompleteExistingRow is the reproduction case: +// both paths and the api binding are missing on the row seedMenuTree finds +// through its idempotency check, the shape a real interrupted retry leaves +// behind. Run against the unfixed reuse branch, this must fail - that is +// what proves the defect is real rather than a three-way guess. +func TestSeedMenusRepairsAnIncompleteExistingRow(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + apis := []seed.ApiSpec{{Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET"}} + apiRows, err := seedApis(db, "order", apis) + if err != nil { + t.Fatalf("seedApis: %v", err) + } + + dir := existingHalfWrittenMenu(t, db, "order", "dir", 0) + list := existingHalfWrittenMenu(t, db, "order", "list", dir.MenuId) + + menus := []seed.MenuSpec{ + {Code: "dir", Kind: contractmodels.Directory, Title: "Order", Sort: 10}, + {Code: "list", Parent: "dir", Kind: contractmodels.Menu, Title: "Orders", Sort: 1, ApiCodes: []string{"list"}}, + } + + if err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", menus, apis) + }); err != nil { + t.Fatalf("SeedMenus: %v", err) + } + + wantDirPaths := "/0/" + strconv.Itoa(dir.MenuId) + wantListPaths := wantDirPaths + "/" + strconv.Itoa(list.MenuId) + + var gotDir, gotList models.SysMenu + if err := db.First(&gotDir, dir.MenuId).Error; err != nil { + t.Fatalf("read dir: %v", err) + } + if err := db.First(&gotList, list.MenuId).Error; err != nil { + t.Fatalf("read list: %v", err) + } + if gotDir.Paths != wantDirPaths { + t.Errorf("dir.Paths = %q, want %q - a retried install left a root menu with no materialized path", gotDir.Paths, wantDirPaths) + } + if gotList.Paths != wantListPaths { + t.Errorf("list.Paths = %q, want %q - a retried install left the seeded subtree with a broken materialized path", gotList.Paths, wantListPaths) + } + if n := bindingCount(t, db, list.MenuId, apiRows["list"].Id); n != 1 { + t.Errorf("sys_menu_api_rule binding count for list = %d, want 1 - a retried install left the menu with its api granted to no one", n) + } +} + +// soloMenuSpec is a single, parent-less menu with one api binding - the +// smallest shape that can exhibit "paths wrong" and "binding missing" +// independently of each other, used by the three tests below to isolate +// one repair path at a time from TestSeedMenusRepairsAnIncompleteExistingRow's +// combined (both broken) case. +func soloMenuSpec() []seed.MenuSpec { + return []seed.MenuSpec{{Code: "solo", Kind: contractmodels.Menu, Title: "Solo", Sort: 1, ApiCodes: []string{"list"}}} +} + +// Only the binding is missing; paths is already correct. The repair must +// add the binding and must not touch the already-correct paths value. +func TestSeedMenusRepairsOnlyAMissingBinding(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + apis := []seed.ApiSpec{{Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET"}} + apiRows, err := seedApis(db, "order", apis) + if err != nil { + t.Fatalf("seedApis: %v", err) + } + + solo := existingHalfWrittenMenu(t, db, "order", "solo", 0) + wantPaths := "/0/" + strconv.Itoa(solo.MenuId) + if err := db.Model(&models.SysMenu{}).Where("menu_id = ?", solo.MenuId). + Update("paths", wantPaths).Error; err != nil { + t.Fatalf("set paths: %v", err) + } + // The binding is deliberately left unwritten. + + if err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", soloMenuSpec(), apis) + }); err != nil { + t.Fatalf("SeedMenus: %v", err) + } + + var got models.SysMenu + if err := db.First(&got, solo.MenuId).Error; err != nil { + t.Fatalf("read solo: %v", err) + } + if got.Paths != wantPaths { + t.Errorf("paths changed from %q to %q; repairing a missing binding must not touch an already-correct path", wantPaths, got.Paths) + } + if n := bindingCount(t, db, solo.MenuId, apiRows["list"].Id); n != 1 { + t.Errorf("binding count = %d, want 1", n) + } +} + +// Only paths is missing; the binding already exists (as if Create's own +// association write had succeeded but the paths UPDATE that follows it +// never ran). The repair must fix paths and must not duplicate the +// already-correct binding. +func TestSeedMenusRepairsOnlyMissingPaths(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + apis := []seed.ApiSpec{{Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET"}} + apiRows, err := seedApis(db, "order", apis) + if err != nil { + t.Fatalf("seedApis: %v", err) + } + + solo := existingHalfWrittenMenu(t, db, "order", "solo", 0) + if err := db.Exec( + "INSERT INTO sys_menu_api_rule (sys_menu_menu_id, sys_api_id) VALUES (?, ?)", + solo.MenuId, apiRows["list"].Id, + ).Error; err != nil { + t.Fatalf("seed binding: %v", err) + } + // solo.Paths is deliberately left "" by existingHalfWrittenMenu. + + if err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", soloMenuSpec(), apis) + }); err != nil { + t.Fatalf("SeedMenus: %v", err) + } + + wantPaths := "/0/" + strconv.Itoa(solo.MenuId) + var got models.SysMenu + if err := db.First(&got, solo.MenuId).Error; err != nil { + t.Fatalf("read solo: %v", err) + } + if got.Paths != wantPaths { + t.Errorf("paths = %q, want %q", got.Paths, wantPaths) + } + if n := bindingCount(t, db, solo.MenuId, apiRows["list"].Id); n != 1 { + t.Errorf("binding count = %d, want 1 - repairing paths must not duplicate an already-correct binding", n) + } +} + +// capturingLogger records every SQL statement gorm actually executes, so a +// test can assert that a fully-consistent retry performs no write at all - +// not just that its net effect happens to be zero rows changed. Mirrors +// common/actions/crud_shim_test.go's logger of the same name and shape; +// duplicated locally rather than exported and shared, matching how small +// gorm-facing test doubles are kept next to the test that needs them +// elsewhere in this repository. +type capturingLogger struct { + gormlogger.Interface + mu sync.Mutex + stmts []string +} + +func (l *capturingLogger) Trace(ctx context.Context, begin time.Time, fc func() (string, int64), err error) { + sql, _ := fc() + l.mu.Lock() + l.stmts = append(l.stmts, sql) + l.mu.Unlock() +} + +func (l *capturingLogger) all() string { + l.mu.Lock() + defer l.mu.Unlock() + return strings.Join(l.stmts, "\n") +} + +// Both paths and the binding are already correct - the ordinary shape of +// every retry after the first one succeeds in full. Repairing an +// already-consistent row must not touch it: paths is read-before-write and +// so must not be UPDATEd at all (asserted directly, by statement, since the +// code gates that call behind a value comparison); the binding's own +// insert is guarded by WHERE NOT EXISTS the same way grantToAdminRole's +// already are, so its row count staying put is the meaningful claim - the +// guarded statement itself may still be sent, the same way it already is +// for sys_role_menu/casbin_rule. +func TestSeedMenusFullyConsistentRowCausesNoPathsUpdate(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + apis := []seed.ApiSpec{{Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET"}} + menus := soloMenuSpec() + + if err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", menus, apis) + }); err != nil { + t.Fatalf("SeedMenus (first): %v", err) + } + + var apiRows []models.SysApi + db.Where("app_code = ?", "order").Find(&apiRows) + var soloRow models.SysMenu + if err := db.Where("app_code = ? AND seed_code = ?", "order", "solo").First(&soloRow).Error; err != nil { + t.Fatalf("read solo after first call: %v", err) + } + if soloRow.Paths == "" { + t.Fatalf("solo.Paths is empty after the first call; the fixture itself is broken, not what this test means to check") + } + wantBindings := bindingCount(t, db, soloRow.MenuId, apiRows[0].Id) + if wantBindings != 1 { + t.Fatalf("binding count after the first call = %d, want 1; the fixture itself is broken", wantBindings) + } + + capturing := &capturingLogger{Interface: gormlogger.Default.LogMode(gormlogger.Info)} + captured := db.Session(&gorm.Session{Logger: capturing}) + + if err := captured.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", menus, apis) + }); err != nil { + t.Fatalf("SeedMenus (retry): %v", err) + } + + all := strings.ToUpper(capturing.all()) + if strings.Contains(all, "UPDATE") && strings.Contains(all, "SYS_MENU") && strings.Contains(all, "PATHS") { + t.Errorf("a fully consistent retry executed a paths UPDATE against sys_menu:\n%s", capturing.all()) + } + if got := bindingCount(t, db, soloRow.MenuId, apiRows[0].Id); got != 1 { + t.Errorf("binding count after the retry = %d, want 1 (unchanged)", got) + } +} + +// An administrator can bind a menu to an additional api by hand through +// the menu management UI - a sys_menu_api_rule row for an api never in +// s.ApiCodes at all. A retried SeedMenus call must not touch it: deleting +// every binding for the menu and reinserting only what s.ApiCodes lists +// would wipe it out silently, the same shape as sys_role_menu/casbin_rule +// getting zeroed by SysRole.Update's FullSaveAssociations save - just with +// this code as the actor instead of the victim this time. +func TestSeedMenusPreservesAHandAddedBinding(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + apis := []seed.ApiSpec{{Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET"}} + menus := soloMenuSpec() + + if err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", menus, apis) + }); err != nil { + t.Fatalf("SeedMenus (first): %v", err) + } + + var soloRow models.SysMenu + if err := db.Where("app_code = ? AND seed_code = ?", "order", "solo").First(&soloRow).Error; err != nil { + t.Fatalf("read solo: %v", err) + } + + // An api this call's ApiSpec list never mentions - standing in for one + // belonging to some other feature entirely, bound to this menu by an + // administrator, not by any SeedMenus call. + handAdded := models.SysApi{Path: "/api/v1/order/export", Action: "GET", Type: "SYS", AppCode: "order"} + if err := db.Create(&handAdded).Error; err != nil { + t.Fatalf("seed the hand-added api: %v", err) + } + if err := db.Exec( + "INSERT INTO sys_menu_api_rule (sys_menu_menu_id, sys_api_id) VALUES (?, ?)", + soloRow.MenuId, handAdded.Id, + ).Error; err != nil { + t.Fatalf("seed the hand-added binding: %v", err) + } + + // A retry with the exact same specs - solo's ApiCodes still names only + // "list". + if err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", menus, apis) + }); err != nil { + t.Fatalf("SeedMenus (retry): %v", err) + } + + if n := bindingCount(t, db, soloRow.MenuId, handAdded.Id); n != 1 { + t.Errorf("hand-added binding count = %d, want 1 - a retry silently deleted a binding it does not own", n) + } + + var apiRows []models.SysApi + db.Where("app_code = ? AND path = ?", "order", "/api/v1/order").Find(&apiRows) + if len(apiRows) != 1 { + t.Fatalf("seeded api not found as expected: %+v", apiRows) + } + if n := bindingCount(t, db, soloRow.MenuId, apiRows[0].Id); n != 1 { + t.Errorf("the seed's own binding count = %d, want 1 - it must survive the retry too", n) + } +} diff --git a/cmd/migrate/migration/version/1786700007000_app_registry_tables.go b/cmd/migrate/migration/version/1786700007000_app_registry_tables.go new file mode 100644 index 00000000..7faf10be --- /dev/null +++ b/cmd/migrate/migration/version/1786700007000_app_registry_tables.go @@ -0,0 +1,42 @@ +package version + +import ( + "runtime" + + "gorm.io/gorm" + + adminmodels "go-admin/app/admin/models" + "go-admin/cmd/migrate/migration" + common "go-admin/common/models" +) + +// Create sys_app (PRD 008 F2) and sys_app_casbin_grant (F4/F6's casbin +// attribution ledger - see the design doc's (docs-prd/008-应用清单与安装器/ +// 数据库变更.md) §2.2/§3 for why casbin_rule itself is not touched: +// gorm-adapter's SavePolicyCtx truncates and reloads that table from its +// in-memory model, and any column this migration added to it would be +// silently zeroed the first time anything calls SavePolicy. +// +// Ordered after 1786700003000 (the soft-delete conversion), so importing +// cmd/migrate/migration/models is banned here - see +// schema_coverage_test.go's TestPostConversionMigrationsAvoidFrozenSeedModels. +// Both new tables are AutoMigrate'd from their runtime model shape under +// app/admin/models directly, which is also why neither one is added to +// 1786700003000's frozen softDeleteTables list: neither embeds +// common.ModelTime in the first place (see design doc §1.1). +func init() { + _, fileName, _, _ := runtime.Caller(0) + migration.Migrate.SetVersion(migration.GetFilename(fileName), _1786700007000AppRegistryTables) +} + +func _1786700007000AppRegistryTables(db *gorm.DB, version string) error { + return db.Transaction(func(tx *gorm.DB) error { + if err := tx.Migrator().AutoMigrate( + new(adminmodels.SysApp), + new(adminmodels.SysAppCasbinGrant), + ); err != nil { + return err + } + return tx.Create(&common.Migration{Version: version}).Error + }) +} diff --git a/cmd/migrate/migration/version/1786700007000_app_registry_tables_postgres_test.go b/cmd/migrate/migration/version/1786700007000_app_registry_tables_postgres_test.go new file mode 100644 index 00000000..7eaf02a5 --- /dev/null +++ b/cmd/migrate/migration/version/1786700007000_app_registry_tables_postgres_test.go @@ -0,0 +1,62 @@ +package version + +import ( + "testing" + + common "go-admin/common/models" + + adminmodels "go-admin/app/admin/models" +) + +// postgresDB is defined in 1786700003000_soft_delete_marker_postgres_test.go +// and shared across this package's PostgreSQL-only tests. +// +// This migration is plain AutoMigrate on two brand-new tables, unlike +// 1786700003000's DROP INDEX (go-admin#919's actual defect), so there is no +// dialect-specific SQL here for AutoMigrate itself to get wrong on +// PostgreSQL specifically. What is worth a real PostgreSQL run is +// 1786700008000's CONCAT()-based duplicate check next door - PostgreSQL has +// had CONCAT() since 9.1, but it was never verified against a real server +// until this file, only inferred from documentation - and the same +// AutoMigrate call this test makes, so a schema/character-set mistake +// AutoMigrate might make silently on a dialect nobody ran it against here +// has somewhere to surface. +func TestAppRegistryTablesAreCreatedOnPostgres(t *testing.T) { + db := postgresDB(t) + const version = "1786700007000-pg" + cleanup := func() { + db.Migrator().DropTable(&adminmodels.SysAppCasbinGrant{}, &adminmodels.SysApp{}) + // Only this test's own row, not the whole shared sys_migration + // table: postgresDB points at a real, persistent database (unlike + // the SQLite tests' fresh in-memory one per run), so a version left + // behind by a previous run of this same binary collides with the + // wrapper's own INSERT the next time this test runs. + db.Exec("DELETE FROM sys_migration WHERE version = ?", version) + } + t.Cleanup(cleanup) + cleanup() + if err := db.AutoMigrate(&common.Migration{}); err != nil { + t.Fatalf("automigrate sys_migration: %v", err) + } + + if err := _1786700007000AppRegistryTables(db, version); err != nil { + t.Fatalf("migrate: %v", err) + } + + if err := db.Create(&adminmodels.SysApp{AppCode: "order", Name: "Order", Version: "v1"}).Error; err != nil { + t.Fatalf("insert sys_app: %v", err) + } + if err := db.Create(&adminmodels.SysApp{AppCode: "order", Name: "dup", Version: "v1"}).Error; err == nil { + t.Fatal("a second sys_app row with the same app_code was accepted on PostgreSQL") + } + + grant := adminmodels.SysAppCasbinGrant{AppCode: "order", Ptype: "p", V0: "admin", V1: "/api/v1/order", V2: "GET"} + if err := db.Create(&grant).Error; err != nil { + t.Fatalf("insert sys_app_casbin_grant: %v", err) + } + dup := grant + dup.Id = 0 + if err := db.Create(&dup).Error; err == nil { + t.Fatal("a second sys_app_casbin_grant row with the same natural key was accepted on PostgreSQL") + } +} diff --git a/cmd/migrate/migration/version/1786700007000_app_registry_tables_test.go b/cmd/migrate/migration/version/1786700007000_app_registry_tables_test.go new file mode 100644 index 00000000..05fec59d --- /dev/null +++ b/cmd/migrate/migration/version/1786700007000_app_registry_tables_test.go @@ -0,0 +1,130 @@ +package version + +import ( + "testing" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" + + adminmodels "go-admin/app/admin/models" + common "go-admin/common/models" +) + +func openAppRegistryDB(t *testing.T) *gorm.DB { + t.Helper() + + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + if err != nil { + t.Fatalf("open: %v", err) + } + if err := db.AutoMigrate(&common.Migration{}); err != nil { + t.Fatalf("automigrate sys_migration: %v", err) + } + return db +} + +// The migration has to build both tables and record itself as applied - +// F2/F6's acceptance case is a row landing in either one, and neither is +// possible if the table it belongs to was never created. +func TestAppRegistryTablesAreCreated(t *testing.T) { + db := openAppRegistryDB(t) + + if err := _1786700007000AppRegistryTables(db, "1786700007000"); err != nil { + t.Fatalf("migrate: %v", err) + } + + if !db.Migrator().HasTable(&adminmodels.SysApp{}) { + t.Fatal("sys_app was not created") + } + if !db.Migrator().HasTable(&adminmodels.SysAppCasbinGrant{}) { + t.Fatal("sys_app_casbin_grant was not created") + } + + // A row that exercises every column, not just HasTable/HasColumn - + // AutoMigrate can build a column with the wrong type and still report + // that it exists. + if err := db.Create(&adminmodels.SysApp{ + AppCode: "order", Name: "Order", Version: "v1", Description: "d", Author: "a", + Requires: "payment", Pricing: "free", License: "MIT", Status: 1, + }).Error; err != nil { + t.Fatalf("insert sys_app: %v", err) + } + if err := db.Create(&adminmodels.SysAppCasbinGrant{ + AppCode: "order", Ptype: "p", V0: "admin", V1: "/api/v1/order", V2: "GET", + }).Error; err != nil { + t.Fatalf("insert sys_app_casbin_grant: %v", err) + } + + var applied common.Migration + if err := db.Where("version = ?", "1786700007000").First(&applied).Error; err != nil { + t.Fatalf("sys_migration was not recorded: %v", err) + } +} + +// Running it twice must be safe: DDL does not roll back on MySQL, so an +// operator whose first attempt failed partway through has nothing to do but +// run it again. This calls AutoMigrate directly rather than the wrapper, +// which also inserts a sys_migration row that a second call would collide +// on - a collision Migrate.run() itself prevents by never calling a +// function twice for the same recorded version, so it is not this +// migration's job to tolerate. +func TestAppRegistryTablesAutoMigrateIsRepeatable(t *testing.T) { + db := openAppRegistryDB(t) + + for i := 0; i < 3; i++ { + if err := db.Migrator().AutoMigrate( + new(adminmodels.SysApp), + new(adminmodels.SysAppCasbinGrant), + ); err != nil { + t.Fatalf("automigrate %d: %v", i, err) + } + } +} + +// sys_app.app_code is the unique key G2 ("is app X installed") answers with +// - a second row for the same app code must be rejected, not tolerated. +func TestSysAppAppCodeIsUnique(t *testing.T) { + db := openAppRegistryDB(t) + if err := _1786700007000AppRegistryTables(db, "1786700007000"); err != nil { + t.Fatalf("migrate: %v", err) + } + + if err := db.Create(&adminmodels.SysApp{AppCode: "order", Name: "Order", Version: "v1"}).Error; err != nil { + t.Fatalf("first insert: %v", err) + } + if err := db.Create(&adminmodels.SysApp{AppCode: "order", Name: "Order dup", Version: "v1"}).Error; err == nil { + t.Fatal("a second sys_app row with the same app_code was accepted") + } +} + +// sys_app_casbin_grant's unique index mirrors casbin_rule's own natural key +// (ptype,v0..v5) exactly - see design doc §3. A duplicate grant for the +// same rule must be rejected the same way gorm-adapter's own unique index +// on casbin_rule would reject it. +func TestSysAppCasbinGrantNaturalKeyIsUnique(t *testing.T) { + db := openAppRegistryDB(t) + if err := _1786700007000AppRegistryTables(db, "1786700007000"); err != nil { + t.Fatalf("migrate: %v", err) + } + + grant := adminmodels.SysAppCasbinGrant{AppCode: "order", Ptype: "p", V0: "admin", V1: "/api/v1/order", V2: "GET"} + if err := db.Create(&grant).Error; err != nil { + t.Fatalf("first insert: %v", err) + } + dup := grant + dup.Id = 0 + if err := db.Create(&dup).Error; err == nil { + t.Fatal("a second sys_app_casbin_grant row with the same natural key was accepted") + } + + // A grant for a different app, but the identical casbin natural key, is + // exactly the collision two applications granting the same api/role + // pair would produce - the natural key has to be the one thing that + // rejects it, app_code is descriptive only and not part of the index. + other := grant + other.Id = 0 + other.AppCode = "another-app" + if err := db.Create(&other).Error; err == nil { + t.Fatal("a duplicate natural key under a different app_code was accepted") + } +} diff --git a/cmd/migrate/migration/version/1786700008000_seed_natural_keys.go b/cmd/migrate/migration/version/1786700008000_seed_natural_keys.go new file mode 100644 index 00000000..b07dbe14 --- /dev/null +++ b/cmd/migrate/migration/version/1786700008000_seed_natural_keys.go @@ -0,0 +1,145 @@ +package version + +import ( + "fmt" + "runtime" + + "gorm.io/gorm" + + adminmodels "go-admin/app/admin/models" + "go-admin/cmd/migrate/migration" + common "go-admin/common/models" +) + +// Give seed.SeedMenus's two write paths (seedApis, seedMenuTree in +// app/admin/service/seed.go) a real natural key to check before inserting, +// so a retried, partially-failed migration (see the design doc +// docs-prd/008-应用清单与安装器/数据库变更.md §1.5/§1.6) does not insert the +// same row twice. This has already happened in production once (duplicate +// sys_menu/casbin_rule rows on the demo site), not a theoretical risk. +func init() { + _, fileName, _, _ := runtime.Caller(0) + migration.Migrate.SetVersion(migration.GetFilename(fileName), _1786700008000SeedNaturalKeys) +} + +func _1786700008000SeedNaturalKeys(db *gorm.DB, version string) error { + if err := seedNaturalKeys(db); err != nil { + return err + } + return db.Create(&common.Migration{Version: version}).Error +} + +// seedNaturalKeys is split out from the wrapper above so tests can call it +// against a database that only has sys_menu/sys_api, without also standing +// up sys_migration - and so it can be called more than once in the same +// test to prove the re-run tolerance the doc comment above promises: DDL +// does not roll back on MySQL, so an operator whose first attempt failed +// partway through has nothing to do but run the whole migration again. +func seedNaturalKeys(db *gorm.DB) error { + m := db.Migrator() + + // sys_menu.seed_code is a brand-new column: every existing row becomes + // NULL, and NULL never collides in the unique index built below, so + // this needs no pre-check. + if !m.HasColumn(&adminmodels.SysMenu{}, "SeedCode") { + if err := m.AddColumn(&adminmodels.SysMenu{}, "SeedCode"); err != nil { + return err + } + } + if !m.HasIndex(&adminmodels.SysMenu{}, "uk_sys_menu_app_seed_code_del") { + if err := db.Exec( + "CREATE UNIQUE INDEX uk_sys_menu_app_seed_code_del ON sys_menu (app_code, seed_code, deleted_at)", + ).Error; err != nil { + return err + } + } + + // sys_api reuses existing, already-populated columns, which the demo + // site has already proven can hold duplicates. Refuse rather than let + // CREATE UNIQUE INDEX fail on an operator with no idea which rows to + // reconcile - same shape as 1786700003000_soft_delete_marker.go's + // refuseOnDuplicates. + if err := refuseOnDuplicateApis(db); err != nil { + return err + } + if !m.HasIndex(&adminmodels.SysApi{}, "uk_sys_api_app_path_action_del") { + if err := db.Exec( + "CREATE UNIQUE INDEX uk_sys_api_app_path_action_del ON sys_api (app_code, path, action, deleted_at)", + ).Error; err != nil { + return err + } + } + + return nil +} + +// refuseOnDuplicateApis reports the (app_code, path, action) values that +// would make the unique index impossible, rather than the index failing to +// build and saying only that it did. Only live rows count: a soft-deleted +// duplicate does not block the index it will never occupy a slot in. +// +// sys_api.path/action (app/admin/models/sys_api.go) carry no NOT NULL +// constraint, and that stays true here on purpose: tightening it is an +// independent, backward-incompatible change of its own - existing NULL +// rows in a real database would need reconciling or backfilling before +// ALTER TABLE ... NOT NULL could even run, which is a decision for +// whoever owns that data, not something this migration should force as a +// side effect of adding an unrelated index. So this function has to +// tolerate NULL path/action rather than assume they cannot occur - see the +// query below for how it does that without either crashing on them +// (MySQL's CONCAT) or wrongly flagging them (GROUP BY's NULL-equals-NULL). +// +// The two are independent bugs that happened to share one root cause, and +// SQLite's own test suite for this file would have caught neither on its +// own: MySQL's CONCAT() returns NULL if any argument is NULL, which turned +// a duplicate check against a NULL-holding library into "converting NULL +// to string is unsupported" instead of a report - but SQLite's (and +// PostgreSQL's) CONCAT() treats a NULL argument as an empty string +// instead, so the exact same query never errors there no matter how it is +// called. A suite that only ever ran on SQLite would report success for +// both defects; only a real MySQL server surfaces the first one at all - +// this migration's PostgreSQL-only sibling test file +// (1786700008000_seed_natural_keys_postgres_test.go) rules out one more +// dialect, but MySQL specifically has to be checked by hand, since this +// repository's test suite has no MySQL service to run against in CI. +func refuseOnDuplicateApis(db *gorm.DB) error { + var dupes []string + if err := db.Raw( + // This has to agree with what the unique index it guards actually + // enforces, not just with what looks like a duplicate at a glance. + // Two different SQL rules collide on a NULL: GROUP BY treats two + // NULLs as equal, so a naive query flags every pair of rows that + // share a NULL path or action - even a pair with only one of the + // two NULL, since GROUP BY's equality still holds on whichever + // column both rows leave NULL - but a UNIQUE INDEX treats every + // NULL as distinct from every other value, including another + // NULL, so the index itself accepts every one of those pairs + // without complaint. Excluding any row missing either column from + // consideration entirely is what makes the two agree: a row + // missing path, or missing action, or missing both, can never + // violate the index no matter how many other rows are also + // missing the same one, so none of them belong in this count. + // + // No COALESCE: with both columns excluded whenever either is + // NULL, CONCAT here never receives a NULL argument for path or + // action - app_code cannot be NULL at all (see its own NOT NULL + // tag) - so there is nothing left for COALESCE to guard against, + // and leaving it out is deliberate rather than an oversight. A + // future regression that removed the two IS NOT NULL conditions + // above would fail loudly on MySQL (the same Scan error this + // query used to produce) instead of quietly reporting a made-up + // "duplicate" whose path and action both print as empty - the + // failure this function exists to prevent in the first place. + `SELECT CONCAT(app_code, '|', path, '|', action) FROM sys_api + WHERE deleted_at = 0 AND path IS NOT NULL AND action IS NOT NULL + GROUP BY app_code, path, action HAVING COUNT(*) > 1`, + ).Scan(&dupes).Error; err != nil { + return fmt.Errorf("checking sys_api for duplicates: %w", err) + } + if len(dupes) > 0 { + return fmt.Errorf( + "sys_api already holds duplicate (app_code,path,action) %v; reconcile them before this migration can add its unique index", + dupes) + } + return nil +} diff --git a/cmd/migrate/migration/version/1786700008000_seed_natural_keys_postgres_test.go b/cmd/migrate/migration/version/1786700008000_seed_natural_keys_postgres_test.go new file mode 100644 index 00000000..3b69ec18 --- /dev/null +++ b/cmd/migrate/migration/version/1786700008000_seed_natural_keys_postgres_test.go @@ -0,0 +1,105 @@ +package version + +import ( + "testing" +) + +// postgresDB is defined in 1786700003000_soft_delete_marker_postgres_test.go. +// +// This file exists because refuseOnDuplicateApis's duplicate check is +// spelled with CONCAT(), a function this migration's design assumed +// PostgreSQL has carried since 9.1 but that nothing had run against a real +// PostgreSQL server before this test - only against the pure-Go SQLite +// driver, which happens to bundle a SQLite new enough to have grown its own +// CONCAT() only recently. A dialect where that assumption were wrong would +// otherwise only be discovered the first time an operator's install hit a +// genuine sys_api duplicate on PostgreSQL in production. +func TestSeedNaturalKeysRefusesDuplicateApisOnPostgres(t *testing.T) { + db := postgresDB(t) + t.Cleanup(func() { db.Migrator().DropTable(&oldSeedMenu{}, &oldSeedApi{}) }) + db.Migrator().DropTable(&oldSeedMenu{}, &oldSeedApi{}) + if err := db.AutoMigrate(&oldSeedMenu{}, &oldSeedApi{}); err != nil { + t.Fatalf("automigrate: %v", err) + } + + for i := 0; i < 2; i++ { + if err := db.Create(&oldSeedApi{AppCode: "order", Path: "/api/v1/order", Action: "GET"}).Error; err != nil { + t.Fatalf("seed duplicate %d: %v", i, err) + } + } + + err := seedNaturalKeys(db) + if err == nil { + t.Fatal("PostgreSQL accepted sys_api rows that already hold a duplicate (app_code, path, action)") + } + if !contains(err.Error(), "order") || !contains(err.Error(), "/api/v1/order") { + t.Errorf("the error does not name the offending row: %v", err) + } + if db.Migrator().HasIndex(&oldSeedApi{}, "uk_sys_api_app_path_action_del") { + t.Error("the unique index was built despite the migration refusing") + } +} + +// GROUP BY treats two NULLs as equal for grouping; a UNIQUE INDEX treats +// every NULL as distinct from every other value, including another NULL. +// Both are standard SQL, not a SQLite/PostgreSQL/MySQL difference - this +// file exists to confirm that on a real server rather than assume it, the +// same reason TestSeedNaturalKeysRefusesDuplicateApisOnPostgres above +// exists for CONCAT(). See TestSeedNaturalKeysDoesNotFlagWhatTheIndexWouldAccept +// in the SQLite-backed test file for the full account of why this matters: +// a naive duplicate check that does not exclude NULL path/action refuses +// an install the unique index itself would accept without complaint. +func TestSeedNaturalKeysDoesNotFlagWhatTheIndexWouldAcceptOnPostgres(t *testing.T) { + db := postgresDB(t) + t.Cleanup(func() { db.Migrator().DropTable(&oldSeedMenu{}, &oldSeedApi{}) }) + db.Migrator().DropTable(&oldSeedMenu{}, &oldSeedApi{}) + if err := db.AutoMigrate(&oldSeedMenu{}, &oldSeedApi{}); err != nil { + t.Fatalf("automigrate: %v", err) + } + + for i := 0; i < 2; i++ { + if err := db.Exec( + "INSERT INTO sys_api (app_code, path, action, deleted_at) VALUES ('order', NULL, NULL, 0)", + ).Error; err != nil { + t.Fatalf("seed NULL row %d: %v", i, err) + } + } + + if err := seedNaturalKeys(db); err != nil { + t.Fatalf("seedNaturalKeys refused a library the unique index itself accepts on PostgreSQL: %v", err) + } + if !db.Migrator().HasIndex(&oldSeedApi{}, "uk_sys_api_app_path_action_del") { + t.Error("the unique index was not built on PostgreSQL even though seedNaturalKeys reported success") + } +} + +// The success path, on the same server: both columns and both unique +// indexes have to actually build on PostgreSQL, not merely fail to error +// out on SQLite. Mirrors TestSeedNaturalKeysIsRepeatable's SQLite coverage. +func TestSeedNaturalKeysBuildsOnPostgres(t *testing.T) { + db := postgresDB(t) + t.Cleanup(func() { db.Migrator().DropTable(&oldSeedMenu{}, &oldSeedApi{}) }) + db.Migrator().DropTable(&oldSeedMenu{}, &oldSeedApi{}) + if err := db.AutoMigrate(&oldSeedMenu{}, &oldSeedApi{}); err != nil { + t.Fatalf("automigrate: %v", err) + } + if err := db.Create(&oldSeedApi{AppCode: "order", Path: "/api/v1/order", Action: "GET"}).Error; err != nil { + t.Fatalf("seed: %v", err) + } + + for i := 0; i < 2; i++ { + if err := seedNaturalKeys(db); err != nil { + t.Fatalf("migrate %d: %v", i, err) + } + } + + if !db.Migrator().HasColumn(&oldSeedMenu{}, "seed_code") { + t.Error("sys_menu.seed_code was not added on PostgreSQL") + } + if !db.Migrator().HasIndex(&oldSeedMenu{}, "uk_sys_menu_app_seed_code_del") { + t.Error("the sys_menu unique index was not built on PostgreSQL") + } + if !db.Migrator().HasIndex(&oldSeedApi{}, "uk_sys_api_app_path_action_del") { + t.Error("the sys_api unique index was not built on PostgreSQL") + } +} diff --git a/cmd/migrate/migration/version/1786700008000_seed_natural_keys_test.go b/cmd/migrate/migration/version/1786700008000_seed_natural_keys_test.go new file mode 100644 index 00000000..ea589c0e --- /dev/null +++ b/cmd/migrate/migration/version/1786700008000_seed_natural_keys_test.go @@ -0,0 +1,305 @@ +package version + +import ( + "testing" + "time" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" + + adminmodels "go-admin/app/admin/models" + common "go-admin/common/models" +) + +// oldSeedMenu/oldSeedApi are the shape of sys_menu/sys_api immediately +// before this migration: post-1786700003000 (deleted_at is the NOT NULL +// millisecond marker) and post-1786700006000 (app_code exists), but before +// seed_code or either unique index. They stand in for the real runtime +// models, which by the time this file is read already carry the columns +// this migration adds - the same relationship oldUser bears to sys_user in +// 1786700003000_soft_delete_marker_test.go. +type oldSeedMenu struct { + MenuId int `gorm:"column:menu_id;primaryKey;autoIncrement"` + AppCode string `gorm:"column:app_code;type:varchar(64);not null;default:''"` + DeletedAt int64 `gorm:"column:deleted_at;not null;default:0"` +} + +func (oldSeedMenu) TableName() string { return "sys_menu" } + +type oldSeedApi struct { + Id int `gorm:"column:id;primaryKey;autoIncrement"` + AppCode string `gorm:"column:app_code;type:varchar(64);not null;default:''"` + Path string `gorm:"column:path;type:varchar(128)"` + Action string `gorm:"column:action;type:varchar(16)"` + DeletedAt int64 `gorm:"column:deleted_at;not null;default:0"` +} + +func (oldSeedApi) TableName() string { return "sys_api" } + +func openSeedNaturalKeysDB(t *testing.T) *gorm.DB { + t.Helper() + + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + if err != nil { + t.Fatalf("open: %v", err) + } + if err := db.AutoMigrate(&oldSeedMenu{}, &oldSeedApi{}, &common.Migration{}); err != nil { + t.Fatalf("automigrate: %v", err) + } + return db +} + +// The host's own hand-placed menus, and every app-seeded row written +// before this column existed, have no seed_code at all - an unbounded +// number of those must coexist under the same app_code without tripping +// the new unique index (design doc §1.6: "NULL never treated as equal to +// NULL"). +func TestSeedNaturalKeysToleratesManyPreExistingMenusWithNoSeedCode(t *testing.T) { + db := openSeedNaturalKeysDB(t) + for i := 0; i < 3; i++ { + if err := db.Create(&oldSeedMenu{AppCode: ""}).Error; err != nil { + t.Fatalf("seed pre-existing menu %d: %v", i, err) + } + } + + if err := seedNaturalKeys(db); err != nil { + t.Fatalf("migrate: %v", err) + } + + if !db.Migrator().HasColumn(&adminmodels.SysMenu{}, "SeedCode") { + t.Fatal("sys_menu.seed_code was not added") + } +} + +// The point of adding seed_code at all: a second row with the same +// (app_code, seed_code) while both are live is what seedMenuTree's +// idempotency check depends on the database to reject if the Go-level +// check above it is ever bypassed or raced. +func TestSeedNaturalKeysMenuUniqueIndexBindsLiveRowsOnly(t *testing.T) { + db := openSeedNaturalKeysDB(t) + if err := seedNaturalKeys(db); err != nil { + t.Fatalf("migrate: %v", err) + } + + if err := db.Exec( + "INSERT INTO sys_menu (app_code, seed_code, deleted_at) VALUES ('order', 'dir', 0)", + ).Error; err != nil { + t.Fatalf("seed: %v", err) + } + + t.Run("a second live row with the same natural key is rejected", func(t *testing.T) { + err := db.Exec( + "INSERT INTO sys_menu (app_code, seed_code, deleted_at) VALUES ('order', 'dir', 0)", + ).Error + if err == nil { + t.Fatal("a duplicate (app_code, seed_code) was accepted while both rows were live") + } + }) + + t.Run("the key is free again once the row is soft-deleted", func(t *testing.T) { + if err := db.Exec("UPDATE sys_menu SET deleted_at = ? WHERE seed_code = 'dir'", time.Now().UnixMilli()).Error; err != nil { + t.Fatalf("soft-delete: %v", err) + } + if err := db.Exec( + "INSERT INTO sys_menu (app_code, seed_code, deleted_at) VALUES ('order', 'dir', 0)", + ).Error; err != nil { + t.Errorf("the key stayed taken after its row was soft-deleted: %v", err) + } + }) +} + +// The demo site has already proven sys_api can hold historical duplicates; +// the migration has to name them and refuse, not let CREATE UNIQUE INDEX +// fail on an operator with no idea which rows to reconcile. +func TestSeedNaturalKeysRefusesDuplicateApis(t *testing.T) { + db := openSeedNaturalKeysDB(t) + for i := 0; i < 2; i++ { + if err := db.Create(&oldSeedApi{AppCode: "order", Path: "/api/v1/order", Action: "GET"}).Error; err != nil { + t.Fatalf("seed duplicate %d: %v", i, err) + } + } + + err := seedNaturalKeys(db) + if err == nil { + t.Fatal("the migration accepted sys_api rows that already hold a duplicate (app_code, path, action)") + } + if !contains(err.Error(), "order") || !contains(err.Error(), "/api/v1/order") { + t.Errorf("the error does not name the offending row: %v", err) + } + if db.Migrator().HasIndex(&adminmodels.SysApi{}, "uk_sys_api_app_path_action_del") { + t.Error("the unique index was built despite the migration refusing") + } + // sys_menu's column and index are independent of sys_api's outcome and + // should already be in place - a partial failure here still leaves a + // record of what succeeded, same as any other non-transactional DDL + // migration in this package. + if !db.Migrator().HasColumn(&adminmodels.SysMenu{}, "SeedCode") { + t.Error("sys_menu.seed_code was not added even though only the sys_api step failed") + } +} + +// Only live rows count towards the duplicate check: a row a prior, +// unrelated soft-delete already retired does not block the index it will +// never occupy a slot in. +func TestSeedNaturalKeysIgnoresSoftDeletedApiDuplicates(t *testing.T) { + db := openSeedNaturalKeysDB(t) + if err := db.Create(&oldSeedApi{AppCode: "order", Path: "/api/v1/order", Action: "GET"}).Error; err != nil { + t.Fatalf("seed live row: %v", err) + } + if err := db.Create(&oldSeedApi{AppCode: "order", Path: "/api/v1/order", Action: "GET", DeletedAt: time.Now().UnixMilli()}).Error; err != nil { + t.Fatalf("seed soft-deleted row: %v", err) + } + + if err := seedNaturalKeys(db); err != nil { + t.Fatalf("migrate: %v", err) + } + if !db.Migrator().HasIndex(&adminmodels.SysApi{}, "uk_sys_api_app_path_action_del") { + t.Error("the unique index was not built") + } +} + +// The point of the sys_api index, mirroring +// TestSeedNaturalKeysMenuUniqueIndexBindsLiveRowsOnly above: a second live +// row is rejected, and the key is free again once the row is +// soft-deleted. +func TestSeedNaturalKeysApiUniqueIndexBindsLiveRowsOnly(t *testing.T) { + db := openSeedNaturalKeysDB(t) + if err := db.Create(&oldSeedApi{AppCode: "order", Path: "/api/v1/order", Action: "GET"}).Error; err != nil { + t.Fatalf("seed: %v", err) + } + if err := seedNaturalKeys(db); err != nil { + t.Fatalf("migrate: %v", err) + } + + t.Run("a second live row with the same natural key is rejected", func(t *testing.T) { + err := db.Exec( + "INSERT INTO sys_api (app_code, path, action, deleted_at) VALUES ('order', '/api/v1/order', 'GET', 0)", + ).Error + if err == nil { + t.Fatal("a duplicate (app_code, path, action) was accepted while both rows were live") + } + }) + + t.Run("the key is free again once the row is soft-deleted", func(t *testing.T) { + if err := db.Exec( + "UPDATE sys_api SET deleted_at = ? WHERE path = '/api/v1/order'", time.Now().UnixMilli(), + ).Error; err != nil { + t.Fatalf("soft-delete: %v", err) + } + if err := db.Exec( + "INSERT INTO sys_api (app_code, path, action, deleted_at) VALUES ('order', '/api/v1/order', 'GET', 0)", + ).Error; err != nil { + t.Errorf("the key stayed taken after its row was soft-deleted: %v", err) + } + }) +} + +// Running it twice must be safe: DDL does not roll back on MySQL, so an +// operator whose first attempt failed partway through (say, sys_menu's step +// succeeded and sys_api's refused) has nothing to do but run the whole +// migration again once the duplicates are reconciled. +func TestSeedNaturalKeysIsRepeatable(t *testing.T) { + db := openSeedNaturalKeysDB(t) + if err := db.Create(&oldSeedApi{AppCode: "order", Path: "/api/v1/order", Action: "GET"}).Error; err != nil { + t.Fatalf("seed: %v", err) + } + + for i := 0; i < 3; i++ { + if err := seedNaturalKeys(db); err != nil { + t.Fatalf("migrate %d: %v", i, err) + } + } +} + +// The wrapper's contract with Migrate.run(): the version is only recorded +// once the whole thing - both columns, both indexes - succeeded. +func TestSeedNaturalKeysWrapperRecordsTheVersion(t *testing.T) { + db := openSeedNaturalKeysDB(t) + if err := _1786700008000SeedNaturalKeys(db, "1786700008000"); err != nil { + t.Fatalf("migrate: %v", err) + } + var applied common.Migration + if err := db.Where("version = ?", "1786700008000").First(&applied).Error; err != nil { + t.Fatalf("sys_migration was not recorded: %v", err) + } +} + +// GROUP BY treats two NULLs as equal for grouping purposes; a UNIQUE INDEX +// treats every NULL as distinct from every other value, including another +// NULL - both are standard SQL semantics, not a quirk of one dialect (see +// the postgres-only test file next to this one for the same check against +// a real server). A duplicate check that groups on the raw columns without +// accounting for that difference refuses an install the index itself would +// accept without complaint, on data there is nothing to "reconcile" - +// worse than the index simply failing to build, because it stops a library +// that has nothing wrong with it. +// +// sys_api.path/action carry no NOT NULL constraint - see the design doc's +// note on this migration for why that stays true in this batch, changing +// it is an independent, backward-incompatible migration of its own - so +// this state is reachable in a real database even though seedApis's own +// Create call, which always writes the Go zero value "" rather than NULL, +// never produces it itself. Inserted via raw SQL for exactly that reason: +// models.SysApi's Path/Action are plain (non-pointer) Go strings, which +// cannot represent NULL through a normal Create call. +func TestSeedNaturalKeysDoesNotFlagWhatTheIndexWouldAccept(t *testing.T) { + db := openSeedNaturalKeysDB(t) + for i := 0; i < 2; i++ { + if err := db.Exec( + "INSERT INTO sys_api (app_code, path, action, deleted_at) VALUES ('order', NULL, NULL, 0)", + ).Error; err != nil { + t.Fatalf("seed NULL row %d: %v", i, err) + } + } + + if err := seedNaturalKeys(db); err != nil { + t.Fatalf("seedNaturalKeys refused a library the unique index itself accepts: %v", err) + } + if !db.Migrator().HasIndex(&adminmodels.SysApi{}, "uk_sys_api_app_path_action_del") { + t.Error("the unique index was not built even though seedNaturalKeys reported success") + } +} + +// The case above has both path and action NULL on every row, which both +// of the query's two NULL-exclusion conditions independently catch - it +// cannot tell "only path IS NOT NULL is doing anything here" apart from +// "both conditions are doing something". A row missing only one of the +// two is exactly as real (an api registered with a path but no method, +// or vice versa) and exercises only one condition at a time: two rows +// sharing a real path but both NULL in action, or two rows sharing a real +// action but both NULL in path. GROUP BY treats each pair's shared NULL +// the same way it treats a shared (NULL, NULL) - as equal - and the +// unique index accepts both pairs for the same reason it accepts the +// (NULL, NULL) case, so neither belongs in the count either. +func TestSeedNaturalKeysDoesNotFlagPartiallyNullRows(t *testing.T) { + cases := []struct { + name string + insert string // two rows, sharing a value in exactly one of path/action + }{ + { + name: "path is null, action repeats", + insert: "INSERT INTO sys_api (app_code, path, action, deleted_at) VALUES ('order', NULL, 'GET', 0)", + }, + { + name: "action is null, path repeats", + insert: "INSERT INTO sys_api (app_code, path, action, deleted_at) VALUES ('order', '/api/v1/order', NULL, 0)", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + db := openSeedNaturalKeysDB(t) + for i := 0; i < 2; i++ { + if err := db.Exec(tc.insert).Error; err != nil { + t.Fatalf("seed row %d: %v", i, err) + } + } + + if err := seedNaturalKeys(db); err != nil { + t.Fatalf("seedNaturalKeys refused a library the unique index itself accepts: %v", err) + } + if !db.Migrator().HasIndex(&adminmodels.SysApi{}, "uk_sys_api_app_path_action_del") { + t.Error("the unique index was not built even though seedNaturalKeys reported success") + } + }) + } +} diff --git a/go.mod b/go.mod index 8e175692..d2d981ba 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/casbin/casbin/v3 v3.8.1 github.com/gin-gonic/gin v1.12.0 github.com/glebarez/sqlite v1.11.0 - github.com/go-admin-team/go-admin-core/v2 v2.7.0 + github.com/go-admin-team/go-admin-core/v2 v2.8.0 github.com/google/uuid v1.6.0 github.com/huaweicloud/huaweicloud-sdk-go-obs v3.26.6+incompatible github.com/mssola/user_agent v0.6.0 diff --git a/go.sum b/go.sum index 97fe8cf1..bacd6323 100644 --- a/go.sum +++ b/go.sum @@ -145,8 +145,8 @@ github.com/glebarez/go-sqlite v1.22.0 h1:uAcMJhaA6r3LHMTFgP0SifzgXg46yJkgxqyuyec github.com/glebarez/go-sqlite v1.22.0/go.mod h1:PlBIdHe0+aUEFn+r2/uthrWq4FxbzugL0L8Li6yQJbc= github.com/glebarez/sqlite v1.11.0 h1:wSG0irqzP6VurnMEpFGer5Li19RpIRi2qvQz++w0GMw= github.com/glebarez/sqlite v1.11.0/go.mod h1:h8/o8j5wiAsqSPoWELDUdJXhjAhsVliSn7bWZjOhrgQ= -github.com/go-admin-team/go-admin-core/v2 v2.7.0 h1:1qV0/5iFBvkE3BRtm4ip0v0QYG9Fgx4UtOTd8zkQT9c= -github.com/go-admin-team/go-admin-core/v2 v2.7.0/go.mod h1:LG/XvEfOplbuadKrPTPm0Nu5pN06aQUNZZC3ao4B4gs= +github.com/go-admin-team/go-admin-core/v2 v2.8.0 h1:ZTw5Z/UT1/7OltbGPEaEVerRk4z3koB6O8nDbb84tPM= +github.com/go-admin-team/go-admin-core/v2 v2.8.0/go.mod h1:LG/XvEfOplbuadKrPTPm0Nu5pN06aQUNZZC3ao4B4gs= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/kit v0.10.0/go.mod h1:xUsJbQ/Fp4kEt7AFgCuvyX4a71u8h9jB8tj/ORgOZ7o=