From 74b0ee8776da505d00060fb29b0557d35758b1b4 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Wed, 9 Sep 2026 12:35:17 +0800 Subject: [PATCH] =?UTF-8?q?fix=F0=9F=90=9B:=20stop=20sys=5Fmenu=20declarin?= =?UTF-8?q?g=20an=20index=20stricter=20than=20the=20real=20one?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit uk_sys_menu_app_seed_code_del covers (app_code, seed_code, deleted_at) and is created by 1786700008000 with explicit SQL. The struct tag named the same index on SeedCode alone, and a named uniqueIndex tag collects only the fields carrying that name - so AutoMigrate on this model would build a unique index on seed_code by itself: stricter than the real one, and forbidding two applications from both having a "dir" node, which the composite key exists to allow. Worse than being stricter, it would win. The migration only creates its index when HasIndex says the name is free, so a schema built by AutoMigrate first keeps the wrong index and the migration steps over it without a word. The tag cannot express the real index: deleted_at comes from the ModelTime embed shared by every table, which no single model can add a tag to. So the tag goes and the migration is the only thing that creates it. No database is affected. The initial table migration AutoMigrates a frozen snapshot of this model that has neither app_code nor seed_code, and nothing else in the repository AutoMigrates the live one - which is why this stayed invisible until a test built the schema from the live model and seeded two applications, and got a unique-constraint failure on a seed code they are supposed to be able to share. --- app/admin/models/sys_menu.go | 13 +++++- app/admin/models/sys_menu_seed_code_test.go | 49 +++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 app/admin/models/sys_menu_seed_code_test.go diff --git a/app/admin/models/sys_menu.go b/app/admin/models/sys_menu.go index 1fdddc57..51adc8ec 100644 --- a/app/admin/models/sys_menu.go +++ b/app/admin/models/sys_menu.go @@ -44,7 +44,18 @@ type SysMenu struct { // unique index below: the database never treats two NULLs as equal, so // only rows that do carry a real code participate in the uniqueness // check at all. - SeedCode *string `json:"seedCode" gorm:"size:64;uniqueIndex:uk_sys_menu_app_seed_code_del;comment:raw MenuSpec.Code, null for rows not written through SeedMenus"` + // uk_sys_menu_app_seed_code_del is created by the migration, not from + // this tag, and deliberately: it covers (app_code, seed_code, + // deleted_at), and this struct cannot say so. A named uniqueIndex tag + // puts every field carrying that name into one index, and deleted_at + // comes from the shared ModelTime embed, which no single model can add a + // tag to. Naming it here anyway declared a unique index on seed_code + // alone under the same name - stricter than the real one, forbidding two + // applications from both having a "dir" node - and AutoMigrate on this + // model would have created that one first, after which the migration's + // HasIndex guard finds the name taken and leaves the wrong index in + // place. + SeedCode *string `json:"seedCode" gorm:"size:64;comment:raw MenuSpec.Code, null for rows not written through SeedMenus"` models.ControlBy models.ModelTime } diff --git a/app/admin/models/sys_menu_seed_code_test.go b/app/admin/models/sys_menu_seed_code_test.go new file mode 100644 index 00000000..aa8d125e --- /dev/null +++ b/app/admin/models/sys_menu_seed_code_test.go @@ -0,0 +1,49 @@ +package models + +import ( + "testing" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" +) + +func ptr(s string) *string { return &s } + +// uk_sys_menu_app_seed_code_del covers (app_code, seed_code, deleted_at) and +// is created by 1786700008000, not from a struct tag. It cannot come from a +// tag: a named uniqueIndex collects every field carrying that name, and +// deleted_at lives in the shared ModelTime embed that no single model can tag. +// +// Naming it on SeedCode alone anyway produced a unique index on seed_code by +// itself under the same name - stricter than the real one - and AutoMigrate +// here would create that one, after which the migration's HasIndex guard +// finds the name taken and leaves the wrong index in place. Nothing in +// production AutoMigrates this model (the initial table migration uses a +// frozen snapshot that has neither column), which is why this never showed up +// as a broken database; it showed up the first time a test built the schema +// from the live model and seeded two applications. +func TestSysMenuDeclaresNoSeedCodeIndexOfItsOwn(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{ + Logger: logger.Default.LogMode(logger.Silent), + }) + if err != nil { + t.Fatalf("open: %v", err) + } + if err := db.AutoMigrate(&SysMenu{}); err != nil { + t.Fatalf("automigrate: %v", err) + } + if db.Migrator().HasIndex(&SysMenu{}, "uk_sys_menu_app_seed_code_del") { + t.Error("AutoMigrate created uk_sys_menu_app_seed_code_del from a tag; " + + "the migration's HasIndex guard will now skip the composite index it should create") + } + + // Two applications, the same seed code. The real index allows it because + // app_code is part of the key; an index on seed_code alone does not. + for _, app := range []string{"order", "crm"} { + row := SysMenu{MenuName: app + "Dir", AppCode: app, SeedCode: ptr("dir")} + if err := db.Create(&row).Error; err != nil { + t.Fatalf("%s could not use the seed code \"dir\": %v", app, err) + } + } +}