Skip to content

chore: Define conversion function for generating API request body from Resource Model in create (non-nested) #3239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Apr 3, 2025
122 changes: 94 additions & 28 deletions internal/common/autogeneration/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,67 +4,133 @@ import (
"encoding/json"
"fmt"
"reflect"
"strings"

"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/huandu/xstrings"
)

const (
tagKey = "autogeneration"
tagValOmitJSON = "omitjson"
tagValCreateOnly = "createonly"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Giving some more thought to the naming, would omitjsonupdate over createonly be more consistent?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, changed here: d6c212b

)

// Marshal gets a Terraform model and marshals it into JSON (e.g. for an Atlas request).
func Marshal(model any, isCreate bool) ([]byte, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would leave an additional comment clarifying when attributes are sent in payload, and how use of tags changes that behaviour

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done here: 065089b

valModel := reflect.ValueOf(model)
if valModel.Kind() != reflect.Ptr {
panic("model must be pointer")
}
valModel = valModel.Elem()
if valModel.Kind() != reflect.Struct {
panic("model must be pointer to struct")
}
objJSON, err := marshalAttrs(valModel, isCreate)
if err != nil {
return nil, err
}
return json.Marshal(objJSON)
}

// Unmarshal gets a JSON (e.g. from an Atlas response) and unmarshals it into a Terraform model.
// It supports the following Terraform model types: String, Bool, Int64, Float64.
func Unmarshal(raw []byte, dest any) error {
var src map[string]any
if err := json.Unmarshal(raw, &src); err != nil {
func Unmarshal(raw []byte, model any) error {
var objJSON map[string]any
if err := json.Unmarshal(raw, &objJSON); err != nil {
return err
}
return mapFields(src, dest)
return unmarshalAttrs(objJSON, model)
}

func marshalAttrs(valModel reflect.Value, isCreate bool) (map[string]any, error) {
objJSON := make(map[string]any)
for i := 0; i < valModel.NumField(); i++ {
attrTypeModel := valModel.Type().Field(i)
tag := attrTypeModel.Tag.Get(tagKey)
if strings.Contains(tag, tagValOmitJSON) {
continue // skip fields with tag `omitjson`
}
if !isCreate && strings.Contains(tag, tagValCreateOnly) {
continue // skip fields with tag `createonly` if not in create
}
attrNameModel := attrTypeModel.Name
attrValModel := valModel.Field(i)
if err := marshalAttr(attrNameModel, attrValModel, objJSON); err != nil {
return nil, err
}
}
return objJSON, nil
}

func marshalAttr(attrNameModel string, attrValModel reflect.Value, objJSON map[string]any) error {
attrNameJSON := xstrings.ToSnakeCase(attrNameModel)
obj, ok := attrValModel.Interface().(attr.Value)
if !ok {
panic("marshal expects only Terraform types in the model")
}
if obj.IsNull() || obj.IsUnknown() {
return nil // skip nil or unknown values
}
switch v := attrValModel.Interface().(type) {
case types.String:
objJSON[attrNameJSON] = v.ValueString()
case types.Int64:
objJSON[attrNameJSON] = v.ValueInt64()
case types.Float64:
objJSON[attrNameJSON] = v.ValueFloat64()
default:
return fmt.Errorf("marshal not supported yet for type %T for field %s", v, attrNameJSON)
}
return nil
}

func mapFields(src map[string]any, dest any) error {
valDest := reflect.ValueOf(dest)
if valDest.Kind() != reflect.Ptr {
panic("dest must be pointer")
func unmarshalAttrs(objJSON map[string]any, model any) error {
valModel := reflect.ValueOf(model)
if valModel.Kind() != reflect.Ptr {
panic("model must be pointer")
}
valDest = valDest.Elem()
if valDest.Kind() != reflect.Struct {
panic("dest must be pointer to struct")
valModel = valModel.Elem()
if valModel.Kind() != reflect.Struct {
panic("model must be pointer to struct")
}
for nameAttrSrc, valueAttrSrc := range src {
if err := mapField(nameAttrSrc, valueAttrSrc, valDest); err != nil {
for attrNameJSON, attrObjJSON := range objJSON {
if err := unmarshalAttr(attrNameJSON, attrObjJSON, valModel); err != nil {
return err
}
}
return nil
}

func mapField(nameAttrSrc string, valueAttrSrc any, valDest reflect.Value) error {
nameDest := xstrings.ToPascalCase(nameAttrSrc)
fieldDest := valDest.FieldByName(nameDest)
if !fieldDest.CanSet() {
func unmarshalAttr(attrNameJSON string, attrObjJSON any, valModel reflect.Value) error {
attrNameModel := xstrings.ToPascalCase(attrNameJSON)
fieldModel := valModel.FieldByName(attrNameModel)
if !fieldModel.CanSet() {
return nil // skip fields that cannot be set, are invalid or not found
}
switch v := valueAttrSrc.(type) {
switch v := attrObjJSON.(type) {
case string:
return assignField(nameDest, fieldDest, types.StringValue(v))
return setAttrModel(attrNameModel, fieldModel, types.StringValue(v))
case bool:
return assignField(nameDest, fieldDest, types.BoolValue(v))
return setAttrModel(attrNameModel, fieldModel, types.BoolValue(v))
case float64: // number: try int or float
if assignField(nameDest, fieldDest, types.Float64Value(v)) == nil {
if setAttrModel(attrNameModel, fieldModel, types.Float64Value(v)) == nil {
return nil
}
return assignField(nameDest, fieldDest, types.Int64Value(int64(v)))
return setAttrModel(attrNameModel, fieldModel, types.Int64Value(int64(v)))
case nil:
return nil // skip nil values, no need to set anything
default:
return fmt.Errorf("not supported yet type %T for field %s", v, nameAttrSrc)
return fmt.Errorf("unmarshal not supported yet for type %T for field %s", v, attrNameJSON)
}
}

func assignField(nameDest string, fieldDest reflect.Value, valueDest attr.Value) error {
valObj := reflect.ValueOf(valueDest)
if !fieldDest.Type().AssignableTo(valObj.Type()) {
return fmt.Errorf("can't assign value to model field %s", nameDest)
func setAttrModel(name string, field reflect.Value, val attr.Value) error {
obj := reflect.ValueOf(val)
if !field.Type().AssignableTo(obj.Type()) {
return fmt.Errorf("unmarshal can't assign value to model field %s", name)
}
fieldDest.Set(valObj)
field.Set(obj)
return nil
}
179 changes: 152 additions & 27 deletions internal/common/autogeneration/marshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,50 +3,175 @@ package autogeneration_test
import (
"testing"

"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/autogeneration"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestMarshalBasic(t *testing.T) {
model := struct {
AttrFloat types.Float64 `tfsdk:"attribute_float"`
AttrString types.String `tfsdk:"attribute_string"`
// values with tag `omitjson` are not marshaled, and they don't need to be Terraform types
AttrOmit types.String `tfsdk:"attribute_omit" autogeneration:"omitjson"`
AttrOmitNoTerraform string `autogeneration:"omitjson"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason why this is not types.String? Would also define it with a known string value to ensure it is never sent even if it has a value in the config

Copy link
Member Author

@lantoli lantoli Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean AttrOmitNoTerraform? It's to prove that in this case you can use "invalid" types that are not Terraform types in the model, and that's ok because it's not sent in JSON, like explained in the comment above.
In the normal case it would panic if the model has an attribute that is not Terraform, e.g test here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aah makes sense, I see AttrOmit covers the case i was mentioning, thanks!

AttrUnkown types.String `tfsdk:"attribute_unknown"`
AttrNull types.String `tfsdk:"attribute_null"`
AttrInt types.Int64 `tfsdk:"attribute_int"`
}{
AttrFloat: types.Float64Value(1.234),
AttrString: types.StringValue("hello"),
AttrOmit: types.StringValue("omit"),
AttrOmitNoTerraform: "omit",
AttrUnkown: types.StringUnknown(), // unknown values are not marshaled
AttrNull: types.StringNull(), // null values are not marshaled
AttrInt: types.Int64Value(1),
}
const expectedJSON = `{ "attr_string": "hello", "attr_int": 1, "attr_float": 1.234 }`
raw, err := autogeneration.Marshal(&model, false)
require.NoError(t, err)
assert.JSONEq(t, expectedJSON, string(raw))
}

func TestMarshalCreateOnly(t *testing.T) {
const (
expectedCreate = `{ "attr": "val1", "attr_create_only": "val2" }`
expectedNoCreate = `{ "attr": "val1" }`
)
model := struct {
Attr types.String `tfsdk:"attr"`
AttrCreateOnly types.String `tfsdk:"attr_create_only" autogeneration:"createonly"`
AttrOmit types.String `tfsdk:"attr_omit" autogeneration:"omitjson"`
}{
Attr: types.StringValue("val1"),
AttrCreateOnly: types.StringValue("val2"),
AttrOmit: types.StringValue("omit"),
}
noCreate, errNoCreate := autogeneration.Marshal(&model, false)
require.NoError(t, errNoCreate)
assert.JSONEq(t, expectedNoCreate, string(noCreate))

create, errCreate := autogeneration.Marshal(&model, true)
require.NoError(t, errCreate)
assert.JSONEq(t, expectedCreate, string(create))
}

func TestMarshalUnsupported(t *testing.T) {
testCases := map[string]any{
"Object not supported yet, only no-nested types": &struct {
Attr types.Object
}{
Attr: types.ObjectValueMust(map[string]attr.Type{
"key": types.StringType,
}, map[string]attr.Value{
"key": types.StringValue("value"),
}),
},
"List not supported yet, only no-nested types": &struct {
Attr types.List
}{
Attr: types.ListValueMust(types.StringType, []attr.Value{
types.StringValue("value"),
}),
},
"Map not supported yet, only no-nested types": &struct {
Attr types.Map
}{
Attr: types.MapValueMust(types.StringType, map[string]attr.Value{
"key": types.StringValue("value"),
}),
},
"Set not supported yet, only no-nested types": &struct {
Attr types.Set
}{
Attr: types.SetValueMust(types.StringType, []attr.Value{
types.StringValue("value"),
}),
},
"Int32 not supported yet as it's not being used in any model": &struct {
Attr types.Int32
}{
Attr: types.Int32Value(1),
},
"Float32 not supported yet as it's not being used in any model": &struct {
Attr types.Float32
}{
Attr: types.Float32Value(1.0),
},
}
for name, model := range testCases {
t.Run(name, func(t *testing.T) {
raw, err := autogeneration.Marshal(model, false)
require.Error(t, err)
assert.Nil(t, raw)
})
}
}

func TestMarshalPanic(t *testing.T) {
str := "string"
testCases := map[string]any{
"no Terraform types": &struct {
Attr string
}{
Attr: "a",
},
"no pointer": struct {
Attr types.String
}{
Attr: types.StringValue("a"),
},
"no struct": &str,
}
for name, model := range testCases {
t.Run(name, func(t *testing.T) {
assert.Panics(t, func() {
_, _ = autogeneration.Marshal(model, false)
})
})
}
}

func TestUnmarshalBasic(t *testing.T) {
var model struct {
AttributeFloat types.Float64 `tfsdk:"attribute_float"`
AttributeFloatWithInt types.Float64 `tfsdk:"attribute_float_with_int"`
AttributeString types.String `tfsdk:"attribute_string"`
AttributeNotInJSON types.String `tfsdk:"attribute_not_in_json"`
AttributeInt types.Int64 `tfsdk:"attribute_int"`
AttributeIntWithFloat types.Int64 `tfsdk:"attribute_int_with_float"`
AttributeTrue types.Bool `tfsdk:"attribute_true"`
AttributeFalse types.Bool `tfsdk:"attribute_false"`
AttrFloat types.Float64 `tfsdk:"attribute_float"`
AttrFloatWithInt types.Float64 `tfsdk:"attribute_float_with_int"`
AttrString types.String `tfsdk:"attribute_string"`
AttrNotInJSON types.String `tfsdk:"attribute_not_in_json"`
AttrInt types.Int64 `tfsdk:"attribute_int"`
AttrIntWithFloat types.Int64 `tfsdk:"attribute_int_with_float"`
AttrTrue types.Bool `tfsdk:"attribute_true"`
AttrFalse types.Bool `tfsdk:"attribute_false"`
}
const (
epsilon = 10e-15 // float tolerance
// attribute_not_in_model is ignored because it is not in the model, no error is thrown.
// attribute_null is ignored because it is null, no error is thrown even if it is not in the model.
tfResponseJSON = `
{
"attribute_string": "value_string",
"attribute_true": true,
"attribute_false": false,
"attribute_int": 123,
"attribute_int_with_float": 10.6,
"attribute_float": 456.1,
"attribute_float_with_int": 13,
"attribute_not_in_model": "val",
"attribute_null": null
"attr_string": "value_string",
"attr_true": true,
"attr_false": false,
"attr_int": 123,
"attr_int_with_float": 10.6,
"attr_float": 456.1,
"attr_float_with_int": 13,
"attr_not_in_model": "val",
"attr_null": null
}
`
)
require.NoError(t, autogeneration.Unmarshal([]byte(tfResponseJSON), &model))
assert.Equal(t, "value_string", model.AttributeString.ValueString())
assert.True(t, model.AttributeTrue.ValueBool())
assert.False(t, model.AttributeFalse.ValueBool())
assert.Equal(t, int64(123), model.AttributeInt.ValueInt64())
assert.Equal(t, int64(10), model.AttributeIntWithFloat.ValueInt64()) // response floats stored in model ints have their decimals stripped.
assert.InEpsilon(t, float64(456.1), model.AttributeFloat.ValueFloat64(), epsilon)
assert.InEpsilon(t, float64(13), model.AttributeFloatWithInt.ValueFloat64(), epsilon)
assert.True(t, model.AttributeNotInJSON.IsNull()) // attributes not in JSON response are not changed, so null is kept.
assert.Equal(t, "value_string", model.AttrString.ValueString())
assert.True(t, model.AttrTrue.ValueBool())
assert.False(t, model.AttrFalse.ValueBool())
assert.Equal(t, int64(123), model.AttrInt.ValueInt64())
assert.Equal(t, int64(10), model.AttrIntWithFloat.ValueInt64()) // response floats stored in model ints have their decimals stripped.
assert.InEpsilon(t, float64(456.1), model.AttrFloat.ValueFloat64(), epsilon)
assert.InEpsilon(t, float64(13), model.AttrFloatWithInt.ValueFloat64(), epsilon)
assert.True(t, model.AttrNotInJSON.IsNull()) // attributes not in JSON response are not changed, so null is kept.
}

func TestUnmarshalErrors(t *testing.T) {
Expand Down Expand Up @@ -132,11 +257,11 @@ func TestUnmarshalUnsupportedResponse(t *testing.T) {
}{
"JSON objects not support yet": {
responseJSON: `{"attr": {"key": "value"}}`,
errorStr: "not supported yet type map[string]interface {} for field attr",
errorStr: "unmarshal not supported yet for type map[string]interface {} for field attr",
},
"JSON arrays not supported yet": {
responseJSON: `{"attr": [{"key": "value"}]}`,
errorStr: "not supported yet type []interface {} for field attr",
errorStr: "unmarshal not supported yet for type []interface {} for field attr",
},
}
for name, tc := range testCases {
Expand Down