Skip to content

Commit

Permalink
Include dyn.Path as argument to the visit callback function (#1260)
Browse files Browse the repository at this point in the history
## Changes

This change means the callback supplied to `dyn.Foreach` can introspect
the path of the value it is being called for. It also prepares for
allowing visiting path patterns where the exact path is not known
upfront.

## Tests

Unit tests.
  • Loading branch information
pietern authored Mar 7, 2024
1 parent a467d01 commit c05c0cd
Show file tree
Hide file tree
Showing 12 changed files with 52 additions and 34 deletions.
2 changes: 1 addition & 1 deletion bundle/config/mutator/merge_job_clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (m *mergeJobClusters) Apply(ctx context.Context, b *bundle.Bundle) error {
return v, nil
}

return dyn.Map(v, "resources.jobs", dyn.Foreach(func(job dyn.Value) (dyn.Value, error) {
return dyn.Map(v, "resources.jobs", dyn.Foreach(func(_ dyn.Path, job dyn.Value) (dyn.Value, error) {
return dyn.Map(job, "job_clusters", merge.ElementsByKey("job_cluster_key", m.jobClusterKey))
}))
})
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/merge_job_tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (m *mergeJobTasks) Apply(ctx context.Context, b *bundle.Bundle) error {
return v, nil
}

return dyn.Map(v, "resources.jobs", dyn.Foreach(func(job dyn.Value) (dyn.Value, error) {
return dyn.Map(v, "resources.jobs", dyn.Foreach(func(_ dyn.Path, job dyn.Value) (dyn.Value, error) {
return dyn.Map(job, "tasks", merge.ElementsByKey("task_key", m.taskKeyString))
}))
})
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/merge_pipeline_clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (m *mergePipelineClusters) Apply(ctx context.Context, b *bundle.Bundle) err
return v, nil
}

return dyn.Map(v, "resources.pipelines", dyn.Foreach(func(pipeline dyn.Value) (dyn.Value, error) {
return dyn.Map(v, "resources.pipelines", dyn.Foreach(func(_ dyn.Path, pipeline dyn.Value) (dyn.Value, error) {
return dyn.Map(pipeline, "clusters", merge.ElementsByKey("label", m.clusterLabel))
}))
})
Expand Down
4 changes: 2 additions & 2 deletions bundle/config/mutator/rewrite_sync_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (m *rewriteSyncPaths) Name() string {
//
// Then the resulting value will be "bar/somefile.*".
func (m *rewriteSyncPaths) makeRelativeTo(root string) dyn.MapFunc {
return func(v dyn.Value) (dyn.Value, error) {
return func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
dir := filepath.Dir(v.Location().File)
rel, err := filepath.Rel(root, dir)
if err != nil {
Expand All @@ -43,7 +43,7 @@ func (m *rewriteSyncPaths) makeRelativeTo(root string) dyn.MapFunc {

func (m *rewriteSyncPaths) Apply(ctx context.Context, b *bundle.Bundle) error {
return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
return dyn.Map(v, "sync", func(v dyn.Value) (nv dyn.Value, err error) {
return dyn.Map(v, "sync", func(_ dyn.Path, v dyn.Value) (nv dyn.Value, err error) {
v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.Config.Path)))
if err != nil {
return dyn.NilValue, err
Expand Down
4 changes: 2 additions & 2 deletions bundle/config/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,14 +409,14 @@ func rewriteShorthands(v dyn.Value) (dyn.Value, error) {
}

// For each target, rewrite the variables block.
return dyn.Map(v, "targets", dyn.Foreach(func(target dyn.Value) (dyn.Value, error) {
return dyn.Map(v, "targets", dyn.Foreach(func(_ dyn.Path, target dyn.Value) (dyn.Value, error) {
// Confirm it has a variables block.
if target.Get("variables") == dyn.NilValue {
return target, nil
}

// For each variable, normalize its contents if it is a single string.
return dyn.Map(target, "variables", dyn.Foreach(func(variable dyn.Value) (dyn.Value, error) {
return dyn.Map(target, "variables", dyn.Foreach(func(_ dyn.Path, variable dyn.Value) (dyn.Value, error) {
if variable.Kind() != dyn.KindString {
return variable, nil
}
Expand Down
4 changes: 2 additions & 2 deletions bundle/deploy/terraform/tfdyn/convert_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func convertJobResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) {
}

// Modify keys in the "git_source" block
vout, err = dyn.Map(vout, "git_source", func(v dyn.Value) (dyn.Value, error) {
vout, err = dyn.Map(vout, "git_source", func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return renameKeys(v, map[string]string{
"git_branch": "branch",
"git_commit": "commit",
Expand All @@ -44,7 +44,7 @@ func convertJobResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) {
}

// Modify keys in the "task" blocks
vout, err = dyn.Map(vout, "task", dyn.Foreach(func(v dyn.Value) (dyn.Value, error) {
vout, err = dyn.Map(vout, "task", dyn.Foreach(func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return renameKeys(v, map[string]string{
"libraries": "library",
})
Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/merge/elements_by_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type elementsByKey struct {
keyFunc func(dyn.Value) string
}

func (e elementsByKey) Map(v dyn.Value) (dyn.Value, error) {
func (e elementsByKey) Map(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
// We know the type of this value is a sequence.
// For additional defence, return self if it is not.
elements, ok := v.AsSequence()
Expand Down
4 changes: 2 additions & 2 deletions libs/dyn/visit.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type visitOptions struct {
//
// If this function returns an error, the original visit function call
// returns this error and the value is left unmodified.
fn func(Value) (Value, error)
fn func(Path, Value) (Value, error)

// If set, tolerate the absence of the last component in the path.
// This option is needed to set a key in a map that is not yet present.
Expand All @@ -53,7 +53,7 @@ type visitOptions struct {

func visit(v Value, prefix, suffix Path, opts visitOptions) (Value, error) {
if len(suffix) == 0 {
return opts.fn(v)
return opts.fn(prefix, v)
}

// Initialize prefix if it is empty.
Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/visit_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func Get(v Value, path string) (Value, error) {
func GetByPath(v Value, p Path) (Value, error) {
out := InvalidValue
_, err := visit(v, EmptyPath, p, visitOptions{
fn: func(ev Value) (Value, error) {
fn: func(_ Path, ev Value) (Value, error) {
// Capture the value argument to return it.
out = ev
return ev, nil
Expand Down
8 changes: 4 additions & 4 deletions libs/dyn/visit_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@ import (
)

// MapFunc is a function that maps a value to another value.
type MapFunc func(Value) (Value, error)
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.
func Foreach(fn MapFunc) MapFunc {
return func(v Value) (Value, error) {
return func(p Path, v Value) (Value, error) {
switch v.Kind() {
case KindMap:
m := maps.Clone(v.MustMap())
for key, value := range m {
var err error
m[key], err = fn(value)
m[key], err = fn(p.Append(Key(key)), value)
if err != nil {
return InvalidValue, err
}
Expand All @@ -28,7 +28,7 @@ func Foreach(fn MapFunc) MapFunc {
s := slices.Clone(v.MustSequence())
for i, value := range s {
var err error
s[i], err = fn(value)
s[i], err = fn(p.Append(Index(i)), value)
if err != nil {
return InvalidValue, err
}
Expand Down
50 changes: 34 additions & 16 deletions libs/dyn/visit_map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
func TestMapWithEmptyPath(t *testing.T) {
// An empty path means to return the value itself.
vin := dyn.V(42)
vout, err := dyn.MapByPath(dyn.InvalidValue, dyn.EmptyPath, func(v dyn.Value) (dyn.Value, error) {
vout, err := dyn.MapByPath(dyn.InvalidValue, dyn.EmptyPath, func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return vin, nil
})
assert.NoError(t, err)
Expand Down Expand Up @@ -45,7 +45,7 @@ func TestMapFuncOnMap(t *testing.T) {

// Note: in the test cases below we implicitly test that the original
// value is not modified as we repeatedly set values on it.
vfoo, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Key("foo")), func(v dyn.Value) (dyn.Value, error) {
vfoo, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Key("foo")), func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
assert.Equal(t, dyn.V(42), v)
return dyn.V(44), nil
})
Expand All @@ -55,7 +55,7 @@ func TestMapFuncOnMap(t *testing.T) {
"bar": 43,
}, vfoo.AsAny())

vbar, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Key("bar")), func(v dyn.Value) (dyn.Value, error) {
vbar, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Key("bar")), func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
assert.Equal(t, dyn.V(43), v)
return dyn.V(45), nil
})
Expand All @@ -67,7 +67,7 @@ func TestMapFuncOnMap(t *testing.T) {

// Return error from map function.
var ref = fmt.Errorf("error")
verr, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Key("foo")), func(v dyn.Value) (dyn.Value, error) {
verr, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Key("foo")), func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return dyn.InvalidValue, ref
})
assert.Equal(t, dyn.InvalidValue, verr)
Expand All @@ -88,7 +88,7 @@ func TestMapFuncOnMapWithEmptySequence(t *testing.T) {
})

for j := 0; j < len(variants); j++ {
vout, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Key("key")), func(v dyn.Value) (dyn.Value, error) {
vout, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Key("key")), func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return variants[j], nil
})
assert.NoError(t, err)
Expand All @@ -115,14 +115,14 @@ func TestMapFuncOnSequence(t *testing.T) {

// Note: in the test cases below we implicitly test that the original
// value is not modified as we repeatedly set values on it.
v0, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Index(0)), func(v dyn.Value) (dyn.Value, error) {
v0, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Index(0)), func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
assert.Equal(t, dyn.V(42), v)
return dyn.V(44), nil
})
assert.NoError(t, err)
assert.Equal(t, []any{44, 43}, v0.AsAny())

v1, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Index(1)), func(v dyn.Value) (dyn.Value, error) {
v1, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Index(1)), func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
assert.Equal(t, dyn.V(43), v)
return dyn.V(45), nil
})
Expand All @@ -131,7 +131,7 @@ func TestMapFuncOnSequence(t *testing.T) {

// Return error from map function.
var ref = fmt.Errorf("error")
verr, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Index(0)), func(v dyn.Value) (dyn.Value, error) {
verr, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Index(0)), func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return dyn.InvalidValue, ref
})
assert.Equal(t, dyn.InvalidValue, verr)
Expand All @@ -152,7 +152,7 @@ func TestMapFuncOnSequenceWithEmptySequence(t *testing.T) {
})

for j := 0; j < len(variants); j++ {
vout, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Index(0)), func(v dyn.Value) (dyn.Value, error) {
vout, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Index(0)), func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return variants[j], nil
})
assert.NoError(t, err)
Expand All @@ -170,10 +170,19 @@ func TestMapForeachOnMap(t *testing.T) {
var err error

// Run foreach, adding 1 to each of the elements.
vout, err := dyn.Map(vin, ".", dyn.Foreach(func(v dyn.Value) (dyn.Value, error) {
vout, err := dyn.Map(vin, ".", dyn.Foreach(func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
i, ok := v.AsInt()
require.True(t, ok, "expected an integer")
return dyn.V(int(i) + 1), nil
switch p[0].Key() {
case "foo":
assert.EqualValues(t, 42, i)
return dyn.V(43), nil
case "bar":
assert.EqualValues(t, 43, i)
return dyn.V(44), nil
default:
return dyn.InvalidValue, fmt.Errorf("unexpected key %q", p[0].Key())
}
}))
assert.NoError(t, err)
assert.Equal(t, map[string]any{
Expand All @@ -196,7 +205,7 @@ func TestMapForeachOnMapError(t *testing.T) {

// Check that an error from the map function propagates.
var ref = fmt.Errorf("error")
_, err := dyn.Map(vin, ".", dyn.Foreach(func(v dyn.Value) (dyn.Value, error) {
_, err := dyn.Map(vin, ".", dyn.Foreach(func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return dyn.InvalidValue, ref
}))
assert.ErrorIs(t, err, ref)
Expand All @@ -211,10 +220,19 @@ func TestMapForeachOnSequence(t *testing.T) {
var err error

// Run foreach, adding 1 to each of the elements.
vout, err := dyn.Map(vin, ".", dyn.Foreach(func(v dyn.Value) (dyn.Value, error) {
vout, err := dyn.Map(vin, ".", dyn.Foreach(func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
i, ok := v.AsInt()
require.True(t, ok, "expected an integer")
return dyn.V(int(i) + 1), nil
switch p[0].Index() {
case 0:
assert.EqualValues(t, 42, i)
return dyn.V(43), nil
case 1:
assert.EqualValues(t, 43, i)
return dyn.V(44), nil
default:
return dyn.InvalidValue, fmt.Errorf("unexpected index %d", p[0].Index())
}
}))
assert.NoError(t, err)
assert.Equal(t, []any{43, 44}, vout.AsAny())
Expand All @@ -231,7 +249,7 @@ func TestMapForeachOnSequenceError(t *testing.T) {

// Check that an error from the map function propagates.
var ref = fmt.Errorf("error")
_, err := dyn.Map(vin, ".", dyn.Foreach(func(v dyn.Value) (dyn.Value, error) {
_, err := dyn.Map(vin, ".", dyn.Foreach(func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return dyn.InvalidValue, ref
}))
assert.ErrorIs(t, err, ref)
Expand All @@ -241,7 +259,7 @@ func TestMapForeachOnOtherError(t *testing.T) {
vin := dyn.V(42)

// Check that if foreach is applied to something other than a map or a sequence, it returns an error.
_, err := dyn.Map(vin, ".", dyn.Foreach(func(v dyn.Value) (dyn.Value, error) {
_, err := dyn.Map(vin, ".", dyn.Foreach(func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return dyn.InvalidValue, nil
}))
assert.ErrorContains(t, err, "expected a map or sequence, found int")
Expand Down
2 changes: 1 addition & 1 deletion libs/dyn/visit_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func Set(v Value, path string, nv Value) (Value, error) {
// If the path doesn't exist, it returns InvalidValue and an error.
func SetByPath(v Value, p Path, nv Value) (Value, error) {
return visit(v, EmptyPath, p, visitOptions{
fn: func(_ Value) (Value, error) {
fn: func(_ Path, _ Value) (Value, error) {
// Return the incoming value to set it.
return nv, nil
},
Expand Down

0 comments on commit c05c0cd

Please sign in to comment.