Skip to content

Commit

Permalink
Handle normalization of dyn.KindTime into an any type (databricks#1836
Browse files Browse the repository at this point in the history
)

## Changes

The issue reported in databricks#1828 illustrates how using a YAML timestamp-like
value (a date in this case) causes an issue during conversion to and
from the typed configuration tree.

We use the `AsAny()` function on the `dyn.Value` when normalizing for
the `any` type. We only use the `any` type for variable values, because
they can assume every type. The `AsAny()` function returns a `time.Time`
for the time value during conversion **to** the typed configuration
tree. Upon conversion **from** the typed configuration tree back into
the dynamic configuration tree, we cannot distinguish a `time.Time`
struct from any other struct.

To address this, we use the underlying string value of the time value
when we normalize for the `any` type.

Fixes databricks#1828.

## Tests

Existing unit tests pass
  • Loading branch information
pietern authored Oct 17, 2024
1 parent cc11296 commit e4d039a
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 9 deletions.
33 changes: 33 additions & 0 deletions bundle/tests/issue_1828/databricks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
bundle:
name: issue_1828

variables:
# One entry for each of the underlying YAML (or [dyn.Kind]) types.
# The test confirms we can convert to and from the typed configuration without losing information.

map:
default:
foo: bar

sequence:
default:
- foo
- bar

string:
default: foo

bool:
default: true

int:
default: 42

float:
default: 3.14

time:
default: 2021-01-01

nil:
default:
48 changes: 48 additions & 0 deletions bundle/tests/issue_1828_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package config_tests

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestIssue1828(t *testing.T) {
b := load(t, "./issue_1828")

if assert.Contains(t, b.Config.Variables, "map") {
assert.Equal(t, map[string]any{
"foo": "bar",
}, b.Config.Variables["map"].Default)
}

if assert.Contains(t, b.Config.Variables, "sequence") {
assert.Equal(t, []any{
"foo",
"bar",
}, b.Config.Variables["sequence"].Default)
}

if assert.Contains(t, b.Config.Variables, "string") {
assert.Equal(t, "foo", b.Config.Variables["string"].Default)
}

if assert.Contains(t, b.Config.Variables, "bool") {
assert.Equal(t, true, b.Config.Variables["bool"].Default)
}

if assert.Contains(t, b.Config.Variables, "int") {
assert.Equal(t, 42, b.Config.Variables["int"].Default)
}

if assert.Contains(t, b.Config.Variables, "float") {
assert.Equal(t, 3.14, b.Config.Variables["float"].Default)
}

if assert.Contains(t, b.Config.Variables, "time") {
assert.Equal(t, "2021-01-01", b.Config.Variables["time"].Default)
}

if assert.Contains(t, b.Config.Variables, "nil") {
assert.Equal(t, nil, b.Config.Variables["nil"].Default)
}
}
30 changes: 29 additions & 1 deletion libs/dyn/convert/normalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,34 @@ func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value, path d
return dyn.NewValue(out, src.Locations()), diags
}

func (n normalizeOptions) normalizeInterface(typ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) {
func (n normalizeOptions) normalizeInterface(_ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) {
// Deal with every [dyn.Kind] here to ensure completeness.
switch src.Kind() {
case dyn.KindMap:
// Fall through
case dyn.KindSequence:
// Fall through
case dyn.KindString:
// Fall through
case dyn.KindBool:
// Fall through
case dyn.KindInt:
// Fall through
case dyn.KindFloat:
// Fall through
case dyn.KindTime:
// Conversion of a time value to an interface{}.
// The [dyn.Value.AsAny] equivalent for this kind is the [time.Time] struct.
// If we convert to a typed representation and back again, we cannot distinguish
// a [time.Time] struct from any other struct.
//
// Therefore, we normalize the time value to a string.
return dyn.NewValue(src.MustTime().String(), src.Locations()), nil
case dyn.KindNil:
// Fall through
default:
return dyn.InvalidValue, diag.Errorf("unsupported kind: %s", src.Kind())
}

return src, nil
}
32 changes: 24 additions & 8 deletions libs/dyn/convert/normalize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -858,28 +858,44 @@ func TestNormalizeAnchors(t *testing.T) {
}, vout.AsAny())
}

func TestNormalizeBoolToAny(t *testing.T) {
func TestNormalizeAnyFromSlice(t *testing.T) {
var typ any
v1 := dyn.NewValue(1, []dyn.Location{{File: "file", Line: 1, Column: 1}})
v2 := dyn.NewValue(2, []dyn.Location{{File: "file", Line: 1, Column: 1}})
vin := dyn.NewValue([]dyn.Value{v1, v2}, []dyn.Location{{File: "file", Line: 1, Column: 1}})
vout, err := Normalize(&typ, vin)
assert.Len(t, err, 0)
assert.Equal(t, dyn.NewValue([]dyn.Value{v1, v2}, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
}

func TestNormalizeAnyFromString(t *testing.T) {
var typ any
vin := dyn.NewValue("string", []dyn.Location{{File: "file", Line: 1, Column: 1}})
vout, err := Normalize(&typ, vin)
assert.Len(t, err, 0)
assert.Equal(t, dyn.NewValue("string", []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
}

func TestNormalizeAnyFromBool(t *testing.T) {
var typ any
vin := dyn.NewValue(false, []dyn.Location{{File: "file", Line: 1, Column: 1}})
vout, err := Normalize(&typ, vin)
assert.Len(t, err, 0)
assert.Equal(t, dyn.NewValue(false, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
}

func TestNormalizeIntToAny(t *testing.T) {
func TestNormalizeAnyFromInt(t *testing.T) {
var typ any
vin := dyn.NewValue(10, []dyn.Location{{File: "file", Line: 1, Column: 1}})
vout, err := Normalize(&typ, vin)
assert.Len(t, err, 0)
assert.Equal(t, dyn.NewValue(10, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
}

func TestNormalizeSliceToAny(t *testing.T) {
func TestNormalizeAnyFromTime(t *testing.T) {
var typ any
v1 := dyn.NewValue(1, []dyn.Location{{File: "file", Line: 1, Column: 1}})
v2 := dyn.NewValue(2, []dyn.Location{{File: "file", Line: 1, Column: 1}})
vin := dyn.NewValue([]dyn.Value{v1, v2}, []dyn.Location{{File: "file", Line: 1, Column: 1}})
vin := dyn.NewValue(dyn.MustTime("2024-08-29"), []dyn.Location{{File: "file", Line: 1, Column: 1}})
vout, err := Normalize(&typ, vin)
assert.Len(t, err, 0)
assert.Equal(t, dyn.NewValue([]dyn.Value{v1, v2}, []dyn.Location{{File: "file", Line: 1, Column: 1}}), vout)
assert.Empty(t, err)
assert.Equal(t, dyn.NewValue("2024-08-29", vin.Locations()), vout)
}

0 comments on commit e4d039a

Please sign in to comment.