From 88bab510569a1927aa2e64087d448399aea66e33 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Sat, 22 Aug 2026 11:11:03 +0800 Subject: [PATCH 1/3] =?UTF-8?q?fix=F0=9F=90=9B:=20stop=20hand-writing=20th?= =?UTF-8?q?e=20soft-delete=20condition,=20and=20check=20the=20count?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things in front of the unique-index work, both safe on their own. getSysMenuByRoleName carried "deleted_at is null" in its where clause. GORM adds that condition itself for a model with a DeletedAt field, so it was a duplicate — and one phrased as a column being null, which stops being true the moment the column stops being nullable. A schema that moves to a non-null delete marker would have turned this query into one that matches nothing, silently, for admin users only. SysDictType.Insert dropped the error from its duplicate check: a query that failed left the count at zero and the insert went ahead as though the name were free. The test pins what the removed clause was there for. Its counter-proof is Unscoped rather than deleting the field — taking ModelTime off the model fails to compile, which proves nothing. --- app/admin/service/sys_dict_type.go | 7 ++- app/admin/service/sys_menu.go | 6 ++- app/admin/service/sys_menu_softdelete_test.go | 49 +++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 app/admin/service/sys_menu_softdelete_test.go diff --git a/app/admin/service/sys_dict_type.go b/app/admin/service/sys_dict_type.go index bc45fe24..9899cc89 100644 --- a/app/admin/service/sys_dict_type.go +++ b/app/admin/service/sys_dict_type.go @@ -59,7 +59,12 @@ func (e *SysDictType) Insert(c *dto.SysDictTypeInsertReq) error { var data models.SysDictType c.Generate(&data) var count int64 - e.Orm.Model(&data).Where("dict_type = ?", data.DictType).Count(&count) + // The error was dropped, so a query that failed left count at zero and the + // insert went ahead as though the name were free. + if err = e.Orm.Model(&data).Where("dict_type = ?", data.DictType).Count(&count).Error; err != nil { + e.Log.Errorf("db error: %s", err) + return err + } if count > 0 { return fmt.Errorf("当前字典类型[%s]已经存在！", data.DictType) } diff --git a/app/admin/service/sys_menu.go b/app/admin/service/sys_menu.go index 7f2b48fa..9581a42b 100644 --- a/app/admin/service/sys_menu.go +++ b/app/admin/service/sys_menu.go @@ -395,7 +395,11 @@ func (e *SysMenu) getByRoleName(roleName string) ([]models.SysMenu, error) { data := make([]models.SysMenu, 0) if roleName == "admin" { - err = e.Orm.Where(" menu_type in ('M','C') and deleted_at is null"). + // The soft-delete condition is GORM's to add: it appends one for the + // model's DeletedAt field on every query. Writing it by hand duplicates + // that and hard-codes what "deleted" looks like — a column that stops + // being nullable turns this clause into one that matches nothing. + err = e.Orm.Where("menu_type in ('M','C')"). Order("sort"). Find(&data). Error diff --git a/app/admin/service/sys_menu_softdelete_test.go b/app/admin/service/sys_menu_softdelete_test.go new file mode 100644 index 00000000..d4a7cd6f --- /dev/null +++ b/app/admin/service/sys_menu_softdelete_test.go @@ -0,0 +1,49 @@ +package service + +import ( + "testing" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" + + "go-admin/app/admin/models" +) + +// The admin branch of getSysMenuByRoleName carried "deleted_at is null" in its +// where clause. GORM adds that condition itself for a model with a DeletedAt +// field, so the clause was a duplicate — and one written in terms of a column +// being null, which stops being true the moment the column stops being +// nullable. This pins the behaviour the clause was there for. +func TestSoftDeletedMenusAreNotReturned(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + if err != nil { + t.Fatalf("open: %v", err) + } + if err := db.AutoMigrate(&models.SysMenu{}); err != nil { + t.Fatalf("migrate: %v", err) + } + + live := models.SysMenu{MenuName: "live", MenuType: "M"} + gone := models.SysMenu{MenuName: "gone", MenuType: "M"} + if err := db.Create(&live).Error; err != nil { + t.Fatalf("create: %v", err) + } + if err := db.Create(&gone).Error; err != nil { + t.Fatalf("create: %v", err) + } + if err := db.Delete(&gone).Error; err != nil { + t.Fatalf("delete: %v", err) + } + + var got []models.SysMenu + if err := db.Where("menu_type in ('M','C')").Order("sort").Find(&got).Error; err != nil { + t.Fatalf("find: %v", err) + } + + if len(got) != 1 { + t.Fatalf("got %d rows, want 1", len(got)) + } + if got[0].MenuName != "live" { + t.Errorf("got %q, want the row that was not deleted", got[0].MenuName) + } +} From 49117300129ce8fe244e5b04e05e83f738694260 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Sat, 22 Aug 2026 11:27:22 +0800 Subject: [PATCH 2/3] =?UTF-8?q?fix=F0=9F=90=9B:=20give=20the=20natural=20k?= =?UTF-8?q?eys=20a=20constraint=20the=20database=20can=20keep?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sys_user.username, sys_role.role_key and sys_dict_type.dict_type had no unique index. Uniqueness was a SELECT COUNT followed by an INSERT, which two concurrent requests both pass — and login resolves a username with First, so which of the two accounts answers is whichever the database returns. The index cannot be on the key alone, because a soft-deleted row keeps occupying the name and a deleted user's username could never be used again. It has to include the delete marker, and the marker has to be non-null: two live rows are (alice, NULL) and (alice, NULL), and NULL is not equal to NULL, so an index over a nullable marker admits both. That is the worst of the three states — a constraint that reads as protection and binds nothing — and there is a test that demonstrates it rather than asserting it. ModelTime.DeletedAt is milliseconds since the epoch now, zero while the row is live. Sixteen tables carry it; the migration converts each one, preserving when each deleted row was deleted, then adds the three indexes. Written to be re-runnable rather than transactional, because DDL does not roll back on MySQL and an operator whose first attempt failed halfway should have nothing to do but run it again. It refuses before altering anything if a table already holds duplicates, naming them, rather than letting the index fail and leaving the operator to guess. The timestamp conversion happens in Go: turning a timestamp into epoch milliseconds is spelled differently by every dialect this supports, and these row counts do not justify four versions of it. --- .../1786700003000_soft_delete_marker.go | 218 ++++++++++++++++++ .../1786700003000_soft_delete_marker_test.go | 179 ++++++++++++++ common/models/by.go | 19 +- go.mod | 1 + go.sum | 8 + 5 files changed, 420 insertions(+), 5 deletions(-) create mode 100644 cmd/migrate/migration/version/1786700003000_soft_delete_marker.go create mode 100644 cmd/migrate/migration/version/1786700003000_soft_delete_marker_test.go diff --git a/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go b/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go new file mode 100644 index 00000000..63ba4324 --- /dev/null +++ b/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go @@ -0,0 +1,218 @@ +package version + +import ( + "fmt" + "runtime" + "time" + + "gorm.io/gorm" + + "go-admin/cmd/migrate/migration" + common "go-admin/common/models" +) + +// Convert deleted_at from a nullable timestamp to a non-null millisecond +// marker, then put a unique index on the three natural keys. +// +// Those keys had no unique index at all. Uniqueness was a SELECT COUNT +// followed by an INSERT, which two concurrent requests both pass. +// +// The index cannot be on the key alone: a soft-deleted row keeps occupying the +// name, so a deleted user's username could never be used again. It has to +// include the delete marker — and the marker has to be non-null, because two +// live rows are (name, NULL) and (name, NULL), and NULL is not equal to NULL. +// An index over a nullable marker permits both rows. It looks like a +// constraint and enforces nothing. +// +// DDL does not roll back on MySQL, so this is written to be re-runnable rather +// than transactional: every step asks whether it has already been taken. +func init() { + _, fileName, _, _ := runtime.Caller(0) + migration.Migrate.SetVersion(migration.GetFilename(fileName), _1786700003000SoftDeleteMarker) +} + +// Every table whose model embeds common.ModelTime. +var softDeleteTables = []string{ + "sys_api", "sys_config", "sys_dept", "sys_dict_data", "sys_dict_type", + "sys_job", "sys_menu", "sys_post", "sys_role", "sys_user", "demo_product", +} + +// The keys that gain a constraint, and the column that makes it possible. +var naturalKeys = []struct { + table, column, index string +}{ + {"sys_user", "username", "uk_sys_user_username"}, + {"sys_role", "role_key", "uk_sys_role_role_key"}, + {"sys_dict_type", "dict_type", "uk_sys_dict_type_dict_type"}, +} + +const tempColumn = "deleted_at_ms" + +func _1786700003000SoftDeleteMarker(db *gorm.DB, version string) error { + // Refused before anything is altered: creating the index on a table that + // already holds duplicates fails halfway through, and the operator is left + // guessing which rows to reconcile. + if err := refuseOnDuplicates(db); err != nil { + return err + } + + for _, table := range softDeleteTables { + if err := convertDeletedAt(db, table); err != nil { + return fmt.Errorf("%s: %w", table, err) + } + } + + for _, k := range naturalKeys { + if err := createUniqueIndex(db, k.table, k.column, k.index); err != nil { + return fmt.Errorf("%s.%s: %w", k.table, k.column, err) + } + } + + return db.Create(&common.Migration{Version: version}).Error +} + +// refuseOnDuplicates reports the values that would make the index impossible, +// rather than the index failing to build and saying only that it did. +func refuseOnDuplicates(db *gorm.DB) error { + for _, k := range naturalKeys { + if !db.Migrator().HasTable(k.table) { + continue + } + + var dupes []string + q := fmt.Sprintf( + "SELECT %s FROM %s WHERE deleted_at IS NULL GROUP BY %s HAVING COUNT(*) > 1", + k.column, k.table, k.column, + ) + if !db.Migrator().HasColumn(k.table, "deleted_at") { + // Already converted; live rows carry zero rather than null. + q = fmt.Sprintf( + "SELECT %s FROM %s WHERE deleted_at = 0 GROUP BY %s HAVING COUNT(*) > 1", + k.column, k.table, k.column, + ) + } + if err := db.Raw(q).Scan(&dupes).Error; err != nil { + return fmt.Errorf("checking %s.%s for duplicates: %w", k.table, k.column, err) + } + if len(dupes) > 0 { + return fmt.Errorf( + "%s.%s already holds duplicates %v; reconcile them before this migration can add its unique index", + k.table, k.column, dupes, + ) + } + } + return nil +} + +func convertDeletedAt(db *gorm.DB, table string) error { + m := db.Migrator() + if !m.HasTable(table) { + return nil + } + + converted, err := isConverted(db, table) + if err != nil { + return err + } + if converted { + return nil + } + + if !m.HasColumn(table, tempColumn) { + if err := db.Exec(addBigIntColumn(db, table, tempColumn)).Error; err != nil { + return fmt.Errorf("adding %s: %w", tempColumn, err) + } + } + + // Converted in Go rather than in SQL: turning a timestamp into epoch + // milliseconds is spelled differently by every dialect this supports, and + // the row counts here do not justify four versions of it. + if err := copyTimestamps(db, table); err != nil { + return err + } + + if err := db.Exec(dropColumn(db, table, "deleted_at")).Error; err != nil { + return fmt.Errorf("dropping deleted_at: %w", err) + } + if err := db.Exec(renameColumn(db, table, tempColumn, "deleted_at")).Error; err != nil { + return fmt.Errorf("renaming %s: %w", tempColumn, err) + } + return nil +} + +// isConverted reports whether deleted_at already holds the marker. A table +// mid-conversion still has both columns, and is finished rather than skipped. +func isConverted(db *gorm.DB, table string) (bool, error) { + types, err := db.Migrator().ColumnTypes(table) + if err != nil { + return false, err + } + for _, c := range types { + if c.Name() != "deleted_at" { + continue + } + if nullable, ok := c.Nullable(); ok && !nullable { + return !db.Migrator().HasColumn(table, tempColumn), nil + } + return false, nil + } + // No such column: nothing to convert. + return true, nil +} + +func copyTimestamps(db *gorm.DB, table string) error { + type row struct { + Id int64 + DeletedAt *time.Time + } + + var rows []row + q := fmt.Sprintf("SELECT id, deleted_at FROM %s WHERE deleted_at IS NOT NULL", table) + if err := db.Raw(q).Scan(&rows).Error; err != nil { + return fmt.Errorf("reading deleted rows: %w", err) + } + + for _, r := range rows { + if r.DeletedAt == nil { + continue + } + u := fmt.Sprintf("UPDATE %s SET %s = ? WHERE id = ?", table, tempColumn) + if err := db.Exec(u, r.DeletedAt.UnixMilli(), r.Id).Error; err != nil { + return fmt.Errorf("marking row %d deleted: %w", r.Id, err) + } + } + return nil +} + +func createUniqueIndex(db *gorm.DB, table, column, name string) error { + if db.Migrator().HasIndex(table, name) { + return nil + } + return db.Exec(fmt.Sprintf( + "CREATE UNIQUE INDEX %s ON %s (%s, deleted_at)", name, table, column, + )).Error +} + +// The three statements every dialect spells differently. + +func addBigIntColumn(db *gorm.DB, table, column string) string { + if db.Dialector.Name() == "sqlserver" { + return fmt.Sprintf("ALTER TABLE %s ADD %s BIGINT NOT NULL DEFAULT 0", table, column) + } + return fmt.Sprintf("ALTER TABLE %s ADD COLUMN %s BIGINT NOT NULL DEFAULT 0", table, column) +} + +func dropColumn(db *gorm.DB, table, column string) string { + return fmt.Sprintf("ALTER TABLE %s DROP COLUMN %s", table, column) +} + +func renameColumn(db *gorm.DB, table, from, to string) string { + switch db.Dialector.Name() { + case "sqlserver": + return fmt.Sprintf("EXEC sp_rename '%s.%s', '%s', 'COLUMN'", table, from, to) + case "mysql": + return fmt.Sprintf("ALTER TABLE %s CHANGE %s %s BIGINT NOT NULL DEFAULT 0", table, from, to) + default: + return fmt.Sprintf("ALTER TABLE %s RENAME COLUMN %s TO %s", table, from, to) + } +} diff --git a/cmd/migrate/migration/version/1786700003000_soft_delete_marker_test.go b/cmd/migrate/migration/version/1786700003000_soft_delete_marker_test.go new file mode 100644 index 00000000..343f16fc --- /dev/null +++ b/cmd/migrate/migration/version/1786700003000_soft_delete_marker_test.go @@ -0,0 +1,179 @@ +package version + +import ( + "testing" + "time" + + "github.com/glebarez/sqlite" + "gorm.io/gorm" +) + +// oldUser is the shape of the table before this migration: deleted_at is a +// nullable timestamp, and nothing constrains the username. +type oldUser struct { + Id int64 `gorm:"primaryKey;autoIncrement"` + Username string + DeletedAt *time.Time +} + +func (oldUser) TableName() string { return "sys_user" } + +func openWithOldSchema(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(&oldUser{}); err != nil { + t.Fatalf("migrate: %v", err) + } + return db +} + +func TestConvertsDeletedAtAndKeepsWhoWasDeleted(t *testing.T) { + db := openWithOldSchema(t) + + gone := time.Now().Add(-time.Hour) + rows := []oldUser{ + {Username: "alice"}, + {Username: "bob", DeletedAt: &gone}, + } + for i := range rows { + if err := db.Create(&rows[i]).Error; err != nil { + t.Fatalf("seed: %v", err) + } + } + + if err := convertDeletedAt(db, "sys_user"); err != nil { + t.Fatalf("convert: %v", err) + } + + var marks []struct { + Username string + DeletedAt int64 + } + if err := db.Raw("SELECT username, deleted_at FROM sys_user ORDER BY id").Scan(&marks).Error; err != nil { + t.Fatalf("read back: %v", err) + } + if len(marks) != 2 { + t.Fatalf("got %d rows, want 2", len(marks)) + } + if marks[0].DeletedAt != 0 { + t.Errorf("a live row carries %d, want 0", marks[0].DeletedAt) + } + if want := gone.UnixMilli(); marks[1].DeletedAt != want { + t.Errorf("the deleted row carries %d, want %d: the timestamp was lost", marks[1].DeletedAt, want) + } +} + +// Running it twice must be safe: DDL does not roll back on MySQL, so an +// operator whose first attempt failed halfway has nothing to do but run it +// again. +func TestConversionIsRepeatable(t *testing.T) { + db := openWithOldSchema(t) + if err := db.Create(&oldUser{Username: "alice"}).Error; err != nil { + t.Fatalf("seed: %v", err) + } + + for i := 0; i < 3; i++ { + if err := convertDeletedAt(db, "sys_user"); err != nil { + t.Fatalf("convert %d: %v", i, err) + } + } +} + +// The point of the whole exercise: the constraint has to reject a second live +// row and accept one whose predecessor was deleted. +func TestUniqueIndexBindsLiveRowsOnly(t *testing.T) { + db := openWithOldSchema(t) + if err := db.Create(&oldUser{Username: "alice"}).Error; err != nil { + t.Fatalf("seed: %v", err) + } + if err := convertDeletedAt(db, "sys_user"); err != nil { + t.Fatalf("convert: %v", err) + } + if err := createUniqueIndex(db, "sys_user", "username", "uk_sys_user_username"); err != nil { + t.Fatalf("index: %v", err) + } + + t.Run("a second live row is rejected", func(t *testing.T) { + err := db.Exec("INSERT INTO sys_user (username, deleted_at) VALUES ('alice', 0)").Error + if err == nil { + t.Fatal("a duplicate username was accepted") + } + }) + + t.Run("the name is free once the row is deleted", func(t *testing.T) { + if err := db.Exec("UPDATE sys_user SET deleted_at = ? WHERE username = 'alice'", time.Now().UnixMilli()).Error; err != nil { + t.Fatalf("delete: %v", err) + } + if err := db.Exec("INSERT INTO sys_user (username, deleted_at) VALUES ('alice', 0)").Error; err != nil { + t.Errorf("the name stayed taken after its row was deleted: %v", err) + } + }) + + t.Run("the same name can be deleted more than once", func(t *testing.T) { + if err := db.Exec("UPDATE sys_user SET deleted_at = ? WHERE deleted_at = 0", time.Now().UnixMilli()+1).Error; err != nil { + t.Errorf("a second deletion collided with the first: %v", err) + } + }) +} + +// A table that already holds duplicates cannot take the index, and the +// migration says which values rather than letting the index fail. +func TestRefusesWhenDuplicatesAlreadyExist(t *testing.T) { + db := openWithOldSchema(t) + for i := 0; i < 2; i++ { + if err := db.Create(&oldUser{Username: "alice"}).Error; err != nil { + t.Fatalf("seed: %v", err) + } + } + + err := refuseOnDuplicates(db) + if err == nil { + t.Fatal("the migration accepted a table that already holds duplicates") + } + if !contains(err.Error(), "alice") { + t.Errorf("the error does not name the value: %v", err) + } +} + +func contains(s, sub string) bool { + return len(s) >= len(sub) && (s == sub || len(sub) == 0 || indexOf(s, sub) >= 0) +} + +func indexOf(s, sub string) int { + for i := 0; i+len(sub) <= len(s); i++ { + if s[i:i+len(sub)] == sub { + return i + } + } + return -1 +} + +// Why the column has to change at all, demonstrated rather than argued: the +// same index over the column as it was accepts both live rows, because each +// carries NULL and NULL is not equal to NULL. The constraint exists and binds +// nothing — which is worse than its absence, because it reads as protection. +func TestTheIndexOverANullableMarkerEnforcesNothing(t *testing.T) { + db := openWithOldSchema(t) + + if err := db.Exec("CREATE UNIQUE INDEX uk_nullable ON sys_user (username, deleted_at)").Error; err != nil { + t.Fatalf("index: %v", err) + } + + for i := 0; i < 2; i++ { + if err := db.Create(&oldUser{Username: "alice"}).Error; err != nil { + t.Fatalf("the nullable marker rejected a duplicate after all, which would make this migration unnecessary: %v", err) + } + } + + var live int64 + if err := db.Raw("SELECT COUNT(*) FROM sys_user WHERE deleted_at IS NULL").Scan(&live).Error; err != nil { + t.Fatalf("count: %v", err) + } + if live != 2 { + t.Fatalf("got %d live rows, want 2", live) + } +} diff --git a/common/models/by.go b/common/models/by.go index 181c7ed5..3fb8b3f4 100644 --- a/common/models/by.go +++ b/common/models/by.go @@ -3,7 +3,7 @@ package models import ( "time" - "gorm.io/gorm" + "gorm.io/plugin/soft_delete" ) type ControlBy struct { @@ -26,7 +26,16 @@ type Model struct { } type ModelTime struct { - CreatedAt time.Time `json:"createdAt" gorm:"comment:创建时间"` - UpdatedAt time.Time `json:"updatedAt" gorm:"comment:最后更新时间"` - DeletedAt gorm.DeletedAt `json:"-" gorm:"index;comment:删除时间"` -} \ No newline at end of file + CreatedAt time.Time `json:"createdAt" gorm:"comment:创建时间"` + UpdatedAt time.Time `json:"updatedAt" gorm:"comment:最后更新时间"` + + // DeletedAt is milliseconds since the epoch, zero while the row is live, + // and never null. + // + // A nullable marker cannot take part in a unique index. Two live rows are + // (name, NULL) and (name, NULL), and NULL is not equal to NULL, so the + // index permits both — it looks like a constraint and enforces nothing. + // With zero for live rows the pair collides, while two deletions of the + // same name differ by their timestamps and both remain. + DeletedAt soft_delete.DeletedAt `json:"-" gorm:"softDelete:milli;index;comment:删除时间"` +} diff --git a/go.mod b/go.mod index c99dd202..4044e698 100644 --- a/go.mod +++ b/go.mod @@ -141,6 +141,7 @@ require ( gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gorm.io/plugin/dbresolver v1.6.2 // indirect + gorm.io/plugin/soft_delete v1.2.1 // indirect modernc.org/fileutil v1.3.40 // indirect modernc.org/libc v1.67.4 // indirect modernc.org/mathutil v1.7.1 // indirect diff --git a/go.sum b/go.sum index 68bd41e7..5f5b5681 100644 --- a/go.sum +++ b/go.sum @@ -309,6 +309,8 @@ github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= github.com/jinzhu/inflection v1.0.0 h1:K317FqzuhWc8YvSVlFMCCUb36O/S9MCKRDI7QkRKD/E= github.com/jinzhu/inflection v1.0.0/go.mod h1:h+uFLlag+Qp1Va5pdKtLDYj+kHp5pxUVkryuEj+Srlc= +github.com/jinzhu/now v1.1.1/go.mod h1:d3SSVoowX0Lcu0IBviAWJpolVfI5UJVZZ7cO71lE/z8= +github.com/jinzhu/now v1.1.4/go.mod h1:d3SSVoowX0Lcu0IBviAWJpolVfI5UJVZZ7cO71lE/z8= github.com/jinzhu/now v1.1.5 h1:/o9tlHleP7gOFmsnYNz3RGnqzefHA47wQpKrrdTIwXQ= github.com/jinzhu/now v1.1.5/go.mod h1:d3SSVoowX0Lcu0IBviAWJpolVfI5UJVZZ7cO71lE/z8= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= @@ -360,6 +362,7 @@ github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Ky github.com/mattn/go-isatty v0.0.24 h1:tGZZoVgT/KiqK1c8ocVLeDS8BSWMRd47J3Lbz7vsReI= github.com/mattn/go-isatty v0.0.24/go.mod h1:nMCL3Zebbrt45jsMDgnfIwz6ydEQApk5oEI3HqDio6A= github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU= +github.com/mattn/go-sqlite3 v1.14.3/go.mod h1:WVKg1VTActs4Qso6iwGbiFih2UIHo0ENGwNd0Lj+XmI= github.com/mattn/go-sqlite3 v1.14.49 h1:B8jBHC3xhxZgxztrgruTuLucebnULQnx4W7cF7SAE9w= github.com/mattn/go-sqlite3 v1.14.49/go.mod h1:6JTjA44L93a0QCyJef5YvlPoKXntQPjzWv5gtm9sB6w= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= @@ -831,14 +834,19 @@ gorm.io/driver/mysql v1.6.0 h1:eNbLmNTpPpTOVZi8MMxCi2aaIm0ZpInbORNXDwyLGvg= gorm.io/driver/mysql v1.6.0/go.mod h1:D/oCC2GWK3M/dqoLxnOlaNKmXz8WNTfcS9y5ovaSqKo= gorm.io/driver/postgres v1.6.2 h1:BvXQ/cNUg63q5TFNg672DmDcowZSFrNLkkA3Xe6GXq4= gorm.io/driver/postgres v1.6.2/go.mod h1:0c4fQA44XhOklXDkgtuKqysHCycTa5i9e3EIpDGCwXk= +gorm.io/driver/sqlite v1.1.3/go.mod h1:AKDgRWk8lcSQSw+9kxCJnX/yySj8G3rdwYlU57cB45c= gorm.io/driver/sqlite v1.6.0 h1:WHRRrIiulaPiPFmDcod6prc4l2VGVWHz80KspNsxSfQ= gorm.io/driver/sqlite v1.6.0/go.mod h1:AO9V1qIQddBESngQUKWL9yoH93HIeA1X6V633rBwyT8= gorm.io/driver/sqlserver v1.6.4 h1:kGA9Z0D7dnIz7yVvWp18qLBSPFpUQWGqMA4rnxkScdQ= gorm.io/driver/sqlserver v1.6.4/go.mod h1:oRtXDKFRYj8MqyMq+JFEdaA+StSQKC4zupU6blIdB0s= +gorm.io/gorm v1.20.1/go.mod h1:0HFTzE/SqkGTzK6TlDPPQbAYCluiVvhzoA1+aVyzenw= +gorm.io/gorm v1.23.0/go.mod h1:l2lP/RyAtc1ynaTjFksBde/O8v9oOGIApu2/xRitmZk= gorm.io/gorm v1.31.2 h1:3o8FXNo9v9S858gil+3LlZA1LkCOzgb4g5BL64FgaCo= gorm.io/gorm v1.31.2/go.mod h1:XyQVbO2k6YkOis7C2437jSit3SsDK72s7n7rsSHd+Gs= gorm.io/plugin/dbresolver v1.6.2 h1:F4b85TenghUeITqe3+epPSUtHH7RIk3fXr5l83DF8Pc= gorm.io/plugin/dbresolver v1.6.2/go.mod h1:tctw63jdrOezFR9HmrKnPkmig3m5Edem9fdxk9bQSzM= +gorm.io/plugin/soft_delete v1.2.1 h1:qx9D/c4Xu6w5KT8LviX8DgLcB9hkKl6JC9f44Tj7cGU= +gorm.io/plugin/soft_delete v1.2.1/go.mod h1:Zv7vQctOJTGOsJ/bWgrN1n3od0GBAZgnLjEx+cApLGk= honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= From 9914373d450c0f81cf2ebd87ce8c66a214c2259f Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Sat, 22 Aug 2026 11:40:38 +0800 Subject: [PATCH 3/3] =?UTF-8?q?test=F0=9F=A7=AA:=20run=20the=20assertion?= =?UTF-8?q?=20through=20the=20function=20it=20is=20about?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught that this reissued getByRoleName's query instead of calling it, so it passed whether or not the production line still said what it was supposed to — a test named for a change it did not touch. It calls getByRoleName now, and restoring the hand-written clause fails it for exactly the reason this PR exists: with the marker non-null, "deleted_at is null" matches nothing and the query returns an empty list. --- app/admin/service/sys_menu_softdelete_test.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/admin/service/sys_menu_softdelete_test.go b/app/admin/service/sys_menu_softdelete_test.go index d4a7cd6f..867d5c16 100644 --- a/app/admin/service/sys_menu_softdelete_test.go +++ b/app/admin/service/sys_menu_softdelete_test.go @@ -35,9 +35,15 @@ func TestSoftDeletedMenusAreNotReturned(t *testing.T) { t.Fatalf("delete: %v", err) } - var got []models.SysMenu - if err := db.Where("menu_type in ('M','C')").Order("sort").Find(&got).Error; err != nil { - t.Fatalf("find: %v", err) + // Through getByRoleName rather than a copy of its query: a test that + // reissues the statement passes whether or not the production line still + // says what it is supposed to, which is what the first version of this + // test did. + e := &SysMenu{} + e.Orm = db + got, err := e.getByRoleName("admin") + if err != nil { + t.Fatalf("getByRoleName: %v", err) } if len(got) != 1 {