From dfbcbfce8b015c183583a2f6b5ec90c9c28d3e58 Mon Sep 17 00:00:00 2001 From: Netra Mali Date: Fri, 24 Jan 2025 15:28:22 -0500 Subject: [PATCH 1/6] add nullable relationship --- models_test.go | 1 + nullable.go | 91 +++++++++++++++++++++++++++++++++++ request.go | 44 +++++++++++++---- request_test.go | 122 +++++++++++++++++++++++++++++++++++++++++++++++ response.go | 27 +++++++++-- response_test.go | 82 +++++++++++++++++++++++++++++++ 6 files changed, 354 insertions(+), 13 deletions(-) diff --git a/models_test.go b/models_test.go index 41f34d6..fe0ceef 100644 --- a/models_test.go +++ b/models_test.go @@ -42,6 +42,7 @@ type WithNullableAttrs struct { 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"` } type Car struct { diff --git a/nullable.go b/nullable.go index 68910f6..b7552f8 100644 --- a/nullable.go +++ b/nullable.go @@ -26,6 +26,35 @@ import ( // Adapted from https://www.jvt.me/posts/2024/01/09/go-json-nullable/ type NullableAttr[T any] map[bool]T +// NullableRelationship is a generic type, which implements a field that can be one of three states: +// +// - relationship is not set in the request +// - relationship is explicitly set to `null` in the request +// - relationship is explicitly set to a valid relationship value in the request +// +// NullableRelationship is intended to be used with JSON marshalling and unmarshalling. +// This is generally useful for PATCH requests, where relationships with zero +// values are intentionally not marshaled into the request payload so that +// existing attribute values are not overwritten. +// +// Internal implementation details: +// +// - map[true]T means a value was provided +// - map[false]T means an explicit null was provided +// - nil or zero map means the field was not provided +// +// If the relationship is expected to be optional, add the `omitempty` JSON tags. Do NOT use `*NullableRelationship`! +// +// Slice types are not currently supported for NullableRelationships as the nullable nature can be expressed via empty array +// `polyrelation` JSON tags are NOT currently supported. +// +// NullableRelationships must have an inner type of pointer: +// +// - NullableRelationship[*Comment] - valid +// - NullableRelationship[[]*Comment] - invalid +// - NullableRelationship[Comment] - invalid +type NullableRelationship[T any] map[bool]T + // NewNullableAttrWithValue is a convenience helper to allow constructing a // NullableAttr with a given value, for instance to construct a field inside a // struct without introducing an intermediate variable. @@ -87,3 +116,65 @@ func (t NullableAttr[T]) IsSpecified() bool { func (t *NullableAttr[T]) SetUnspecified() { *t = map[bool]T{} } + +// NewNullableAttrWithValue is a convenience helper to allow constructing a +// NullableAttr with a given value, for instance to construct a field inside a +// struct without introducing an intermediate variable. +func NewNullableRelationshipWithValue[T any](t T) NullableRelationship[T] { + var n NullableRelationship[T] + n.Set(t) + return n +} + +// NewNullNullableAttr is a convenience helper to allow constructing a NullableAttr with +// an explicit `null`, for instance to construct a field inside a struct +// without introducing an intermediate variable +func NewNullNullableRelationship[T any]() NullableRelationship[T] { + var n NullableRelationship[T] + n.SetNull() + return n +} + +// Get retrieves the underlying value, if present, and returns an error if the value was not present +func (t NullableRelationship[T]) Get() (T, error) { + var empty T + if t.IsNull() { + return empty, errors.New("value is null") + } + if !t.IsSpecified() { + return empty, errors.New("value is not specified") + } + return t[true], nil +} + +// Set sets the underlying value to a given value +func (t *NullableRelationship[T]) Set(value T) { + *t = map[bool]T{true: value} +} + +// Set sets the underlying value to a given value +func (t *NullableRelationship[T]) SetInterface(value interface{}) { + t.Set(value.(T)) +} + +// IsNull indicates whether the field was sent, and had a value of `null` +func (t NullableRelationship[T]) IsNull() bool { + _, foundNull := t[false] + return foundNull +} + +// SetNull sets the value to an explicit `null` +func (t *NullableRelationship[T]) SetNull() { + var empty T + *t = map[bool]T{false: empty} +} + +// IsSpecified indicates whether the field was sent +func (t NullableRelationship[T]) IsSpecified() bool { + return len(t) != 0 +} + +// SetUnspecified sets the value to be absent from the serialized payload +func (t *NullableRelationship[T]) SetUnspecified() { + *t = map[bool]T{} +} diff --git a/request.go b/request.go index a21bc2b..e6f6749 100644 --- a/request.go +++ b/request.go @@ -488,10 +488,30 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ buf := bytes.NewBuffer(nil) - json.NewEncoder(buf).Encode( - data.Relationships[args[1]], - ) - json.NewDecoder(buf).Decode(relationship) + relDataStr := data.Relationships[args[1]] + json.NewEncoder(buf).Encode(relDataStr) + + isExplicitNull := false + relationshipDecodeErr := json.NewDecoder(buf).Decode(relationship) + if relationshipDecodeErr == nil && relationship.Data == nil { + // If the relationship was a valid node and relationship data was null + // this indicates disassociating the relationship + isExplicitNull = true + } else if relationshipDecodeErr != nil { + er = fmt.Errorf("decode err %v\n", relationshipDecodeErr) + } + + // This will hold either the value of the choice type model or the actual + // model, depending on annotation + m := reflect.New(fieldValue.Type().Elem()) + + // Nullable relationships have an extra pointer indirection + // unwind that here + if strings.HasPrefix(fieldType.Type.Name(), "NullableRelationship[") { + if m.Kind() == reflect.Ptr { + m = reflect.New(fieldValue.Type().Elem().Elem()) + } + } /* http://jsonapi.org/format/#document-resource-object-relationships @@ -500,6 +520,12 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ 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 field value to a map with a single entry + if isExplicitNull { + fieldValue.Set(reflect.MakeMapWithSize(fieldValue.Type(), 1)) + fieldValue.SetMapIndex(reflect.ValueOf(false), m) + } continue } @@ -510,9 +536,6 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ continue } - // This will hold either the value of the choice type model or the actual - // model, depending on annotation - m := reflect.New(fieldValue.Type().Elem()) // Check if the item in the relationship was already processed elsewhere. Avoids potential infinite recursive loops // caused by circular references between included relationships (two included items include one another) @@ -537,7 +560,12 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ break } - fieldValue.Set(m) + if strings.HasPrefix(fieldType.Type.Name(), "NullableRelationship[") { + fieldValue.Set(reflect.MakeMapWithSize(fieldValue.Type(), 1)) + fieldValue.SetMapIndex(reflect.ValueOf(true), m) + } else { + fieldValue.Set(m) + } } } else if annotation == annotationLinks { if data.Links == nil { diff --git a/request_test.go b/request_test.go index 4c11a19..56c407e 100644 --- a/request_test.go +++ b/request_test.go @@ -8,6 +8,7 @@ import ( "io" "reflect" "sort" + "strconv" "strings" "testing" "time" @@ -382,6 +383,127 @@ func TestUnmarshalNullableBool(t *testing.T) { } } +func TestUnmarshalNullableRelationshipsNonNullValue(t *testing.T) { + comment := &Comment{ + ID: 5, + Body: "Hello World", + } + + payload := &OnePayload{ + Data: &Node{ + ID: "10", + Type: "with-nullables", + Relationships: map[string]interface{}{ + "nullable_comment": &RelationshipOneNode{ + Data: &Node{ + Type: "comments", + ID: strconv.Itoa(comment.ID), + }, + }, + }, + }, + } + + outBuf := bytes.NewBuffer(nil) + json.NewEncoder(outBuf).Encode(payload) + + out := new(WithNullableAttrs) + + if err := UnmarshalPayload(outBuf, out); err != nil { + t.Fatal(err) + } + + nullableCommentOpt := out.NullableComment + if !nullableCommentOpt.IsSpecified() { + t.Fatal("Expected NullableComment to be specified") + } + + nullableComment, err := nullableCommentOpt.Get() + if err != nil { + t.Fatal(err) + } + + if expected, actual := comment.ID, nullableComment.ID; expected != actual { + t.Fatalf("Was expecting NullableComment to be `%d`, got `%d`", expected, actual) + } +} + +func TestUnmarshalNullableRelationshipsExplicitNullValue(t *testing.T) { + payload := &OnePayload{ + Data: &Node{ + ID: "10", + Type: "with-nullables", + Relationships: map[string]interface{}{ + "nullable_comment": &RelationshipOneNode{ + Data: nil, + }, + }, + }, + } + + outBuf := bytes.NewBuffer(nil) + json.NewEncoder(outBuf).Encode(payload) + + out := new(WithNullableAttrs) + + if err := UnmarshalPayload(outBuf, out); err != nil { + t.Fatal(err) + } + + nullableCommentOpt := out.NullableComment + if !nullableCommentOpt.IsSpecified() || !nullableCommentOpt.IsNull() { + t.Fatal("Expected NullableComment to be specified and explicit null") + } + +} + +func TestUnmarshalNullableRelationshipsNonExistentValue(t *testing.T) { + payload := &OnePayload{ + Data: &Node{ + ID: "10", + Type: "with-nullables", + Relationships: map[string]interface{}{}, + }, + } + + outBuf := bytes.NewBuffer(nil) + json.NewEncoder(outBuf).Encode(payload) + + out := new(WithNullableAttrs) + + if err := UnmarshalPayload(outBuf, out); err != nil { + t.Fatal(err) + } + + nullableCommentOpt := out.NullableComment + if nullableCommentOpt.IsSpecified() || nullableCommentOpt.IsNull() { + t.Fatal("Expected NullableComment to NOT be specified and NOT be explicit null") + } +} + +func TestUnmarshalNullableRelationshipsNoRelationships(t *testing.T) { + payload := &OnePayload{ + Data: &Node{ + ID: "10", + Type: "with-nullables", + }, + } + + outBuf := bytes.NewBuffer(nil) + json.NewEncoder(outBuf).Encode(payload) + + out := new(WithNullableAttrs) + + if err := UnmarshalPayload(outBuf, out); err != nil { + t.Fatal(err) + } + + nullableCommentOpt := out.NullableComment + if nullableCommentOpt.IsSpecified() || nullableCommentOpt.IsNull() { + t.Fatal("Expected NullableComment to NOT be specified and NOT be explicit null") + } +} + func TestMalformedTag(t *testing.T) { out := new(BadModel) err := UnmarshalPayload(samplePayload(), out) diff --git a/response.go b/response.go index f22358b..c06921f 100644 --- a/response.go +++ b/response.go @@ -253,7 +253,7 @@ func visitModelNodeAttribute(args []string, node *Node, fieldValue reflect.Value node.Attributes = make(map[string]interface{}) } - // Handle Nullable[T] + // Handle NullableAttr[T] if strings.HasPrefix(fieldValue.Type().Name(), "NullableAttr[") { // handle unspecified if fieldValue.IsNil() { @@ -390,6 +390,27 @@ func visitModelNodeRelation(model any, annotation string, args []string, node *N omitEmpty = args[2] == annotationOmitEmpty } + if node.Relationships == nil { + node.Relationships = make(map[string]interface{}) + } + + // Handle NullableRelationship[T] + if strings.HasPrefix(fieldValue.Type().Name(), "NullableRelationship[") { + + if fieldValue.MapIndex(reflect.ValueOf(false)).IsValid() { + innerTypeIsSlice := fieldValue.MapIndex(reflect.ValueOf(false)).Type().Kind() == reflect.Slice + // handle explicit null + if innerTypeIsSlice { + node.Relationships[args[1]] = json.RawMessage("[]") + } else { + node.Relationships[args[1]] = json.RawMessage("{\"data\":null}") + } + } else if fieldValue.MapIndex(reflect.ValueOf(true)).IsValid() { + // handle value + fieldValue = fieldValue.MapIndex(reflect.ValueOf(true)) + } + } + isSlice := fieldValue.Type().Kind() == reflect.Slice if omitEmpty && (isSlice && fieldValue.Len() < 1 || @@ -454,10 +475,6 @@ func visitModelNodeRelation(model any, annotation string, args []string, node *N } } - if node.Relationships == nil { - node.Relationships = make(map[string]interface{}) - } - var relLinks *Links if linkableModel, ok := model.(RelationshipLinkable); ok { relLinks = linkableModel.JSONAPIRelationshipLinks(args[1]) diff --git a/response_test.go b/response_test.go index 2691a1f..509b656 100644 --- a/response_test.go +++ b/response_test.go @@ -6,6 +6,7 @@ import ( "fmt" "reflect" "sort" + "strconv" "testing" "time" ) @@ -945,6 +946,87 @@ func TestMarshal_Times(t *testing.T) { } } +func TestNullableRelationship(t *testing.T) { + comment := &Comment{ + ID: 5, + Body: "Hello World", + } + + for _, tc := range []struct { + desc string + input *WithNullableAttrs + verification func(data map[string]interface{}) error + }{ + { + desc: "nullable_comment_unspecified", + input: &WithNullableAttrs{ + ID: 5, + NullableComment: nil, + }, + verification: func(root map[string]interface{}) error { + _, ok := root["data"].(map[string]interface{})["relationships"] + + if got, want := ok, false; got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + return nil + }, + }, + { + desc: "nullable_comment_null", + input: &WithNullableAttrs{ + ID: 5, + NullableComment: NewNullNullableRelationship[*Comment](), + }, + verification: func(root map[string]interface{}) error { + commentData, ok := root["data"].(map[string]interface{})["relationships"].(map[string]interface{})["nullable_comment"].(map[string]interface{})["data"] + + if got, want := ok, true; got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + + if commentData != nil { + return fmt.Errorf("Expected nil data for nullable_comment but was '%v'", commentData) + } + return nil + }, + }, + { + desc: "nullable_comment_not_null", + input: &WithNullableAttrs{ + ID: 5, + NullableComment: NewNullableRelationshipWithValue(comment), + }, + verification: func(root map[string]interface{}) error { + relationships := root["data"].(map[string]interface{})["relationships"] + nullableComment := relationships.(map[string]interface{})["nullable_comment"] + idStr := nullableComment.(map[string]interface{})["data"].(map[string]interface{})["id"].(string) + id, _ := strconv.Atoi(idStr) + if got, want := id, comment.ID; got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + return nil + }, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + out := bytes.NewBuffer(nil) + if err := MarshalPayload(out, tc.input); err != nil { + t.Fatal(err) + } + + // Use the standard JSON library to traverse the genereated JSON payload. + data := map[string]interface{}{} + json.Unmarshal(out.Bytes(), &data) + if tc.verification != nil { + if err := tc.verification(data); err != nil { + t.Fatal(err) + } + } + }) + } +} + func TestNullableAttr_Time(t *testing.T) { aTime := time.Date(2016, 8, 17, 8, 27, 12, 23849, time.UTC) From 9387e0dac7f4679ee041aca7d70cd784901a3bfc Mon Sep 17 00:00:00 2001 From: Netra Mali Date: Mon, 3 Feb 2025 15:24:47 -0500 Subject: [PATCH 2/6] add nil --- response.go | 1 + 1 file changed, 1 insertion(+) diff --git a/response.go b/response.go index c06921f..888d650 100644 --- a/response.go +++ b/response.go @@ -405,6 +405,7 @@ func visitModelNodeRelation(model any, annotation string, args []string, node *N } else { node.Relationships[args[1]] = json.RawMessage("{\"data\":null}") } + return nil } else if fieldValue.MapIndex(reflect.ValueOf(true)).IsValid() { // handle value fieldValue = fieldValue.MapIndex(reflect.ValueOf(true)) From 9eb4a9574d81261e5f9cfd08e13e9b25e600e2d6 Mon Sep 17 00:00:00 2001 From: Netra Mali Date: Wed, 5 Feb 2025 11:22:28 -0500 Subject: [PATCH 3/6] merge conflict --- models_test.go | 12 ++++++------ request.go | 7 ++----- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/models_test.go b/models_test.go index fe0ceef..02e5d71 100644 --- a/models_test.go +++ b/models_test.go @@ -36,12 +36,12 @@ type TimestampModel struct { } type WithNullableAttrs struct { - ID int `jsonapi:"primary,with-nullables"` - Name string `jsonapi:"attr,name"` - IntTime NullableAttr[time.Time] `jsonapi:"attr,int_time,omitempty"` - 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"` + ID int `jsonapi:"primary,with-nullables"` + Name string `jsonapi:"attr,name"` + IntTime NullableAttr[time.Time] `jsonapi:"attr,int_time,omitempty"` + 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"` } diff --git a/request.go b/request.go index e6f6749..48504dd 100644 --- a/request.go +++ b/request.go @@ -487,7 +487,6 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ relationship := new(RelationshipOneNode) buf := bytes.NewBuffer(nil) - relDataStr := data.Relationships[args[1]] json.NewEncoder(buf).Encode(relDataStr) @@ -498,7 +497,7 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ // this indicates disassociating the relationship isExplicitNull = true } else if relationshipDecodeErr != nil { - er = fmt.Errorf("decode err %v\n", relationshipDecodeErr) + er = fmt.Errorf("Could not unmarshal json: %w", relationshipDecodeErr) } // This will hold either the value of the choice type model or the actual @@ -512,7 +511,6 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ m = reflect.New(fieldValue.Type().Elem().Elem()) } } - /* http://jsonapi.org/format/#document-resource-object-relationships http://jsonapi.org/format/#document-resource-object-linkage @@ -522,7 +520,7 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ if relationship.Data == nil { // Explicit null supplied for the field value // If a nullable relationship we set the field value to a map with a single entry - if isExplicitNull { + if isExplicitNull && strings.HasPrefix(fieldType.Type.Name(), "NullableRelationship[") { fieldValue.Set(reflect.MakeMapWithSize(fieldValue.Type(), 1)) fieldValue.SetMapIndex(reflect.ValueOf(false), m) } @@ -536,7 +534,6 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*includ continue } - // Check if the item in the relationship was already processed elsewhere. Avoids potential infinite recursive loops // caused by circular references between included relationships (two included items include one another) includedKey := fmt.Sprintf("%s,%s", relationship.Data.Type, relationship.Data.ID) From 1ec1b74506d733b6718ae2ab0cc6650cdd029733 Mon Sep 17 00:00:00 2001 From: Netra Mali <104793044+netramali@users.noreply.github.com> Date: Wed, 5 Feb 2025 10:46:46 -0500 Subject: [PATCH 4/6] Update nullable.go Co-authored-by: Chris Trombley --- nullable.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nullable.go b/nullable.go index b7552f8..c54fe26 100644 --- a/nullable.go +++ b/nullable.go @@ -117,8 +117,8 @@ func (t *NullableAttr[T]) SetUnspecified() { *t = map[bool]T{} } -// NewNullableAttrWithValue is a convenience helper to allow constructing a -// NullableAttr with a given value, for instance to construct a field inside a +// NewNullableRelationshipWithValue is a convenience helper to allow constructing a +// NullableRelationship with a given value, for instance to construct a field inside a // struct without introducing an intermediate variable. func NewNullableRelationshipWithValue[T any](t T) NullableRelationship[T] { var n NullableRelationship[T] From 8c58fdb3896167fd4d3f4ce5e9e936d610094a98 Mon Sep 17 00:00:00 2001 From: Netra Mali <104793044+netramali@users.noreply.github.com> Date: Wed, 5 Feb 2025 10:47:15 -0500 Subject: [PATCH 5/6] Update nullable.go Co-authored-by: Chris Trombley --- nullable.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullable.go b/nullable.go index c54fe26..db5a29f 100644 --- a/nullable.go +++ b/nullable.go @@ -126,7 +126,7 @@ func NewNullableRelationshipWithValue[T any](t T) NullableRelationship[T] { return n } -// NewNullNullableAttr is a convenience helper to allow constructing a NullableAttr with +// NewNullNullableRelationship is a convenience helper to allow constructing a NullableRelationship with // an explicit `null`, for instance to construct a field inside a struct // without introducing an intermediate variable func NewNullNullableRelationship[T any]() NullableRelationship[T] { From e123c06fd587576d5e98a50dd92fa91b72968253 Mon Sep 17 00:00:00 2001 From: Netra Mali <104793044+netramali@users.noreply.github.com> Date: Wed, 5 Feb 2025 10:47:25 -0500 Subject: [PATCH 6/6] Update nullable.go Co-authored-by: Chris Trombley --- nullable.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nullable.go b/nullable.go index db5a29f..7a66149 100644 --- a/nullable.go +++ b/nullable.go @@ -152,7 +152,8 @@ func (t *NullableRelationship[T]) Set(value T) { *t = map[bool]T{true: value} } -// Set sets the underlying value to a given value +// SetInterface sets the underlying value from an empty interface, +// performing a type assertion to T. func (t *NullableRelationship[T]) SetInterface(value interface{}) { t.Set(value.(T)) }