refactor CopyModel to return only de model and panic if there is any structural issue (#2905)

Author: lantoli

internal/common/conversion/model_generation.go

@@ -6,17 +6,18 @@ import (
 )
 
 // CopyModel creates a new struct with the same values as the source struct. Fields in destination struct that are not in source are left with zero value.
-func CopyModel[T any](src any) (*T, error) {
+// It panics if there are some structural problems so it should only happen during development.
+func CopyModel[T any](src any) *T {
 	dest := new(T)
 	valSrc := reflect.ValueOf(src)
 	valDest := reflect.ValueOf(dest)
 	if valSrc.Kind() != reflect.Ptr || valDest.Kind() != reflect.Ptr {
-		return nil, fmt.Errorf("params must be pointers")
+		panic("params must be pointers")
 	}
 	valSrc = valSrc.Elem()
 	valDest = valDest.Elem()
 	if valSrc.Kind() != reflect.Struct || valDest.Kind() != reflect.Struct {
-		return nil, fmt.Errorf("params must be pointers to structs")
+		panic("params must be pointers to structs")
 	}
 	typeSrc := valSrc.Type()
 	typeDest := valDest.Type()
@@ -29,13 +30,13 @@ func CopyModel[T any](src any) (*T, error) {
 				continue
 			}
 			if fieldDest.Type != fieldSrc.Type {
-				return nil, fmt.Errorf("field has different type: %s", name)
+				panic(fmt.Sprintf("field has different type: %s", name))
 			}
 		}
 		if !valDest.Field(i).CanSet() {
-			return nil, fmt.Errorf("field can't be set, probably unexported: %s", name)
+			panic(fmt.Sprintf("field can't be set, probably unexported: %s", name))
 		}
 		valDest.Field(i).Set(valSrc.FieldByName(name))
 	}
-	return dest, nil
+	return dest
 }

internal/common/conversion/model_generation_test.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func TestCopyModel(t *testing.T) {
@@ -18,7 +17,7 @@ func TestCopyModel(t *testing.T) {
 	testCases := map[string]struct {
 		input            any
 		expected         any
-		expectedErrorStr string
+		expectedPanicStr string
 	}{
 		"basic": {
 			input: &struct {
@@ -65,27 +64,26 @@ func TestCopyModel(t *testing.T) {
 			}{
 				AttrStr: true,
 			},
-			expectedErrorStr: "field has different type: AttrStr",
+			expectedPanicStr: "field has different type: AttrStr",
 		},
 		"unexported": {
 			input: &struct {
 				attrUnexported string
 			}{
 				attrUnexported: "val",
 			},
-			expectedErrorStr: "field can't be set, probably unexported: attrUnexported",
+			expectedPanicStr: "field can't be set, probably unexported: attrUnexported",
 		},
 	}
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {
-			dest, err := conversion.CopyModel[destType](tc.input)
-			if err == nil {
-				assert.Equal(t, tc.expected, dest)
-				assert.Equal(t, "", tc.expectedErrorStr)
+			if tc.expectedPanicStr == "" {
+				assert.Equal(t, tc.expected, conversion.CopyModel[destType](tc.input))
 			} else {
-				require.ErrorContains(t, err, tc.expectedErrorStr)
-				assert.Nil(t, dest)
 				assert.Nil(t, tc.expected)
+				assert.PanicsWithValue(t, tc.expectedPanicStr, func() {
+					conversion.CopyModel[destType](tc.input)
+				})
 			}
 		})
 	}

internal/service/advancedclustertpf/data_source.go

@@ -72,11 +72,7 @@ func (d *ds) readCluster(ctx context.Context, diags *diag.Diagnostics, modelDS *
 	if diags.HasError() {
 		return nil
 	}
-	modelOutDS, err := conversion.CopyModel[TFModelDS](modelOut)
-	if err != nil {
-		diags.AddError(errorRead, fmt.Sprintf("error setting model: %s", err.Error()))
-		return nil
-	}
+	modelOutDS := conversion.CopyModel[TFModelDS](modelOut)
 	modelOutDS.UseReplicationSpecPerShard = modelDS.UseReplicationSpecPerShard // attrs not in resource model
 	return modelOutDS
 }

internal/service/advancedclustertpf/plural_data_source.go

@@ -84,11 +84,7 @@ func (d *pluralDS) readClusters(ctx context.Context, diags *diag.Diagnostics, pl
 		if diags.HasError() {
 			return nil
 		}
-		modelOutDS, err := conversion.CopyModel[TFModelDS](modelOut)
-		if err != nil {
-			diags.AddError(errorList, fmt.Sprintf("error setting model: %s", err.Error()))
-			return nil
-		}
+		modelOutDS := conversion.CopyModel[TFModelDS](modelOut)
 		modelOutDS.UseReplicationSpecPerShard = pluralModel.UseReplicationSpecPerShard // attrs not in resource model
 		outs.Results = append(outs.Results, modelOutDS)
 	}