From bdc34900a5c378e1d6b514f8fb81291bb46428d1 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 8 Jul 2026 05:52:46 +0000 Subject: [PATCH 1/2] feat: formalize action constants and remove Uint from codec interfaces - Added ActionCreate, ActionRead, ActionUpdate, ActionDelete byte constants in field.go. - Replaced magic action literals in ValidateFields and its documentation. - Removed Uint from FieldWriter and FieldReader interfaces in codec.go. - Updated tests and documentation (README.md, docs/API_FIELD.md, docs/API_CODEC.md, docs/WHY_64BIT_ONLY.md). - Added value-pinning contract tests for action constants. Co-authored-by: cdvelop <44058491+cdvelop@users.noreply.github.com> --- README.md | 6 ++++ codec.go | 2 -- docs/API_CODEC.md | 2 -- docs/API_FIELD.md | 21 +++++++++++--- docs/WHY_64BIT_ONLY.md | 8 ++++++ field.go | 30 ++++++++++++++++---- tests/codec_test.go | 21 -------------- tests/field_test.go | 62 +++++++++++++++++++++++------------------- 8 files changed, 89 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index 8bc882f..b4217a4 100644 --- a/README.md +++ b/README.md @@ -86,6 +86,12 @@ func (u *User) Validate(action byte) error { } ``` +Usage: + +```go +err := model.ValidateFields(model.ActionUpdate, user) +``` + ### Using the Codec Generated `EncodeFields` and `DecodeFields` methods follow the typed codec pattern: diff --git a/codec.go b/codec.go index 6c91c96..f2f9308 100644 --- a/codec.go +++ b/codec.go @@ -6,7 +6,6 @@ package model type FieldWriter interface { String(name, val string) Int(name string, val int64) - Uint(name string, val uint64) Float(name string, val float64) Bool(name string, val bool) Bytes(name string, val []byte) @@ -42,7 +41,6 @@ type Encodable interface { type FieldReader interface { String(name string) (string, bool) Int(name string) (int64, bool) - Uint(name string) (uint64, bool) Float(name string) (float64, bool) Bool(name string) (bool, bool) Bytes(name string) ([]byte, bool) diff --git a/docs/API_CODEC.md b/docs/API_CODEC.md index 8de8ed4..c12a668 100644 --- a/docs/API_CODEC.md +++ b/docs/API_CODEC.md @@ -44,7 +44,6 @@ Receives typed field values. Concrete implementations (like JSON or JS-value) re type FieldWriter interface { String(name, val string) Int(name string, val int64) - Uint(name string, val uint64) Float(name string, val float64) Bool(name string, val bool) Bytes(name string, val []byte) @@ -79,7 +78,6 @@ Delivers typed field values by name. Implementations avoid building internal map type FieldReader interface { String(name string) (string, bool) Int(name string) (int64, bool) - Uint(name string) (uint64, bool) Float(name string) (float64, bool) Bool(name string) (bool, bool) Bytes(name string) ([]byte, bool) diff --git a/docs/API_FIELD.md b/docs/API_FIELD.md index 00b19da..e70203c 100644 --- a/docs/API_FIELD.md +++ b/docs/API_FIELD.md @@ -299,15 +299,28 @@ type SafeFields interface { Generic function that iterates through a `Fielder`'s schema and pointers to perform full validation based on the action. It handles `FieldText` (calling `Field.Validate`) and `FieldStruct` (recursive validation). +#### Action Constants + +The following `byte` constants should be used for the `action` parameter: + +| Constant | Value | Description | +|----------|-------|-------------| +| `ActionCreate` | `'c'` | Model is being created (skips PK+AutoInc) | +| `ActionRead` | `'r'` | Model is being read (used for RBAC/tool convention) | +| `ActionUpdate` | `'u'` | Model is being updated (requires PK) | +| `ActionDelete` | `'d'` | Model is being deleted (only requires PK, skips other validation) | + +#### Validation Matrix + | Action | PK + AutoInc | PK without AutoInc | NotNull | Permitted | |--------|--------------|--------------------|---------|-----------| -| `'c'` create | skip (DB assigns) | required | required | applies | -| `'u'` update | required | required | required | applies | -| `'d'` delete | required | required | skip | skip | +| `ActionCreate` | skip (DB assigns) | required | required | applies | +| `ActionUpdate` | required | required | required | applies | +| `ActionDelete` | required | required | skip | skip | | other/unknown | required | required | required | applies | ```go -err := model.ValidateFields('u', myFielder) +err := model.ValidateFields(model.ActionUpdate, myFielder) ``` ## Conversion and Reading Helpers diff --git a/docs/WHY_64BIT_ONLY.md b/docs/WHY_64BIT_ONLY.md index a10aa2d..45bd22d 100644 --- a/docs/WHY_64BIT_ONLY.md +++ b/docs/WHY_64BIT_ONLY.md @@ -62,3 +62,11 @@ truncation bugs. The storage layer agrees: SQLite integers are 64-bit and Postgr silent `int64 → int` narrowing already exists there. Keep `FieldIntSlice` for small enumerations (role lists, option ids); values beyond 32 bits belong in scalar `int64` fields or `FieldBlob`. + +## Removal of `Uint` + +The codec interfaces (`FieldWriter` and `FieldReader`) originally included `Uint` methods. +These were removed to simplify the API and reduce binary size. The single ecosystem-wide +caller (`binary.Message`'s `uint32` correlation ID) was migrated losslessly to `Int`. +Genuine 64-bit unsigned values (like hashes) that might exceed `int64` should be +represented as `FieldText` (hex) or `FieldBlob` (8 bytes). diff --git a/field.go b/field.go index b28bc40..82bcecb 100644 --- a/field.go +++ b/field.go @@ -154,18 +154,36 @@ func (f Field) hasPermittedRules() bool { len(f.NotAllowed) > 0 || f.StartWith != nil } -// ValidateFields validates all fields of a Fielder based on the action ('c', 'u', 'd'). +// CRUD action bytes — the single source of truth for the ecosystem-wide +// action convention used by Validator, ValidateFields, and downstream +// libraries (crudp HTTP mapping, mcp Tool.Action / RBAC, form). +// +// Deliberately plain byte constants (not a named type) so every existing +// `action byte` signature accepts them unchanged. +// +// Not to be confused with orm.Action (an int enum describing query +// execution plans) — that is a different, orm-internal concept. +const ( + ActionCreate byte = 'c' + ActionRead byte = 'r' + ActionUpdate byte = 'u' + ActionDelete byte = 'd' +) + +// ValidateFields validates all fields of a Fielder based on the action. // For each FieldText field, reads the string value and calls Field.Validate. // For non-text fields with NotNull, checks against zero value. // +// Common actions are ActionCreate, ActionUpdate, and ActionDelete. +// // This is the single validation entry point — ormc-generated Validate() // methods call this function first. func ValidateFields(action byte, f Fielder) error { schema := f.Schema() ptrs := f.Pointers() for i, field := range schema { - // 'd' delete: only PK required, skip everything else - if action == 'd' { + // ActionDelete: only PK required, skip everything else + if action == ActionDelete { if field.IsPK() { switch field.Type { case FieldText, FieldRaw: @@ -182,8 +200,8 @@ func ValidateFields(action byte, f Fielder) error { continue } - // 'c' create: skip PK+AutoInc (DB assigns it) - if action == 'c' && field.IsPK() && field.IsAutoInc() { + // ActionCreate: skip PK+AutoInc (DB assigns it) + if action == ActionCreate && field.IsPK() && field.IsAutoInc() { continue } @@ -191,7 +209,7 @@ func ValidateFields(action byte, f Fielder) error { case FieldText, FieldRaw: val, _ := ReadStringPtr(ptrs[i]) - // PK always required (in 'c' without AutoInc, in 'u', and any other) + // PK always required (in ActionCreate without AutoInc, in ActionUpdate, and any other) if field.IsPK() && val == "" { return fmt.Err(field.Name, "required") } diff --git a/tests/codec_test.go b/tests/codec_test.go index 1a503cc..ab0e23a 100644 --- a/tests/codec_test.go +++ b/tests/codec_test.go @@ -33,15 +33,6 @@ func (m *mockFieldWriter) Int(name string, val int64) { m.buf.WriteByte(';') } -func (m *mockFieldWriter) Uint(name string, val uint64) { - m.buf.WriteString(name) - m.buf.WriteByte('=') - m.conv.ResetBuffer(fmt.BuffWork) - m.conv.WrIntBase(fmt.BuffWork, int64(val), 10, false) - m.buf.Write(m.conv.GetBytes(fmt.BuffWork)) - m.buf.WriteByte(';') -} - func (m *mockFieldWriter) Float(name string, val float64) { m.buf.WriteString(name) m.buf.WriteByte('=') @@ -181,18 +172,6 @@ func (r *mockFieldReader) Int(name string) (int64, bool) { return v, true } -func (r *mockFieldReader) Uint(name string) (uint64, bool) { - val, ok := r.find(name) - if !ok { - return 0, false - } - c := fmt.GetConv() - c.LoadBytes(val) - v, _ := c.Uint64() - c.PutConv() - return v, true -} - func (r *mockFieldReader) Float(name string) (float64, bool) { val, ok := r.find(name) if !ok { diff --git a/tests/field_test.go b/tests/field_test.go index 0671bf3..9c72277 100644 --- a/tests/field_test.go +++ b/tests/field_test.go @@ -269,9 +269,9 @@ type fullMock struct { Int int Int32 int32 Int64 int64 - Uint uint - Uint32 uint32 - Uint64 uint64 + U uint + U32 uint32 + U64 uint64 Float float64 Float32 float32 Bool bool @@ -286,9 +286,9 @@ func (m *fullMock) Schema() []Field { {Name: "int", Type: FieldInt, NotNull: true}, {Name: "int32", Type: FieldInt}, {Name: "int64", Type: FieldInt}, - {Name: "uint", Type: FieldInt}, - {Name: "uint32", Type: FieldInt}, - {Name: "uint64", Type: FieldInt}, + {Name: "u", Type: FieldInt}, + {Name: "u32", Type: FieldInt}, + {Name: "u64", Type: FieldInt}, {Name: "float", Type: FieldFloat, NotNull: true}, {Name: "float32", Type: FieldFloat}, {Name: "bool", Type: FieldBool, NotNull: true}, @@ -299,7 +299,7 @@ func (m *fullMock) Schema() []Field { } func (m *fullMock) Pointers() []any { - return []any{&m.Text, &m.Int, &m.Int32, &m.Int64, &m.Uint, &m.Uint32, &m.Uint64, &m.Float, &m.Float32, &m.Bool, &m.Blob, &m.Raw, m.Nested} + return []any{&m.Text, &m.Int, &m.Int32, &m.Int64, &m.U, &m.U32, &m.U64, &m.Float, &m.Float32, &m.Bool, &m.Blob, &m.Raw, m.Nested} } type mockUser struct { @@ -316,6 +316,12 @@ func (m *mockUser) Schema() []Field { func (m *mockUser) Pointers() []any { return []any{&m.id, &m.name} } func (m *mockUser) Validate(action byte) error { return ValidateFields(action, m) } +func TestActionConstantValues(t *testing.T) { + if ActionCreate != 'c' || ActionRead != 'r' || ActionUpdate != 'u' || ActionDelete != 'd' { + t.Fatal("action constants must keep their CRUD byte values — ecosystem wire/RBAC contract") + } +} + func TestValidateFieldsRecursive(t *testing.T) { m := &fullMock{ Text: "hello", @@ -327,20 +333,20 @@ func TestValidateFieldsRecursive(t *testing.T) { Nested: &mockUser{id: "u1", name: "Alice"}, } - if err := ValidateFields('u', m); err != nil { + if err := ValidateFields(ActionUpdate, m); err != nil { t.Errorf("expected success, got %v", err) } // Fail nested m.Nested.name = "" - if err := ValidateFields('u', m); err == nil { + if err := ValidateFields(ActionUpdate, m); err == nil { t.Error("expected failure in nested struct") } // Fail other types m.Nested.name = "Alice" m.Int = 0 // NotNull - if err := ValidateFields('u', m); err == nil { + if err := ValidateFields(ActionUpdate, m); err == nil { t.Error("expected failure for int zero") } } @@ -351,9 +357,9 @@ func TestReadValuesAllTypes(t *testing.T) { Int: 10, Int32: 32, Int64: 64, - Uint: 100, - Uint32: 320, - Uint64: 640, + U: 100, + U32: 320, + U64: 640, Float: 1.1, Float32: 2.2, Bool: true, @@ -527,7 +533,7 @@ func TestValidateFieldsWithOnlyFielder(t *testing.T) { ptrs := []any{sub} // Validate it through the manualFielder helper - if err := ValidateFields('u', &manualFielder{schema, ptrs}); err != nil { + if err := ValidateFields(ActionUpdate, &manualFielder{schema, ptrs}); err != nil { t.Errorf("expected success, got %v", err) } } @@ -562,19 +568,19 @@ func TestValidateFieldsActions(t *testing.T) { f := getFielder(m) // PK+AutoInc should be skipped in 'c' - if err := ValidateFields('c', f); err != nil { - t.Errorf("expected success in 'c' with zero ID, got %v", err) + if err := ValidateFields(ActionCreate, f); err != nil { + t.Errorf("expected success in ActionCreate with zero ID, got %v", err) } // NotNull still applies m.Name = "" - if err := ValidateFields('c', f); err == nil { - t.Error("expected failure in 'c' with empty Name") + if err := ValidateFields(ActionCreate, f); err == nil { + t.Error("expected failure in ActionCreate with empty Name") } m.Name = "Alice" m.Raw = "" - if err := ValidateFields('c', f); err == nil { - t.Error("expected failure in 'c' with empty Raw") + if err := ValidateFields(ActionCreate, f); err == nil { + t.Error("expected failure in ActionCreate with empty Raw") } }) @@ -582,14 +588,14 @@ func TestValidateFieldsActions(t *testing.T) { m := &userActionMock{ID: 1, Name: "Alice", Email: "a@b.com", Raw: "{}", Version: 1} f := getFielder(m) - if err := ValidateFields('u', f); err != nil { - t.Errorf("expected success in 'u', got %v", err) + if err := ValidateFields(ActionUpdate, f); err != nil { + t.Errorf("expected success in ActionUpdate, got %v", err) } // PK is required in 'u' m.ID = 0 - if err := ValidateFields('u', f); err == nil { - t.Error("expected failure in 'u' with zero ID") + if err := ValidateFields(ActionUpdate, f); err == nil { + t.Error("expected failure in ActionUpdate with zero ID") } }) @@ -598,14 +604,14 @@ func TestValidateFieldsActions(t *testing.T) { f := getFielder(m) // Only PK matters in 'd' - if err := ValidateFields('d', f); err != nil { - t.Errorf("expected success in 'd' with only PK, got %v", err) + if err := ValidateFields(ActionDelete, f); err != nil { + t.Errorf("expected success in ActionDelete with only PK, got %v", err) } // PK missing m.ID = 0 - if err := ValidateFields('d', f); err == nil { - t.Error("expected failure in 'd' with missing PK") + if err := ValidateFields(ActionDelete, f); err == nil { + t.Error("expected failure in ActionDelete with missing PK") } }) From 656ded17c54b12f20e40282909f619b2d96f9667 Mon Sep 17 00:00:00 2001 From: Cesar Solis <44058491+cdvelop@users.noreply.github.com> Date: Wed, 8 Jul 2026 11:44:15 -0400 Subject: [PATCH 2/2] review: corrections before merge --- docs/{PLAN.md => CHECK_PLAN.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/{PLAN.md => CHECK_PLAN.md} (100%) diff --git a/docs/PLAN.md b/docs/CHECK_PLAN.md similarity index 100% rename from docs/PLAN.md rename to docs/CHECK_PLAN.md