helper/schema: diff field reader should merge result with source

helper/schema/field_reader_diff.go

@@ -9,8 +9,25 @@ import (
 )
 
 // DiffFieldReader reads fields out of a diff structures.
+//
+// It also requires access to a Reader that reads fields from the structure
+// that the diff was derived from. This is usually the state. This is required
+// because a diff on its own doesn't have complete data about full objects
+// such as maps.
+//
+// The Source MUST be the data that the diff was derived from. If it isn't,
+// the behavior of this struct is undefined.
+//
+// Reading fields from a DiffFieldReader is identical to reading from
+// Source except the diff will be applied to the end result.
+//
+// The "Exists" field on the result will be set to true if the complete
+// field exists whether its from the source, diff, or a combination of both.
+// It cannot be determined whether a retrieved value is composed of
+// diff elements.
 type DiffFieldReader struct {
-	Diff *terraform.InstanceDiff
+	Diff   *terraform.InstanceDiff
+	Source FieldReader
 }
 
 func (r *DiffFieldReader) ReadField(
@@ -27,7 +44,7 @@ func (r *DiffFieldReader) ReadField(
 	case TypeList:
 		return readListField(r, k, schema)
 	case TypeMap:
-		return r.readMap(k)
+		return r.readMap(k, schema)
 	case TypeSet:
 		return r.readSet(k, schema)
 	case typeObject:
@@ -37,11 +54,23 @@ func (r *DiffFieldReader) ReadField(
 	}
 }
 
-func (r *DiffFieldReader) readMap(k string) (FieldReadResult, error) {
+func (r *DiffFieldReader) readMap(
+	k string, schema *Schema) (FieldReadResult, error) {
 	result := make(map[string]interface{})
-	negresult := make(map[string]interface{})
 	resultSet := false
 
+	// First read the map from the underlying source
+	source, err := r.Source.ReadField([]string{k}, schema)
+	if err != nil {
+		return FieldReadResult{}, err
+	}
+	if source.Exists {
+		result = source.Value.(map[string]interface{})
+		resultSet = true
+	}
+
+	// Next, read all the elements we have in our diff, and apply
+	// the diff to our result.
 	prefix := k + "."
 	for k, v := range r.Diff.Attributes {
 		if !strings.HasPrefix(k, prefix) {
@@ -51,7 +80,7 @@ func (r *DiffFieldReader) readMap(k string) (FieldReadResult, error) {
 
 		k = k[len(prefix):]
 		if v.NewRemoved {
-			negresult[k] = ""
+			delete(result, k)
 			continue
 		}
 
@@ -64,39 +93,41 @@ func (r *DiffFieldReader) readMap(k string) (FieldReadResult, error) {
 	}
 
 	return FieldReadResult{
-		Value:    resultVal,
-		NegValue: negresult,
-		Exists:   resultSet,
+		Value:  resultVal,
+		Exists: resultSet,
 	}, nil
 }
 
 func (r *DiffFieldReader) readPrimitive(
 	k string, schema *Schema) (FieldReadResult, error) {
-	attrD, ok := r.Diff.Attributes[k]
-	if !ok {
-		return FieldReadResult{}, nil
+	result, err := r.Source.ReadField([]string{k}, schema)
+	if err != nil {
+		return FieldReadResult{}, err
 	}
 
-	var result string
+	attrD, ok := r.Diff.Attributes[k]
+	if !ok {
+		return result, nil
+	}
+
+	var resultVal string
 	if !attrD.NewComputed {
-		result = attrD.New
+		resultVal = attrD.New
 		if attrD.NewExtra != nil {
-			if err := mapstructure.WeakDecode(attrD.NewExtra, &result); err != nil {
+			if err := mapstructure.WeakDecode(attrD.NewExtra, &resultVal); err != nil {
 				return FieldReadResult{}, err
 			}
 		}
 	}
 
-	returnVal, err := stringToPrimitive(result, false, schema)
+	result.Exists = true
+	result.Computed = attrD.NewComputed
+	result.Value, err = stringToPrimitive(resultVal, false, schema)
 	if err != nil {
 		return FieldReadResult{}, err
 	}
 
-	return FieldReadResult{
-		Value:    returnVal,
-		Exists:   true,
-		Computed: attrD.NewComputed,
-	}, nil
+	return result, nil
 }
 
 func (r *DiffFieldReader) readSet(

helper/schema/field_reader_diff_test.go

@@ -76,11 +76,6 @@ func TestDiffFieldReader(t *testing.T) {
 					New: "baz",
 				},
 
-				"mapRemove.foo": &terraform.ResourceAttrDiff{
-					Old: "",
-					New: "bar",
-				},
-
 				"mapRemove.bar": &terraform.ResourceAttrDiff{
 					NewRemoved: true,
 				},
@@ -124,6 +119,31 @@ func TestDiffFieldReader(t *testing.T) {
 					Old: "",
 					New: "bar",
 				},
+
+				"listMap.0.bar": &terraform.ResourceAttrDiff{
+					NewRemoved: true,
+				},
+
+				"setChange.10.value": &terraform.ResourceAttrDiff{
+					Old: "50",
+					New: "80",
+				},
+			},
+		},
+
+		Source: &MapFieldReader{
+			Map: map[string]string{
+				"listMap.#":     "2",
+				"listMap.0.foo": "bar",
+				"listMap.0.bar": "baz",
+				"listMap.1.baz": "baz",
+
+				"mapRemove.foo": "bar",
+				"mapRemove.bar": "bar",
+
+				"setChange.#":        "1",
+				"setChange.10.index": "10",
+				"setChange.10.value": "50",
 			},
 		},
 	}
@@ -231,7 +251,6 @@ func TestDiffFieldReader(t *testing.T) {
 					"foo": "bar",
 					"bar": "baz",
 				},
-				NegValue: map[string]interface{}{},
 				Exists:   true,
 				Computed: false,
 			},
@@ -256,9 +275,6 @@ func TestDiffFieldReader(t *testing.T) {
 				Value: map[string]interface{}{
 					"foo": "bar",
 				},
-				NegValue: map[string]interface{}{
-					"bar": "",
-				},
 				Exists:   true,
 				Computed: false,
 			},
@@ -312,6 +328,63 @@ func TestDiffFieldReader(t *testing.T) {
 			},
 			false,
 		},
+
+		"listMapRemoval": {
+			[]string{"listMap"},
+			&Schema{
+				Type: TypeList,
+				Elem: &Schema{
+					Type: TypeMap,
+				},
+			},
+			FieldReadResult{
+				Value: []interface{}{
+					map[string]interface{}{
+						"foo": "bar",
+					},
+					map[string]interface{}{
+						"baz": "baz",
+					},
+				},
+				Exists: true,
+			},
+			false,
+		},
+
+		"setChange": {
+			[]string{"setChange"},
+			&Schema{
+				Type:     TypeSet,
+				Optional: true,
+				Elem: &Resource{
+					Schema: map[string]*Schema{
+						"index": &Schema{
+							Type:     TypeInt,
+							Required: true,
+						},
+
+						"value": &Schema{
+							Type:     TypeString,
+							Required: true,
+						},
+					},
+				},
+				Set: func(a interface{}) int {
+					m := a.(map[string]interface{})
+					return m["index"].(int)
+				},
+			},
+			FieldReadResult{
+				Value: []interface{}{
+					map[string]interface{}{
+						"index": 10,
+						"value": "80",
+					},
+				},
+				Exists: true,
+			},
+			false,
+		},
 	}
 
 	for name, tc := range cases {