update

databricks · Aug 20, 2024 · 7683b98 · 7683b98
1 parent e870855
commit 7683b98
Show file tree

Hide file tree

Showing 7 changed files with 116 additions and 138 deletions.
diff --git a/common/force_send_fields.go b/common/force_send_fields.go
@@ -5,7 +5,7 @@ import (
 	"reflect"
 	"strings"
 
-	"github.com/databricks/terraform-provider-databricks/internal/reflect_utils"
+	"github.com/databricks/terraform-provider-databricks/internal/tfreflect"
 	"golang.org/x/exp/slices"
 )
 
@@ -30,22 +30,22 @@ func SetForceSendFields(req any, d attributeGetter, fields []string) {
 	if !ok {
 		panic(fmt.Errorf("request argument to setForceSendFields must have ForceSendFields field of type []string (got %s)", forceSendFieldsField.Type()))
 	}
-	fs := reflect_utils.ListAllFields(rv)
+	fs := tfreflect.ListAllFields(rv)
 	for _, fieldName := range fields {
 		found := false
 		var structField reflect.StructField
 		for _, f := range fs {
-			fn := chooseFieldName(f.Sf)
+			fn := chooseFieldName(f.StructField)
 			if fn != "-" && fn == fieldName {
 				found = true
-				structField = f.Sf
+				structField = f.StructField
 				break
 			}
 		}
 		if !found {
 			allFieldNames := make([]string, 0)
 			for _, f := range fs {
-				fn := chooseFieldName(f.Sf)
+				fn := chooseFieldName(f.StructField)
 				if fn == "-" || fn == "force_send_fields" {
 					continue
 				}

diff --git a/common/reflect_resource.go b/common/reflect_resource.go
@@ -7,7 +7,7 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/databricks/terraform-provider-databricks/internal/reflect_utils"
+	"github.com/databricks/terraform-provider-databricks/internal/tfreflect"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
@@ -397,9 +397,9 @@ func typeToSchema(v reflect.Value, aliases map[string]map[string]string, tc trac
 		panic(fmt.Errorf("Schema value of Struct is expected, but got %s: %#v", reflectKind(rk), v))
 	}
 	tc = tc.visit(v)
-	fields := reflect_utils.ListAllFields(v)
+	fields := tfreflect.ListAllFields(v)
 	for _, field := range fields {
-		typeField := field.Sf
+		typeField := field.StructField
 		if tc.depthExceeded(typeField) {
 			// Skip the field if recursion depth is over the limit.
 			log.Printf("[TRACE] over recursion limit, skipping field: %s, max depth: %d", getNameForType(typeField.Type), tc.getMaxDepthForTypeField(typeField))
@@ -576,9 +576,9 @@ func iterFields(rv reflect.Value, path []string, s map[string]*schema.Schema, al
 		return fmt.Errorf("%s: got invalid reflect value %#v", path, rv)
 	}
 	isGoSDK := isGoSdk(rv)
-	fields := reflect_utils.ListAllFields(rv)
+	fields := tfreflect.ListAllFields(rv)
 	for _, field := range fields {
-		typeField := field.Sf
+		typeField := field.StructField
 		fieldName := chooseFieldNameWithAliases(typeField, rv.Type(), aliases)
 		if fieldName == "-" {
 			continue
@@ -596,7 +596,7 @@ func iterFields(rv reflect.Value, path []string, s map[string]*schema.Schema, al
 		if !isGoSDK && fieldSchema.Optional && defaultEmpty && !omitEmpty {
 			return fmt.Errorf("inconsistency: %s is optional, default is empty, but has no omitempty", fieldName)
 		}
-		valueField := field.V
+		valueField := field.Value
 		err := cb(fieldSchema, append(path, fieldName), &valueField)
 		if err != nil {
 			return fmt.Errorf("%s: %s", fieldName, err)

diff --git a/common/util.go b/common/util.go
@@ -16,7 +16,8 @@ import (
 )
 
 var (
-	uuidRegex = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)
+	uuidRegex                = regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)
+	TerraformBugErrorMessage = "This is a bug. Please report this to the maintainers at github.com/databricks/terraform-provider-databricks."
 )
 
 func StringIsUUID(s string) bool {

diff --git a/internal/reflect_utils/reflect_utils.go → internal/tfreflect/reflect_utils.go b/internal/reflect_utils/reflect_utils.go → internal/tfreflect/reflect_utils.go
@@ -1,12 +1,14 @@
-package reflect_utils
+package tfreflect
 
 import "reflect"
 
 type Field struct {
-	Sf reflect.StructField
-	V  reflect.Value
+	StructField reflect.StructField
+	Value       reflect.Value
 }
 
+// Given a reflect.Value of a struct, list all of the fields for both struct field and
+// the value. This function also extracts and flattens the anonymous fields nested inside.
 func ListAllFields(v reflect.Value) []Field {
 	t := v.Type()
 	fields := make([]Field, 0, v.NumField())
@@ -16,8 +18,8 @@ func ListAllFields(v reflect.Value) []Field {
 			fields = append(fields, ListAllFields(v.Field(i))...)
 		} else {
 			fields = append(fields, Field{
-				Sf: f,
-				V:  v.Field(i),
+				StructField: f,
+				Value:       v.Field(i),
 			})
 		}
 	}

diff --git a/pluginframework/common/converters_test.go → ...inframework/converters/converters_test.go b/pluginframework/common/converters_test.go → ...inframework/converters/converters_test.go
@@ -1,4 +1,4 @@
-package pluginframework
+package converters
 
 import (
 	"context"
@@ -12,8 +12,8 @@ import (
 
 type DummyTfSdk struct {
 	Enabled           types.Bool                  `tfsdk:"enabled" tf:"optional"`
-	Workers           types.Int64                 `tfsdk:"workers" tf:""` // Test required field
-	Floats            types.Float64               `tfsdk:"floats" tf:""`  // Test required field
+	Workers           types.Int64                 `tfsdk:"workers" tf:""`
+	Floats            types.Float64               `tfsdk:"floats" tf:""`
 	Description       types.String                `tfsdk:"description" tf:""`
 	Tasks             types.String                `tfsdk:"task" tf:"optional"`
 	Nested            *DummyNestedTfSdk           `tfsdk:"nested" tf:"optional"`
@@ -26,7 +26,7 @@ type DummyTfSdk struct {
 	Attributes        map[string]types.String     `tfsdk:"attributes" tf:"optional"`
 	EnumField         types.String                `tfsdk:"enum_field" tf:"optional"`
 	AdditionalField   types.String                `tfsdk:"additional_field" tf:"optional"`
-	DistinctField     types.String                `tfsdk:"distinct_field" tf:"optional"` // distinct field that the gosdk struct doesn't have
+	DistinctField     types.String                `tfsdk:"distinct_field" tf:"optional"`
 	Irrelevant        types.String                `tfsdk:"-"`
 }
 
@@ -90,55 +90,52 @@ type DummyNestedGoSdk struct {
 
 // Function to construct individual test case with a pair of matching tfSdkStruct and gosdkStruct.
 // Verifies that the conversion both ways are working as expected.
-func ConverterTestCase(t *testing.T, description string, tfSdkStruct DummyTfSdk, goSdkStruct DummyGoSdk) {
+func RunConverterTest(t *testing.T, description string, tfSdkStruct DummyTfSdk, goSdkStruct DummyGoSdk) {
 	convertedGoSdkStruct := DummyGoSdk{}
-	assert.True(t, !TfSdkToGoSdkStruct(tfSdkStruct, &convertedGoSdkStruct, context.Background()).HasError())
+	assert.True(t, !TfSdkToGoSdkStruct(context.Background(), tfSdkStruct, &convertedGoSdkStruct).HasError())
 	assert.True(t, reflect.DeepEqual(convertedGoSdkStruct, goSdkStruct), fmt.Sprintf("tfsdk to gosdk conversion - %s", description))
 
 	convertedTfSdkStruct := DummyTfSdk{}
-	assert.True(t, !GoSdkToTfSdkStruct(goSdkStruct, &convertedTfSdkStruct, context.Background()).HasError())
+	assert.True(t, !GoSdkToTfSdkStruct(context.Background(), goSdkStruct, &convertedTfSdkStruct).HasError())
 	assert.True(t, reflect.DeepEqual(convertedTfSdkStruct, tfSdkStruct), fmt.Sprintf("gosdk to tfsdk conversion - %s", description))
 }
 
-func TestConverter(t *testing.T) {
-	ConverterTestCase(
-		t,
+var tests = []struct {
+	name        string
+	tfSdkStruct DummyTfSdk
+	goSdkStruct DummyGoSdk
+}{
+	{
 		"string conversion",
 		DummyTfSdk{Description: types.StringValue("abc")},
 		DummyGoSdk{Description: "abc", ForceSendFields: []string{"Description"}},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"bool conversion",
 		DummyTfSdk{Enabled: types.BoolValue(true)},
 		DummyGoSdk{Enabled: true, ForceSendFields: []string{"Enabled"}},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"int64 conversion",
 		DummyTfSdk{Workers: types.Int64Value(123)},
 		DummyGoSdk{Workers: 123, ForceSendFields: []string{"Workers"}},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"tf null value conversion",
 		DummyTfSdk{Workers: types.Int64Null()},
 		DummyGoSdk{},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"float64 conversion",
 		DummyTfSdk{Floats: types.Float64Value(1.1)},
 		DummyGoSdk{Floats: 1.1, ForceSendFields: []string{"Floats"}},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"enum conversion",
 		DummyTfSdk{EnumField: types.StringValue("TEST_ENUM_A")},
 		DummyGoSdk{EnumField: TestEnumA},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"struct conversion",
 		DummyTfSdk{NoPointerNested: DummyNestedTfSdk{
 			Name:    types.StringValue("def"),
@@ -149,9 +146,8 @@ func TestConverter(t *testing.T) {
 			Enabled:         true,
 			ForceSendFields: []string{"Name", "Enabled"},
 		}},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"pointer conversion",
 		DummyTfSdk{Nested: &DummyNestedTfSdk{
 			Name:    types.StringValue("def"),
@@ -162,21 +158,18 @@ func TestConverter(t *testing.T) {
 			Enabled:         true,
 			ForceSendFields: []string{"Name", "Enabled"},
 		}},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"list conversion",
 		DummyTfSdk{Repeated: []types.Int64{types.Int64Value(12), types.Int64Value(34)}},
 		DummyGoSdk{Repeated: []int64{12, 34}},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"map conversion",
 		DummyTfSdk{Attributes: map[string]types.String{"key": types.StringValue("value")}},
 		DummyGoSdk{Attributes: map[string]string{"key": "value"}},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"nested list conversion",
 		DummyTfSdk{NestedList: []DummyNestedTfSdk{
 			{
@@ -200,35 +193,8 @@ func TestConverter(t *testing.T) {
 				ForceSendFields: []string{"Name", "Enabled"},
 			},
 		}},
-	)
-	ConverterTestCase(
-		t,
-		"nested list conversion",
-		DummyTfSdk{NestedList: []DummyNestedTfSdk{
-			{
-				Name:    types.StringValue("abc"),
-				Enabled: types.BoolValue(true),
-			},
-			{
-				Name:    types.StringValue("def"),
-				Enabled: types.BoolValue(false),
-			},
-		}},
-		DummyGoSdk{NestedList: []DummyNestedGoSdk{
-			{
-				Name:            "abc",
-				Enabled:         true,
-				ForceSendFields: []string{"Name", "Enabled"},
-			},
-			{
-				Name:            "def",
-				Enabled:         false,
-				ForceSendFields: []string{"Name", "Enabled"},
-			},
-		}},
-	)
-	ConverterTestCase(
-		t,
+	},
+	{
 		"nested map conversion",
 		DummyTfSdk{NestedMap: map[string]DummyNestedTfSdk{
 			"key1": {
@@ -252,5 +218,11 @@ func TestConverter(t *testing.T) {
 				ForceSendFields: []string{"Name", "Enabled"},
 			},
 		}},
-	)
+	},
+}
+
+func TestConverter(t *testing.T) {
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) { RunConverterTest(t, test.name, test.tfSdkStruct, test.goSdkStruct) })
+	}
 }