diff --git a/app/admin/service/seed.go b/app/admin/service/seed.go index 37705693..bfb09d45 100644 --- a/app/admin/service/seed.go +++ b/app/admin/service/seed.go @@ -210,9 +210,23 @@ func seedMenuTree(tx *gorm.DB, appCode string, specs []seed.MenuSpec, apiRows ma // the soft-delete plugin scopes deleted_at = 0 automatically on // every query against models.SysMenu. var existing models.SysMenu + found := false err := tx.Where("app_code = ? AND seed_code = ?", appCode, s.Code).First(&existing).Error switch { case err == nil: + found = true + case errors.Is(err, gorm.ErrRecordNotFound): + // Nothing under the natural key. It may still be here from + // before seed_code existed, under the name that identified + // it then. + existing, found, err = adoptLegacyMenu(tx, appCode, s) + if err != nil { + return nil, fmt.Errorf("%q: %w", s.Code, err) + } + default: + return nil, fmt.Errorf("%q: checking for an existing row: %w", s.Code, err) + } + if found { row, err := repairExistingMenu(tx, existing, appCode, s, parentRow, apiRows) if err != nil { return nil, fmt.Errorf("%q: repairing an existing row: %w", s.Code, err) @@ -221,10 +235,6 @@ func seedMenuTree(tx *gorm.DB, appCode string, specs []seed.MenuSpec, apiRows ma 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) } row := menuRowFor(appCode, s, parentRow) @@ -567,3 +577,45 @@ func repairPaths(tx *gorm.DB, row *models.SysMenu, s seed.MenuSpec, parentRow mo row.Paths = want return nil } + +// adoptLegacyMenu claims a row this application wrote before sys_menu had a +// seed_code column, so a reseed repairs it instead of inserting a second copy +// beside it. +// +// 1786700008000 added the column and left it NULL on every row already there, +// which is right for the host's own hand-placed menus - there is nothing to +// derive one from. An application's rows are in that population too, and for +// those the value is derivable, because menu_name is what identified them +// before the column existed. Without this the natural-key lookup misses them, +// the seed inserts a duplicate, and the unique index cannot object: NULL +// never collides. +// +// Ambiguity is refused rather than guessed. menuName concatenates two +// pascalCase strings and pascalCase is not injective, so two specs can land +// on one name; picking one of several rows would attach an application's +// menu to whichever the database returned first. +func adoptLegacyMenu(tx *gorm.DB, appCode string, s seed.MenuSpec) (models.SysMenu, bool, error) { + name := menuName(appCode, s.Code) + var rows []models.SysMenu + if err := tx.Where("app_code = ? AND menu_name = ? AND seed_code IS NULL", appCode, name). + Find(&rows).Error; err != nil { + return models.SysMenu{}, false, fmt.Errorf("looking for a row written before seed_code existed: %w", err) + } + switch len(rows) { + case 0: + return models.SysMenu{}, false, nil + case 1: + default: + return models.SysMenu{}, false, fmt.Errorf( + "%d rows carry menu_name %q with no seed_code; which of them belongs to %q cannot be decided here, because menuName is not reversible - reconcile them by hand", + len(rows), name, s.Code) + } + + seedCode := s.Code + if err := tx.Model(&models.SysMenu{}).Where("menu_id = ?", rows[0].MenuId). + Update("seed_code", seedCode).Error; err != nil { + return models.SysMenu{}, false, fmt.Errorf("claiming the row written before seed_code existed: %w", err) + } + rows[0].SeedCode = &seedCode + return rows[0], true, nil +} diff --git a/app/admin/service/seed_test.go b/app/admin/service/seed_test.go index 9826689b..983850f3 100644 --- a/app/admin/service/seed_test.go +++ b/app/admin/service/seed_test.go @@ -3,6 +3,7 @@ package service import ( "context" "errors" + "fmt" "strconv" "strings" "sync" @@ -1140,3 +1141,113 @@ func orderMenuSpecs(title, component string) ([]seed.MenuSpec, []seed.ApiSpec) { } return menus, apis } + +// 1786700008000 added seed_code and left it NULL on every row already there. +// An application's rows are in that population, and the natural-key lookup +// misses them, so the seed used to insert a second copy beside each one - +// which the unique index cannot object to, because NULL never collides. +func TestSeedMenusAdoptsARowWrittenBeforeSeedCodeExisted(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + // What an older SeedMenus left: app_code set, seed_code absent, and the + // name that identified it then. + legacy := models.SysMenu{ + MenuName: menuName("order", "dir"), AppCode: "order", Title: "the old title", + MenuType: contractmodels.Directory, Path: "/apps/order", Component: "Layout", Sort: 10, + } + if err := db.Create(&legacy).Error; err != nil { + t.Fatal(err) + } + + menus, apis := orderMenuSpecs("Orders", "apps/order/index") + if err := (adminSeeder{}).SeedMenus(db, "order", menus, apis); err != nil { + t.Fatalf("seed: %v", err) + } + + var rows []models.SysMenu + if err := db.Where("app_code = ? AND menu_name = ?", "order", menuName("order", "dir")). + Find(&rows).Error; err != nil { + t.Fatal(err) + } + if len(rows) != 1 { + t.Fatalf("%d rows carry that name; the row from before the column existed was not found", len(rows)) + } + if rows[0].MenuId != legacy.MenuId { + t.Errorf("menu_id = %d, want the row that was already there (%d)", rows[0].MenuId, legacy.MenuId) + } + if rows[0].SeedCode == nil || *rows[0].SeedCode != "dir" { + t.Errorf("seed_code = %v, want it claimed", rows[0].SeedCode) + } + // Adopted and then repaired, like any other existing row. + if rows[0].Title != "Order Example" { + t.Errorf("title = %q; the adopted row was not brought up to the spec", rows[0].Title) + } +} + +// menuName concatenates two pascalCase strings and pascalCase is not +// injective, so two specs can land on one name. Picking one of several rows +// would attach an application's menu to whichever the database returned +// first. +func TestSeedMenusRefusesAnAmbiguousAdoption(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + for i := 0; i < 2; i++ { + row := models.SysMenu{ + MenuName: menuName("order", "dir"), AppCode: "order", Title: fmt.Sprintf("copy %d", i), + MenuType: contractmodels.Directory, + } + if err := db.Create(&row).Error; err != nil { + t.Fatal(err) + } + } + + menus, apis := orderMenuSpecs("Orders", "apps/order/index") + err := (adminSeeder{}).SeedMenus(db, "order", menus, apis) + if err == nil { + t.Fatal("an ambiguous adoption was accepted") + } + if !strings.Contains(err.Error(), "2 rows") || !strings.Contains(err.Error(), "by hand") { + t.Errorf("error = %q, it has to say how many and that it is not deciding", err) + } + // And it did not write a third. + var n int64 + db.Model(&models.SysMenu{}).Where("app_code = ? AND menu_name = ?", "order", menuName("order", "dir")).Count(&n) + if n != 2 { + t.Errorf("%d rows carry that name; the refusal still inserted", n) + } +} + +// A row belonging to another application, or to the host, carries a different +// app_code and is not this application's to claim. +func TestSeedMenusDoesNotAdoptAnotherApplicationsRow(t *testing.T) { + db := newSeedTestDB(t) + useCompositeSeedCodeIndex(t, db) + seedAdminRole(t, db) + + other := models.SysMenu{ + MenuName: menuName("order", "dir"), AppCode: "crm", Title: "crm's own", + MenuType: contractmodels.Directory, + } + if err := db.Create(&other).Error; err != nil { + t.Fatal(err) + } + + menus, apis := orderMenuSpecs("Orders", "apps/order/index") + if err := (adminSeeder{}).SeedMenus(db, "order", menus, apis); err != nil { + t.Fatalf("seed: %v", err) + } + + var after models.SysMenu + db.Where("menu_id = ?", other.MenuId).First(&after) + if after.SeedCode != nil || after.Title != "crm's own" { + t.Errorf("another application's row was claimed: %+v", after) + } + var mine models.SysMenu + if err := db.Where("app_code = ? AND seed_code = ?", "order", "dir").First(&mine).Error; err != nil { + t.Fatalf("this application's own row was not created: %v", err) + } +}