also deduplicate requiresreplace

hashicorp · Aug 20, 2021 · a8991d7 · a8991d7
1 parent 583f552
commit a8991d7
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 4 deletions.
diff --git a/tfsdk/serve.go b/tfsdk/serve.go
@@ -737,14 +737,39 @@ func (s *server) PlanResourceChange(ctx context.Context, req *tfprotov6.PlanReso
 	resp.PlannedState = &plannedState
 	resp.RequiresReplace = append(resp.RequiresReplace, modifyPlanResp.RequiresReplace...)
 
-	// sort the RequiresReplace slice so response is deterministic
-	sort.Slice(resp.RequiresReplace, func(i, j int) bool {
-		return resp.RequiresReplace[i].String() < resp.RequiresReplace[j].String()
-	})
+	// ensure deterministic RequiresReplace by sorting and deduplicating
+	resp.RequiresReplace = normaliseRequiresReplace(resp.RequiresReplace)
 
 	return resp, nil
 }
 
+// normaliseRequiresReplace sorts and deduplicates the slice of AttributePaths
+// used in the RequiresReplace response field.
+// Sorting is lexical based on the string representation of each AttributePath.
+func normaliseRequiresReplace(rs []*tftypes.AttributePath) []*tftypes.AttributePath {
+	if len(rs) < 2 {
+		return rs
+	}
+
+	sort.Slice(rs, func(i, j int) bool {
+		return rs[i].String() < rs[j].String()
+	})
+
+	ret := make([]*tftypes.AttributePath, len(rs))
+	ret[0] = rs[0]
+
+	// deduplicate
+	j := 1
+	for i := 1; i < len(rs); i++ {
+		if rs[i].Equal(ret[j-1]) {
+			continue
+		}
+		ret[j] = rs[i]
+		j++
+	}
+	return ret[:j]
+}
+
 func (s *server) ApplyResourceChange(ctx context.Context, req *tfprotov6.ApplyResourceChangeRequest) (*tfprotov6.ApplyResourceChangeResponse, error) {
 	ctx = s.registerContext(ctx)
 	resp := &tfprotov6.ApplyResourceChangeResponse{

diff --git a/tfsdk/serve_test.go b/tfsdk/serve_test.go
@@ -201,6 +201,63 @@ func TestMarkComputedNilsAsUnknown(t *testing.T) {
 	}
 }
 
+func TestNormaliseRequiresReplace(t *testing.T) {
+	t.Parallel()
+
+	type testCase struct {
+		input    []*tftypes.AttributePath
+		expected []*tftypes.AttributePath
+	}
+
+	tests := map[string]testCase{
+		"nil": {
+			input:    nil,
+			expected: nil,
+		},
+		"no-duplicates": {
+			input: []*tftypes.AttributePath{
+				tftypes.NewAttributePath().WithAttributeName("name2"),
+				tftypes.NewAttributePath().WithAttributeName("name1"),
+				tftypes.NewAttributePath().WithElementKeyInt(1234),
+				tftypes.NewAttributePath().WithAttributeName("name1").WithElementKeyString("elementkey"),
+			},
+			expected: []*tftypes.AttributePath{
+				tftypes.NewAttributePath().WithAttributeName("name1"),
+				tftypes.NewAttributePath().WithAttributeName("name1").WithElementKeyString("elementkey"),
+				tftypes.NewAttributePath().WithAttributeName("name2"),
+				tftypes.NewAttributePath().WithElementKeyInt(1234),
+			},
+		},
+		"duplicates": {
+			input: []*tftypes.AttributePath{
+				tftypes.NewAttributePath().WithAttributeName("name1"),
+				tftypes.NewAttributePath().WithAttributeName("name1"),
+				tftypes.NewAttributePath().WithElementKeyInt(1234),
+				tftypes.NewAttributePath().WithElementKeyInt(1234),
+			},
+			expected: []*tftypes.AttributePath{
+				tftypes.NewAttributePath().WithAttributeName("name1"),
+				tftypes.NewAttributePath().WithElementKeyInt(1234),
+			},
+		},
+	}
+
+	for name, tc := range tests {
+		name, tc := name, tc
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			actual := normaliseRequiresReplace(tc.input)
+
+			if diff := cmp.Diff(actual, tc.expected, cmpopts.EquateEmpty()); diff != "" {
+				t.Errorf("Unexpected diff (+wanted, -got): %s", diff)
+				return
+			}
+		})
+	}
+}
+
 func TestServerGetProviderSchema(t *testing.T) {
 	t.Parallel()