Skip to content

Commit 69d1881

Browse files
authored
Revert "Set ForceSendFields for boolean/integer values explicitly set to false/0 (#3385)" (#3627)
* Revert "Set ForceSendFields for boolean/integer values explicitly set to false/0 (#3385)" This reverts commit 89fcf94. * - * cleanup * - * -
1 parent 644d53f commit 69d1881

14 files changed

+218
-213
lines changed

clusters/resource_cluster.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -656,11 +656,13 @@ func resourceClusterUpdate(ctx context.Context, d *schema.ResourceData, c *commo
656656

657657
// We prefer to use the resize API in cases when only the number of
658658
// workers is changed because a resizing cluster can still serve queries
659+
659660
if isNumWorkersResizeForNonAutoscalingCluster ||
660661
isAutoScalingToNonAutoscalingResize {
661662
_, err = clusters.Resize(ctx, compute.ResizeCluster{
662-
ClusterId: clusterId,
663-
NumWorkers: cluster.NumWorkers,
663+
ClusterId: clusterId,
664+
NumWorkers: cluster.NumWorkers,
665+
ForceSendFields: []string{"NumWorkers"},
664666
})
665667
if err != nil {
666668
return err
@@ -676,6 +678,7 @@ func resourceClusterUpdate(ctx context.Context, d *schema.ResourceData, c *commo
676678
if err != nil {
677679
return err
678680
}
681+
cluster.ForceSendFields = []string{"NumWorkers"}
679682
_, err = clusters.Edit(ctx, cluster)
680683
}
681684
if err != nil {

common/force_send_fields.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package common
2+
3+
import (
4+
"fmt"
5+
"reflect"
6+
"strings"
7+
8+
"golang.org/x/exp/slices"
9+
)
10+
11+
// SetForceSendFields adds any fields specified in the `fields` parameter to the ForceSendFields field of the
12+
// request structure if they are present in the resource state. The provided fields must match the JSON tag
13+
// for some field in the request structure. This ensures that fields explicitly set to the zero value of its
14+
// type (e.g. `0` for an `int`) will be serialized and sent to the platform.
15+
//
16+
// This function requires that the request structure has a `ForceSendFields` field of type `[]string`. If not,
17+
// it panics with an appropriate error message.
18+
func SetForceSendFields(req any, d attributeGetter, fields []string) {
19+
rv := reflect.ValueOf(req)
20+
if rv.Kind() != reflect.Ptr {
21+
panic("request argument to setForceSendFields must be a pointer")
22+
}
23+
rv = rv.Elem()
24+
forceSendFieldsField := rv.FieldByName("ForceSendFields")
25+
if !forceSendFieldsField.IsValid() {
26+
panic("request argument to setForceSendFields must have ForceSendFields field")
27+
}
28+
forceSendFields, ok := forceSendFieldsField.Interface().([]string)
29+
if !ok {
30+
panic(fmt.Errorf("request argument to setForceSendFields must have ForceSendFields field of type []string (got %s)", forceSendFieldsField.Type()))
31+
}
32+
fs := listAllFields(rv)
33+
for _, fieldName := range fields {
34+
found := false
35+
var structField reflect.StructField
36+
for _, f := range fs {
37+
fn := chooseFieldName(f.sf)
38+
if fn != "-" && fn == fieldName {
39+
found = true
40+
structField = f.sf
41+
break
42+
}
43+
}
44+
if !found {
45+
allFieldNames := make([]string, 0)
46+
for _, f := range fs {
47+
fn := chooseFieldName(f.sf)
48+
if fn == "-" || fn == "force_send_fields" {
49+
continue
50+
}
51+
allFieldNames = append(allFieldNames, fn)
52+
}
53+
panic(fmt.Errorf("unexpected field %s not found in request structure, expected one of: %s", fieldName, strings.Join(allFieldNames, ", ")))
54+
}
55+
// Check if the field was ever set, even to the zero value of the type.
56+
// Technically we should probably check this based on the the TF schema
57+
// for this field, but this is a reasonable approximation.
58+
v, ok := d.GetOkExists(fieldName)
59+
if !(ok && isZeroValueOfType(v)) {
60+
continue
61+
}
62+
if !slices.Contains[[]string, string](forceSendFields, structField.Name) {
63+
forceSendFields = append(forceSendFields, structField.Name)
64+
}
65+
}
66+
forceSendFieldsField.Set(reflect.ValueOf(forceSendFields))
67+
}
68+
69+
func isZeroValueOfType(v any) bool {
70+
return reflect.ValueOf(v).IsZero()
71+
}

common/force_send_fields_test.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package common
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func expectPanic(t *testing.T, message string) func() {
10+
return func() {
11+
r := recover()
12+
if r == nil {
13+
t.Errorf("expected panic: %s", message)
14+
}
15+
if str, ok := r.(string); ok && str != message {
16+
t.Errorf("expected panic: %s, got: %v", message, r)
17+
}
18+
if err, ok := r.(error); ok && err.Error() != message {
19+
t.Errorf("expected panic: %s, got: %v", message, r)
20+
}
21+
}
22+
}
23+
24+
func TestSetForceSendFields_RequestMustBePointer(t *testing.T) {
25+
defer expectPanic(t, "request argument to setForceSendFields must be a pointer")()
26+
27+
SetForceSendFields(1, nil, nil)
28+
}
29+
30+
type noForceSendFields struct {
31+
A string `json:"a"`
32+
}
33+
34+
func TestSetForceSendFields_RequestMustHaveForceSendFields(t *testing.T) {
35+
defer expectPanic(t, "request argument to setForceSendFields must have ForceSendFields field")()
36+
37+
SetForceSendFields(&noForceSendFields{}, nil, nil)
38+
}
39+
40+
type incorrectForceSendFields struct {
41+
A string `json:"a"`
42+
ForceSendFields int `json:"force_send_fields"`
43+
}
44+
45+
func TestSetForceSendFields_RequestMustHaveForceSendFieldsWithCorrectType(t *testing.T) {
46+
defer expectPanic(t, "request argument to setForceSendFields must have ForceSendFields field of type []string (got int)")()
47+
48+
SetForceSendFields(&incorrectForceSendFields{}, nil, nil)
49+
}
50+
51+
type withForceSendFields struct {
52+
A string `json:"a"`
53+
B string `json:"-"`
54+
ForceSendFields []string `json:"force_send_fields"`
55+
}
56+
57+
func TestSetForceSendFields_NoFields(t *testing.T) {
58+
req := &withForceSendFields{}
59+
SetForceSendFields(req, nil, nil)
60+
assert.Len(t, req.ForceSendFields, 0)
61+
}
62+
63+
func TestSetForceSendFields_ForceAWhenPresent(t *testing.T) {
64+
req := &withForceSendFields{}
65+
SetForceSendFields(req, data{"a": ""}, []string{"a"})
66+
assert.Len(t, req.ForceSendFields, 1)
67+
assert.Equal(t, "A", req.ForceSendFields[0])
68+
}
69+
70+
func TestSetForceSendFields_DoNotForceAWhenAbsent(t *testing.T) {
71+
req := &withForceSendFields{}
72+
SetForceSendFields(req, data{}, []string{"a"})
73+
assert.Len(t, req.ForceSendFields, 0)
74+
}
75+
76+
func TestSetForceSendFields_DoNotDuplicate(t *testing.T) {
77+
req := &withForceSendFields{ForceSendFields: []string{"A"}}
78+
SetForceSendFields(req, data{"a": ""}, []string{"a"})
79+
assert.Len(t, req.ForceSendFields, 1)
80+
assert.Equal(t, "A", req.ForceSendFields[0])
81+
}
82+
83+
func TestSetForceSendFields_CannotForceNonJsonFields(t *testing.T) {
84+
defer expectPanic(t, "unexpected field b not found in request structure, expected one of: a")()
85+
req := &withForceSendFields{}
86+
SetForceSendFields(req, data{"b": ""}, []string{"b"})
87+
}
88+
89+
func TestSetForceSendFields_CannotForceUnknownFields(t *testing.T) {
90+
defer expectPanic(t, "unexpected field c not found in request structure, expected one of: a")()
91+
req := &withForceSendFields{}
92+
SetForceSendFields(req, data{"b": ""}, []string{"c"})
93+
}

0 commit comments

Comments
 (0)