diff --git a/helper/schema/field_reader_config.go b/helper/schema/field_reader_config.go index 215c0957e..4c63bd07c 100644 --- a/helper/schema/field_reader_config.go +++ b/helper/schema/field_reader_config.go @@ -160,17 +160,17 @@ func (r *ConfigFieldReader) readPrimitive( func (r *ConfigFieldReader) readSet( address []string, schema *Schema) (FieldReadResult, map[int]int, error) { indexMap := make(map[int]int) + // Create the set that will be our result + set := &Set{F: schema.Set} + raw, err := readListField(&nestedConfigFieldReader{r}, address, schema) if err != nil { return FieldReadResult{}, indexMap, err } if !raw.Exists { - return FieldReadResult{}, indexMap, nil + return FieldReadResult{Value: set}, indexMap, nil } - // Create the set that will be our result - set := &Set{F: schema.Set} - // If the list is computed, the set is necessarilly computed if raw.Computed { return FieldReadResult{ diff --git a/helper/schema/field_reader_config_test.go b/helper/schema/field_reader_config_test.go index f194b3fa0..5eecbcdf2 100644 --- a/helper/schema/field_reader_config_test.go +++ b/helper/schema/field_reader_config_test.go @@ -1,7 +1,6 @@ package schema import ( - "reflect" "testing" "github.com/hashicorp/terraform/config" @@ -13,200 +12,38 @@ func TestConfigFieldReader_impl(t *testing.T) { } func TestConfigFieldReader(t *testing.T) { - r := &ConfigFieldReader{ - Schema: map[string]*Schema{ - "bool": &Schema{Type: TypeBool}, - "int": &Schema{Type: TypeInt}, - "string": &Schema{Type: TypeString}, - "list": &Schema{ - Type: TypeList, - Elem: &Schema{Type: TypeString}, - }, - "listInt": &Schema{ - Type: TypeList, - Elem: &Schema{Type: TypeInt}, - }, - "map": &Schema{Type: TypeMap}, - "set": &Schema{ - Type: TypeSet, - Elem: &Schema{Type: TypeInt}, - Set: func(a interface{}) int { - return a.(int) + testFieldReader(t, func(s map[string]*Schema) FieldReader { + return &ConfigFieldReader{ + Schema: s, + + Config: testConfig(t, map[string]interface{}{ + "bool": true, + "int": 42, + "string": "string", + + "list": []interface{}{"foo", "bar"}, + + "listInt": []interface{}{21, 42}, + + "map": map[string]interface{}{ + "foo": "bar", + "bar": "baz", }, - }, - "setDeep": &Schema{ - Type: TypeSet, - Elem: &Resource{ - Schema: map[string]*Schema{ - "index": &Schema{Type: TypeInt}, - "value": &Schema{Type: TypeString}, + + "set": []interface{}{10, 50}, + "setDeep": []interface{}{ + map[string]interface{}{ + "index": 10, + "value": "foo", + }, + map[string]interface{}{ + "index": 50, + "value": "bar", }, }, - Set: func(a interface{}) int { - return a.(map[string]interface{})["index"].(int) - }, - }, - }, - - Config: testConfig(t, map[string]interface{}{ - "bool": true, - "int": 42, - "string": "string", - - "list": []interface{}{"foo", "bar"}, - - "listInt": []interface{}{21, 42}, - - "map": map[string]interface{}{ - "foo": "bar", - "bar": "baz", - }, - - "set": []interface{}{10, 50}, - - "setDeep": []interface{}{ - map[string]interface{}{ - "index": 10, - "value": "foo", - }, - map[string]interface{}{ - "index": 50, - "value": "bar", - }, - }, - }), - } - - cases := map[string]struct { - Addr []string - Out interface{} - OutOk bool - OutComputed bool - OutErr bool - }{ - "noexist": { - []string{"boolNOPE"}, - nil, - false, - false, - false, - }, - - "bool": { - []string{"bool"}, - true, - true, - false, - false, - }, - - "int": { - []string{"int"}, - 42, - true, - false, - false, - }, - - "string": { - []string{"string"}, - "string", - true, - false, - false, - }, - - "list": { - []string{"list"}, - []interface{}{ - "foo", - "bar", - }, - true, - false, - false, - }, - - "listInt": { - []string{"listInt"}, - []interface{}{ - 21, - 42, - }, - true, - false, - false, - }, - - "map": { - []string{"map"}, - map[string]interface{}{ - "foo": "bar", - "bar": "baz", - }, - true, - false, - false, - }, - - "mapelem": { - []string{"map", "foo"}, - "bar", - true, - false, - false, - }, - - "set": { - []string{"set"}, - []interface{}{10, 50}, - true, - false, - false, - }, - - "setDeep": { - []string{"setDeep"}, - []interface{}{ - map[string]interface{}{ - "index": 10, - "value": "foo", - }, - map[string]interface{}{ - "index": 50, - "value": "bar", - }, - }, - true, - false, - false, - }, - } - - for name, tc := range cases { - if name != "list" { - continue + }), } - out, err := r.ReadField(tc.Addr) - if (err != nil) != tc.OutErr { - t.Fatalf("%s: err: %s", name, err) - } - if out.Computed != tc.OutComputed { - t.Fatalf("%s: err: %#v", name, out.Computed) - } - - if s, ok := out.Value.(*Set); ok { - // If it is a set, convert to a list so its more easily checked. - out.Value = s.List() - } - - if !reflect.DeepEqual(out.Value, tc.Out) { - t.Fatalf("%s: out: %#v", name, out.Value) - } - if out.Exists != tc.OutOk { - t.Fatalf("%s: outOk: %#v", name, out.Exists) - } - } + }) } func testConfig( diff --git a/helper/schema/field_reader_diff_test.go b/helper/schema/field_reader_diff_test.go index adcdbe18e..7c8a94ada 100644 --- a/helper/schema/field_reader_diff_test.go +++ b/helper/schema/field_reader_diff_test.go @@ -11,35 +11,19 @@ func TestDiffFieldReader_impl(t *testing.T) { var _ FieldReader = new(DiffFieldReader) } -func TestDiffFieldReader(t *testing.T) { +func TestDiffFieldReader_extra(t *testing.T) { schema := map[string]*Schema{ - "bool": &Schema{Type: TypeBool}, - "int": &Schema{Type: TypeInt}, - "string": &Schema{Type: TypeString}, "stringComputed": &Schema{Type: TypeString}, - "list": &Schema{ - Type: TypeList, - Elem: &Schema{Type: TypeString}, - }, - "listInt": &Schema{ - Type: TypeList, - Elem: &Schema{Type: TypeInt}, - }, + "listMap": &Schema{ Type: TypeList, Elem: &Schema{ Type: TypeMap, }, }, - "map": &Schema{Type: TypeMap}, + "mapRemove": &Schema{Type: TypeMap}, - "set": &Schema{ - Type: TypeSet, - Elem: &Schema{Type: TypeInt}, - Set: func(a interface{}) int { - return a.(int) - }, - }, + "setChange": &Schema{ Type: TypeSet, Optional: true, @@ -61,137 +45,23 @@ func TestDiffFieldReader(t *testing.T) { return m["index"].(int) }, }, - "setDeep": &Schema{ - Type: TypeSet, - Elem: &Resource{ - Schema: map[string]*Schema{ - "index": &Schema{Type: TypeInt}, - "value": &Schema{Type: TypeString}, - }, - }, - Set: func(a interface{}) int { - return a.(map[string]interface{})["index"].(int) - }, - }, - "setEmpty": &Schema{ - Type: TypeSet, - Elem: &Schema{Type: TypeInt}, - Set: func(a interface{}) int { - return a.(int) - }, - }, } r := &DiffFieldReader{ Schema: schema, Diff: &terraform.InstanceDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{ - "bool": &terraform.ResourceAttrDiff{ - Old: "", - New: "true", - }, - - "int": &terraform.ResourceAttrDiff{ - Old: "", - New: "42", - }, - - "string": &terraform.ResourceAttrDiff{ - Old: "", - New: "string", - }, - "stringComputed": &terraform.ResourceAttrDiff{ Old: "foo", New: "bar", NewComputed: true, }, - "list.#": &terraform.ResourceAttrDiff{ - Old: "0", - New: "2", - }, - - "list.0": &terraform.ResourceAttrDiff{ - Old: "", - New: "foo", - }, - - "list.1": &terraform.ResourceAttrDiff{ - Old: "", - New: "bar", - }, - - "listInt.#": &terraform.ResourceAttrDiff{ - Old: "0", - New: "2", - }, - - "listInt.0": &terraform.ResourceAttrDiff{ - Old: "", - New: "21", - }, - - "listInt.1": &terraform.ResourceAttrDiff{ - Old: "", - New: "42", - }, - - "map.foo": &terraform.ResourceAttrDiff{ - Old: "", - New: "bar", - }, - - "map.bar": &terraform.ResourceAttrDiff{ - Old: "", - New: "baz", - }, - - "mapRemove.bar": &terraform.ResourceAttrDiff{ + "listMap.0.bar": &terraform.ResourceAttrDiff{ NewRemoved: true, }, - "set.#": &terraform.ResourceAttrDiff{ - Old: "0", - New: "2", - }, - - "set.10": &terraform.ResourceAttrDiff{ - Old: "", - New: "10", - }, - - "set.50": &terraform.ResourceAttrDiff{ - Old: "", - New: "50", - }, - - "setDeep.#": &terraform.ResourceAttrDiff{ - Old: "0", - New: "2", - }, - - "setDeep.10.index": &terraform.ResourceAttrDiff{ - Old: "", - New: "10", - }, - - "setDeep.10.value": &terraform.ResourceAttrDiff{ - Old: "", - New: "foo", - }, - - "setDeep.50.index": &terraform.ResourceAttrDiff{ - Old: "", - New: "50", - }, - - "setDeep.50.value": &terraform.ResourceAttrDiff{ - Old: "", - New: "bar", - }, - - "listMap.0.bar": &terraform.ResourceAttrDiff{ + "mapRemove.bar": &terraform.ResourceAttrDiff{ NewRemoved: true, }, @@ -225,46 +95,6 @@ func TestDiffFieldReader(t *testing.T) { Result FieldReadResult Err bool }{ - "noexist": { - []string{"boolNOPE"}, - FieldReadResult{ - Value: nil, - Exists: false, - Computed: false, - }, - false, - }, - - "bool": { - []string{"bool"}, - FieldReadResult{ - Value: true, - Exists: true, - Computed: false, - }, - false, - }, - - "int": { - []string{"int"}, - FieldReadResult{ - Value: 42, - Exists: true, - Computed: false, - }, - false, - }, - - "string": { - []string{"string"}, - FieldReadResult{ - Value: "string", - Exists: true, - Computed: false, - }, - false, - }, - "stringComputed": { []string{"stringComputed"}, FieldReadResult{ @@ -275,96 +105,6 @@ func TestDiffFieldReader(t *testing.T) { false, }, - "list": { - []string{"list"}, - FieldReadResult{ - Value: []interface{}{ - "foo", - "bar", - }, - Exists: true, - Computed: false, - }, - false, - }, - - "listInt": { - []string{"listInt"}, - FieldReadResult{ - Value: []interface{}{ - 21, - 42, - }, - Exists: true, - Computed: false, - }, - false, - }, - - "map": { - []string{"map"}, - FieldReadResult{ - Value: map[string]interface{}{ - "foo": "bar", - "bar": "baz", - }, - Exists: true, - Computed: false, - }, - false, - }, - - "mapelem": { - []string{"map", "foo"}, - FieldReadResult{ - Value: "bar", - Exists: true, - Computed: false, - }, - false, - }, - - "mapRemove": { - []string{"mapRemove"}, - FieldReadResult{ - Value: map[string]interface{}{ - "foo": "bar", - }, - Exists: true, - Computed: false, - }, - false, - }, - - "set": { - []string{"set"}, - FieldReadResult{ - Value: []interface{}{10, 50}, - Exists: true, - Computed: false, - }, - false, - }, - - "setDeep": { - []string{"setDeep"}, - FieldReadResult{ - Value: []interface{}{ - map[string]interface{}{ - "index": 10, - "value": "foo", - }, - map[string]interface{}{ - "index": 50, - "value": "bar", - }, - }, - Exists: true, - Computed: false, - }, - false, - }, - "listMapRemoval": { []string{"listMap"}, FieldReadResult{ @@ -381,6 +121,18 @@ func TestDiffFieldReader(t *testing.T) { false, }, + "mapRemove": { + []string{"mapRemove"}, + FieldReadResult{ + Value: map[string]interface{}{ + "foo": "bar", + }, + Exists: true, + Computed: false, + }, + false, + }, + "setChange": { []string{"setChange"}, FieldReadResult{ @@ -394,15 +146,6 @@ func TestDiffFieldReader(t *testing.T) { }, false, }, - - "setEmpty": { - []string{"setEmpty"}, - FieldReadResult{ - Value: []interface{}{}, - Exists: false, - }, - false, - }, } for name, tc := range cases { @@ -419,3 +162,125 @@ func TestDiffFieldReader(t *testing.T) { } } } + +func TestDiffFieldReader(t *testing.T) { + testFieldReader(t, func(s map[string]*Schema) FieldReader { + return &DiffFieldReader{ + Schema: s, + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "bool": &terraform.ResourceAttrDiff{ + Old: "", + New: "true", + }, + + "int": &terraform.ResourceAttrDiff{ + Old: "", + New: "42", + }, + + "string": &terraform.ResourceAttrDiff{ + Old: "", + New: "string", + }, + + "stringComputed": &terraform.ResourceAttrDiff{ + Old: "foo", + New: "bar", + NewComputed: true, + }, + + "list.#": &terraform.ResourceAttrDiff{ + Old: "0", + New: "2", + }, + + "list.0": &terraform.ResourceAttrDiff{ + Old: "", + New: "foo", + }, + + "list.1": &terraform.ResourceAttrDiff{ + Old: "", + New: "bar", + }, + + "listInt.#": &terraform.ResourceAttrDiff{ + Old: "0", + New: "2", + }, + + "listInt.0": &terraform.ResourceAttrDiff{ + Old: "", + New: "21", + }, + + "listInt.1": &terraform.ResourceAttrDiff{ + Old: "", + New: "42", + }, + + "map.foo": &terraform.ResourceAttrDiff{ + Old: "", + New: "bar", + }, + + "map.bar": &terraform.ResourceAttrDiff{ + Old: "", + New: "baz", + }, + + "set.#": &terraform.ResourceAttrDiff{ + Old: "0", + New: "2", + }, + + "set.10": &terraform.ResourceAttrDiff{ + Old: "", + New: "10", + }, + + "set.50": &terraform.ResourceAttrDiff{ + Old: "", + New: "50", + }, + + "setDeep.#": &terraform.ResourceAttrDiff{ + Old: "0", + New: "2", + }, + + "setDeep.10.index": &terraform.ResourceAttrDiff{ + Old: "", + New: "10", + }, + + "setDeep.10.value": &terraform.ResourceAttrDiff{ + Old: "", + New: "foo", + }, + + "setDeep.50.index": &terraform.ResourceAttrDiff{ + Old: "", + New: "50", + }, + + "setDeep.50.value": &terraform.ResourceAttrDiff{ + Old: "", + New: "bar", + }, + }, + }, + + Source: &MapFieldReader{ + Schema: s, + Map: BasicMapReader(map[string]string{ + "listMap.#": "2", + "listMap.0.foo": "bar", + "listMap.0.bar": "baz", + "listMap.1.baz": "baz", + }), + }, + } + }) +} diff --git a/helper/schema/field_reader_map_test.go b/helper/schema/field_reader_map_test.go index 57a02f846..369e2ac0e 100644 --- a/helper/schema/field_reader_map_test.go +++ b/helper/schema/field_reader_map_test.go @@ -10,76 +10,48 @@ func TestMapFieldReader_impl(t *testing.T) { } func TestMapFieldReader(t *testing.T) { + testFieldReader(t, func(s map[string]*Schema) FieldReader { + return &MapFieldReader{ + Schema: s, + + Map: BasicMapReader(map[string]string{ + "bool": "true", + "int": "42", + "string": "string", + + "list.#": "2", + "list.0": "foo", + "list.1": "bar", + + "listInt.#": "2", + "listInt.0": "21", + "listInt.1": "42", + + "map.foo": "bar", + "map.bar": "baz", + + "set.#": "2", + "set.10": "10", + "set.50": "50", + + "setDeep.#": "2", + "setDeep.10.index": "10", + "setDeep.10.value": "foo", + "setDeep.50.index": "50", + "setDeep.50.value": "bar", + }), + } + }) +} + +func TestMapFieldReader_extra(t *testing.T) { r := &MapFieldReader{ Schema: map[string]*Schema{ - "bool": &Schema{Type: TypeBool}, - "int": &Schema{Type: TypeInt}, - "string": &Schema{Type: TypeString}, - "list": &Schema{ - Type: TypeList, - Elem: &Schema{Type: TypeString}, - }, - "listInt": &Schema{ - Type: TypeList, - Elem: &Schema{Type: TypeInt}, - }, - "map": &Schema{Type: TypeMap}, "mapDel": &Schema{Type: TypeMap}, - "set": &Schema{ - Type: TypeSet, - Elem: &Schema{Type: TypeInt}, - Set: func(a interface{}) int { - return a.(int) - }, - }, - "setDeep": &Schema{ - Type: TypeSet, - Elem: &Resource{ - Schema: map[string]*Schema{ - "index": &Schema{Type: TypeInt}, - "value": &Schema{Type: TypeString}, - }, - }, - Set: func(a interface{}) int { - return a.(map[string]interface{})["index"].(int) - }, - }, - "setEmpty": &Schema{ - Type: TypeSet, - Elem: &Schema{Type: TypeInt}, - Set: func(a interface{}) int { - return a.(int) - }, - }, }, Map: BasicMapReader(map[string]string{ - "bool": "true", - "int": "42", - "string": "string", - - "list.#": "2", - "list.0": "foo", - "list.1": "bar", - - "listInt.#": "2", - "listInt.0": "21", - "listInt.1": "42", - - "map.foo": "bar", - "map.bar": "baz", - "mapDel": "", - - "set.#": "2", - "set.10": "10", - "set.50": "50", - - "setDeep.#": "2", - "setDeep.10.index": "10", - "setDeep.10.value": "foo", - "setDeep.50.index": "50", - "setDeep.50.value": "bar", }), } @@ -90,71 +62,6 @@ func TestMapFieldReader(t *testing.T) { OutComputed bool OutErr bool }{ - "noexist": { - []string{"boolNOPE"}, - nil, - false, - false, - false, - }, - - "bool": { - []string{"bool"}, - true, - true, - false, - false, - }, - - "int": { - []string{"int"}, - 42, - true, - false, - false, - }, - - "string": { - []string{"string"}, - "string", - true, - false, - false, - }, - - "list": { - []string{"list"}, - []interface{}{ - "foo", - "bar", - }, - true, - false, - false, - }, - - "listInt": { - []string{"listInt"}, - []interface{}{ - 21, - 42, - }, - true, - false, - false, - }, - - "map": { - []string{"map"}, - map[string]interface{}{ - "foo": "bar", - "bar": "baz", - }, - true, - false, - false, - }, - "mapDel": { []string{"mapDel"}, map[string]interface{}{}, @@ -162,47 +69,6 @@ func TestMapFieldReader(t *testing.T) { false, false, }, - - "mapelem": { - []string{"map", "foo"}, - "bar", - true, - false, - false, - }, - - "set": { - []string{"set"}, - []interface{}{10, 50}, - true, - false, - false, - }, - - "setEmpty": { - []string{"setEmpty"}, - []interface{}{}, - false, - false, - false, - }, - - "setDeep": { - []string{"setDeep"}, - []interface{}{ - map[string]interface{}{ - "index": 10, - "value": "foo", - }, - map[string]interface{}{ - "index": 50, - "value": "bar", - }, - }, - true, - false, - false, - }, } for name, tc := range cases { diff --git a/helper/schema/field_reader_test.go b/helper/schema/field_reader_test.go index 200c9ca62..ebbc3f375 100644 --- a/helper/schema/field_reader_test.go +++ b/helper/schema/field_reader_test.go @@ -182,3 +182,209 @@ func TestAddrToSchema(t *testing.T) { } } } + +// testFieldReader is a helper that should be used to verify that +// a FieldReader behaves properly in all the common cases. +func testFieldReader(t *testing.T, f func(map[string]*Schema) FieldReader) { + schema := map[string]*Schema{ + // Primitives + "bool": &Schema{Type: TypeBool}, + "int": &Schema{Type: TypeInt}, + "string": &Schema{Type: TypeString}, + + // Lists + "list": &Schema{ + Type: TypeList, + Elem: &Schema{Type: TypeString}, + }, + "listInt": &Schema{ + Type: TypeList, + Elem: &Schema{Type: TypeInt}, + }, + "listMap": &Schema{ + Type: TypeList, + Elem: &Schema{ + Type: TypeMap, + }, + }, + + // Maps + "map": &Schema{Type: TypeMap}, + + // Sets + "set": &Schema{ + Type: TypeSet, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, + "setDeep": &Schema{ + Type: TypeSet, + Elem: &Resource{ + Schema: map[string]*Schema{ + "index": &Schema{Type: TypeInt}, + "value": &Schema{Type: TypeString}, + }, + }, + Set: func(a interface{}) int { + return a.(map[string]interface{})["index"].(int) + }, + }, + "setEmpty": &Schema{ + Type: TypeSet, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, + } + + cases := map[string]struct { + Addr []string + Result FieldReadResult + Err bool + }{ + "noexist": { + []string{"boolNOPE"}, + FieldReadResult{ + Value: nil, + Exists: false, + Computed: false, + }, + false, + }, + + "bool": { + []string{"bool"}, + FieldReadResult{ + Value: true, + Exists: true, + Computed: false, + }, + false, + }, + + "int": { + []string{"int"}, + FieldReadResult{ + Value: 42, + Exists: true, + Computed: false, + }, + false, + }, + + "string": { + []string{"string"}, + FieldReadResult{ + Value: "string", + Exists: true, + Computed: false, + }, + false, + }, + + "list": { + []string{"list"}, + FieldReadResult{ + Value: []interface{}{ + "foo", + "bar", + }, + Exists: true, + Computed: false, + }, + false, + }, + + "listInt": { + []string{"listInt"}, + FieldReadResult{ + Value: []interface{}{ + 21, + 42, + }, + Exists: true, + Computed: false, + }, + false, + }, + + "map": { + []string{"map"}, + FieldReadResult{ + Value: map[string]interface{}{ + "foo": "bar", + "bar": "baz", + }, + Exists: true, + Computed: false, + }, + false, + }, + + "mapelem": { + []string{"map", "foo"}, + FieldReadResult{ + Value: "bar", + Exists: true, + Computed: false, + }, + false, + }, + + "set": { + []string{"set"}, + FieldReadResult{ + Value: []interface{}{10, 50}, + Exists: true, + Computed: false, + }, + false, + }, + + "setDeep": { + []string{"setDeep"}, + FieldReadResult{ + Value: []interface{}{ + map[string]interface{}{ + "index": 10, + "value": "foo", + }, + map[string]interface{}{ + "index": 50, + "value": "bar", + }, + }, + Exists: true, + Computed: false, + }, + false, + }, + + "setEmpty": { + []string{"setEmpty"}, + FieldReadResult{ + Value: []interface{}{}, + Exists: false, + }, + false, + }, + } + + for name, tc := range cases { + r := f(schema) + out, err := r.ReadField(tc.Addr) + if (err != nil) != tc.Err { + t.Fatalf("%s: err: %s", name, err) + } + if s, ok := out.Value.(*Set); ok { + // If it is a set, convert to a list so its more easily checked. + out.Value = s.List() + } + if !reflect.DeepEqual(tc.Result, out) { + t.Fatalf("%s: bad: %#v", name, out) + } + } +}