diff --git a/bundle/config/mutator/populate_locations.go b/bundle/config/mutator/populate_locations.go index cfdf6dd9c..c414a136b 100644 --- a/bundle/config/mutator/populate_locations.go +++ b/bundle/config/mutator/populate_locations.go @@ -2,11 +2,10 @@ package mutator import ( "context" - "path/filepath" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/dynloc" ) type populateLocations struct{} @@ -21,53 +20,21 @@ func (m *populateLocations) Name() string { return "PopulateLocations" } -func computeRelativeLocations(base string, v dyn.Value) []dyn.Location { - // Skip values that don't have locations. - // Examples include defaults or values that are set by the program itself. - locs := v.Locations() - if len(locs) == 0 { - return nil - } - - // Convert absolute paths to relative paths. - for i := range locs { - rel, err := filepath.Rel(base, locs[i].File) - if err != nil { - return nil - } - // Convert the path separator to forward slashes. - // This makes it possible to compare output across platforms. - locs[i].File = filepath.ToSlash(rel) - } - - return locs -} - func (m *populateLocations) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - loc := make(map[string][]dyn.Location) - _, err := dyn.Walk(b.Config.Value(), func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - // Skip the root value. - if len(p) == 0 { - return v, nil - } + locs, err := dynloc.Build( + b.Config.Value(), - // Skip values that don't have locations. - // Examples include defaults or values that are set by the program itself. - locs := computeRelativeLocations(b.BundleRootPath, v) - if len(locs) > 0 { - // Semantics for a value having multiple locations can be found in [merge.Merge]. - // We don't need to externalize these at the moment, so we limit the number - // of locations to 1 while still using a slice for the output. This allows us - // to include multiple entries in the future if we need to. - loc[p.String()] = locs[0:1] - } + // Make all paths relative to the bundle root. + dynloc.WithBasePath(b.BundleRootPath), - return v, nil - }) + // Limit to maximum depth of 3. + // The intent is to capture locations of all resources but not their configurations. + dynloc.WithMaxDepth(3), + ) if err != nil { return diag.FromErr(err) } - b.Config.Locations = loc + b.Config.Locations = &locs return nil } diff --git a/bundle/config/root.go b/bundle/config/root.go index fcbb4ff21..9904664eb 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -12,6 +12,7 @@ import ( "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/cli/libs/dyn/dynloc" "github.com/databricks/cli/libs/dyn/merge" "github.com/databricks/cli/libs/dyn/yamlloader" "github.com/databricks/cli/libs/log" @@ -72,7 +73,7 @@ type Root struct { // Locations is an output-only field that holds configuration location // information for every path in the configuration tree. - Locations map[string][]dyn.Location `json:"__locations,omitempty" bundle:"internal"` + Locations *dynloc.Locations `json:"__locations,omitempty" bundle:"internal"` } // Load loads the bundle configuration file at the specified path. diff --git a/libs/dyn/dynloc/locations.go b/libs/dyn/dynloc/locations.go new file mode 100644 index 000000000..74efa9ee9 --- /dev/null +++ b/libs/dyn/dynloc/locations.go @@ -0,0 +1,124 @@ +package dynloc + +import ( + "path/filepath" + + "github.com/databricks/cli/libs/dyn" +) + +const ( + // Version is the version of the location information structure. + // Increment if the structure changes. + Version = 1 +) + +// Locations is a structure that holds location information for (a subset of) a [dyn.Value] value. +type Locations struct { + // Version is the version of the location information. + Version int `json:"version"` + + // Files is a list of file paths. + Files []string `json:"files"` + + // Locations maps the string representation of a [dyn.Path] to a list of 3-tuples that represent the index + // of the file in the [Files] array, followed by the line and column number. + // A single [dyn.Path] can have multiple locations (e.g. the effective location and original definition). + Locations map[string][][]int `json:"locations"` + + // fileToIndex maps file paths to their index in the [Files] array. + // This is used to avoid duplicate entries in the [Files] array and keep the + // map with locations as compact as possible. + fileToIndex map[string]int + + // maxDepth is the maximum depth of the [dyn.Path] keys in the [Locations] map. + maxDepth int + + // basePath is the base path used to compute relative paths. + basePath string +} + +func (l *Locations) addLocation(p dyn.Path, file string, line, col int) error { + var err error + + // Compute the relative path. The base path may be empty. + file, err = filepath.Rel(l.basePath, file) + if err != nil { + return err + } + + // Convert the path separator to forward slashes. + // This makes it possible to compare output across platforms. + file = filepath.ToSlash(file) + + // If the file is not yet in the list, add it. + if _, ok := l.fileToIndex[file]; !ok { + l.fileToIndex[file] = len(l.Files) + l.Files = append(l.Files, file) + } + + // Add the location to the map. + l.Locations[p.String()] = append( + l.Locations[p.String()], + []int{l.fileToIndex[file], line, col}, + ) + + return nil +} + +// Option is a functional option for the [Build] function. +type Option func(l *Locations) + +// WithMaxDepth sets the maximum depth of the [dyn.Path] keys in the [Locations] map. +func WithMaxDepth(depth int) Option { + return func(l *Locations) { + l.maxDepth = depth + } +} + +// WithBasePath sets the base path used to compute relative paths. +func WithBasePath(basePath string) Option { + return func(l *Locations) { + l.basePath = basePath + } +} + +// Build constructs a [Locations] object from a [dyn.Value]. +func Build(v dyn.Value, opts ...Option) (Locations, error) { + l := Locations{ + Version: Version, + Files: make([]string, 0), + Locations: make(map[string][][]int), + + // Internal state. + fileToIndex: make(map[string]int), + } + + // Apply options. + for _, opt := range opts { + opt(&l) + } + + // Traverse the value and collect locations. + _, err := dyn.Walk(v, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + // Skip the root value. + if len(p) == 0 { + return v, nil + } + + // Skip if the path depth exceeds the maximum depth. + if l.maxDepth > 0 && len(p) > l.maxDepth { + return v, dyn.ErrSkip + } + + for _, loc := range v.Locations() { + err := l.addLocation(p, loc.File, loc.Line, loc.Column) + if err != nil { + return dyn.InvalidValue, err + } + } + + return v, nil + }) + + return l, err +} diff --git a/libs/dyn/dynloc/locations_test.go b/libs/dyn/dynloc/locations_test.go new file mode 100644 index 000000000..a1858b89e --- /dev/null +++ b/libs/dyn/dynloc/locations_test.go @@ -0,0 +1,100 @@ +package dynloc + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/merge" + "github.com/databricks/cli/libs/dyn/yamlloader" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func load(t *testing.T, path string) dyn.Value { + matches, err := filepath.Glob(path + "/*.yml") + require.NoError(t, err) + require.NotEmpty(t, matches) + + // Load all files. + vout := dyn.NilValue + for _, match := range matches { + buf, err := os.ReadFile(match) + require.NoError(t, err) + + v, err := yamlloader.LoadYAML(match, bytes.NewBuffer(buf)) + require.NoError(t, err) + + vout, err = merge.Merge(vout, v) + require.NoError(t, err) + } + + return vout +} + +func TestLocations_Default(t *testing.T) { + v := load(t, "testdata/default") + locs, err := Build(v) + require.NoError(t, err) + assert.Equal(t, 1, locs.Version) + assert.Equal(t, []string{"testdata/default/a.yml", "testdata/default/b.yml"}, locs.Files) + assert.Equal(t, map[string][][]int{ + "a": {{0, 2, 3}}, + "a.b": {{0, 2, 6}}, + "b": {{1, 2, 3}}, + "b.c": {{1, 2, 6}}, + }, locs.Locations) +} + +func TestLocations_DefaultWithBasePath(t *testing.T) { + v := load(t, "testdata/default") + locs, err := Build(v, WithBasePath("testdata/default")) + require.NoError(t, err) + assert.Equal(t, 1, locs.Version) + assert.Equal(t, []string{"a.yml", "b.yml"}, locs.Files) + assert.Equal(t, map[string][][]int{ + "a": {{0, 2, 3}}, + "a.b": {{0, 2, 6}}, + "b": {{1, 2, 3}}, + "b.c": {{1, 2, 6}}, + }, locs.Locations) +} + +func TestLocations_Override(t *testing.T) { + v := load(t, "testdata/override") + locs, err := Build(v) + require.NoError(t, err) + assert.Equal(t, 1, locs.Version) + assert.Equal(t, []string{"testdata/override/a.yml", "testdata/override/b.yml"}, locs.Files) + + // Note: specific ordering of locations is described in [merge.Merge]. + assert.Equal(t, map[string][][]int{ + "a": { + {0, 2, 3}, + {1, 2, 3}, + }, + "a.b": { + {1, 2, 6}, + {0, 2, 6}, + }, + }, locs.Locations) +} + +func TestLocations_MaxDepth(t *testing.T) { + v := load(t, "testdata/depth") + + var locs Locations + var err error + + // Test with no max depth. + locs, err = Build(v) + require.NoError(t, err) + assert.Len(t, locs.Locations, 5) + + // Test with max depth and see that the number of locations is reduced. + locs, err = Build(v, WithMaxDepth(3)) + require.NoError(t, err) + assert.Len(t, locs.Locations, 3) +} diff --git a/libs/dyn/dynloc/testdata/default/a.yml b/libs/dyn/dynloc/testdata/default/a.yml new file mode 100644 index 000000000..e9a726411 --- /dev/null +++ b/libs/dyn/dynloc/testdata/default/a.yml @@ -0,0 +1,2 @@ +a: + b: 42 diff --git a/libs/dyn/dynloc/testdata/default/b.yml b/libs/dyn/dynloc/testdata/default/b.yml new file mode 100644 index 000000000..667086eba --- /dev/null +++ b/libs/dyn/dynloc/testdata/default/b.yml @@ -0,0 +1,2 @@ +b: + c: 43 diff --git a/libs/dyn/dynloc/testdata/depth/a.yml b/libs/dyn/dynloc/testdata/depth/a.yml new file mode 100644 index 000000000..5591d6b57 --- /dev/null +++ b/libs/dyn/dynloc/testdata/depth/a.yml @@ -0,0 +1,5 @@ +a: + b: + c: + d: + e: 42 diff --git a/libs/dyn/dynloc/testdata/override/a.yml b/libs/dyn/dynloc/testdata/override/a.yml new file mode 100644 index 000000000..e9a726411 --- /dev/null +++ b/libs/dyn/dynloc/testdata/override/a.yml @@ -0,0 +1,2 @@ +a: + b: 42 diff --git a/libs/dyn/dynloc/testdata/override/b.yml b/libs/dyn/dynloc/testdata/override/b.yml new file mode 100644 index 000000000..da53fd2da --- /dev/null +++ b/libs/dyn/dynloc/testdata/override/b.yml @@ -0,0 +1,2 @@ +a: + b: 43