diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index 968e23bf94f..b55fa4c136a 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -209,3 +209,23 @@ func embeddedStructTypes(t reflect.Type) []reflect.Type { } return out } + +// IsFlattenedEmbed reports whether the field is an embed encoding/json flattens into the +// outer object. An anonymous field that carries a json *name* is a named field instead: it +// serializes as a nested object under that name. The name is what matters, not the presence +// of a tag: `json:",omitempty"` leaves the name empty, so such a field is still flattened. +func IsFlattenedEmbed(sf reflect.StructField) bool { + if !sf.Anonymous { + return false + } + if structtag.JSONTag(sf.Tag.Get("json")).Name() != "" { + return false + } + // Only an anonymous *struct* is promoted. An embedded scalar, slice or interface + // is a member named after its type, not a flattened embed. + ft := sf.Type + for ft.Kind() == reflect.Pointer { + ft = ft.Elem() + } + return ft.Kind() == reflect.Struct +} diff --git a/libs/structs/structdiff/diff.go b/libs/structs/structdiff/diff.go index 30c0fc541f3..5e7a00cac4f 100644 --- a/libs/structs/structdiff/diff.go +++ b/libs/structs/structdiff/diff.go @@ -209,8 +209,10 @@ func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Valu continue } - // Continue traversing embedded structs. Do not add the key to the path though. - if sf.Anonymous { + // Continue traversing embedded structs. Do not add the key to the path though. An + // anonymous field carrying a json name is not one of these: encoding/json serializes it + // as a nested object, so it is handled as a named field below. + if structaccess.IsFlattenedEmbed(sf) { if err := diffValues(ctx, path, s1.Field(i), s2.Field(i), changes); err != nil { return err } diff --git a/libs/structs/structdiff/diff_test.go b/libs/structs/structdiff/diff_test.go index 478cfd08a2e..34b877d50dd 100644 --- a/libs/structs/structdiff/diff_test.go +++ b/libs/structs/structdiff/diff_test.go @@ -1,6 +1,7 @@ package structdiff import ( + "encoding/json" "reflect" "testing" "time" @@ -9,6 +10,7 @@ import ( sdktime "github.com/databricks/databricks-sdk-go/common/types/time" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type B struct{ S string } @@ -924,6 +926,32 @@ func TestGetStructDiffSliceKeysDuplicates(t *testing.T) { } } +// An anonymous field carrying a json name is a named field to encoding/json: it serializes as +// a nested object, so a change inside it belongs at that nested path. +type DiffEmbedLeaf struct { + Value string `json:"value,omitempty"` +} + +type diffTaggedEmbed struct { + DiffEmbedLeaf `json:"leaf"` + + Own string `json:"own,omitempty"` +} + +func TestDiffTaggedEmbedIsReportedUnderItsName(t *testing.T) { + before := &diffTaggedEmbed{DiffEmbedLeaf: DiffEmbedLeaf{Value: "before"}, Own: "o"} + after := &diffTaggedEmbed{DiffEmbedLeaf: DiffEmbedLeaf{Value: "after"}, Own: "o"} + + blob, err := json.Marshal(after) + require.NoError(t, err) + assert.JSONEq(t, `{"leaf":{"value":"after"},"own":"o"}`, string(blob)) + + changes, err := GetStructDiff(before, after, nil) + require.NoError(t, err) + require.Len(t, changes, 1) + assert.Equal(t, "leaf.value", changes[0].Path.String()) +} + // namedMapKey is a defined string type used as a map key, like config's // ScriptHook (`type ScriptHook string`). Its kind is String but its dynamic // type is not string. diff --git a/libs/structs/structdiff/equal.go b/libs/structs/structdiff/equal.go index 6bf253c7317..50e703a97c0 100644 --- a/libs/structs/structdiff/equal.go +++ b/libs/structs/structdiff/equal.go @@ -4,6 +4,7 @@ import ( "reflect" "slices" + "github.com/databricks/cli/libs/structs/structaccess" "github.com/databricks/cli/libs/structs/structtag" ) @@ -107,8 +108,9 @@ func equalStruct(s1, s2 reflect.Value) bool { continue } - // Continue traversing embedded structs. - if sf.Anonymous { + // Continue traversing embedded structs. A tagged anonymous field is a named field to + // encoding/json, so it goes through the path below. + if structaccess.IsFlattenedEmbed(sf) { if !equalValues(s1.Field(i), s2.Field(i)) { return false } diff --git a/libs/structs/structwalk/walk.go b/libs/structs/structwalk/walk.go index 96a0cd1271a..57d07c5138f 100644 --- a/libs/structs/structwalk/walk.go +++ b/libs/structs/structwalk/walk.go @@ -115,8 +115,10 @@ func walkStruct(path *structpath.PathNode, s reflect.Value, visit VisitFunc) { continue } - // Directly walk into embedded structs without adding the key to the path. - if sf.Anonymous { + // Directly walk into embedded structs without adding the key to the path. An + // anonymous field carrying a json name is not one of these: encoding/json serializes it + // as a nested object, so it is walked as a named field below. + if structaccess.IsFlattenedEmbed(sf) { walkValue(path, s.Field(i), &sf, visit) continue } diff --git a/libs/structs/structwalk/walk_test.go b/libs/structs/structwalk/walk_test.go index aae419bfaee..eb71a02046a 100644 --- a/libs/structs/structwalk/walk_test.go +++ b/libs/structs/structwalk/walk_test.go @@ -1,7 +1,9 @@ package structwalk import ( + "encoding/json" "reflect" + "slices" "testing" "github.com/databricks/cli/libs/structs/structpath" @@ -276,3 +278,53 @@ func TestEmbeddedStructWithJSONTagDash(t *testing.T) { "parent_field": "parent", }, flatten(t, parent)) } + +// An anonymous field carrying a json name is a named field to encoding/json: it serializes as +// a nested object under that name rather than being flattened into the outer one. A field whose +// tag sets only an option, leaving the name empty, is still flattened. +type WalkEmbedLeaf struct { + Value string `json:"value,omitempty"` +} + +type walkTaggedEmbed struct { + WalkEmbedLeaf `json:"leaf"` + + Own string `json:"own,omitempty"` +} + +type walkOptionOnlyEmbed struct { + WalkEmbedLeaf `json:",omitempty"` + + Own string `json:"own,omitempty"` +} + +func TestWalkTaggedEmbedIsANamedField(t *testing.T) { + value := &walkTaggedEmbed{WalkEmbedLeaf: WalkEmbedLeaf{Value: "v"}, Own: "o"} + + blob, err := json.Marshal(value) + require.NoError(t, err) + assert.JSONEq(t, `{"leaf":{"value":"v"},"own":"o"}`, string(blob)) + + assert.Equal(t, []string{"leaf.value", "own"}, walkPaths(t, value)) +} + +func TestWalkOptionOnlyEmbedIsStillFlattened(t *testing.T) { + value := &walkOptionOnlyEmbed{WalkEmbedLeaf: WalkEmbedLeaf{Value: "v"}, Own: "o"} + + blob, err := json.Marshal(value) + require.NoError(t, err) + assert.JSONEq(t, `{"value":"v","own":"o"}`, string(blob)) + + assert.Equal(t, []string{"own", "value"}, walkPaths(t, value)) +} + +// walkPaths returns the sorted paths Walk visits for v. +func walkPaths(t *testing.T, value any) []string { + t.Helper() + var paths []string + require.NoError(t, Walk(value, func(path *structpath.PathNode, _ any, _ *reflect.StructField) { + paths = append(paths, path.String()) + })) + slices.Sort(paths) + return paths +} diff --git a/libs/structs/structwalk/walktype.go b/libs/structs/structwalk/walktype.go index cd4be28c843..bdb58cd1644 100644 --- a/libs/structs/structwalk/walktype.go +++ b/libs/structs/structwalk/walktype.go @@ -109,11 +109,11 @@ func walkTypeStruct(path *structpath.PatternNode, st reflect.Type, visit VisitTy continue // unexported } - // Handle embedded structs (anonymous fields without json tags) + // Handle embedded structs that encoding/json flattens. The json *name* decides, not + // the presence of a tag: `json:",omitempty"` on an embed leaves the name empty and is + // still flattened, while a name makes it a nested object. jsonTag := sf.Tag.Get("json") - if sf.Anonymous && jsonTag == "" { - // For embedded structs, walk the embedded type at the current path level - // This flattens the embedded struct's fields into the parent struct + if structaccess.IsFlattenedEmbed(sf) { walkTypeValue(path, sf.Type, &sf, visit, visitedCount) continue }