From 9217f5a0630ebd3962e0b2b0a4752375e71beb3e Mon Sep 17 00:00:00 2001 From: Justin Marney Date: Fri, 16 Sep 2016 23:57:33 +0000 Subject: [PATCH 1/3] Unchanged NestedSets are not returned by DiffFieldReader. --- helper/schema/field_reader_diff.go | 10 ++ helper/schema/field_reader_diff_test.go | 127 ++++++++++++++++++++++++ 2 files changed, 137 insertions(+) diff --git a/helper/schema/field_reader_diff.go b/helper/schema/field_reader_diff.go index 661c5687c..7a072fc0e 100644 --- a/helper/schema/field_reader_diff.go +++ b/helper/schema/field_reader_diff.go @@ -186,6 +186,16 @@ func (r *DiffFieldReader) readSet( } } + if !exists { + result, err := r.Source.ReadField(address) + if err != nil { + return FieldReadResult{}, err + } + if result.Exists { + return result, nil + } + } + return FieldReadResult{ Value: set, Exists: exists, diff --git a/helper/schema/field_reader_diff_test.go b/helper/schema/field_reader_diff_test.go index cfd329492..732d9fd49 100644 --- a/helper/schema/field_reader_diff_test.go +++ b/helper/schema/field_reader_diff_test.go @@ -11,6 +11,133 @@ func TestDiffFieldReader_impl(t *testing.T) { var _ FieldReader = new(DiffFieldReader) } +func TestDiffFieldReader_ASDF(t *testing.T) { + schema := map[string]*Schema{ + "list_of_sets_1": &Schema{ + Type: TypeList, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_set": &Schema{ + Type: TypeSet, + Elem: &Resource{ + Schema: map[string]*Schema{ + "val": &Schema{ + Type: TypeInt, + }, + }, + }, + Set: func(a interface{}) int { + m := a.(map[string]interface{}) + return m["val"].(int) + }, + }, + }, + }, + }, + "list_of_sets_2": &Schema{ + Type: TypeList, + Elem: &Resource{ + Schema: map[string]*Schema{ + "nested_set": &Schema{ + Type: TypeSet, + Elem: &Resource{ + Schema: map[string]*Schema{ + "val": &Schema{ + Type: TypeInt, + }, + }, + }, + Set: func(a interface{}) int { + m := a.(map[string]interface{}) + return m["val"].(int) + }, + }, + }, + }, + }, + } + + r := &DiffFieldReader{ + Schema: schema, + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "list_of_sets_1.0.nested_set.1.val": &terraform.ResourceAttrDiff{ + Old: "1", + New: "0", + NewRemoved: true, + }, + "list_of_sets_1.0.nested_set.2.val": &terraform.ResourceAttrDiff{ + New: "2", + }, + }, + }, + } + + r.Source = &MultiLevelFieldReader{ + Readers: map[string]FieldReader{ + "diff": r, + "set": &MapFieldReader{Schema: schema}, + "state": &MapFieldReader{ + Map: &BasicMapReader{ + "list_of_sets_1.#": "1", + "list_of_sets_1.0.nested_set.#": "1", + "list_of_sets_1.0.nested_set.1.val": "1", + "list_of_sets_2.#": "1", + "list_of_sets_2.0.nested_set.#": "1", + "list_of_sets_2.0.nested_set.1.val": "1", + }, + Schema: schema, + }, + }, + Levels: []string{"state", "config"}, + } + + out, err := r.ReadField([]string{"list_of_sets_2"}) + if err != nil { + t.Fatalf("err: %v", err) + } + + list, ok := out.Value.([]interface{}) + if !ok { + t.Fatalf("nestedSetUpdate: bad: Value type\n\nexpected: []interface{}\n\ngot: %v", reflect.TypeOf(out.Value)) + } + if len(list) != 1 { + t.Fatalf("nestedSetUpdate: bad: list length\n\nexpected: 1\n\ngot: %v\n\n", len(list)) + } + + var m map[string]interface{} + m, ok = list[0].(map[string]interface{}) + if !ok { + t.Fatalf("nestedSetUpdate: bad: list item type\n\nexpected: map[string]interface{}\n\ngot: %v\n\n", reflect.TypeOf(list[0])) + } + + rawSet := m["nested_set"] + if rawSet == nil { + t.Fatalf("nestedSetUpdate: bad: nested_set\n\nexpected: not nil\n\ngot: nil\n\n") + } + + var set *Set + set, ok = rawSet.(*Set) + if !ok { + t.Fatalf("nestedSetUpdate: bad: nested set type\n\nexpected: *Set\n\ngot: %v\n\n", reflect.TypeOf(m["nested_set"])) + } + if set.Len() != 1 { + t.Fatalf("nestedSetUpdate: bad: nested set length\n\nexpected: 1\n\ngot: %v\n\n", set.Len()) + } + + setList := set.List() + var item map[string]interface{} + item, ok = setList[0].(map[string]interface{}) + if !ok { + t.Fatalf("nestedSetUpdate: bad: nested set item type\n\nexpected: map[string]interface{}\n\ngot: %v\n\n", reflect.TypeOf(setList[0])) + } + + val := item["val"].(int) + if val != 1 { + t.Fatalf("nestedSetUpdate: bad: val\n\nexpected: 1\n\ngot: %v\n\n", val) + } +} + // https://github.com/hashicorp/terraform/issues/914 func TestDiffFieldReader_MapHandling(t *testing.T) { schema := map[string]*Schema{ From 53e10dfa89b7c6f454f92d910cf5815d33dce411 Mon Sep 17 00:00:00 2001 From: Justin Marney Date: Sat, 17 Sep 2016 16:56:12 +0000 Subject: [PATCH 2/3] Name test appropriately. --- helper/schema/field_reader_diff_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helper/schema/field_reader_diff_test.go b/helper/schema/field_reader_diff_test.go index 732d9fd49..65a0d4496 100644 --- a/helper/schema/field_reader_diff_test.go +++ b/helper/schema/field_reader_diff_test.go @@ -11,7 +11,7 @@ func TestDiffFieldReader_impl(t *testing.T) { var _ FieldReader = new(DiffFieldReader) } -func TestDiffFieldReader_ASDF(t *testing.T) { +func TestDiffFieldReader_NestedSetUpdate(t *testing.T) { schema := map[string]*Schema{ "list_of_sets_1": &Schema{ Type: TypeList, From 895d10a62729722ac1f0fdfb361422a1f554e2df Mon Sep 17 00:00:00 2001 From: Justin Marney Date: Sat, 17 Sep 2016 17:41:44 +0000 Subject: [PATCH 3/3] Use DeepEqual instead of multiple if statements to reduce noise. --- helper/schema/field_reader_diff_test.go | 59 +++++++------------------ 1 file changed, 15 insertions(+), 44 deletions(-) diff --git a/helper/schema/field_reader_diff_test.go b/helper/schema/field_reader_diff_test.go index 65a0d4496..7339dea7a 100644 --- a/helper/schema/field_reader_diff_test.go +++ b/helper/schema/field_reader_diff_test.go @@ -12,6 +12,11 @@ func TestDiffFieldReader_impl(t *testing.T) { } func TestDiffFieldReader_NestedSetUpdate(t *testing.T) { + hashFn := func(a interface{}) int { + m := a.(map[string]interface{}) + return m["val"].(int) + } + schema := map[string]*Schema{ "list_of_sets_1": &Schema{ Type: TypeList, @@ -26,10 +31,7 @@ func TestDiffFieldReader_NestedSetUpdate(t *testing.T) { }, }, }, - Set: func(a interface{}) int { - m := a.(map[string]interface{}) - return m["val"].(int) - }, + Set: hashFn, }, }, }, @@ -47,10 +49,7 @@ func TestDiffFieldReader_NestedSetUpdate(t *testing.T) { }, }, }, - Set: func(a interface{}) int { - m := a.(map[string]interface{}) - return m["val"].(int) - }, + Set: hashFn, }, }, }, @@ -97,44 +96,16 @@ func TestDiffFieldReader_NestedSetUpdate(t *testing.T) { t.Fatalf("err: %v", err) } - list, ok := out.Value.([]interface{}) - if !ok { - t.Fatalf("nestedSetUpdate: bad: Value type\n\nexpected: []interface{}\n\ngot: %v", reflect.TypeOf(out.Value)) - } - if len(list) != 1 { - t.Fatalf("nestedSetUpdate: bad: list length\n\nexpected: 1\n\ngot: %v\n\n", len(list)) - } + s := &Set{F: hashFn} + s.Add(map[string]interface{}{"val": 1}) + expected := s.List() - var m map[string]interface{} - m, ok = list[0].(map[string]interface{}) - if !ok { - t.Fatalf("nestedSetUpdate: bad: list item type\n\nexpected: map[string]interface{}\n\ngot: %v\n\n", reflect.TypeOf(list[0])) - } + l := out.Value.([]interface{}) + i := l[0].(map[string]interface{}) + actual := i["nested_set"].(*Set).List() - rawSet := m["nested_set"] - if rawSet == nil { - t.Fatalf("nestedSetUpdate: bad: nested_set\n\nexpected: not nil\n\ngot: nil\n\n") - } - - var set *Set - set, ok = rawSet.(*Set) - if !ok { - t.Fatalf("nestedSetUpdate: bad: nested set type\n\nexpected: *Set\n\ngot: %v\n\n", reflect.TypeOf(m["nested_set"])) - } - if set.Len() != 1 { - t.Fatalf("nestedSetUpdate: bad: nested set length\n\nexpected: 1\n\ngot: %v\n\n", set.Len()) - } - - setList := set.List() - var item map[string]interface{} - item, ok = setList[0].(map[string]interface{}) - if !ok { - t.Fatalf("nestedSetUpdate: bad: nested set item type\n\nexpected: map[string]interface{}\n\ngot: %v\n\n", reflect.TypeOf(setList[0])) - } - - val := item["val"].(int) - if val != 1 { - t.Fatalf("nestedSetUpdate: bad: val\n\nexpected: 1\n\ngot: %v\n\n", val) + if !reflect.DeepEqual(expected, actual) { + t.Fatalf("bad: NestedSetUpdate\n\nexpected: %#v\n\ngot: %#v\n\n", expected, actual) } }