From ea2d80707123c934993d84d75911fab483b09fe4 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Sun, 27 Sep 2026 20:45:45 +0800 Subject: [PATCH 1/6] =?UTF-8?q?fix=F0=9F=90=9B:=20give=20queries=20the=20r?= =?UTF-8?q?equest's=20context,=20not=20the=20pooled=20gin.Context?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WithContextDb and the five CRUD actions passed the *gin.Context itself to GORM as the query's context. database/sql watches the context of a query that returns rows inside a transaction from a goroutine of its own, which can still read it after the handler returns - and gin hands that Context to the next request as soon as the handler returns. GORM wraps every write in a transaction, and an INSERT on SQLite or PostgreSQL returns the new key as a row, so any write raced with the next request's reset. The race detector reports it on the second of two sequential requests. They now pass c.Request.Context(), which belongs to one request only and is cancelled when its client goes away. --- common/actions/create.go | 2 +- common/actions/delete.go | 2 +- common/actions/index.go | 2 +- common/actions/update.go | 2 +- common/actions/view.go | 2 +- common/middleware/db.go | 2 +- common/middleware/db_test.go | 60 ++++++++++++++++++++++++++++++++++++ 7 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 common/middleware/db_test.go diff --git a/common/actions/create.go b/common/actions/create.go index f667fc97..9fe75bd7 100644 --- a/common/actions/create.go +++ b/common/actions/create.go @@ -37,7 +37,7 @@ func CreateAction(control dto.Control) gin.HandlerFunc { return } object.SetCreateBy(user.GetUserId(c)) - err = db.WithContext(c).Create(object).Error + err = db.WithContext(c.Request.Context()).Create(object).Error if err != nil { log.Errorf("Create error: %s", err) response.Error(c, 500, err, "创建失败") diff --git a/common/actions/delete.go b/common/actions/delete.go index b8e992b6..530ec997 100644 --- a/common/actions/delete.go +++ b/common/actions/delete.go @@ -43,7 +43,7 @@ func DeleteAction(control dto.Control) gin.HandlerFunc { //数据权限检查 p := GetPermissionFromContext(c) - db = db.WithContext(c).Scopes( + db = db.WithContext(c.Request.Context()).Scopes( Permission(object.TableName(), p), ).Where(req.GetId()).Delete(object) if err = db.Error; err != nil { diff --git a/common/actions/index.go b/common/actions/index.go index 17758c65..eb9afb40 100644 --- a/common/actions/index.go +++ b/common/actions/index.go @@ -39,7 +39,7 @@ func IndexAction(m models.ActiveRecord, d dto.Index, f func() interface{}) gin.H //数据权限检查 p := GetPermissionFromContext(c) - err = db.WithContext(c).Model(object). + err = db.WithContext(c.Request.Context()).Model(object). Scopes( dto.MakeCondition(req.GetNeedSearch()), dto.Paginate(req.GetPageSize(), req.GetPageIndex()), diff --git a/common/actions/update.go b/common/actions/update.go index 2c0afde6..3ce96d07 100644 --- a/common/actions/update.go +++ b/common/actions/update.go @@ -41,7 +41,7 @@ func UpdateAction(control dto.Control) gin.HandlerFunc { //数据权限检查 p := GetPermissionFromContext(c) - db = db.WithContext(c).Scopes( + db = db.WithContext(c.Request.Context()).Scopes( Permission(object.TableName(), p), ).Where(req.GetId()).Updates(object) if err = db.Error; err != nil { diff --git a/common/actions/view.go b/common/actions/view.go index 03d26821..3f3a0f2f 100644 --- a/common/actions/view.go +++ b/common/actions/view.go @@ -48,7 +48,7 @@ func ViewAction(control dto.Control, f func() interface{}) gin.HandlerFunc { //数据权限检查 p := GetPermissionFromContext(c) - err = db.Model(object).WithContext(c).Scopes( + err = db.Model(object).WithContext(c.Request.Context()).Scopes( Permission(object.TableName(), p), ).Where(req.GetId()).First(rsp).Error diff --git a/common/middleware/db.go b/common/middleware/db.go index 905eb04e..06d2bcc7 100644 --- a/common/middleware/db.go +++ b/common/middleware/db.go @@ -6,6 +6,6 @@ import ( ) func WithContextDb(c *gin.Context) { - c.Set("db", sdk.Runtime.GetDbByTenant(c.Request.Host).WithContext(c)) + c.Set("db", sdk.Runtime.GetDbByTenant(c.Request.Host).WithContext(c.Request.Context())) c.Next() } diff --git a/common/middleware/db_test.go b/common/middleware/db_test.go new file mode 100644 index 00000000..c150fb76 --- /dev/null +++ b/common/middleware/db_test.go @@ -0,0 +1,60 @@ +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/glebarez/sqlite" + "github.com/go-admin-team/go-admin-core/v2/sdk" + "gorm.io/gorm" +) + +// database/sql watches the context of a query that returns rows inside a +// transaction from a goroutine of its own, which can still be reading it +// after the handler has returned. GORM wraps every write in a transaction, +// and an INSERT on SQLite or PostgreSQL returns the new key as a row. gin +// reuses its Context for the next request as soon as the handler returns, so +// a query given the gin.Context races with that reuse; given the request's +// own context, it does not. Sequential requests are enough for the reuse, +// and -race, which make test runs with, reports it. +type probeRow struct { + Id int `gorm:"primaryKey;autoIncrement"` +} + +func (probeRow) TableName() string { return "with_context_db_probe" } + +func TestWithContextDbDoesNotHandQueriesThePooledContext(t *testing.T) { + const host = "with-context-db.test" + db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{}) + if err != nil { + t.Fatal(err) + } + if err := db.AutoMigrate(&probeRow{}); err != nil { + t.Fatal(err) + } + previous := sdk.Runtime.GetDbByTenant(host) + sdk.Runtime.SetDbByTenant(host, db) + t.Cleanup(func() { sdk.Runtime.SetDbByTenant(host, previous) }) + + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(WithContextDb) + r.GET("/", func(c *gin.Context) { + if err := c.MustGet("db").(*gorm.DB).Create(&probeRow{}).Error; err != nil { + c.Status(http.StatusInternalServerError) + return + } + c.Status(http.StatusOK) + }) + for i := 0; i < 20; i++ { + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Host = host + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("request %d answered %d", i, w.Code) + } + } +} From 024dd047ae348fda358d110bd1c2ee7fef30998c Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Sun, 27 Sep 2026 20:46:01 +0800 Subject: [PATCH 2/6] =?UTF-8?q?feat=E2=9C=A8:=20add=20generic=20CRUD=20act?= =?UTF-8?q?ions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Index, View, ViewAs, Create, Update and Delete take the model and the request as type parameters. They serve the same routes as IndexAction and its siblings and answer them the same way; what they drop is what the caller had to get right. The older actions serve every request from instances passed at registration, so each model and request type needs a Generate that returns a copy, and a list route takes a func() interface{} whose element type nothing checks. Here every request declares its own values, and a model paired with the wrong request does not compile. ViewAs answers a detail route with a type other than the model, as ViewAction's f did. Beyond that they differ in three places, all where the older ones were wrong: a request with no database is answered with 500 instead of an empty 200; the key is matched against the primary-key column as a value, not handed to Where, which reads a string as SQL; and there is no GenerateM whose error could be dropped. A test runs the old and new actions on the same types through one request script and requires every response to match. --- common/actions/generic.go | 294 +++++++++++++++++++++++++++++++++ common/actions/generic_test.go | 257 ++++++++++++++++++++++++++++ 2 files changed, 551 insertions(+) create mode 100644 common/actions/generic.go create mode 100644 common/actions/generic_test.go diff --git a/common/actions/generic.go b/common/actions/generic.go new file mode 100644 index 00000000..10c62ae7 --- /dev/null +++ b/common/actions/generic.go @@ -0,0 +1,294 @@ +package actions + +import ( + "errors" + "net/http" + "reflect" + + "github.com/gin-gonic/gin" + "github.com/go-admin-team/go-admin-core/v2/jwtauth/user" + "github.com/go-admin-team/go-admin-core/v2/response" + "github.com/go-admin-team/go-admin-core/v2/sdk/api" + "github.com/go-admin-team/go-admin-core/v2/sdk/pkg" + "gorm.io/gorm" + "gorm.io/gorm/clause" + "gorm.io/gorm/schema" + + "go-admin/common/dto" +) + +// The generic CRUD actions. They serve the same five kinds of route as +// IndexAction and its siblings, and answer them the same way; what changes is +// what the caller has to get right. +// +// The older actions take instances at registration and serve every request +// from them, so each model and DTO has to implement a Generate that returns a +// copy - return the receiver and concurrent requests share one struct - and a +// list route takes a func() interface{} whose element type nothing checks. +// Here each request declares its own values of the types it was given, so +// neither convention exists, and a model paired with the wrong DTO does not +// compile. +// +// Three things differ from the older actions, all on paths they got wrong: +// +// - A request whose database connection cannot be found is answered with +// 500. The older actions logged it and wrote nothing, which reaches the +// client as an empty 200. +// - The key is matched against the model's primary-key column as a value. +// The older actions passed GetId() to Where, which GORM reads as a SQL +// condition when it is a string. +// - There is no GenerateM, so no error of its to drop. +// +// The type constraints are unexported: callers never name them, and an +// exported constraint would be a promise that could not be taken back. + +// record is a model the actions can create, read, change and delete. +type record[T any] interface { + *T + schema.Tabler + SetCreateBy(int) + SetUpdateBy(int) + GetId() any +} + +// searchReq is a list request: it binds itself, reports its page, and hands +// back the struct whose search tags MakeCondition turns into WHERE clauses. +type searchReq[S any] interface { + *S + Bind(*gin.Context) error + GetPageIndex() int + GetPageSize() int + GetNeedSearch() any +} + +// idReq is a request naming rows by key: one from the URI, or several from a +// DELETE body, as dto.ObjectById binds them. +type idReq[I any] interface { + *I + Bind(*gin.Context) error + GetId() any +} + +// controlReq is a create or update request: it binds itself and builds the +// model row it describes. +type controlReq[T, C any] interface { + *C + Bind(*gin.Context) error + ToModel() (*T, error) +} + +// Index lists T, filtered by S and by the caller's data permission. +func Index[T, S any, PT record[T], PS searchReq[S]]() gin.HandlerFunc { + return func(c *gin.Context) { + log := api.GetRequestLogger(c) + db, ok := genericOrm(c) + if !ok { + return + } + msgID := pkg.GenerateMsgIDFromContext(c) + + var req S + if err := PS(&req).Bind(c); err != nil { + response.Error(c, http.StatusUnprocessableEntity, err, "参数验证失败") + return + } + var object T + list := make([]T, 0) + var count int64 + p := GetPermissionFromContext(c) + err := db.WithContext(c.Request.Context()).Model(&object). + Scopes( + dto.MakeCondition(PS(&req).GetNeedSearch()), + dto.Paginate(PS(&req).GetPageSize(), PS(&req).GetPageIndex()), + Permission(PT(&object).TableName(), p), + ). + Find(&list).Limit(-1).Offset(-1). + Count(&count).Error + if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { + log.Errorf("MsgID[%s] Index error: %s", msgID, err) + response.Error(c, 500, err, "查询失败") + return + } + response.PageOK(c, &list, int(count), PS(&req).GetPageIndex(), PS(&req).GetPageSize(), "查询成功") + c.Next() + } +} + +// View answers one T, named by I's key, if the caller's data permission +// reaches it. +func View[T, I any, PT record[T], PI idReq[I]]() gin.HandlerFunc { + return ViewAs[T, I, T, PT, PI]() +} + +// ViewAs is View answering with R instead of T: the row is looked up as T and +// scanned into R, for a detail route that shows a different shape than the +// model stores. +func ViewAs[T, I, R any, PT record[T], PI idReq[I]]() gin.HandlerFunc { + return func(c *gin.Context) { + log := api.GetRequestLogger(c) + db, ok := genericOrm(c) + if !ok { + return + } + msgID := pkg.GenerateMsgIDFromContext(c) + + var req I + if err := PI(&req).Bind(c); err != nil { + response.Error(c, http.StatusUnprocessableEntity, err, "参数验证失败") + return + } + var object T + var rsp R + p := GetPermissionFromContext(c) + err := db.Model(&object).WithContext(c.Request.Context()).Scopes( + Permission(PT(&object).TableName(), p), + ).Where(keyIs(PI(&req).GetId())).First(&rsp).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + response.Error(c, http.StatusNotFound, nil, "查看对象不存在或无权查看") + return + } + if err != nil { + log.Errorf("MsgID[%s] View error: %s", msgID, err) + response.Error(c, 500, err, "查看失败") + return + } + response.OK(c, &rsp, "查询成功") + c.Next() + } +} + +// Create inserts the T that C builds, recording the caller as its creator. +func Create[T, C any, PT record[T], PC controlReq[T, C]]() gin.HandlerFunc { + return func(c *gin.Context) { + log := api.GetRequestLogger(c) + db, ok := genericOrm(c) + if !ok { + return + } + + var req C + if err := PC(&req).Bind(c); err != nil { + response.Error(c, http.StatusUnprocessableEntity, err, err.Error()) + return + } + object, err := PC(&req).ToModel() + if err != nil { + response.Error(c, 500, err, "模型生成失败") + return + } + PT(object).SetCreateBy(user.GetUserId(c)) + if err = db.WithContext(c.Request.Context()).Create(object).Error; err != nil { + log.Errorf("Create error: %s", err) + response.Error(c, 500, err, "创建失败") + return + } + response.OK(c, PT(object).GetId(), "创建成功") + c.Next() + } +} + +// Update writes the T that C builds over the row with its key, if the +// caller's data permission reaches that row. +func Update[T, C any, PT record[T], PC controlReq[T, C]]() gin.HandlerFunc { + return func(c *gin.Context) { + log := api.GetRequestLogger(c) + db, ok := genericOrm(c) + if !ok { + return + } + msgID := pkg.GenerateMsgIDFromContext(c) + + var req C + if err := PC(&req).Bind(c); err != nil { + response.Error(c, http.StatusUnprocessableEntity, err, "参数验证失败") + return + } + object, err := PC(&req).ToModel() + if err != nil { + response.Error(c, 500, err, "模型生成失败") + return + } + PT(object).SetUpdateBy(user.GetUserId(c)) + + p := GetPermissionFromContext(c) + result := db.WithContext(c.Request.Context()).Scopes( + Permission(PT(object).TableName(), p), + ).Where(keyIs(PT(object).GetId())).Updates(object) + if err = result.Error; err != nil { + log.Errorf("MsgID[%s] Update error: %s", msgID, err) + response.Error(c, 500, err, "更新失败") + return + } + if result.RowsAffected == 0 { + response.Error(c, http.StatusForbidden, nil, "无权更新该数据") + return + } + response.OK(c, PT(object).GetId(), "更新成功") + c.Next() + } +} + +// Delete removes the rows I names, where the caller's data permission +// reaches them. +func Delete[T, I any, PT record[T], PI idReq[I]]() gin.HandlerFunc { + return func(c *gin.Context) { + log := api.GetRequestLogger(c) + db, ok := genericOrm(c) + if !ok { + return + } + msgID := pkg.GenerateMsgIDFromContext(c) + + var req I + if err := PI(&req).Bind(c); err != nil { + log.Errorf("MsgID[%s] Bind error: %s", msgID, err) + response.Error(c, http.StatusUnprocessableEntity, err, "参数验证失败") + return + } + var object T + PT(&object).SetUpdateBy(user.GetUserId(c)) + + p := GetPermissionFromContext(c) + result := db.WithContext(c.Request.Context()).Scopes( + Permission(PT(&object).TableName(), p), + ).Where(keyIs(PI(&req).GetId())).Delete(&object) + if err := result.Error; err != nil { + log.Errorf("MsgID[%s] Delete error: %s", msgID, err) + response.Error(c, 500, err, "删除失败") + return + } + if result.RowsAffected == 0 { + response.Error(c, http.StatusForbidden, nil, "无权删除该数据") + return + } + // The key of the empty model, as DeleteAction answers: the rows are + // named by the request, not by a model. + response.OK(c, PT(&object).GetId(), "删除成功") + c.Next() + } +} + +// genericOrm reads the request's database, answering 500 when there is none. +func genericOrm(c *gin.Context) (*gorm.DB, bool) { + db, err := pkg.GetOrm(c) + if err != nil { + api.GetRequestLogger(c).Error(err) + response.Error(c, 500, err, "数据库连接获取失败") + return nil, false + } + return db, true +} + +// keyIs matches the primary-key column against id as a value, or against +// each element when id is a slice or array. +func keyIs(id any) clause.Expression { + v := reflect.ValueOf(id) + if v.Kind() == reflect.Slice || v.Kind() == reflect.Array { + values := make([]any, v.Len()) + for i := range values { + values[i] = v.Index(i).Interface() + } + return clause.IN{Column: clause.PrimaryColumn, Values: values} + } + return clause.Eq{Column: clause.PrimaryColumn, Value: id} +} diff --git a/common/actions/generic_test.go b/common/actions/generic_test.go new file mode 100644 index 00000000..208c33f6 --- /dev/null +++ b/common/actions/generic_test.go @@ -0,0 +1,257 @@ +package actions_test + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "regexp" + "strings" + "sync" + "testing" + + "github.com/gin-gonic/gin" + "github.com/glebarez/sqlite" + "github.com/go-admin-team/go-admin-core/v2/jwtauth" + "github.com/go-admin-team/go-admin-core/v2/sdk/config" + "gorm.io/gorm" + gormlogger "gorm.io/gorm/logger" + + "go-admin/common/actions" + "go-admin/common/dto" + "go-admin/common/models" +) + +// probeSearch is probeIndexReq with a search field, so a list request can +// be told apart from another in its response. +type probeSearch struct { + dto.Pagination `search:"-"` + Name string `form:"name" search:"type:exact;column:name;table:action_probe_row"` +} + +func (p *probeSearch) Generate() dto.Index { o := *p; return &o } +func (p *probeSearch) Bind(c *gin.Context) error { return c.ShouldBind(p) } +func (p *probeSearch) GetNeedSearch() interface{} { return *p } + +// probeById names rows the way every ById DTO here does. +type probeById struct { + dto.ObjectById +} + +func (s *probeById) Generate() dto.Control { o := *s; return &o } +func (s *probeById) GenerateM() (models.ActiveRecord, error) { return &probeRow{}, nil } + +// probeControl carries both the old actions' methods and ToModel, so one +// request type serves both generations of action. +type probeControl struct { + Id int `json:"id"` + Name string `json:"name"` +} + +func (s *probeControl) Bind(c *gin.Context) error { return c.ShouldBindJSON(s) } +func (s *probeControl) Generate() dto.Control { o := *s; return &o } +func (s *probeControl) GetId() interface{} { return s.Id } +func (s *probeControl) GenerateM() (models.ActiveRecord, error) { + return &probeRow{Model: models.Model{Id: s.Id}, Name: s.Name}, nil +} +func (s *probeControl) ToModel() (*probeRow, error) { + return &probeRow{Model: models.Model{Id: s.Id}, Name: s.Name}, nil +} + +// probeKey names a row by a string key, which the old actions handed to GORM +// as a SQL condition. +type probeKey struct { + Key string `uri:"id"` +} + +func (s *probeKey) Bind(c *gin.Context) error { return c.ShouldBindUri(s) } +func (s *probeKey) GetId() interface{} { return s.Key } + +func probeDB(t *testing.T, name string, l gormlogger.Interface) *gorm.DB { + t.Helper() + if l == nil { + l = gormlogger.Default.LogMode(gormlogger.Silent) + } + db, err := gorm.Open(sqlite.Open("file:"+name+"?mode=memory&cache=shared"), &gorm.Config{Logger: l}) + if err != nil { + t.Fatal(err) + } + if err := db.AutoMigrate(&probeRow{}); err != nil { + t.Fatal(err) + } + return db +} + +// probeEngine serves the five routes from db, as caller 7. +func probeEngine(db *gorm.DB, register func(r gin.IRoutes)) *gin.Engine { + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(func(c *gin.Context) { + if db != nil { + c.Set("db", db) + } + c.Set(jwtauth.JwtPayloadKey, jwtauth.MapClaims{"identity": float64(7)}) + c.Next() + }) + register(r) + return r +} + +func oldRoutes(r gin.IRoutes) { + r.GET("/x", actions.IndexAction(&probeRow{}, &probeSearch{}, func() interface{} { l := make([]probeRow, 0); return &l })) + r.GET("/x/:id", actions.ViewAction(&probeById{}, func() interface{} { return &probeRow{} })) + r.POST("/x", actions.CreateAction(&probeControl{})) + r.PUT("/x", actions.UpdateAction(&probeControl{})) + r.DELETE("/x", actions.DeleteAction(&probeById{})) +} + +func newRoutes(r gin.IRoutes) { + r.GET("/x", actions.Index[probeRow, probeSearch]()) + r.GET("/x/:id", actions.View[probeRow, probeById]()) + r.POST("/x", actions.Create[probeRow, probeControl]()) + r.PUT("/x", actions.Update[probeRow, probeControl]()) + r.DELETE("/x", actions.Delete[probeRow, probeById]()) +} + +var requestIDField = regexp.MustCompile(`"requestId":"[^"]*"`) + +func serve(r *gin.Engine, method, path, body string) (int, string) { + w := httptest.NewRecorder() + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + return w.Code, requestIDField.ReplaceAllString(w.Body.String(), `"requestId":""`) +} + +// The generic actions answer every request exactly as the actions they +// replace, on the same types and the same data, until the old ones go. +func TestGenericActionsAnswerAsTheOldOnesDo(t *testing.T) { + script := []struct{ method, path, body string }{ + {"POST", "/x", `{"name":"a"}`}, + {"POST", "/x", `{"name":"b"}`}, + {"POST", "/x", `not json`}, + {"GET", "/x?pageIndex=1&pageSize=10", ""}, + {"GET", "/x?pageIndex=1&pageSize=1&name=b", ""}, + {"GET", "/x?pageIndex=2&pageSize=1", ""}, + {"GET", "/x/1", ""}, + {"GET", "/x/99", ""}, + {"GET", "/x/abc", ""}, + {"PUT", "/x", `{"id":1,"name":"a2"}`}, + {"PUT", "/x", `{"id":99,"name":"ghost"}`}, + {"PUT", "/x", `{"name":"no-id"}`}, + {"GET", "/x/1", ""}, + {"DELETE", "/x", `{"ids":[2]}`}, + {"DELETE", "/x", `{"ids":[99]}`}, + {"GET", "/x?pageIndex=1&pageSize=10", ""}, + } + old := probeEngine(probeDB(t, t.Name()+"-old", nil), oldRoutes) + gen := probeEngine(probeDB(t, t.Name()+"-new", nil), newRoutes) + for i, s := range script { + oc, ob := serve(old, s.method, s.path, s.body) + nc, nb := serve(gen, s.method, s.path, s.body) + if oc != nc || ob != nb { + t.Errorf("step %d, %s %s %s:\nold %d %s\nnew %d %s", i+1, s.method, s.path, s.body, oc, ob, nc, nb) + } + } +} + +// Every action that reads or changes existing rows applies the caller's +// data permission: Index, View, Update and Delete. +func TestGenericActionsApplyDataPermission(t *testing.T) { + previous := config.ApplicationConfig.EnableDP + config.ApplicationConfig.EnableDP = true + t.Cleanup(func() { config.ApplicationConfig.EnableDP = previous }) + + for _, req := range []struct{ method, path, body string }{ + {"GET", "/x?pageIndex=1&pageSize=10", ""}, + {"GET", "/x/1", ""}, + {"PUT", "/x", `{"id":1,"name":"a"}`}, + {"DELETE", "/x", `{"ids":[1]}`}, + } { + cl := &capturingLogger{Interface: gormlogger.Default.LogMode(gormlogger.Silent)} + db := probeDB(t, strings.NewReplacer("/", "_", "?", "_").Replace(t.Name()+req.method+req.path), cl) + r := probeEngine(db, func(r gin.IRoutes) { + self := func(c *gin.Context) { + c.Set(actions.PermissionKey, &actions.DataPermission{DataScope: actions.DataScopeSelf, UserId: 7}) + } + r.GET("/x", self, actions.Index[probeRow, probeSearch]()) + r.GET("/x/:id", self, actions.View[probeRow, probeById]()) + r.PUT("/x", self, actions.Update[probeRow, probeControl]()) + r.DELETE("/x", self, actions.Delete[probeRow, probeById]()) + }) + serve(r, req.method, req.path, req.body) + if !strings.Contains(cl.all(), "action_probe_row.create_by = ") { + t.Errorf("%s %s ran without the data-permission scope:\n%s", req.method, req.path, cl.all()) + } + } +} + +// A string key is compared as a value. Handed to Where on its own, "1=1" +// is read as a SQL condition and matches every row. +func TestGenericViewMatchesAStringKeyAsAValue(t *testing.T) { + db := probeDB(t, t.Name(), nil) + if err := db.Create(&probeRow{Name: "a"}).Error; err != nil { + t.Fatal(err) + } + r := probeEngine(db, func(r gin.IRoutes) { r.GET("/x/:id", actions.View[probeRow, probeKey]()) }) + + _, body := serve(r, "GET", "/x/1=1", "") + var res struct{ Code int } + _ = json.Unmarshal([]byte(body), &res) + if res.Code != http.StatusNotFound { + t.Errorf("GET /x/1=1 answered %s; want the row not found", body) + } +} + +// Without a database in the request the old actions wrote nothing, which a +// client reads as an empty 200. +func TestGenericActionsAnswerAMissingDatabase(t *testing.T) { + r := probeEngine(nil, newRoutes) + for _, req := range []struct{ method, path, body string }{ + {"GET", "/x?pageIndex=1&pageSize=10", ""}, + {"GET", "/x/1", ""}, + {"POST", "/x", `{"name":"a"}`}, + {"PUT", "/x", `{"id":1}`}, + {"DELETE", "/x", `{"ids":[1]}`}, + } { + _, body := serve(r, req.method, req.path, req.body) + if !strings.Contains(body, `"code":500`) || !strings.Contains(body, "数据库连接获取失败") { + t.Errorf("%s %s answered %q; want a 500 naming the connection", req.method, req.path, body) + } + } +} + +// Concurrent requests to one route each see only what they asked for. The +// generic actions build every value per request, so there is nothing to +// share; this holds that shape. +func TestGenericIndexKeepsConcurrentRequestsApart(t *testing.T) { + db := probeDB(t, t.Name(), nil) + for i := 0; i < 20; i++ { + if err := db.Create(&probeRow{Name: fmt.Sprintf("n%d", i)}).Error; err != nil { + t.Fatal(err) + } + } + r := probeEngine(db, newRoutes) + + var wg sync.WaitGroup + for i := 0; i < 20; i++ { + name := fmt.Sprintf("n%d", i) + wg.Go(func() { + for j := 0; j < 10; j++ { + _, body := serve(r, "GET", "/x?pageIndex=1&pageSize=10&name="+name, "") + var res struct { + Data struct{ List []probeRow } + } + if err := json.Unmarshal([]byte(body), &res); err != nil { + t.Errorf("decoding %q: %v", body, err) + return + } + if len(res.Data.List) != 1 || res.Data.List[0].Name != name { + t.Errorf("asked for %s, got %+v", name, res.Data.List) + return + } + } + }) + } + wg.Wait() +} From f7c223e0dc9a4b54221cb87a9045d32793c9e3df Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Sun, 27 Sep 2026 20:46:12 +0800 Subject: [PATCH 3/6] =?UTF-8?q?refactor=F0=9F=8E=A8:=20move=20app/demo=20o?= =?UTF-8?q?nto=20the=20generic=20actions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The five routes use actions.Index, View, Create, Update and Delete. The control request's GenerateM becomes ToModel, returning the model type itself, and the Generate methods on the model and the three request types go, along with the tests that held them to returning copies: nothing calls them any more. Recorded before the change and replayed after it, the same 18 requests - creates, lists with filters and paging, details, updates and deletes, found and missing - get byte-identical responses. --- app/demo/models/demo_product.go | 7 --- app/demo/router/demo_product.go | 19 ++---- app/demo/service/dto/demo_product.go | 33 ++-------- app/demo/service/dto/demo_product_test.go | 76 +++-------------------- 4 files changed, 18 insertions(+), 117 deletions(-) diff --git a/app/demo/models/demo_product.go b/app/demo/models/demo_product.go index fb616730..61bb852f 100644 --- a/app/demo/models/demo_product.go +++ b/app/demo/models/demo_product.go @@ -25,13 +25,6 @@ func (DemoProduct) TableName() string { return "demo_product" } -// Generate 返回副本，供通用 Action 使用。 -// 必须返回新实例：Action 在并发请求间复用同一个模型指针，就地返回会串数据。 -func (e *DemoProduct) Generate() models.ActiveRecord { - o := *e - return &o -} - func (e *DemoProduct) GetId() interface{} { return e.Id } diff --git a/app/demo/router/demo_product.go b/app/demo/router/demo_product.go index 4d721543..00c0840d 100644 --- a/app/demo/router/demo_product.go +++ b/app/demo/router/demo_product.go @@ -29,21 +29,12 @@ func registerDemoProductRouter(v1 *gin.RouterGroup, authMiddleware *jwt.GinJWTMi Use(authMiddleware.MiddlewareFunc()). // JWT 认证 Use(middleware.AuthCheckRole()) // Casbin 鉴权 { - m := &models.DemoProduct{} - // actions.PermissionAction() 注入数据权限上下文， // 列表与详情缺少它会绕过 DataScope 过滤 - r.GET("", actions.PermissionAction(), actions.IndexAction(m, new(dto.DemoProductSearch), func() interface{} { - list := make([]models.DemoProduct, 0) - return &list - })) - - r.GET("/:id", actions.PermissionAction(), actions.ViewAction(new(dto.DemoProductById), func() interface{} { - return &models.DemoProduct{} - })) - - r.POST("", actions.CreateAction(new(dto.DemoProductControl))) - r.PUT("/:id", actions.PermissionAction(), actions.UpdateAction(new(dto.DemoProductControl))) - r.DELETE("", actions.PermissionAction(), actions.DeleteAction(new(dto.DemoProductById))) + r.GET("", actions.PermissionAction(), actions.Index[models.DemoProduct, dto.DemoProductSearch]()) + r.GET("/:id", actions.PermissionAction(), actions.View[models.DemoProduct, dto.DemoProductById]()) + r.POST("", actions.Create[models.DemoProduct, dto.DemoProductControl]()) + r.PUT("/:id", actions.PermissionAction(), actions.Update[models.DemoProduct, dto.DemoProductControl]()) + r.DELETE("", actions.PermissionAction(), actions.Delete[models.DemoProduct, dto.DemoProductById]()) } } diff --git a/app/demo/service/dto/demo_product.go b/app/demo/service/dto/demo_product.go index 7516794b..7109b43d 100644 --- a/app/demo/service/dto/demo_product.go +++ b/app/demo/service/dto/demo_product.go @@ -36,15 +36,10 @@ func (m *DemoProductSearch) Bind(ctx *gin.Context) error { return ctx.ShouldBind(m) } -func (m *DemoProductSearch) Generate() dto.Index { - o := *m - return &o -} - -// DemoProductControl 新增与修改共用的入参 +// DemoProductControl is the request body of both create and update. // -// 通用 Action（Create / Update）通过 GenerateM 拿到落库对象， -// 因此这里不直接暴露 Model，字段校验用 validate tag 声明。 +// actions.Create / actions.Update get the row to write from ToModel, so the +// model itself is not exposed; field rules are declared with validate tags. type DemoProductControl struct { Id int `json:"id" comment:"主键"` Name string `json:"name" comment:"名称" validate:"required"` @@ -58,16 +53,9 @@ func (s *DemoProductControl) Bind(ctx *gin.Context) error { return ctx.ShouldBind(s) } -func (s *DemoProductControl) Generate() dto.Control { - o := *s - return &o -} - -func (s *DemoProductControl) GetId() interface{} { return s.Id } - -// GenerateM 组装落库对象。CreateBy / UpdateBy 由通用 Action 在此之后注入， -// 此处不要手动赋值。 -func (s *DemoProductControl) GenerateM() (common.ActiveRecord, error) { +// ToModel builds the row to write. The actions set CreateBy / UpdateBy +// afterwards, so do not set them here. +func (s *DemoProductControl) ToModel() (*models.DemoProduct, error) { return &models.DemoProduct{ Model: common.Model{Id: s.Id}, Name: s.Name, @@ -85,12 +73,3 @@ type DemoProductById struct { // Bind 与 GetId 由内嵌的 dto.ObjectById 提供：它已处理好 uri 绑定、 // DELETE 时的批量 ids 合并与参数校验，无需在此重复实现。 - -func (s *DemoProductById) Generate() dto.Control { - o := *s - return &o -} - -func (s *DemoProductById) GenerateM() (common.ActiveRecord, error) { - return &models.DemoProduct{}, nil -} diff --git a/app/demo/service/dto/demo_product_test.go b/app/demo/service/dto/demo_product_test.go index 32b1a7f3..ce1d19b0 100644 --- a/app/demo/service/dto/demo_product_test.go +++ b/app/demo/service/dto/demo_product_test.go @@ -4,83 +4,21 @@ import ( "testing" "go-admin/app/demo/models" - "go-admin/common/dto" - common "go-admin/common/models" ) -// 通用 Action 依赖 DTO 与 Model 实现一组接口。这些约束在编译期无法完全覆盖 -// （接口是在路由注册处才被要求的），因此用测试锁定，避免改动后在运行时才暴露。 - -func TestImplementsIndexInterface(t *testing.T) { - var _ dto.Index = (*DemoProductSearch)(nil) -} - -func TestImplementsControlInterface(t *testing.T) { - var _ dto.Control = (*DemoProductControl)(nil) - var _ dto.Control = (*DemoProductById)(nil) -} - -func TestModelImplementsActiveRecord(t *testing.T) { - var _ common.ActiveRecord = (*models.DemoProduct)(nil) -} - -// Generate 必须返回副本：通用 Action 在并发请求间复用同一个实例， -// 就地返回会导致请求之间串数据。 -func TestGenerateReturnsCopy(t *testing.T) { - src := &DemoProductControl{Id: 1, Name: "原始"} - got := src.Generate().(*DemoProductControl) - - if got == src { - t.Fatal("Generate 返回了同一指针，应返回副本") - } - got.Name = "被修改" - if src.Name != "原始" { - t.Errorf("修改副本影响了原对象：src.Name = %q", src.Name) - } -} - -func TestSearchGenerateReturnsCopy(t *testing.T) { - src := &DemoProductSearch{Name: "原始"} - got := src.Generate().(*DemoProductSearch) - - if got == src { - t.Fatal("Generate 返回了同一指针，应返回副本") - } - got.Name = "被修改" - if src.Name != "原始" { - t.Errorf("修改副本影响了原对象：src.Name = %q", src.Name) - } -} - -func TestModelGenerateReturnsCopy(t *testing.T) { - src := &models.DemoProduct{Name: "原始"} - got := src.Generate().(*models.DemoProduct) - - if got == src { - t.Fatal("Generate 返回了同一指针，应返回副本") - } - got.Name = "被修改" - if src.Name != "原始" { - t.Errorf("修改副本影响了原对象：src.Name = %q", src.Name) - } -} - -// GenerateM 组装落库对象，主键需正确传递，否则更新会退化成插入。 -func TestGenerateMCarriesId(t *testing.T) { +// ToModel builds the row to write; the key has to reach it, or an update +// matches no row. +func TestToModelCarriesId(t *testing.T) { c := &DemoProductControl{Id: 42, Name: "示例", Code: "P-42", Price: 9.9} - m, err := c.GenerateM() + p, err := c.ToModel() if err != nil { - t.Fatalf("GenerateM 返回错误: %v", err) - } - p, ok := m.(*models.DemoProduct) - if !ok { - t.Fatalf("GenerateM 返回类型错误: %T", m) + t.Fatalf("ToModel: %v", err) } if p.Id != 42 { - t.Errorf("主键未传递: got %d, want 42", p.Id) + t.Errorf("key not carried: got %d, want 42", p.Id) } if p.Name != "示例" || p.Code != "P-42" || p.Price != 9.9 { - t.Errorf("字段映射有误: %+v", p) + t.Errorf("fields mapped wrongly: %+v", p) } } From fda94c49656e561fcbbdbe3a862303ba304b3757 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Sun, 27 Sep 2026 20:46:12 +0800 Subject: [PATCH 4/6] =?UTF-8?q?refactor=F0=9F=8E=A8:=20move=20app/jobs=20o?= =?UTF-8?q?nto=20the=20generic=20actions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The five /sysjob routes use the generic actions. The detail route uses ViewAs to keep answering SysJobItem, whose entryId the edit form reads where the model marshals entry_id. GenerateM becomes ToModel, and the Generate methods go. Recorded before the change and replayed after it, the same 14 requests get byte-identical responses; answering the detail with the model instead of SysJobItem makes exactly the two detail requests differ. With a real ToModel on both modules, a test now builds a package that pairs the demo model with the jobs request and requires the build to fail on that pairing, next to the same package with the pair corrected. --- app/jobs/models/sys_job.go | 5 --- app/jobs/router/sys_job.go | 18 ++++------ app/jobs/service/dto/sys_job.go | 26 +------------- common/actions/generic_compile_test.go | 36 ++++++++++++++++++++ common/actions/testdata/matched/matched.go | 11 ++++++ common/actions/testdata/mismatch/mismatch.go | 12 +++++++ 6 files changed, 67 insertions(+), 41 deletions(-) create mode 100644 common/actions/generic_compile_test.go create mode 100644 common/actions/testdata/matched/matched.go create mode 100644 common/actions/testdata/mismatch/mismatch.go diff --git a/app/jobs/models/sys_job.go b/app/jobs/models/sys_job.go index a8cc6bf9..9107465a 100644 --- a/app/jobs/models/sys_job.go +++ b/app/jobs/models/sys_job.go @@ -27,11 +27,6 @@ func (*SysJob) TableName() string { return "sys_job" } -func (e *SysJob) Generate() models.ActiveRecord { - o := *e - return &o -} - func (e *SysJob) GetId() interface{} { return e.JobId } diff --git a/app/jobs/router/sys_job.go b/app/jobs/router/sys_job.go index 090be794..3c6d44d5 100644 --- a/app/jobs/router/sys_job.go +++ b/app/jobs/router/sys_job.go @@ -19,17 +19,13 @@ func registerSysJobRouter(v1 *gin.RouterGroup, authMiddleware *jwt.GinJWTMiddlew r := v1.Group("/sysjob").Use(authMiddleware.MiddlewareFunc()).Use(middleware.AuthCheckRole()) { - sysJob := &models2.SysJob{} - r.GET("", actions.PermissionAction(), actions.IndexAction(sysJob, new(dto2.SysJobSearch), func() interface{} { - list := make([]models2.SysJob, 0) - return &list - })) - r.GET("/:id", actions.PermissionAction(), actions.ViewAction(new(dto2.SysJobById), func() interface{} { - return &dto2.SysJobItem{} - })) - r.POST("", actions.CreateAction(new(dto2.SysJobControl))) - r.PUT("", actions.PermissionAction(), actions.UpdateAction(new(dto2.SysJobControl))) - r.DELETE("", actions.PermissionAction(), actions.DeleteAction(new(dto2.SysJobById))) + r.GET("", actions.PermissionAction(), actions.Index[models2.SysJob, dto2.SysJobSearch]()) + // The detail answers SysJobItem, whose entryId the edit form reads; + // the model itself marshals it as entry_id. + r.GET("/:id", actions.PermissionAction(), actions.ViewAs[models2.SysJob, dto2.SysJobById, dto2.SysJobItem]()) + r.POST("", actions.Create[models2.SysJob, dto2.SysJobControl]()) + r.PUT("", actions.PermissionAction(), actions.Update[models2.SysJob, dto2.SysJobControl]()) + r.DELETE("", actions.PermissionAction(), actions.Delete[models2.SysJob, dto2.SysJobById]()) } sysJob := apis.SysJob{} diff --git a/app/jobs/service/dto/sys_job.go b/app/jobs/service/dto/sys_job.go index f342f927..1ab9310f 100644 --- a/app/jobs/service/dto/sys_job.go +++ b/app/jobs/service/dto/sys_job.go @@ -6,7 +6,6 @@ import ( "go-admin/app/jobs/models" "go-admin/common/dto" - common "go-admin/common/models" ) type SysJobSearch struct { @@ -32,11 +31,6 @@ func (m *SysJobSearch) Bind(ctx *gin.Context) error { return err } -func (m *SysJobSearch) Generate() dto.Index { - o := *m - return &o -} - type SysJobControl struct { JobId int `json:"jobId"` JobName string `json:"jobName" validate:"required"` // 名称 @@ -55,12 +49,7 @@ func (s *SysJobControl) Bind(ctx *gin.Context) error { return ctx.ShouldBind(s) } -func (s *SysJobControl) Generate() dto.Control { - cp := *s - return &cp -} - -func (s *SysJobControl) GenerateM() (common.ActiveRecord, error) { +func (s *SysJobControl) ToModel() (*models.SysJob, error) { return &models.SysJob{ JobId: s.JobId, JobName: s.JobName, @@ -76,23 +65,10 @@ func (s *SysJobControl) GenerateM() (common.ActiveRecord, error) { }, nil } -func (s *SysJobControl) GetId() interface{} { - return s.JobId -} - type SysJobById struct { dto.ObjectById } -func (s *SysJobById) Generate() dto.Control { - cp := *s - return &cp -} - -func (s *SysJobById) GenerateM() (common.ActiveRecord, error) { - return &models.SysJob{}, nil -} - type SysJobItem struct { JobId int `json:"jobId"` JobName string `json:"jobName" validate:"required"` // 名称 diff --git a/common/actions/generic_compile_test.go b/common/actions/generic_compile_test.go new file mode 100644 index 00000000..d9838bc6 --- /dev/null +++ b/common/actions/generic_compile_test.go @@ -0,0 +1,36 @@ +package actions_test + +import ( + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// A model paired with a request that builds a different model does not +// compile - the thing the older actions left to a type assertion at run +// time. Checked by building a package that does it, next to a control that +// is the same package with the pair corrected. +func TestGenericActionsRefuseAMismatchedPair(t *testing.T) { + root, err := filepath.Abs("../..") + if err != nil { + t.Fatal(err) + } + vet := func(pkg string) (string, error) { + cmd := exec.Command("go", "vet", "./common/actions/testdata/"+pkg) + cmd.Dir = root + out, err := cmd.CombinedOutput() + return string(out), err + } + + if out, err := vet("matched"); err != nil { + t.Fatalf("the control does not build, so the mismatch below proves nothing:\n%s", out) + } + out, err := vet("mismatch") + if err == nil { + t.Fatal("a model paired with another model's request compiled") + } + if !strings.Contains(out, "does not satisfy") || !strings.Contains(out, "ToModel") { + t.Fatalf("the build failed, but not on the pairing:\n%s", out) + } +} diff --git a/common/actions/testdata/matched/matched.go b/common/actions/testdata/matched/matched.go new file mode 100644 index 00000000..64d80265 --- /dev/null +++ b/common/actions/testdata/matched/matched.go @@ -0,0 +1,11 @@ +// Package matched is mismatch with the pair corrected: the control that +// shows mismatch fails for the pairing, not for how it is built. +package matched + +import ( + "go-admin/app/demo/models" + "go-admin/app/demo/service/dto" + "go-admin/common/actions" +) + +var _ = actions.Create[models.DemoProduct, dto.DemoProductControl] diff --git a/common/actions/testdata/mismatch/mismatch.go b/common/actions/testdata/mismatch/mismatch.go new file mode 100644 index 00000000..99eac73d --- /dev/null +++ b/common/actions/testdata/mismatch/mismatch.go @@ -0,0 +1,12 @@ +// Package mismatch pairs a model with another model's request, which must +// not compile. TestGenericActionsRefuseAMismatchedPair builds it and expects +// the build to fail; testdata keeps it out of ./... . +package mismatch + +import ( + "go-admin/app/demo/models" + jobdto "go-admin/app/jobs/service/dto" + "go-admin/common/actions" +) + +var _ = actions.Create[models.DemoProduct, jobdto.SysJobControl] From 4d17146a25ec4612e2bcbfb9a4bb48a809f243b5 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Sun, 27 Sep 2026 20:46:21 +0800 Subject: [PATCH 5/6] =?UTF-8?q?chore=F0=9F=94=A7:=20deprecate=20the=20five?= =?UTF-8?q?=20CRUD=20actions=20the=20generic=20ones=20replace?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IndexAction, ViewAction, CreateAction, UpdateAction and DeleteAction stay and behave as before; each now points at its generic replacement. Nothing in this repository calls them any more. --- common/actions/create.go | 4 ++++ common/actions/delete.go | 4 ++++ common/actions/index.go | 4 ++++ common/actions/update.go | 4 ++++ common/actions/view.go | 5 +++++ 5 files changed, 21 insertions(+) diff --git a/common/actions/create.go b/common/actions/create.go index 9fe75bd7..abaf57d5 100644 --- a/common/actions/create.go +++ b/common/actions/create.go @@ -14,6 +14,10 @@ import ( ) // CreateAction 通用新增动作 +// +// Deprecated: Use Create[Model, Control](). It builds its values per request, so the +// Generate() copies this action depends on are not needed, and a model +// paired with the wrong request does not compile. See generic.go. func CreateAction(control dto.Control) gin.HandlerFunc { return func(c *gin.Context) { log := api.GetRequestLogger(c) diff --git a/common/actions/delete.go b/common/actions/delete.go index 530ec997..444fb690 100644 --- a/common/actions/delete.go +++ b/common/actions/delete.go @@ -14,6 +14,10 @@ import ( ) // DeleteAction 通用删除动作 +// +// Deprecated: Use Delete[Model, ById](). It builds its values per request, so the +// Generate() copies this action depends on are not needed, and a model +// paired with the wrong request does not compile. See generic.go. func DeleteAction(control dto.Control) gin.HandlerFunc { return func(c *gin.Context) { db, err := pkg.GetOrm(c) diff --git a/common/actions/index.go b/common/actions/index.go index eb9afb40..fa7a5c66 100644 --- a/common/actions/index.go +++ b/common/actions/index.go @@ -15,6 +15,10 @@ import ( ) // IndexAction 通用查询动作 +// +// Deprecated: Use Index[Model, Search](). It builds its values per request, so the +// Generate() copies this action depends on are not needed, and a model +// paired with the wrong request does not compile. See generic.go. func IndexAction(m models.ActiveRecord, d dto.Index, f func() interface{}) gin.HandlerFunc { return func(c *gin.Context) { db, err := pkg.GetOrm(c) diff --git a/common/actions/update.go b/common/actions/update.go index 3ce96d07..6e849813 100644 --- a/common/actions/update.go +++ b/common/actions/update.go @@ -14,6 +14,10 @@ import ( ) // UpdateAction 通用更新动作 +// +// Deprecated: Use Update[Model, Control](). It builds its values per request, so the +// Generate() copies this action depends on are not needed, and a model +// paired with the wrong request does not compile. See generic.go. func UpdateAction(control dto.Control) gin.HandlerFunc { return func(c *gin.Context) { db, err := pkg.GetOrm(c) diff --git a/common/actions/view.go b/common/actions/view.go index 3f3a0f2f..60480f8c 100644 --- a/common/actions/view.go +++ b/common/actions/view.go @@ -15,6 +15,11 @@ import ( ) // ViewAction 通用详情动作 +// +// Deprecated: Use View[Model, ById](), or ViewAs[Model, ById, Response]() +// where f returned another type. It builds its values per request, so the +// Generate() copies this action depends on are not needed, and a model +// paired with the wrong request does not compile. See generic.go. func ViewAction(control dto.Control, f func() interface{}) gin.HandlerFunc { return func(c *gin.Context) { db, err := pkg.GetOrm(c) From eb4e0f21dfd9d5f8010fd9f115a64ad746477ddf Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Sun, 27 Sep 2026 20:46:21 +0800 Subject: [PATCH 6/6] =?UTF-8?q?docs=F0=9F=93=9D:=20teach=20the=20generic?= =?UTF-8?q?=20actions=20instead=20of=20Generate()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AGENTS.md, docs/contract.md and the new-business-module skill showed the older actions and made returning a copy from Generate() a rule to remember. They now show the generic actions, whose type parameters are the rule, and say the Generate() rule holds only for modules still on the deprecated actions. --- .claude/skills/new-business-module/SKILL.md | 18 ++++++----- AGENTS.md | 34 ++++++++++----------- docs/contract.md | 4 +-- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/.claude/skills/new-business-module/SKILL.md b/.claude/skills/new-business-module/SKILL.md index 1cc0b66c..c0a93e13 100644 --- a/.claude/skills/new-business-module/SKILL.md +++ b/.claude/skills/new-business-module/SKILL.md @@ -35,15 +35,17 @@ description: Scaffold a new single-table CRUD business module end to end — mig ### 3. 生成 model / dto / router 三个文件（Actions 模式） -不要手写 Api 与 Service。使用 `common/actions` 的通用 Action，一个模块只需 -model、dto、router 三个文件，完整写法照抄 `app/demo/` 的结构。 +不要手写 Api 与 Service。使用 `common/actions` 的泛型 Action +（`actions.Index[Model, Search]()` 等），一个模块只需 model、dto、router +三个文件，完整写法照抄 `app/demo/` 的结构。 -**关键正确性要求**（这三条是实际出问题最多的地方）： +**关键正确性要求**： -- Model 实现 `models.ActiveRecord`（`Generate` / `GetId` / `TableName`）， - `TableName()` 必须显式声明——GORM 配置了 `SingularTable`，不会自动推导 -- **`Generate()` 必须返回副本，不要就地返回**——Action 在并发请求间复用实例， - 就地返回会导致请求之间串数据；这个问题单人测试时几乎不出现，上线后才暴露 +- `TableName()` 必须显式声明——GORM 配置了 `SingularTable`，不会自动推导 +- 增改 DTO 写 `ToModel() (*Model, error)`；模型与 DTO 配错会编译不过 +- **不要写 `Generate()`**，也不要用旧的 `IndexAction` 等五个（已 Deprecated）： + 旧写法在并发请求间复用实例，`Generate()` 一旦就地返回就会串数据， + 单人测试时几乎不出现、上线后才暴露；泛型 Action 每个请求新建自己的值 - 完成后确认 `cmd/api/` 中已用 `_` 导入新包，否则路由不会被注册 ### 4. 写菜单、接口与权限种子数据 @@ -82,7 +84,7 @@ model、dto、router 三个文件，完整写法照抄 `app/demo/` 的结构。 | 检查项 | 出错后果 | | --- | --- | -| `Generate()` 是否返回副本 | 并发请求之间串数据 | +| 路由是否用泛型 Action 而非旧的 `IndexAction` 等 | 旧写法要靠 `Generate()` 返回副本，漏了就并发串数据 | | 是否使用 `e.Orm` 而非全局 DB | 多租户下拿到错误的数据库连接 | | `TableName()` 是否显式声明 | GORM 不会自动推导 | | 迁移文件是否放在 `version/` | 放进 `version-local/` 会被忽略，别人拉代码看不到 | diff --git a/AGENTS.md b/AGENTS.md index b3e221be..01f544a2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,34 +20,34 @@ Router → Api → Service → Model ## 优先使用通用 Action -单表 CRUD **不要手写 Api 与 Service**。`common/actions` 提供的五个 -Action 已覆盖参数绑定、数据权限过滤、操作人注入、分页与错误响应： +单表 CRUD **不要手写 Api 与 Service**。`common/actions` 提供的泛型 Action +已覆盖参数绑定、数据权限过滤、操作人注入、分页与错误响应： ```go r := v1.Group("/demo-product").Use(authMiddleware.MiddlewareFunc()).Use(middleware.AuthCheckRole()) { - m := &models.DemoProduct{} - r.GET("", actions.PermissionAction(), actions.IndexAction(m, new(dto.DemoProductSearch), func() interface{} { - list := make([]models.DemoProduct, 0); return &list - })) - r.GET("/:id", actions.PermissionAction(), actions.ViewAction(new(dto.DemoProductById), func() interface{} { - return &models.DemoProduct{} - })) - r.POST("", actions.CreateAction(new(dto.DemoProductControl))) - r.PUT("/:id", actions.PermissionAction(), actions.UpdateAction(new(dto.DemoProductControl))) - r.DELETE("", actions.PermissionAction(), actions.DeleteAction(new(dto.DemoProductById))) + r.GET("", actions.PermissionAction(), actions.Index[models.DemoProduct, dto.DemoProductSearch]()) + r.GET("/:id", actions.PermissionAction(), actions.View[models.DemoProduct, dto.DemoProductById]()) + r.POST("", actions.Create[models.DemoProduct, dto.DemoProductControl]()) + r.PUT("/:id", actions.PermissionAction(), actions.Update[models.DemoProduct, dto.DemoProductControl]()) + r.DELETE("", actions.PermissionAction(), actions.Delete[models.DemoProduct, dto.DemoProductById]()) } ``` 这样一个模块只需 **model + dto + router** 三个文件，完整示例见 `app/demo/`。 -使用通用 Action 的前提： +类型参数就是约束，写错了编译不过： -- Model 实现 `models.ActiveRecord`（`Generate` / `GetId` / `TableName`） -- 列表 DTO 实现 `dto.Index`，增改删 DTO 实现 `dto.Control` -- **所有 `Generate()` 必须返回副本** —— Action 在并发请求间复用实例， - 就地返回会串数据（`app/demo` 的测试锁定了这一点） +- Model 需有 `TableName` / `GetId`，并内嵌 `models.ControlBy`（提供 `SetCreateBy` / `SetUpdateBy`） +- 列表 DTO：`Bind`、`GetNeedSearch`，内嵌 `dto.Pagination` +- 增改 DTO：`Bind` 与 `ToModel() (*Model, error)`——配错模型编译不过 - 详情/删除 DTO 内嵌 `dto.ObjectById` 即可继承 `Bind` 与 `GetId`，无需重写 +- 详情要返回与模型不同的结构时用 `ViewAs[Model, ById, Response]()`（见 `app/jobs`） +- **不需要 `Generate()`**：每个请求都新建自己的值，不存在跨请求共享的实例 + +旧的 `IndexAction` 等五个仍可用，已标记 Deprecated。它们在并发请求间复用 +注册时传入的实例，所以依赖「所有 `Generate()` 必须返回副本」这条约定—— +仍在用旧写法的模块要继续遵守它。 仅当业务超出单表 CRUD（跨表事务、外部调用、复杂校验）时才自行编写 Api 与 Service，写法见下。 diff --git a/docs/contract.md b/docs/contract.md index 599e7f7f..d0a88070 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -356,11 +356,11 @@ if res.RowsAffected == 0 { return ErrAlreadyPaid } // 别人先改了 | `api.Api` | core `sdk/api` | 一条链式糖：`MakeContext` / `Bind` / `MakeOrm` / `OK` / `PageOK` / `Error` | | `service.Service` | core `sdk/service` | 一个装 `Orm` / `Log` / `Cache` / `Error` 的结构体加一个 `AddError` | | `MakeCondition` / `search` tag | core `sdk/contract/dto` | 把 DTO 上的 `search:"type:exact;column:name;table:xx"` 翻成 WHERE | -| 通用 CRUD Action | go-admin `common/actions` | `IndexAction` 等五个。**留在 go-admin，没有下沉** | +| 通用 CRUD Action | go-admin `common/actions` | 泛型的 `Index` / `View` / `ViewAs` / `Create` / `Update` / `Delete`（旧的 `IndexAction` 等五个已标 Deprecated）。**留在 go-admin，没有下沉** | 最后一行是有意的：CRUD Action 是最需要演进的一类东西（分页参数、批量操作、 软删语义、字段级权限），而 core 的每一个导出都是永久承诺——放进去容易， -拿出来不可能。想用就把那 294 行抄走，抄走的那份还能按你自己的需要改。 +拿出来不可能。想用就把 `common/actions` 抄走，抄走的那份还能按你自己的需要改。 主仓唯一的真实业务模块 `app/admin` **一个 CRUD Action 都没用**，全是手写 Service。 `MakeCondition` 返回的是 `func(db *gorm.DB) *gorm.DB` 闭包，方言从闭包里那个