helper/schema: Default hashing function for sets

A common issue with new resource implementations is not considering parts
of a complex structure that's used inside a set, which causes quirky
behavior.

The schema helper has enough information to provide a default reasonable
implementation of a set function that includes all non-computed attributes
in a deterministic way. Here we implement such a function and use it
when no explicit hashing function is provided.

In order to achieve this we encapsulate the construction of the zero
value for a schema in a new method schema.ZeroValue, which allows us to
put the fallback logic to the new default function in a single spot.
It is no longer valid to use &Set{F: schema.Set} and all uses of that
construct should be replaced with schema.ZeroValue().(*Set) .

helper/schema/field_reader.go

@@ -38,15 +38,7 @@ func (r *FieldReadResult) ValueOrZero(s *Schema) interface{} {
 		return r.Value
 	}
 
-	result := s.Type.Zero()
-
-	// The zero value of a set is nil, but we want it
-	// to actually be an empty set object...
-	if set, ok := result.(*Set); ok && set.F == nil {
-		set.F = s.Set
-	}
-
-	return result
+	return s.ZeroValue()
 }
 
 // addrToSchema finds the final element schema for the given address

helper/schema/field_reader_config.go

@@ -201,7 +201,7 @@ 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}
+	set := schema.ZeroValue().(*Set)
 
 	raw, err := readListField(&nestedConfigFieldReader{r}, address, schema)
 	if err != nil {

helper/schema/field_reader_diff.go

@@ -141,7 +141,7 @@ func (r *DiffFieldReader) readSet(
 	prefix := strings.Join(address, ".") + "."
 
 	// Create the set that will be our result
-	set := &Set{F: schema.Set}
+	set := schema.ZeroValue().(*Set)
 
 	// Go through the map and find all the set items
 	for k, d := range r.Diff.Attributes {

helper/schema/field_reader_map.go

@@ -105,7 +105,7 @@ func (r *MapFieldReader) readSet(
 	}
 
 	// Create the set that will be our result
-	set := &Set{F: schema.Set}
+	set := schema.ZeroValue().(*Set)
 
 	// If we have an empty list, then return an empty list
 	if countRaw.Computed || countRaw.Value.(int) == 0 {

helper/schema/schema.go

@@ -207,6 +207,30 @@ func (s *Schema) DefaultValue() (interface{}, error) {
 	return nil, nil
 }
 
+// Returns a zero value for the schema.
+func (s *Schema) ZeroValue() interface{} {
+	// If it's a set then we'll do a bit of extra work to provide the
+	// right hashing function in our empty value.
+	if s.Type == TypeSet {
+		setFunc := s.Set
+		if setFunc == nil {
+			// Default set function uses the schema to hash the whole value
+			elem := s.Elem
+			switch t := elem.(type) {
+			case *Schema:
+				setFunc = HashSchema(t)
+			case *Resource:
+				setFunc = HashResource(t)
+			default:
+				panic("invalid set element type")
+			}
+		}
+		return &Set{F: setFunc}
+	} else {
+		return s.Type.Zero()
+	}
+}
+
 func (s *Schema) finalizeDiff(
 	d *terraform.ResourceAttrDiff) *terraform.ResourceAttrDiff {
 	if d == nil {
@@ -496,10 +520,8 @@ func (m schemaMap) InternalValidate(topSchemaMap schemaMap) error {
 				return fmt.Errorf("%s: Default is not valid for lists or sets", k)
 			}
 
-			if v.Type == TypeList && v.Set != nil {
+			if v.Type != TypeSet && 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) {
@@ -782,10 +804,10 @@ func (m schemaMap) diffSet(
 	}
 
 	if o == nil {
-		o = &Set{F: schema.Set}
+		o = schema.ZeroValue().(*Set)
 	}
 	if n == nil {
-		n = &Set{F: schema.Set}
+		n = schema.ZeroValue().(*Set)
 	}
 	os := o.(*Set)
 	ns := n.(*Set)

helper/schema/schema_test.go

@@ -2789,7 +2789,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) {
 					Optional: true,
 				},
 			},
-			true,
+			false,
 		},
 
 		// Required but computed

helper/schema/serialize.go

@@ -0,0 +1,105 @@
+package schema
+
+import (
+	"bytes"
+	"sort"
+	"strconv"
+)
+
+func SerializeValueForHash(buf *bytes.Buffer, val interface{}, schema *Schema) {
+	if val == nil {
+		buf.WriteRune(';')
+		return
+	}
+
+	switch schema.Type {
+	case TypeBool:
+		if val.(bool) {
+			buf.WriteRune('1')
+		} else {
+			buf.WriteRune('0')
+		}
+	case TypeInt:
+		buf.WriteString(strconv.Itoa(val.(int)))
+	case TypeFloat:
+		buf.WriteString(strconv.FormatFloat(val.(float64), 'g', -1, 64))
+	case TypeString:
+		buf.WriteString(val.(string))
+	case TypeList:
+		buf.WriteRune('(')
+		l := val.([]interface{})
+		for _, innerVal := range l {
+			serializeCollectionMemberForHash(buf, innerVal, schema.Elem)
+		}
+		buf.WriteRune(')')
+	case TypeMap:
+		m := val.(map[string]interface{})
+		var keys []string
+		for k := range m {
+			keys = append(keys, k)
+		}
+		sort.Strings(keys)
+		buf.WriteRune('[')
+		for _, k := range keys {
+			innerVal := m[k]
+			buf.WriteString(k)
+			buf.WriteRune(':')
+			serializeCollectionMemberForHash(buf, innerVal, schema.Elem)
+		}
+		buf.WriteRune(']')
+	case TypeSet:
+		buf.WriteRune('{')
+		s := val.(*Set)
+		for _, innerVal := range s.List() {
+			serializeCollectionMemberForHash(buf, innerVal, schema.Elem)
+		}
+		buf.WriteRune('}')
+	default:
+		panic("unknown schema type to serialize")
+	}
+	buf.WriteRune(';')
+}
+
+// SerializeValueForHash appends a serialization of the given resource config
+// to the given buffer, guaranteeing deterministic results given the same value
+// and schema.
+//
+// Its primary purpose is as input into a hashing function in order
+// to hash complex substructures when used in sets, and so the serialization
+// is not reversible.
+func SerializeResourceForHash(buf *bytes.Buffer, val interface{}, resource *Resource) {
+	sm := resource.Schema
+	m := val.(map[string]interface{})
+	var keys []string
+	for k := range sm {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	for _, k := range keys {
+		innerSchema := sm[k]
+		// Skip attributes that are not user-provided. Computed attributes
+		// do not contribute to the hash since their ultimate value cannot
+		// be known at plan/diff time.
+		if !(innerSchema.Required || innerSchema.Optional) {
+			continue
+		}
+
+		buf.WriteString(k)
+		buf.WriteRune(':')
+		innerVal := m[k]
+		SerializeValueForHash(buf, innerVal, innerSchema)
+	}
+}
+
+func serializeCollectionMemberForHash(buf *bytes.Buffer, val interface{}, elem interface{}) {
+	switch tElem := elem.(type) {
+	case *Schema:
+		SerializeValueForHash(buf, val, tElem)
+	case *Resource:
+		buf.WriteRune('<')
+		SerializeResourceForHash(buf, val, tElem)
+		buf.WriteString(">;")
+	default:
+		panic("invalid element type")
+	}
+}

helper/schema/serialize_test.go

@@ -0,0 +1,214 @@
+package schema
+
+import (
+	"bytes"
+	"testing"
+)
+
+func TestSerializeForHash(t *testing.T) {
+	type testCase struct {
+		Schema   interface{}
+		Value    interface{}
+		Expected string
+	}
+
+	tests := []testCase{
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeInt,
+			},
+			Value:    0,
+			Expected: "0;",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeInt,
+			},
+			Value:    200,
+			Expected: "200;",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeBool,
+			},
+			Value:    true,
+			Expected: "1;",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeBool,
+			},
+			Value:    false,
+			Expected: "0;",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeFloat,
+			},
+			Value:    1.0,
+			Expected: "1;",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeFloat,
+			},
+			Value:    1.54,
+			Expected: "1.54;",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeFloat,
+			},
+			Value:    0.1,
+			Expected: "0.1;",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeString,
+			},
+			Value:    "hello",
+			Expected: "hello;",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeString,
+			},
+			Value:    "1",
+			Expected: "1;",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeList,
+				Elem: &Schema{
+					Type: TypeString,
+				},
+			},
+			Value:    []interface{}{},
+			Expected: "();",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeList,
+				Elem: &Schema{
+					Type: TypeString,
+				},
+			},
+			Value:    []interface{}{"hello", "world"},
+			Expected: "(hello;world;);",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeList,
+				Elem: &Resource{
+					Schema: map[string]*Schema{
+						"fo": &Schema{
+							Type:     TypeString,
+							Required: true,
+						},
+						"fum": &Schema{
+							Type:     TypeString,
+							Required: true,
+						},
+					},
+				},
+			},
+			Value: []interface{}{
+				map[string]interface{}{
+					"fo": "bar",
+				},
+				map[string]interface{}{
+					"fo":  "baz",
+					"fum": "boz",
+				},
+			},
+			Expected: "(<fo:bar;fum:;>;<fo:baz;fum:boz;>;);",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeSet,
+				Elem: &Schema{
+					Type: TypeString,
+				},
+			},
+			Value: NewSet(func(i interface{}) int { return len(i.(string)) }, []interface{}{
+				"hello",
+				"woo",
+			}),
+			Expected: "{woo;hello;};",
+		},
+
+		testCase{
+			Schema: &Schema{
+				Type: TypeMap,
+				Elem: &Schema{
+					Type: TypeString,
+				},
+			},
+			Value: map[string]interface{}{
+				"foo": "bar",
+				"baz": "foo",
+			},
+			Expected: "[baz:foo;foo:bar;];",
+		},
+
+		testCase{
+			Schema: &Resource{
+				Schema: map[string]*Schema{
+					"name": &Schema{
+						Type:     TypeString,
+						Required: true,
+					},
+					"size": &Schema{
+						Type:     TypeInt,
+						Optional: true,
+					},
+					"green": &Schema{
+						Type:     TypeBool,
+						Optional: true,
+						Computed: true,
+					},
+					"upside_down": &Schema{
+						Type:     TypeBool,
+						Computed: true,
+					},
+				},
+			},
+			Value: map[string]interface{}{
+				"name":  "my-fun-database",
+				"size":  12,
+				"green": true,
+			},
+			Expected: "green:1;name:my-fun-database;size:12;",
+		},
+	}
+
+	for _, test := range tests {
+		var gotBuf bytes.Buffer
+		schema := test.Schema
+
+		switch s := schema.(type) {
+		case *Schema:
+			SerializeValueForHash(&gotBuf, test.Value, s)
+		case *Resource:
+			SerializeResourceForHash(&gotBuf, test.Value, s)
+		}
+
+		got := gotBuf.String()
+		if got != test.Expected {
+			t.Errorf("hash(%#v) got %#v, but want %#v", test.Value, got, test.Expected)
+		}
+	}
+}

helper/schema/set.go

@@ -1,6 +1,7 @@
 package schema
 
 import (
+	"bytes"
 	"fmt"
 	"reflect"
 	"sort"
@@ -15,6 +16,28 @@ func HashString(v interface{}) int {
 	return hashcode.String(v.(string))
 }
 
+// HashResource hashes complex structures that are described using
+// a *Resource. This is the default set implementation used when a set's
+// element type is a full resource.
+func HashResource(resource *Resource) SchemaSetFunc {
+	return func(v interface{}) int {
+		var buf bytes.Buffer
+		SerializeResourceForHash(&buf, v, resource)
+		return hashcode.String(buf.String())
+	}
+}
+
+// HashSchema hashes values that are described using a *Schema. This is the
+// default set implementation used when a set's element type is a single
+// schema.
+func HashSchema(schema *Schema) SchemaSetFunc {
+	return func(v interface{}) int {
+		var buf bytes.Buffer
+		SerializeValueForHash(&buf, v, schema)
+		return hashcode.String(buf.String())
+	}
+}
+
 // Set is a set data structure that is returned for elements of type
 // TypeSet.
 type Set struct {