From 279c180d85cedfb9591e5900d0cad52637220d93 Mon Sep 17 00:00:00 2001 From: Netra Mali Date: Fri, 24 Jan 2025 15:52:55 -0500 Subject: [PATCH 1/6] merge conflict --- models_test.go | 14 +++-- nullable.go | 91 +++++++++++++++++++++++++++++ request.go | 55 +++++++++++++++--- request_test.go | 146 +++++++++++++++++++++++++++++++++++++++++++++++ response.go | 28 +++++++-- response_test.go | 141 +++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 455 insertions(+), 20 deletions(-) diff --git a/models_test.go b/models_test.go index df3b43b..3a42f2e 100644 --- a/models_test.go +++ b/models_test.go @@ -36,12 +36,14 @@ 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"` + NullableComments NullableRelationship[[]*Comment] `jsonapi:"relation,nullable_comments,omitempty"` } type Car struct { diff --git a/nullable.go b/nullable.go index 68910f6..61bec6f 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 allowed for NullableRelationships. +// `polyrelation` JSON tags are NOT currently supported. +// +// NullableRelationships must have an inner type of pointer: +// +// - NullableRelationship[*Comment] - valid +// - NullableRelationship[[]*Comment] - valid +// - 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 indicate 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 27f628e..46e101f 100644 --- a/request.go +++ b/request.go @@ -467,6 +467,14 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) // model, depending on annotation m := reflect.New(sliceType.Elem().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(sliceType.Elem().Elem().Elem()) + } + } + err = unmarshalNodeMaybeChoice(&m, n, annotation, choiceMapping, included) if err != nil { er = err @@ -483,10 +491,30 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) 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 + if err := json.NewDecoder(buf).Decode(relationship); err != nil { + // We couldn't decode the data into the relationship type + // check if this is a string "null" which indicates + // disassociating the relationship + if relDataStr == "null" { + isExplicitNull = true + } + } + + // 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 @@ -495,6 +523,14 @@ 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 + if isExplicitNull && strings.HasPrefix(fieldType.Type.Name(), "NullableRelationship[") { + fieldValue.Set(reflect.MakeMapWithSize(fieldValue.Type(), 1)) + fieldValue.SetMapIndex(reflect.ValueOf(false), m) + } + continue } @@ -505,17 +541,18 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) 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()) - err = unmarshalNodeMaybeChoice(&m, relationship.Data, annotation, choiceMapping, included) if err != nil { er = err 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 1408ad9..90a5dd1 100644 --- a/request_test.go +++ b/request_test.go @@ -8,6 +8,7 @@ import ( "io" "reflect" "sort" + "strconv" "strings" "testing" "time" @@ -382,6 +383,151 @@ 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 TestUnmarshalNullableRelationshipsNullStringValue(t *testing.T) { + payload := &OnePayload{ + Data: &Node{ + ID: "10", + Type: "with-nullables", + Relationships: map[string]interface{}{ + "nullable_comment": "null", + }, + }, + } + + 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 TestUnmarshalNullableRelationshipsNilValue(t *testing.T) { + payload := &OnePayload{ + Data: &Node{ + ID: "10", + Type: "with-nullables", + Relationships: map[string]interface{}{ + "nullable_comment": 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 NOT be specified and NOT be 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 cc989f2..3fe4d64 100644 --- a/response.go +++ b/response.go @@ -241,7 +241,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() { @@ -353,6 +353,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 || @@ -417,10 +438,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]) @@ -711,3 +728,4 @@ func convertToSliceInterface(i *interface{}) ([]interface{}, error) { } return response, nil } + diff --git a/response_test.go b/response_test.go index 2691a1f..ab5cc0a 100644 --- a/response_test.go +++ b/response_test.go @@ -6,6 +6,7 @@ import ( "fmt" "reflect" "sort" + "strconv" "testing" "time" ) @@ -1112,6 +1113,146 @@ func TestNullableAttr_Bool(t *testing.T) { } } +func TestNullableRelationship(t *testing.T) { + comment := &Comment{ + ID: 5, + Body: "Hello World", + } + + comments := []*Comment{ + { + ID: 6, + Body: "Hello World", + }, + { + ID: 7, + 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 { + _, ok := root["data"].(map[string]interface{})["relationships"].(map[string]interface{})["nullable_comment"] + + if got, want := ok, true; got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + 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 + }, + }, + { + desc: "nullable_comments_unspecified", + input: &WithNullableAttrs{ + ID: 5, + NullableComments: 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_comments_null", + input: &WithNullableAttrs{ + ID: 5, + NullableComments: NewNullNullableRelationship[[]*Comment](), + }, + verification: func(root map[string]interface{}) error { + _, ok := root["data"].(map[string]interface{})["relationships"].(map[string]interface{})["nullable_comments"] + + if got, want := ok, true; got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + return nil + }, + }, + { + desc: "nullable_comments_not_null", + input: &WithNullableAttrs{ + ID: 5, + NullableComments: NewNullableRelationshipWithValue(comments), + }, + verification: func(root map[string]interface{}) error { + relationships := root["data"].(map[string]interface{})["relationships"] + nullableComments := relationships.(map[string]interface{})["nullable_comments"].(map[string]interface{})["data"].([]interface{}) + + for i := 0; i < len(comments); i++ { + c := nullableComments[i].(map[string]interface{}) + idStr := c["id"].(string) + id, _ := strconv.Atoi(idStr) + if got, want := id, comments[i].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 TestSupportsLinkable(t *testing.T) { testModel := &Blog{ ID: 5, From 4a11a30b870346c2e946d0e86c9954d4e0e51549 Mon Sep 17 00:00:00 2001 From: Netra Mali Date: Fri, 24 Jan 2025 15:54:37 -0500 Subject: [PATCH 2/6] merge conflict --- models_test.go | 15 ++-- nullable.go | 4 +- request.go | 22 ++---- request_test.go | 32 +------- response.go | 192 ++++++++++++++++++++++++++++++++++++++++++++++- response_test.go | 69 ++--------------- 6 files changed, 214 insertions(+), 120 deletions(-) diff --git a/models_test.go b/models_test.go index 3a42f2e..0961e1b 100644 --- a/models_test.go +++ b/models_test.go @@ -36,14 +36,13 @@ 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"` - NullableComment NullableRelationship[*Comment] `jsonapi:"relation,nullable_comment,omitempty"` - NullableComments NullableRelationship[[]*Comment] `jsonapi:"relation,nullable_comments,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"` } type Car struct { diff --git a/nullable.go b/nullable.go index 61bec6f..7bf2c0c 100644 --- a/nullable.go +++ b/nullable.go @@ -45,13 +45,13 @@ type NullableAttr[T any] map[bool]T // // If the relationship is expected to be optional, add the `omitempty` JSON tags. Do NOT use `*NullableRelationship`! // -// Slice types are allowed for NullableRelationships. +// 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] - valid +// - NullableRelationship[[]*Comment] - invalid // - NullableRelationship[Comment] - invalid type NullableRelationship[T any] map[bool]T diff --git a/request.go b/request.go index 46e101f..0fed712 100644 --- a/request.go +++ b/request.go @@ -467,14 +467,6 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) // model, depending on annotation m := reflect.New(sliceType.Elem().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(sliceType.Elem().Elem().Elem()) - } - } - err = unmarshalNodeMaybeChoice(&m, n, annotation, choiceMapping, included) if err != nil { er = err @@ -495,13 +487,13 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) json.NewEncoder(buf).Encode(relDataStr) isExplicitNull := false - if err := json.NewDecoder(buf).Decode(relationship); err != nil { - // We couldn't decode the data into the relationship type - // check if this is a string "null" which indicates - // disassociating the relationship - if relDataStr == "null" { - isExplicitNull = true - } + 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 { + fmt.Printf("decode err %v\n", relationshipDecodeErr) } // This will hold either the value of the choice type model or the actual diff --git a/request_test.go b/request_test.go index 90a5dd1..7fa0d3e 100644 --- a/request_test.go +++ b/request_test.go @@ -428,13 +428,15 @@ func TestUnmarshalNullableRelationshipsNonNullValue(t *testing.T) { } } -func TestUnmarshalNullableRelationshipsNullStringValue(t *testing.T) { +func TestUnmarshalNullableRelationshipsExplicitNullValue(t *testing.T) { payload := &OnePayload{ Data: &Node{ ID: "10", Type: "with-nullables", Relationships: map[string]interface{}{ - "nullable_comment": "null", + "nullable_comment": &RelationshipOneNode{ + Data: nil, + }, }, }, } @@ -455,32 +457,6 @@ func TestUnmarshalNullableRelationshipsNullStringValue(t *testing.T) { } -func TestUnmarshalNullableRelationshipsNilValue(t *testing.T) { - payload := &OnePayload{ - Data: &Node{ - ID: "10", - Type: "with-nullables", - Relationships: map[string]interface{}{ - "nullable_comment": 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 NOT be specified and NOT be explicit null") - } -} - func TestUnmarshalNullableRelationshipsNonExistentValue(t *testing.T) { payload := &OnePayload{ Data: &Node{ diff --git a/response.go b/response.go index 3fe4d64..624e05d 100644 --- a/response.go +++ b/response.go @@ -374,6 +374,27 @@ func visitModelNodeRelation(model any, annotation string, args []string, node *N } } + 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 || @@ -616,9 +637,174 @@ func visitModelNode(model interface{}, included *map[string]*Node, break } } else if annotation == annotationRelation || annotation == annotationPolyRelation { - er = visitModelNodeRelation(model, annotation, args, node, fieldValue, included, sideload) - if er != nil { - break + var omitEmpty bool + + //add support for 'omitempty' struct tag for marshaling as absent + if len(args) > 2 { + omitEmpty = args[2] == annotationOmitEmpty + } + + // 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}") + } + continue + } 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 || + (!isSlice && fieldValue.IsNil())) { + continue + } + + if annotation == annotationPolyRelation { + // for polyrelation, we'll snoop out the actual relation model + // through the choice type value by choosing the first non-nil + // field that has a jsonapi type annotation and overwriting + // `fieldValue` so normal annotation-assisted marshaling + // can continue + if !isSlice { + choiceValue := fieldValue + + // must be a pointer type + if choiceValue.Type().Kind() != reflect.Ptr { + er = ErrUnexpectedType + break + } + + if choiceValue.IsNil() { + fieldValue = reflect.ValueOf(nil) + } + structValue := choiceValue.Elem() + + // Short circuit if field is omitted from model + if !structValue.IsValid() { + break + } + + if found, err := selectChoiceTypeStructField(structValue); err == nil { + fieldValue = found + } + } else { + // A slice polyrelation field can be... polymorphic... meaning + // that we might snoop different types within each slice element. + // Each snooped value will added to this collection and then + // the recursion will take care of the rest. The only special case + // is nil. For that, we'll just choose the first + collection := make([]interface{}, 0) + + for i := 0; i < fieldValue.Len(); i++ { + itemValue := fieldValue.Index(i) + // Once again, must be a pointer type + if itemValue.Type().Kind() != reflect.Ptr { + er = ErrUnexpectedType + break + } + + if itemValue.IsNil() { + er = ErrUnexpectedNil + break + } + + structValue := itemValue.Elem() + + if found, err := selectChoiceTypeStructField(structValue); err == nil { + collection = append(collection, found.Interface()) + } + } + + if er != nil { + break + } + + fieldValue = reflect.ValueOf(collection) + } + } + + var relLinks *Links + if linkableModel, ok := model.(RelationshipLinkable); ok { + relLinks = linkableModel.JSONAPIRelationshipLinks(args[1]) + } + + var relMeta *Meta + if metableModel, ok := model.(RelationshipMetable); ok { + relMeta = metableModel.JSONAPIRelationshipMeta(args[1]) + } + + if isSlice { + // to-many relationship + relationship, err := visitModelNodeRelationships( + fieldValue, + included, + sideload, + ) + if err != nil { + er = err + break + } + relationship.Links = relLinks + relationship.Meta = relMeta + + if sideload { + shallowNodes := []*Node{} + for _, n := range relationship.Data { + appendIncluded(included, n) + shallowNodes = append(shallowNodes, toShallowNode(n)) + } + + node.Relationships[args[1]] = &RelationshipManyNode{ + Data: shallowNodes, + Links: relationship.Links, + Meta: relationship.Meta, + } + } else { + node.Relationships[args[1]] = relationship + } + } else { + // to-one relationships + + // Handle null relationship case + if fieldValue.IsNil() { + node.Relationships[args[1]] = &RelationshipOneNode{Data: nil} + continue + } + + relationship, err := visitModelNode( + fieldValue.Interface(), + included, + sideload, + ) + if err != nil { + er = err + break + } + + if sideload { + appendIncluded(included, relationship) + node.Relationships[args[1]] = &RelationshipOneNode{ + Data: toShallowNode(relationship), + Links: relLinks, + Meta: relMeta, + } + } else { + node.Relationships[args[1]] = &RelationshipOneNode{ + Data: relationship, + Links: relLinks, + Meta: relMeta, + } + } } } else if annotation == annotationLinks { // Nothing. Ignore this field, as Links fields are only for unmarshaling requests. diff --git a/response_test.go b/response_test.go index ab5cc0a..5d1c995 100644 --- a/response_test.go +++ b/response_test.go @@ -1119,17 +1119,6 @@ func TestNullableRelationship(t *testing.T) { Body: "Hello World", } - comments := []*Comment{ - { - ID: 6, - Body: "Hello World", - }, - { - ID: 7, - Body: "Hello World", - }, - } - for _, tc := range []struct { desc string input *WithNullableAttrs @@ -1157,11 +1146,15 @@ func TestNullableRelationship(t *testing.T) { NullableComment: NewNullNullableRelationship[*Comment](), }, verification: func(root map[string]interface{}) error { - _, ok := root["data"].(map[string]interface{})["relationships"].(map[string]interface{})["nullable_comment"] + 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 }, }, @@ -1182,58 +1175,6 @@ func TestNullableRelationship(t *testing.T) { return nil }, }, - { - desc: "nullable_comments_unspecified", - input: &WithNullableAttrs{ - ID: 5, - NullableComments: 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_comments_null", - input: &WithNullableAttrs{ - ID: 5, - NullableComments: NewNullNullableRelationship[[]*Comment](), - }, - verification: func(root map[string]interface{}) error { - _, ok := root["data"].(map[string]interface{})["relationships"].(map[string]interface{})["nullable_comments"] - - if got, want := ok, true; got != want { - return fmt.Errorf("got %v, want %v", got, want) - } - return nil - }, - }, - { - desc: "nullable_comments_not_null", - input: &WithNullableAttrs{ - ID: 5, - NullableComments: NewNullableRelationshipWithValue(comments), - }, - verification: func(root map[string]interface{}) error { - relationships := root["data"].(map[string]interface{})["relationships"] - nullableComments := relationships.(map[string]interface{})["nullable_comments"].(map[string]interface{})["data"].([]interface{}) - - for i := 0; i < len(comments); i++ { - c := nullableComments[i].(map[string]interface{}) - idStr := c["id"].(string) - id, _ := strconv.Atoi(idStr) - if got, want := id, comments[i].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) From ccc60b322cbfb071a4896723c500f0d1a3beeb27 Mon Sep 17 00:00:00 2001 From: Netra Mali Date: Thu, 23 Jan 2025 15:41:17 -0500 Subject: [PATCH 3/6] addressed comment --- nullable.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullable.go b/nullable.go index 7bf2c0c..b7552f8 100644 --- a/nullable.go +++ b/nullable.go @@ -157,7 +157,7 @@ func (t *NullableRelationship[T]) SetInterface(value interface{}) { t.Set(value.(T)) } -// IsNull indicate whether the field was sent, and had a value of `null` +// IsNull indicates whether the field was sent, and had a value of `null` func (t NullableRelationship[T]) IsNull() bool { _, foundNull := t[false] return foundNull From f63a032a8aebfee3116ffcfada40c00db9290c9f Mon Sep 17 00:00:00 2001 From: Netra Mali Date: Fri, 24 Jan 2025 10:15:36 -0500 Subject: [PATCH 4/6] addressed comments --- request.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/request.go b/request.go index 0fed712..35cb590 100644 --- a/request.go +++ b/request.go @@ -493,7 +493,7 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) // this indicates disassociating the relationship isExplicitNull = true } else if relationshipDecodeErr != nil { - fmt.Printf("decode err %v\n", relationshipDecodeErr) + er = fmt.Errorf("decode err %v\n", relationshipDecodeErr) } // This will hold either the value of the choice type model or the actual @@ -517,11 +517,11 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) if relationship.Data == nil { // Explicit null supplied for the field value - // If a nullable relationship we set the - if isExplicitNull && strings.HasPrefix(fieldType.Type.Name(), "NullableRelationship[") { - fieldValue.Set(reflect.MakeMapWithSize(fieldValue.Type(), 1)) - fieldValue.SetMapIndex(reflect.ValueOf(false), m) - } + // 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 } From 5dd72ceddf04005d5de9ee60519f07f476cabe58 Mon Sep 17 00:00:00 2001 From: Netra Mali <104793044+netramali@users.noreply.github.com> Date: Fri, 31 Jan 2025 14:51:26 -0500 Subject: [PATCH 5/6] Update response.go --- response.go | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/response.go b/response.go index 624e05d..ff4a718 100644 --- a/response.go +++ b/response.go @@ -374,27 +374,6 @@ func visitModelNodeRelation(model any, annotation string, args []string, node *N } } - 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 || From 5814468871158db3ca39401d36c214c7e5ba7a45 Mon Sep 17 00:00:00 2001 From: Netra Mali <104793044+netramali@users.noreply.github.com> Date: Fri, 31 Jan 2025 15:13:10 -0500 Subject: [PATCH 6/6] Update response.go --- response.go | 171 +--------------------------------------------------- 1 file changed, 3 insertions(+), 168 deletions(-) diff --git a/response.go b/response.go index ff4a718..3fe4d64 100644 --- a/response.go +++ b/response.go @@ -616,174 +616,9 @@ func visitModelNode(model interface{}, included *map[string]*Node, break } } else if annotation == annotationRelation || annotation == annotationPolyRelation { - var omitEmpty bool - - //add support for 'omitempty' struct tag for marshaling as absent - if len(args) > 2 { - omitEmpty = args[2] == annotationOmitEmpty - } - - // 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}") - } - continue - } 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 || - (!isSlice && fieldValue.IsNil())) { - continue - } - - if annotation == annotationPolyRelation { - // for polyrelation, we'll snoop out the actual relation model - // through the choice type value by choosing the first non-nil - // field that has a jsonapi type annotation and overwriting - // `fieldValue` so normal annotation-assisted marshaling - // can continue - if !isSlice { - choiceValue := fieldValue - - // must be a pointer type - if choiceValue.Type().Kind() != reflect.Ptr { - er = ErrUnexpectedType - break - } - - if choiceValue.IsNil() { - fieldValue = reflect.ValueOf(nil) - } - structValue := choiceValue.Elem() - - // Short circuit if field is omitted from model - if !structValue.IsValid() { - break - } - - if found, err := selectChoiceTypeStructField(structValue); err == nil { - fieldValue = found - } - } else { - // A slice polyrelation field can be... polymorphic... meaning - // that we might snoop different types within each slice element. - // Each snooped value will added to this collection and then - // the recursion will take care of the rest. The only special case - // is nil. For that, we'll just choose the first - collection := make([]interface{}, 0) - - for i := 0; i < fieldValue.Len(); i++ { - itemValue := fieldValue.Index(i) - // Once again, must be a pointer type - if itemValue.Type().Kind() != reflect.Ptr { - er = ErrUnexpectedType - break - } - - if itemValue.IsNil() { - er = ErrUnexpectedNil - break - } - - structValue := itemValue.Elem() - - if found, err := selectChoiceTypeStructField(structValue); err == nil { - collection = append(collection, found.Interface()) - } - } - - if er != nil { - break - } - - fieldValue = reflect.ValueOf(collection) - } - } - - var relLinks *Links - if linkableModel, ok := model.(RelationshipLinkable); ok { - relLinks = linkableModel.JSONAPIRelationshipLinks(args[1]) - } - - var relMeta *Meta - if metableModel, ok := model.(RelationshipMetable); ok { - relMeta = metableModel.JSONAPIRelationshipMeta(args[1]) - } - - if isSlice { - // to-many relationship - relationship, err := visitModelNodeRelationships( - fieldValue, - included, - sideload, - ) - if err != nil { - er = err - break - } - relationship.Links = relLinks - relationship.Meta = relMeta - - if sideload { - shallowNodes := []*Node{} - for _, n := range relationship.Data { - appendIncluded(included, n) - shallowNodes = append(shallowNodes, toShallowNode(n)) - } - - node.Relationships[args[1]] = &RelationshipManyNode{ - Data: shallowNodes, - Links: relationship.Links, - Meta: relationship.Meta, - } - } else { - node.Relationships[args[1]] = relationship - } - } else { - // to-one relationships - - // Handle null relationship case - if fieldValue.IsNil() { - node.Relationships[args[1]] = &RelationshipOneNode{Data: nil} - continue - } - - relationship, err := visitModelNode( - fieldValue.Interface(), - included, - sideload, - ) - if err != nil { - er = err - break - } - - if sideload { - appendIncluded(included, relationship) - node.Relationships[args[1]] = &RelationshipOneNode{ - Data: toShallowNode(relationship), - Links: relLinks, - Meta: relMeta, - } - } else { - node.Relationships[args[1]] = &RelationshipOneNode{ - Data: relationship, - Links: relLinks, - Meta: relMeta, - } - } + er = visitModelNodeRelation(model, annotation, args, node, fieldValue, included, sideload) + if er != nil { + break } } else if annotation == annotationLinks { // Nothing. Ignore this field, as Links fields are only for unmarshaling requests.