diff --git a/app/admin/models/sys_api.go b/app/admin/models/sys_api.go index 5981dda6..fea4d492 100644 --- a/app/admin/models/sys_api.go +++ b/app/admin/models/sys_api.go @@ -23,6 +23,10 @@ type SysApi struct { Path string `json:"path" gorm:"size:128;comment:地址"` Action string `json:"action" gorm:"size:16;comment:请求类型"` Type string `json:"type" gorm:"size:16;comment:接口类型"` + // AppCode identifies which application's seed.SeedMenus call wrote this + // row; empty for the host's own built-in APIs. Same NOT NULL DEFAULT '' + // reasoning as SysMenu.AppCode. + AppCode string `json:"appCode" gorm:"type:varchar(64);not null;default:'';index:idx_sys_api_app_code;comment:AppCode"` models.ModelTime models.ControlBy } diff --git a/app/admin/models/sys_menu.go b/app/admin/models/sys_menu.go index ea7e6693..2f4baf5c 100644 --- a/app/admin/models/sys_menu.go +++ b/app/admin/models/sys_menu.go @@ -26,6 +26,12 @@ type SysMenu struct { RoleId int `gorm:"-"` Children []SysMenu `json:"children,omitempty" gorm:"-"` IsSelect bool `json:"is_select" gorm:"-"` + // AppCode identifies which application's seed.SeedMenus call wrote this + // row; empty for the host's own built-in menus. NOT NULL DEFAULT '' for + // the same reason sys_migration.app_code is (see contract/models.Migration): + // 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"` models.ControlBy models.ModelTime } diff --git a/app/admin/service/seed.go b/app/admin/service/seed.go new file mode 100644 index 00000000..0dc4562a --- /dev/null +++ b/app/admin/service/seed.go @@ -0,0 +1,308 @@ +package service + +import ( + "errors" + "fmt" + "strconv" + "strings" + + "gorm.io/gorm" + + 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" + + "go-admin/app/admin/models" +) + +// adminSeeder is go-admin's own implementation of seed.Seeder: it turns the +// MenuSpec/ApiSpec values a third-party application asks for into rows +// across the four tables a visible, working menu entry needs - sys_api, +// sys_menu, sys_menu_api_rule, and sys_role_menu/casbin_rule - following the +// same shape cmd/migrate/migration/version/1786700001000_demo_menu.go +// already hand-writes for the host's own demo module. +// +// See go-admin-core's docs/contract.md, "Application-supplied menu and API +// entries", for the requirements this satisfies, and the security note on +// seed.Seeder for what this boundary does and does not protect against: an +// application already holds the same *gorm.DB this receives and could write +// sys_menu/sys_api/casbin_rule directly, bypassing this entirely. +type adminSeeder struct{} + +func init() { + seed.RegisterSeeder(adminSeeder{}) +} + +// adminRoleKey is the role every seeded menu is granted to. This mirrors +// 1786700001000_demo_menu.go's own convention rather than inventing a +// second one: MenuSpec carries no "which roles should see this" field for a +// Seeder to consult instead, and admin is the one role guaranteed to exist +// once the framework's own seed data has run. +const adminRoleKey = "admin" + +// menuSortRange is what sys_menu.sort's column type actually holds. +// +// sort is `gorm:"size:4"`, which MySQL builds as a tinyint (-128..127); +// sqlite ignores the width and accepts anything, so this only ever surfaces +// on a real install, mid-migration, as Error 1264 - by which point the +// migration has already run other, non-transactional DDL that will not be +// retried. tools/checksilent's menu-sort-overflow check catches this for +// every MenuSpec-shaped literal committed to this repository, but it walks +// the repository's own source tree: a third-party application living in the +// module cache is invisible to it. This is the equivalent check for that +// application, run when its migration actually calls SeedMenus rather than +// never. +const ( + menuSortMin = -128 + menuSortMax = 127 +) + +func (adminSeeder) SeedMenus(tx *gorm.DB, appCode string, menus []seed.MenuSpec, apis []seed.ApiSpec) error { + apiRows, err := seedApis(tx, appCode, apis) + if err != nil { + return fmt.Errorf("seed: app %q: apis: %w", appCode, err) + } + + menuIDs, err := seedMenuTree(tx, appCode, menus, apiRows) + if err != nil { + return fmt.Errorf("seed: app %q: menus: %w", appCode, err) + } + + // Not `len(menuIDs) == 0`: grantToAdminRole grants two independent + // things, and an application is free to register apis without menus - + // endpoints another service calls, or a UI mounted somewhere else. + // Skipping the whole call on an empty menu list wrote the sys_api rows + // and then no casbin rule for them, so those endpoints were denied to + // everyone, admin included, with a migration that reported success. + if len(menuIDs) == 0 && len(apiRows) == 0 { + return nil + } + return grantToAdminRole(tx, menuIDs, apiRows) +} + +// seedApis writes one sys_api row per ApiSpec and returns them keyed by +// ApiSpec.Code, so seedMenuTree can resolve a MenuSpec's ApiCodes into the +// rows sys_menu_api_rule needs to reference. +// +// sys_api.id is left to autoincrement rather than assigned by the caller, +// unlike 1786700001000_demo_menu.go's hand-picked ids: that migration is +// the one file tools/checksilent's menu-id-collision check can see, because +// it lives in this repository; nothing plays that role for a third-party +// 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. +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)) + for _, a := range apis { + if a.Code == "" { + return nil, errors.New("ApiSpec.Code must not be empty") + } + if seen[a.Code] { + return nil, fmt.Errorf("duplicate ApiSpec.Code %q", a.Code) + } + seen[a.Code] = true + + row := models.SysApi{ + Handle: a.Handle, + Title: a.Title, + Path: a.Path, + Action: a.Method, + Type: "SYS", + AppCode: appCode, + } + if err := tx.Create(&row).Error; err != nil { + return nil, fmt.Errorf("api %q: %w", a.Code, err) + } + rows[a.Code] = row + } + return rows, nil +} + +// seedMenuTree writes one sys_menu row per MenuSpec, resolving Parent/Code +// references into parent_id/paths, and returns every menu id created so the +// caller can grant them to a role. +// +// Specs do not have to be given in parent-before-child order: this makes +// repeated passes over the remaining specs, creating whichever ones have +// their Parent (if any) already created, until every spec is placed. A +// spec whose Parent never resolves - naming a Code missing from this call, +// or only reachable through a cycle - stops making progress and is reported +// rather than looping forever. +func seedMenuTree(tx *gorm.DB, appCode string, specs []seed.MenuSpec, apiRows map[string]models.SysApi) ([]int, error) { + byCode := make(map[string]seed.MenuSpec, len(specs)) + for _, s := range specs { + if s.Code == "" { + return nil, errors.New("MenuSpec.Code must not be empty") + } + if _, dup := byCode[s.Code]; dup { + return nil, fmt.Errorf("duplicate MenuSpec.Code %q", s.Code) + } + if err := validateMenuSpec(s); err != nil { + return nil, fmt.Errorf("%q: %w", s.Code, err) + } + byCode[s.Code] = s + } + + created := make(map[string]models.SysMenu, len(specs)) + ids := make([]int, 0, len(specs)) + + for len(created) < len(specs) { + progressed := false + for _, s := range specs { + if _, done := created[s.Code]; done { + continue + } + + var parentRow models.SysMenu + if s.Parent != "" { + parent, ok := created[s.Parent] + if !ok { + if _, exists := byCode[s.Parent]; !exists { + return nil, fmt.Errorf("%q: Parent %q is not a Code in this call", s.Code, s.Parent) + } + continue // s.Parent exists but has not been created yet; retry next pass + } + parentRow = parent + } + + row := models.SysMenu{ + MenuName: menuName(appCode, s.Code), + Title: s.Title, + Icon: s.Icon, + Path: s.Path, + MenuType: s.Kind, + Permission: s.Permission, + ParentId: parentRow.MenuId, + Component: s.Component, + Sort: s.Sort, + // Visible "0" is shown, not hidden - the same defaults + // 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, + } + for _, code := range s.ApiCodes { + api, ok := apiRows[code] + if !ok { + return nil, fmt.Errorf("%q: ApiCodes references %q, which is not an ApiSpec.Code in this call", s.Code, code) + } + // The full row, not just {Id: api.Id}: gorm's many2many + // association save upserts an associated row whose primary + // key is already set, so a stub carrying only Id would + // overwrite every other column of an sys_api row this same + // call just wrote with zero values. + row.SysApi = append(row.SysApi, api) + } + + if err := tx.Create(&row).Error; err != nil { + return nil, fmt.Errorf("%q: %w", s.Code, err) + } + + // paths is a materialized path from the root ("/0"), built from + // ids that only exist once the row above is created - the same + // 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) + } + 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) + } + + created[s.Code] = row + ids = append(ids, row.MenuId) + progressed = true + } + if !progressed { + return nil, fmt.Errorf("unresolved Parent reference(s) among %d remaining spec(s); check for a cycle", len(specs)-len(created)) + } + } + return ids, 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 +// comment. +func validateMenuSpec(s seed.MenuSpec) error { + switch s.Kind { + case contractmodels.Directory, contractmodels.Menu, contractmodels.Button: + default: + return fmt.Errorf("Kind %q is not one of Directory/Menu/Button", s.Kind) + } + if s.Sort < menuSortMin || s.Sort > menuSortMax { + return fmt.Errorf("Sort %d does not fit sys_menu.sort's tinyint column (%d..%d)", s.Sort, menuSortMin, menuSortMax) + } + return nil +} + +// menuName synthesizes sys_menu.menu_name from appCode and the spec's Code, +// since MenuSpec carries no field of its own for it - contract/seed's +// package doc says a MenuSpec is what rendering a menu and checking a +// button permission need, not a mirror of sys_menu's columns. +// +// PascalCasing both and concatenating them, rather than using Code alone, +// is what keeps two applications that both picked the plain word "list" as +// a Code from producing the identical menu_name: the frontend's keep-alive +// cache matches a route by this exact string, not by (appCode, Code), so a +// collision there is a UI bug, not a database error, and nothing else here +// would ever surface it. +func menuName(appCode, code string) string { + return pascalCase(appCode) + pascalCase(code) +} + +func pascalCase(s string) string { + var b strings.Builder + for _, part := range strings.FieldsFunc(s, func(r rune) bool { return r == '-' || r == '_' }) { + b.WriteString(strings.ToUpper(part[:1])) + b.WriteString(part[1:]) + } + return b.String() +} + +// grantToAdminRole is sys_role_menu and casbin_rule: the two tables +// go-admin-core's contract.md requires alongside sys_menu/sys_api, without +// which a seeded menu is invisible to every role and its apis are +// authorized for no one. +// +// It follows 1786700001000_demo_menu.go's exact pattern, including +// tolerating a missing admin role: a database that has not yet run the +// framework's own seed data (config/db.sql, inside 1599190683659_tables.go) +// has nothing to grant to yet, and namespacedKey's ordering guarantee - every +// 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 { + var role models.SysRole + if err := tx.Where("role_key = ?", adminRoleKey).First(&role).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil + } + return err + } + + for _, id := range menuIDs { + if err := tx.Exec( + "INSERT INTO sys_role_menu (role_id, menu_id) SELECT ?, ? WHERE NOT EXISTS (SELECT 1 FROM sys_role_menu WHERE role_id = ? AND menu_id = ?)", + role.RoleId, id, role.RoleId, id, + ).Error; err != nil { + return err + } + } + + for _, a := range apiRows { + if err := 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 { + return err + } + } + return nil +} diff --git a/app/admin/service/seed_test.go b/app/admin/service/seed_test.go new file mode 100644 index 00000000..fe7a9845 --- /dev/null +++ b/app/admin/service/seed_test.go @@ -0,0 +1,291 @@ +package service + +import ( + "errors" + "strconv" + "strings" + "testing" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" + + 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" + + "go-admin/app/admin/models" +) + +// newSeedTestDB builds the tables adminSeeder.SeedMenus writes to. sys_menu, +// sys_api, sys_role and sys_role_menu (GORM's own join table for +// SysRole.SysMenu) come from AutoMigrate; casbin_rule does not have a GORM +// model anywhere in this codebase - see 1786700001000_demo_menu.go's own +// comment on why models.CasbinRule (-> sys_casbin_rule) is the wrong table - +// so it is created directly, matching the columns grantToAdminRole's INSERT +// addresses. +func newSeedTestDB(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(&models.SysMenu{}, &models.SysApi{}, &models.SysRole{}); err != nil { + t.Fatalf("automigrate: %v", err) + } + 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) + } + return db +} + +func seedAdminRole(t *testing.T, db *gorm.DB) models.SysRole { + t.Helper() + role := models.SysRole{RoleName: "Administrator", RoleKey: adminRoleKey} + if err := db.Create(&role).Error; err != nil { + t.Fatalf("seed admin role: %v", err) + } + return role +} + +// This is the acceptance case go-admin-core's docs/contract.md requires: one +// SeedMenus call populates all four tables a visible, working menu entry +// needs, every row tagged with the appCode it was called with, and the +// parent/child tree resolved into sys_menu's parent_id/paths. +func TestSeedMenusPopulatesAllFourTables(t *testing.T) { + db := newSeedTestDB(t) + seedAdminRole(t, db) + + menus := []seed.MenuSpec{ + {Code: "dir", Kind: contractmodels.Directory, Title: "Order Example", Path: "/apps/order", Component: "Layout", Sort: 10}, + {Code: "list", Parent: "dir", Kind: contractmodels.Menu, Title: "Orders", Path: "list", Component: "apps/order/order/index", Sort: 1, ApiCodes: []string{"list"}}, + {Code: "btn-create", Parent: "list", Kind: contractmodels.Button, Title: "Create", Permission: "order:order:create", Sort: 1}, + } + apis := []seed.ApiSpec{ + {Code: "list", Title: "Order list", Path: "/api/v1/order", Method: "GET", Handle: "apis.Order.GetPage-fm"}, + } + + err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", menus, apis) + }) + if err != nil { + t.Fatalf("SeedMenus: %v", err) + } + + var apiRows []models.SysApi + if err := db.Find(&apiRows).Error; err != nil { + t.Fatal(err) + } + if len(apiRows) != 1 || apiRows[0].AppCode != "order" || apiRows[0].Path != "/api/v1/order" { + t.Fatalf("sys_api = %+v", apiRows) + } + + var menuRows []models.SysMenu + if err := db.Order("sort").Find(&menuRows).Error; err != nil { + t.Fatal(err) + } + if len(menuRows) != 3 { + t.Fatalf("sys_menu has %d rows, want 3: %+v", len(menuRows), menuRows) + } + byName := map[string]models.SysMenu{} + for _, m := range menuRows { + if m.AppCode != "order" { + t.Errorf("menu %q app_code = %q, want order", m.MenuName, m.AppCode) + } + byName[m.MenuName] = m + } + dir, ok := byName[menuName("order", "dir")] + if !ok || dir.ParentId != 0 || dir.Paths != "/0/"+strconv.Itoa(dir.MenuId) { + t.Fatalf("dir menu = %+v", dir) + } + list, ok := byName[menuName("order", "list")] + if !ok || list.ParentId != dir.MenuId || list.Paths != dir.Paths+"/"+strconv.Itoa(list.MenuId) { + t.Fatalf("list menu = %+v (dir=%+v)", list, dir) + } + btn, ok := byName[menuName("order", "btn-create")] + if !ok || btn.ParentId != list.MenuId { + t.Fatalf("btn menu = %+v (list=%+v)", btn, list) + } + + // sys_menu_api_rule: gorm's own many2many join table for SysMenu.SysApi. + var joinCount int64 + if err := db.Table("sys_menu_api_rule"). + Where("sys_menu_menu_id = ? AND sys_api_id = ?", list.MenuId, apiRows[0].Id). + Count(&joinCount).Error; err != nil { + t.Fatal(err) + } + if joinCount != 1 { + t.Errorf("sys_menu_api_rule has %d row(s) linking list to its api, want 1", joinCount) + } + + // sys_role_menu: every seeded menu granted to the admin role. + var roleMenuCount int64 + if err := db.Table("sys_role_menu").Count(&roleMenuCount).Error; err != nil { + t.Fatal(err) + } + if roleMenuCount != 3 { + t.Errorf("sys_role_menu has %d row(s), want 3 (one per seeded menu)", roleMenuCount) + } + + // casbin_rule: the api's path/method granted to the admin role. + var casbinCount int64 + if err := db.Table("casbin_rule"). + Where("ptype = 'p' AND v0 = ? AND v1 = ? AND v2 = ?", adminRoleKey, "/api/v1/order", "GET"). + Count(&casbinCount).Error; err != nil { + t.Fatal(err) + } + if casbinCount != 1 { + t.Errorf("casbin_rule has %d matching row(s), want 1", casbinCount) + } +} + +// A database that has not run the framework's own seed data yet (no admin +// role) must not fail SeedMenus - 1786700001000_demo_menu.go tolerates +// exactly the same condition for the host's own demo module. +func TestSeedMenusToleratesMissingAdminRole(t *testing.T) { + db := newSeedTestDB(t) + + err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", []seed.MenuSpec{ + {Code: "dir", Kind: contractmodels.Directory, Title: "Order"}, + }, nil) + }) + if err != nil { + t.Fatalf("SeedMenus: %v", err) + } + + var roleMenuCount int64 + if err := db.Table("sys_role_menu").Count(&roleMenuCount).Error; err != nil { + t.Fatal(err) + } + if roleMenuCount != 0 { + t.Errorf("sys_role_menu has %d row(s) with no role to grant to", roleMenuCount) + } +} + +func TestSeedMenusRejectsMalformedSpecs(t *testing.T) { + cases := []struct { + name string + menus []seed.MenuSpec + apis []seed.ApiSpec + want string + }{ + { + name: "duplicate menu code", + menus: []seed.MenuSpec{{Code: "a", Kind: contractmodels.Directory}, {Code: "a", Kind: contractmodels.Directory}}, + want: `duplicate MenuSpec.Code "a"`, + }, + { + name: "unresolved parent", + menus: []seed.MenuSpec{{Code: "a", Parent: "missing", Kind: contractmodels.Menu}}, + want: `Parent "missing" is not a Code in this call`, + }, + { + name: "unresolved api code", + menus: []seed.MenuSpec{{Code: "a", Kind: contractmodels.Menu, ApiCodes: []string{"missing"}}}, + want: `ApiCodes references "missing"`, + }, + { + name: "unknown kind", + menus: []seed.MenuSpec{{Code: "a", Kind: "X"}}, + want: `Kind "X" is not one of Directory/Menu/Button`, + }, + { + name: "sort overflows a tinyint", + menus: []seed.MenuSpec{{Code: "a", Kind: contractmodels.Directory, Sort: 900}}, + want: `Sort 900 does not fit sys_menu.sort's tinyint column`, + }, + { + name: "duplicate api code", + apis: []seed.ApiSpec{{Code: "x"}, {Code: "x"}}, + want: `duplicate ApiSpec.Code "x"`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + db := newSeedTestDB(t) + err := db.Transaction(func(tx *gorm.DB) error { + return adminSeeder{}.SeedMenus(tx, "order", tc.menus, tc.apis) + }) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("err = %v, want it to contain %q", err, tc.want) + } + }) + } +} + +// TestSeederIsRegistered pins the registration itself, not the behaviour. +// +// Every other test here calls adminSeeder{}.SeedMenus directly, which proves +// the implementation is right and proves nothing about whether anything ever +// reaches it: delete the RegisterSeeder call in init() and they all stay +// green, while a real migrate fails with ErrNoSeeder and no menu is written. +// Going through the package-level SeedMenus is what closes that gap - it is +// the door an application actually knocks on. +func TestSeederIsRegistered(t *testing.T) { + db := newSeedTestDB(t) + err := db.Transaction(func(tx *gorm.DB) error { + return seed.SeedMenus(tx, "probe", []seed.MenuSpec{{ + Code: "root", Kind: contractmodels.Directory, Title: "Probe", Sort: 1, + }}, nil) + }) + if errors.Is(err, seed.ErrNoSeeder) { + t.Fatal("no Seeder is registered: an application's SeedMenus would write no menu at all") + } + if err != nil { + t.Fatalf("SeedMenus through the package-level entry point: %v", err) + } +} + +// An application is free to register apis with no menus at all - endpoints +// another service calls, or a UI mounted somewhere else. Skipping +// grantToAdminRole on an empty menu list wrote the sys_api rows and then no +// casbin rule for them, so every one of those endpoints was denied to +// everyone including admin, from a migration that reported success. +func TestSeedMenusGrantsApisWhenThereAreNoMenus(t *testing.T) { + db := newSeedTestDB(t) + role := seedAdminRole(t, db) + + apis := []seed.ApiSpec{ + {Code: "hook", Title: "Inbound hook", Path: "/api/v1/hook", Method: "POST", Handle: "hook.Receive"}, + {Code: "sync", Title: "Sync", Path: "/api/v1/sync", Method: "GET", Handle: "hook.Sync"}, + } + if err := (adminSeeder{}).SeedMenus(db, "hooks", nil, apis); err != nil { + t.Fatalf("SeedMenus: %v", err) + } + + var apiCount int64 + db.Model(&models.SysApi{}).Where("app_code = ?", "hooks").Count(&apiCount) + if apiCount != int64(len(apis)) { + t.Fatalf("sys_api rows = %d, want %d", apiCount, len(apis)) + } + + for _, a := range apis { + var n int64 + db.Table("casbin_rule"). + Where("ptype = 'p' AND v0 = ? AND v1 = ? AND v2 = ?", role.RoleKey, a.Path, a.Method). + Count(&n) + if n != 1 { + t.Errorf("casbin_rule for %s %s = %d rows, want 1: the endpoint is denied to admin", a.Method, a.Path, n) + } + } +} + +// The other half of the same guard: nothing registered at all must stay a +// no-op rather than start touching sys_role_menu or casbin_rule. +func TestSeedMenusWithNothingRegisteredWritesNothing(t *testing.T) { + db := newSeedTestDB(t) + seedAdminRole(t, db) + + if err := (adminSeeder{}).SeedMenus(db, "empty", nil, nil); err != nil { + t.Fatalf("SeedMenus: %v", err) + } + for _, table := range []string{"casbin_rule", "sys_role_menu"} { + var n int64 + db.Table(table).Count(&n) + if n != 0 { + t.Errorf("%s has %d rows, want 0", table, n) + } + } +} diff --git a/cmd/migrate/migration/init.go b/cmd/migrate/migration/init.go index ac58b582..669bec65 100644 --- a/cmd/migrate/migration/init.go +++ b/cmd/migrate/migration/init.go @@ -3,7 +3,6 @@ package migration import ( "fmt" "log" - "path/filepath" "sort" "strings" "sync" @@ -11,11 +10,21 @@ import ( "gorm.io/gorm" + contractmigration "github.com/go-admin-team/go-admin-core/v2/sdk/contract/migration" + common "go-admin/common/models" ) var Migrate = newMigration() +// contractSnapshot is contractmigration.Snapshot, indirected through a +// package-level variable so tests can substitute an isolated +// *contractmigration.Registry's Snapshot instead of reaching into +// go-admin-core's single process-wide registry, which every *Migration in +// this process - test-local or the package-level Migrate - reads through the +// same call. See mergedEntries. +var contractSnapshot = contractmigration.Snapshot + func newMigration() *Migration { return &Migration{version: make(map[string]versionEntry)} } @@ -135,6 +144,47 @@ func namespacedKey(appCode, k string) string { return appCode + "-" + k } +// mergedEntries returns every migration this process knows about: the +// host's own registry (e.version, filled by version/*.go and +// version-local/*.go through SetVersion/ForApp) plus whatever a third-party +// application registered through go-admin-core's sdk/contract/migration +// package (PRD 006, F9's host wiring). +// +// That package keeps its own process-wide registry, entirely separate from +// e.version, because a third-party application cannot reach into this +// process to call an unexported method on *Migration - contract/migration's +// package-level ForApp/Snapshot are the only door open to it. Without this +// merge, migrate/status/--dry-run would only ever see the host's own +// migrations: an application's ForApp("crm").SetVersion(...) would compile, +// register successfully into contract/migration's registry, and then never +// run, with no error anywhere - the exact silent gap this method closes. +// +// Entry and versionEntry are structurally identical (an app code plus a +// func(db, version) error); the conversion below exists only because they +// are two distinct named types, one per package, not because the data +// differs. +func (e *Migration) mergedEntries() map[string]versionEntry { + e.mutex.Lock() + out := make(map[string]versionEntry, len(e.version)) + for k, v := range e.version { + out[k] = v + } + e.mutex.Unlock() + + for k, entry := range contractSnapshot() { + if _, exists := out[k]; exists { + // contract/migration.ForApp namespaces every app-owned key as + // appCode + "-" + k, and appCode is reserved from ""/"core", so + // this should never collide with a host-registered key. If it + // somehow does, the host's own registration wins rather than + // silently overwriting it. + continue + } + out[k] = versionEntry{appCode: entry.AppCode, fn: entry.Fn} + } + return out +} + // StatusEntry is one row of migrate status. type StatusEntry struct { AppCode string @@ -156,12 +206,11 @@ func (e *Migration) Status() ([]StatusEntry, error) { return nil, fmt.Errorf("migration: no database configured") } - e.mutex.Lock() - registered := make(map[string]string, len(e.version)) - for k, v := range e.version { + all := e.mergedEntries() + registered := make(map[string]string, len(all)) + for k, v := range all { registered[k] = v.appCode } - e.mutex.Unlock() applied := make(map[string]common.Migration) // A database that has never been migrated has no sys_migration table. @@ -247,12 +296,11 @@ func DisplayAppCode(code string) string { // AppCodes lists the app codes with at least one registered migration, framework // included under its display name, sorted. func (e *Migration) AppCodes() []string { - e.mutex.Lock() + all := e.mergedEntries() seen := map[string]struct{}{} - for _, v := range e.version { + for _, v := range all { seen[DisplayAppCode(v.appCode)] = struct{}{} } - e.mutex.Unlock() out := make([]string, 0, len(seen)) for code := range seen { @@ -263,17 +311,16 @@ func (e *Migration) AppCodes() []string { } func (e *Migration) run(appCode string) { - e.mutex.Lock() - versions := make([]string, 0, len(e.version)) - entries := make(map[string]versionEntry, len(e.version)) - for k, v := range e.version { + all := e.mergedEntries() + versions := make([]string, 0, len(all)) + entries := make(map[string]versionEntry, len(all)) + for k, v := range all { if appCode != allApps && v.appCode != appCode { continue } versions = append(versions, k) entries[k] = v } - e.mutex.Unlock() sort.Strings(versions) // A mistyped --app would otherwise select nothing and report "no @@ -315,7 +362,10 @@ func (e *Migration) run(appCode string) { // from the empty app code, which selects the framework's own migrations. const allApps = "\x00all" +// GetFilename derives a migration's version from its file name. The rule +// lives in contract/migration, because an application registering through +// that package names its files by the same convention and must land on the +// same version string; a second copy here is a second thing to keep in step. func GetFilename(s string) string { - s = filepath.Base(s) - return s[:13] + return contractmigration.GetFilename(s) } diff --git a/cmd/migrate/migration/init_test.go b/cmd/migrate/migration/init_test.go index ec59a8d6..13d8c6a6 100644 --- a/cmd/migrate/migration/init_test.go +++ b/cmd/migrate/migration/init_test.go @@ -12,9 +12,26 @@ import ( "gorm.io/gorm" "gorm.io/gorm/logger" + contractmigration "github.com/go-admin-team/go-admin-core/v2/sdk/contract/migration" + common "go-admin/common/models" ) +// withContractRegistry points contractSnapshot at an isolated +// *contractmigration.Registry for the duration of one test, instead of +// go-admin-core's single process-wide one - see contractSnapshot's doc +// comment for why that indirection exists. Restored on cleanup so other +// tests in this package keep seeing an empty contract registry regardless of +// run order. +func withContractRegistry(t *testing.T) *contractmigration.Registry { + t.Helper() + reg := contractmigration.NewRegistry() + orig := contractSnapshot + contractSnapshot = reg.Snapshot + t.Cleanup(func() { contractSnapshot = orig }) + return reg +} + func newTestDB(t *testing.T) *gorm.DB { t.Helper() db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{ @@ -388,3 +405,178 @@ func TestMigrateAppOnAnUnknownCodeSaysSo(t *testing.T) { t.Errorf("a typo ran %v", rows) } } + +// This is the acceptance test for PRD 006's host-wiring gap: a migration +// registered through contract/migration.ForApp - the only door open to a +// third-party application - must actually run, be recorded under its app +// code, and show up in AppCodes/Status/--app the same as one registered +// through the host's own m.ForApp. Before mergedEntries existed, m.Migrate() +// never looked at contract/migration's registry at all, so this compiled, +// registered, and silently never ran. +func TestMergedEntriesRunsAContractRegisteredAppMigration(t *testing.T) { + reg := withContractRegistry(t) + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + ran := false + reg.ForApp("order").SetVersion("1793800000000", func(db *gorm.DB, version, appCode string) error { + ran = true + return recordFor(db, version, appCode) + }) + + m.Migrate() + + if !ran { + t.Fatal("contract-registered migration did not run") + } + rows := rowsByVersion(t, db) + row, ok := rows["order-1793800000000"] + if !ok { + t.Fatalf("no row for order-1793800000000; got %v", rows) + } + if row.AppCode != "order" { + t.Errorf("app_code = %q, want %q", row.AppCode, "order") + } +} + +// migrate status and --dry-run both read Status; a contract-registered +// migration has to appear there under its app code exactly like a +// host-registered one, both before and after it is applied. +func TestMergedEntriesStatusIncludesContractRegisteredMigrations(t *testing.T) { + reg := withContractRegistry(t) + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + reg.ForApp("order").SetVersion("1793800000000", func(db *gorm.DB, version, appCode string) error { + return recordFor(db, version, appCode) + }) + + entries, err := m.Status() + if err != nil { + t.Fatal(err) + } + byVersion := map[string]StatusEntry{} + for _, e := range entries { + byVersion[e.Version] = e + } + e, ok := byVersion["order-1793800000000"] + if !ok || !e.Registered || e.Applied || e.AppCode != "order" { + t.Fatalf("pending contract entry = %+v (ok=%v)", e, ok) + } + + m.Migrate() + + entries, err = m.Status() + if err != nil { + t.Fatal(err) + } + byVersion = map[string]StatusEntry{} + for _, e := range entries { + byVersion[e.Version] = e + } + if e := byVersion["order-1793800000000"]; !e.Applied { + t.Errorf("applied contract entry = %+v", e) + } +} + +// AppCodes feeds both --app's typo detection (appRegistrationError) and the +// group headings status prints; a contract-registered app has to appear +// there or a real "go-admin migrate --app order" would be told the app does +// not exist. +func TestMergedEntriesAppCodesIncludesContractRegisteredApps(t *testing.T) { + reg := withContractRegistry(t) + m := newMigration() + m.SetVersion("1786700009000", func(db *gorm.DB, version string) error { return nil }) + reg.ForApp("order").SetVersion("1793800000000", func(db *gorm.DB, version, appCode string) error { return nil }) + + got := m.AppCodes() + want := []string{"core", "order"} + if len(got) != len(want) { + t.Fatalf("AppCodes = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("AppCodes = %v, want %v", got, want) + } + } +} + +// --app order has to actually run only order's migrations - the same +// per-app isolation MigrateApp already gives host-registered apps - even +// though order is registered in a different registry entirely. +func TestMergedEntriesMigrateAppRunsOnlyThatContractApp(t *testing.T) { + reg := withContractRegistry(t) + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + ran := map[string]bool{} + m.SetVersion("1786700009000", func(db *gorm.DB, version string) error { + ran["core"] = true + return db.Create(&common.Migration{Version: version}).Error + }) + reg.ForApp("order").SetVersion("1793800000000", func(db *gorm.DB, version, appCode string) error { + ran["order"] = true + return recordFor(db, version, appCode) + }) + + m.MigrateApp("order") + + if !ran["order"] { + t.Error("order did not run") + } + if ran["core"] { + t.Errorf("MigrateApp(order) also ran %v", ran) + } +} + +// A host-registered key is not supposed to collide with a namespaced +// contract key (see mergedEntries' doc comment), but if it somehow did, the +// host's own registration must win rather than a third-party application +// silently overwriting a framework migration under the same key. +func TestMergedEntriesHostRegistrationWinsOnKeyCollision(t *testing.T) { + reg := withContractRegistry(t) + db := newTestDB(t) + m := newMigration() + m.SetDb(db) + + hostRan, contractRan := false, false + m.ForApp("dup").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { + hostRan = true + return recordFor(db, version, appCode) + }) + reg.ForApp("dup").SetVersion("1786800001000", func(db *gorm.DB, version, appCode string) error { + contractRan = true + return recordFor(db, version, appCode) + }) + + m.Migrate() + + if !hostRan { + t.Error("host registration did not run") + } + if contractRan { + t.Error("contract registration ran; host registration should have won the collision") + } +} + +// GetFilename must stay the same rule the contract package applies, since an +// application registering through contract/migration names its files by that +// convention and has to land on the same version string. Pinning the reject +// case is what catches a re-divergence: a local copy that only sliced would +// return "add_orders.go" here and register a migration under a key that never +// matches anything. +func TestGetFilenameDelegatesToTheContractRule(t *testing.T) { + if got := GetFilename("version/1786700001000_demo_menu.go"); got != "1786700001000" { + t.Fatalf("GetFilename = %q, want %q", got, "1786700001000") + } + + defer func() { + if recover() == nil { + t.Fatal("a file name carrying no version did not panic") + } + }() + GetFilename("version/add_orders.go") +} diff --git a/cmd/migrate/migration/version/1786700006000_app_code_columns.go b/cmd/migrate/migration/version/1786700006000_app_code_columns.go new file mode 100644 index 00000000..b7bc2157 --- /dev/null +++ b/cmd/migrate/migration/version/1786700006000_app_code_columns.go @@ -0,0 +1,46 @@ +package version + +import ( + "runtime" + + "gorm.io/gorm" + + adminmodels "go-admin/app/admin/models" + "go-admin/cmd/migrate/migration" + common "go-admin/common/models" +) + +// Add sys_menu.app_code and sys_api.app_code ahead of PRD 006 F9's Seeder. +// +// Every row a third-party application's migration writes through +// seed.SeedMenus must be attributable to the app that wrote it, so +// installing, auditing, or removing one application does not require +// guessing which rows belong to it - see go-admin-core's docs/contract.md, +// "Application-supplied menu and API entries", for the requirement this +// satisfies. +// +// Ordered after 1786700003000, so importing cmd/migrate/migration/models is +// banned here (see schema_coverage_test.go's +// TestPostConversionMigrationsAvoidFrozenSeedModels): AddColumn instead +// reads the runtime models' own gorm tags directly, which is also what +// makes the column this adds match the one the admin Seeder writes through +// those same structs. +func init() { + _, fileName, _, _ := runtime.Caller(0) + migration.Migrate.SetVersion(migration.GetFilename(fileName), _1786700006000AppCodeColumns) +} + +func _1786700006000AppCodeColumns(db *gorm.DB, version string) error { + m := db.Migrator() + if !m.HasColumn(&adminmodels.SysMenu{}, "AppCode") { + if err := m.AddColumn(&adminmodels.SysMenu{}, "AppCode"); err != nil { + return err + } + } + if !m.HasColumn(&adminmodels.SysApi{}, "AppCode") { + if err := m.AddColumn(&adminmodels.SysApi{}, "AppCode"); err != nil { + return err + } + } + return db.Create(&common.Migration{Version: version}).Error +}