From 629fac4d29ff02b1be3e1664a86ef2170306a43d Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 1 Oct 2026 21:31:46 +0200 Subject: [PATCH] fix(09): WR-16 resolve model columns through embedded structs and explicit column tags --- docs/backend/admin-controllers.md | 2 +- modules/cabana/crud.go | 22 ++----- modules/cabana/filter_schema.go | 6 +- modules/cabana/http.go | 35 ++++++++--- modules/cabana/list_schema.go | 7 +-- modules/cabana/model_fields.go | 98 +++++++++++++++++++++++++++++ modules/cabana/model_fields_test.go | 86 +++++++++++++++++++++++++ modules/cabana/query.go | 20 ++---- modules/cabana/relation_field.go | 7 +-- 9 files changed, 229 insertions(+), 54 deletions(-) create mode 100644 modules/cabana/model_fields.go create mode 100644 modules/cabana/model_fields_test.go diff --git a/docs/backend/admin-controllers.md b/docs/backend/admin-controllers.md index fd744bd..321210e 100644 --- a/docs/backend/admin-controllers.md +++ b/docs/backend/admin-controllers.md @@ -154,7 +154,7 @@ func (p *BlogPlugin) Settings() []pact.SettingsItem { summer make:admin-controller acme.blog Posts ``` -The generated controller requires the permission `acme.blog.access_posts`, so declare it in the plugin's `pact.HasPermissions` or the admin API refuses to start with an unknown-permission error. Its `NewRecord` returns `nil` until you return the model, and the list and every write answer 500 until then. The controller ID maps to the admin API path: `acme.blog.posts` is served under `/api/v1/acme/blog/posts`. The admin prefix is `backend.uri`, `/backend` by default. A model's `Fillable` method decides which form fields the API may write; see [Forms](forms.md). +The generated controller requires the permission `acme.blog.access_posts`, so declare it in the plugin's `pact.HasPermissions` or the admin API refuses to start with an unknown-permission error. Its `NewRecord` returns `nil` until you return the model, and the list and every write answer 500 until then. The controller ID maps to the admin API path: `acme.blog.posts` is served under `/api/v1/acme/blog/posts`. The admin prefix is `backend.uri`, `/backend` by default. A model's `Fillable` method decides which form fields the API may write; see [Forms](forms.md). The admin finds a model's columns through its struct fields, including those of an embedded struct such as `gorm.Model`. A field with a `gorm:"column:..."` tag is known by that column alone; an untagged field is known by GORM's default column name. ## Compilation at boot diff --git a/modules/cabana/crud.go b/modules/cabana/crud.go index c64f926..0d37382 100644 --- a/modules/cabana/crud.go +++ b/modules/cabana/crud.go @@ -785,21 +785,9 @@ func projectRecord(cc *CompiledController, model any) map[string]any { } func modelColumns(model any) map[string]struct{} { - t := reflect.TypeOf(model) - for t != nil && t.Kind() == reflect.Pointer { - t = t.Elem() - } cols := map[string]struct{}{} - if t == nil || t.Kind() != reflect.Struct { - return cols - } - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - if field.PkgPath != "" { - continue - } - name := gormColumn(field) - if name != "" { + for _, mf := range modelFields(reflect.TypeOf(model)) { + if name := gormColumn(mf.Field); name != "" { cols[name] = struct{}{} } } @@ -873,9 +861,9 @@ func castPK(model any, n uint) any { if t == nil || t.Kind() != reflect.Struct { return n } - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - if !strings.Contains(field.Tag.Get("gorm"), "primaryKey") { + for _, mf := range modelFields(t) { + field := mf.Field + if !hasPrimaryKeyTag(field) { continue } switch field.Type.Kind() { diff --git a/modules/cabana/filter_schema.go b/modules/cabana/filter_schema.go index a643979..92c3519 100644 --- a/modules/cabana/filter_schema.go +++ b/modules/cabana/filter_schema.go @@ -319,9 +319,9 @@ func modelColumnType(ctl pact.AdminController, column string) (reflect.Type, boo if t == nil || t.Kind() != reflect.Struct { return nil, false } - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - if field.PkgPath != "" || isListRelation(field.Type) { + for _, mf := range modelFields(t) { + field := mf.Field + if isListRelation(field.Type) { continue } name := gormColumn(field) diff --git a/modules/cabana/http.go b/modules/cabana/http.go index 8ae2629..6fbbbb1 100644 --- a/modules/cabana/http.go +++ b/modules/cabana/http.go @@ -842,21 +842,38 @@ func projectRow(row any, controller pact.AdminController, cols []ListColumn) map return out } +// fieldByColumn returns the model field stored in column, looking through +// embedded structs such as gorm.Model. A field with an explicit `column:` tag is +// matched by that tag alone; only an untagged field falls back to its Go name +// (case-insensitive) or GORM's default column name for it, and a shallower +// field shadows an embedded one, as in Go. func fieldByColumn(v reflect.Value, column string) reflect.Value { if v.Kind() != reflect.Struct { return reflect.Value{} } - t := v.Type() - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - if field.PkgPath != "" { - continue - } - if gormColumn(field) == column || strings.EqualFold(field.Name, column) { - return v.Field(i) + fields := modelFields(v.Type()) + best := -1 + for i := range fields { + if gormColumn(fields[i].Field) == column && (best < 0 || len(fields[i].Path) < len(fields[best].Path)) { + best = i } } - return reflect.Value{} + if best < 0 { + for i := range fields { + untagged := gormColumn(fields[i].Field) == "" + if untagged && (strings.EqualFold(fields[i].Field.Name, column) || defaultColumnName(fields[i].Field) == column) && (best < 0 || len(fields[i].Path) < len(fields[best].Path)) { + best = i + } + } + } + if best < 0 { + return reflect.Value{} + } + field, err := v.FieldByIndexErr(fields[best].Path) + if err != nil { + return reflect.Value{} + } + return field } func gormColumn(field reflect.StructField) string { diff --git a/modules/cabana/list_schema.go b/modules/cabana/list_schema.go index 78ac632..44e7a2f 100644 --- a/modules/cabana/list_schema.go +++ b/modules/cabana/list_schema.go @@ -397,11 +397,8 @@ func listModelContract(ctl pact.AdminController) (map[string]struct{}, map[strin } cols := map[string]struct{}{} rels := map[string]struct{}{} - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - if field.PkgPath != "" { - continue - } + for _, mf := range modelFields(t) { + field := mf.Field if isListRelation(field.Type) { rels[field.Name] = struct{}{} // Winter YAML spells relations in lowercase (relation: genre) diff --git a/modules/cabana/model_fields.go b/modules/cabana/model_fields.go new file mode 100644 index 0000000..e10a0dc --- /dev/null +++ b/modules/cabana/model_fields.go @@ -0,0 +1,98 @@ +package cabana + +import ( + "database/sql" + "database/sql/driver" + "reflect" + "strings" + "sync" + "time" + + "gorm.io/gorm/schema" +) + +// modelField is an exported struct field of a model together with its index +// path from the model type, so a field promoted from an embedded struct +// (gorm.Model, a shared timestamps struct) is reachable like a top-level one. +type modelField struct { + Field reflect.StructField + Path []int +} + +var modelFieldCache sync.Map // reflect.Type -> []modelField + +// modelFields lists the exported fields of the struct type behind t in +// declaration order, with anonymous (embedded) structs flattened in place. +// Embedded scalar-like structs (time.Time, sql.Null*, anything that is a +// Scanner or Valuer) stay single fields. It returns nil for a non-struct. +func modelFields(t reflect.Type) []modelField { + for t != nil && t.Kind() == reflect.Pointer { + t = t.Elem() + } + if t == nil || t.Kind() != reflect.Struct { + return nil + } + if cached, ok := modelFieldCache.Load(t); ok { + return cached.([]modelField) + } + fields := flattenModelFields(t, nil, 0) + modelFieldCache.Store(t, fields) + return fields +} + +func flattenModelFields(t reflect.Type, prefix []int, depth int) []modelField { + var out []modelField + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + path := append(append([]int(nil), prefix...), i) + if f.Anonymous && depth < 5 { + if inner := embeddedStructType(f.Type); inner != nil { + out = append(out, flattenModelFields(inner, path, depth+1)...) + continue + } + } + if f.PkgPath != "" { + continue + } + out = append(out, modelField{Field: f, Path: path}) + } + return out +} + +var ( + scannerType = reflect.TypeOf((*sql.Scanner)(nil)).Elem() + valuerType = reflect.TypeOf((*driver.Valuer)(nil)).Elem() + timeType = reflect.TypeOf(time.Time{}) +) + +// embeddedStructType returns the struct type to flatten for an embedded field, +// or nil when the field is a single value. +func embeddedStructType(t reflect.Type) reflect.Type { + if t.Kind() == reflect.Pointer { + t = t.Elem() + } + if t.Kind() != reflect.Struct || t == timeType { + return nil + } + ptr := reflect.PointerTo(t) + if ptr.Implements(scannerType) || t.Implements(valuerType) || ptr.Implements(valuerType) { + return nil + } + return t +} + +// hasPrimaryKeyTag reports whether a field's gorm tag declares it the primary +// key. GORM reads the key case-insensitively (gorm.Model writes "primarykey"). +func hasPrimaryKeyTag(field reflect.StructField) bool { + for _, part := range strings.Split(field.Tag.Get("gorm"), ";") { + if strings.EqualFold(strings.TrimSpace(part), "primarykey") { + return true + } + } + return false +} + +// defaultColumnName is GORM's default column for an untagged field. +func defaultColumnName(field reflect.StructField) string { + return schema.NamingStrategy{}.ColumnName("", field.Name) +} diff --git a/modules/cabana/model_fields_test.go b/modules/cabana/model_fields_test.go new file mode 100644 index 0000000..3d88ea3 --- /dev/null +++ b/modules/cabana/model_fields_test.go @@ -0,0 +1,86 @@ +package cabana + +import ( + "reflect" + "testing" + "time" + + "gorm.io/gorm" +) + +type embeddedBase struct { + ID uint `gorm:"primarykey"` + CreatedAt time.Time `gorm:"column:created_at"` +} + +type embeddedModel struct { + embeddedBase + Name string `gorm:"column:name"` +} + +type gormModelRow struct { + gorm.Model + Name string `gorm:"column:name"` +} + +type retaggedModel struct { + Name string `gorm:"column:title"` + Title string `gorm:"column:name"` + Plain string +} + +// TestModelHelpersLookThroughEmbeddedStructs pins WR-16: the reflection helpers +// that resolve columns see fields promoted from an embedded struct such as +// gorm.Model, so list rows keep their id and timestamps and a relation link +// writes the real owner key instead of 0. +func TestModelHelpersLookThroughEmbeddedStructs(t *testing.T) { + row := &embeddedModel{embeddedBase: embeddedBase{ID: 7, CreatedAt: time.Unix(1700000000, 0).UTC()}, Name: "n"} + if got := primaryColumn(row); got != "id" { + t.Fatalf("primaryColumn = %q, want id (the embedded primarykey field)", got) + } + if got := pkUint(row); got != 7 { + t.Fatalf("pkUint = %d, want 7", got) + } + cols := modelColumns(row) + for _, want := range []string{"created_at", "name"} { + if _, ok := cols[want]; !ok { + t.Fatalf("modelColumns = %v, missing %s", cols, want) + } + } + projected := projectRow(row, nil, []ListColumn{{Key: "name"}, {Key: "created_at"}}) + if projected["id"] != uint(7) || projected["name"] != "n" || projected["created_at"] == nil { + t.Fatalf("projectRow = %#v, want the embedded id and created_at", projected) + } + if castPK(row, 9) != uint(9) { + t.Fatalf("castPK = %#v", castPK(row, 9)) + } + + gm := &gormModelRow{Model: gorm.Model{ID: 11}, Name: "g"} + if primaryColumn(gm) != "id" || pkUint(gm) != 11 { + t.Fatalf("gorm.Model primary key = %q/%d, want id/11", primaryColumn(gm), pkUint(gm)) + } + if !fieldByColumn(reflect.ValueOf(gm).Elem(), "updated_at").IsValid() { + t.Fatal("gorm.Model's UpdatedAt is not reachable by column") + } +} + +// TestFieldByColumnTagBeatsGoName pins the other half of WR-16: a field with an +// explicit column tag is matched by that tag alone, never by its Go name. +func TestFieldByColumnTagBeatsGoName(t *testing.T) { + row := retaggedModel{Name: "in-title-column", Title: "in-name-column", Plain: "p"} + v := reflect.ValueOf(row) + if got := fieldByColumn(v, "name"); !got.IsValid() || got.String() != "in-name-column" { + t.Fatalf("column name resolved to %v, want the field tagged column:name", got) + } + if got := fieldByColumn(v, "title"); !got.IsValid() || got.String() != "in-title-column" { + t.Fatalf("column title resolved to %v, want the field tagged column:title", got) + } + if got := fieldByColumn(v, "plain"); !got.IsValid() || got.String() != "p" { + t.Fatalf("an untagged field must still match its Go name case-insensitively, got %v", got) + } + if got := fieldByColumn(reflect.ValueOf(struct { + Name string `gorm:"column:title"` + }{Name: "x"}), "name"); got.IsValid() { + t.Fatalf("a tagged field matched by its Go name: %v", got) + } +} diff --git a/modules/cabana/query.go b/modules/cabana/query.go index 2f7c8cc..101be54 100644 --- a/modules/cabana/query.go +++ b/modules/cabana/query.go @@ -477,24 +477,14 @@ func relationFieldName(model any, relation string) (string, bool) { } func primaryColumn(model any) string { - t := reflect.TypeOf(model) - for t != nil && t.Kind() == reflect.Pointer { - t = t.Elem() - } - if t == nil || t.Kind() != reflect.Struct { - return "id" - } - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - if field.PkgPath != "" { + for _, mf := range modelFields(reflect.TypeOf(model)) { + if !hasPrimaryKeyTag(mf.Field) { continue } - if strings.Contains(field.Tag.Get("gorm"), "primaryKey") { - if name := gormColumn(field); name != "" { - return name - } - return field.Name + if name := gormColumn(mf.Field); name != "" { + return name } + return defaultColumnName(mf.Field) } return "id" } diff --git a/modules/cabana/relation_field.go b/modules/cabana/relation_field.go index 7bc9e7d..f77d1db 100644 --- a/modules/cabana/relation_field.go +++ b/modules/cabana/relation_field.go @@ -245,10 +245,9 @@ func structFieldByColumn(model any, column string) (reflect.StructField, bool) { if t == nil || t.Kind() != reflect.Struct { return reflect.StructField{}, false } - for i := 0; i < t.NumField(); i++ { - field := t.Field(i) - if field.PkgPath == "" && gormColumn(field) == column { - return field, true + for _, mf := range modelFields(t) { + if gormColumn(mf.Field) == column { + return mf.Field, true } } return reflect.StructField{}, false