From 7bc0be4b8137180f9f86b356f80f10dbc6eb347c Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Sat, 16 Aug 2014 09:49:22 -0700
Subject: [PATCH] helper/schema: couple more tests around Computed (+ fix)

---
 helper/schema/resource.go      |  4 ++
 helper/schema/resource_test.go | 14 +++++++
 helper/schema/schema.go        | 35 ++++++++++++++---
 helper/schema/schema_test.go   | 69 ++++++++++++++++++++++++++++++++++
 4 files changed, 116 insertions(+), 6 deletions(-)

diff --git a/helper/schema/resource.go b/helper/schema/resource.go
index e112b380f..64ae3fcd3 100644
--- a/helper/schema/resource.go
+++ b/helper/schema/resource.go
@@ -66,6 +66,10 @@ func (r *Resource) InternalValidate() error {
 			return fmt.Errorf("%s: Cannot be both Required and Computed", k)
 		}
 
+		if len(v.ComputedWhen) > 0 && !v.Computed {
+			return fmt.Errorf("%s: ComputedWhen can only be set with Computed", k)
+		}
+
 		if v.Type == TypeList {
 			if v.Elem == nil {
 				return fmt.Errorf("%s: Elem must be set for lists", k)
diff --git a/helper/schema/resource_test.go b/helper/schema/resource_test.go
index fa2c7d0a7..f0690a65a 100644
--- a/helper/schema/resource_test.go
+++ b/helper/schema/resource_test.go
@@ -94,6 +94,20 @@ func TestResourceInternalValidate(t *testing.T) {
 			},
 			true,
 		},
+
+		// Required but computed
+		{
+			&Resource{
+				Schema: map[string]*Schema{
+					"foo": &Schema{
+						Type:         TypeInt,
+						Required:     true,
+						ComputedWhen: []string{"foo"},
+					},
+				},
+			},
+			true,
+		},
 	}
 
 	for i, tc := range cases {
diff --git a/helper/schema/schema.go b/helper/schema/schema.go
index 7eb50203d..9c4b83378 100644
--- a/helper/schema/schema.go
+++ b/helper/schema/schema.go
@@ -30,9 +30,13 @@ type Schema struct {
 	Optional bool
 	Required bool
 
-	// The fields below relate to diffs: if Computed is true, then the
-	// result of this value is computed (unless specified by config).
-	// If ForceNew is true
+	// The fields below relate to diffs.
+	//
+	// If Computed is true, then the result of this value is computed
+	// (unless specified by config) on creation.
+	//
+	// If ForceNew is true, then a change in this resource necessitates
+	// the creation of a new resource.
 	Computed bool
 	ForceNew bool
 
@@ -43,6 +47,11 @@ type Schema struct {
 	// 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{}
+
+	// ComputedWhen is a set of queries on the configuration. Whenever any
+	// of these things is changed, it will require a recompute (this requires
+	// that Computed is set to true).
+	ComputedWhen []string
 }
 
 func (s *Schema) finalizeDiff(
@@ -51,9 +60,17 @@ func (s *Schema) finalizeDiff(
 		return d
 	}
 
-	if s.Computed && d.New == "" {
-		// Computed attribute without a new value set
-		d.NewComputed = true
+	if s.Computed {
+		if d.Old != "" && d.New == "" {
+			// This is a computed value with an old value set already,
+			// just let it go.
+			return nil
+		}
+
+		if d.New == "" {
+			// Computed attribute without a new value set
+			d.NewComputed = true
+		}
 	}
 
 	if s.ForceNew {
@@ -95,6 +112,12 @@ func (m schemaMap) Diff(
 		}
 	}
 
+	for k, v := range result.Attributes {
+		if v == nil {
+			delete(result.Attributes, k)
+		}
+	}
+
 	if result.Empty() {
 		// If we don't have any diff elements, just return nil
 		return nil, nil
diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go
index 2a8c4b482..4ff8607c2 100644
--- a/helper/schema/schema_test.go
+++ b/helper/schema/schema_test.go
@@ -436,6 +436,75 @@ func TestSchemaMap_Diff(t *testing.T) {
 
 			Err: true,
 		},
+
+		/*
+		 * ComputedWhen
+		 */
+
+		{
+			Schema: map[string]*Schema{
+				"availability_zone": &Schema{
+					Type:         TypeString,
+					Computed:     true,
+					ComputedWhen: []string{"port"},
+				},
+
+				"port": &Schema{
+					Type:     TypeInt,
+					Optional: true,
+				},
+			},
+
+			State: &terraform.ResourceState{
+				Attributes: map[string]string{
+					"availability_zone": "foo",
+					"port":              "80",
+				},
+			},
+
+			Config: map[string]interface{}{
+				"port": 80,
+			},
+
+			Diff: nil,
+
+			Err: false,
+		},
+
+		{
+			Schema: map[string]*Schema{
+				"availability_zone": &Schema{
+					Type:         TypeString,
+					Computed:     true,
+					ComputedWhen: []string{"port"},
+				},
+
+				"port": &Schema{
+					Type:     TypeInt,
+					Optional: true,
+				},
+			},
+
+			State: &terraform.ResourceState{
+				Attributes: map[string]string{
+					"port": "80",
+				},
+			},
+
+			Config: map[string]interface{}{
+				"port": 80,
+			},
+
+			Diff: &terraform.ResourceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"availability_zone": &terraform.ResourceAttrDiff{
+						NewComputed: true,
+					},
+				},
+			},
+
+			Err: false,
+		},
 	}
 
 	for i, tc := range cases {