diff --git a/app/other/models/tools/sys_tables.go b/app/other/models/tools/sys_tables.go index 4417a3e0..9dd2de9a 100644 --- a/app/other/models/tools/sys_tables.go +++ b/app/other/models/tools/sys_tables.go @@ -1,6 +1,7 @@ package tools import ( + "fmt" common "go-admin/common/models" "strings" @@ -132,22 +133,29 @@ func (e *SysTables) GetTree(tx *gorm.DB) ([]SysTables, error) { return doc, nil } +// Create inserts the table and every one of its columns, or none of them. A +// column that failed to insert used to be dropped without a word, leaving a +// table the config page lists with columns missing and code generated from +// fewer columns than the table has. func (e *SysTables) Create(tx *gorm.DB) (SysTables, error) { var doc SysTables e.CreateBy = 0 - result := tx.Table("sys_tables").Create(&e) - if result.Error != nil { - err := result.Error + err := tx.Transaction(func(tx *gorm.DB) error { + if err := tx.Table("sys_tables").Create(&e).Error; err != nil { + return err + } + for i := range e.Columns { + e.Columns[i].TableId = e.TableId + if _, err := e.Columns[i].Create(tx); err != nil { + return fmt.Errorf("column %s: %w", e.Columns[i].ColumnName, err) + } + } + return nil + }) + if err != nil { return doc, err } - doc = *e - for i := 0; i < len(e.Columns); i++ { - e.Columns[i].TableId = doc.TableId - - _, _ = e.Columns[i].Create(tx) - } - - return doc, nil + return *e, nil } func (e *SysTables) Update(tx *gorm.DB) (update SysTables, err error) { @@ -158,6 +166,16 @@ func (e *SysTables) Update(tx *gorm.DB) (update SysTables, err error) { //参数1:是要修改的数据 //参数2:是修改的数据 e.UpdateBy = 0 + // One transaction for the table and its columns, as in Create: a column + // whose write failed used to be skipped while the request reported + // success, so the config page showed a save that had not happened. + err = tx.Transaction(func(tx *gorm.DB) error { + return e.updateWithColumns(tx) + }) + return +} + +func (e *SysTables) updateWithColumns(tx *gorm.DB) (err error) { if err = tx.Table("sys_tables").Where("table_id = ?", e.TableId).Updates(&e).Error; err != nil { return } @@ -199,7 +217,9 @@ func (e *SysTables) Update(tx *gorm.DB) (update SysTables, err error) { } } } - _, _ = e.Columns[i].Update(tx) + if _, err = e.Columns[i].Update(tx); err != nil { + return fmt.Errorf("column %s: %w", e.Columns[i].ColumnName, err) + } } return } diff --git a/app/other/models/tools/sys_tables_atomic_test.go b/app/other/models/tools/sys_tables_atomic_test.go new file mode 100644 index 00000000..9d1ebdcb --- /dev/null +++ b/app/other/models/tools/sys_tables_atomic_test.go @@ -0,0 +1,102 @@ +package tools + +import ( + "errors" + "testing" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" +) + +// A named, shared in-memory database: with a bare ":memory:" every pooled +// connection opens its own empty database, and a row written inside a +// transaction would be invisible to the query that checks for it. +func openAtomicDB(t *testing.T) *gorm.DB { + t.Helper() + db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{ + Logger: logger.Default.LogMode(logger.Silent), + }) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + if err := db.AutoMigrate(new(SysTables), new(SysColumns)); err != nil { + t.Fatalf("migrate: %v", err) + } + return db +} + +// rejectNthColumnWrite makes the n-th write of a sys_columns row fail, the way +// it does when the database refuses it. +func rejectNthColumnWrite(register func(name string, fn func(*gorm.DB)) error, n int) error { + seen := 0 + return register("test:reject_column", func(tx *gorm.DB) { + if tx.Statement.Table != "sys_columns" { + return + } + seen++ + if seen == n { + _ = tx.AddError(errors.New("column rejected")) + } + }) +} + +func threeColumns() []SysColumns { + return []SysColumns{{ColumnName: "id"}, {ColumnName: "name"}, {ColumnName: "note"}} +} + +func TestSysTablesCreateWritesAllColumnsOrNone(t *testing.T) { + db := openAtomicDB(t) + if err := rejectNthColumnWrite(db.Callback().Create().Before("gorm:create").Register, 2); err != nil { + t.Fatal(err) + } + + table := SysTables{TBName: "orders", Columns: threeColumns()} + if _, err := table.Create(db); err == nil { + t.Fatal("Create reported success although a column was rejected") + } + + var tables, columns int64 + db.Model(new(SysTables)).Where("table_name = ?", "orders").Count(&tables) + db.Model(new(SysColumns)).Count(&columns) + if tables != 0 || columns != 0 { + t.Errorf("left %d table row(s) and %d column row(s) behind; want none", tables, columns) + } +} + +func TestSysTablesUpdateWritesAllColumnsOrNone(t *testing.T) { + db := openAtomicDB(t) + table := SysTables{TBName: "orders", TableComment: "before", Columns: threeColumns()} + created, err := table.Create(db) + if err != nil { + t.Fatal(err) + } + if err := rejectNthColumnWrite(db.Callback().Update().Before("gorm:update").Register, 2); err != nil { + t.Fatal(err) + } + + edited, err := (&SysTables{TableId: created.TableId}).Get(db, false) + if err != nil { + t.Fatal(err) + } + edited.TableComment = "after" + for i := range edited.Columns { + edited.Columns[i].ColumnComment = "after" + } + if _, err := edited.Update(db); err == nil { + t.Fatal("Update reported success although a column was rejected") + } + + stored, err := (&SysTables{TableId: created.TableId}).Get(db, false) + if err != nil { + t.Fatal(err) + } + if stored.TableComment != "before" { + t.Errorf("table comment = %q; the table row was kept although a column failed", stored.TableComment) + } + for _, c := range stored.Columns { + if c.ColumnComment != "" { + t.Errorf("column %s comment = %q; a column write was kept although another failed", c.ColumnName, c.ColumnComment) + } + } +}