chore: Support nested attributes in conversion logic for generating Resource Model from API response #3261
Conversation
| "JSON arrays not supported yet": { | ||
| responseJSON: `{"attr": [{"key": "value"}]}`, | ||
| errorStr: "unmarshal not supported yet for type []interface {} for field attr", | ||
| "JSON maps not supported yet": { |
There was a problem hiding this comment.
nice that we capture current limitation with a test
| } | ||
| } | ||
|
|
||
| func convertUnknownToNull(valModel reflect.Value) { |
There was a problem hiding this comment.
should this logic be recursive?
There was a problem hiding this comment.
unknown to null logic moved to another PR
| @@ -164,26 +173,211 @@ func unmarshalAttr(attrNameJSON string, attrObjJSON any, valModel reflect.Value) | |||
| } | |||
There was a problem hiding this comment.
Is it worth it to log a debug message?
There was a problem hiding this comment.
not sure, in general I prefer not to log debug messages, I don't know if this case is so important to be logged
| AttrSetString: types.SetUnknown(types.StringType), | ||
| AttrSetObj: types.SetUnknown(objTypeTest), | ||
| } | ||
| // attrUnexisting is ignored because it is in JSON but not in the model, no error is returned |
There was a problem hiding this comment.
nice with these comments
| func convertUnknownToNull(valModel reflect.Value) { | ||
| for i := 0; i < valModel.NumField(); i++ { | ||
| field := valModel.Field(i) | ||
| if obj, ok := field.Interface().(types.Object); ok { |
There was a problem hiding this comment.
This would only work for types.Object? What about other types.XXX?
There was a problem hiding this comment.
you're right, creating ticket created for to null logic: CLOUDP-312809
| return err | ||
| } | ||
| } | ||
| convertUnknownToNull(valModel) |
There was a problem hiding this comment.
I think this would be better to do in a different step?
Then it would be easier to override and possible use values from state instead?
There was a problem hiding this comment.
@EspenAlbert do you mean not doing it in Unmarshal, but in another public func?
There was a problem hiding this comment.
@EspenAlbert ticket created for to null logic: CLOUDP-312809 cc @AgustinBettati
There was a problem hiding this comment.
but in another public func
Yes, I believe so. Sounds good to me with a different ticket 👍
Description
Support nested attributes in conversion logic for generating Resource Model from API response. It includes ListNest, SetNested and SingleNested. MapNested is not supported as it's not needed at the moment.
Link to any related issue(s): CLOUDP-311710
Type of change:
Required Checklist:
Further comments