Merge pull request #986 from hashicorp/b-remove-set

helper/schema: Diff with set going to 0 elements removes it from state

helper/schema/field_reader_diff.go

@@ -144,11 +144,12 @@ func (r *DiffFieldReader) readPrimitive(
 
 func (r *DiffFieldReader) readSet(
 	address []string, schema *Schema) (FieldReadResult, error) {
+	prefix := strings.Join(address, ".") + "."
+
 	// Create the set that will be our result
 	set := &Set{F: schema.Set}
 
 	// Go through the map and find all the set items
-	prefix := strings.Join(address, ".") + "."
 	for k, _ := range r.Diff.Attributes {
 		if !strings.HasPrefix(k, prefix) {
 			continue
@@ -174,8 +175,21 @@ func (r *DiffFieldReader) readSet(
 		set.Add(raw.Value)
 	}
 
+	// Determine if the set "exists". It exists if there are items or if
+	// the diff explicitly wanted it empty.
+	exists := set.Len() > 0
+	if !exists {
+		// We could check if the diff value is "0" here but I think the
+		// existence of "#" on its own is enough to show it existed. This
+		// protects us in the future from the zero value changing from
+		// "0" to "" breaking us (if that were to happen).
+		if _, ok := r.Diff.Attributes[prefix+"#"]; ok {
+			exists = true
+		}
+	}
+
 	return FieldReadResult{
 		Value:  set,
-		Exists: set.Len() > 0,
+		Exists: exists,
 	}, nil
 }

helper/schema/field_reader_diff_test.go

@@ -90,6 +90,28 @@ func TestDiffFieldReader_extra(t *testing.T) {
 				return m["index"].(int)
 			},
 		},
+
+		"setEmpty": &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)
+			},
+		},
 	}
 
 	r := &DiffFieldReader{
@@ -114,6 +136,11 @@ func TestDiffFieldReader_extra(t *testing.T) {
 					Old: "50",
 					New: "80",
 				},
+
+				"setEmpty.#": &terraform.ResourceAttrDiff{
+					Old: "2",
+					New: "0",
+				},
 			},
 		},
 
@@ -131,6 +158,12 @@ func TestDiffFieldReader_extra(t *testing.T) {
 				"setChange.#":        "1",
 				"setChange.10.index": "10",
 				"setChange.10.value": "50",
+
+				"setEmpty.#":        "2",
+				"setEmpty.10.index": "10",
+				"setEmpty.10.value": "50",
+				"setEmpty.20.index": "20",
+				"setEmpty.20.value": "50",
 			}),
 		},
 	}
@@ -191,6 +224,15 @@ func TestDiffFieldReader_extra(t *testing.T) {
 			},
 			false,
 		},
+
+		"setEmpty": {
+			[]string{"setEmpty"},
+			FieldReadResult{
+				Value:  []interface{}{},
+				Exists: true,
+			},
+			false,
+		},
 	}
 
 	for name, tc := range cases {

helper/schema/getsource_string.go

@@ -0,0 +1,36 @@
+// generated by stringer -type=getSource resource_data_get_source.go; DO NOT EDIT
+
+package schema
+
+import "fmt"
+
+const (
+	_getSource_name_0 = "getSourceStategetSourceConfig"
+	_getSource_name_1 = "getSourceDiff"
+	_getSource_name_2 = "getSourceSet"
+	_getSource_name_3 = "getSourceLevelMaskgetSourceExact"
+)
+
+var (
+	_getSource_index_0 = [...]uint8{0, 14, 29}
+	_getSource_index_1 = [...]uint8{0, 13}
+	_getSource_index_2 = [...]uint8{0, 12}
+	_getSource_index_3 = [...]uint8{0, 18, 32}
+)
+
+func (i getSource) String() string {
+	switch {
+	case 1 <= i && i <= 2:
+		i -= 1
+		return _getSource_name_0[_getSource_index_0[i]:_getSource_index_0[i+1]]
+	case i == 4:
+		return _getSource_name_1
+	case i == 8:
+		return _getSource_name_2
+	case 15 <= i && i <= 16:
+		i -= 15
+		return _getSource_name_3[_getSource_index_3[i]:_getSource_index_3[i+1]]
+	default:
+		return fmt.Sprintf("getSource(%d)", i)
+	}
+}

helper/schema/resource_data.go

@@ -32,20 +32,6 @@ type ResourceData struct {
 	once        sync.Once
 }
 
-// getSource represents the level we want to get for a value (internally).
-// Any source less than or equal to the level will be loaded (whichever
-// has a value first).
-type getSource byte
-
-const (
-	getSourceState getSource = 1 << iota
-	getSourceConfig
-	getSourceDiff
-	getSourceSet
-	getSourceExact               // Only get from the _exact_ level
-	getSourceLevelMask getSource = getSourceState | getSourceConfig | getSourceDiff | getSourceSet
-)
-
 // getResult is the internal structure that is generated when a Get
 // is called that contains some extra data that might be used.
 type getResult struct {

helper/schema/resource_data_get_source.go

@@ -0,0 +1,17 @@
+package schema
+
+//go:generate stringer -type=getSource resource_data_get_source.go
+
+// getSource represents the level we want to get for a value (internally).
+// Any source less than or equal to the level will be loaded (whichever
+// has a value first).
+type getSource byte
+
+const (
+	getSourceState getSource = 1 << iota
+	getSourceConfig
+	getSourceDiff
+	getSourceSet
+	getSourceExact               // Only get from the _exact_ level
+	getSourceLevelMask getSource = getSourceState | getSourceConfig | getSourceDiff | getSourceSet
+)

helper/schema/resource_data_test.go

@@ -652,7 +652,6 @@ func TestResourceDataGet(t *testing.T) {
 				},
 			},
 
-
 			Diff: nil,
 
 			Key: "ratio",
@@ -676,7 +675,6 @@ func TestResourceDataGet(t *testing.T) {
 				},
 			},
 
-
 			Diff: &terraform.InstanceDiff{
 				Attributes: map[string]*terraform.ResourceAttrDiff{
 					"ratio": &terraform.ResourceAttrDiff{
@@ -686,7 +684,6 @@ func TestResourceDataGet(t *testing.T) {
 				},
 			},
 
-
 			Key: "ratio",
 
 			Value: 33.0,
@@ -1533,7 +1530,6 @@ func TestResourceDataSet(t *testing.T) {
 			GetKey:   "ratios",
 			GetValue: []interface{}{1.0, 2.2, 5.5},
 		},
-
 	}
 
 	for i, tc := range cases {
@@ -2500,6 +2496,45 @@ func TestResourceDataState(t *testing.T) {
 				},
 			},
 		},
+
+		// #24
+		{
+			Schema: map[string]*Schema{
+				"ports": &Schema{
+					Type:     TypeSet,
+					Optional: true,
+					Computed: true,
+					Elem:     &Schema{Type: TypeInt},
+					Set: func(a interface{}) int {
+						return a.(int)
+					},
+				},
+			},
+
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"ports.#":   "3",
+					"ports.100": "100",
+					"ports.80":  "80",
+					"ports.81":  "81",
+				},
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"ports.#": &terraform.ResourceAttrDiff{
+						Old: "3",
+						New: "0",
+					},
+				},
+			},
+
+			Result: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"ports.#": "0",
+				},
+			},
+		},
 	}
 
 	for i, tc := range cases {