From 31e6a41abb1031d23001f237f2b9fcb3e8164500 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 18 Sep 2025 16:41:16 +0200 Subject: [PATCH 1/5] internal: structpath: accurate struct <-> string mapping Previously structpath could represent AnyKey and AnyIndex as struct but as string they were both represented as field[*]. On the other hand, there was no unambigous representation of x.*. This commit fixes struct mapping to match string representation 1-1 so that Parse(x.String()) == x. --- bundle/direct/dresources/all_test.go | 8 +- bundle/internal/validation/enum.go | 3 +- bundle/internal/validation/required.go | 3 +- libs/structs/dynpath/dynpath.go | 101 ++++++++++++ libs/structs/dynpath/dynpath_test.go | 202 +++++++++++++++++++++++ libs/structs/structaccess/typecheck.go | 22 ++- libs/structs/structdiff/diff.go | 11 +- libs/structs/structpath/path.go | 126 +++++++------- libs/structs/structpath/path_test.go | 188 +++++++++------------ libs/structs/structwalk/walk.go | 8 +- libs/structs/structwalk/walktype.go | 11 +- libs/structs/structwalk/walktype_test.go | 10 +- 12 files changed, 501 insertions(+), 192 deletions(-) create mode 100644 libs/structs/dynpath/dynpath.go create mode 100644 libs/structs/dynpath/dynpath_test.go diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index 18af7bf4e9..7fee2829b7 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/structs/dynpath" "github.com/databricks/cli/libs/structs/structaccess" "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/structs/structwalk" @@ -155,9 +156,10 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W require.NotNil(t, remappedState) require.NoError(t, structwalk.Walk(newState, func(path *structpath.PathNode, val any, field *reflect.StructField) { - remoteValue, err := structaccess.Get(remappedState, dyn.MustPathFromString(path.DynPath())) + dynPath := dynpath.ConvertPathNodeToDynPath(path, reflect.TypeOf(newState)) + remoteValue, err := structaccess.Get(remappedState, dyn.MustPathFromString(dynPath)) if err != nil { - t.Errorf("Failed to read %s from remapped remote state %#v", path.DynPath(), remappedState) + t.Errorf("Failed to read %s from remapped remote state %#v", dynPath, remappedState) } if val == nil { // t.Logf("Ignoring %s nil, remoteValue=%#v", path.String(), remoteValue) @@ -172,7 +174,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W // t.Logf("Testing %s v=%#v, remoteValue=%#v", path.String(), val, remoteValue) // We expect fields set explicitly to be preserved by testserver, which is true for all resources as of today. // If not true for your resource, add exception here: - assert.Equal(t, val, remoteValue, path.DynPath()) + assert.Equal(t, val, remoteValue, dynPath) })) err = adapter.DoDelete(ctx, createdID) diff --git a/bundle/internal/validation/enum.go b/bundle/internal/validation/enum.go index cd5bfc2044..d48cfa9dc2 100644 --- a/bundle/internal/validation/enum.go +++ b/bundle/internal/validation/enum.go @@ -11,6 +11,7 @@ import ( "text/template" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/structs/dynpath" "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/structs/structtag" "github.com/databricks/cli/libs/structs/structwalk" @@ -132,7 +133,7 @@ func extractEnumFields(typ reflect.Type) ([]EnumPatternInfo, error) { return true } - fieldPath := path.DynPath() + fieldPath := dynpath.ConvertPathNodeToDynPath(path, typ) fieldsByPattern[fieldPath] = enumValues } return true diff --git a/bundle/internal/validation/required.go b/bundle/internal/validation/required.go index 72e8f15fab..8de2f0e589 100644 --- a/bundle/internal/validation/required.go +++ b/bundle/internal/validation/required.go @@ -11,6 +11,7 @@ import ( "text/template" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/structs/dynpath" "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/structs/structtag" "github.com/databricks/cli/libs/structs/structwalk" @@ -68,7 +69,7 @@ func extractRequiredFields(typ reflect.Type) ([]RequiredPatternInfo, error) { return true } - parentPath := path.Parent().DynPath() + parentPath := dynpath.ConvertPathNodeToDynPath(path.Parent(), typ) fieldsByPattern[parentPath] = append(fieldsByPattern[parentPath], fieldName) return true }) diff --git a/libs/structs/dynpath/dynpath.go b/libs/structs/dynpath/dynpath.go new file mode 100644 index 0000000000..50744919c1 --- /dev/null +++ b/libs/structs/dynpath/dynpath.go @@ -0,0 +1,101 @@ +package dynpath + +import ( + "reflect" + "strconv" + "strings" + + "github.com/databricks/cli/libs/structs/structaccess" + "github.com/databricks/cli/libs/structs/structpath" +) + +// ConvertPathNodeToDynPath converts a PathNode to dyn path format string. +// Uses the provided root type to determine context-aware wildcard formatting: +// - BracketStar accessing maps renders as "parent.*" +// - BracketStar accessing arrays/slices renders as "parent[*]" +// - DotStar always renders as "parent.*" +func ConvertPathNodeToDynPath(path *structpath.PathNode, rootType reflect.Type) string { + if path == nil { + return "" + } + + segments := path.AsSlice() + var result strings.Builder + currentType := rootType + + for _, segment := range segments { + // Dereference pointers + for currentType != nil && currentType.Kind() == reflect.Pointer { + currentType = currentType.Elem() + } + + if index, ok := segment.Index(); ok { + // Array/slice index access + result.WriteString("[") + result.WriteString(strconv.Itoa(index)) + result.WriteString("]") + if currentType != nil && (currentType.Kind() == reflect.Array || currentType.Kind() == reflect.Slice) { + currentType = currentType.Elem() + } else { + currentType = nil + } + + } else if field, ok := segment.Field(); ok { + // Struct field access + if result.Len() > 0 { + result.WriteString(".") + } + result.WriteString(field) + if currentType != nil && currentType.Kind() == reflect.Struct { + if sf, _, ok := structaccess.FindStructFieldByKeyType(currentType, field); ok { + currentType = sf.Type + } else { + currentType = nil + } + } else { + currentType = nil + } + + } else if key, ok := segment.MapKey(); ok { + // Map key access - always use dot notation in DynPath + if result.Len() > 0 { + result.WriteString(".") + } + result.WriteString(key) + if currentType != nil && currentType.Kind() == reflect.Map { + currentType = currentType.Elem() + } else { + currentType = nil + } + + } else if segment.DotStar() { + // Field wildcard - always uses dot notation + if result.Len() > 0 { + result.WriteString(".") + } + result.WriteString("*") + // Type doesn't change for wildcards + + } else if segment.BracketStar() { + // Context-aware wildcard rendering + if currentType != nil && currentType.Kind() == reflect.Map { + // Map wildcard uses dot notation in DynPath + if result.Len() > 0 { + result.WriteString(".") + } + result.WriteString("*") + currentType = currentType.Elem() + } else { + // Array/slice wildcard or fallback uses bracket notation + result.WriteString("[*]") + if currentType != nil && (currentType.Kind() == reflect.Array || currentType.Kind() == reflect.Slice) { + currentType = currentType.Elem() + } else { + currentType = nil + } + } + } + } + + return result.String() +} diff --git a/libs/structs/dynpath/dynpath_test.go b/libs/structs/dynpath/dynpath_test.go new file mode 100644 index 0000000000..0a89e6d966 --- /dev/null +++ b/libs/structs/dynpath/dynpath_test.go @@ -0,0 +1,202 @@ +package dynpath + +import ( + "reflect" + "testing" + + "github.com/databricks/cli/libs/structs/structpath" + "github.com/stretchr/testify/assert" +) + +func TestConvertPathNodeToDynPath(t *testing.T) { + tests := []struct { + name string + structPath string // PathNode string representation to be parsed + dynPath string // Expected DynPath format + rootType reflect.Type // optional, for context-aware wildcard rendering + }{ + // Basic node types + { + name: "nil path", + structPath: "", + dynPath: "", + }, + { + name: "array index", + structPath: "[5]", + dynPath: "[5]", + }, + { + name: "map key", + structPath: "['mykey']", + dynPath: "mykey", + }, + { + name: "struct field with JSON tag", + structPath: "json_name", + dynPath: "json_name", + }, + { + name: "struct field without JSON tag", + structPath: "GoFieldName", + dynPath: "GoFieldName", + }, + { + name: "dot star", + structPath: "*", + dynPath: "*", + }, + { + name: "bracket star - array type", + structPath: "[*]", + rootType: reflect.TypeOf([]int{}), + dynPath: "[*]", + }, + { + name: "bracket star - map type", + structPath: "[*]", + rootType: reflect.TypeOf(map[string]int{}), + dynPath: "*", + }, + + // Compound paths + { + name: "struct field -> array index", + structPath: "items[3]", + dynPath: "items[3]", + }, + { + name: "struct field -> map key", + structPath: "config['database']", + dynPath: "config.database", + }, + { + name: "struct field -> struct field", + structPath: "user.name", + dynPath: "user.name", + }, + { + name: "map key -> array index", + structPath: "['servers'][0]", + dynPath: "servers[0]", + }, + { + name: "map key -> struct field", + structPath: "['primary'].host", + dynPath: "primary.host", + }, + { + name: "array index -> struct field", + structPath: "[2].id", + dynPath: "[2].id", + }, + { + name: "array index -> map key", + structPath: "[1]['status']", + dynPath: "[1].status", + }, + + // Wildcard combinations + { + name: "dot star with parent", + structPath: "Parent.*", + dynPath: "Parent.*", + }, + { + name: "bracket star with parent - array", + structPath: "Parent[*]", + rootType: reflect.TypeOf(struct{ Parent []int }{}), + dynPath: "Parent[*]", + }, + { + name: "bracket star with parent - map", + structPath: "Parent[*]", + rootType: reflect.TypeOf(struct{ Parent map[string]int }{}), + dynPath: "Parent.*", + }, + + // Special characters and edge cases + { + name: "map key with single quote", + structPath: "['key''s']", + dynPath: "key's", + }, + { + name: "map key with multiple single quotes", + structPath: "['''''']", + dynPath: "''", + }, + { + name: "empty map key", + structPath: "['']", + dynPath: "", + }, + { + name: "map key with reserved characters", + structPath: "['key\x00[],`']", + dynPath: "key\x00[],`", + }, + { + name: "field with special characters", + structPath: "field@name:with#symbols!", + dynPath: "field@name:with#symbols!", + }, + { + name: "field with spaces", + structPath: "field with spaces", + dynPath: "field with spaces", + }, + { + name: "field with unicode", + structPath: "名前🙂", + dynPath: "名前🙂", + }, + + // Complex real-world example + { + name: "complex nested path", + structPath: "user.settings['theme'][0].color", + dynPath: "user.settings.theme[0].color", + }, + + // Mixed JSON tag and Go field name scenarios + { + name: "Go field -> JSON field", + structPath: "Parent.child_name", + dynPath: "Parent.child_name", + }, + { + name: "JSON field -> Go field", + structPath: "parent.ChildName", + dynPath: "parent.ChildName", + }, + { + name: "dash JSON tag field", + structPath: "-", + dynPath: "-", + }, + { + name: "JSON tag with options", + structPath: "lazy_field", + dynPath: "lazy_field", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Parse the structPath string into a PathNode + var pathNode *structpath.PathNode + var err error + if tt.structPath == "" { + pathNode = nil + } else { + pathNode, err = structpath.Parse(tt.structPath) + assert.NoError(t, err, "Failed to parse structPath: %s", tt.structPath) + } + + // Convert to DynPath and verify result + result := ConvertPathNodeToDynPath(pathNode, tt.rootType) + assert.Equal(t, tt.dynPath, result, "ConvertPathNodeToDynPath conversion should match expected DynPath format") + }) + } +} diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index a7b804c748..2582c963cd 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -51,7 +51,7 @@ func Validate(t reflect.Type, path dyn.Path) error { switch cur.Kind() { case reflect.Struct: - sf, _, ok := findStructFieldByKeyType(cur, c.Key()) + sf, _, ok := FindStructFieldByKeyType(cur, c.Key()) if !ok { return fmt.Errorf("%s: field %q not found in %s", newPrefix, c.Key(), cur.String()) } @@ -83,11 +83,15 @@ func Validate(t reflect.Type, path dyn.Path) error { return nil } -// findStructFieldByKeyType searches exported fields of struct type t for a field matching key. +// FindStructFieldByKeyType searches exported fields of struct type t for a field matching key. // It matches json tag name (when present and not "-") only. // It also searches embedded anonymous structs (pointer or value) recursively. // Returns the StructField, the declaring owner type, and whether it was found. -func findStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, reflect.Type, bool) { +func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, reflect.Type, bool) { + if t.Kind() != reflect.Struct { + return reflect.StructField{}, reflect.TypeOf(nil), false + } + // First pass: direct fields for i := range t.NumField() { sf := t.Field(i) @@ -106,6 +110,16 @@ func findStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, } return sf, t, true } + + // Fallback to Go field name when no JSON tag + if name == "" && sf.Name == key { + // Skip fields marked as internal/readonly + btag := structtag.BundleTag(sf.Tag.Get("bundle")) + if btag.Internal() || btag.ReadOnly() { + continue + } + return sf, t, true + } } // Second pass: search embedded anonymous structs recursively (flattening semantics) @@ -121,7 +135,7 @@ func findStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, if ft.Kind() != reflect.Struct { continue } - if osf, owner, ok := findStructFieldByKeyType(ft, key); ok { + if osf, owner, ok := FindStructFieldByKeyType(ft, key); ok { // Skip fields marked as internal/readonly btag := structtag.BundleTag(osf.Tag.Get("bundle")) if btag.Internal() || btag.ReadOnly() { diff --git a/libs/structs/structdiff/diff.go b/libs/structs/structdiff/diff.go index ef1e5056b0..9b24b02192 100644 --- a/libs/structs/structdiff/diff.go +++ b/libs/structs/structdiff/diff.go @@ -123,7 +123,15 @@ func diffStruct(path *structpath.PathNode, s1, s2 reflect.Value, changes *[]Chan continue } - node := structpath.NewStructField(path, sf.Tag, sf.Name) + jsonTag := structtag.JSONTag(sf.Tag.Get("json")) + + // Resolve field name from JSON tag or fall back to Go field name + fieldName := jsonTag.Name() + if fieldName == "" { + fieldName = sf.Name + } + node := structpath.NewStructField(path, fieldName) + v1Field := s1.Field(i) v2Field := s2.Field(i) @@ -131,7 +139,6 @@ func diffStruct(path *structpath.PathNode, s1, s2 reflect.Value, changes *[]Chan zero2 := v2Field.IsZero() if zero1 || zero2 { - jsonTag := structtag.JSONTag(sf.Tag.Get("json")) if jsonTag.OmitEmpty() { if zero1 { if !slices.Contains(forced1, sf.Name) { diff --git a/libs/structs/structpath/path.go b/libs/structs/structpath/path.go index dd12b62972..25d411cedd 100644 --- a/libs/structs/structpath/path.go +++ b/libs/structs/structpath/path.go @@ -3,18 +3,15 @@ package structpath import ( "errors" "fmt" - "reflect" "strconv" "strings" - - "github.com/databricks/cli/libs/structs/structtag" ) const ( - tagStruct = -1 - tagMapKey = -2 - tagAnyKey = -4 - tagAnyIndex = -5 + tagStruct = -1 + tagMapKey = -2 + tagDotStar = -4 + tagBracketStar = -5 ) // PathNode represents a node in a path for struct diffing. @@ -51,18 +48,18 @@ func (p *PathNode) MapKey() (string, bool) { return "", false } -func (p *PathNode) AnyKey() bool { +func (p *PathNode) DotStar() bool { if p == nil { return false } - return p.index == tagAnyKey + return p.index == tagDotStar } -func (p *PathNode) AnyIndex() bool { +func (p *PathNode) BracketStar() bool { if p == nil { return false } - return p.index == tagAnyIndex + return p.index == tagBracketStar } func (p *PathNode) Field() (string, bool) { @@ -82,6 +79,34 @@ func (p *PathNode) Parent() *PathNode { return p.prev } +// AsSlice returns the path as a slice of PathNodes from root to current. +// Efficiently pre-allocates the exact length and fills in reverse order. +func (p *PathNode) AsSlice() []*PathNode { + if p == nil { + return nil + } + + // First pass: count the length + length := 0 + current := p + for current != nil { + length++ + current = current.Parent() + } + + // Allocate slice with exact capacity + segments := make([]*PathNode, length) + + // Second pass: fill in reverse order (from end to start) + current = p + for i := length - 1; i >= 0; i-- { + segments[i] = current + current = current.Parent() + } + + return segments +} + // NewIndex creates a new PathNode for an array/slice index. func NewIndex(prev *PathNode, index int) *PathNode { if index < 0 { @@ -103,33 +128,26 @@ func NewMapKey(prev *PathNode, key string) *PathNode { } // NewStructField creates a new PathNode for a struct field. -// The jsonTag is used for JSON key resolution, and fieldName is used as fallback. -func NewStructField(prev *PathNode, tag reflect.StructTag, fieldName string) *PathNode { - jsonTag := structtag.JSONTag(tag.Get("json")) - - key := fieldName - if name := jsonTag.Name(); name != "" { - key = name - } - +// The fieldName should be the resolved field name (e.g., from JSON tag or Go field name). +func NewStructField(prev *PathNode, fieldName string) *PathNode { return &PathNode{ prev: prev, - key: key, + key: fieldName, index: tagStruct, } } -func NewAnyKey(prev *PathNode) *PathNode { +func NewDotStar(prev *PathNode) *PathNode { return &PathNode{ prev: prev, - index: tagAnyKey, + index: tagDotStar, } } -func NewAnyIndex(prev *PathNode) *PathNode { +func NewBracketStar(prev *PathNode) *PathNode { return &PathNode{ prev: prev, - index: tagAnyIndex, + index: tagBracketStar, } } @@ -149,8 +167,20 @@ func (p *PathNode) String() string { return p.prev.String() + "[" + strconv.Itoa(p.index) + "]" } - if p.index == tagAnyKey || p.index == tagAnyIndex { - return p.prev.String() + "[*]" + if p.index == tagDotStar { + prev := p.prev.String() + if prev == "" { + return "*" + } + return prev + ".*" + } + + if p.index == tagBracketStar { + prev := p.prev.String() + if prev == "" { + return "[*]" + } + return prev + "[*]" } if p.index == tagStruct { @@ -240,11 +270,11 @@ func Parse(s string) (*PathNode, error) { case stateField: if ch == '.' { - result = NewStructField(result, reflect.StructTag(""), currentToken.String()) + result = NewStructField(result, currentToken.String()) currentToken.Reset() state = stateFieldStart } else if ch == '[' { - result = NewStructField(result, reflect.StructTag(""), currentToken.String()) + result = NewStructField(result, currentToken.String()) currentToken.Reset() state = stateBracketOpen } else if !isReservedFieldChar(ch) { @@ -305,9 +335,7 @@ func Parse(s string) (*PathNode, error) { case stateWildcard: if ch == ']' { - // Note, since we're parsing this without type info present, we don't know if it's AnyKey or AnyIndex - // Perhaps structpath should be simplified to have Wildcard as merged representation of AnyKey/AnyIndex - result = NewAnyKey(result) + result = NewBracketStar(result) state = stateExpectDotOrEnd } else { return nil, fmt.Errorf("unexpected character '%c' after '*' at position %d", ch, pos) @@ -338,7 +366,7 @@ func Parse(s string) (*PathNode, error) { case stateStart: return result, nil // Empty path, result is nil case stateField: - result = NewStructField(result, reflect.StructTag(""), currentToken.String()) + result = NewStructField(result, currentToken.String()) return result, nil case stateExpectDotOrEnd: return result, nil @@ -380,35 +408,3 @@ func isReservedFieldChar(ch byte) bool { return false } } - -// Path in libs/dyn format -func (p *PathNode) DynPath() string { - if p == nil { - return "" - } - - if p.index >= 0 { - return p.prev.DynPath() + "[" + strconv.Itoa(p.index) + "]" - } - - if p.index == tagAnyKey { - prev := p.prev.DynPath() - if prev == "" { - return "*" - } else { - return prev + ".*" - } - } - - if p.index == tagAnyIndex { - return p.prev.DynPath() + "[*]" - } - - // Both struct fields and map keys use dot notation in DynPath - prev := p.prev.DynPath() - if prev == "" { - return p.key - } else { - return prev + "." + p.key - } -} diff --git a/libs/structs/structpath/path_test.go b/libs/structs/structpath/path_test.go index a2be306a94..0bb2543be1 100644 --- a/libs/structs/structpath/path_test.go +++ b/libs/structs/structpath/path_test.go @@ -1,7 +1,6 @@ package structpath import ( - "reflect" "testing" "github.com/stretchr/testify/assert" @@ -9,17 +8,15 @@ import ( func TestPathNode(t *testing.T) { tests := []struct { - name string - node *PathNode - String string - DynPath string // Only set when different from String - IgnoreDynPath bool // Do not test DynPath - Index any - MapKey any - Field any - Root any - AnyKey bool - AnyIndex bool + name string + node *PathNode + String string + Index any + MapKey any + Field any + Root any + DotStar bool + BracketStar bool }{ // Single node tests { @@ -35,137 +32,127 @@ func TestPathNode(t *testing.T) { Index: 5, }, { - name: "map key", - node: NewMapKey(nil, "mykey"), - String: `['mykey']`, - DynPath: "mykey", - MapKey: "mykey", + name: "map key", + node: NewMapKey(nil, "mykey"), + String: `['mykey']`, + MapKey: "mykey", }, { name: "struct field with JSON tag", - node: NewStructField(nil, reflect.StructTag(`json:"json_name"`), "GoFieldName"), + node: NewStructField(nil, "json_name"), String: "json_name", Field: "json_name", }, { name: "struct field without JSON tag (fallback to Go name)", - node: NewStructField(nil, reflect.StructTag(""), "GoFieldName"), + node: NewStructField(nil, "GoFieldName"), String: "GoFieldName", Field: "GoFieldName", }, { name: "struct field with dash JSON tag", - node: NewStructField(nil, reflect.StructTag(`json:"-"`), "GoFieldName"), + node: NewStructField(nil, "-"), String: "-", Field: "-", }, { name: "struct field with JSON tag options", - node: NewStructField(nil, reflect.StructTag(`json:"lazy_field,omitempty"`), "LazyField"), + node: NewStructField(nil, "lazy_field"), String: "lazy_field", Field: "lazy_field", }, { - name: "any key", - node: NewAnyKey(nil), - String: "[*]", - DynPath: "*", - AnyKey: true, + name: "dot star", + node: NewDotStar(nil), + String: "*", + DotStar: true, }, { - name: "any index", - node: NewAnyIndex(nil), - String: "[*]", - AnyIndex: true, + name: "bracket star", + node: NewBracketStar(nil), + String: "[*]", + BracketStar: true, }, // Two node tests { name: "struct field -> array index", - node: NewIndex(NewStructField(nil, reflect.StructTag(`json:"items"`), "Items"), 3), + node: NewIndex(NewStructField(nil, "items"), 3), String: "items[3]", Index: 3, }, { - name: "struct field -> map key", - node: NewMapKey(NewStructField(nil, reflect.StructTag(`json:"config"`), "Config"), "database"), - String: `config['database']`, - DynPath: "config.database", - MapKey: "database", + name: "struct field -> map key", + node: NewMapKey(NewStructField(nil, "config"), "database"), + String: `config['database']`, + MapKey: "database", }, { name: "struct field -> struct field", - node: NewStructField(NewStructField(nil, reflect.StructTag(`json:"user"`), "User"), reflect.StructTag(`json:"name"`), "Name"), + node: NewStructField(NewStructField(nil, "user"), "name"), String: "user.name", Field: "name", }, { - name: "map key -> array index", - node: NewIndex(NewMapKey(nil, "servers"), 0), - String: `['servers'][0]`, - DynPath: "servers[0]", - Index: 0, + name: "map key -> array index", + node: NewIndex(NewMapKey(nil, "servers"), 0), + String: `['servers'][0]`, + Index: 0, }, { - name: "map key -> struct field", - node: NewStructField(NewMapKey(nil, "primary"), reflect.StructTag(`json:"host"`), "Host"), - String: `['primary'].host`, - DynPath: `primary.host`, - Field: "host", + name: "map key -> struct field", + node: NewStructField(NewMapKey(nil, "primary"), "host"), + String: `['primary'].host`, + Field: "host", }, { name: "array index -> struct field", - node: NewStructField(NewIndex(nil, 2), reflect.StructTag(`json:"id"`), "ID"), + node: NewStructField(NewIndex(nil, 2), "id"), String: "[2].id", Field: "id", }, { - name: "array index -> map key", - node: NewMapKey(NewIndex(nil, 1), "status"), - String: `[1]['status']`, - DynPath: "[1].status", - MapKey: "status", + name: "array index -> map key", + node: NewMapKey(NewIndex(nil, 1), "status"), + String: `[1]['status']`, + MapKey: "status", }, { name: "struct field without JSON tag -> struct field with JSON tag", - node: NewStructField(NewStructField(nil, reflect.StructTag(""), "Parent"), reflect.StructTag(`json:"child_name"`), "ChildName"), + node: NewStructField(NewStructField(nil, "Parent"), "child_name"), String: "Parent.child_name", Field: "child_name", }, { - name: "any key", - node: NewAnyKey(NewStructField(nil, reflect.StructTag(""), "Parent")), - String: "Parent[*]", - DynPath: "Parent.*", - AnyKey: true, + name: "dot star with parent", + node: NewDotStar(NewStructField(nil, "Parent")), + String: "Parent.*", + DotStar: true, }, { - name: "any index", - node: NewAnyIndex(NewStructField(nil, reflect.StructTag(""), "Parent")), - String: "Parent[*]", - AnyIndex: true, + name: "bracket star with parent", + node: NewBracketStar(NewStructField(nil, "Parent")), + String: "Parent[*]", + BracketStar: true, }, // Edge cases with special characters in map keys { - name: "map key with single quote", - node: NewMapKey(nil, "key's"), - String: `['key''s']`, - DynPath: "key's", - MapKey: "key's", + name: "map key with single quote", + node: NewMapKey(nil, "key's"), + String: `['key''s']`, + MapKey: "key's", }, { - name: "map key with multiple single quotes", - node: NewMapKey(nil, "''"), - String: `['''''']`, - DynPath: "''", - MapKey: "''", + name: "map key with multiple single quotes", + node: NewMapKey(nil, "''"), + String: `['''''']`, + MapKey: "''", }, { - name: "empty map key", - node: NewMapKey(nil, ""), - String: `['']`, - IgnoreDynPath: true, - MapKey: "", + name: "empty map key", + node: NewMapKey(nil, ""), + String: `['']`, + MapKey: "", }, { name: "complex path", @@ -173,45 +160,43 @@ func TestPathNode(t *testing.T) { NewIndex( NewMapKey( NewStructField( - NewStructField(nil, reflect.StructTag(`json:"user"`), "User"), - reflect.StructTag(`json:"settings"`), "Settings"), + NewStructField(nil, "user"), + "settings"), "theme"), 0), - reflect.StructTag(`json:"color"`), "Color"), - String: "user.settings['theme'][0].color", - DynPath: "user.settings.theme[0].color", - Field: "color", + "color"), + String: "user.settings['theme'][0].color", + Field: "color", }, { name: "field with special characters", - node: NewStructField(nil, reflect.StructTag(""), "field@name:with#symbols!"), + node: NewStructField(nil, "field@name:with#symbols!"), String: "field@name:with#symbols!", Field: "field@name:with#symbols!", }, { name: "field with spaces", - node: NewStructField(nil, reflect.StructTag(""), "field with spaces"), + node: NewStructField(nil, "field with spaces"), String: "field with spaces", Field: "field with spaces", }, { name: "field starting with digit", - node: NewStructField(nil, reflect.StructTag(""), "123field"), + node: NewStructField(nil, "123field"), String: "123field", Field: "123field", }, { name: "field with unicode", - node: NewStructField(nil, reflect.StructTag(""), "名前🙂"), + node: NewStructField(nil, "名前🙂"), String: "名前🙂", Field: "名前🙂", }, { - name: "map key with reserved characters", - node: NewMapKey(nil, "key\x00[],`"), - String: "['key\x00[],`']", - DynPath: "key\x00[],`", - MapKey: "key\x00[],`", + name: "map key with reserved characters", + node: NewMapKey(nil, "key\x00[],`"), + String: "['key\x00[],`']", + MapKey: "key\x00[],`", }, } @@ -229,17 +214,6 @@ func TestPathNode(t *testing.T) { assert.Equal(t, tt.String, roundtripResult, "Roundtrip conversion should be identical") } - if !tt.IgnoreDynPath { - dynResult := tt.node.DynPath() - expectedDyn := tt.String - if tt.DynPath != "" { - expectedDyn = tt.DynPath - // Enforce rule: DynPath should only be set when different from String - assert.NotEqual(t, expectedDyn, tt.String, "Test case %q: DynPath should only be set when different from String", tt.name) - } - assert.Equal(t, expectedDyn, dynResult, "DynPath() method") - } - // Index gotIndex, isIndex := tt.node.Index() if tt.Index == nil { @@ -281,9 +255,9 @@ func TestPathNode(t *testing.T) { assert.True(t, isRoot) } - // AnyKey, AnyIndex - assert.Equal(t, tt.AnyKey, tt.node.AnyKey()) - assert.Equal(t, tt.AnyIndex, tt.node.AnyIndex()) + // DotStar and BracketStar + assert.Equal(t, tt.DotStar, tt.node.DotStar()) + assert.Equal(t, tt.BracketStar, tt.node.BracketStar()) }) } } diff --git a/libs/structs/structwalk/walk.go b/libs/structs/structwalk/walk.go index 4eeaed348f..4acf6d2ba0 100644 --- a/libs/structs/structwalk/walk.go +++ b/libs/structs/structwalk/walk.go @@ -115,12 +115,18 @@ func walkStruct(path *structpath.PathNode, s reflect.Value, visit VisitFunc) { continue } - node := structpath.NewStructField(path, sf.Tag, sf.Name) jsonTag := structtag.JSONTag(sf.Tag.Get("json")) if jsonTag.Name() == "-" { continue // skip fields without json name } + // Resolve field name from JSON tag or fall back to Go field name + fieldName := jsonTag.Name() + if fieldName == "" { + fieldName = sf.Name + } + node := structpath.NewStructField(path, fieldName) + fieldVal := s.Field(i) // Skip zero values with omitempty unless field is explicitly forced. if jsonTag.OmitEmpty() && fieldVal.IsZero() && !slices.Contains(forced, sf.Name) { diff --git a/libs/structs/structwalk/walktype.go b/libs/structs/structwalk/walktype.go index d1f5a361af..8d9ddcf20b 100644 --- a/libs/structs/structwalk/walktype.go +++ b/libs/structs/structwalk/walktype.go @@ -84,14 +84,14 @@ func walkTypeValue(path *structpath.PathNode, typ reflect.Type, field *reflect.S walkTypeStruct(path, typ, visit, visitedCount) case reflect.Slice, reflect.Array: - walkTypeValue(structpath.NewAnyIndex(path), typ.Elem(), nil, visit, visitedCount) + walkTypeValue(structpath.NewBracketStar(path), typ.Elem(), nil, visit, visitedCount) case reflect.Map: if typ.Key().Kind() != reflect.String { return // unsupported map key type } // For maps, we walk the value type directly at the current path - walkTypeValue(structpath.NewAnyKey(path), typ.Elem(), nil, visit, visitedCount) + walkTypeValue(structpath.NewBracketStar(path), typ.Elem(), nil, visit, visitedCount) default: // func, chan, interface, invalid, etc. -> ignore @@ -122,7 +122,12 @@ func walkTypeStruct(path *structpath.PathNode, st reflect.Type, visit VisitTypeF continue } - node := structpath.NewStructField(path, sf.Tag, sf.Name) + // Resolve field name from JSON tag or fall back to Go field name + fieldName := jsonTagName + if fieldName == "" { + fieldName = sf.Name + } + node := structpath.NewStructField(path, fieldName) walkTypeValue(node, sf.Type, &sf, visit, visitedCount) } } diff --git a/libs/structs/structwalk/walktype_test.go b/libs/structs/structwalk/walktype_test.go index 4b1ce085b0..f49496f26c 100644 --- a/libs/structs/structwalk/walktype_test.go +++ b/libs/structs/structwalk/walktype_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/structs/dynpath" "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/assert" @@ -29,8 +30,7 @@ func getScalarFields(t *testing.T, typ reflect.Type) map[string]any { pathNew, err := structpath.Parse(s) if assert.NoError(t, err, "Parse(path.String()) failed for %q: %s", s, err) { newS := pathNew.String() - // This still does not work because of AnyKey / AnyIndex ambiguity - // assert.Equal(t, path, pathNew, "Parse(path.String()) returned different path;\npath=%#v %q\npathNew=%#v %q", path, s, pathNew, newS) + assert.Equal(t, path, pathNew, "Parse(path.String()) returned different path;\npath=%#v %q\npathNew=%#v %q", path, s, pathNew, newS) assert.Equal(t, s, newS, "Parse(path.String()).String() is different from path.String()\npath.String()=%q\npathNew.String()=%q", path, pathNew) } @@ -166,15 +166,15 @@ func TestTypeRoot(t *testing.T) { ) } -func getReadonlyFields(t *testing.T, typ reflect.Type) []string { +func getReadonlyFields(t *testing.T, rootType reflect.Type) []string { var results []string - err := WalkType(typ, func(path *structpath.PathNode, typ reflect.Type, field *reflect.StructField) (continueWalk bool) { + err := WalkType(rootType, func(path *structpath.PathNode, typ reflect.Type, field *reflect.StructField) (continueWalk bool) { if path == nil || field == nil { return true } bundleTag := field.Tag.Get("bundle") if strings.Contains(bundleTag, "readonly") { - results = append(results, path.DynPath()) + results = append(results, dynpath.ConvertPathNodeToDynPath(path, rootType)) } return true }) From 249a7d6f269809475102f4671a7c51e11fb2b9d5 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 19 Sep 2025 11:14:11 +0200 Subject: [PATCH 2/5] wip --- libs/structs/dynpath/dynpath.go | 3 + libs/structs/dynpath/dynpath_test.go | 135 +++++++++++++-------------- 2 files changed, 67 insertions(+), 71 deletions(-) diff --git a/libs/structs/dynpath/dynpath.go b/libs/structs/dynpath/dynpath.go index 50744919c1..2fcacb7b78 100644 --- a/libs/structs/dynpath/dynpath.go +++ b/libs/structs/dynpath/dynpath.go @@ -1,6 +1,8 @@ package dynpath import ( + "fmt" + "os" "reflect" "strconv" "strings" @@ -18,6 +20,7 @@ func ConvertPathNodeToDynPath(path *structpath.PathNode, rootType reflect.Type) if path == nil { return "" } + fmt.Fprintf(os.Stderr, "pathg=%#v\n", path) segments := path.AsSlice() var result strings.Builder diff --git a/libs/structs/dynpath/dynpath_test.go b/libs/structs/dynpath/dynpath_test.go index 0a89e6d966..8dcbc3265c 100644 --- a/libs/structs/dynpath/dynpath_test.go +++ b/libs/structs/dynpath/dynpath_test.go @@ -32,15 +32,10 @@ func TestConvertPathNodeToDynPath(t *testing.T) { dynPath: "mykey", }, { - name: "struct field with JSON tag", + name: "struct field", structPath: "json_name", dynPath: "json_name", }, - { - name: "struct field without JSON tag", - structPath: "GoFieldName", - dynPath: "GoFieldName", - }, { name: "dot star", structPath: "*", @@ -60,41 +55,42 @@ func TestConvertPathNodeToDynPath(t *testing.T) { }, // Compound paths - { - name: "struct field -> array index", - structPath: "items[3]", - dynPath: "items[3]", - }, - { - name: "struct field -> map key", - structPath: "config['database']", - dynPath: "config.database", - }, - { - name: "struct field -> struct field", - structPath: "user.name", - dynPath: "user.name", - }, - { - name: "map key -> array index", - structPath: "['servers'][0]", - dynPath: "servers[0]", - }, - { - name: "map key -> struct field", - structPath: "['primary'].host", - dynPath: "primary.host", - }, - { - name: "array index -> struct field", - structPath: "[2].id", - dynPath: "[2].id", - }, - { - name: "array index -> map key", - structPath: "[1]['status']", - dynPath: "[1].status", - }, + /* + { + name: "struct field -> array index", + structPath: "items[3]", + dynPath: "items[3]", + }, + { + name: "struct field -> map key", + structPath: "config['database']", + dynPath: "config.database", + }, + { + name: "struct field -> struct field", + structPath: "user.name", + dynPath: "user.name", + }, + { + name: "map key -> array index", + structPath: "['servers'][0]", + dynPath: "servers[0]", + }, + { + name: "map key -> struct field", + structPath: "['primary'].host", + dynPath: "primary.host", + }, + { + name: "array index -> struct field", + structPath: "[2].id", + dynPath: "[2].id", + }, + { + name: "array index -> map key", + structPath: "[1]['status']", + dynPath: "[1].status", + },*/ // Wildcard combinations { @@ -155,44 +151,41 @@ func TestConvertPathNodeToDynPath(t *testing.T) { // Complex real-world example { name: "complex nested path", - structPath: "user.settings['theme'][0].color", - dynPath: "user.settings.theme[0].color", + structPath: "user.*.settings['theme'][0].color", + dynPath: "user.*.settings.theme[0].color", }, // Mixed JSON tag and Go field name scenarios - { - name: "Go field -> JSON field", - structPath: "Parent.child_name", - dynPath: "Parent.child_name", - }, - { - name: "JSON field -> Go field", - structPath: "parent.ChildName", - dynPath: "parent.ChildName", - }, - { - name: "dash JSON tag field", - structPath: "-", - dynPath: "-", - }, - { - name: "JSON tag with options", - structPath: "lazy_field", - dynPath: "lazy_field", - }, + //{ + // name: "Go field -> JSON field", + // structPath: "Parent.child_name", + // dynPath: "Parent.child_name", + //}, + //{ + // name: "JSON field -> Go field", + // structPath: "parent.ChildName", + // dynPath: "parent.ChildName", + //}, + //{ + // name: "dash JSON tag field", + // structPath: "-", + // dynPath: "-", + //}, + //{ + // name: "JSON tag with options", + // structPath: "lazy_field", + // dynPath: "lazy_field", + //}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Parse the structPath string into a PathNode - var pathNode *structpath.PathNode - var err error - if tt.structPath == "" { - pathNode = nil - } else { - pathNode, err = structpath.Parse(tt.structPath) - assert.NoError(t, err, "Failed to parse structPath: %s", tt.structPath) + if tt.name != "dot star with parent" { + return } + pathNode, err := structpath.Parse(tt.structPath) + t.Logf("%q node=%#v", tt.structPath, pathNode) + assert.NoError(t, err, "Failed to parse structPath: %s", tt.structPath) // Convert to DynPath and verify result result := ConvertPathNodeToDynPath(pathNode, tt.rootType) From 6197f343b5192eee1d16799e9aa94d186c20b81b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 19 Sep 2025 11:38:08 +0200 Subject: [PATCH 3/5] fixes --- libs/structs/dynpath/dynpath.go | 23 ++++++--- libs/structs/dynpath/dynpath_test.go | 74 +++++++++++++--------------- libs/structs/structpath/path.go | 31 ++++++++++-- libs/structs/structpath/path_test.go | 38 +++++++++++++- 4 files changed, 112 insertions(+), 54 deletions(-) diff --git a/libs/structs/dynpath/dynpath.go b/libs/structs/dynpath/dynpath.go index 2fcacb7b78..eb7fbd5f4e 100644 --- a/libs/structs/dynpath/dynpath.go +++ b/libs/structs/dynpath/dynpath.go @@ -77,25 +77,32 @@ func ConvertPathNodeToDynPath(path *structpath.PathNode, rootType reflect.Type) result.WriteString(".") } result.WriteString("*") - // Type doesn't change for wildcards - } else if segment.BracketStar() { - // Context-aware wildcard rendering + // If it's a map, we can inspect the type; otherwise we cannot since we don't know what field to look into if currentType != nil && currentType.Kind() == reflect.Map { + currentType = currentType.Elem() + } else { + currentType = nil + } + + } else if segment.BracketStar() { + if currentType != nil && (currentType.Kind() == reflect.Array || currentType.Kind() == reflect.Slice) { + result.WriteString("[*]") + currentType = currentType.Elem() + } else { // Map wildcard uses dot notation in DynPath if result.Len() > 0 { result.WriteString(".") } result.WriteString("*") - currentType = currentType.Elem() - } else { - // Array/slice wildcard or fallback uses bracket notation - result.WriteString("[*]") - if currentType != nil && (currentType.Kind() == reflect.Array || currentType.Kind() == reflect.Slice) { + + if currentType != nil && currentType.Kind() == reflect.Map { currentType = currentType.Elem() } else { currentType = nil } + + // QQQ return error if we cannot disambiguate? } } } diff --git a/libs/structs/dynpath/dynpath_test.go b/libs/structs/dynpath/dynpath_test.go index 8dcbc3265c..1d038fa4f5 100644 --- a/libs/structs/dynpath/dynpath_test.go +++ b/libs/structs/dynpath/dynpath_test.go @@ -55,42 +55,41 @@ func TestConvertPathNodeToDynPath(t *testing.T) { }, // Compound paths - /* - { - name: "struct field -> array index", - structPath: "items[3]", - dynPath: "items[3]", - }, - { - name: "struct field -> map key", - structPath: "config['database']", - dynPath: "config.database", - }, - { - name: "struct field -> struct field", - structPath: "user.name", - dynPath: "user.name", - }, - { - name: "map key -> array index", - structPath: "['servers'][0]", - dynPath: "servers[0]", - }, - { - name: "map key -> struct field", - structPath: "['primary'].host", - dynPath: "primary.host", - }, - { - name: "array index -> struct field", - structPath: "[2].id", - dynPath: "[2].id", - }, - { - name: "array index -> map key", - structPath: "[1]['status']", - dynPath: "[1].status", - },*/ + { + name: "struct field -> array index", + structPath: "items[3]", + dynPath: "items[3]", + }, + { + name: "struct field -> map key", + structPath: "config['database']", + dynPath: "config.database", + }, + { + name: "struct field -> struct field", + structPath: "user.name", + dynPath: "user.name", + }, + { + name: "map key -> array index", + structPath: "['servers'][0]", + dynPath: "servers[0]", + }, + { + name: "map key -> struct field", + structPath: "['primary'].host", + dynPath: "primary.host", + }, + { + name: "array index -> struct field", + structPath: "[2].id", + dynPath: "[2].id", + }, + { + name: "array index -> map key", + structPath: "[1]['status']", + dynPath: "[1].status", + }, // Wildcard combinations { @@ -180,9 +179,6 @@ func TestConvertPathNodeToDynPath(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.name != "dot star with parent" { - return - } pathNode, err := structpath.Parse(tt.structPath) t.Logf("%q node=%#v", tt.structPath, pathNode) assert.NoError(t, err, "Failed to parse structPath: %s", tt.structPath) diff --git a/libs/structs/structpath/path.go b/libs/structs/structpath/path.go index 25d411cedd..9c900b80be 100644 --- a/libs/structs/structpath/path.go +++ b/libs/structs/structpath/path.go @@ -201,9 +201,10 @@ func (p *PathNode) String() string { // State Machine for Path Parsing: // // States: -// - START: Beginning of parsing, expects field name or "[" -// - FIELD_START: After a dot, expects field name only +// - START: Beginning of parsing, expects field name, "[", or "*" +// - FIELD_START: After a dot, expects field name or "*" // - FIELD: Reading field name characters +// - DOT_STAR: Encountered "*" (at start or after dot), expects ".", "[", or EOF // - BRACKET_OPEN: Just encountered "[", expects digit, "'" or "*" // - INDEX: Reading array index digits, expects more digits or "]" // - MAP_KEY: Reading map key content, expects any char or "'" @@ -213,9 +214,10 @@ func (p *PathNode) String() string { // - END: Successfully completed parsing // // Transitions: -// - START: [a-zA-Z_-] -> FIELD, "[" -> BRACKET_OPEN, EOF -> END -// - FIELD_START: [a-zA-Z_-] -> FIELD, other -> ERROR +// - START: [a-zA-Z_-] -> FIELD, "[" -> BRACKET_OPEN, "*" -> DOT_STAR, EOF -> END +// - FIELD_START: [a-zA-Z_-] -> FIELD, "*" -> DOT_STAR, other -> ERROR // - FIELD: [a-zA-Z0-9_-] -> FIELD, "." -> FIELD_START, "[" -> BRACKET_OPEN, EOF -> END +// - DOT_STAR: "." -> FIELD_START, "[" -> BRACKET_OPEN, EOF -> END, other -> ERROR // - BRACKET_OPEN: [0-9] -> INDEX, "'" -> MAP_KEY, "*" -> WILDCARD // - INDEX: [0-9] -> INDEX, "]" -> EXPECT_DOT_OR_END // - MAP_KEY: (any except "'") -> MAP_KEY, "'" -> MAP_KEY_QUOTE @@ -232,6 +234,7 @@ func Parse(s string) (*PathNode, error) { stateStart = iota stateFieldStart stateField + stateDotStar stateBracketOpen stateIndex stateMapKey @@ -253,6 +256,8 @@ func Parse(s string) (*PathNode, error) { case stateStart: if ch == '[' { state = stateBracketOpen + } else if ch == '*' { + state = stateDotStar } else if !isReservedFieldChar(ch) { currentToken.WriteByte(ch) state = stateField @@ -261,7 +266,9 @@ func Parse(s string) (*PathNode, error) { } case stateFieldStart: - if !isReservedFieldChar(ch) { + if ch == '*' { + state = stateDotStar + } else if !isReservedFieldChar(ch) { currentToken.WriteByte(ch) state = stateField } else { @@ -283,6 +290,17 @@ func Parse(s string) (*PathNode, error) { return nil, fmt.Errorf("invalid character '%c' in field name at position %d", ch, pos) } + case stateDotStar: + if ch == '.' { + result = NewDotStar(result) + state = stateFieldStart + } else if ch == '[' { + result = NewDotStar(result) + state = stateBracketOpen + } else { + return nil, fmt.Errorf("unexpected character '%c' after '.*' at position %d", ch, pos) + } + case stateBracketOpen: if ch >= '0' && ch <= '9' { currentToken.WriteByte(ch) @@ -368,6 +386,9 @@ func Parse(s string) (*PathNode, error) { case stateField: result = NewStructField(result, currentToken.String()) return result, nil + case stateDotStar: + result = NewDotStar(result) + return result, nil case stateExpectDotOrEnd: return result, nil case stateFieldStart: diff --git a/libs/structs/structpath/path_test.go b/libs/structs/structpath/path_test.go index 0bb2543be1..a833e5611e 100644 --- a/libs/structs/structpath/path_test.go +++ b/libs/structs/structpath/path_test.go @@ -135,6 +135,7 @@ func TestPathNode(t *testing.T) { String: "Parent[*]", BracketStar: true, }, + // Edge cases with special characters in map keys { name: "map key with single quote", @@ -198,6 +199,32 @@ func TestPathNode(t *testing.T) { String: "['key\x00[],`']", MapKey: "key\x00[],`", }, + + // Additional dot-star pattern tests + { + name: "field dot star", + node: NewDotStar(NewStructField(nil, "bla")), + String: "bla.*", + DotStar: true, + }, + { + name: "field dot star dot field", + node: NewStructField(NewDotStar(NewStructField(nil, "bla")), "foo"), + String: "bla.*.foo", + Field: "foo", + }, + { + name: "field dot star bracket index", + node: NewIndex(NewDotStar(NewStructField(nil, "bla")), 0), + String: "bla.*[0]", + Index: 0, + }, + { + name: "field dot star bracket star", + node: NewBracketStar(NewDotStar(NewStructField(nil, "bla"))), + String: "bla.*[*]", + BracketStar: true, + }, } for _, tt := range tests { @@ -208,8 +235,8 @@ func TestPathNode(t *testing.T) { // Test roundtrip conversion: String() -> Parse() -> String() parsed, err := Parse(tt.String) - assert.NoError(t, err, "Parse() should not error") - if parsed != nil { + if assert.NoError(t, err, "Parse() should not error") { + assert.Equal(t, tt.node, parsed) roundtripResult := parsed.String() assert.Equal(t, tt.String, roundtripResult, "Roundtrip conversion should be identical") } @@ -408,6 +435,13 @@ func TestParseErrors(t *testing.T) { input: "field['key'", error: "unexpected end of input after quote in map key", }, + + // Invalid dot-star patterns + { + name: "dot star followed by field name", + input: "bla.*foo", + error: "unexpected character 'f' after '.*' at position 5", + }, } for _, tt := range tests { From 761f5b36cad99f387f24609f4e6ee868e08c60e0 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 19 Sep 2025 11:46:42 +0200 Subject: [PATCH 4/5] clean up --- libs/structs/dynpath/dynpath.go | 7 ------- libs/structs/dynpath/dynpath_test.go | 22 ++++++++++++++++++---- libs/structs/structpath/path.go | 4 ---- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/libs/structs/dynpath/dynpath.go b/libs/structs/dynpath/dynpath.go index eb7fbd5f4e..d8901e657f 100644 --- a/libs/structs/dynpath/dynpath.go +++ b/libs/structs/dynpath/dynpath.go @@ -1,8 +1,6 @@ package dynpath import ( - "fmt" - "os" "reflect" "strconv" "strings" @@ -17,11 +15,6 @@ import ( // - BracketStar accessing arrays/slices renders as "parent[*]" // - DotStar always renders as "parent.*" func ConvertPathNodeToDynPath(path *structpath.PathNode, rootType reflect.Type) string { - if path == nil { - return "" - } - fmt.Fprintf(os.Stderr, "pathg=%#v\n", path) - segments := path.AsSlice() var result strings.Builder currentType := rootType diff --git a/libs/structs/dynpath/dynpath_test.go b/libs/structs/dynpath/dynpath_test.go index 1d038fa4f5..4e0bb4c3ae 100644 --- a/libs/structs/dynpath/dynpath_test.go +++ b/libs/structs/dynpath/dynpath_test.go @@ -100,14 +100,28 @@ func TestConvertPathNodeToDynPath(t *testing.T) { { name: "bracket star with parent - array", structPath: "Parent[*]", - rootType: reflect.TypeOf(struct{ Parent []int }{}), - dynPath: "Parent[*]", + rootType: reflect.TypeOf(struct { + Parent []int + }{}), + dynPath: "Parent[*]", }, + + { + name: "bracket star with parent - array", + structPath: "parent[*]", + rootType: reflect.TypeOf(struct { + Parent []int `json:"parent"` + }{}), + dynPath: "parent[*]", + }, + { name: "bracket star with parent - map", structPath: "Parent[*]", - rootType: reflect.TypeOf(struct{ Parent map[string]int }{}), - dynPath: "Parent.*", + rootType: reflect.TypeOf(struct { + Parent map[string]int + }{}), + dynPath: "Parent.*", }, // Special characters and edge cases diff --git a/libs/structs/structpath/path.go b/libs/structs/structpath/path.go index 9c900b80be..04f49cfc2d 100644 --- a/libs/structs/structpath/path.go +++ b/libs/structs/structpath/path.go @@ -82,10 +82,6 @@ func (p *PathNode) Parent() *PathNode { // AsSlice returns the path as a slice of PathNodes from root to current. // Efficiently pre-allocates the exact length and fills in reverse order. func (p *PathNode) AsSlice() []*PathNode { - if p == nil { - return nil - } - // First pass: count the length length := 0 current := p From c795428dd11b64b8538b556de4dda006676a3aa9 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 19 Sep 2025 12:02:24 +0200 Subject: [PATCH 5/5] lint fix --- libs/structs/structpath/path.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libs/structs/structpath/path.go b/libs/structs/structpath/path.go index 04f49cfc2d..4c4ae2daef 100644 --- a/libs/structs/structpath/path.go +++ b/libs/structs/structpath/path.go @@ -287,13 +287,14 @@ func Parse(s string) (*PathNode, error) { } case stateDotStar: - if ch == '.' { + switch ch { + case '.': result = NewDotStar(result) state = stateFieldStart - } else if ch == '[' { + case '[': result = NewDotStar(result) state = stateBracketOpen - } else { + default: return nil, fmt.Errorf("unexpected character '%c' after '.*' at position %d", ch, pos) }