diff --git a/bundle/tests/override_sync_test.go b/bundle/tests/override_sync_test.go deleted file mode 100644 index 64f28e377e..0000000000 --- a/bundle/tests/override_sync_test.go +++ /dev/null @@ -1,41 +0,0 @@ -package config_tests - -import ( - "path/filepath" - "testing" - - "github.com/databricks/cli/bundle" - "github.com/stretchr/testify/assert" -) - -func TestOverrideSyncTarget(t *testing.T) { - var b *bundle.Bundle - - b = loadTarget(t, "./override_sync", "development") - assert.ElementsMatch(t, []string{filepath.FromSlash("src/*"), filepath.FromSlash("tests/*")}, b.Config.Sync.Include) - assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude) - - b = loadTarget(t, "./override_sync", "staging") - assert.ElementsMatch(t, []string{filepath.FromSlash("src/*"), filepath.FromSlash("fixtures/*")}, b.Config.Sync.Include) - assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude) - - b = loadTarget(t, "./override_sync", "prod") - assert.ElementsMatch(t, []string{filepath.FromSlash("src/*")}, b.Config.Sync.Include) - assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude) -} - -func TestOverrideSyncTargetNoRootSync(t *testing.T) { - var b *bundle.Bundle - - b = loadTarget(t, "./override_sync_no_root", "development") - assert.ElementsMatch(t, []string{filepath.FromSlash("tests/*")}, b.Config.Sync.Include) - assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude) - - b = loadTarget(t, "./override_sync_no_root", "staging") - assert.ElementsMatch(t, []string{filepath.FromSlash("fixtures/*")}, b.Config.Sync.Include) - assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude) - - b = loadTarget(t, "./override_sync_no_root", "prod") - assert.ElementsMatch(t, []string{}, b.Config.Sync.Include) - assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude) -} diff --git a/bundle/tests/sync/nil/databricks.yml b/bundle/tests/sync/nil/databricks.yml new file mode 100644 index 0000000000..a8b4b901ef --- /dev/null +++ b/bundle/tests/sync/nil/databricks.yml @@ -0,0 +1,19 @@ +bundle: + name: sync_nil + +workspace: + host: https://acme.cloud.databricks.com/ + +sync: + include: ~ + exclude: ~ + +targets: + development: + + staging: + sync: + include: + - tests/* + exclude: + - dist diff --git a/bundle/tests/sync/nil_root/databricks.yml b/bundle/tests/sync/nil_root/databricks.yml new file mode 100644 index 0000000000..44e6c48ea7 --- /dev/null +++ b/bundle/tests/sync/nil_root/databricks.yml @@ -0,0 +1,17 @@ +bundle: + name: sync_nil_root + +workspace: + host: https://acme.cloud.databricks.com/ + +sync: ~ + +targets: + development: + + staging: + sync: + include: + - tests/* + exclude: + - dist diff --git a/bundle/tests/override_sync/databricks.yml b/bundle/tests/sync/override/databricks.yml similarity index 93% rename from bundle/tests/override_sync/databricks.yml rename to bundle/tests/sync/override/databricks.yml index 1417b86441..8bb0e1def4 100644 --- a/bundle/tests/override_sync/databricks.yml +++ b/bundle/tests/sync/override/databricks.yml @@ -1,5 +1,5 @@ bundle: - name: override_sync + name: sync_override workspace: host: https://acme.cloud.databricks.com/ diff --git a/bundle/tests/override_sync_no_root/databricks.yml b/bundle/tests/sync/override_no_root/databricks.yml similarity index 90% rename from bundle/tests/override_sync_no_root/databricks.yml rename to bundle/tests/sync/override_no_root/databricks.yml index 109d8da1fd..bd1bfe8e07 100644 --- a/bundle/tests/override_sync_no_root/databricks.yml +++ b/bundle/tests/sync/override_no_root/databricks.yml @@ -1,5 +1,5 @@ bundle: - name: override_sync + name: sync_override_no_root workspace: host: https://acme.cloud.databricks.com/ diff --git a/bundle/tests/sync_include_exclude_no_matches_test.go b/bundle/tests/sync_include_exclude_no_matches_test.go index 135e2faaca..94cedbaa62 100644 --- a/bundle/tests/sync_include_exclude_no_matches_test.go +++ b/bundle/tests/sync_include_exclude_no_matches_test.go @@ -13,7 +13,7 @@ import ( ) func TestSyncIncludeExcludeNoMatchesTest(t *testing.T) { - b := loadTarget(t, "./override_sync", "development") + b := loadTarget(t, "./sync/override", "development") diags := bundle.ApplyReadOnly(context.Background(), bundle.ReadOnly(b), validate.ValidateSyncPatterns()) require.Len(t, diags, 3) @@ -21,7 +21,7 @@ func TestSyncIncludeExcludeNoMatchesTest(t *testing.T) { require.Equal(t, diags[0].Severity, diag.Warning) require.Equal(t, diags[0].Summary, "Pattern dist does not match any files") - require.Equal(t, diags[0].Location.File, filepath.Join("override_sync", "databricks.yml")) + require.Equal(t, diags[0].Location.File, filepath.Join("sync", "override", "databricks.yml")) require.Equal(t, diags[0].Location.Line, 17) require.Equal(t, diags[0].Location.Column, 11) require.Equal(t, diags[0].Path.String(), "sync.exclude[0]") diff --git a/bundle/tests/sync_test.go b/bundle/tests/sync_test.go new file mode 100644 index 0000000000..d08e889c34 --- /dev/null +++ b/bundle/tests/sync_test.go @@ -0,0 +1,65 @@ +package config_tests + +import ( + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/stretchr/testify/assert" +) + +func TestSyncOverride(t *testing.T) { + var b *bundle.Bundle + + b = loadTarget(t, "./sync/override", "development") + assert.ElementsMatch(t, []string{filepath.FromSlash("src/*"), filepath.FromSlash("tests/*")}, b.Config.Sync.Include) + assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude) + + b = loadTarget(t, "./sync/override", "staging") + assert.ElementsMatch(t, []string{filepath.FromSlash("src/*"), filepath.FromSlash("fixtures/*")}, b.Config.Sync.Include) + assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude) + + b = loadTarget(t, "./sync/override", "prod") + assert.ElementsMatch(t, []string{filepath.FromSlash("src/*")}, b.Config.Sync.Include) + assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude) +} + +func TestSyncOverrideNoRootSync(t *testing.T) { + var b *bundle.Bundle + + b = loadTarget(t, "./sync/override_no_root", "development") + assert.ElementsMatch(t, []string{filepath.FromSlash("tests/*")}, b.Config.Sync.Include) + assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude) + + b = loadTarget(t, "./sync/override_no_root", "staging") + assert.ElementsMatch(t, []string{filepath.FromSlash("fixtures/*")}, b.Config.Sync.Include) + assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude) + + b = loadTarget(t, "./sync/override_no_root", "prod") + assert.ElementsMatch(t, []string{}, b.Config.Sync.Include) + assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude) +} + +func TestSyncNil(t *testing.T) { + var b *bundle.Bundle + + b = loadTarget(t, "./sync/nil", "development") + assert.Nil(t, b.Config.Sync.Include) + assert.Nil(t, b.Config.Sync.Exclude) + + b = loadTarget(t, "./sync/nil", "staging") + assert.ElementsMatch(t, []string{filepath.FromSlash("tests/*")}, b.Config.Sync.Include) + assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude) +} + +func TestSyncNilRoot(t *testing.T) { + var b *bundle.Bundle + + b = loadTarget(t, "./sync/nil_root", "development") + assert.Nil(t, b.Config.Sync.Include) + assert.Nil(t, b.Config.Sync.Exclude) + + b = loadTarget(t, "./sync/nil_root", "staging") + assert.ElementsMatch(t, []string{filepath.FromSlash("tests/*")}, b.Config.Sync.Include) + assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude) +} diff --git a/libs/dyn/visit.go b/libs/dyn/visit.go index 3fe3561943..4d3cf50142 100644 --- a/libs/dyn/visit.go +++ b/libs/dyn/visit.go @@ -6,6 +6,28 @@ import ( "slices" ) +// This error is returned if the path indicates that a map or sequence is expected, but the value is nil. +type cannotTraverseNilError struct { + p Path +} + +func (e cannotTraverseNilError) Error() string { + component := e.p[len(e.p)-1] + switch { + case component.isKey(): + return fmt.Sprintf("expected a map to index %q, found nil", e.p) + case component.isIndex(): + return fmt.Sprintf("expected a sequence to index %q, found nil", e.p) + default: + panic("invalid component") + } +} + +func IsCannotTraverseNilError(err error) bool { + var target cannotTraverseNilError + return errors.As(err, &target) +} + type noSuchKeyError struct { p Path } @@ -70,11 +92,17 @@ func (component pathComponent) visit(v Value, prefix Path, suffix Pattern, opts switch { case component.isKey(): // Expect a map to be set if this is a key. - m, ok := v.AsMap() - if !ok { + switch v.Kind() { + case KindMap: + // OK + case KindNil: + return InvalidValue, cannotTraverseNilError{path} + default: return InvalidValue, fmt.Errorf("expected a map to index %q, found %s", path, v.Kind()) } + m := v.MustMap() + // Lookup current value in the map. ev, ok := m.GetByString(component.key) if !ok { @@ -103,11 +131,17 @@ func (component pathComponent) visit(v Value, prefix Path, suffix Pattern, opts case component.isIndex(): // Expect a sequence to be set if this is an index. - s, ok := v.AsSequence() - if !ok { + switch v.Kind() { + case KindSequence: + // OK + case KindNil: + return InvalidValue, cannotTraverseNilError{path} + default: return InvalidValue, fmt.Errorf("expected a sequence to index %q, found %s", path, v.Kind()) } + s := v.MustSequence() + // Lookup current value in the sequence. if component.index < 0 || component.index >= len(s) { return InvalidValue, indexOutOfBoundsError{path} diff --git a/libs/dyn/visit_map.go b/libs/dyn/visit_map.go index f5cfea3114..56a9cf9f3c 100644 --- a/libs/dyn/visit_map.go +++ b/libs/dyn/visit_map.go @@ -10,9 +10,12 @@ type MapFunc func(Path, Value) (Value, error) // Foreach returns a [MapFunc] that applies the specified [MapFunc] to each // value in a map or sequence and returns the new map or sequence. +// If the input is nil, it returns nil. func Foreach(fn MapFunc) MapFunc { return func(p Path, v Value) (Value, error) { switch v.Kind() { + case KindNil: + return v, nil case KindMap: m := v.MustMap().Clone() for _, pair := range m.Pairs() { @@ -75,8 +78,10 @@ func MapByPattern(v Value, p Pattern, fn MapFunc) (Value, error) { return nv, nil } - // Return original value if a key or index is missing. - if IsNoSuchKeyError(err) || IsIndexOutOfBoundsError(err) { + // Return original value if: + // - any map or sequence is a nil, or + // - a key or index is missing + if IsCannotTraverseNilError(err) || IsNoSuchKeyError(err) || IsIndexOutOfBoundsError(err) { return v, nil } diff --git a/libs/dyn/visit_map_test.go b/libs/dyn/visit_map_test.go index df6bad4965..2cea0913be 100644 --- a/libs/dyn/visit_map_test.go +++ b/libs/dyn/visit_map_test.go @@ -20,11 +20,14 @@ func TestMapWithEmptyPath(t *testing.T) { } func TestMapOnNilValue(t *testing.T) { + var nv dyn.Value var err error - _, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Key("foo")), nil) - assert.ErrorContains(t, err, `expected a map to index "foo", found nil`) - _, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Index(42)), nil) - assert.ErrorContains(t, err, `expected a sequence to index "[42]", found nil`) + nv, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Key("foo")), nil) + assert.NoError(t, err) + assert.Equal(t, dyn.NilValue, nv) + nv, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Index(42)), nil) + assert.NoError(t, err) + assert.Equal(t, dyn.NilValue, nv) } func TestMapFuncOnMap(t *testing.T) { @@ -269,6 +272,17 @@ func TestMapForeachOnOtherError(t *testing.T) { assert.ErrorContains(t, err, "expected a map or sequence, found int") } +func TestMapForeachOnNil(t *testing.T) { + vin := dyn.NilValue + + // Check that if foreach is applied to nil, it returns nil. + vout, err := dyn.Map(vin, ".", dyn.Foreach(func(_ dyn.Path, v dyn.Value) (dyn.Value, error) { + return dyn.InvalidValue, nil + })) + assert.NoError(t, err) + assert.Equal(t, dyn.NilValue, vout) +} + func TestMapByPatternOnNilValue(t *testing.T) { var err error _, err = dyn.MapByPattern(dyn.NilValue, dyn.NewPattern(dyn.AnyKey()), nil)