Skip to content

Commit 16e0aa4

Browse files
denikdenik
committed
Record map keys
1 parent eef2982 commit 16e0aa4

File tree

12 files changed

+116
-87
lines changed

12 files changed

+116
-87
lines changed

‎acceptance/bundle/resources/jobs/remote_add_tag/out.plan_post_update.direct.json‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,40 @@
11
{
22
"plan": {
33
"resources.jobs.foo": {
4-
"action": "skip",
4+
"action": "update",
5+
"new_state": {
6+
"value": {
7+
"deployment": {
8+
"kind": "BUNDLE",
9+
"metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
10+
},
11+
"edit_mode": "UI_LOCKED",
12+
"format": "MULTI_TASK",
13+
"max_concurrent_runs": 1,
14+
"name": "My Wheel Job",
15+
"queue": {
16+
"enabled": true
17+
},
18+
"tags": {
19+
"tag1": "tag value"
20+
},
21+
"tasks": [
22+
{
23+
"environment_key": "test_env",
24+
"libraries": [
25+
{
26+
"whl": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/hello.whl"
27+
}
28+
],
29+
"python_wheel_task": {
30+
"entry_point": "run",
31+
"package_name": "my_test_code"
32+
},
33+
"task_key": "TestTask"
34+
}
35+
]
36+
}
37+
},
538
"remote_state": {
639
"created_time": [UNIX_TIME_MILLIS],
740
"creator_user_name": "[USERNAME]",
@@ -49,9 +82,8 @@
4982
"action": "skip",
5083
"reason": "server_side_default"
5184
},
52-
"tags.new_tag": {
53-
"action": "skip",
54-
"reason": "server_side_default"
85+
"tags['new_tag']": {
86+
"action": "update"
5587
},
5688
"timeout_seconds": {
5789
"action": "skip",

‎acceptance/bundle/resources/jobs/remote_add_tag/out.plan_post_update.direct.txt‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎acceptance/bundle/resources/jobs/remote_add_tag/out.plan_post_update.terraform.txt‎

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎acceptance/bundle/resources/jobs/remote_add_tag/output.txt‎

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,46 +6,6 @@ Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/defaul
66
Deploying resources...
77
Updating deployment state...
88
Deployment complete!
9-
+ [CLI] jobs get [JOB_ID]
10-
{
11-
"created_time":[UNIX_TIME_MILLIS],
12-
"creator_user_name":"[USERNAME]",
13-
"job_id":[JOB_ID],
14-
"run_as_user_name":"[USERNAME]",
15-
"settings": {
16-
"deployment": {
17-
"kind":"BUNDLE",
18-
"metadata_file_path":"/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
19-
},
20-
"edit_mode":"UI_LOCKED",
21-
"email_notifications": {},
22-
"format":"MULTI_TASK",
23-
"max_concurrent_runs":1,
24-
"name":"My Wheel Job",
25-
"queue": {
26-
"enabled":true
27-
},
28-
"tags": {
29-
"tag1":"tag value"
30-
},
31-
"tasks": [
32-
{
33-
"environment_key":"test_env",
34-
"libraries": [
35-
{
36-
"whl":"/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/hello.whl"
37-
}
38-
],
39-
"python_wheel_task": {
40-
"entry_point":"run",
41-
"package_name":"my_test_code"
42-
},
43-
"task_key":"TestTask"
44-
}
45-
],
46-
"timeout_seconds":0,
47-
"webhook_notifications": {}
48-
}
49-
}
9+
update jobs.foo
5010

51-
+ [CLI] jobs reset [JOB_ID] --json '{"job_id": "[JOB_ID]", "new_settings": {"deployment": {"kind": "BUNDLE", "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"}, "edit_mode": "UI_LOCKED", "email_notifications": {}, "format": "MULTI_TASK", "max_concurrent_runs": 1, "name": "My Wheel Job", "queue": {"enabled": true}, "tags": {"tag1": "tag value", "new_tag": "new_value"}, "tasks": [{"environment_key": "test_env", "libraries": [{"whl": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/hello.whl"}], "python_wheel_task": {"entry_point": "run", "package_name": "my_test_code"}, "task_key": "TestTask"}], "timeout_seconds": 0, "webhook_notifications": {}}}'
11+
Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged

‎acceptance/bundle/resources/jobs/remote_add_tag/script‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ $CLI bundle deploy
44
job_id="$(read_id.py jobs foo)"
55
echo "$job_id:JOB_ID" >> ACC_REPLS
66

7-
edit_resource.py -v jobs $job_id <<EOF
7+
edit_resource.py jobs $job_id <<EOF
88
r["tags"]["new_tag"] = "new_value"
99
EOF
1010

11-
$CLI bundle plan > out.plan_post_update.$DATABRICKS_BUNDLE_ENGINE.txt
11+
$CLI bundle plan
1212
$CLI bundle plan -o json > out.plan_post_update.$DATABRICKS_BUNDLE_ENGINE.json

‎bundle/direct/bundle_plan.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,12 +260,11 @@ func interpretOldStateVsRemoteState(ctx context.Context, adapter *dresources.Ada
260260
m := make(map[string]deployplan.Trigger)
261261

262262
for _, ch := range diff {
263-
if ch.Old == nil && ch.Path.IsStringKey() {
263+
if ch.Old == nil && ch.Path.IsDotString() {
264264
// The field was not set by us, but comes from the remote state.
265265
// This could either be server-side default or a policy.
266266
// In any case, this is not a change we should react to.
267-
// Note, we only consider StringKeys here, because indexes and key-value pairs refer to slices and we want to react to new element in slices.
268-
// Note, IsStringKey is also too broad - it currently covers struct fields and map keys, we don't want to include map keys here.
267+
// Note, we only consider struct fields here. Adding/removing elements to/from maps and slices should trigger updates.
269268
m[ch.Path.String()] = deployplan.Trigger{
270269
Action: deployplan.ActionTypeSkipString,
271270
Reason: "server_side_default",

‎libs/structs/structdiff/diff.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Valu
205205
if fieldName == "" {
206206
fieldName = sf.Name
207207
}
208-
node := structpath.NewStringKey(path, fieldName)
208+
node := structpath.NewDotString(path, fieldName)
209209

210210
v1Field := s1.Field(i)
211211
v2Field := s2.Field(i)
@@ -257,7 +257,7 @@ func diffMapStringKey(ctx *diffContext, path *structpath.PathNode, m1, m2 reflec
257257
k := keySet[ks]
258258
v1 := m1.MapIndex(k)
259259
v2 := m2.MapIndex(k)
260-
node := structpath.NewStringKey(path, ks)
260+
node := structpath.NewBracketString(path, ks)
261261
if err := diffValues(ctx, node, v1, v2, changes); err != nil {
262262
return err
263263
}

‎libs/structs/structdiff/diff_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func TestGetStructDiff(t *testing.T) {
147147
name: "map diff",
148148
a: A{M: map[string]int{"a": 1}},
149149
b: A{M: map[string]int{"a": 2}},
150-
want: []ResolvedChange{{Field: "m.a", Old: 1, New: 2}},
150+
want: []ResolvedChange{{Field: "m['a']", Old: 1, New: 2}},
151151
},
152152
{
153153
name: "slice diff",
@@ -258,9 +258,9 @@ func TestGetStructDiff(t *testing.T) {
258258
a: map[string]C{"key1": {Title: "title", ForceSendFields: []string{"Name", "IsEnabled", "Title"}}},
259259
b: map[string]C{"key1": {Title: "title", ForceSendFields: []string{"Age"}}},
260260
want: []ResolvedChange{
261-
{Field: "key1.name", Old: "", New: nil},
262-
{Field: "key1.age", Old: nil, New: 0},
263-
{Field: "key1.is_enabled", Old: false, New: nil},
261+
{Field: "['key1'].name", Old: "", New: nil},
262+
{Field: "['key1'].age", Old: nil, New: 0},
263+
{Field: "['key1'].is_enabled", Old: false, New: nil},
264264
},
265265
},
266266

@@ -270,9 +270,9 @@ func TestGetStructDiff(t *testing.T) {
270270
a: map[string]*C{"key1": {Title: "title", ForceSendFields: []string{"Name", "IsEnabled", "Title"}}},
271271
b: map[string]*C{"key1": {Title: "title", ForceSendFields: []string{"Age"}}},
272272
want: []ResolvedChange{
273-
{Field: "key1.name", Old: "", New: nil},
274-
{Field: "key1.age", Old: nil, New: 0},
275-
{Field: "key1.is_enabled", Old: false, New: nil},
273+
{Field: "['key1'].name", Old: "", New: nil},
274+
{Field: "['key1'].age", Old: nil, New: 0},
275+
{Field: "['key1'].is_enabled", Old: false, New: nil},
276276
},
277277
},
278278

‎libs/structs/structpath/path.go‎

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@ import (
1111
)
1212

1313
const (
14-
// Encodes string key, which is encoded as .field or as ['spark.conf']
15-
tagStringKey = -1
16-
1714
// Encodes wildcard after a dot: foo.*
1815
tagDotStar = -2
1916

@@ -22,6 +19,13 @@ const (
2219

2320
// Encodes key/value index, which is encoded as [key='value'] or [key="value"]
2421
tagKeyValue = -4
22+
23+
// Encodes .field syntax
24+
// Note, most users should use StringKey() method which represents both DotString and BracketString
25+
tagDotString = -5
26+
27+
// Encodes ["field"] syntax
28+
tagBracketString = -6
2529
)
2630

2731
// PathNode represents a node in a path for struct diffing.
@@ -73,16 +77,36 @@ func (p *PathNode) KeyValue() (key, value string, ok bool) {
7377
return "", "", false
7478
}
7579

76-
func (p *PathNode) IsStringKey() bool {
77-
return p != nil && p.index == tagStringKey
80+
func (p *PathNode) IsDotString() bool {
81+
return p != nil && p.index == tagDotString
82+
}
83+
84+
func (p *PathNode) DotString() (string, bool) {
85+
if p == nil {
86+
return "", false
87+
}
88+
if p.index == tagDotString {
89+
return p.key, true
90+
}
91+
return "", false
92+
}
93+
94+
func (p *PathNode) BracketString() (string, bool) {
95+
if p == nil {
96+
return "", false
97+
}
98+
if p.index == tagBracketString {
99+
return p.key, true
100+
}
101+
return "", false
78102
}
79103

80104
// StringKey returns either Field() or MapKey() if either is available
81105
func (p *PathNode) StringKey() (string, bool) {
82106
if p == nil {
83107
return "", false
84108
}
85-
if p.index == tagStringKey {
109+
if p.index == tagDotString || p.index == tagBracketString {
86110
return p.key, true
87111
}
88112
return "", false
@@ -122,14 +146,32 @@ func NewIndex(prev *PathNode, index int) *PathNode {
122146
}
123147
}
124148

125-
// NewStringKey creates either StructField or MapKey
126-
// The fieldName should be the resolved field name (e.g., from JSON tag or Go field name).
127-
func NewStringKey(prev *PathNode, fieldName string) *PathNode {
149+
// NewDotString creates a PathNode for dot notation (.field).
150+
func NewDotString(prev *PathNode, fieldName string) *PathNode {
128151
return &PathNode{
129152
prev: prev,
130153
key: fieldName,
131-
index: tagStringKey,
154+
index: tagDotString,
155+
}
156+
}
157+
158+
// NewBracketString creates a PathNode for bracket notation (["field"]).
159+
func NewBracketString(prev *PathNode, fieldName string) *PathNode {
160+
return &PathNode{
161+
prev: prev,
162+
key: fieldName,
163+
index: tagBracketString,
164+
}
165+
}
166+
167+
// NewStringKey creates a PathNode, choosing dot notation if the field name is valid,
168+
// otherwise bracket notation. This maintains backward compatibility while automatically
169+
// selecting the appropriate representation.
170+
func NewStringKey(prev *PathNode, fieldName string) *PathNode {
171+
if isValidField(fieldName) {
172+
return NewDotString(prev, fieldName)
132173
}
174+
return NewBracketString(prev, fieldName)
133175
}
134176

135177
func NewDotStar(prev *PathNode) *PathNode {
@@ -193,14 +235,14 @@ func (p *PathNode) String() string {
193235
result.WriteString("=")
194236
result.WriteString(EncodeMapKey(node.value))
195237
result.WriteString("]")
196-
} else if isValidField(node.key) {
197-
// Valid field name
238+
} else if node.index == tagDotString {
239+
// Dot notation: .field
198240
if i != 0 {
199241
result.WriteString(".")
200242
}
201243
result.WriteString(node.key)
202-
} else {
203-
// Map key with single quotes
244+
} else if node.index == tagBracketString {
245+
// Bracket notation: ['field']
204246
result.WriteString("[")
205247
result.WriteString(EncodeMapKey(node.key))
206248
result.WriteString("]")
@@ -309,11 +351,11 @@ func Parse(s string) (*PathNode, error) {
309351

310352
case stateField:
311353
if ch == '.' {
312-
result = NewStringKey(result, currentToken.String())
354+
result = NewDotString(result, currentToken.String())
313355
currentToken.Reset()
314356
state = stateFieldStart
315357
} else if ch == '[' {
316-
result = NewStringKey(result, currentToken.String())
358+
result = NewDotString(result, currentToken.String())
317359
currentToken.Reset()
318360
state = stateBracketOpen
319361
} else if !isReservedFieldChar(ch) {
@@ -380,7 +422,7 @@ func Parse(s string) (*PathNode, error) {
380422
state = stateMapKey
381423
case ']':
382424
// End of map key
383-
result = NewStringKey(result, currentToken.String())
425+
result = NewBracketString(result, currentToken.String())
384426
currentToken.Reset()
385427
state = stateExpectDotOrEnd
386428
default:
@@ -461,7 +503,7 @@ func Parse(s string) (*PathNode, error) {
461503
case stateStart:
462504
return result, nil // Empty path, result is nil
463505
case stateField:
464-
result = NewStringKey(result, currentToken.String())
506+
result = NewDotString(result, currentToken.String())
465507
return result, nil
466508
case stateDotStar:
467509
result = NewDotStar(result)

‎libs/structs/structwalk/walk.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func walkValue(path *structpath.PathNode, val reflect.Value, field *reflect.Stru
9292
sort.Strings(keys)
9393
for _, ks := range keys {
9494
v := val.MapIndex(reflect.ValueOf(ks))
95-
node := structpath.NewStringKey(path, ks)
95+
node := structpath.NewBracketString(path, ks)
9696
walkValue(node, v, nil, visit)
9797
}
9898

@@ -131,7 +131,7 @@ func walkStruct(path *structpath.PathNode, s reflect.Value, visit VisitFunc) {
131131
if fieldName == "" {
132132
fieldName = sf.Name
133133
}
134-
node := structpath.NewStringKey(path, fieldName)
134+
node := structpath.NewDotString(path, fieldName)
135135

136136
fieldVal := s.Field(i)
137137
// Skip zero values with omitempty unless field is explicitly forced.

0 commit comments

Comments
 (0)