fix🐛: write a table's configuration and its columns in one transaction

SysTables.Create and Update wrote each column with _, _ = …, so a column
the database rejected was dropped while the request reported success. And
because neither ran in a transaction, returning that error alone would not
have been enough: the table row and every column before the failing one
were already committed, leaving a table the config page lists with columns
missing and code generated from fewer columns than the table has.

Both now run the table and its columns in one transaction and return the
first column's error, named. Create rolls back to nothing; Update leaves the
table and every column as they were. Both callers already answer an error
with a 500.

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
 }

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)
+		}
+	}
+}