Skip to content

Commit 13e2dad

Browse files
authored
internal: update structpath to accept both dot and bracket syntax for fields and maps (#3640)
## Changes Simplify internal representation of structpath.PathNode: struct fields and map keys are represented the same way internally. When parsing, users can specify either `.field` or `[‘field’]`, both will be encoded as tagStringKey in PathNode. When saving PathNode as string, String() selects appropriate encoding (prefers dot unless it uses reserved characters). DynPath() is removed, String() returns DynPath()-like value for those values that can be represented by dot notation. At the same time, PathNode now accurately represents `foo.*` vs `foo[*]`, this structure is preserved and not equivalent. This again improves compatibility with dyn.Path as it represents index and keys wildcard differently. structaccess is updated to accept PathNode. Since PathNode only gives StringKey() now and does not distinguish between map and struct fields, structaccess.Get essentially supports both syntaxes for both kinds. structwalk is updated to emit dotStar for map wildcards, making it compatible with code expecting dyn.Pattern (this is used for required fields and enum validation). structwalk.PathNode.String() is rewritten to be iterative rather than recursive. PathNode gets new methods: Prefix(N), SkipPrefix(N), AsSlice(). ## Why Dot notation is currently used for maps e.g. "resources.jobs.foo". With precise map encoding that would look like "resources.jobs['foo']" which looks unfamiliar and also will not match if someone does string comparison against "resources.jobs.foo". Adopting this scheme makes it easier to work with PathNode in dyn.Path world and in fact makes it drop-in replacement (the syntax is superset). ## Tests Existing and new unit tests.
1 parent 79f959c commit 13e2dad

File tree

18 files changed

+737
-507
lines changed

18 files changed

+737
-507
lines changed

acceptance/bundle/refschema/out.fields.txt

Lines changed: 65 additions & 65 deletions
Large diffs are not rendered by default.

bundle/config/mutator/log_resource_references.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/databricks/cli/libs/dyn/dynvar"
1212
"github.com/databricks/cli/libs/log"
1313
"github.com/databricks/cli/libs/structs/structaccess"
14+
"github.com/databricks/cli/libs/structs/structpath"
1415
)
1516

1617
// Longest field name:
@@ -115,7 +116,14 @@ func truncate(s string, n int, suffix string) string {
115116
}
116117

117118
func censorValue(ctx context.Context, v any, path dyn.Path) (string, error) {
118-
v, err := structaccess.Get(v, path)
119+
pathString := path.String()
120+
pathNode, err := structpath.Parse(pathString)
121+
if err != nil {
122+
log.Warnf(ctx, "internal error: parsing %q: %s", pathString, err)
123+
return "err", err
124+
}
125+
126+
v, err = structaccess.Get(v, pathNode)
119127
if err != nil {
120128
log.Infof(ctx, "internal error: path=%s: %s", path, err)
121129
return "err", err

bundle/direct/dresources/all_test.go

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

99
"github.com/databricks/cli/bundle/config/resources"
1010
"github.com/databricks/cli/bundle/deployplan"
11-
"github.com/databricks/cli/libs/dyn"
1211
"github.com/databricks/cli/libs/structs/structaccess"
1312
"github.com/databricks/cli/libs/structs/structpath"
1413
"github.com/databricks/cli/libs/structs/structwalk"
@@ -155,9 +154,9 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
155154
require.NotNil(t, remappedState)
156155

157156
require.NoError(t, structwalk.Walk(newState, func(path *structpath.PathNode, val any, field *reflect.StructField) {
158-
remoteValue, err := structaccess.Get(remappedState, dyn.MustPathFromString(path.DynPath()))
157+
remoteValue, err := structaccess.Get(remappedState, path)
159158
if err != nil {
160-
t.Errorf("Failed to read %s from remapped remote state %#v", path.DynPath(), remappedState)
159+
t.Errorf("Failed to read %s from remapped remote state %#v", path.String(), remappedState)
161160
}
162161
if val == nil {
163162
// t.Logf("Ignoring %s nil, remoteValue=%#v", path.String(), remoteValue)
@@ -172,7 +171,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
172171
// t.Logf("Testing %s v=%#v, remoteValue=%#v", path.String(), val, remoteValue)
173172
// We expect fields set explicitly to be preserved by testserver, which is true for all resources as of today.
174173
// If not true for your resource, add exception here:
175-
assert.Equal(t, val, remoteValue, path.DynPath())
174+
assert.Equal(t, val, remoteValue, path.String())
176175
}))
177176

178177
err = adapter.DoDelete(ctx, createdID)

bundle/direct/plan.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,12 @@ import (
99
"github.com/databricks/cli/bundle/deployplan"
1010
"github.com/databricks/cli/bundle/direct/dresources"
1111
"github.com/databricks/cli/bundle/direct/dstate"
12-
"github.com/databricks/cli/libs/dyn"
1312
"github.com/databricks/cli/libs/structs/structdiff"
1413
"github.com/databricks/databricks-sdk-go"
1514

16-
"github.com/databricks/cli/libs/dyn/dynvar"
1715
"github.com/databricks/cli/libs/log"
1816
"github.com/databricks/cli/libs/structs/structaccess"
17+
"github.com/databricks/cli/libs/structs/structpath"
1918
)
2019

2120
func (d *DeploymentUnit) Plan(ctx context.Context, client *databricks.WorkspaceClient, db *dstate.DeploymentState, inputConfig any, localOnly, refresh bool) (deployplan.ActionType, error) {
@@ -68,12 +67,12 @@ func (d *DeploymentUnit) refreshRemoteState(ctx context.Context, id string) erro
6867
var ErrDelayed = errors.New("must be resolved after apply")
6968

7069
func (d *DeploymentUnit) ResolveReferenceLocalOrRemote(ctx context.Context, db *dstate.DeploymentState, reference string, actionType deployplan.ActionType, config any) (any, error) {
71-
path, ok := dynvar.PureReferenceToPath(reference)
72-
if !ok || len(path) <= 3 || path[0:3].String() != d.ResourceKey {
70+
path, ok := structpath.PureReferenceToPath(reference)
71+
if !ok || path.Len() <= 3 || path.Prefix(3).String() != d.ResourceKey {
7372
return nil, fmt.Errorf("internal error: expected reference to %q, got %q", d.ResourceKey, reference)
7473
}
7574

76-
fieldPath := path[3:]
75+
fieldPath := path.SkipPrefix(3)
7776

7877
if fieldPath.String() == "id" {
7978
if actionType.KeepsID() {
@@ -136,12 +135,12 @@ func (d *DeploymentUnit) ResolveReferenceLocalOrRemote(ctx context.Context, db *
136135
}
137136

138137
func (d *DeploymentUnit) ResolveReferenceRemote(ctx context.Context, db *dstate.DeploymentState, reference string) (any, error) {
139-
path, ok := dynvar.PureReferenceToPath(reference)
140-
if !ok || len(path) <= 3 || path[0:3].String() != d.ResourceKey {
138+
path, ok := structpath.PureReferenceToPath(reference)
139+
if !ok || path.Len() <= 3 || path.Prefix(3).String() != d.ResourceKey {
141140
return nil, fmt.Errorf("internal error: expected reference to %q, got %q", d.ResourceKey, reference)
142141
}
143142

144-
fieldPath := path[3:]
143+
fieldPath := path.SkipPrefix(3)
145144

146145
// Handle "id" field separately - read from state, not remote state
147146
if fieldPath.String() == "id" {
@@ -157,7 +156,7 @@ func (d *DeploymentUnit) ResolveReferenceRemote(ctx context.Context, db *dstate.
157156
}
158157

159158
// ReadRemoteStateField reads a field from remote state with refresh if needed.
160-
func (d *DeploymentUnit) ReadRemoteStateField(ctx context.Context, db *dstate.DeploymentState, fieldPath dyn.Path) (any, error) {
159+
func (d *DeploymentUnit) ReadRemoteStateField(ctx context.Context, db *dstate.DeploymentState, fieldPath *structpath.PathNode) (any, error) {
161160
// We have options:
162161
// 1) Rely on the cached value; refresh if not cached.
163162
// 2) Always refresh, read the value.

bundle/internal/validation/enum.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,7 @@ func extractEnumFields(typ reflect.Type) ([]EnumPatternInfo, error) {
132132
return true
133133
}
134134

135-
fieldPath := path.DynPath()
136-
fieldsByPattern[fieldPath] = enumValues
135+
fieldsByPattern[path.String()] = enumValues
137136
}
138137
return true
139138
})

bundle/internal/validation/required.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,12 @@ func extractRequiredFields(typ reflect.Type) ([]RequiredPatternInfo, error) {
6363
return true
6464
}
6565

66-
fieldName, ok := path.Field()
66+
fieldName, ok := path.StringKey()
6767
if !ok {
6868
return true
6969
}
7070

71-
parentPath := path.Parent().DynPath()
71+
parentPath := path.Parent().String()
7272
fieldsByPattern[parentPath] = append(fieldsByPattern[parentPath], fieldName)
7373
return true
7474
})

libs/structs/structaccess/bundle_test.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,6 @@ func TestGet_ConfigRoot_JobTagsAccess(t *testing.T) {
5252
require.Error(t, ValidateByString(reflect.TypeOf(root), "resources.jobs.my_job.tags.env.inner"))
5353
require.Error(t, ValidateByString(reflect.TypeOf(root), "resources.jobs.my_job.tags1"))
5454

55-
// Leading dot is allowed
56-
v, err = GetByString(root, ".resources.jobs.my_job.tags.team")
57-
require.NoError(t, err)
58-
require.Equal(t, "platform", v)
59-
require.NoError(t, ValidateByString(reflect.TypeOf(root), ".resources.jobs.my_job.tags.team"))
60-
require.Error(t, ValidateByString(reflect.TypeOf(root), ".resources.jobs.my_job.tags.team.inner"))
61-
require.Error(t, ValidateByString(reflect.TypeOf(root), ".resources.jobs.my_job.tags1"))
62-
6355
// Array indexing test (1)
6456
v, err = GetByString(root, "resources.jobs.my_job.tasks[0].task_key")
6557
require.NoError(t, err)

libs/structs/structaccess/get.go

Lines changed: 36 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,87 +1,74 @@
11
package structaccess
22

33
import (
4+
"errors"
45
"fmt"
56
"reflect"
6-
"strconv"
77

8-
"github.com/databricks/cli/libs/dyn"
8+
"github.com/databricks/cli/libs/structs/structpath"
99
"github.com/databricks/cli/libs/structs/structtag"
1010
)
1111

1212
// GetByString returns the value at the given path inside v.
1313
// This is a convenience function that parses the path string and calls Get.
14-
//
15-
// Path grammar (compatible with dyn path):
16-
// - Struct field names and map keys separated by '.' (e.g., connection.id)
17-
// - (Note, this prevents maps keys that are not id-like from being referenced, but this general problem with references today.)
18-
// - Numeric indices in brackets for arrays/slices (e.g., items[0].name)
19-
// - Leading '.' is allowed (e.g., .connection.id)
20-
//
21-
// Behavior:
22-
// - For structs: a key matches a field by its json tag name (if present and not "-").
23-
// Embedded anonymous structs are searched.
24-
// - For maps: a key indexes map[string]T (or string alias key types).
25-
// - For slices/arrays: an index [N] selects the N-th element.
26-
// - Wildcards ("*" or "[*]") are not supported and return an error.
2714
func GetByString(v any, path string) (any, error) {
2815
if path == "" {
2916
return v, nil
3017
}
3118

32-
dynPath, err := dyn.NewPathFromString(path)
19+
pathNode, err := structpath.Parse(path)
3320
if err != nil {
3421
return nil, err
3522
}
3623

37-
return Get(v, dynPath)
24+
return Get(v, pathNode)
3825
}
3926

4027
// Get returns the value at the given path inside v.
41-
func Get(v any, path dyn.Path) (any, error) {
42-
if len(path) == 0 {
28+
// Wildcards ("*" or "[*]") are not supported and return an error.
29+
func Get(v any, path *structpath.PathNode) (any, error) {
30+
if path.IsRoot() {
4331
return v, nil
4432
}
4533

34+
// Convert path to slice for easier iteration
35+
pathSegments := path.AsSlice()
36+
4637
cur := reflect.ValueOf(v)
47-
prefix := ""
48-
for _, c := range path {
38+
for _, node := range pathSegments {
4939
var ok bool
5040
cur, ok = deref(cur)
5141
if !ok {
5242
// cannot proceed further due to nil encountered at current location
53-
return nil, fmt.Errorf("%s: cannot access nil value", prefix)
43+
return nil, fmt.Errorf("%s: cannot access nil value", node.Parent().String())
5444
}
5545

56-
if c.Key() != "" {
57-
// Key access: struct field (by json tag) or map key.
58-
newPrefix := prefix
59-
if newPrefix == "" {
60-
newPrefix = c.Key()
61-
} else {
62-
newPrefix = newPrefix + "." + c.Key()
46+
if idx, isIndex := node.Index(); isIndex {
47+
kind := cur.Kind()
48+
if kind != reflect.Slice && kind != reflect.Array {
49+
return nil, fmt.Errorf("%s: cannot index %s", node.String(), kind)
6350
}
64-
nv, err := accessKey(cur, c.Key(), newPrefix)
65-
if err != nil {
66-
return nil, err
51+
if idx < 0 || idx >= cur.Len() {
52+
return nil, fmt.Errorf("%s: index out of range, length is %d", node.String(), cur.Len())
6753
}
68-
cur = nv
69-
prefix = newPrefix
54+
cur = cur.Index(idx)
7055
continue
7156
}
7257

73-
// Index access: slice/array
74-
idx := c.Index()
75-
newPrefix := prefix + "[" + strconv.Itoa(idx) + "]"
76-
kind := cur.Kind()
77-
if kind != reflect.Slice && kind != reflect.Array {
78-
return nil, fmt.Errorf("%s: cannot index %s", newPrefix, kind)
58+
if node.DotStar() || node.BracketStar() {
59+
return nil, fmt.Errorf("wildcards not supported: %s", path.String())
7960
}
80-
if idx < 0 || idx >= cur.Len() {
81-
return nil, fmt.Errorf("%s: index out of range, length is %d", newPrefix, cur.Len())
61+
62+
key, ok := node.StringKey()
63+
if !ok {
64+
return nil, errors.New("unsupported path node type")
65+
}
66+
67+
nv, err := accessKey(cur, key, node)
68+
if err != nil {
69+
return nil, err
8270
}
83-
cur = cur.Index(idx)
84-
prefix = newPrefix
71+
cur = nv
8572
}
8673

8774
// If the current value is invalid (e.g., omitted due to omitempty), return nil.
@@ -100,12 +87,12 @@ func Get(v any, path dyn.Path) (any, error) {
10087

10188
// accessKey returns the field or map entry value selected by key from v.
10289
// v must be non-pointer, non-interface reflect.Value.
103-
func accessKey(v reflect.Value, key, prefix string) (reflect.Value, error) {
90+
func accessKey(v reflect.Value, key string, path *structpath.PathNode) (reflect.Value, error) {
10491
switch v.Kind() {
10592
case reflect.Struct:
10693
fv, sf, owner, ok := findStructFieldByKey(v, key)
10794
if !ok {
108-
return reflect.Value{}, fmt.Errorf("%s: field %q not found in %s", prefix, key, v.Type())
95+
return reflect.Value{}, fmt.Errorf("%s: field %q not found in %s", path.String(), key, v.Type())
10996
}
11097
// Evaluate ForceSendFields on both the current struct and the declaring owner
11198
force := containsForceSendField(v, sf.Name) || containsForceSendField(owner, sf.Name)
@@ -129,19 +116,19 @@ func accessKey(v reflect.Value, key, prefix string) (reflect.Value, error) {
129116
case reflect.Map:
130117
kt := v.Type().Key()
131118
if kt.Kind() != reflect.String {
132-
return reflect.Value{}, fmt.Errorf("%s: map key must be string, got %s", prefix, kt)
119+
return reflect.Value{}, fmt.Errorf("%s: map key must be string, got %s", path.String(), kt)
133120
}
134121
mk := reflect.ValueOf(key)
135122
if kt != mk.Type() {
136123
mk = mk.Convert(kt)
137124
}
138125
mv := v.MapIndex(mk)
139126
if !mv.IsValid() {
140-
return reflect.Value{}, fmt.Errorf("%s: key %q not found in map", prefix, key)
127+
return reflect.Value{}, fmt.Errorf("%s: key %q not found in map", path.String(), key)
141128
}
142129
return mv, nil
143130
default:
144-
return reflect.Value{}, fmt.Errorf("%s: cannot access key %q on %s", prefix, key, v.Kind())
131+
return reflect.Value{}, fmt.Errorf("%s: cannot access key %q on %s", path.String(), key, v.Kind())
145132
}
146133
}
147134

libs/structs/structaccess/get_test.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,15 @@ func runCommonTests(t *testing.T, obj any) {
138138
want: "bar",
139139
},
140140
{
141-
name: "leading dot allowed",
142-
path: ".connection.id",
141+
name: "struct field with bracket notation",
142+
path: "['connection']['id']",
143143
want: "abc",
144144
},
145+
{
146+
name: "map key with bracket notation",
147+
path: "labels['env']",
148+
want: "dev",
149+
},
145150

146151
// Regular scalar fields - always return zero values
147152
{
@@ -169,7 +174,7 @@ func runCommonTests(t *testing.T, obj any) {
169174
{
170175
name: "wildcard not supported",
171176
path: "items[*].id",
172-
errFmt: "invalid path: items[*].id",
177+
errFmt: "wildcards not supported: items[*].id",
173178
},
174179
{
175180
name: "missing field",

0 commit comments

Comments
 (0)