Skip to content

Commit 6d30f59

Browse files
authored
internal: remove tags from structpath.PathNode (#3626)
## Why The PathNode represents path alone, not what it points to. Stacked on top of #3620
1 parent e54c3bf commit 6d30f59

File tree

11 files changed

+85
-74
lines changed

11 files changed

+85
-74
lines changed

bundle/direct/dresources/all_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
154154
require.NoError(t, err)
155155
require.NotNil(t, remappedState)
156156

157-
require.NoError(t, structwalk.Walk(newState, func(path *structpath.PathNode, val any) {
157+
require.NoError(t, structwalk.Walk(newState, func(path *structpath.PathNode, val any, field *reflect.StructField) {
158158
remoteValue, err := structaccess.Get(remappedState, dyn.MustPathFromString(path.DynPath()))
159159
if err != nil {
160160
t.Errorf("Failed to read %s from remapped remote state %#v", path.DynPath(), remappedState)
@@ -187,7 +187,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
187187
func validateFields(t *testing.T, configType reflect.Type, fields map[string]deployplan.ActionType) {
188188
validPaths := make(map[string]struct{})
189189

190-
err := structwalk.WalkType(configType, func(path *structpath.PathNode, typ reflect.Type) bool {
190+
err := structwalk.WalkType(configType, func(path *structpath.PathNode, typ reflect.Type, field *reflect.StructField) bool {
191191
validPaths[path.String()] = struct{}{}
192192
return true // continue walking
193193
})

bundle/internal/validation/enum.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/databricks/cli/bundle/config"
1414
"github.com/databricks/cli/libs/structs/structpath"
15+
"github.com/databricks/cli/libs/structs/structtag"
1516
"github.com/databricks/cli/libs/structs/structwalk"
1617
)
1718

@@ -109,15 +110,17 @@ func getEnumValues(typ reflect.Type) ([]string, error) {
109110
func extractEnumFields(typ reflect.Type) ([]EnumPatternInfo, error) {
110111
fieldsByPattern := make(map[string][]string)
111112

112-
err := structwalk.WalkType(typ, func(path *structpath.PathNode, fieldType reflect.Type) bool {
113+
err := structwalk.WalkType(typ, func(path *structpath.PathNode, fieldType reflect.Type, field *reflect.StructField) bool {
113114
if path == nil {
114115
return true
115116
}
116117

117118
// Do not generate enum validation code for fields that are internal or readonly.
118-
bundleTag := path.BundleTag()
119-
if bundleTag.Internal() || bundleTag.ReadOnly() {
120-
return false
119+
if field != nil {
120+
bundleTag := structtag.BundleTag(field.Tag.Get("bundle"))
121+
if bundleTag.Internal() || bundleTag.ReadOnly() {
122+
return false
123+
}
121124
}
122125

123126
// Check if this field type is an enum

bundle/internal/validation/required.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/databricks/cli/bundle/config"
1414
"github.com/databricks/cli/libs/structs/structpath"
15+
"github.com/databricks/cli/libs/structs/structtag"
1516
"github.com/databricks/cli/libs/structs/structwalk"
1617
)
1718

@@ -45,29 +46,30 @@ func formatSliceToString(values []string) string {
4546
func extractRequiredFields(typ reflect.Type) ([]RequiredPatternInfo, error) {
4647
fieldsByPattern := make(map[string][]string)
4748

48-
err := structwalk.WalkType(typ, func(path *structpath.PathNode, _ reflect.Type) bool {
49-
if path == nil {
49+
err := structwalk.WalkType(typ, func(path *structpath.PathNode, _ reflect.Type, field *reflect.StructField) bool {
50+
if path == nil || field == nil {
5051
return true
5152
}
5253

5354
// Do not generate required validation code for fields that are internal or readonly.
54-
bundleTag := path.BundleTag()
55+
bundleTag := structtag.BundleTag(field.Tag.Get("bundle"))
5556
if bundleTag.Internal() || bundleTag.ReadOnly() {
5657
return false
5758
}
5859

5960
// The "omitempty" tag indicates the field is optional in bundle config.
60-
if path.JSONTag().OmitEmpty() {
61+
jsonTag := structtag.JSONTag(field.Tag.Get("json"))
62+
if jsonTag.OmitEmpty() {
6163
return true
6264
}
6365

64-
field, ok := path.Field()
66+
fieldName, ok := path.Field()
6567
if !ok {
6668
return true
6769
}
6870

6971
parentPath := path.Parent().DynPath()
70-
fieldsByPattern[parentPath] = append(fieldsByPattern[parentPath], field)
72+
fieldsByPattern[parentPath] = append(fieldsByPattern[parentPath], fieldName)
7173
return true
7274
})
7375

cmd/bundle/debug/refschema.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func dumpRemoteSchemas(out io.Writer) error {
5454
pathTypes := make(map[string]map[string]map[string]struct{})
5555

5656
collect := func(root reflect.Type, source string) error {
57-
return structwalk.WalkType(root, func(path *structpath.PathNode, typ reflect.Type) bool {
57+
return structwalk.WalkType(root, func(path *structpath.PathNode, typ reflect.Type, field *reflect.StructField) bool {
5858
if path == nil {
5959
return true
6060
}

libs/structs/structdiff/diff.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"sort"
88

99
"github.com/databricks/cli/libs/structs/structpath"
10+
"github.com/databricks/cli/libs/structs/structtag"
1011
)
1112

1213
type Change struct {
@@ -130,7 +131,8 @@ func diffStruct(path *structpath.PathNode, s1, s2 reflect.Value, changes *[]Chan
130131
zero2 := v2Field.IsZero()
131132

132133
if zero1 || zero2 {
133-
if node.JSONTag().OmitEmpty() {
134+
jsonTag := structtag.JSONTag(sf.Tag.Get("json"))
135+
if jsonTag.OmitEmpty() {
134136
if zero1 {
135137
if !slices.Contains(forced1, sf.Name) {
136138
v1Field = reflect.ValueOf(nil)

libs/structs/structpath/path.go

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,13 @@ const (
2020
// PathNode represents a node in a path for struct diffing.
2121
// It can represent struct fields, map keys, or array/slice indices.
2222
type PathNode struct {
23-
prev *PathNode
24-
jsonTag structtag.JSONTag // For JSON key resolution
25-
bundleTag structtag.BundleTag
26-
key string // Computed key (JSON key for structs, string key for maps, or Go field name for fallback)
23+
prev *PathNode
24+
key string // Computed key (JSON key for structs, string key for maps, or Go field name for fallback)
2725
// If index >= 0, the node specifies a slice/array index in index.
2826
// If index < 0, this describes the type of node (see tagStruct and other consts above)
2927
index int
3028
}
3129

32-
func (p *PathNode) JSONTag() structtag.JSONTag {
33-
return p.jsonTag
34-
}
35-
36-
func (p *PathNode) BundleTag() structtag.BundleTag {
37-
return p.bundleTag
38-
}
39-
4030
func (p *PathNode) IsRoot() bool {
4131
return p == nil
4232
}
@@ -116,19 +106,16 @@ func NewMapKey(prev *PathNode, key string) *PathNode {
116106
// The jsonTag is used for JSON key resolution, and fieldName is used as fallback.
117107
func NewStructField(prev *PathNode, tag reflect.StructTag, fieldName string) *PathNode {
118108
jsonTag := structtag.JSONTag(tag.Get("json"))
119-
bundleTag := structtag.BundleTag(tag.Get("bundle"))
120109

121110
key := fieldName
122111
if name := jsonTag.Name(); name != "" {
123112
key = name
124113
}
125114

126115
return &PathNode{
127-
prev: prev,
128-
jsonTag: jsonTag,
129-
bundleTag: bundleTag,
130-
key: key,
131-
index: tagStruct,
116+
prev: prev,
117+
key: key,
118+
index: tagStruct,
132119
}
133120
}
134121

libs/structs/structwalk/walk.go

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@ import (
77
"sort"
88

99
"github.com/databricks/cli/libs/structs/structpath"
10+
"github.com/databricks/cli/libs/structs/structtag"
1011
)
1112

1213
// VisitFunc is invoked for every scalar (int, uint, float, string, bool) field encountered while walking v.
1314
//
1415
// path PathNode representing the JSON-style path to the field.
1516
// val the field's value – if the field is a pointer to a scalar the pointer is *not* dereferenced; the
1617
// callback receives either nil (for a nil pointer) or the concrete value.
18+
// field the reflect.StructField for struct fields, nil for map keys and array indices.
1719
//
1820
// NOTE: Fields lacking a json tag or tagged as "-" are ignored entirely.
1921
// Composite kinds (struct, slice/array, map, interface, function, chan, etc.) are *not* visited, but the walk
@@ -23,13 +25,13 @@ import (
2325
// The walk is depth-first and deterministic (map keys are sorted lexicographically).
2426
//
2527
// Example:
26-
// err := structwalk.Walk(cfg, func(path *structpath.PathNode, v any) {
28+
// err := structwalk.Walk(cfg, func(path *structpath.PathNode, v any, field *reflect.StructField) {
2729
// fmt.Printf("%s = %v\n", path.String(), v)
2830
// })
2931
//
3032
// ******************************************************************************************************
3133

32-
type VisitFunc func(path *structpath.PathNode, val any)
34+
type VisitFunc func(path *structpath.PathNode, val any, field *reflect.StructField)
3335

3436
// Walk validates that v is a struct or pointer to one and starts the recursive traversal.
3537
func Walk(v any, visit VisitFunc) error {
@@ -40,7 +42,7 @@ func Walk(v any, visit VisitFunc) error {
4042
if !rv.IsValid() {
4143
return nil
4244
}
43-
walkValue(nil, rv, visit)
45+
walkValue(nil, rv, nil, visit)
4446
return nil
4547
}
4648

@@ -58,25 +60,25 @@ func isScalar(k reflect.Kind) bool {
5860
}
5961
}
6062

61-
func walkValue(path *structpath.PathNode, val reflect.Value, visit VisitFunc) {
63+
func walkValue(path *structpath.PathNode, val reflect.Value, field *reflect.StructField, visit VisitFunc) {
6264
kind := val.Kind()
6365

6466
if isScalar(kind) {
65-
visit(path, val.Interface())
67+
visit(path, val.Interface(), field)
6668
return
6769
}
6870

6971
switch kind {
7072
case reflect.Pointer:
71-
walkValue(path, val.Elem(), visit)
73+
walkValue(path, val.Elem(), field, visit)
7274

7375
case reflect.Struct:
7476
walkStruct(path, val, visit)
7577

7678
case reflect.Slice, reflect.Array:
7779
for i := range val.Len() {
7880
node := structpath.NewIndex(path, i)
79-
walkValue(node, val.Index(i), visit)
81+
walkValue(node, val.Index(i), nil, visit)
8082
}
8183

8284
case reflect.Map:
@@ -91,7 +93,7 @@ func walkValue(path *structpath.PathNode, val reflect.Value, visit VisitFunc) {
9193
for _, ks := range keys {
9294
v := val.MapIndex(reflect.ValueOf(ks))
9395
node := structpath.NewMapKey(path, ks)
94-
walkValue(node, v, visit)
96+
walkValue(node, v, nil, visit)
9597
}
9698

9799
default:
@@ -114,17 +116,18 @@ func walkStruct(path *structpath.PathNode, s reflect.Value, visit VisitFunc) {
114116
}
115117

116118
node := structpath.NewStructField(path, sf.Tag, sf.Name)
117-
if node.JSONTag().Name() == "-" {
119+
jsonTag := structtag.JSONTag(sf.Tag.Get("json"))
120+
if jsonTag.Name() == "-" {
118121
continue // skip fields without json name
119122
}
120123

121124
fieldVal := s.Field(i)
122125
// Skip zero values with omitempty unless field is explicitly forced.
123-
if node.JSONTag().OmitEmpty() && fieldVal.IsZero() && !slices.Contains(forced, sf.Name) {
126+
if jsonTag.OmitEmpty() && fieldVal.IsZero() && !slices.Contains(forced, sf.Name) {
124127
continue
125128
}
126129

127-
walkValue(node, fieldVal, visit)
130+
walkValue(node, fieldVal, &sf, visit)
128131
}
129132
}
130133

libs/structs/structwalk/walk_test.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,27 @@
11
package structwalk
22

33
import (
4+
"reflect"
45
"testing"
56

67
"github.com/databricks/cli/libs/structs/structpath"
8+
"github.com/databricks/cli/libs/structs/structtag"
79
"github.com/databricks/databricks-sdk-go/service/jobs"
810
"github.com/stretchr/testify/assert"
911
"github.com/stretchr/testify/require"
1012
)
1113

1214
func flatten(t *testing.T, value any) map[string]any {
1315
results := make(map[string]any)
14-
err := Walk(value, func(path *structpath.PathNode, value any) {
16+
err := Walk(value, func(path *structpath.PathNode, value any, field *reflect.StructField) {
1517
s := path.String()
1618
results[s] = value
1719

1820
// Test path parsing round trip
1921
newPath, err := structpath.Parse(s)
2022
if assert.NoError(t, err, s) {
21-
// newPath is not comparable with path a.t.m due to bundle tags on PathNode
2223
newS := newPath.String()
24+
assert.Equal(t, path, newPath, "s=%q newS=%q", s, newS)
2325
assert.Equal(t, s, newS)
2426
}
2527
})
@@ -107,11 +109,16 @@ func TestValueBundleTag(t *testing.T) {
107109
B: "b",
108110
C: "c",
109111
D: "d",
110-
}, func(path *structpath.PathNode, value any) {
111-
if path.BundleTag().ReadOnly() {
112+
}, func(path *structpath.PathNode, value any, field *reflect.StructField) {
113+
if field == nil {
114+
return
115+
}
116+
117+
bundleTag := structtag.BundleTag(field.Tag.Get("bundle"))
118+
if bundleTag.ReadOnly() {
112119
readonly = append(readonly, path.String())
113120
}
114-
if path.BundleTag().Internal() {
121+
if bundleTag.Internal() {
115122
internal = append(internal, path.String())
116123
}
117124
})

0 commit comments

Comments
 (0)