diff --git a/libs/dyn/convert/end_to_end_test.go b/libs/dyn/convert/end_to_end_test.go index fbb84336..7c048136 100644 --- a/libs/dyn/convert/end_to_end_test.go +++ b/libs/dyn/convert/end_to_end_test.go @@ -39,6 +39,15 @@ func TestAdditional(t *testing.T) { }) }) + t.Run("map with empty string value", func(t *testing.T) { + s := "" + assertFromTypedToTypedEqual(t, Tmp{ + MapToPointer: map[string]*string{ + "key": &s, + }, + }) + }) + t.Run("map with nil value", func(t *testing.T) { assertFromTypedToTypedEqual(t, Tmp{ MapToPointer: map[string]*string{ diff --git a/libs/dyn/convert/from_typed.go b/libs/dyn/convert/from_typed.go index 0659d1cd..75f1c721 100644 --- a/libs/dyn/convert/from_typed.go +++ b/libs/dyn/convert/from_typed.go @@ -3,13 +3,35 @@ package convert import ( "fmt" "reflect" + "slices" "github.com/databricks/cli/libs/dyn" ) +type fromTypedOptions int + +const ( + // Use the zero value instead of setting zero values to nil. This is useful + // for types where the zero values and nil are semantically different. That is + // strings, bools, ints, floats. + // + // Note: this is not needed for structs because dyn.NilValue is converted back + // to a zero value when using the convert.ToTyped function. + // + // Values in maps and slices should be set to zero values, and not nil in the + // dynamic representation. + includeZeroValues fromTypedOptions = 1 << iota +) + // FromTyped converts changes made in the typed structure w.r.t. the configuration value // back to the configuration value, retaining existing location information where possible. func FromTyped(src any, ref dyn.Value) (dyn.Value, error) { + return fromTyped(src, ref) +} + +// Private implementation of FromTyped that allows for additional options not exposed +// in the public API. +func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) { srcv := reflect.ValueOf(src) // Dereference pointer if necessary @@ -28,13 +50,13 @@ func FromTyped(src any, ref dyn.Value) (dyn.Value, error) { case reflect.Slice: return fromTypedSlice(srcv, ref) case reflect.String: - return fromTypedString(srcv, ref) + return fromTypedString(srcv, ref, options...) case reflect.Bool: - return fromTypedBool(srcv, ref) + return fromTypedBool(srcv, ref, options...) case reflect.Int, reflect.Int32, reflect.Int64: - return fromTypedInt(srcv, ref) + return fromTypedInt(srcv, ref, options...) case reflect.Float32, reflect.Float64: - return fromTypedFloat(srcv, ref) + return fromTypedFloat(srcv, ref, options...) } return dyn.NilValue, fmt.Errorf("unsupported type: %s", srcv.Kind()) @@ -52,7 +74,7 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value) (dyn.Value, error) { info := getStructInfo(src.Type()) for k, v := range info.FieldValues(src) { // Convert the field taking into account the reference value (may be equal to config.NilValue). - nv, err := FromTyped(v.Interface(), ref.Get(k)) + nv, err := fromTyped(v.Interface(), ref.Get(k)) if err != nil { return dyn.Value{}, err } @@ -89,8 +111,8 @@ func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) { k := iter.Key().String() v := iter.Value() - // Convert entry taking into account the reference value (may be equal to config.NilValue). - nv, err := FromTyped(v.Interface(), ref.Get(k)) + // Convert entry taking into account the reference value (may be equal to dyn.NilValue). + nv, err := fromTyped(v.Interface(), ref.Get(k), includeZeroValues) if err != nil { return dyn.Value{}, err } @@ -120,8 +142,8 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) { for i := 0; i < src.Len(); i++ { v := src.Index(i) - // Convert entry taking into account the reference value (may be equal to config.NilValue). - nv, err := FromTyped(v.Interface(), ref.Index(i)) + // Convert entry taking into account the reference value (may be equal to dyn.NilValue). + nv, err := fromTyped(v.Interface(), ref.Index(i), includeZeroValues) if err != nil { return dyn.Value{}, err } @@ -132,7 +154,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) { return dyn.NewValue(out, ref.Location()), nil } -func fromTypedString(src reflect.Value, ref dyn.Value) (dyn.Value, error) { +func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) { switch ref.Kind() { case dyn.KindString: value := src.String() @@ -142,9 +164,9 @@ func fromTypedString(src reflect.Value, ref dyn.Value) (dyn.Value, error) { return dyn.V(value), nil case dyn.KindNil: - // This field is not set in the reference, so we only include it if it has a non-zero value. - // Otherwise, we would always include all zero valued fields. - if src.IsZero() { + // This field is not set in the reference. We set it to nil if it's zero + // valued in the typed representation and the includeZeroValues option is not set. + if src.IsZero() && !slices.Contains(options, includeZeroValues) { return dyn.NilValue, nil } return dyn.V(src.String()), nil @@ -153,7 +175,7 @@ func fromTypedString(src reflect.Value, ref dyn.Value) (dyn.Value, error) { return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind()) } -func fromTypedBool(src reflect.Value, ref dyn.Value) (dyn.Value, error) { +func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) { switch ref.Kind() { case dyn.KindBool: value := src.Bool() @@ -162,9 +184,9 @@ func fromTypedBool(src reflect.Value, ref dyn.Value) (dyn.Value, error) { } return dyn.V(value), nil case dyn.KindNil: - // This field is not set in the reference, so we only include it if it has a non-zero value. - // Otherwise, we would always include all zero valued fields. - if src.IsZero() { + // This field is not set in the reference. We set it to nil if it's zero + // valued in the typed representation and the includeZeroValues option is not set. + if src.IsZero() && !slices.Contains(options, includeZeroValues) { return dyn.NilValue, nil } return dyn.V(src.Bool()), nil @@ -173,7 +195,7 @@ func fromTypedBool(src reflect.Value, ref dyn.Value) (dyn.Value, error) { return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind()) } -func fromTypedInt(src reflect.Value, ref dyn.Value) (dyn.Value, error) { +func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) { switch ref.Kind() { case dyn.KindInt: value := src.Int() @@ -182,9 +204,9 @@ func fromTypedInt(src reflect.Value, ref dyn.Value) (dyn.Value, error) { } return dyn.V(value), nil case dyn.KindNil: - // This field is not set in the reference, so we only include it if it has a non-zero value. - // Otherwise, we would always include all zero valued fields. - if src.IsZero() { + // This field is not set in the reference. We set it to nil if it's zero + // valued in the typed representation and the includeZeroValues option is not set. + if src.IsZero() && !slices.Contains(options, includeZeroValues) { return dyn.NilValue, nil } return dyn.V(src.Int()), nil @@ -193,7 +215,7 @@ func fromTypedInt(src reflect.Value, ref dyn.Value) (dyn.Value, error) { return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind()) } -func fromTypedFloat(src reflect.Value, ref dyn.Value) (dyn.Value, error) { +func fromTypedFloat(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) { switch ref.Kind() { case dyn.KindFloat: value := src.Float() @@ -202,9 +224,9 @@ func fromTypedFloat(src reflect.Value, ref dyn.Value) (dyn.Value, error) { } return dyn.V(value), nil case dyn.KindNil: - // This field is not set in the reference, so we only include it if it has a non-zero value. - // Otherwise, we would always include all zero valued fields. - if src.IsZero() { + // This field is not set in the reference. We set it to nil if it's zero + // valued in the typed representation and the includeZeroValues option is not set. + if src.IsZero() && !slices.Contains(options, includeZeroValues) { return dyn.NilValue, nil } return dyn.V(src.Float()), nil diff --git a/libs/dyn/convert/from_typed_test.go b/libs/dyn/convert/from_typed_test.go index 0e9b9c7c..d7fa60bb 100644 --- a/libs/dyn/convert/from_typed_test.go +++ b/libs/dyn/convert/from_typed_test.go @@ -68,6 +68,190 @@ func TestFromTypedStructSetFieldsRetainLocationIfUnchanged(t *testing.T) { assert.Equal(t, dyn.NewValue("qux", dyn.Location{}), nv.Get("bar")) } +func TestFromTypedStringMapWithZeroValue(t *testing.T) { + ref := dyn.NilValue + src := map[string]string{ + "foo": "", + "bar": "fuzz", + } + + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V(map[string]dyn.Value{ + "foo": dyn.V(""), + "bar": dyn.V("fuzz"), + }), nv) +} + +func TestFromTypedStringSliceWithZeroValue(t *testing.T) { + ref := dyn.NilValue + src := []string{"a", "", "c"} + + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V([]dyn.Value{ + dyn.V("a"), dyn.V(""), dyn.V("c"), + }), nv) +} + +func TestFromTypedStringStructWithZeroValue(t *testing.T) { + type Tmp struct { + Foo string `json:"foo"` + Bar string `json:"bar"` + } + + ref := dyn.NilValue + src := Tmp{ + Foo: "foo", + Bar: "", + } + + // Note, the zero value is not included in the output. + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V(map[string]dyn.Value{ + "foo": dyn.V("foo"), + }), nv) +} + +func TestFromTypedBoolMapWithZeroValue(t *testing.T) { + ref := dyn.NilValue + src := map[string]bool{ + "foo": false, + "bar": true, + } + + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V(map[string]dyn.Value{ + "foo": dyn.V(false), + "bar": dyn.V(true), + }), nv) +} + +func TestFromTypedBoolSliceWithZeroValue(t *testing.T) { + ref := dyn.NilValue + src := []bool{true, false, true} + + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V([]dyn.Value{ + dyn.V(true), dyn.V(false), dyn.V(true), + }), nv) +} + +func TestFromTypedBoolStructWithZeroValue(t *testing.T) { + type Tmp struct { + Foo bool `json:"foo"` + Bar bool `json:"bar"` + } + + ref := dyn.NilValue + src := Tmp{ + Foo: true, + Bar: false, + } + + // Note, the zero value is not included in the output. + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V(map[string]dyn.Value{ + "foo": dyn.V(true), + }), nv) +} + +func TestFromTypedIntMapWithZeroValue(t *testing.T) { + ref := dyn.NilValue + src := map[string]int{ + "foo": 0, + "bar": 1, + } + + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V(map[string]dyn.Value{ + "foo": dyn.V(int64(0)), + "bar": dyn.V(int64(1)), + }), nv) +} + +func TestFromTypedIntSliceWithZeroValue(t *testing.T) { + ref := dyn.NilValue + src := []int{1, 0, 2} + + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V([]dyn.Value{ + dyn.V(int64(1)), dyn.V(int64(0)), dyn.V(int64(2)), + }), nv) +} + +func TestFromTypedIntStructWithZeroValue(t *testing.T) { + type Tmp struct { + Foo int `json:"foo"` + Bar int `json:"bar"` + } + + ref := dyn.NilValue + src := Tmp{ + Foo: 1, + Bar: 0, + } + + // Note, the zero value is not included in the output. + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V(map[string]dyn.Value{ + "foo": dyn.V(int64(1)), + }), nv) +} + +func TestFromTypedFloatMapWithZeroValue(t *testing.T) { + ref := dyn.NilValue + src := map[string]float64{ + "foo": 0.0, + "bar": 1.0, + } + + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V(map[string]dyn.Value{ + "foo": dyn.V(0.0), + "bar": dyn.V(1.0), + }), nv) +} + +func TestFromTypedFloatSliceWithZeroValue(t *testing.T) { + ref := dyn.NilValue + src := []float64{1.0, 0.0, 2.0} + + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V([]dyn.Value{ + dyn.V(1.0), dyn.V(0.0), dyn.V(2.0), + }), nv) +} + +func TestFromTypedFloatStructWithZeroValue(t *testing.T) { + type Tmp struct { + Foo float64 `json:"foo"` + Bar float64 `json:"bar"` + } + + ref := dyn.NilValue + src := Tmp{ + Foo: 1.0, + Bar: 0.0, + } + + // Note, the zero value is not included in the output. + nv, err := FromTyped(src, ref) + require.NoError(t, err) + assert.Equal(t, dyn.V(map[string]dyn.Value{ + "foo": dyn.V(1.0), + }), nv) +} + func TestFromTypedMapNil(t *testing.T) { var src map[string]string = nil @@ -139,7 +323,7 @@ func TestFromTypedMapFieldWithZeroValue(t *testing.T) { nv, err := FromTyped(src, ref) require.NoError(t, err) assert.Equal(t, dyn.V(map[string]dyn.Value{ - "foo": dyn.NilValue, + "foo": dyn.V(""), }), nv) }