Skip to content
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

Support nested objects within attributes when Marshaling #28

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

brandonc
Copy link
Collaborator

@brandonc brandonc commented Jan 11, 2025

Unmarshaling an attribute that an object or array of objects that is decorated with jsonapi tags has historically worked as expected, but, curiously, marshaling did not and required the use of json tags instead, making struct reuse difficult for both input and output object attributes.

Now, you can specify a struct, struct pointer, slice of structs, or slice of struct pointers as an attribute and it will be marshaled into the correct keys.

https://hashicorp.atlassian.net/browse/TF-23299

@brandonc brandonc requested a review from a team as a code owner January 11, 2025 00:20
Unmarshaling an attribute that is a struct or struct pointer that is
decorated with jsonapi tags has historically worked as expected, but,
curiously, marshaling did not and required the use of `json` tags
instead, making struct reuse difficult for both input and output
object attributes.

Now, you can specify a struct or struct pointer as an attribute and
it will be marshaled into the correct keys.

Note that this implemenation does not yet support slices of structs or
struct pointers.
@brandonc brandonc force-pushed the marshal_nested_object_attributes branch from b04ea2e to 9333e5c Compare January 11, 2025 00:23
Copy link

@uturunku1 uturunku1 left a comment

Choose a reason for hiding this comment

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

This works well! I tested the changes using Michael Yocca's nested object that we discussed in Slack.
I noticed that the visitModelNode function is starting to get super long. So a bit of refactoring around the code that you added within that else block could help with readability.

response.go Outdated Show resolved Hide resolved
response.go Outdated
strAttr, ok := fieldValue.Interface().(string)
if ok {
node.Attributes[args[1]] = strAttr
isStruct := fieldValue.Type().Kind() == reflect.Struct

Choose a reason for hiding this comment

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

Could this new logic (that starts at this line and ends prior to strAttr, ok := fieldValue.Interface().(string) line) be extracted into a helper function like "handleStructFieldValues"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a refactor commit that breaks up individual attr/relation field handling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants