Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions libs/structs/structaccess/typecheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
6 changes: 4 additions & 2 deletions libs/structs/structdiff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
28 changes: 28 additions & 0 deletions libs/structs/structdiff/diff_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package structdiff

import (
"encoding/json"
"reflect"
"testing"
"time"
Expand All @@ -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 }
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 4 additions & 2 deletions libs/structs/structdiff/equal.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"reflect"
"slices"

"github.com/databricks/cli/libs/structs/structaccess"
"github.com/databricks/cli/libs/structs/structtag"
)

Expand Down Expand Up @@ -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
}
Expand Down
6 changes: 4 additions & 2 deletions libs/structs/structwalk/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
52 changes: 52 additions & 0 deletions libs/structs/structwalk/walk_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package structwalk

import (
"encoding/json"
"reflect"
"slices"
"testing"

"github.com/databricks/cli/libs/structs/structpath"
Expand Down Expand Up @@ -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
}
8 changes: 4 additions & 4 deletions libs/structs/structwalk/walktype.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading