From 56dde5c0c1d2e088345161a32104c25c0f157dac Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2014 17:51:27 -0700 Subject: [PATCH 1/6] helper/schema: can read and get the state of sets --- helper/schema/resource_data.go | 122 +++++++++++++++++++++++++++- helper/schema/resource_data_test.go | 65 +++++++++++++++ helper/schema/schema.go | 25 ++++-- helper/schema/schema_sort.go | 20 ----- helper/schema/schema_sort_test.go | 29 ------- helper/schema/schema_test.go | 8 +- helper/schema/set.go | 58 +++++++++++++ 7 files changed, 262 insertions(+), 65 deletions(-) delete mode 100644 helper/schema/schema_sort.go delete mode 100644 helper/schema/schema_sort_test.go create mode 100644 helper/schema/set.go diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index e34689b063..5a2f0b7103 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -173,11 +173,83 @@ func (d *ResourceData) get( return d.getList(k, parts, schema, source) case TypeMap: return d.getMap(k, parts, schema, source) - default: + case TypeSet: + return d.getSet(k, parts, schema, source) + case TypeBool: + fallthrough + case TypeInt: + fallthrough + case TypeString: return d.getPrimitive(k, parts, schema, source) + default: + panic(fmt.Sprintf("%s: unknown type %s", k, schema.Type)) } } +func (d *ResourceData) getSet( + k string, + parts []string, + schema *Schema, + source getSource) interface{} { + raw := d.getList(k, nil, schema, source) + if raw == nil { + return nil + } + + list := raw.([]interface{}) + if len(list) == 0 { + return nil + } + + // This is a reverse map of hash code => index in config used to + // resolve direct set item lookup for turning into state. Confused? + // Read on... + // + // To create the state (the state* functions), a Get call is done + // with a full key such as "ports.0". The index of a set ("0") doesn't + // make a lot of sense, but we need to deterministically list out + // elements of the set like this. Luckily, same sets have a deterministic + // List() output, so we can use that to look things up. + // + // This mapping makes it so that we can look up the hash code of an + // object back to its index in the REAL config. + var indexMap map[int]int + if len(parts) > 0 { + indexMap = make(map[int]int) + } + + // Build the set from all the items using the given hash code + s := &Set{F: schema.Set} + for i, v := range list { + code := s.add(v) + if indexMap != nil { + indexMap[code] = i + } + } + + // If we're trying to get a specific element, then rewrite the + // index to be just that, then jump direct to getList. + if len(parts) > 0 { + index := parts[0] + indexInt, err := strconv.ParseInt(index, 0, 0) + if err != nil { + return nil + } + + codes := s.listCode() + if int(indexInt) >= len(codes) { + return nil + } + code := codes[indexInt] + realIndex := indexMap[code] + + parts[0] = strconv.FormatInt(int64(realIndex), 10) + return d.getList(k, parts, schema, source) + } + + return s +} + func (d *ResourceData) getMap( k string, parts []string, @@ -241,6 +313,11 @@ func (d *ResourceData) getMap( } } + // If we're requesting a specific element, return that + if len(parts) > 0 { + return result[parts[0]] + } + return result } @@ -657,7 +734,7 @@ func (d *ResourceData) stateObject( func (d *ResourceData) statePrimitive( prefix string, schema *Schema) map[string]string { - v := d.getPrimitive(prefix, nil, schema, getSourceSet) + v := d.Get(prefix) if v == nil { return nil } @@ -679,6 +756,37 @@ func (d *ResourceData) statePrimitive( } } +func (d *ResourceData) stateSet( + prefix string, + schema *Schema) map[string]string { + raw := d.get(prefix, nil, schema, getSourceSet) + if raw == nil { + return nil + } + + set := raw.(*Set) + list := set.List() + result := make(map[string]string) + result[prefix+".#"] = strconv.FormatInt(int64(len(list)), 10) + for i := 0; i < len(list); i++ { + key := fmt.Sprintf("%s.%d", prefix, i) + + var m map[string]string + switch t := schema.Elem.(type) { + case *Resource: + m = d.stateObject(key, t.Schema) + case *Schema: + m = d.stateSingle(key, t) + } + + for k, v := range m { + result[k] = v + } + } + + return result +} + func (d *ResourceData) stateSingle( prefix string, schema *Schema) map[string]string { @@ -687,7 +795,15 @@ func (d *ResourceData) stateSingle( return d.stateList(prefix, schema) case TypeMap: return d.stateMap(prefix, schema) - default: + case TypeSet: + return d.stateSet(prefix, schema) + case TypeBool: + fallthrough + case TypeInt: + fallthrough + case TypeString: return d.statePrimitive(prefix, schema) + default: + panic(fmt.Sprintf("%s: unknown type %s", prefix, schema.Type)) } } diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index 36f2e92eb9..ec00c0d10b 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -444,6 +444,34 @@ func TestResourceDataGet(t *testing.T) { Value: []interface{}{}, }, + + // Sets + { + 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.ResourceState{ + Attributes: map[string]string{ + "ports.#": "1", + "ports.0": "80", + }, + }, + + Diff: nil, + + Key: "ports", + + Value: []interface{}{80}, + }, } for i, tc := range cases { @@ -453,6 +481,10 @@ func TestResourceDataGet(t *testing.T) { } v := d.Get(tc.Key) + if s, ok := v.(*Set); ok { + v = s.List() + } + if !reflect.DeepEqual(v, tc.Value) { t.Fatalf("Bad: %d\n\n%#v", i, v) } @@ -1346,6 +1378,39 @@ func TestResourceDataState(t *testing.T) { }, }, }, + + // Sets + { + 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.ResourceState{ + Attributes: map[string]string{ + "ports.#": "2", + "ports.0": "100", + "ports.1": "80", + }, + }, + + Diff: nil, + + Result: &terraform.ResourceState{ + Attributes: map[string]string{ + "ports.#": "2", + "ports.0": "80", + "ports.1": "100", + }, + }, + }, } for i, tc := range cases { diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 296366d119..ae57e10df0 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -20,6 +20,7 @@ const ( TypeString TypeList TypeMap + TypeSet ) // Schema is used to describe the structure of a value. @@ -43,18 +44,19 @@ type Schema struct { Computed bool ForceNew bool - // The following fields are only set for a TypeList Type. + // The following fields are only set for a TypeList or TypeSet Type. // // Elem must be either a *Schema or a *Resource only if the Type is // TypeList, and represents what the element type is. If it is *Schema, // the element type is just a simple value. If it is *Resource, the // element type is a complex structure, potentially with its own lifecycle. + Elem interface{} + + // The follow fields are only valid for a TypeSet type. // - // Order defines a function to be called to order the elements in the - // list. See SchemaOrderFunc for more info. If Order is set, then any - // access of this list will result in the ordered list. - Elem interface{} - Order SchemaOrderFunc + // Set defines a function to determine the unique ID of an item so that + // a proper set can be built. + Set SchemaSetFunc // ComputedWhen is a set of queries on the configuration. Whenever any // of these things is changed, it will require a recompute (this requires @@ -62,9 +64,9 @@ type Schema struct { ComputedWhen []string } -// SchemaOrderFunc is the function used to compare two elements in a list -// for ordering. It should return a boolean true if a is less than b. -type SchemaOrderFunc func(a, b interface{}) bool +// SchemaSetFunc is a function that must return a unique ID for the given +// element. This unique ID is used to store the element in a hash. +type SchemaSetFunc func(a interface{}) int func (s *Schema) finalizeDiff( d *terraform.ResourceAttrDiff) *terraform.ResourceAttrDiff { @@ -181,6 +183,11 @@ func (m schemaMap) InternalValidate() error { return fmt.Errorf("%s: Elem must be set for lists", k) } + // TODO: test + if v.Set != nil { + return fmt.Errorf("%s: Set can only be set for TypeSet", k) + } + switch t := v.Elem.(type) { case *Resource: if err := t.InternalValidate(); err != nil { diff --git a/helper/schema/schema_sort.go b/helper/schema/schema_sort.go deleted file mode 100644 index d803250c8b..0000000000 --- a/helper/schema/schema_sort.go +++ /dev/null @@ -1,20 +0,0 @@ -package schema - -// listSort implements sort.Interface to sort a list of []interface according -// to a schema. -type listSort struct { - List []interface{} - Schema *Schema -} - -func (s *listSort) Len() int { - return len(s.List) -} - -func (s *listSort) Less(i, j int) bool { - return s.Schema.Order(s.List[i], s.List[j]) -} - -func (s *listSort) Swap(i, j int) { - s.List[i], s.List[j] = s.List[j], s.List[i] -} diff --git a/helper/schema/schema_sort_test.go b/helper/schema/schema_sort_test.go deleted file mode 100644 index 6a86f338d2..0000000000 --- a/helper/schema/schema_sort_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package schema - -import ( - "reflect" - "sort" - "testing" -) - -func TestListSort_impl(t *testing.T) { - var _ sort.Interface = new(listSort) -} - -func TestListSort(t *testing.T) { - s := &listSort{ - List: []interface{}{5, 2, 1, 3, 4}, - Schema: &Schema{ - Order: func(a, b interface{}) bool { - return a.(int) < b.(int) - }, - }, - } - - sort.Sort(s) - - expected := []interface{}{1, 2, 3, 4, 5} - if !reflect.DeepEqual(s.List, expected) { - t.Fatalf("bad: %#v", s.List) - } -} diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 0b60f43057..3215081aed 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -310,7 +310,7 @@ func TestSchemaMap_Diff(t *testing.T) { Diff: &terraform.ResourceDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{ "ports.#": &terraform.ResourceAttrDiff{ - Old: "", + Old: "", NewComputed: true, }, }, @@ -323,11 +323,11 @@ func TestSchemaMap_Diff(t *testing.T) { { Schema: map[string]*Schema{ "ports": &Schema{ - Type: TypeList, + Type: TypeSet, Required: true, Elem: &Schema{Type: TypeInt}, - Order: func(a, b interface{}) bool { - return a.(int) < b.(int) + Set: func(a interface{}) int { + return a.(int) }, }, }, diff --git a/helper/schema/set.go b/helper/schema/set.go new file mode 100644 index 0000000000..479ff3db96 --- /dev/null +++ b/helper/schema/set.go @@ -0,0 +1,58 @@ +package schema + +import ( + "sort" + "sync" +) + +// Set is a set data structure that is returned for elements of type +// TypeSet. +type Set struct { + F SchemaSetFunc + + m map[int]interface{} + once sync.Once +} + +// Add adds an item to the set if it isn't already in the set. +func (s *Set) Add(item interface{}) { + s.add(item) +} + +// List returns the elements of this set in slice format. +// +// The order of the returned elements is deterministic. Given the same +// set, the order of this will always be the same. +func (s *Set) List() []interface{}{ + result := make([]interface{}, len(s.m)) + for i, k := range s.listCode() { + result[i] = s.m[k] + } + + return result +} + +func (s *Set) init() { + s.m = make(map[int]interface{}) +} + +func (s *Set) add(item interface{}) int { + s.once.Do(s.init) + + code := s.F(item) + if _, ok := s.m[code]; !ok { + s.m[code] = item + } + + return code +} + +func (s *Set) listCode() []int{ + // Sort the hash codes so the order of the list is deterministic + keys := make([]int, 0, len(s.m)) + for k, _ := range s.m { + keys = append(keys, k) + } + sort.Sort(sort.IntSlice(keys)) + return keys +} From 9fe21f0423acf916e39cb02d1496f5124d2b2cdb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2014 17:52:01 -0700 Subject: [PATCH 2/6] helper/schema: verify that sets remove duplicates --- helper/schema/resource_data_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index ec00c0d10b..b311dd78c5 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -1395,9 +1395,10 @@ func TestResourceDataState(t *testing.T) { State: &terraform.ResourceState{ Attributes: map[string]string{ - "ports.#": "2", + "ports.#": "3", "ports.0": "100", "ports.1": "80", + "ports.2": "80", }, }, From ca18e971d13ea5432faf4adbee7e80d93c45d39b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2014 18:11:14 -0700 Subject: [PATCH 3/6] helper/schema: can set sets --- helper/schema/resource_data.go | 26 +++++++- helper/schema/resource_data_test.go | 98 +++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index 5a2f0b7103..4b92173056 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -480,8 +480,16 @@ func (d *ResourceData) set( return d.setList(k, parts, schema, value) case TypeMap: return d.setMapValue(k, parts, schema, value) - default: + case TypeSet: + return d.setSet(k, parts, schema, value) + case TypeBool: + fallthrough + case TypeInt: + fallthrough + case TypeString: return d.setPrimitive(k, schema, value) + default: + panic(fmt.Sprintf("%s: unknown type %s", k, schema.Type)) } } @@ -661,6 +669,22 @@ func (d *ResourceData) setPrimitive( return nil } +func (d *ResourceData) setSet( + k string, + parts []string, + schema *Schema, + value interface{}) error { + if len(parts) > 0 { + return fmt.Errorf("%s: can only set the full set, not elements", k) + } + + if s, ok := value.(*Set); ok { + value = s.List() + } + + return d.setList(k, nil, schema, value) +} + func (d *ResourceData) stateList( prefix string, schema *Schema) map[string]string { diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index b311dd78c5..072e8d7d80 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -1055,6 +1055,101 @@ func TestResourceDataSet(t *testing.T) { }, }, }, + + // Set, with list + { + 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.ResourceState{ + Attributes: map[string]string{ + "ports.#": "3", + "ports.0": "100", + "ports.1": "80", + "ports.2": "80", + }, + }, + + Key: "ports", + Value: []interface{}{100, 125, 125}, + + GetKey: "ports", + GetValue: []interface{}{100, 125}, + }, + + // Set, with Set + { + 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.ResourceState{ + Attributes: map[string]string{ + "ports.#": "3", + "ports.0": "100", + "ports.1": "80", + "ports.2": "80", + }, + }, + + Key: "ports", + Value: &Set{ + m: map[int]interface{}{ + 1: 1, + 2: 2, + }, + }, + + GetKey: "ports", + GetValue: []interface{}{1, 2}, + }, + + // Set single item + { + 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.ResourceState{ + Attributes: map[string]string{ + "ports.#": "2", + "ports.0": "100", + "ports.1": "80", + }, + }, + + Key: "ports.0", + Value: 256, + Err: true, + + GetKey: "ports", + GetValue: []interface{}{80, 100}, + }, } for i, tc := range cases { @@ -1069,6 +1164,9 @@ func TestResourceDataSet(t *testing.T) { } v := d.Get(tc.GetKey) + if s, ok := v.(*Set); ok { + v = s.List() + } if !reflect.DeepEqual(v, tc.GetValue) { t.Fatalf("Get Bad: %d\n\n%#v", i, v) } From 475528adc3834bce78671e64af3adba44b118c4a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2014 18:30:28 -0700 Subject: [PATCH 4/6] helper/schema: Set operations --- helper/schema/schema.go | 10 +++++ helper/schema/schema_test.go | 4 +- helper/schema/set.go | 54 ++++++++++++++++++++++++-- helper/schema/set_test.go | 74 ++++++++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 helper/schema/set_test.go diff --git a/helper/schema/schema.go b/helper/schema/schema.go index ae57e10df0..cff662164d 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -223,6 +223,8 @@ func (m schemaMap) diff( err = m.diffList(k, schema, diff, d) case TypeMap: err = m.diffMap(k, schema, diff, d) + case TypeSet: + err = m.diffSet(k, schema, diff, d) default: err = fmt.Errorf("%s: unknown type %s", k, schema.Type) } @@ -348,6 +350,14 @@ func (m schemaMap) diffMap( return nil } +func (m schemaMap) diffSet( + k string, + schema *Schema, + diff *terraform.ResourceDiff, + d *ResourceData) error { + return nil +} + func (m schemaMap) diffString( k string, schema *Schema, diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 3215081aed..70ef1ddb4b 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -320,6 +320,9 @@ func TestSchemaMap_Diff(t *testing.T) { }, /* + * Set + */ + { Schema: map[string]*Schema{ "ports": &Schema{ @@ -361,7 +364,6 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, - */ /* * List of structure decode diff --git a/helper/schema/set.go b/helper/schema/set.go index 479ff3db96..73b6da8a34 100644 --- a/helper/schema/set.go +++ b/helper/schema/set.go @@ -8,9 +8,9 @@ import ( // Set is a set data structure that is returned for elements of type // TypeSet. type Set struct { - F SchemaSetFunc + F SchemaSetFunc - m map[int]interface{} + m map[int]interface{} once sync.Once } @@ -23,7 +23,7 @@ func (s *Set) Add(item interface{}) { // // The order of the returned elements is deterministic. Given the same // set, the order of this will always be the same. -func (s *Set) List() []interface{}{ +func (s *Set) List() []interface{} { result := make([]interface{}, len(s.m)) for i, k := range s.listCode() { result[i] = s.m[k] @@ -32,6 +32,52 @@ func (s *Set) List() []interface{}{ return result } +// Differences performs a set difference of the two sets, returning +// a new third set that has only the elements unique to this set. +func (s *Set) Difference(other *Set) *Set { + result := &Set{F: s.F} + result.init() + + for k, v := range s.m { + if _, ok := other.m[k]; !ok { + result.m[k] = v + } + } + + return result +} + +// Intersection performs the set intersection of the two sets +// and returns a new third set. +func (s *Set) Intersection(other *Set) *Set { + result := &Set{F: s.F} + result.init() + + for k, v := range s.m { + if _, ok := other.m[k]; ok { + result.m[k] = v + } + } + + return result +} + +// Union performs the set union of the two sets and returns a new third +// set. +func (s *Set) Union(other *Set) *Set { + result := &Set{F: s.F} + result.init() + + for k, v := range s.m { + result.m[k] = v + } + for k, v := range other.m { + result.m[k] = v + } + + return result +} + func (s *Set) init() { s.m = make(map[int]interface{}) } @@ -47,7 +93,7 @@ func (s *Set) add(item interface{}) int { return code } -func (s *Set) listCode() []int{ +func (s *Set) listCode() []int { // Sort the hash codes so the order of the list is deterministic keys := make([]int, 0, len(s.m)) for k, _ := range s.m { diff --git a/helper/schema/set_test.go b/helper/schema/set_test.go new file mode 100644 index 0000000000..88b1ebbfe4 --- /dev/null +++ b/helper/schema/set_test.go @@ -0,0 +1,74 @@ +package schema + +import ( + "reflect" + "testing" +) + +func TestSetAdd(t *testing.T) { + s := &Set{F: testSetInt} + s.Add(1) + s.Add(5) + s.Add(25) + + expected := []interface{}{1, 5, 25} + actual := s.List() + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("bad: %#v", actual) + } +} + +func TestSetDifference(t *testing.T) { + s1 := &Set{F: testSetInt} + s2:= &Set{F: testSetInt} + + s1.Add(1) + s1.Add(5) + + s2.Add(5) + s2.Add(25) + + expected := []interface{}{1} + actual := s1.Difference(s2).List() + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("bad: %#v", actual) + } +} + +func TestSetIntersection(t *testing.T) { + s1 := &Set{F: testSetInt} + s2:= &Set{F: testSetInt} + + s1.Add(1) + s1.Add(5) + + s2.Add(5) + s2.Add(25) + + expected := []interface{}{5} + actual := s1.Intersection(s2).List() + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("bad: %#v", actual) + } +} + +func TestSetUnion(t *testing.T) { + s1 := &Set{F: testSetInt} + s2:= &Set{F: testSetInt} + + s1.Add(1) + s1.Add(5) + + s2.Add(5) + s2.Add(25) + + expected := []interface{}{1, 5, 25} + actual := s1.Union(s2).List() + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("bad: %#v", actual) + } +} + +func testSetInt(v interface{}) int { + return v.(int) +} From 9ab5577beb6b44f73346635b1d54ec6a75618428 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2014 21:02:42 -0700 Subject: [PATCH 5/6] helper/schema: set diff tests --- helper/schema/resource_data.go | 23 +++++++--- helper/schema/schema.go | 18 ++++++-- helper/schema/schema_test.go | 82 ++++++++++++++++++++++++++++++++++ helper/schema/set.go | 15 +++++++ helper/schema/set_test.go | 12 +++++ 5 files changed, 141 insertions(+), 9 deletions(-) diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index 4b92173056..ccb1183f94 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -153,13 +153,14 @@ func (d *ResourceData) getChange( key string, oldLevel getSource, newLevel getSource) (interface{}, interface{}) { - var parts []string + var parts, parts2 []string if key != "" { parts = strings.Split(key, ".") + parts2 = strings.Split(key, ".") } o := d.getObject("", parts, d.schema, oldLevel) - n := d.getObject("", parts, d.schema, newLevel) + n := d.getObject("", parts2, d.schema, newLevel) return o, n } @@ -191,14 +192,23 @@ func (d *ResourceData) getSet( parts []string, schema *Schema, source getSource) interface{} { + s := &Set{F: schema.Set} raw := d.getList(k, nil, schema, source) if raw == nil { - return nil + if len(parts) > 0 { + return d.getList(k, parts, schema, source) + } + + return s } list := raw.([]interface{}) if len(list) == 0 { - return nil + if len(parts) > 0 { + return d.getList(k, parts, schema, source) + } + + return s } // This is a reverse map of hash code => index in config used to @@ -219,7 +229,6 @@ func (d *ResourceData) getSet( } // Build the set from all the items using the given hash code - s := &Set{F: schema.Set} for i, v := range list { code := s.add(v) if indexMap != nil { @@ -413,11 +422,13 @@ func (d *ResourceData) getPrimitive( if err := mapstructure.WeakDecode(v, &result); err != nil { panic(err) } + + resultSet = true } else { result = "" + resultSet = false } - resultSet = true } if d.diff != nil && source >= getSourceDiff { diff --git a/helper/schema/schema.go b/helper/schema/schema.go index cff662164d..69c12759cb 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -238,6 +238,12 @@ func (m schemaMap) diffList( diff *terraform.ResourceDiff, d *ResourceData) error { o, n, _ := d.diffChange(k) + if s, ok := o.(*Set); ok { + o = s.List() + } + if s, ok := n.(*Set); ok { + n = s.List() + } os := o.([]interface{}) vs := n.([]interface{}) @@ -355,7 +361,7 @@ func (m schemaMap) diffSet( schema *Schema, diff *terraform.ResourceDiff, d *ResourceData) error { - return nil + return m.diffList(k, schema, diff, d) } func (m schemaMap) diffString( @@ -380,9 +386,15 @@ func (m schemaMap) diffString( } } + removed := false + if o != nil && n == nil { + removed = true + } + diff.Attributes[k] = schema.finalizeDiff(&terraform.ResourceAttrDiff{ - Old: os, - New: ns, + Old: os, + New: ns, + NewRemoved: removed, }) return nil diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 70ef1ddb4b..36ca3bb544 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -365,6 +365,88 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, + { + Schema: map[string]*Schema{ + "ports": &Schema{ + Type: TypeSet, + Required: true, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, + }, + + State: &terraform.ResourceState{ + Attributes: map[string]string{ + "ports.#": "2", + "ports.0": "2", + "ports.1": "1", + }, + }, + + Config: map[string]interface{}{ + "ports": []interface{}{5, 2, 1}, + }, + + Diff: &terraform.ResourceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "ports.#": &terraform.ResourceAttrDiff{ + Old: "2", + New: "3", + }, + "ports.2": &terraform.ResourceAttrDiff{ + Old: "", + New: "5", + }, + }, + }, + + Err: false, + }, + + { + Schema: map[string]*Schema{ + "ports": &Schema{ + Type: TypeSet, + Required: true, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, + }, + + State: &terraform.ResourceState{ + Attributes: map[string]string{ + "ports.#": "2", + "ports.0": "2", + "ports.1": "1", + }, + }, + + Config: map[string]interface{}{}, + + Diff: &terraform.ResourceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "ports.#": &terraform.ResourceAttrDiff{ + Old: "2", + New: "0", + }, + "ports.0": &terraform.ResourceAttrDiff{ + Old: "1", + NewRemoved: true, + }, + "ports.1": &terraform.ResourceAttrDiff{ + Old: "2", + NewRemoved: true, + }, + }, + }, + + Err: false, + }, + /* * List of structure decode */ diff --git a/helper/schema/set.go b/helper/schema/set.go index 73b6da8a34..21e329f22d 100644 --- a/helper/schema/set.go +++ b/helper/schema/set.go @@ -19,6 +19,17 @@ func (s *Set) Add(item interface{}) { s.add(item) } +// Contains checks if the set has the given item. +func (s *Set) Contains(item interface{}) bool { + _, ok := s.m[s.F(item)] + return ok +} + +// Len returns the amount of items in the set. +func (s *Set) Len() int { + return len(s.m) +} + // List returns the elements of this set in slice format. // // The order of the returned elements is deterministic. Given the same @@ -93,6 +104,10 @@ func (s *Set) add(item interface{}) int { return code } +func (s *Set) index(item interface{}) int { + return sort.SearchInts(s.listCode(), s.F(item)) +} + func (s *Set) listCode() []int { // Sort the hash codes so the order of the list is deterministic keys := make([]int, 0, len(s.m)) diff --git a/helper/schema/set_test.go b/helper/schema/set_test.go index 88b1ebbfe4..7a5778af41 100644 --- a/helper/schema/set_test.go +++ b/helper/schema/set_test.go @@ -18,6 +18,18 @@ func TestSetAdd(t *testing.T) { } } +func TestSetContains(t *testing.T) { + s := &Set{F: testSetInt} + s.Add(5) + + if s.Contains(2) { + t.Fatal("should not contain") + } + if !s.Contains(5) { + t.Fatal("should contain") + } +} + func TestSetDifference(t *testing.T) { s1 := &Set{F: testSetInt} s2:= &Set{F: testSetInt} From e9cc09a8862897013d4baccb53961b28d89fe218 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 20 Aug 2014 21:13:18 -0700 Subject: [PATCH 6/6] helper/schema: improve InternalValidate for sets --- helper/schema/schema.go | 7 ++++--- helper/schema/schema_test.go | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 69c12759cb..11906e373e 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -178,14 +178,15 @@ func (m schemaMap) InternalValidate() error { return fmt.Errorf("%s: ComputedWhen can only be set with Computed", k) } - if v.Type == TypeList { + if v.Type == TypeList || v.Type == TypeSet { if v.Elem == nil { return fmt.Errorf("%s: Elem must be set for lists", k) } - // TODO: test - if v.Set != nil { + if v.Type == TypeList && v.Set != nil { return fmt.Errorf("%s: Set can only be set for TypeSet", k) + } else if v.Type == TypeSet && v.Set == nil { + return fmt.Errorf("%s: Set must be set", k) } switch t := v.Elem.(type) { diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 36ca3bb544..ee06514fec 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -784,6 +784,29 @@ func TestSchemaMap_InternalValidate(t *testing.T) { true, }, + // List element with Set set + { + map[string]*Schema{ + "foo": &Schema{ + Type: TypeList, + Elem: &Schema{Type: TypeInt}, + Set: func(interface{}) int { return 0 }, + }, + }, + true, + }, + + // Set element with no Set set + { + map[string]*Schema{ + "foo": &Schema{ + Type: TypeSet, + Elem: &Schema{Type: TypeInt}, + }, + }, + true, + }, + // Required but computed { map[string]*Schema{