diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index e79cea28be..2a3d05f004 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -8,6 +8,8 @@ ### Bug Fixes +* Correctly handling tags update in `databricks_sql_endpoint` ([#5060](https://github.com/databricks/terraform-provider-databricks/pull/5060)) + ### Documentation ### Exporter diff --git a/common/reflect_resource.go b/common/reflect_resource.go index ca0d3b7d88..7df4dcbf1a 100644 --- a/common/reflect_resource.go +++ b/common/reflect_resource.go @@ -688,6 +688,21 @@ func isValueNilOrEmpty(valueField *reflect.Value, fieldPath string) bool { return false } +// isStructEmpty checks if a struct has all zero values +func isStructEmpty(v reflect.Value) bool { + if v.Kind() != reflect.Struct { + return false + } + + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + if !field.IsZero() { + return false + } + } + return true +} + // StructToData reads result using schema onto resource data func StructToData(result any, s map[string]*schema.Schema, d *schema.ResourceData) error { aliases := getAliasesMapFromStruct(result) @@ -922,10 +937,9 @@ func readListFromData(path []string, d attributeGetter, return nil case reflect.Slice: k := valueField.Type().Elem().Kind() - newSlice := reflect.MakeSlice(valueField.Type(), len(rawList), len(rawList)) - valueField.Set(newSlice) + var validItems []reflect.Value + for i, elem := range rawList { - item := newSlice.Index(i) switch k { case reflect.Struct: // here we rely on Terraform SDK to perform validation, so we don't to it twice @@ -937,14 +951,27 @@ func readListFromData(path []string, d attributeGetter, if err != nil { return err } - item.Set(ve) + // Only add non-empty structs to avoid sending empty objects to the API + if !isStructEmpty(ve) { + validItems = append(validItems, ve) + } default: - err := setPrimitiveValueOfKind(fieldPath, k, item, elem) + // For non-struct types, add all primitive values + tempItem := reflect.New(valueField.Type().Elem()).Elem() + err := setPrimitiveValueOfKind(fieldPath, k, tempItem, elem) if err != nil { return err } + validItems = append(validItems, tempItem) } } + + // Create slice with only valid items + newSlice := reflect.MakeSlice(valueField.Type(), len(validItems), len(validItems)) + for i, item := range validItems { + newSlice.Index(i).Set(item) + } + valueField.Set(newSlice) default: return fmt.Errorf("%s[%v] unknown collection field", fieldPath, rawList) } diff --git a/common/reflect_resource_test.go b/common/reflect_resource_test.go index 593bc8c735..41edb9b2d1 100644 --- a/common/reflect_resource_test.go +++ b/common/reflect_resource_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "strconv" "testing" "github.com/databricks/databricks-sdk-go/service/sql" @@ -752,6 +753,183 @@ func TestReadListFromData(t *testing.T) { assert.EqualError(t, err, "[[1]] unknown collection field") } +func TestIsStructEmpty(t *testing.T) { + // Test with empty struct + type EmptyStruct struct{} + emptyStruct := reflect.ValueOf(EmptyStruct{}) + assert.True(t, isStructEmpty(emptyStruct)) + + // Test with struct containing zero values + type ZeroStruct struct { + StringField string + IntField int + BoolField bool + } + zeroStruct := reflect.ValueOf(ZeroStruct{}) + assert.True(t, isStructEmpty(zeroStruct)) + + // Test with struct containing non-zero values + type NonZeroStruct struct { + StringField string + IntField int + BoolField bool + } + nonZeroStruct := reflect.ValueOf(NonZeroStruct{ + StringField: "test", + IntField: 42, + BoolField: true, + }) + assert.False(t, isStructEmpty(nonZeroStruct)) + + // Test with struct containing some zero and some non-zero values + type MixedStruct struct { + StringField string + IntField int + BoolField bool + } + mixedStruct := reflect.ValueOf(MixedStruct{ + StringField: "test", + IntField: 0, // zero value + BoolField: false, // zero value + }) + assert.False(t, isStructEmpty(mixedStruct)) + + // Test with non-struct value + nonStruct := reflect.ValueOf("string") + assert.False(t, isStructEmpty(nonStruct)) +} + +func TestReadListFromDataWithEmptyStructs(t *testing.T) { + // Define a test struct similar to EndpointTagPair + type TestTag struct { + Key string `json:"key,omitempty"` + Value string `json:"value,omitempty"` + } + + // Create schema for the test + schema := &schema.Schema{ + Type: schema.TypeSet, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "key": { + Type: schema.TypeString, + Optional: true, + }, + "value": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + } + + // Test case 1: Empty list should result in empty slice + t.Run("EmptyList", func(t *testing.T) { + var result []TestTag + valueField := reflect.ValueOf(&result).Elem() + + err := readListFromData([]string{"tags"}, &mockAttributeGetter{}, []any{}, &valueField, schema, map[string]map[string]string{}, strconv.Itoa) + assert.NoError(t, err) + assert.Len(t, result, 0) + }) + + // Test case 2: List with empty structs should be filtered out + t.Run("EmptyStructsFiltered", func(t *testing.T) { + var result []TestTag + valueField := reflect.ValueOf(&result).Elem() + + // Create mock data with empty structs + rawList := []any{ + map[string]any{"key": "", "value": ""}, // empty struct + map[string]any{"key": "", "value": ""}, // another empty struct + } + + mockGetter := &mockAttributeGetter{ + data: map[string]any{ + "tags.0.key": "", + "tags.0.value": "", + "tags.1.key": "", + "tags.1.value": "", + }, + } + + err := readListFromData([]string{"tags"}, mockGetter, rawList, &valueField, schema, map[string]map[string]string{}, strconv.Itoa) + assert.NoError(t, err) + assert.Len(t, result, 0) // Should be empty because all structs are empty + }) + + // Test case 3: List with mixed empty and non-empty structs + t.Run("MixedStructs", func(t *testing.T) { + var result []TestTag + valueField := reflect.ValueOf(&result).Elem() + + // Create mock data with one empty and one non-empty struct + rawList := []any{ + map[string]any{"key": "", "value": ""}, // empty struct + map[string]any{"key": "test", "value": "value"}, // non-empty struct + } + + mockGetter := &mockAttributeGetter{ + data: map[string]any{ + "tags.0.key": "", + "tags.0.value": "", + "tags.1.key": "test", + "tags.1.value": "value", + }, + } + + err := readListFromData([]string{"tags"}, mockGetter, rawList, &valueField, schema, map[string]map[string]string{}, strconv.Itoa) + assert.NoError(t, err) + assert.Len(t, result, 1) // Should contain only the non-empty struct + assert.Equal(t, "test", result[0].Key) + assert.Equal(t, "value", result[0].Value) + }) + + // Test case 4: List with only non-empty structs + t.Run("NonEmptyStructs", func(t *testing.T) { + var result []TestTag + valueField := reflect.ValueOf(&result).Elem() + + // Create mock data with non-empty structs + rawList := []any{ + map[string]any{"key": "key1", "value": "value1"}, + map[string]any{"key": "key2", "value": "value2"}, + } + + mockGetter := &mockAttributeGetter{ + data: map[string]any{ + "tags.0.key": "key1", + "tags.0.value": "value1", + "tags.1.key": "key2", + "tags.1.value": "value2", + }, + } + + err := readListFromData([]string{"tags"}, mockGetter, rawList, &valueField, schema, map[string]map[string]string{}, strconv.Itoa) + assert.NoError(t, err) + assert.Len(t, result, 2) // Should contain both structs + assert.Equal(t, "key1", result[0].Key) + assert.Equal(t, "value1", result[0].Value) + assert.Equal(t, "key2", result[1].Key) + assert.Equal(t, "value2", result[1].Value) + }) +} + +// mockAttributeGetter implements the attributeGetter interface for testing +type mockAttributeGetter struct { + data map[string]any +} + +func (m *mockAttributeGetter) GetOk(key string) (any, bool) { + value, ok := m.data[key] + return value, ok +} + +func (m *mockAttributeGetter) GetOkExists(key string) (any, bool) { + value, ok := m.data[key] + return value, ok +} + func TestReadReflectValueFromDataCornerCases(t *testing.T) { type Nonsense struct { New float64 `json:"new,omitempty"`