diff --git a/.changes/unreleased/BUG FIXES-20230630-103655.yaml b/.changes/unreleased/BUG FIXES-20230630-103655.yaml new file mode 100644 index 000000000..4ae13ffa9 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20230630-103655.yaml @@ -0,0 +1,6 @@ +kind: BUG FIXES +body: 'tftypes: Significantly reduced compute and memory usage of `Value` type + walking and transformation' +time: 2023-06-30T10:36:55.119505-04:00 +custom: + Issue: "307" diff --git a/.changes/unreleased/BUG FIXES-20230630-144701.yaml b/.changes/unreleased/BUG FIXES-20230630-144701.yaml new file mode 100644 index 000000000..311dd559c --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20230630-144701.yaml @@ -0,0 +1,6 @@ +kind: BUG FIXES +body: 'tftypes: Removed unnecessary memory allocations from `AttributePath` type + `Equal()`, `LastStep()`, and `WithoutLastStep()` methods' +time: 2023-06-30T14:47:01.127283-04:00 +custom: + Issue: "307" diff --git a/.changes/unreleased/BUG FIXES-20230630-144945.yaml b/.changes/unreleased/BUG FIXES-20230630-144945.yaml new file mode 100644 index 000000000..2ab382ad5 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20230630-144945.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'tftypes: Removed unnecessary memory allocations from `NewValue()` function' +time: 2023-06-30T14:49:45.828209-04:00 +custom: + Issue: "307" diff --git a/.changes/unreleased/ENHANCEMENTS-20230630-143817.yaml b/.changes/unreleased/ENHANCEMENTS-20230630-143817.yaml new file mode 100644 index 000000000..133bfbd91 --- /dev/null +++ b/.changes/unreleased/ENHANCEMENTS-20230630-143817.yaml @@ -0,0 +1,6 @@ +kind: ENHANCEMENTS +body: 'tftypes: Added `AttributePath` type `NextStep()` method, which returns the + next step in the path without first copying via `Steps()`' +time: 2023-06-30T14:38:17.600608-04:00 +custom: + Issue: "307" diff --git a/tftypes/attribute_path.go b/tftypes/attribute_path.go index 486c2d8b7..809cc99f6 100644 --- a/tftypes/attribute_path.go +++ b/tftypes/attribute_path.go @@ -80,14 +80,20 @@ func (a *AttributePath) String() string { // AttributePaths are considered equal if they have the same number of steps, // the steps are all the same types, and the steps have all the same values. func (a *AttributePath) Equal(o *AttributePath) bool { - if len(a.Steps()) == 0 && len(o.Steps()) == 0 { - return true + if a == nil { + return o == nil || len(o.steps) == 0 + } + + if o == nil { + return len(a.steps) == 0 } - if len(a.Steps()) != len(o.Steps()) { + + if len(a.steps) != len(o.steps) { return false } - for pos, aStep := range a.Steps() { - oStep := o.Steps()[pos] + + for pos, aStep := range a.steps { + oStep := o.steps[pos] if !aStep.Equal(oStep) { return false @@ -120,63 +126,112 @@ func (a *AttributePath) NewError(err error) error { } } -// LastStep returns the last step in the path. If the path was -// empty, nil is returned. +// LastStep returns the last step in the path. If the path is nil or empty, nil +// is returned. func (a *AttributePath) LastStep() AttributePathStep { - steps := a.Steps() + if a == nil || len(a.steps) == 0 { + return nil + } - if len(steps) == 0 { + return a.steps[len(a.steps)-1] +} + +// NextStep returns the next step in the path. If the path is nil or empty, nil +// is returned. +func (a *AttributePath) NextStep() AttributePathStep { + if a == nil || len(a.steps) == 0 { return nil } - return steps[len(steps)-1] + return a.steps[0] } // WithAttributeName adds an AttributeName step to `a`, using `name` as the // attribute's name. `a` is copied, not modified. func (a *AttributePath) WithAttributeName(name string) *AttributePath { - steps := a.Steps() + if a == nil { + return &AttributePath{ + steps: []AttributePathStep{AttributeName(name)}, + } + } + + // Avoid re-allocating larger slice + steps := make([]AttributePathStep, len(a.steps)+1) + copy(steps, a.steps) + steps[len(steps)-1] = AttributeName(name) + return &AttributePath{ - steps: append(steps, AttributeName(name)), + steps: steps, } } // WithElementKeyString adds an ElementKeyString step to `a`, using `key` as // the element's key. `a` is copied, not modified. func (a *AttributePath) WithElementKeyString(key string) *AttributePath { - steps := a.Steps() + if a == nil { + return &AttributePath{ + steps: []AttributePathStep{ElementKeyString(key)}, + } + } + + // Avoid re-allocating larger slice + steps := make([]AttributePathStep, len(a.steps)+1) + copy(steps, a.steps) + steps[len(steps)-1] = ElementKeyString(key) + return &AttributePath{ - steps: append(steps, ElementKeyString(key)), + steps: steps, } } // WithElementKeyInt adds an ElementKeyInt step to `a`, using `key` as the // element's key. `a` is copied, not modified. func (a *AttributePath) WithElementKeyInt(key int) *AttributePath { - steps := a.Steps() + if a == nil { + return &AttributePath{ + steps: []AttributePathStep{ElementKeyInt(key)}, + } + } + + // Avoid re-allocating larger slice + steps := make([]AttributePathStep, len(a.steps)+1) + copy(steps, a.steps) + steps[len(steps)-1] = ElementKeyInt(key) + return &AttributePath{ - steps: append(steps, ElementKeyInt(key)), + steps: steps, } } // WithElementKeyValue adds an ElementKeyValue to `a`, using `key` as the // element's key. `a` is copied, not modified. func (a *AttributePath) WithElementKeyValue(key Value) *AttributePath { - steps := a.Steps() + if a == nil { + return &AttributePath{ + steps: []AttributePathStep{ElementKeyValue(key)}, + } + } + + // Avoid re-allocating larger slice + steps := make([]AttributePathStep, len(a.steps)+1) + copy(steps, a.steps) + steps[len(steps)-1] = ElementKeyValue(key) + return &AttributePath{ - steps: append(steps, ElementKeyValue(key.Copy())), + steps: steps, } } // WithoutLastStep removes the last step, whatever kind of step it was, from // `a`. `a` is copied, not modified. func (a *AttributePath) WithoutLastStep() *AttributePath { - steps := a.Steps() - if len(steps) == 0 { + if a == nil || len(a.steps) == 0 { return nil } + return &AttributePath{ - steps: steps[:len(steps)-1], + // Paths are immutable, so this should be safe without copying. + steps: a.steps[:len(a.steps)-1], } } @@ -296,7 +351,7 @@ type AttributePathStepper interface { // types need to use the AttributePathStepper interface to tell // WalkAttributePath how to traverse themselves. func WalkAttributePath(in interface{}, path *AttributePath) (interface{}, *AttributePath, error) { - if len(path.Steps()) < 1 { + if path == nil || len(path.steps) == 0 { return in, path, nil } stepper, ok := in.(AttributePathStepper) @@ -306,11 +361,11 @@ func WalkAttributePath(in interface{}, path *AttributePath) (interface{}, *Attri return in, path, ErrNotAttributePathStepper } } - next, err := stepper.ApplyTerraform5AttributePathStep(path.Steps()[0]) + next, err := stepper.ApplyTerraform5AttributePathStep(path.NextStep()) if err != nil { return in, path, err } - return WalkAttributePath(next, &AttributePath{steps: path.Steps()[1:]}) + return WalkAttributePath(next, NewAttributePathWithSteps(path.steps[1:])) } func builtinAttributePathStepper(in interface{}) (AttributePathStepper, bool) { diff --git a/tftypes/value.go b/tftypes/value.go index af9f680f9..b84507839 100644 --- a/tftypes/value.go +++ b/tftypes/value.go @@ -134,15 +134,17 @@ func (val Value) ApplyTerraform5AttributePathStep(step AttributePathStep) (inter if !val.IsKnown() || val.IsNull() { return nil, ErrInvalidStep } + + // Since this logic is very hot path, it is optimized to use Value + // implementation details rather than As() to avoid memory allocations. switch s := step.(type) { case AttributeName: - if !val.Type().Is(Object{}) { + if _, ok := val.Type().(Object); !ok { return nil, ErrInvalidStep } - o := map[string]Value{} - err := val.As(&o) - if err != nil { - return nil, err + o, ok := val.value.(map[string]Value) + if !ok { + return nil, fmt.Errorf("cannot convert %T into map[string]tftypes.Value", val.value) } res, ok := o[string(s)] if !ok { @@ -150,13 +152,12 @@ func (val Value) ApplyTerraform5AttributePathStep(step AttributePathStep) (inter } return res, nil case ElementKeyString: - if !val.Type().Is(Map{}) { + if _, ok := val.Type().(Map); !ok { return nil, ErrInvalidStep } - m := map[string]Value{} - err := val.As(&m) - if err != nil { - return nil, err + m, ok := val.value.(map[string]Value) + if !ok { + return nil, fmt.Errorf("cannot convert %T into map[string]tftypes.Value", val.value) } res, ok := m[string(s)] if !ok { @@ -164,36 +165,37 @@ func (val Value) ApplyTerraform5AttributePathStep(step AttributePathStep) (inter } return res, nil case ElementKeyInt: - if !val.Type().Is(List{}) && !val.Type().Is(Tuple{}) { + _, listOk := val.Type().(List) + _, tupleOk := val.Type().(Tuple) + if !listOk && !tupleOk { return nil, ErrInvalidStep } if int64(s) < 0 { return nil, ErrInvalidStep } - sl := []Value{} - err := val.As(&sl) - if err != nil { - return nil, err + sl, ok := val.value.([]Value) + if !ok { + return nil, fmt.Errorf("cannot convert %T into []tftypes.Value", val.value) } if int64(len(sl)) <= int64(s) { return nil, ErrInvalidStep } return sl[int64(s)], nil case ElementKeyValue: - if !val.Type().Is(Set{}) { + if _, ok := val.Type().(Set); !ok { return nil, ErrInvalidStep } - sl := []Value{} - err := val.As(&sl) - if err != nil { - return nil, err + sl, ok := val.value.([]Value) + if !ok { + return nil, fmt.Errorf("cannot convert %T into []tftypes.Value", val.value) } + stepValue := Value(s) for _, el := range sl { - diffs, err := el.Diff(Value(s)) + deepEqual, err := stepValue.deepEqual(el) if err != nil { return nil, err } - if len(diffs) == 0 { + if deepEqual { return el, nil } } @@ -219,11 +221,11 @@ func (val Value) Equal(o Value) bool { if !val.Type().Equal(o.Type()) { return false } - diff, err := val.Diff(o) + deepEqual, err := val.deepEqual(o) if err != nil { panic(err) } - return len(diff) < 1 + return deepEqual } // Copy returns a defensively-copied clone of Value that shares no underlying @@ -302,57 +304,62 @@ func newValue(t Type, val interface{}) (Value, error) { } } - switch { - case t.Is(String): - v, err := valueFromString(val) - if err != nil { - return Value{}, err - } - return v, nil - case t.Is(Number): - v, err := valueFromNumber(val) - if err != nil { - return Value{}, err - } - return v, nil - case t.Is(Bool): - v, err := valueFromBool(val) - if err != nil { - return Value{}, err - } - return v, nil - case t.Is(Map{}): - v, err := valueFromMap(t.(Map).ElementType, val) - if err != nil { - return Value{}, err + switch typ := t.(type) { + case primitive: + switch typ.name { + case String.name: + v, err := valueFromString(val) + if err != nil { + return Value{}, err + } + return v, nil + case Number.name: + v, err := valueFromNumber(val) + if err != nil { + return Value{}, err + } + return v, nil + case Bool.name: + v, err := valueFromBool(val) + if err != nil { + return Value{}, err + } + return v, nil + case DynamicPseudoType.name: + v, err := valueFromDynamicPseudoType(val) + if err != nil { + return Value{}, err + } + return v, nil + default: + return Value{}, fmt.Errorf("unknown primitive type %v passed to tftypes.NewValue", typ) } - return v, nil - case t.Is(Object{}): - v, err := valueFromObject(t.(Object).AttributeTypes, t.(Object).OptionalAttributes, val) + case Map: + v, err := valueFromMap(typ.ElementType, val) if err != nil { return Value{}, err } return v, nil - case t.Is(List{}): - v, err := valueFromList(t.(List).ElementType, val) + case Object: + v, err := valueFromObject(typ.AttributeTypes, typ.OptionalAttributes, val) if err != nil { return Value{}, err } return v, nil - case t.Is(Set{}): - v, err := valueFromSet(t.(Set).ElementType, val) + case List: + v, err := valueFromList(typ.ElementType, val) if err != nil { return Value{}, err } return v, nil - case t.Is(Tuple{}): - v, err := valueFromTuple(t.(Tuple).ElementTypes, val) + case Set: + v, err := valueFromSet(typ.ElementType, val) if err != nil { return Value{}, err } return v, nil - case t.Is(DynamicPseudoType): - v, err := valueFromDynamicPseudoType(val) + case Tuple: + v, err := valueFromTuple(typ.ElementTypes, val) if err != nil { return Value{}, err } diff --git a/tftypes/value_benchmark_test.go b/tftypes/value_benchmark_test.go new file mode 100644 index 000000000..26132ed29 --- /dev/null +++ b/tftypes/value_benchmark_test.go @@ -0,0 +1,50 @@ +package tftypes + +import "testing" + +func BenchmarkValueApplyTerraform5AttributePathStep1000(b *testing.B) { + benchmarkValueApplyTerraform5AttributePathStep(b, 1000) +} + +// This benchmark iterates through an entire set of objects, which is one of the +// most expensive, but common, use cases. +func benchmarkValueApplyTerraform5AttributePathStep(b *testing.B, elements int) { + // Set of objects is one of the most expensive operations + objectType := Object{ + AttributeTypes: map[string]Type{ + "element_index": Number, // guaranteed to be different each element + "test_string": String, + }, + } + setType := Set{ + ElementType: objectType, + } + + setElements := make([]Value, elements) + + for index := range setElements { + setElements[index] = NewValue( + objectType, + map[string]Value{ + "element_index": NewValue(Number, index), + "test_string": NewValue(String, "test value"), + }, + ) + } + + value := NewValue( + setType, + setElements, + ) + + // ensure iteration occurs through whole set + step := ElementKeyValue(setElements[elements-1].Copy()) + + for n := 0; n < b.N; n++ { + _, err := value.ApplyTerraform5AttributePathStep(step) + + if err != nil { + b.Fatalf("unexpected ApplyTerraform5AttributePathStep error: %s", err) + } + } +} diff --git a/tftypes/value_equal.go b/tftypes/value_equal.go new file mode 100644 index 000000000..79a4e0fef --- /dev/null +++ b/tftypes/value_equal.go @@ -0,0 +1,210 @@ +package tftypes + +import ( + "errors" + "fmt" + "math/big" +) + +// deepEqual walks both Value to ensure any underlying Value are equal. This +// logic is essentially a duplicate of Diff, however it is intended to return +// early on any inequality and avoids memory allocations where possible. +// +// There might be ways to better share the internal logic of this method with +// Diff, however that effort is reserved for a time when the effort is justified +// over resolving the inherent compute and memory performance issues with Diff +// when only checking for inequality. +func (val1 Value) deepEqual(val2 Value) (bool, error) { + if val1.Type() == nil && val2.Type() == nil && val1.value == nil && val2.value == nil { + return false, nil + } + + if (val1.Type() == nil && val2.Type() != nil) || (val1.Type() != nil && val2.Type() == nil) { + return false, errors.New("cannot diff value missing type") + } + + if !val1.Type().Is(val2.Type()) { + return false, errors.New("Can't diff values of different types") + } + + // Capture walk differences for returning early + var hasDiff bool + + // make sure everything in val2 is also in val1 + err := Walk(val2, func(path *AttributePath, _ Value) (bool, error) { + _, _, err := val1.walkAttributePath(path) + + if err != nil && err != ErrInvalidStep { + return false, fmt.Errorf("Error walking %q: %w", path, err) + } else if err == ErrInvalidStep { + hasDiff = true + + return false, stopWalkError + } + + return true, nil + }) + + if err != nil { + return false, err + } + + if hasDiff { + return false, nil + } + + // make sure everything in val1 is also in val2 and also that it all matches + err = Walk(val1, func(path *AttributePath, value1 Value) (bool, error) { + // pull out the Value at the same path in val2 + value2, _, err := val2.walkAttributePath(path) + + if err != nil && err != ErrInvalidStep { + return false, fmt.Errorf("Error walking %q: %w", path, err) + } else if err == ErrInvalidStep { + hasDiff = true + + return false, stopWalkError + } + + // if they're both unknown, no need to continue + if !value1.IsKnown() && !value2.IsKnown() { + return false, nil + } + + if value1.IsKnown() != value2.IsKnown() { + hasDiff = true + + return false, stopWalkError + } + + // if they're both null, no need to continue + if value1.IsNull() && value2.IsNull() { + return false, nil + } + + if value1.IsNull() != value2.IsNull() { + hasDiff = true + + return false, stopWalkError + } + + // We know there are known, non-null values, time to compare them. + // Since this logic is very hot path, it is optimized to use type and + // value implementation details rather than Equal() and As() + // respectively, since both result in memory allocations. + switch typ := value1.Type().(type) { + case primitive: + switch typ.name { + case String.name: + s1, ok := value1.value.(string) + + if !ok { + return false, fmt.Errorf("cannot convert %T into string", value1.value) + } + + s2, ok := value2.value.(string) + + if !ok { + return false, fmt.Errorf("cannot convert %T into string", value2.value) + } + + if s1 != s2 { + hasDiff = true + + return false, stopWalkError + } + case Number.name: + n1, ok := value1.value.(*big.Float) + + if !ok { + return false, fmt.Errorf("cannot convert %T into *big.Float", value1.value) + } + + n2, ok := value2.value.(*big.Float) + + if !ok { + return false, fmt.Errorf("cannot convert %T into *big.Float", value2.value) + } + + if n1.Cmp(n2) != 0 { + hasDiff = true + + return false, stopWalkError + } + case Bool.name: + b1, ok := value1.value.(bool) + + if !ok { + return false, fmt.Errorf("cannot convert %T into bool", value1.value) + } + + b2, ok := value2.value.(bool) + + if !ok { + return false, fmt.Errorf("cannot convert %T into bool", value2.value) + } + + if b1 != b2 { + hasDiff = true + + return false, stopWalkError + } + case DynamicPseudoType.name: + // Let recursion from the walk check the sub-values match + return true, nil + } + + return false, nil + case List, Set, Tuple: + s1, ok := value1.value.([]Value) + + if !ok { + return false, fmt.Errorf("cannot convert %T into []tftypes.Value", value1.value) + } + + s2, ok := value2.value.([]Value) + + if !ok { + return false, fmt.Errorf("cannot convert %T into []tftypes.Value", value2.value) + } + + // we only care about if the lengths match for lists, + // sets, and tuples. If any of the elements differ, + // the recursion of the walk will find them for us. + if len(s1) != len(s2) { + hasDiff = true + + return false, stopWalkError + } + + return true, nil + case Map, Object: + m1, ok := value1.value.(map[string]Value) + + if !ok { + return false, fmt.Errorf("cannot convert %T into map[string]tftypes.Value", value1.value) + } + + m2, ok := value2.value.(map[string]Value) + + if !ok { + return false, fmt.Errorf("cannot convert %T into map[string]tftypes.Value", value2.value) + } + + // we only care about if the number of keys match for maps and + // objects. If any of the elements differ, the recursion of the walk + // will find them for us. + if len(m1) != len(m2) { + hasDiff = true + + return false, stopWalkError + } + + return true, nil + } + + return false, fmt.Errorf("unexpected type %v in Diff at %s", value1.Type(), path) + }) + + return !hasDiff, err +} diff --git a/tftypes/value_walk.go b/tftypes/value_walk.go new file mode 100644 index 000000000..d086a74e4 --- /dev/null +++ b/tftypes/value_walk.go @@ -0,0 +1,30 @@ +package tftypes + +import "fmt" + +// walkAttributePath will return the Value that `path` is pointing to within the +// Value. If an error is returned, the AttributePath returned will indicate +// will indicate the steps that remained to be applied when the error was +// encountered. +// +// This implementation, along with one for Type, could be exported to deprecate +// the overly generic WalkAttributePath function. +func (v Value) walkAttributePath(path *AttributePath) (Value, *AttributePath, error) { + if path == nil || len(path.steps) == 0 { + return v, path, nil + } + + nextValueI, err := v.ApplyTerraform5AttributePathStep(path.NextStep()) + + if err != nil { + return Value{}, path, err + } + + nextValue, ok := nextValueI.(Value) + + if !ok { + return Value{}, path, fmt.Errorf("unknown type %T returned from tftypes.ApplyTerraform5AttributePathStep", nextValueI) + } + + return nextValue.walkAttributePath(NewAttributePathWithSteps(path.steps[1:])) +} diff --git a/tftypes/walk.go b/tftypes/walk.go index 6b26f8ff3..e06a540ab 100644 --- a/tftypes/walk.go +++ b/tftypes/walk.go @@ -27,6 +27,18 @@ import ( "errors" ) +// stopWalkError is a well-known error for immediately stopping walk() without +// returning an actual error. +// +// The implementation of walk() will continue walking all attributes/elements +// within an object/collection since the boolean return value of the callback +// function is only intended to signal whether to stop descending into the same +// Value. Changing that behavior would be considered a breaking change. +// +// This could be considered for exporting to give external consumers better +// performance. +var stopWalkError = errors.New("walk stop requested") + // Walk traverses a Value, calling the passed function for every element and // attribute in the Value. The AttributePath passed to the callback function // will identify which attribute or element is currently being surfaced by the @@ -38,65 +50,116 @@ import ( // not matter when the Value that has been surfaced has no elements or // attributes. Walk uses a depth-first traversal. func Walk(val Value, cb func(*AttributePath, Value) (bool, error)) error { - return walk(nil, val, cb) + _, err := walk(NewAttributePath(), val, cb) + + return err } -func walk(path *AttributePath, val Value, cb func(*AttributePath, Value) (bool, error)) error { +// walk is the internal implementation of Walk(). It includes a bool return for +// whether callers should continue walking any remaining Value. +func walk(path *AttributePath, val Value, cb func(*AttributePath, Value) (bool, error)) (bool, error) { shouldContinue, err := cb(path, val) + + if errors.Is(err, stopWalkError) { + return false, nil + } + if err != nil { - return path.NewError(err) + return false, path.NewError(err) } + if !shouldContinue { - return nil + // The callback bool return is intended to signal that this Value should + // no longer be descended. Changing this behavior is a breaking change. + // A stopWalkError can be used to signal that all remaining Value can be + // skipped. + return true, nil } if val.IsNull() || !val.IsKnown() { - return nil + return true, nil } - ty := val.Type() - switch { - case ty.Is(List{}), ty.Is(Set{}), ty.Is(Tuple{}): - var v []Value - err := val.As(&v) - if err != nil { - // should never happen - return path.NewError(err) + switch val.Type().(type) { + case List, Tuple: + v, ok := val.value.([]Value) + + if !ok { + return false, path.NewErrorf("cannot convert %T into []tftypes.Value", val.value) } + for pos, el := range v { - if ty.Is(Set{}) { - path = path.WithElementKeyValue(el) - } else { - path = path.WithElementKeyInt(pos) + elementPath := path.WithElementKeyInt(pos) + shouldContinue, err := walk(elementPath, el, cb) + + if err != nil { + return false, elementPath.NewError(err) } - err = walk(path, el, cb) + + if !shouldContinue { + return false, nil + } + } + case Map: + v, ok := val.value.(map[string]Value) + + if !ok { + return false, path.NewErrorf("cannot convert %T into map[string]tftypes.Value", val.value) + } + + for k, el := range v { + elementPath := path.WithElementKeyString(k) + shouldContinue, err := walk(elementPath, el, cb) + if err != nil { - return path.NewError(err) + return false, elementPath.NewError(err) + } + + if !shouldContinue { + return false, nil } - path = path.WithoutLastStep() } - case ty.Is(Map{}), ty.Is(Object{}): - v := map[string]Value{} - err := val.As(&v) - if err != nil { - // should never happen - return err + case Object: + v, ok := val.value.(map[string]Value) + + if !ok { + return false, path.NewErrorf("cannot convert %T into map[string]tftypes.Value", val.value) } + for k, el := range v { - if ty.Is(Map{}) { - path = path.WithElementKeyString(k) - } else if ty.Is(Object{}) { - path = path.WithAttributeName(k) + attributePath := path.WithAttributeName(k) + shouldContinue, err := walk(attributePath, el, cb) + + if err != nil { + return false, attributePath.NewError(err) + } + + if !shouldContinue { + return false, nil } - err = walk(path, el, cb) + } + case Set: + v, ok := val.value.([]Value) + + if !ok { + return false, path.NewErrorf("cannot convert %T into []tftypes.Value", val.value) + } + + for _, el := range v { + elementPath := path.WithElementKeyValue(el) + shouldContinue, err := walk(elementPath, el, cb) + if err != nil { - return path.NewError(err) + return false, elementPath.NewError(err) + } + + if !shouldContinue { + return false, nil } - path = path.WithoutLastStep() } } - return nil + return true, nil } // Transform uses a callback to mutate a Value. Each element or attribute will @@ -113,85 +176,174 @@ func Transform(val Value, cb func(*AttributePath, Value) (Value, error)) (Value, } func transform(path *AttributePath, val Value, cb func(*AttributePath, Value) (Value, error)) (Value, error) { - var newVal Value - ty := val.Type() - - if ty == nil { + switch val.Type().(type) { + case nil: return val, path.NewError(errors.New("invalid transform: value missing type")) } - switch { - case val.IsNull() || !val.IsKnown(): - newVal = val - case ty.Is(List{}), ty.Is(Set{}), ty.Is(Tuple{}): - var v []Value - err := val.As(&v) - if err != nil { - return val, err - } - if len(v) == 0 { - newVal = val - } else { - elems := make([]Value, 0, len(v)) - for pos, el := range v { - if ty.Is(Set{}) { - path = path.WithElementKeyValue(el) - } else { - path = path.WithElementKeyInt(pos) - } - newEl, err := transform(path, el, cb) - if err != nil { - return val, path.NewError(err) - } - elems = append(elems, newEl) - path = path.WithoutLastStep() - } - newVal, err = newValue(ty, elems) - if err != nil { - return val, path.NewError(err) - } - } - case ty.Is(Map{}), ty.Is(Object{}): - v := map[string]Value{} - err := val.As(&v) - if err != nil { - return val, err - } - if len(v) == 0 { - newVal = val - } else { - elems := map[string]Value{} - for k, el := range v { - if ty.Is(Map{}) { - path = path.WithElementKeyString(k) - } else { - path = path.WithAttributeName(k) - } - newEl, err := transform(path, el, cb) - if err != nil { - return val, path.NewError(err) - } - elems[k] = newEl - path = path.WithoutLastStep() - } - newVal, err = newValue(ty, elems) - if err != nil { - return val, path.NewError(err) - } - } - default: - newVal = val + newVal, err := transformUnderlying(path, val, cb) + + if err != nil { + return val, err } + res, err := cb(path, newVal) + if err != nil { return res, path.NewError(err) } + newTy := newVal.Type() + if newTy == nil { return val, path.NewError(errors.New("invalid transform: new value missing type")) } - if !newTy.UsableAs(ty) { + + if !newTy.UsableAs(val.Type()) { return val, path.NewError(errors.New("invalid transform: value changed type")) } + return res, err } + +// transformUnderlying returns the Value with any underlying attribute or +// element transformations completed. +func transformUnderlying(path *AttributePath, val Value, cb func(*AttributePath, Value) (Value, error)) (Value, error) { + // If the Value is null or unknown, there is nothing to descend. + if val.IsNull() || !val.IsKnown() { + return val, nil + } + + switch val.Type().(type) { + case List, Tuple: + elements, ok := val.value.([]Value) + + if !ok { + return val, path.NewErrorf("cannot convert %T into []tftypes.Value", val.value) + } + + if len(elements) == 0 { + return val, nil + } + + newElements := make([]Value, 0, len(elements)) + + for index, element := range elements { + elementPath := path.WithElementKeyInt(index) + + newElement, err := transform(elementPath, element, cb) + + if err != nil { + return val, elementPath.NewError(err) + } + + newElements = append(newElements, newElement) + } + + newVal, err := newValue(val.Type(), newElements) + + if err != nil { + return val, path.NewError(err) + } + + return newVal, nil + case Map: + elements, ok := val.value.(map[string]Value) + + if !ok { + return val, path.NewErrorf("cannot convert %T into map[string]tftypes.Value", val.value) + } + + if len(elements) == 0 { + return val, nil + } + + newElements := make(map[string]Value, len(elements)) + + for key, element := range elements { + elementPath := path.WithElementKeyString(key) + + newElement, err := transform(elementPath, element, cb) + + if err != nil { + return val, elementPath.NewError(err) + } + + newElements[key] = newElement + } + + newVal, err := newValue(val.Type(), newElements) + + if err != nil { + return val, path.NewError(err) + } + + return newVal, nil + case Object: + attributes, ok := val.value.(map[string]Value) + + if !ok { + return val, path.NewErrorf("cannot convert %T into map[string]tftypes.Value", val.value) + } + + if len(attributes) == 0 { + return val, nil + } + + newAttributes := make(map[string]Value, len(attributes)) + + for name, attribute := range attributes { + attributePath := path.WithAttributeName(name) + + newAttribute, err := transform(attributePath, attribute, cb) + + if err != nil { + return val, attributePath.NewError(err) + } + + newAttributes[name] = newAttribute + } + + newVal, err := newValue(val.Type(), newAttributes) + + if err != nil { + return val, path.NewError(err) + } + + return newVal, nil + case Set: + elements, ok := val.value.([]Value) + + if !ok { + return val, path.NewErrorf("cannot convert %T into []tftypes.Value", val.value) + } + + if len(elements) == 0 { + return val, nil + } + + newElements := make([]Value, 0, len(elements)) + + for _, element := range elements { + elementPath := path.WithElementKeyValue(element) + + newElement, err := transform(elementPath, element, cb) + + if err != nil { + return val, elementPath.NewError(err) + } + + newElements = append(newElements, newElement) + } + + newVal, err := newValue(val.Type(), newElements) + + if err != nil { + return val, path.NewError(err) + } + + return newVal, nil + } + + return val, nil +} diff --git a/tftypes/walk_benchmark_test.go b/tftypes/walk_benchmark_test.go new file mode 100644 index 000000000..f5a94a67c --- /dev/null +++ b/tftypes/walk_benchmark_test.go @@ -0,0 +1,52 @@ +package tftypes + +import "testing" + +func BenchmarkTransform1000(b *testing.B) { + benchmarkTransform(b, 1000) +} + +// This benchmark iterates through an entire set of objects, which is one of the +// most expensive, but common, use cases. +func benchmarkTransform(b *testing.B, elements int) { + // Set of objects is one of the most expensive operations + objectType := Object{ + AttributeTypes: map[string]Type{ + "element_index": Number, // guaranteed to be different each element + "test_string": String, + }, + } + setType := Set{ + ElementType: objectType, + } + + setElements := make([]Value, elements) + + for index := range setElements { + setElements[index] = NewValue( + objectType, + map[string]Value{ + "element_index": NewValue(Number, index), + "test_string": NewValue(String, "test value"), + }, + ) + } + + value := NewValue( + setType, + setElements, + ) + + for n := 0; n < b.N; n++ { + _, err := Transform( + value, + func(_ *AttributePath, value Value) (Value, error) { + return value, nil + }, + ) + + if err != nil { + b.Fatalf("unexpected Transform error: %s", err) + } + } +} diff --git a/tftypes/walk_test.go b/tftypes/walk_test.go index 9b39d1b05..7d9afce5c 100644 --- a/tftypes/walk_test.go +++ b/tftypes/walk_test.go @@ -1748,3 +1748,236 @@ func TestTransform(t *testing.T) { }) } } + +// This test is similar to TestTransform, but notably also verifies that the +// original Value is not modified, which could cause an unexpected breaking +// change for consumers. +func TestTransform_OriginalValueUnmodified(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + value Value + callback func(*AttributePath, Value) (Value, error) + expected Value // Transform return, which should be modified + }{ + "Bool": { + value: NewValue(Bool, false), + callback: func(_ *AttributePath, v Value) (Value, error) { + return NewValue(Bool, true), nil + }, + expected: NewValue(Bool, true), + }, + "List-element": { + value: NewValue( + List{ElementType: Bool}, + []Value{ + NewValue(Bool, false), + }, + ), + callback: func(_ *AttributePath, v Value) (Value, error) { + switch v.Type().(type) { + case primitive: + return NewValue(Bool, true), nil + default: + return v, nil + } + }, + expected: NewValue( + List{ElementType: Bool}, + []Value{ + NewValue(Bool, true), + }, + ), + }, + "Map-element": { + value: NewValue( + Map{ElementType: Bool}, + map[string]Value{ + "testkey": NewValue(Bool, false), + }, + ), + callback: func(_ *AttributePath, v Value) (Value, error) { + switch v.Type().(type) { + case primitive: + return NewValue(Bool, true), nil + default: + return v, nil + } + }, + expected: NewValue( + Map{ElementType: Bool}, + map[string]Value{ + "testkey": NewValue(Bool, true), + }, + ), + }, + "Number": { + value: NewValue(Number, 123), + callback: func(_ *AttributePath, v Value) (Value, error) { + return NewValue(Number, 456), nil + }, + expected: NewValue(Number, 456), + }, + "Object-attribute": { + value: NewValue( + Object{ + AttributeTypes: map[string]Type{ + "testattr": Bool, + }, + }, + map[string]Value{ + "testattr": NewValue(Bool, false), + }, + ), + callback: func(_ *AttributePath, v Value) (Value, error) { + switch v.Type().(type) { + case primitive: + return NewValue(Bool, true), nil + default: + return v, nil + } + }, + expected: NewValue( + Object{ + AttributeTypes: map[string]Type{ + "testattr": Bool, + }, + }, + map[string]Value{ + "testattr": NewValue(Bool, true), + }, + ), + }, + "Object-Object-attribute": { + value: NewValue( + Object{ + AttributeTypes: map[string]Type{ + "testobj": Object{ + AttributeTypes: map[string]Type{ + "testattr": Bool, + }, + }, + }, + }, + map[string]Value{ + "testobj": NewValue( + Object{ + AttributeTypes: map[string]Type{ + "testattr": Bool, + }, + }, + map[string]Value{ + "testattr": NewValue(Bool, false), + }, + ), + }, + ), + callback: func(_ *AttributePath, v Value) (Value, error) { + switch v.Type().(type) { + case primitive: + return NewValue(Bool, true), nil + default: + return v, nil + } + }, + expected: NewValue( + Object{ + AttributeTypes: map[string]Type{ + "testobj": Object{ + AttributeTypes: map[string]Type{ + "testattr": Bool, + }, + }, + }, + }, + map[string]Value{ + "testobj": NewValue( + Object{ + AttributeTypes: map[string]Type{ + "testattr": Bool, + }, + }, + map[string]Value{ + "testattr": NewValue(Bool, true), + }, + ), + }, + ), + }, + "Set-element": { + value: NewValue( + Set{ElementType: Bool}, + []Value{ + NewValue(Bool, false), + }, + ), + callback: func(_ *AttributePath, v Value) (Value, error) { + switch v.Type().(type) { + case primitive: + return NewValue(Bool, true), nil + default: + return v, nil + } + }, + expected: NewValue( + Set{ElementType: Bool}, + []Value{ + NewValue(Bool, true), + }, + ), + }, + "String": { + value: NewValue(String, "original value"), + callback: func(_ *AttributePath, v Value) (Value, error) { + return NewValue(String, "new value"), nil + }, + expected: NewValue(String, "new value"), + }, + "Tuple-element": { + value: NewValue( + Tuple{ElementTypes: []Type{Bool}}, + []Value{ + NewValue(Bool, false), + }, + ), + callback: func(_ *AttributePath, v Value) (Value, error) { + switch v.Type().(type) { + case primitive: + return NewValue(Bool, true), nil + default: + return v, nil + } + }, + expected: NewValue( + Tuple{ElementTypes: []Type{Bool}}, + []Value{ + NewValue(Bool, true), + }, + ), + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + copiedValue := testCase.value.Copy() + + got, err := Transform(testCase.value, testCase.callback) + + if err != nil { + t.Fatalf("unexpected Transform error: %s", err) + } + + if diff := cmp.Diff(got, testCase.expected); diff != "" { + t.Errorf("unexpected Transform difference: %s", diff) + } + + if diff := cmp.Diff(testCase.value, copiedValue); diff != "" { + t.Errorf("unexpected original Value difference: %s", diff) + } + }) + } +}