Fix dynamic representation of zero values in maps and slices (#1154)

## Changes
In the dynamic configuration, the nil value (dyn.NilValue) denotes a
value that should not be serialized, ie a value being nil is the same as
it not existing in the first place.

This is not true for zero values in maps and slices. This PR fixes the
conversion from typed values to dyn.Value, to treat zero values in maps
and slices as zero and not nil.

## Tests
Unit tests

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{

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).
+		// Convert entry taking into account the reference value (may be equal to dyn.NilValue).
-		nv, err := FromTyped(v.Interface(), ref.Get(k))
+		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).
+		// Convert entry taking into account the reference value (may be equal to dyn.NilValue).
-		nv, err := FromTyped(v.Interface(), ref.Index(i))
+		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.
+		// This field is not set in the reference. We set it to nil if it's zero
-		// Otherwise, we would always include all zero valued fields.
+		// valued in the typed representation and the includeZeroValues option is not set.
-		if src.IsZero() {
+		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.
+		// This field is not set in the reference. We set it to nil if it's zero
-		// Otherwise, we would always include all zero valued fields.
+		// valued in the typed representation and the includeZeroValues option is not set.
-		if src.IsZero() {
+		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.
+		// This field is not set in the reference. We set it to nil if it's zero
-		// Otherwise, we would always include all zero valued fields.
+		// valued in the typed representation and the includeZeroValues option is not set.
-		if src.IsZero() {
+		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.
+		// This field is not set in the reference. We set it to nil if it's zero
-		// Otherwise, we would always include all zero valued fields.
+		// valued in the typed representation and the includeZeroValues option is not set.
-		if src.IsZero() {
+		if src.IsZero() && !slices.Contains(options, includeZeroValues) {
 			return dyn.NilValue, nil
 		}
 		return dyn.V(src.Float()), nil

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)
 }