Add NullableRelationship support #23
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
July 5, 2024 14:41
http://jsonapi.org/format/#document-resource-object-relationships http://jsonapi.org/format/#document-resource-object-linkage Relationships can have a data node set to null (e.g. to disassociate the relationship) The NullableRelationship type allows this data to be marshalled/unmarshalled Supports slice and regular reference types
joekarl
commented
Jul 11, 2024
| RFC3339Time NullableAttr[time.Time] `jsonapi:"attr,rfc3339_time,rfc3339,omitempty"` | ||
| ISO8601Time NullableAttr[time.Time] `jsonapi:"attr,iso8601_time,iso8601,omitempty"` | ||
| Bool NullableAttr[bool] `jsonapi:"attr,bool,omitempty"` | ||
| NullableComment NullableRelationship[*Comment] `jsonapi:"relation,nullable_comment,omitempty"` |
There was a problem hiding this comment.
This is the intended usage.
By setting the NullableRelationship to an explicit value it will serialize that value, by setting the NullableRelationship to an explicit null will serialize a null value which is intended to clear the relationship.
joekarl
commented
Jul 11, 2024
| // unwind that here | ||
| if strings.HasPrefix(fieldType.Type.Name(), "NullableRelationship[") { | ||
| if m.Kind() == reflect.Ptr { | ||
| m = reflect.New(fieldValue.Type().Elem().Elem()) |
There was a problem hiding this comment.
This is the grossest part of the code. It feels wrong to "just unwrap one more pointer" but it does work as expected.
joekarl
commented
Jul 11, 2024
| }, | ||
| }, | ||
| { | ||
| desc: "nullable_comment_not_null", |
There was a problem hiding this comment.
If we were to want to support de-serializing slice types we would implement tests for that here.
As it's not included in the PR those sorts of tests are omitted here.
ctrombley
reviewed
Jul 13, 2024
| t.Set(value.(T)) | ||
| } | ||
|
|
||
| // IsNull indicate whether the field was sent, and had a value of `null` |
| @@ -471,20 +491,29 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) | |||
| so unmarshal and set fieldValue only if data obj is not null | |||
| */ | |||
| if relationship.Data == nil { | |||
|
|
|||
| // Explicit null supplied for the field value | |||
| // If a nullable relationship we set the | |||
| // this indicates disassociating the relationship | ||
| isExplicitNull = true | ||
| } else if relationshipDecodeErr != nil { | ||
| fmt.Printf("decode err %v\n", relationshipDecodeErr) |
| fieldValue.Set(m) | ||
| if strings.HasPrefix(fieldType.Type.Name(), "NullableRelationship[") { | ||
| fieldValue.Set(reflect.MakeMapWithSize(fieldValue.Type(), 1)) | ||
| fieldValue.SetMapIndex(reflect.ValueOf(true), m) |
|
|
||
| // Explicit null supplied for the field value | ||
| // If a nullable relationship we set the | ||
| if isExplicitNull && strings.HasPrefix(fieldType.Type.Name(), "NullableRelationship[") { |
There was a problem hiding this comment.
Would it be enough to simply check isExplicitNull? It looks like the only way it can be true here is if we have a NullableRelationship.
| @@ -331,7 +331,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, | |||
| node.Attributes = make(map[string]interface{}) | |||
| } | |||
|
|
|||
| // Handle Nullable[T] | |||
| // Handle NullableAttr[T] | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
http://jsonapi.org/format/#document-resource-object-relationships
http://jsonapi.org/format/#document-resource-object-linkage
Relationships can have a data node set to null (e.g. to disassociate the relationship)
The NullableRelationship type allows this data to be serialized/de-serialized. When serialized, a NullableRelationship with an explicit null will serialize to the appropriate "null" type. Either '"data": null' or '[]'.
Supports slice and regular reference types for serialization, and regular reference types for de-serialization.
The reason to not support slices for de-serialization is we need to decide how to handle the zero type for a NullableRelationship[[]*...] slice. Since we aren't using this and can emulate the nulling behavior by omitting omit-empty we decided to ignore this case for now.