Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dyn.InvalidValue to indicate absence #1507

Merged
merged 5 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions bundle/config/mutator/environments_compat.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@ func (m *environmentsToTargets) Apply(ctx context.Context, b *bundle.Bundle) dia
targets := v.Get("targets")

// Return an error if both "environments" and "targets" are set.
if environments != dyn.NilValue && targets != dyn.NilValue {
return dyn.NilValue, fmt.Errorf(
if environments != dyn.InvalidValue && targets != dyn.InvalidValue {
return dyn.InvalidValue, fmt.Errorf(
"both 'environments' and 'targets' are specified; only 'targets' should be used: %s",
environments.Location().String(),
)
}

// Rewrite "environments" to "targets".
if environments != dyn.NilValue && targets == dyn.NilValue {
if environments != dyn.InvalidValue && targets == dyn.InvalidValue {
nv, err := dyn.Set(v, "targets", environments)
if err != nil {
return dyn.NilValue, err
return dyn.InvalidValue, err
}
// Drop the "environments" key.
return dyn.Walk(nv, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
Expand Down
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 @@ -21,7 +21,7 @@ func (m *mergeJobClusters) Name() string {

func (m *mergeJobClusters) jobClusterKey(v dyn.Value) string {
switch v.Kind() {
case dyn.KindNil:
case dyn.KindInvalid, dyn.KindNil:
return ""
case dyn.KindString:
return v.MustString()
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 @@ -21,7 +21,7 @@ func (m *mergeJobTasks) Name() string {

func (m *mergeJobTasks) taskKeyString(v dyn.Value) string {
switch v.Kind() {
case dyn.KindNil:
case dyn.KindInvalid, dyn.KindNil:
return ""
case dyn.KindString:
return v.MustString()
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 @@ -22,7 +22,7 @@ func (m *mergePipelineClusters) Name() string {

func (m *mergePipelineClusters) clusterLabel(v dyn.Value) string {
switch v.Kind() {
case dyn.KindNil:
case dyn.KindInvalid, dyn.KindNil:
// Note: the cluster label is optional and defaults to 'default'.
// We therefore ALSO merge all clusters without a label.
return "default"
Expand Down
30 changes: 17 additions & 13 deletions bundle/config/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,39 +337,39 @@ func (r *Root) MergeTargetOverrides(name string) error {
}

// Merge `run_as`. This field must be overwritten if set, not merged.
if v := target.Get("run_as"); v != dyn.NilValue {
if v := target.Get("run_as"); v != dyn.InvalidValue {
root, err = dyn.Set(root, "run_as", v)
if err != nil {
return err
}
}

// Below, we're setting fields on the bundle key, so make sure it exists.
if root.Get("bundle") == dyn.NilValue {
if root.Get("bundle") == dyn.InvalidValue {
root, err = dyn.Set(root, "bundle", dyn.NewValue(map[string]dyn.Value{}, dyn.Location{}))
if err != nil {
return err
}
}

// Merge `mode`. This field must be overwritten if set, not merged.
if v := target.Get("mode"); v != dyn.NilValue {
if v := target.Get("mode"); v != dyn.InvalidValue {
root, err = dyn.SetByPath(root, dyn.NewPath(dyn.Key("bundle"), dyn.Key("mode")), v)
if err != nil {
return err
}
}

// Merge `compute_id`. This field must be overwritten if set, not merged.
if v := target.Get("compute_id"); v != dyn.NilValue {
if v := target.Get("compute_id"); v != dyn.InvalidValue {
root, err = dyn.SetByPath(root, dyn.NewPath(dyn.Key("bundle"), dyn.Key("compute_id")), v)
if err != nil {
return err
}
}

// Merge `git`.
if v := target.Get("git"); v != dyn.NilValue {
if v := target.Get("git"); v != dyn.InvalidValue {
ref, err := dyn.GetByPath(root, dyn.NewPath(dyn.Key("bundle"), dyn.Key("git")))
if err != nil {
ref = dyn.NewValue(map[string]dyn.Value{}, dyn.Location{})
Expand All @@ -382,7 +382,7 @@ func (r *Root) MergeTargetOverrides(name string) error {
}

// If the branch was overridden, we need to clear the inferred flag.
if branch := v.Get("branch"); branch != dyn.NilValue {
if branch := v.Get("branch"); branch != dyn.InvalidValue {
out, err = dyn.SetByPath(out, dyn.NewPath(dyn.Key("inferred")), dyn.NewValue(false, dyn.Location{}))
if err != nil {
return err
Expand Down Expand Up @@ -410,7 +410,7 @@ func rewriteShorthands(v dyn.Value) (dyn.Value, error) {
// For each target, rewrite the variables block.
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 {
if target.Get("variables") == dyn.InvalidValue {
return target, nil
}

Expand Down Expand Up @@ -440,15 +440,19 @@ func validateVariableOverrides(root, target dyn.Value) (err error) {
var tv map[string]variable.Variable

// Collect variables from the root.
err = convert.ToTyped(&rv, root.Get("variables"))
if err != nil {
return fmt.Errorf("unable to collect variables from root: %w", err)
if v := root.Get("variables"); v != dyn.InvalidValue {
err = convert.ToTyped(&rv, v)
if err != nil {
return fmt.Errorf("unable to collect variables from root: %w", err)
}
}

// Collect variables from the target.
err = convert.ToTyped(&tv, target.Get("variables"))
if err != nil {
return fmt.Errorf("unable to collect variables from target: %w", err)
if v := target.Get("variables"); v != dyn.InvalidValue {
err = convert.ToTyped(&tv, v)
if err != nil {
return fmt.Errorf("unable to collect variables from target: %w", err)
}
}

// Check that all variables in the target exist in the root.
Expand Down
8 changes: 7 additions & 1 deletion libs/dyn/convert/from_typed.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,15 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
out := make([]dyn.Value, src.Len())
for i := 0; i < src.Len(); i++ {
v := src.Index(i)
refv := ref.Index(i)

// Use nil reference if there is no reference for this index.
if refv == dyn.InvalidValue {
refv = dyn.NilValue
}

// Convert entry taking into account the reference value (may be equal to dyn.NilValue).
nv, err := fromTyped(v.Interface(), ref.Index(i), includeZeroValuedScalars)
nv, err := fromTyped(v.Interface(), refv, includeZeroValuedScalars)
if err != nil {
return dyn.InvalidValue, err
}
Expand Down
8 changes: 4 additions & 4 deletions libs/dyn/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ func (v Value) AsAny() any {
func (v Value) Get(key string) Value {
m, ok := v.AsMap()
if !ok {
return NilValue
return InvalidValue
}

vv, ok := m.GetByString(key)
if !ok {
return NilValue
return InvalidValue
}

return vv
Expand All @@ -124,11 +124,11 @@ func (v Value) Get(key string) Value {
func (v Value) Index(i int) Value {
s, ok := v.v.([]Value)
if !ok {
return NilValue
return InvalidValue
}

if i < 0 || i >= len(s) {
return NilValue
return InvalidValue
}

return s[i]
Expand Down
42 changes: 21 additions & 21 deletions libs/dyn/value_underlying_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ func TestValueUnderlyingMap(t *testing.T) {
vv1, ok := v.AsMap()
assert.True(t, ok)

_, ok = dyn.NilValue.AsMap()
_, ok = dyn.InvalidValue.AsMap()
assert.False(t, ok)

vv2 := v.MustMap()
assert.Equal(t, vv1, vv2)

// Test panic.
assert.PanicsWithValue(t, "expected kind map, got nil", func() {
dyn.NilValue.MustMap()
assert.PanicsWithValue(t, "expected kind map, got invalid", func() {
dyn.InvalidValue.MustMap()
})
}

Expand All @@ -40,15 +40,15 @@ func TestValueUnderlyingSequence(t *testing.T) {
vv1, ok := v.AsSequence()
assert.True(t, ok)

_, ok = dyn.NilValue.AsSequence()
_, ok = dyn.InvalidValue.AsSequence()
assert.False(t, ok)

vv2 := v.MustSequence()
assert.Equal(t, vv1, vv2)

// Test panic.
assert.PanicsWithValue(t, "expected kind sequence, got nil", func() {
dyn.NilValue.MustSequence()
assert.PanicsWithValue(t, "expected kind sequence, got invalid", func() {
dyn.InvalidValue.MustSequence()
})
}

Expand All @@ -58,15 +58,15 @@ func TestValueUnderlyingString(t *testing.T) {
vv1, ok := v.AsString()
assert.True(t, ok)

_, ok = dyn.NilValue.AsString()
_, ok = dyn.InvalidValue.AsString()
assert.False(t, ok)

vv2 := v.MustString()
assert.Equal(t, vv1, vv2)

// Test panic.
assert.PanicsWithValue(t, "expected kind string, got nil", func() {
dyn.NilValue.MustString()
assert.PanicsWithValue(t, "expected kind string, got invalid", func() {
dyn.InvalidValue.MustString()
})
}

Expand All @@ -76,15 +76,15 @@ func TestValueUnderlyingBool(t *testing.T) {
vv1, ok := v.AsBool()
assert.True(t, ok)

_, ok = dyn.NilValue.AsBool()
_, ok = dyn.InvalidValue.AsBool()
assert.False(t, ok)

vv2 := v.MustBool()
assert.Equal(t, vv1, vv2)

// Test panic.
assert.PanicsWithValue(t, "expected kind bool, got nil", func() {
dyn.NilValue.MustBool()
assert.PanicsWithValue(t, "expected kind bool, got invalid", func() {
dyn.InvalidValue.MustBool()
})
}

Expand All @@ -94,15 +94,15 @@ func TestValueUnderlyingInt(t *testing.T) {
vv1, ok := v.AsInt()
assert.True(t, ok)

_, ok = dyn.NilValue.AsInt()
_, ok = dyn.InvalidValue.AsInt()
assert.False(t, ok)

vv2 := v.MustInt()
assert.Equal(t, vv1, vv2)

// Test panic.
assert.PanicsWithValue(t, "expected kind int, got nil", func() {
dyn.NilValue.MustInt()
assert.PanicsWithValue(t, "expected kind int, got invalid", func() {
dyn.InvalidValue.MustInt()
})

// Test int32 type specifically.
Expand All @@ -124,15 +124,15 @@ func TestValueUnderlyingFloat(t *testing.T) {
vv1, ok := v.AsFloat()
assert.True(t, ok)

_, ok = dyn.NilValue.AsFloat()
_, ok = dyn.InvalidValue.AsFloat()
assert.False(t, ok)

vv2 := v.MustFloat()
assert.Equal(t, vv1, vv2)

// Test panic.
assert.PanicsWithValue(t, "expected kind float, got nil", func() {
dyn.NilValue.MustFloat()
assert.PanicsWithValue(t, "expected kind float, got invalid", func() {
dyn.InvalidValue.MustFloat()
})

// Test float64 type specifically.
Expand All @@ -148,14 +148,14 @@ func TestValueUnderlyingTime(t *testing.T) {
vv1, ok := v.AsTime()
assert.True(t, ok)

_, ok = dyn.NilValue.AsTime()
_, ok = dyn.InvalidValue.AsTime()
assert.False(t, ok)

vv2 := v.MustTime()
assert.Equal(t, vv1, vv2)

// Test panic.
assert.PanicsWithValue(t, "expected kind time, got nil", func() {
dyn.NilValue.MustTime()
assert.PanicsWithValue(t, "expected kind time, got invalid", func() {
dyn.InvalidValue.MustTime()
})
}
6 changes: 3 additions & 3 deletions libs/dyn/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func walk(v Value, p Path, fn func(p Path, v Value) (Value, error)) (Value, erro
if err == ErrSkip {
return v, nil
}
return NilValue, err
return InvalidValue, err
}

switch v.Kind() {
Expand All @@ -43,7 +43,7 @@ func walk(v Value, p Path, fn func(p Path, v Value) (Value, error)) (Value, erro
continue
}
if err != nil {
return NilValue, err
return InvalidValue, err
}
out.Set(pk, nv)
}
Expand All @@ -57,7 +57,7 @@ func walk(v Value, p Path, fn func(p Path, v Value) (Value, error)) (Value, erro
continue
}
if err != nil {
return NilValue, err
return InvalidValue, err
}
out = append(out, nv)
}
Expand Down
6 changes: 3 additions & 3 deletions libs/dyn/walk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (w *walkCallTracker) returnSkip(path string) {
}

func (w *walkCallTracker) returnDrop(path string) {
w.on(path, func(v Value) Value { return NilValue }, ErrDrop)
w.on(path, func(v Value) Value { return InvalidValue }, ErrDrop)
}

func (w *walkCallTracker) track(p Path, v Value) (Value, error) {
Expand Down Expand Up @@ -148,7 +148,7 @@ func TestWalkMapError(t *testing.T) {
})
out, err := Walk(value, tracker.track)
assert.Equal(t, cerr, err)
assert.Equal(t, NilValue, out)
assert.Equal(t, InvalidValue, out)

// The callback should have been called twice.
assert.Len(t, tracker.calls, 2)
Expand Down Expand Up @@ -239,7 +239,7 @@ func TestWalkSequenceError(t *testing.T) {
})
out, err := Walk(value, tracker.track)
assert.Equal(t, cerr, err)
assert.Equal(t, NilValue, out)
assert.Equal(t, InvalidValue, out)

// The callback should have been called three times.
assert.Len(t, tracker.calls, 3)
Expand Down
Loading