From 2212d6895d8bb0cd0aa36a6c54f24457b6fec155 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 17 Feb 2015 11:38:56 -0800 Subject: [PATCH] helper/schema: diff with set going to 0 elements removes it from state --- helper/schema/field_reader_diff.go | 18 ++++++++- helper/schema/field_reader_diff_test.go | 42 ++++++++++++++++++++ helper/schema/getsource_string.go | 36 +++++++++++++++++ helper/schema/resource_data.go | 14 ------- helper/schema/resource_data_get_source.go | 17 ++++++++ helper/schema/resource_data_test.go | 47 ++++++++++++++++++++--- 6 files changed, 152 insertions(+), 22 deletions(-) create mode 100644 helper/schema/getsource_string.go create mode 100644 helper/schema/resource_data_get_source.go diff --git a/helper/schema/field_reader_diff.go b/helper/schema/field_reader_diff.go index ec875421b..bfac2a0b0 100644 --- a/helper/schema/field_reader_diff.go +++ b/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 } diff --git a/helper/schema/field_reader_diff_test.go b/helper/schema/field_reader_diff_test.go index fbb10fcaf..205b254f4 100644 --- a/helper/schema/field_reader_diff_test.go +++ b/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 { diff --git a/helper/schema/getsource_string.go b/helper/schema/getsource_string.go new file mode 100644 index 000000000..039bb561a --- /dev/null +++ b/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) + } +} diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index 924b9a877..b4feff220 100644 --- a/helper/schema/resource_data.go +++ b/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 { diff --git a/helper/schema/resource_data_get_source.go b/helper/schema/resource_data_get_source.go new file mode 100644 index 000000000..7dd655de3 --- /dev/null +++ b/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 +) diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index f7b211810..ff847298f 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -648,11 +648,10 @@ func TestResourceDataGet(t *testing.T) { State: &terraform.InstanceState{ Attributes: map[string]string{ - "ratio": "0.5", + "ratio": "0.5", }, }, - Diff: nil, Key: "ratio", @@ -672,11 +671,10 @@ func TestResourceDataGet(t *testing.T) { State: &terraform.InstanceState{ Attributes: map[string]string{ - "ratio": "-0.5", + "ratio": "-0.5", }, }, - 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 {