helper/schema: Remove exported SetDiff method

The consensus is that it's a generally better idea to move setting the
functionality of old values completely to the refresh/read process,
hence it's moot to have this function around anymore. This also means we
don't need the old value reader/writer anymore, which simplifies things.

helper/schema/resource_diff.go

@@ -124,9 +124,6 @@ type ResourceDiff struct {
 	// diff, and the new diff.
 	multiReader *MultiLevelFieldReader
 
-	// A writer that writes overridden old fields.
-	oldWriter *MapFieldWriter
-
 	// A writer that writes overridden new fields.
 	newWriter *newValueWriter
 
@@ -146,7 +143,6 @@ func newResourceDiff(schema map[string]*Schema, config *terraform.ResourceConfig
 		schema: schema,
 	}
 
-	d.oldWriter = &MapFieldWriter{Schema: d.schema}
 	d.newWriter = &newValueWriter{
 		MapFieldWriter: &MapFieldWriter{Schema: d.schema},
 	}
@@ -175,11 +171,7 @@ func newResourceDiff(schema map[string]*Schema, config *terraform.ResourceConfig
 			},
 		}
 	}
-	readers["newDiffOld"] = &MapFieldReader{
+	readers["newDiff"] = &newValueReader{
-		Schema: d.schema,
-		Map:    BasicMapReader(d.oldWriter.Map()),
-	}
-	readers["newDiffNew"] = &newValueReader{
 		MapFieldReader: &MapFieldReader{
 			Schema: d.schema,
 			Map:    BasicMapReader(d.newWriter.Map()),
@@ -191,8 +183,7 @@ func newResourceDiff(schema map[string]*Schema, config *terraform.ResourceConfig
 			"state",
 			"config",
 			"diff",
-			"newDiffOld",
+			"newDiff",
-			"newDiffNew",
 		},
 
 		Readers: readers,
@@ -261,7 +252,10 @@ func (d *ResourceDiff) diffChange(key string) (interface{}, interface{}, bool, b
 //
 // This function is only allowed on computed attributes.
 func (d *ResourceDiff) SetNew(key string, value interface{}) error {
-	return d.SetDiff(key, d.get(strings.Split(key, "."), "state").Value, value, false)
+	if !d.schema[key].Computed {
+		return fmt.Errorf("SetNew only operates on computed keys - %s is not one", key)
+	}
+	return d.setDiff(key, d.get(strings.Split(key, "."), "state").Value, value, false)
 }
 
 // SetNewComputed functions like SetNew, except that it blanks out a new value
@@ -269,33 +263,18 @@ func (d *ResourceDiff) SetNew(key string, value interface{}) error {
 //
 // This function is only allowed on computed attributes.
 func (d *ResourceDiff) SetNewComputed(key string) error {
-	return d.SetDiff(key, d.get(strings.Split(key, "."), "state").Value, nil, true)
-}
-
-// SetDiff allows the setting of both old and new values for the diff
-// referenced by a given key. This can be used to completely override
-// Terraform's own diff behaviour, and can be used in conjunction with Clear or
-// ClearAll to construct a compleletely new diff based off of provider logic
-// alone.
-//
-// This function is only allowed on computed attributes.
-func (d *ResourceDiff) SetDiff(key string, old, new interface{}, computed bool) error {
 	if !d.schema[key].Computed {
-		return fmt.Errorf("SetNew, SetNewComputed, and SetDiff are allowed on computed attributes only - %s is not one", key)
+		return fmt.Errorf("SetNewComputed only operates on computed keys - %s is not one", key)
 	}
-
+	return d.setDiff(key, d.get(strings.Split(key, "."), "state").Value, nil, true)
-	return d.setDiff(key, old, new, computed)
 }
 
+// setDiff performs common diff setting behaviour.
 func (d *ResourceDiff) setDiff(key string, old, new interface{}, computed bool) error {
 	if err := d.clear(key); err != nil {
 		return err
 	}
 
-	if err := d.oldWriter.WriteField(strings.Split(key, "."), old); err != nil {
-		return fmt.Errorf("Cannot set old diff value for key %s: %s", key, err)
-	}
-
 	if err := d.newWriter.WriteField(strings.Split(key, "."), new, computed); err != nil {
 		return fmt.Errorf("Cannot set new diff value for key %s: %s", key, err)
 	}
@@ -341,7 +320,7 @@ func (d *ResourceDiff) GetChange(key string) (interface{}, interface{}) {
 // new diff levels to provide data consistent with the current state of the
 // customized diff.
 func (d *ResourceDiff) GetOk(key string) (interface{}, bool) {
-	r := d.get(strings.Split(key, "."), "newDiffNew")
+	r := d.get(strings.Split(key, "."), "newDiff")
 	exists := r.Exists && !r.Computed
 	if exists {
 		// If it exists, we also want to verify it is not the zero-value.
@@ -394,19 +373,16 @@ func (d *ResourceDiff) Id() string {
 // results from the exact levels for the new diff, then from state and diff as
 // per normal.
 func (d *ResourceDiff) getChange(key string) (getResult, getResult) {
-	old := d.getExact(strings.Split(key, "."), "newDiffOld")
+	old := d.get(strings.Split(key, "."), "state")
-	new := d.getExact(strings.Split(key, "."), "newDiffNew")
+	var new getResult
-
+	for p := range d.updatedKeys {
-	if old.Exists || new.Exists {
+		if childAddrOf(key, p) {
-		// If one of these values exists, then SetDiff has operated on this value,
+			new = d.getExact(strings.Split(key, "."), "newDiff")
-		// and as such the newDiff level should be used.
+			goto done
-		return old, new
+		}
 	}
-
+	new = d.get(strings.Split(key, "."), "newDiff")
-	// If we haven't set this in the new diff, then we want to get the default
+done:
-	// levels as if we were using ResourceData normally.
-	old = d.get(strings.Split(key, "."), "state")
-	new = d.get(strings.Split(key, "."), "diff")
 	return old, new
 }
 
@@ -453,3 +429,11 @@ func (d *ResourceDiff) finalizeResult(addr []string, result FieldReadResult) get
 		Schema:         schema,
 	}
 }
+
+// childAddrOf does a comparison of two addresses to see if one is the child of
+// the other.
+func childAddrOf(child, parent string) bool {
+	cs := strings.Split(child, ".")
+	ps := strings.Split(parent, ".")
+	return reflect.DeepEqual(ps, cs[:len(ps)])
+}

helper/schema/resource_diff_test.go

@@ -1,7 +1,6 @@
 package schema
 
 import (
-	"fmt"
 	"reflect"
 	"testing"
 
@@ -62,12 +61,11 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 				},
 			},
 			Key:      "foo",
-			OldValue: fmt.Sprintf("%sbar", oldPrefix),
 			NewValue: "qux",
 			Expected: &terraform.InstanceDiff{
 				Attributes: map[string]*terraform.ResourceAttrDiff{
 					"foo": &terraform.ResourceAttrDiff{
-						Old: fmt.Sprintf("%sbar", oldPrefix),
+						Old: "bar",
 						New: func() string {
 							if computed {
 								return ""
@@ -113,7 +111,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 				},
 			},
 			Key:      "foo",
-			OldValue: []interface{}{fmt.Sprintf("%sbar", oldPrefix)},
 			NewValue: []interface{}{"qux"},
 			Expected: &terraform.InstanceDiff{
 				Attributes: func() map[string]*terraform.ResourceAttrDiff {
@@ -129,8 +126,8 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 							Old: "",
 							New: "qux",
 						}
-						result[fmt.Sprintf("foo.%d", HashString(fmt.Sprintf("%sbar", oldPrefix)))] = &terraform.ResourceAttrDiff{
+						result["foo.1996459178"] = &terraform.ResourceAttrDiff{
-							Old:        fmt.Sprintf("%sbar", oldPrefix),
+							Old:        "bar",
 							New:        "",
 							NewRemoved: true,
 						}
@@ -167,7 +164,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 				},
 			},
 			Key:      "foo",
-			OldValue: []interface{}{fmt.Sprintf("%sbar", oldPrefix)},
 			NewValue: []interface{}{"qux"},
 			Expected: &terraform.InstanceDiff{
 				Attributes: func() map[string]*terraform.ResourceAttrDiff {
@@ -180,7 +176,7 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 						}
 					} else {
 						result["foo.0"] = &terraform.ResourceAttrDiff{
-							Old: fmt.Sprintf("%sbar", oldPrefix),
+							Old: "bar",
 							New: "qux",
 						}
 					}
@@ -215,7 +211,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 				},
 			},
 			Key:      "foo",
-			OldValue: map[string]interface{}{"bar": fmt.Sprintf("%sbaz", oldPrefix)},
 			NewValue: map[string]interface{}{"bar": "quux"},
 			Expected: &terraform.InstanceDiff{
 				Attributes: func() map[string]*terraform.ResourceAttrDiff {
@@ -233,7 +228,7 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 						}
 					} else {
 						result["foo.bar"] = &terraform.ResourceAttrDiff{
-							Old: fmt.Sprintf("%sbaz", oldPrefix),
+							Old: "baz",
 							New: "quux",
 						}
 					}
@@ -272,7 +267,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 				},
 			},
 			Key:      "one",
-			OldValue: fmt.Sprintf("%stwo", oldPrefix),
 			NewValue: "four",
 			Expected: &terraform.InstanceDiff{
 				Attributes: map[string]*terraform.ResourceAttrDiff{
@@ -281,7 +275,7 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 						New: "baz",
 					},
 					"one": &terraform.ResourceAttrDiff{
-						Old: fmt.Sprintf("%stwo", oldPrefix),
+						Old: "two",
 						New: func() string {
 							if computed {
 								return ""
@@ -323,7 +317,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 				},
 			},
 			Key:      "one",
-			OldValue: fmt.Sprintf("%stwo", oldPrefix),
 			NewValue: "three",
 			Expected: &terraform.InstanceDiff{
 				Attributes: map[string]*terraform.ResourceAttrDiff{
@@ -332,7 +325,7 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 						New: "baz",
 					},
 					"one": &terraform.ResourceAttrDiff{
-						Old: fmt.Sprintf("%stwo", oldPrefix),
+						Old: "two",
 						New: func() string {
 							if computed {
 								return ""
@@ -410,30 +403,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 				},
 			},
 			Key: "top",
-			OldValue: NewSet(testSetFunc, []interface{}{
-				map[string]interface{}{
-					"foo": 1,
-					"bar": 4 + oldOffset,
-				},
-				map[string]interface{}{
-					"foo": 13 + oldOffset,
-					"bar": 12,
-				},
-			}),
-			NewValue: NewSet(testSetFunc, []interface{}{
-				map[string]interface{}{
-					"foo": 1,
-					"bar": 4,
-				},
-				map[string]interface{}{
-					"foo": 13,
-					"bar": 12,
-				},
-				map[string]interface{}{
-					"foo": 21,
-					"bar": 22,
-				},
-			}),
 			Expected: &terraform.InstanceDiff{
 				Attributes: func() map[string]*terraform.ResourceAttrDiff {
 					result := make(map[string]*terraform.ResourceAttrDiff)
@@ -493,12 +462,11 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 			Config:   testConfig(t, map[string]interface{}{}),
 			Diff:     &terraform.InstanceDiff{Attributes: map[string]*terraform.ResourceAttrDiff{}},
 			Key:      "foo",
-			OldValue: fmt.Sprintf("%sbar", oldPrefix),
 			NewValue: "baz",
 			Expected: &terraform.InstanceDiff{
 				Attributes: map[string]*terraform.ResourceAttrDiff{
 					"foo": &terraform.ResourceAttrDiff{
-						Old: fmt.Sprintf("%sbar", oldPrefix),
+						Old: "bar",
 						New: func() string {
 							if computed {
 								return ""
@@ -535,7 +503,6 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 				},
 			},
 			Key:           "foo",
-			OldValue:      fmt.Sprintf("%sbar", oldPrefix),
 			NewValue:      "qux",
 			ExpectedError: true,
 		},
@@ -596,33 +563,6 @@ func TestSetNewComputed(t *testing.T) {
 	}
 }
 
-func TestSetDiff(t *testing.T) {
-	testCases := testDiffCases(t, "testSetDiff", 1, false)
-	for _, tc := range testCases {
-		t.Run(tc.Name, func(t *testing.T) {
-			m := schemaMap(tc.Schema)
-			d := newResourceDiff(tc.Schema, tc.Config, tc.State, tc.Diff)
-			err := d.SetDiff(tc.Key, tc.OldValue, tc.NewValue, false)
-			switch {
-			case err != nil && !tc.ExpectedError:
-				t.Fatalf("bad: %s", err)
-			case err == nil && tc.ExpectedError:
-				t.Fatalf("Expected error, got none")
-			case err != nil && tc.ExpectedError:
-				return
-			}
-			for _, k := range d.UpdatedKeys() {
-				if err := m.diff(k, m[k], tc.Diff, d, false); err != nil {
-					t.Fatalf("bad: %s", err)
-				}
-			}
-			if !reflect.DeepEqual(tc.Expected, tc.Diff) {
-				t.Fatalf("Expected %s, got %s", spew.Sdump(tc.Expected), spew.Sdump(tc.Diff))
-			}
-		})
-	}
-}
-
 func TestForceNew(t *testing.T) {
 	cases := []resourceDiffTestCase{
 		resourceDiffTestCase{