From 4965d30d30143dbda9a1de02e0bb32d08a8bb92d Mon Sep 17 00:00:00 2001 From: Omar Ismail Date: Tue, 20 Apr 2021 10:14:18 -0400 Subject: [PATCH 01/49] Add ability to unmarshal slice of pointers --- models_test.go | 6 +++++ request.go | 25 +++++++++++++++++++++ request_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/models_test.go b/models_test.go index f552d4f..408e7a8 100644 --- a/models_test.go +++ b/models_test.go @@ -165,9 +165,15 @@ type Company struct { Name string `jsonapi:"attr,name"` Boss Employee `jsonapi:"attr,boss"` Teams []Team `jsonapi:"attr,teams"` + People []*People `jsonapi:"attr,people"` FoundedAt time.Time `jsonapi:"attr,founded-at,iso8601"` } +type People struct { + Name string `jsonapi:"attr,name"` + Age int `jsonapi:"attr,age"` +} + type Team struct { Name string `jsonapi:"attr,name"` Leader *Employee `jsonapi:"attr,leader"` diff --git a/request.go b/request.go index f665857..1e1d485 100644 --- a/request.go +++ b/request.go @@ -416,6 +416,12 @@ func unmarshalAttribute( return } + if fieldValue.Type().Kind() == reflect.Slice && + reflect.TypeOf(fieldValue.Interface()).Elem().Kind() == reflect.Ptr { + value, err = handleStructPointerSlice(attribute, args, fieldValue) + return + } + // JSON value was a float (numeric) if value.Kind() == reflect.Float64 { value, err = handleNumeric(attribute, fieldType, fieldValue) @@ -654,3 +660,22 @@ func handleStructSlice( return models, nil } + +func handleStructPointerSlice( + attribute interface{}, + args []string, + fieldValue reflect.Value) (reflect.Value, error) { + + dataMap := reflect.ValueOf(attribute).Interface().([]interface{}) + models := reflect.New(fieldValue.Type()).Elem() + for _, data := range dataMap { + model := reflect.New(fieldValue.Type().Elem()).Elem() + value, err := handleStruct(data, model) + if err != nil { + continue + } + + models = reflect.Append(models, value) + } + return models, nil +} diff --git a/request_test.go b/request_test.go index 300c7de..5911120 100644 --- a/request_test.go +++ b/request_test.go @@ -1355,7 +1355,6 @@ func TestUnmarshalNestedStruct(t *testing.T) { } func TestUnmarshalNestedStructSlice(t *testing.T) { - fry := map[string]interface{}{ "firstname": "Philip J.", "surname": "Fry", @@ -1416,3 +1415,60 @@ func TestUnmarshalNestedStructSlice(t *testing.T) { out.Teams[0].Members[0].Firstname) } } + +func TestUnmarshalNestedStructPointerSlice(t *testing.T) { + personA := map[string]interface{}{ + "name": "persona", + "age": 25, + } + + personB := map[string]interface{}{ + "name": "personb", + "age": 19, + } + + sample := map[string]interface{}{ + "data": map[string]interface{}{ + "type": "companies", + "id": "123", + "attributes": map[string]interface{}{ + "name": "Planet Express", + "people": []interface{}{ + personA, + personB, + }, + }, + }, + } + + data, err := json.Marshal(sample) + if err != nil { + t.Fatal(err) + } + in := bytes.NewReader(data) + out := new(Company) + + if err := UnmarshalPayload(in, out); err != nil { + t.Fatal(err) + } + + if len(out.People) != 2 { + t.Fatalf("Length of people should be 2, but is instead %d", len(out.People)) + } + + if out.People[0].Name != "persona" { + t.Fatalf("Nested pointer struct not unmarshalled: Expected `persona` but got `%s`", out.People[0].Name) + } + + if out.People[0].Age != 25 { + t.Fatalf("Nested pointer struct not unmarshalled: Expected `25` but got `%d`", out.People[0].Age) + } + + if out.People[1].Name != "personb" { + t.Fatalf("Nested pointer struct not unmarshalled: Expected `personb` but got `%s`", out.People[1].Name) + } + + if out.People[1].Age != 19 { + t.Fatalf("Nested pointer struct not unmarshalled: Expected `19` but got `%d`", out.People[1].Age) + } +} From edf82c9774bf7d032e286c2699be69c536cf06c8 Mon Sep 17 00:00:00 2001 From: Omar Ismail Date: Tue, 20 Apr 2021 11:19:30 -0400 Subject: [PATCH 02/49] update go mod --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index d1fb59d..f8ae3f6 100644 --- a/go.mod +++ b/go.mod @@ -1 +1 @@ -module github.com/google/jsonapi \ No newline at end of file +module github.com/hashicorp/jsonapi From 7827d7ad9e7e07ad255c0b5819d20d7ef992f987 Mon Sep 17 00:00:00 2001 From: Chris Arcand Date: Tue, 11 May 2021 17:37:29 -0500 Subject: [PATCH 03/49] Unmarshal Links type to 'links' annotated struct fields These changes add the ability to unmarshal links members to links-annotated struct fields. The specification for links members can be found here: https://jsonapi.org/format/#document-links Note that this does not change the existing marshaling behavior of links (the Linkable interface methods), though a fully fleshed out implementation probably should - it's awkward that now interface methods are used to marshal but struct fields for unmarshaling. In other words, an implementation of links marshaling similar to the implementation proposed in PR 58 of the canonical google/jsonapi repository would be most appropriate to pair with these unmarshaling changes. The actual implementation that was accepted is PR 57, utilizing those Linkable/Metable interfaces. From 57: > After looking at both implementations, I think I'd like to vote for this one because it feels flexible, and doesn't add any additional fields to our structs. In general I don't think LINK and META objects really have much to do with the resource objects themselves, they are really more related to the context in which the object is used. This approach keeps structs cleaner. This is subjectively wrong, and assumes that this package is used in the context of a server and not a client. In a client, links members can contain information that is vital to the functionality of the resource itself, so we add that here. --- README.md | 12 ++++++++ constants.go | 5 ++++ models_test.go | 6 ++++ request.go | 41 ++++++++++++++++++++++++++ request_test.go | 78 +++++++++++++++++++++++++++++++++++++++++++++++++ response.go | 4 ++- 6 files changed, 145 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8dfb943..99327e6 100644 --- a/README.md +++ b/README.md @@ -179,6 +179,18 @@ used as the key in the `relationships` hash for the record. The optional third argument is `omitempty` - if present will prevent non existent to-one and to-many from being serialized. +#### `links` + +*Note: This annotation is an added feature independent of the canonical google/jsonapi package* + +``` +`jsonapi:"links,omitempty"` +``` + +A field annotated with `links` will have the links members of the request unmarshaled to it. Note +that this field should _always_ be annotated with `omitempty`, as marshaling of links members is +instead handled by the `Linkable` interface (see `Links` below). + ## Methods Reference **All `Marshal` and `Unmarshal` methods expect pointers to struct diff --git a/constants.go b/constants.go index 35bbe05..7e443f4 100644 --- a/constants.go +++ b/constants.go @@ -7,6 +7,7 @@ const ( annotationClientID = "client-id" annotationAttribute = "attr" annotationRelation = "relation" + annotationLinks = "links" annotationOmitEmpty = "omitempty" annotationISO8601 = "iso8601" annotationRFC3339 = "rfc3339" @@ -53,4 +54,8 @@ const ( // QueryParamPageCursor is a JSON API query parameter used with a cursor-based // strategy QueryParamPageCursor = "page[cursor]" + + // KeySelfLink is the key within a top-level links object that denotes the link that + // generated the current response document. + KeySelfLink = "self" ) diff --git a/models_test.go b/models_test.go index 408e7a8..68c03f1 100644 --- a/models_test.go +++ b/models_test.go @@ -51,6 +51,8 @@ type Post struct { Body string `jsonapi:"attr,body"` Comments []*Comment `jsonapi:"relation,comments"` LatestComment *Comment `jsonapi:"relation,latest_comment"` + + Links Links `jsonapi:"links,omitempty"` } type Comment struct { @@ -58,6 +60,8 @@ type Comment struct { ClientID string `jsonapi:"client-id"` PostID int `jsonapi:"attr,post_id"` Body string `jsonapi:"attr,body"` + + Links Links `jsonapi:"links,omitempty"` } type Book struct { @@ -80,6 +84,8 @@ type Blog struct { CurrentPostID int `jsonapi:"attr,current_post_id"` CreatedAt time.Time `jsonapi:"attr,created_at"` ViewCount int `jsonapi:"attr,view_count"` + + Links Links `jsonapi:"links,omitempty"` } func (b *Blog) JSONAPILinks() *Links { diff --git a/request.go b/request.go index 1e1d485..98563bb 100644 --- a/request.go +++ b/request.go @@ -327,6 +327,47 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) } + } else if annotation == annotationLinks { + if data.Links == nil { + continue + } + + links := make(Links, len(*data.Links)) + + for k, v := range *data.Links { + link := v // default case (including string urls) + + // Unmarshal link objects to Link + if t, ok := v.(map[string]interface{}); ok { + unmarshaledHref := "" + href, ok := t["href"].(string) + if ok { + unmarshaledHref = href + } + + unmarshaledMeta := make(Meta) + meta, ok := t["meta"].(map[string]interface{}) + if ok { + for metaK, metaV := range meta { + unmarshaledMeta[metaK] = metaV + } + } + + link = Link{ + Href: unmarshaledHref, + Meta: unmarshaledMeta, + } + } + + links[k] = link + } + + if err != nil { + er = err + break + } + + assign(fieldValue, reflect.ValueOf(links)) } else { er = fmt.Errorf(unsupportedStructTagMsg, annotation) } diff --git a/request_test.go b/request_test.go index 5911120..0bc0fba 100644 --- a/request_test.go +++ b/request_test.go @@ -745,6 +745,35 @@ func TestUnmarshalNestedRelationshipsEmbedded_withClientIDs(t *testing.T) { } } +func TestUnmarshalLinks(t *testing.T) { + model := new(Blog) + + if err := UnmarshalPayload(samplePayload(), model); err != nil { + t.Fatal(err) + } + + if model.Links == nil { + t.Fatalf("Expected Links field on model to be set") + } + + if e, a := "http://somesite.com/blogs/1", model.Links[KeySelfLink]; e != a { + t.Fatalf("Was expecting links.%s to have a value of %s, got %s", KeySelfLink, e, a) + } + + if e, a := "http://somesite.com/posts/1", model.Posts[0].Links[KeySelfLink]; e != a { + t.Fatalf("Was expecting posts.0.links.%s to have a value of %s, got %s", KeySelfLink, e, a) + } + + expectedLinkObject := Link{Href: "http://somesite.com/posts/2", Meta: Meta{"foo": "bar"}} + if e, a := expectedLinkObject, model.CurrentPost.Links[KeySelfLink]; !reflect.DeepEqual(e, a) { + t.Fatalf("Was expecting posts.0.links.%s to have a value of %s, got %s", KeySelfLink, e, a) + } + + if e, a := "http://somesite.com/comments/1", model.CurrentPost.Comments[0].Links[KeySelfLink]; e != a { + t.Fatalf("Was expecting posts.0.links.%s to have a value of %s, got %s", KeySelfLink, e, a) + } +} + func unmarshalSamplePayload() (*Blog, error) { in := samplePayload() out := new(Blog) @@ -801,6 +830,32 @@ func TestUnmarshalManyPayload(t *testing.T) { } } +func TestOnePayload_withLinks(t *testing.T) { + rawJSON := []byte("{\"data\": { \"type\": \"posts\", \"id\": \"1\", \"attributes\": { \"body\": \"First\", \"title\": \"Post\" } }, \"links\": { \"self\": \"http://somesite.com/posts/1\" } }") + + in := bytes.NewReader(rawJSON) + + payload := new(OnePayload) + if err := json.NewDecoder(in).Decode(payload); err != nil { + t.Fatal(err) + } + + if payload.Links == nil { + t.Fatal("Was expecting a non nil ptr Link field") + } + + links := *payload.Links + + self, ok := links[KeySelfLink] + if !ok { + t.Fatal("Was expecting a non nil 'self' link field") + } + if e, a := "http://somesite.com/posts/1", self; e != a { + t.Fatalf("Was expecting links.%s to have a value of %s, got %s", KeySelfLink, e, a) + } + +} + func TestManyPayload_withLinks(t *testing.T) { firstPageURL := "http://somesite.com/movies?page[limit]=50&page[offset]=50" prevPageURL := "http://somesite.com/movies?page[limit]=50&page[offset]=0" @@ -1016,6 +1071,9 @@ func samplePayload() io.Reader { "body": "Bar", }, ClientID: "1", + Links: &Links{ + "self": "http://somesite.com/posts/1", + }, }, { Type: "posts", @@ -1024,6 +1082,9 @@ func samplePayload() io.Reader { "body": "Y", }, ClientID: "2", + Links: &Links{ + "self": "http://somesite.com/posts/2", + }, }, }, }, @@ -1044,6 +1105,9 @@ func samplePayload() io.Reader { "body": "Great post!", }, ClientID: "4", + Links: &Links{ + "self": "http://somesite.com/comments/1", + }, }, { Type: "comments", @@ -1051,13 +1115,27 @@ func samplePayload() io.Reader { "body": "Needs some work!", }, ClientID: "5", + Links: &Links{ + "self": "http://somesite.com/comments/2", + }, }, }, }, }, + Links: &Links{ + "self": &Link{ + Href: "http://somesite.com/posts/2", + Meta: Meta{ + "foo": "bar", + }, + }, + }, }, }, }, + Links: &Links{ + "self": "http://somesite.com/blogs/1", + }, }, } diff --git a/response.go b/response.go index b44e4e9..b598937 100644 --- a/response.go +++ b/response.go @@ -447,7 +447,9 @@ func visitModelNode(model interface{}, included *map[string]*Node, } } } - + } else if annotation == annotationLinks { + // Nothing. Ignore this field, as Links fields are only for unmarshaling requests. + // The Linkable interface methods are used for marshaling data in a response. } else { er = ErrBadJSONAPIStructTag break From 4ff67af3cda40c1e67281919a5f154d0a11e7ed1 Mon Sep 17 00:00:00 2001 From: Chris Arcand Date: Mon, 17 May 2021 22:53:37 -0500 Subject: [PATCH 04/49] Update request.go Co-authored-by: Cameron Stitt --- request.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/request.go b/request.go index 98563bb..4f76c3a 100644 --- a/request.go +++ b/request.go @@ -346,8 +346,7 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) } unmarshaledMeta := make(Meta) - meta, ok := t["meta"].(map[string]interface{}) - if ok { + if meta, ok := t["meta"].(map[string]interface{}); ok { for metaK, metaV := range meta { unmarshaledMeta[metaK] = metaV } From 15d51814255505be97b09f3ce3cffd81989f858f Mon Sep 17 00:00:00 2001 From: Omar Ismail Date: Tue, 17 Aug 2021 16:33:59 -0400 Subject: [PATCH 05/49] Add unmarshaling of interface attribute (#11) --- models_test.go | 5 +++++ request.go | 4 ++++ request_test.go | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/models_test.go b/models_test.go index 68c03f1..4be2306 100644 --- a/models_test.go +++ b/models_test.go @@ -75,6 +75,11 @@ type Book struct { Tags []string `jsonapi:"attr,tags"` } +type GenericInterface struct { + ID uint64 `jsonapi:"primary,generic"` + Data interface{} `jsonapi:"attr,interface"` +} + type Blog struct { ID int `jsonapi:"primary,blogs"` ClientID string `jsonapi:"client-id"` diff --git a/request.go b/request.go index 4f76c3a..de86624 100644 --- a/request.go +++ b/request.go @@ -443,6 +443,10 @@ func unmarshalAttribute( return } + if fieldValue.Type().Kind() == reflect.Interface { + return reflect.ValueOf(attribute), nil + } + // Handle field of type struct if fieldValue.Type().Kind() == reflect.Struct { value, err = handleStruct(attribute, fieldValue) diff --git a/request_test.go b/request_test.go index 0bc0fba..af6c34f 100644 --- a/request_test.go +++ b/request_test.go @@ -46,6 +46,60 @@ func TestUnmarshall_attrStringSlice(t *testing.T) { } } +func TestUnmarshall_attrInterface(t *testing.T) { + tests := []struct { + genericData interface{} + expected reflect.Kind + }{ + { + genericData: "foo", + expected: reflect.String, + }, + { + genericData: true, + expected: reflect.Bool, + }, + { + genericData: float64(5), + expected: reflect.Float64, + }, + { + genericData: []string{"foo", "bar"}, + expected: reflect.Slice, + }, + { + genericData: map[string]string{ + "foo": "bar", + }, + expected: reflect.Map, + }, + } + + for _, tc := range tests { + out := &GenericInterface{} + data := map[string]interface{}{ + "data": map[string]interface{}{ + "type": "generic", + "id": "1", + "attributes": map[string]interface{}{ + "interface": tc.genericData, + }, + }, + } + b, err := json.Marshal(data) + if err != nil { + t.Fatal(err) + } + + if err := UnmarshalPayload(bytes.NewReader(b), out); err != nil { + t.Fatal(err) + } + if reflect.TypeOf(out.Data).Kind() != tc.expected { + t.Fatalf("Expected %v to match interface %v", out.Data, tc.expected) + } + } +} + func TestUnmarshalToStructWithPointerAttr(t *testing.T) { out := new(WithPointer) in := map[string]interface{}{ From 12ae9868d95afdb694f4a50d2a694367c163ec50 Mon Sep 17 00:00:00 2001 From: Cameron Stitt Date: Fri, 20 Aug 2021 13:58:49 +1000 Subject: [PATCH 06/49] Allow an extra field to be marshalled for relations --- response.go | 23 +++++++++++++++++------ response_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/response.go b/response.go index b598937..ac588c5 100644 --- a/response.go +++ b/response.go @@ -358,9 +358,16 @@ func visitModelNode(model interface{}, included *map[string]*Node, } else if annotation == annotationRelation { var omitEmpty bool + var extraField string //add support for 'omitempty' struct tag for marshaling as absent if len(args) > 2 { omitEmpty = args[2] == annotationOmitEmpty + if !omitEmpty { + extraField = args[2] + } + if len(args) > 3 { + omitEmpty = args[3] == annotationOmitEmpty + } } isSlice := fieldValue.Type().Kind() == reflect.Slice @@ -402,7 +409,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, shallowNodes := []*Node{} for _, n := range relationship.Data { appendIncluded(included, n) - shallowNodes = append(shallowNodes, toShallowNode(n)) + shallowNodes = append(shallowNodes, toShallowNode(n, extraField)) } node.Relationships[args[1]] = &RelationshipManyNode{ @@ -435,7 +442,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, if sideload { appendIncluded(included, relationship) node.Relationships[args[1]] = &RelationshipOneNode{ - Data: toShallowNode(relationship), + Data: toShallowNode(relationship, extraField), Links: relLinks, Meta: relMeta, } @@ -475,11 +482,15 @@ func visitModelNode(model interface{}, included *map[string]*Node, return node, nil } -func toShallowNode(node *Node) *Node { - return &Node{ - ID: node.ID, - Type: node.Type, +func toShallowNode(node *Node, extraField string) *Node { + ret := &Node{Type: node.Type} + if extraField != "" { + ret.Attributes = map[string]interface{}{} + ret.Attributes[extraField] = node.Attributes[extraField] + return ret } + ret.ID = node.ID + return ret } func visitModelNodeRelationships(models reflect.Value, included *map[string]*Node, diff --git a/response_test.go b/response_test.go index b1d5967..f6e0aed 100644 --- a/response_test.go +++ b/response_test.go @@ -183,6 +183,36 @@ func TestWithOmitsEmptyAnnotationOnRelation(t *testing.T) { } } +func TestWithExtraFieldOnRelation(t *testing.T) { + type BlogExtraField struct { + ID int `jsonapi:"primary,blogs"` + Title string `jsonapi:"attr,title"` + CurrentPost *Post `jsonapi:"relation,current_post,title,omitempty"` + } + + blog := &BlogExtraField{ + ID: 999, + CurrentPost: &Post{ + Title: "Extra", + }, + } + + out := bytes.NewBuffer(nil) + if err := MarshalPayload(out, blog); err != nil { + t.Fatal(err) + } + + expected := &BlogExtraField{} + + if err := UnmarshalPayload(out, expected); err != nil { + t.Fatal(err) + } + + if expected.CurrentPost.Title != blog.CurrentPost.Title { + t.Fatal("Was expecting extra attribute to be equal") + } +} + func TestWithOmitsEmptyAnnotationOnRelation_MixedData(t *testing.T) { type BlogOptionalPosts struct { ID int `jsonapi:"primary,blogs"` From 7401b016fc3c0bb7ce06545bf918840c5a2abf35 Mon Sep 17 00:00:00 2001 From: Cameron Stitt Date: Tue, 24 Aug 2021 10:09:31 +1000 Subject: [PATCH 07/49] Add allowattrs annotation for relations --- constants.go | 21 ++++----- response.go | 19 ++++---- response_test.go | 115 ++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 114 insertions(+), 41 deletions(-) diff --git a/constants.go b/constants.go index 7e443f4..ec6fb06 100644 --- a/constants.go +++ b/constants.go @@ -2,16 +2,17 @@ package jsonapi const ( // StructTag annotation strings - annotationJSONAPI = "jsonapi" - annotationPrimary = "primary" - annotationClientID = "client-id" - annotationAttribute = "attr" - annotationRelation = "relation" - annotationLinks = "links" - annotationOmitEmpty = "omitempty" - annotationISO8601 = "iso8601" - annotationRFC3339 = "rfc3339" - annotationSeperator = "," + annotationJSONAPI = "jsonapi" + annotationPrimary = "primary" + annotationClientID = "client-id" + annotationAttribute = "attr" + annotationRelation = "relation" + annotationRelationAllowAttributes = "allowattrs" + annotationLinks = "links" + annotationOmitEmpty = "omitempty" + annotationISO8601 = "iso8601" + annotationRFC3339 = "rfc3339" + annotationSeperator = "," iso8601TimeFormat = "2006-01-02T15:04:05Z" diff --git a/response.go b/response.go index ac588c5..94748ff 100644 --- a/response.go +++ b/response.go @@ -358,12 +358,12 @@ func visitModelNode(model interface{}, included *map[string]*Node, } else if annotation == annotationRelation { var omitEmpty bool - var extraField string + var sideloadType string //add support for 'omitempty' struct tag for marshaling as absent if len(args) > 2 { omitEmpty = args[2] == annotationOmitEmpty if !omitEmpty { - extraField = args[2] + sideloadType = args[2] } if len(args) > 3 { omitEmpty = args[3] == annotationOmitEmpty @@ -409,7 +409,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, shallowNodes := []*Node{} for _, n := range relationship.Data { appendIncluded(included, n) - shallowNodes = append(shallowNodes, toShallowNode(n, extraField)) + shallowNodes = append(shallowNodes, toShallowNode(n, sideloadType)) } node.Relationships[args[1]] = &RelationshipManyNode{ @@ -442,7 +442,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, if sideload { appendIncluded(included, relationship) node.Relationships[args[1]] = &RelationshipOneNode{ - Data: toShallowNode(relationship, extraField), + Data: toShallowNode(relationship, sideloadType), Links: relLinks, Meta: relMeta, } @@ -482,14 +482,11 @@ func visitModelNode(model interface{}, included *map[string]*Node, return node, nil } -func toShallowNode(node *Node, extraField string) *Node { - ret := &Node{Type: node.Type} - if extraField != "" { - ret.Attributes = map[string]interface{}{} - ret.Attributes[extraField] = node.Attributes[extraField] - return ret +func toShallowNode(node *Node, sideloadType string) *Node { + ret := &Node{Type: node.Type, ID: node.ID} + if sideloadType == annotationRelationAllowAttributes { + ret.Attributes = node.Attributes } - ret.ID = node.ID return ret } diff --git a/response_test.go b/response_test.go index f6e0aed..bf8aeeb 100644 --- a/response_test.go +++ b/response_test.go @@ -184,32 +184,107 @@ func TestWithOmitsEmptyAnnotationOnRelation(t *testing.T) { } func TestWithExtraFieldOnRelation(t *testing.T) { - type BlogExtraField struct { - ID int `jsonapi:"primary,blogs"` - Title string `jsonapi:"attr,title"` - CurrentPost *Post `jsonapi:"relation,current_post,title,omitempty"` - } - - blog := &BlogExtraField{ - ID: 999, - CurrentPost: &Post{ - Title: "Extra", + type Book struct { + ID string `jsonapi:"primary,book"` + Title string `jsonapi:"attr,title,omitempty"` + Author string `jsonapi:"attr,author,omitempty"` + } + type Library struct { + ID int `jsonapi:"primary,library"` + CurrentBook *Book `jsonapi:"relation,book,allowattrs,omitempty"` + Books []*Book `jsonapi:"relation,books,allowattrs,omitempty"` + OtherBooks []*Book `jsonapi:"relation,other_books,omitempty"` + } + + testCases := []struct { + desc string + input Library + expected Library + }{ + { + "to-one success", + Library{ + ID: 999, + CurrentBook: &Book{ + Title: "A Good Book", + }, + }, + Library{ + ID: 999, + CurrentBook: &Book{ + Title: "A Good Book", + }, + }, + }, + { + "to-many success", + Library{ + ID: 999, + Books: []*Book{ + { + Title: "A Good Book", + }, + { + ID: "123", + }, + }, + }, + Library{ + ID: 999, + Books: []*Book{ + { + Title: "A Good Book", + }, + { + ID: "123", + }, + }, + }, + }, + { + "to-many without annotation", + Library{ + ID: 999, + OtherBooks: []*Book{ + { + Title: "A Good Book", + }, + { + ID: "123", + }, + }, + }, + Library{ + ID: 999, + OtherBooks: []*Book{ + { + Title: "", + }, + { + ID: "123", + }, + }, + }, }, } - out := bytes.NewBuffer(nil) - if err := MarshalPayload(out, blog); err != nil { - t.Fatal(err) - } + for _, tC := range testCases { + t.Run(tC.desc, func(t *testing.T) { + out := bytes.NewBuffer(nil) + if err := MarshalPayloadWithoutIncluded(out, &tC.input); err != nil { + t.Fatal(err) + } - expected := &BlogExtraField{} + actual := Library{} - if err := UnmarshalPayload(out, expected); err != nil { - t.Fatal(err) - } + if err := UnmarshalPayload(out, &actual); err != nil { + t.Fatal(err) + } - if expected.CurrentPost.Title != blog.CurrentPost.Title { - t.Fatal("Was expecting extra attribute to be equal") + if !reflect.DeepEqual(actual, tC.expected) { + t.Fatal("Was expecting nested relationships to be equal") + } + }) } } From 27f7e4ac19030efc8969ccbebe438bff34dce71a Mon Sep 17 00:00:00 2001 From: Cameron Stitt Date: Tue, 24 Aug 2021 10:26:27 +1000 Subject: [PATCH 08/49] Add doc on toShallowNode --- response.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/response.go b/response.go index 94748ff..ca8c1ef 100644 --- a/response.go +++ b/response.go @@ -482,6 +482,14 @@ func visitModelNode(model interface{}, included *map[string]*Node, return node, nil } +// toShallowNode takes a node and an optional sideload type +// and returns a shallow version of the node. If the sideload +// type is annotationRelationAllowAttributes, we include +// attributes into the shallow version. +// +// An example of where this is useful would be if an object +// within a relationship can be created at the same time as +// the root node. func toShallowNode(node *Node, sideloadType string) *Node { ret := &Node{Type: node.Type, ID: node.ID} if sideloadType == annotationRelationAllowAttributes { From 4018410438c17e426ef34096652d258ce698ab42 Mon Sep 17 00:00:00 2001 From: Cameron Stitt Date: Thu, 26 Aug 2021 09:19:36 +1000 Subject: [PATCH 09/49] Remove redundant annotation --- constants.go | 21 ++++++++++----------- response.go | 21 ++++++--------------- response_test.go | 30 ++---------------------------- 3 files changed, 18 insertions(+), 54 deletions(-) diff --git a/constants.go b/constants.go index ec6fb06..7e443f4 100644 --- a/constants.go +++ b/constants.go @@ -2,17 +2,16 @@ package jsonapi const ( // StructTag annotation strings - annotationJSONAPI = "jsonapi" - annotationPrimary = "primary" - annotationClientID = "client-id" - annotationAttribute = "attr" - annotationRelation = "relation" - annotationRelationAllowAttributes = "allowattrs" - annotationLinks = "links" - annotationOmitEmpty = "omitempty" - annotationISO8601 = "iso8601" - annotationRFC3339 = "rfc3339" - annotationSeperator = "," + annotationJSONAPI = "jsonapi" + annotationPrimary = "primary" + annotationClientID = "client-id" + annotationAttribute = "attr" + annotationRelation = "relation" + annotationLinks = "links" + annotationOmitEmpty = "omitempty" + annotationISO8601 = "iso8601" + annotationRFC3339 = "rfc3339" + annotationSeperator = "," iso8601TimeFormat = "2006-01-02T15:04:05Z" diff --git a/response.go b/response.go index ca8c1ef..4d1d2c1 100644 --- a/response.go +++ b/response.go @@ -358,16 +358,9 @@ func visitModelNode(model interface{}, included *map[string]*Node, } else if annotation == annotationRelation { var omitEmpty bool - var sideloadType string //add support for 'omitempty' struct tag for marshaling as absent if len(args) > 2 { omitEmpty = args[2] == annotationOmitEmpty - if !omitEmpty { - sideloadType = args[2] - } - if len(args) > 3 { - omitEmpty = args[3] == annotationOmitEmpty - } } isSlice := fieldValue.Type().Kind() == reflect.Slice @@ -409,7 +402,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, shallowNodes := []*Node{} for _, n := range relationship.Data { appendIncluded(included, n) - shallowNodes = append(shallowNodes, toShallowNode(n, sideloadType)) + shallowNodes = append(shallowNodes, toShallowNode(n)) } node.Relationships[args[1]] = &RelationshipManyNode{ @@ -442,7 +435,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, if sideload { appendIncluded(included, relationship) node.Relationships[args[1]] = &RelationshipOneNode{ - Data: toShallowNode(relationship, sideloadType), + Data: toShallowNode(relationship), Links: relLinks, Meta: relMeta, } @@ -482,17 +475,15 @@ func visitModelNode(model interface{}, included *map[string]*Node, return node, nil } -// toShallowNode takes a node and an optional sideload type -// and returns a shallow version of the node. If the sideload -// type is annotationRelationAllowAttributes, we include -// attributes into the shallow version. +// toShallowNode takes a node and returns a shallow version of the node. +// If the ID is empty, we include attributes into the shallow version. // // An example of where this is useful would be if an object // within a relationship can be created at the same time as // the root node. -func toShallowNode(node *Node, sideloadType string) *Node { +func toShallowNode(node *Node) *Node { ret := &Node{Type: node.Type, ID: node.ID} - if sideloadType == annotationRelationAllowAttributes { + if node.ID == "" { ret.Attributes = node.Attributes } return ret diff --git a/response_test.go b/response_test.go index bf8aeeb..f446626 100644 --- a/response_test.go +++ b/response_test.go @@ -191,9 +191,8 @@ func TestWithExtraFieldOnRelation(t *testing.T) { } type Library struct { ID int `jsonapi:"primary,library"` - CurrentBook *Book `jsonapi:"relation,book,allowattrs,omitempty"` - Books []*Book `jsonapi:"relation,books,allowattrs,omitempty"` - OtherBooks []*Book `jsonapi:"relation,other_books,omitempty"` + CurrentBook *Book `jsonapi:"relation,book,omitempty"` + Books []*Book `jsonapi:"relation,books,omitempty"` } testCases := []struct { @@ -241,31 +240,6 @@ func TestWithExtraFieldOnRelation(t *testing.T) { }, }, }, - { - "to-many without annotation", - Library{ - ID: 999, - OtherBooks: []*Book{ - { - Title: "A Good Book", - }, - { - ID: "123", - }, - }, - }, - Library{ - ID: 999, - OtherBooks: []*Book{ - { - Title: "", - }, - { - ID: "123", - }, - }, - }, - }, } for _, tC := range testCases { From 6ea81fe61d68e6e6e767914eef9cc02e861c19da Mon Sep 17 00:00:00 2001 From: Cameron Stitt Date: Thu, 26 Aug 2021 12:16:52 +1000 Subject: [PATCH 10/49] Redundant test logic --- response_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/response_test.go b/response_test.go index f446626..616255b 100644 --- a/response_test.go +++ b/response_test.go @@ -245,7 +245,7 @@ func TestWithExtraFieldOnRelation(t *testing.T) { for _, tC := range testCases { t.Run(tC.desc, func(t *testing.T) { out := bytes.NewBuffer(nil) - if err := MarshalPayloadWithoutIncluded(out, &tC.input); err != nil { + if err := MarshalPayload(out, &tC.input); err != nil { t.Fatal(err) } From df7b62a050e9efacbe1d98ea7818e478cff56670 Mon Sep 17 00:00:00 2001 From: Cameron Stitt Date: Thu, 26 Aug 2021 12:32:47 +1000 Subject: [PATCH 11/49] Fix ID logic --- response.go | 4 +++- response_test.go | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/response.go b/response.go index 4d1d2c1..eba8fd2 100644 --- a/response.go +++ b/response.go @@ -482,9 +482,11 @@ func visitModelNode(model interface{}, included *map[string]*Node, // within a relationship can be created at the same time as // the root node. func toShallowNode(node *Node) *Node { - ret := &Node{Type: node.Type, ID: node.ID} + ret := &Node{Type: node.Type} if node.ID == "" { ret.Attributes = node.Attributes + } else { + ret.ID = node.ID } return ret } diff --git a/response_test.go b/response_test.go index 616255b..a95bb39 100644 --- a/response_test.go +++ b/response_test.go @@ -224,7 +224,8 @@ func TestWithExtraFieldOnRelation(t *testing.T) { Title: "A Good Book", }, { - ID: "123", + ID: "123", + Title: "Don't come back", }, }, }, @@ -245,7 +246,7 @@ func TestWithExtraFieldOnRelation(t *testing.T) { for _, tC := range testCases { t.Run(tC.desc, func(t *testing.T) { out := bytes.NewBuffer(nil) - if err := MarshalPayload(out, &tC.input); err != nil { + if err := MarshalPayloadWithoutIncluded(out, &tC.input); err != nil { t.Fatal(err) } From 45d18d43050018b543cbfdeb2b0e5799c44ce779 Mon Sep 17 00:00:00 2001 From: Paul Thrasher Date: Thu, 26 Aug 2021 15:24:44 -0700 Subject: [PATCH 12/49] add comment for spec compliance --- response.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/response.go b/response.go index eba8fd2..e21ac91 100644 --- a/response.go +++ b/response.go @@ -481,6 +481,9 @@ func visitModelNode(model interface{}, included *map[string]*Node, // An example of where this is useful would be if an object // within a relationship can be created at the same time as // the root node. +// +// This is not 1.0 jsonapi spec compliant--it's a bespoke variation on +// resource object identifiers discussed in the pending 1.1 spec. func toShallowNode(node *Node) *Node { ret := &Node{Type: node.Type} if node.ID == "" { From 271d8a05b128c23885dfb3aa46a5990221c81625 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Mon, 23 Oct 2023 12:08:25 -0600 Subject: [PATCH 13/49] Add CI workflow for github actions --- .github/workflows/ci.yml | 22 ++++++++++++++++++++++ .travis.yml | 13 ------------- 2 files changed, 22 insertions(+), 13 deletions(-) create mode 100644 .github/workflows/ci.yml delete mode 100644 .travis.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..52d6093 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,22 @@ +name: CI +on: + push: + branches: + - main + pull_request: + +jobs: + unit-test: + runs-on: ubuntu-latest + strategy: + matrix: + go: [ '1.20', '1.19', '1.18', '1.17', '1.16' ] + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - uses: actions/setup-go@93397bea11091df50f3d7e59dc26a7711a8bcfbe # v4.1.0 + with: + go-version: ${{ matrix.go }} + + - name: test + run: go test -race . -v diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index abc7d1b..0000000 --- a/.travis.yml +++ /dev/null @@ -1,13 +0,0 @@ -language: go -arch: - - amd64 - - ppc64le -go: - - 1.11.x - - 1.12.x - - 1.13.x - - 1.14.x - - 1.15.x - - 1.16.x - - tip -script: go test ./... -v From d4bec3a9464892f03f26e7d924975192c7ba02f5 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Mon, 23 Oct 2023 12:20:59 -0600 Subject: [PATCH 14/49] Adjust go test versions --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 52d6093..8df71bd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go: [ '1.20', '1.19', '1.18', '1.17', '1.16' ] + go: [ '1.21', '1.20', '1.19', '1.18', '1.17', '1.11' ] steps: - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 From 473eb2147c7278e86e9f06f3958986408755c706 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Fri, 20 Oct 2023 09:49:01 -0500 Subject: [PATCH 15/49] Decode polymorphic relationships Creates a new annotation that invokes special handling for the associated field, assigning exactly one of a selection of models in an intermediate struct. (See README) --- README.md | 50 +++++++++++++ constants.go | 21 +++--- request.go | 195 ++++++++++++++++++++++++++++++++++++++++-------- request_test.go | 114 ++++++++++++++++++++++++++++ 4 files changed, 338 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 99327e6..e4e0d3d 100644 --- a/README.md +++ b/README.md @@ -179,6 +179,56 @@ used as the key in the `relationships` hash for the record. The optional third argument is `omitempty` - if present will prevent non existent to-one and to-many from being serialized. + +#### `polyrelation` + +``` +`jsonapi:"polyrelation,,"` +``` + +Polymorphic relations can be represented exactly as relations, except that +an intermediate type is needed within your model struct that will be populated +with exactly one value among all the fields in that struct. + +Example: + +```go +type Video struct { + ID int `jsonapi:"primary,videos"` + SourceURL string `jsonapi:"attr,source-url"` + CaptionsURL string `jsonapi:"attr,captions-url"` +} + +type Image struct { + ID int `jsonapi:"primary,images"` + SourceURL string `jsonapi:"attr,src"` + AltText string `jsonapi:"attr,alt"` +} + +type OneOfMedia struct { + Video *Video + Image *Image +} + +type Post struct { + ID int `jsonapi:"primary,posts"` + Title string `jsonapi:"attr,title"` + Body string `jsonapi:"attr,body"` + Gallery []*OneOfMedia `jsonapi:"polyrelation,gallery"` + Hero *OneOfMedia `jsonapi:"polyrelation,hero"` +} +``` + +During decoding, the `polyrelation` annotation instructs jsonapi to assign each relationship +to either `Video` or `Image` within the value of the associated field. This value must be +a pointer to a struct containing other pointer fields to jsonapi models. The actual field +assignment depends on that type having a jsonapi "primary" annotation with a type matching +the relationship type found in the response. All other fields will be remain nil. + +During encoding, the very first non-nil field will be used to populate the payload. Others +will be ignored. Therefore, it's critical to set the value of only one field within the join +struct. + #### `links` *Note: This annotation is an added feature independent of the canonical google/jsonapi package* diff --git a/constants.go b/constants.go index 7e443f4..d3d81cc 100644 --- a/constants.go +++ b/constants.go @@ -2,16 +2,17 @@ package jsonapi const ( // StructTag annotation strings - annotationJSONAPI = "jsonapi" - annotationPrimary = "primary" - annotationClientID = "client-id" - annotationAttribute = "attr" - annotationRelation = "relation" - annotationLinks = "links" - annotationOmitEmpty = "omitempty" - annotationISO8601 = "iso8601" - annotationRFC3339 = "rfc3339" - annotationSeperator = "," + annotationJSONAPI = "jsonapi" + annotationPrimary = "primary" + annotationClientID = "client-id" + annotationAttribute = "attr" + annotationRelation = "relation" + annotationPolyRelation = "polyrelation" + annotationLinks = "links" + annotationOmitEmpty = "omitempty" + annotationISO8601 = "iso8601" + annotationRFC3339 = "rfc3339" + annotationSeperator = "," iso8601TimeFormat = "2006-01-02T15:04:05Z" diff --git a/request.go b/request.go index de86624..c7cb287 100644 --- a/request.go +++ b/request.go @@ -32,7 +32,9 @@ var ( ErrUnknownFieldNumberType = errors.New("The struct field was not of a known number type") // ErrInvalidType is returned when the given type is incompatible with the expected type. ErrInvalidType = errors.New("Invalid type provided") // I wish we used punctuation. - + // ErrBadJSONAPIJoinStruct is returned when the polyrelation type did not contain + // an appropriate join type to contain the required jsonapi node. + ErrBadJSONAPIJoinStruct = errors.New("Invalid join struct for polymorphic relation field") ) // ErrUnsupportedPtrType is returned when the Struct field was a pointer but @@ -142,6 +144,131 @@ func UnmarshalManyPayload(in io.Reader, t reflect.Type) ([]interface{}, error) { return models, nil } +// jsonapiTypeOfModel returns a jsonapi primary type string +// given a struct type that has typical jsonapi struct tags +// +// Example: +// For this type, "posts" is returned. An error is returned if +// no properly-formatted "primary" tag is found for jsonapi +// annotations +// +// type Post struct { +// ID string `jsonapi:"primary,posts"` +// } +func jsonapiTypeOfModel(structModel reflect.Type) (string, error) { + for i := 0; i < structModel.NumField(); i++ { + fieldType := structModel.Field(i) + args, err := getStructTags(fieldType) + if err != nil || len(args) < 2 { + continue + } + + if args[0] == annotationPrimary { + return args[1], nil + } + } + + return "", errors.New("no primary annotation found on model") +} + +// structFieldIndex holds a bit of information about a type found at a struct field index +type structFieldIndex struct { + Type reflect.Type + FieldNum int +} + +// joinStructMapping reflects on a value that may be a slice +// of join structs or a join struct. A join struct is a struct +// comprising of pointers to other jsonapi models, only one of +// which is populated with a value by the decoder. The join struct is +// probed and a data structure is generated that maps the +// underlying model type (its 'primary' type) to the field number +// within the join struct. +// +// This data can then be used to correctly assign each data relationship +// to the correct join struct field. +func joinStructMapping(join reflect.Type) (result map[string]structFieldIndex, err error) { + result = make(map[string]structFieldIndex) + + for join.Kind() != reflect.Struct { + join = join.Elem() + } + + for i := 0; i < join.NumField(); i++ { + fieldType := join.Field(i) + + if fieldType.Type.Kind() != reflect.Ptr { + continue + } + + subtype := fieldType.Type.Elem() + if t, err := jsonapiTypeOfModel(subtype); err == nil { + result[t] = structFieldIndex{ + Type: subtype, + FieldNum: i, + } + } + } + + return result, nil +} + +func getStructTags(field reflect.StructField) ([]string, error) { + tag := field.Tag.Get("jsonapi") + if tag == "" { + return []string{}, nil + } + + args := strings.Split(tag, ",") + if len(args) < 1 { + return nil, ErrBadJSONAPIStructTag + } + + annotation := args[0] + + if (annotation == annotationClientID && len(args) != 1) || + (annotation != annotationClientID && len(args) < 2) { + return nil, ErrBadJSONAPIStructTag + } + + return args, nil +} + +// unmarshalNodeMaybeJoin populates a model that may or may not be +// a join struct that corresponds to a polyrelation or relation +func unmarshalNodeMaybeJoin(m *reflect.Value, data *Node, annotation string, joinMapping map[string]structFieldIndex, included *map[string]*Node) error { + // This will hold either the value of the join model or the actual + // model, depending on annotation + var actualModel = *m + var joinElem *structFieldIndex = nil + + if annotation == annotationPolyRelation { + j, ok := joinMapping[data.Type] + if !ok { + // There is no valid join field to assign this type of relation. + return ErrBadJSONAPIJoinStruct + } + joinElem = &j + actualModel = reflect.New(joinElem.Type) + } + + if err := unmarshalNode( + fullNode(data, included), + actualModel, + included, + ); err != nil { + return err + } + + if joinElem != nil { + // actualModel is a pointer to the model type + // m is a pointer to a struct that should hold the actualModel at joinElem.FieldNum + v := m.Elem() + v.Field(joinElem.FieldNum).Set(actualModel) + } + return nil +} + func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) (err error) { defer func() { if r := recover(); r != nil { @@ -155,27 +282,18 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) var er error for i := 0; i < modelValue.NumField(); i++ { - fieldType := modelType.Field(i) - tag := fieldType.Tag.Get("jsonapi") - if tag == "" { - continue - } - fieldValue := modelValue.Field(i) + fieldType := modelType.Field(i) - args := strings.Split(tag, ",") - if len(args) < 1 { - er = ErrBadJSONAPIStructTag + args, err := getStructTags(fieldType) + if err != nil { + er = err break } - - annotation := args[0] - - if (annotation == annotationClientID && len(args) != 1) || - (annotation != annotationClientID && len(args) < 2) { - er = ErrBadJSONAPIStructTag - break + if len(args) == 0 { + continue } + annotation := args[0] if annotation == annotationPrimary { // Check the JSON API Type @@ -257,16 +375,29 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) } assign(fieldValue, value) - } else if annotation == annotationRelation { + } else if annotation == annotationRelation || annotation == annotationPolyRelation { isSlice := fieldValue.Type().Kind() == reflect.Slice + // No relations of the given name were provided if data.Relationships == nil || data.Relationships[args[1]] == nil { continue } + // If this is a polymorphic relation, each data relationship needs to be assigned + // to it's appropriate join field and fieldValue should be a join field. + var joinMapping map[string]structFieldIndex = nil + if annotation == annotationPolyRelation { + joinMapping, err = joinStructMapping(fieldValue.Type()) + if err != nil { + er = err + break + } + } + if isSlice { // to-many relationship relationship := new(RelationshipManyNode) + sliceType := fieldValue.Type() buf := bytes.NewBuffer(nil) @@ -274,16 +405,18 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) json.NewDecoder(buf).Decode(relationship) data := relationship.Data - models := reflect.New(fieldValue.Type()).Elem() + + // This will hold either the value of the slice of join models or + // the slice of models, depending on the annotation + models := reflect.New(sliceType).Elem() for _, n := range data { - m := reflect.New(fieldValue.Type().Elem().Elem()) + // This will hold either the value of the join model or the actual + // model, depending on annotation + m := reflect.New(sliceType.Elem().Elem()) - if err := unmarshalNode( - fullNode(n, included), - m, - included, - ); err != nil { + err = unmarshalNodeMaybeJoin(&m, n, annotation, joinMapping, included) + if err != nil { er = err break } @@ -313,20 +446,18 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) continue } + // This will hold either the value of the join model or the actual + // model, depending on annotation m := reflect.New(fieldValue.Type().Elem()) - if err := unmarshalNode( - fullNode(relationship.Data, included), - m, - included, - ); err != nil { + + err = unmarshalNodeMaybeJoin(&m, relationship.Data, annotation, joinMapping, included) + if err != nil { er = err break } fieldValue.Set(m) - } - } else if annotation == annotationLinks { if data.Links == nil { continue diff --git a/request_test.go b/request_test.go index af6c34f..2d2d8b2 100644 --- a/request_test.go +++ b/request_test.go @@ -607,6 +607,120 @@ func TestUnmarshalRelationships(t *testing.T) { } } +type Image struct { + ID int `jsonapi:"primary,images"` + Src string `jsonapi:"attr,src"` +} + +type Video struct { + ID int `jsonapi:"primary,videos"` + Captions string `jsonapi:"attr,captions"` +} + +type OneOfMedia struct { + Image *Image + Video *Video +} + +var polySamplePayload = `{ + "data": { + "type": "blogs", + "id": "3", + "attributes": { + "title": "Hello, World" + }, + "relationships": { + "hero-media": { + "data": { + "type": "videos", + "id": "1", + "attributes": { + "captions": "It's Awesome!" + } + } + }, + "media": { + "data": [ + { + "type": "images", + "id": "1", + "attributes": { + "src": "/media/clear1x1.gif" + } + }, + { + "type": "videos", + "id": "2", + "attributes": { + "captions": "Oh, I didn't see you there" + } + } + ] + } + } + } +}` + +func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) { + type pointerToOne struct { + ID int `jsonapi:"primary,blogs"` + Title string `jsonapi:"attr,title"` + Hero *OneOfMedia `jsonapi:"polyrelation,hero-media,omitempty"` + Media []*OneOfMedia `jsonapi:"polyrelation,media,omitempty"` + } + + in := bytes.NewReader([]byte(polySamplePayload)) + out := new(pointerToOne) + + if err := UnmarshalPayload(in, out); err != nil { + t.Fatal(err) + } + + if out.Title != "Hello, World" { + t.Errorf("expected Title %q but got %q", "Hello, World", out.Title) + } + + if out.Hero.Image != nil { + t.Errorf("expected Hero image to be nil but got %+v", out.Hero.Image) + } + + if out.Hero.Video == nil || out.Hero.Video.Captions != "It's Awesome!" { + t.Errorf("expected Hero to be the expected video relation but got %+v", out.Hero.Video) + } + + if out.Media[0].Image == nil || out.Media[0].Image.Src != "/media/clear1x1.gif" { + t.Errorf("expected Media 0 to be the expected image relation but got %+v", out.Media[0]) + } + + if out.Media[1].Video == nil || out.Media[1].Video.Captions != "Oh, I didn't see you there" { + t.Errorf("expected Media 0 to be the expected video relation but got %+v", out.Media[1]) + } +} + +func Test_joinStructMapping(t *testing.T) { + cases := []struct { + val reflect.Type + }{ + {val: reflect.TypeOf(&OneOfMedia{})}, + {val: reflect.TypeOf([]*OneOfMedia{{}})}, + } + + for _, c := range cases { + result, err := joinStructMapping(c.val) + if err != nil { + t.Fatal(err) + } + imageField, ok := result["images"] + if !ok || imageField.FieldNum != 0 { + t.Errorf("expected \"images\" to be the first field, but got %d", imageField.FieldNum) + } + videoField, ok := result["videos"] + if !ok || videoField.FieldNum != 1 { + t.Errorf("expected \"videos\" to be the second field, but got %d", videoField.FieldNum) + } + } +} + func TestUnmarshalNullRelationship(t *testing.T) { sample := map[string]interface{}{ "data": map[string]interface{}{ From 7460950bd1a69394844e91c4789b20ccdc8b9110 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Fri, 20 Oct 2023 09:49:52 -0500 Subject: [PATCH 16/49] Represent fork repo in the README --- README.md | 6 +++--- examples/app.go | 2 +- examples/handler.go | 2 +- examples/handler_test.go | 2 +- examples/models.go | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index e4e0d3d..48207c6 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ all of your data easily. ## Example App -[examples/app.go](https://github.com/google/jsonapi/blob/master/examples/app.go) +[examples/app.go](https://github.com/hashicorp/jsonapi/blob/main/examples/app.go) This program demonstrates the implementation of a create, a show, and a list [http.Handler](http://golang.org/pkg/net/http#Handler). It @@ -521,13 +521,13 @@ I use git subtrees to manage dependencies rather than `go get` so that the src is committed to my repo. ``` -git subtree add --squash --prefix=src/github.com/google/jsonapi https://github.com/google/jsonapi.git master +git subtree add --squash --prefix=src/github.com/hashicorp/jsonapi https://github.com/hashicorp/jsonapi.git main ``` To update, ``` -git subtree pull --squash --prefix=src/github.com/google/jsonapi https://github.com/google/jsonapi.git master +git subtree pull --squash --prefix=src/github.com/hashicorp/jsonapi https://github.com/hashicorp/jsonapi.git main ``` This assumes that I have my repo structured with a `src` dir containing diff --git a/examples/app.go b/examples/app.go index 2b29e0d..e94a101 100644 --- a/examples/app.go +++ b/examples/app.go @@ -10,7 +10,7 @@ import ( "net/http/httptest" "time" - "github.com/google/jsonapi" + "github.com/hashicorp/jsonapi" ) func main() { diff --git a/examples/handler.go b/examples/handler.go index 77894c7..4500ca8 100644 --- a/examples/handler.go +++ b/examples/handler.go @@ -4,7 +4,7 @@ import ( "net/http" "strconv" - "github.com/google/jsonapi" + "github.com/hashicorp/jsonapi" ) const ( diff --git a/examples/handler_test.go b/examples/handler_test.go index 34c0bc5..20adc29 100644 --- a/examples/handler_test.go +++ b/examples/handler_test.go @@ -6,7 +6,7 @@ import ( "net/http/httptest" "testing" - "github.com/google/jsonapi" + "github.com/hashicorp/jsonapi" ) func TestExampleHandler_post(t *testing.T) { diff --git a/examples/models.go b/examples/models.go index 080790e..4842361 100644 --- a/examples/models.go +++ b/examples/models.go @@ -4,7 +4,7 @@ import ( "fmt" "time" - "github.com/google/jsonapi" + "github.com/hashicorp/jsonapi" ) // Blog is a model representing a blog site From 61e814c36bb2c81ecdc1d47ff1009e49c4a49846 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Fri, 20 Oct 2023 16:07:52 -0500 Subject: [PATCH 17/49] Fix spelling of private const --- constants.go | 2 +- response.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/constants.go b/constants.go index d3d81cc..591d1ca 100644 --- a/constants.go +++ b/constants.go @@ -12,7 +12,7 @@ const ( annotationOmitEmpty = "omitempty" annotationISO8601 = "iso8601" annotationRFC3339 = "rfc3339" - annotationSeperator = "," + annotationSeparator = "," iso8601TimeFormat = "2006-01-02T15:04:05Z" diff --git a/response.go b/response.go index e21ac91..8444fd5 100644 --- a/response.go +++ b/response.go @@ -215,7 +215,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, fieldValue := modelValue.Field(i) fieldType := modelType.Field(i) - args := strings.Split(tag, annotationSeperator) + args := strings.Split(tag, annotationSeparator) if len(args) < 1 { er = ErrBadJSONAPIStructTag From 9c90a4f03c1727ee9f7eb3d097b29797013f8c25 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Tue, 24 Oct 2023 16:39:09 -0600 Subject: [PATCH 18/49] Rename references to "join" or "union" to "choice type" --- README.md | 20 +++++++++------ request.go | 68 +++++++++++++++++++++++++------------------------ request_test.go | 4 +-- 3 files changed, 49 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 48207c6..7b86d7d 100644 --- a/README.md +++ b/README.md @@ -187,8 +187,8 @@ to-many from being serialized. ``` Polymorphic relations can be represented exactly as relations, except that -an intermediate type is needed within your model struct that will be populated -with exactly one value among all the fields in that struct. +an intermediate type is needed within your model struct that provides a choice +for the actual value to be populated within. Example: @@ -220,14 +220,18 @@ type Post struct { ``` During decoding, the `polyrelation` annotation instructs jsonapi to assign each relationship -to either `Video` or `Image` within the value of the associated field. This value must be -a pointer to a struct containing other pointer fields to jsonapi models. The actual field -assignment depends on that type having a jsonapi "primary" annotation with a type matching -the relationship type found in the response. All other fields will be remain nil. +to either `Video` or `Image` within the value of the associated field, provided that the +payload contains either a "videos" or "images" type. This field value must be +a pointer to a special choice type struct (also known as a tagged union, or sum type) containing +other pointer fields to jsonapi models. The actual field assignment depends on that type having +a jsonapi "primary" annotation with a type matching the relationship type found in the response. +All other fields will be remain empty. If no matching types are represented by the choice type, +all fields will be empty. During encoding, the very first non-nil field will be used to populate the payload. Others -will be ignored. Therefore, it's critical to set the value of only one field within the join -struct. +will be ignored. Therefore, it's critical to set the value of only one field within the choice +struct. When accepting input values on this type of choice type, it would a good idea to enforce +and check that the value is set on only one field. #### `links` diff --git a/request.go b/request.go index c7cb287..cdb5e62 100644 --- a/request.go +++ b/request.go @@ -177,25 +177,25 @@ type structFieldIndex struct { FieldNum int } -// joinStructMapping reflects on a value that may be a slice -// of join structs or a join struct. A join struct is a struct -// comprising of pointers to other jsonapi models, only one of -// which is populated with a value by the decoder. The join struct is -// probed and a data structure is generated that maps the -// underlying model type (its 'primary' type) to the field number -// within the join struct. +// choiceStructMapping reflects on a value that may be a slice +// of choice type structs or a choice type struct. A choice type +// struct is a struct comprising of pointers to other jsonapi models, +// only one of which is populated with a value by the decoder. // -// This data can then be used to correctly assign each data relationship -// to the correct join struct field. -func joinStructMapping(join reflect.Type) (result map[string]structFieldIndex, err error) { +// The specified type is probed and a map is generated that maps the +// underlying model type (its 'primary' type) to the field number +// within the choice type struct. This data can then be used to correctly +// assign each data relationship node to the correct choice type +// struct field. +func choiceStructMapping(choice reflect.Type) (result map[string]structFieldIndex, err error) { result = make(map[string]structFieldIndex) - for join.Kind() != reflect.Struct { - join = join.Elem() + for choice.Kind() != reflect.Struct { + choice = choice.Elem() } - for i := 0; i < join.NumField(); i++ { - fieldType := join.Field(i) + for i := 0; i < choice.NumField(); i++ { + fieldType := choice.Field(i) if fieldType.Type.Kind() != reflect.Ptr { continue @@ -234,22 +234,22 @@ func getStructTags(field reflect.StructField) ([]string, error) { return args, nil } -// unmarshalNodeMaybeJoin populates a model that may or may not be -// a join struct that corresponds to a polyrelation or relation -func unmarshalNodeMaybeJoin(m *reflect.Value, data *Node, annotation string, joinMapping map[string]structFieldIndex, included *map[string]*Node) error { - // This will hold either the value of the join model or the actual +// unmarshalNodeMaybeChoice populates a model that may or may not be +// a choice type struct that corresponds to a polyrelation or relation +func unmarshalNodeMaybeChoice(m *reflect.Value, data *Node, annotation string, choiceTypeMapping map[string]structFieldIndex, included *map[string]*Node) error { + // This will hold either the value of the choice type model or the actual // model, depending on annotation var actualModel = *m - var joinElem *structFieldIndex = nil + var choiceElem *structFieldIndex = nil if annotation == annotationPolyRelation { - j, ok := joinMapping[data.Type] + c, ok := choiceTypeMapping[data.Type] if !ok { // There is no valid join field to assign this type of relation. return ErrBadJSONAPIJoinStruct } - joinElem = &j - actualModel = reflect.New(joinElem.Type) + choiceElem = &c + actualModel = reflect.New(choiceElem.Type) } if err := unmarshalNode( @@ -260,11 +260,12 @@ func unmarshalNodeMaybeJoin(m *reflect.Value, data *Node, annotation string, joi return err } - if joinElem != nil { + if choiceElem != nil { // actualModel is a pointer to the model type - // m is a pointer to a struct that should hold the actualModel at joinElem.FieldNum + // m is a pointer to a struct that should hold the actualModel + // at choiceElem.FieldNum v := m.Elem() - v.Field(joinElem.FieldNum).Set(actualModel) + v.Field(choiceElem.FieldNum).Set(actualModel) } return nil } @@ -384,10 +385,11 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) } // If this is a polymorphic relation, each data relationship needs to be assigned - // to it's appropriate join field and fieldValue should be a join field. - var joinMapping map[string]structFieldIndex = nil + // to it's appropriate choice field and fieldValue should be a choice + // struct type field. + var choiceMapping map[string]structFieldIndex = nil if annotation == annotationPolyRelation { - joinMapping, err = joinStructMapping(fieldValue.Type()) + choiceMapping, err = choiceStructMapping(fieldValue.Type()) if err != nil { er = err break @@ -406,16 +408,16 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) data := relationship.Data - // This will hold either the value of the slice of join models or + // This will hold either the value of the slice of choice type models or // the slice of models, depending on the annotation models := reflect.New(sliceType).Elem() for _, n := range data { - // This will hold either the value of the join model or the actual + // This will hold either the value of the choice type model or the actual // model, depending on annotation m := reflect.New(sliceType.Elem().Elem()) - err = unmarshalNodeMaybeJoin(&m, n, annotation, joinMapping, included) + err = unmarshalNodeMaybeChoice(&m, n, annotation, choiceMapping, included) if err != nil { er = err break @@ -446,11 +448,11 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) continue } - // This will hold either the value of the join model or the actual + // 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 = unmarshalNodeMaybeJoin(&m, relationship.Data, annotation, joinMapping, included) + err = unmarshalNodeMaybeChoice(&m, relationship.Data, annotation, choiceMapping, included) if err != nil { er = err break diff --git a/request_test.go b/request_test.go index 2d2d8b2..05bb60f 100644 --- a/request_test.go +++ b/request_test.go @@ -697,7 +697,7 @@ func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) { } } -func Test_joinStructMapping(t *testing.T) { +func Test_choiceStructMapping(t *testing.T) { cases := []struct { val reflect.Type }{ @@ -706,7 +706,7 @@ func Test_joinStructMapping(t *testing.T) { } for _, c := range cases { - result, err := joinStructMapping(c.val) + result, err := choiceStructMapping(c.val) if err != nil { t.Fatal(err) } From 448279afe88e6ef98661448974cbdaf8bf08e176 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Tue, 24 Oct 2023 16:42:42 -0600 Subject: [PATCH 19/49] Don't raise error when an invalid choice node is decoded --- request.go | 10 ++-- request_test.go | 143 ++++++++++++++++++++++++++++++++++-------------- 2 files changed, 106 insertions(+), 47 deletions(-) diff --git a/request.go b/request.go index cdb5e62..d947a13 100644 --- a/request.go +++ b/request.go @@ -32,9 +32,6 @@ var ( ErrUnknownFieldNumberType = errors.New("The struct field was not of a known number type") // ErrInvalidType is returned when the given type is incompatible with the expected type. ErrInvalidType = errors.New("Invalid type provided") // I wish we used punctuation. - // ErrBadJSONAPIJoinStruct is returned when the polyrelation type did not contain - // an appropriate join type to contain the required jsonapi node. - ErrBadJSONAPIJoinStruct = errors.New("Invalid join struct for polymorphic relation field") ) // ErrUnsupportedPtrType is returned when the Struct field was a pointer but @@ -245,8 +242,11 @@ func unmarshalNodeMaybeChoice(m *reflect.Value, data *Node, annotation string, c if annotation == annotationPolyRelation { c, ok := choiceTypeMapping[data.Type] if !ok { - // There is no valid join field to assign this type of relation. - return ErrBadJSONAPIJoinStruct + // If there is no valid choice field to assign this type of relation, + // this shouldn't necessarily be an error because a newer version of + // the API could be communicating with an older version of the client + // library, in which case all choice variants would be nil. + return nil } choiceElem = &c actualModel = reflect.New(choiceElem.Type) diff --git a/request_test.go b/request_test.go index 05bb60f..e175814 100644 --- a/request_test.go +++ b/request_test.go @@ -622,45 +622,6 @@ type OneOfMedia struct { Video *Video } -var polySamplePayload = `{ - "data": { - "type": "blogs", - "id": "3", - "attributes": { - "title": "Hello, World" - }, - "relationships": { - "hero-media": { - "data": { - "type": "videos", - "id": "1", - "attributes": { - "captions": "It's Awesome!" - } - } - }, - "media": { - "data": [ - { - "type": "images", - "id": "1", - "attributes": { - "src": "/media/clear1x1.gif" - } - }, - { - "type": "videos", - "id": "2", - "attributes": { - "captions": "Oh, I didn't see you there" - } - } - ] - } - } - } -}` - func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) { type pointerToOne struct { ID int `jsonapi:"primary,blogs"` @@ -669,7 +630,58 @@ func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) { Media []*OneOfMedia `jsonapi:"polyrelation,media,omitempty"` } - in := bytes.NewReader([]byte(polySamplePayload)) + in := bytes.NewReader([]byte(`{ + "data": { + "type": "blogs", + "id": "3", + "attributes": { + "title": "Hello, World" + }, + "relationships": { + "hero-media": { + "data": { + "type": "videos", + "id": "1" + } + }, + "media": { + "data": [ + { + "type": "images", + "id": "1" + }, + { + "type": "videos", + "id": "2" + } + ] + } + } + }, + "included": [ + { + "type": "videos", + "id": "1", + "attributes": { + "captions": "It's Awesome!" + } + }, + { + "type": "images", + "id": "1", + "attributes": { + "src": "/media/clear1x1.gif" + } + }, + { + "type": "videos", + "id": "2", + "attributes": { + "captions": "Oh, I didn't see you there" + } + } + ] + }`)) out := new(pointerToOne) if err := UnmarshalPayload(in, out); err != nil { @@ -688,12 +700,59 @@ func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) { t.Errorf("expected Hero to be the expected video relation but got %+v", out.Hero.Video) } + // Unmarshals included records if out.Media[0].Image == nil || out.Media[0].Image.Src != "/media/clear1x1.gif" { - t.Errorf("expected Media 0 to be the expected image relation but got %+v", out.Media[0]) + t.Errorf("expected Media 0 to be the expected image relation but got %+v", out.Media[0].Image) } if out.Media[1].Video == nil || out.Media[1].Video.Captions != "Oh, I didn't see you there" { - t.Errorf("expected Media 0 to be the expected video relation but got %+v", out.Media[1]) + t.Errorf("expected Media 1 to be the expected video relation but got %+v", out.Media[1].Video) + } +} + +func Test_UnmarshalPayload_polymorphicRelations_no_choice(t *testing.T) { + type pointerToOne struct { + ID int `jsonapi:"primary,blogs"` + Title string `jsonapi:"attr,title"` + Hero *OneOfMedia `jsonapi:"polyrelation,hero-media,omitempty"` + } + + in := bytes.NewReader([]byte(`{ + "data": { + "type": "blogs", + "id": "3", + "attributes": { + "title": "Hello, World" + }, + "relationships": { + "hero-media": { + "data": { + "type": "absolutely-not", + "id": "1", + "attributes": { + "captions": "It's Awesome!" + } + } + } + } + } + }`)) + out := new(pointerToOne) + + if err := UnmarshalPayload(in, out); err != nil { + t.Fatal(err) + } + + if out.Title != "Hello, World" { + t.Errorf("expected Title %q but got %q", "Hello, World", out.Title) + } + + if out.Hero == nil { + t.Fatal("expected Hero to not be nil") + } + + if out.Hero.Image != nil || out.Hero.Video != nil { + t.Fatal("expected both Hero fields to be nil") } } From 2a00bb5f71900d7055987977542322f8818839aa Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Wed, 25 Oct 2023 09:39:30 -0600 Subject: [PATCH 20/49] Reformat code comments --- request.go | 27 +++++++++++++-------------- response.go | 15 +++++++-------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/request.go b/request.go index d947a13..a204b16 100644 --- a/request.go +++ b/request.go @@ -69,24 +69,23 @@ func newErrUnsupportedPtrType(rf reflect.Value, t reflect.Type, structField refl // For example you could pass it, in, req.Body and, model, a BlogPost // struct instance to populate in an http handler, // -// func CreateBlog(w http.ResponseWriter, r *http.Request) { -// blog := new(Blog) +// func CreateBlog(w http.ResponseWriter, r *http.Request) { +// blog := new(Blog) // -// if err := jsonapi.UnmarshalPayload(r.Body, blog); err != nil { -// http.Error(w, err.Error(), 500) -// return -// } +// if err := jsonapi.UnmarshalPayload(r.Body, blog); err != nil { +// http.Error(w, err.Error(), 500) +// return +// } // -// // ...do stuff with your blog... +// // ...do stuff with your blog... // -// w.Header().Set("Content-Type", jsonapi.MediaType) -// w.WriteHeader(201) -// -// if err := jsonapi.MarshalPayload(w, blog); err != nil { -// http.Error(w, err.Error(), 500) -// } -// } +// w.Header().Set("Content-Type", jsonapi.MediaType) +// w.WriteHeader(201) // +// if err := jsonapi.MarshalPayload(w, blog); err != nil { +// http.Error(w, err.Error(), 500) +// } +// } // // Visit https://github.com/google/jsonapi#create for more info. // diff --git a/response.go b/response.go index 8444fd5..94ccf8c 100644 --- a/response.go +++ b/response.go @@ -51,17 +51,16 @@ var ( // Many Example: you could pass it, w, your http.ResponseWriter, and, models, a // slice of Blog struct instance pointers to be written to the response body: // -// func ListBlogs(w http.ResponseWriter, r *http.Request) { -// blogs := []*Blog{} +// func ListBlogs(w http.ResponseWriter, r *http.Request) { +// blogs := []*Blog{} // -// w.Header().Set("Content-Type", jsonapi.MediaType) -// w.WriteHeader(http.StatusOK) +// w.Header().Set("Content-Type", jsonapi.MediaType) +// w.WriteHeader(http.StatusOK) // -// if err := jsonapi.MarshalPayload(w, blogs); err != nil { -// http.Error(w, err.Error(), http.StatusInternalServerError) +// if err := jsonapi.MarshalPayload(w, blogs); err != nil { +// http.Error(w, err.Error(), http.StatusInternalServerError) +// } // } -// } -// func MarshalPayload(w io.Writer, models interface{}) error { payload, err := Marshal(models) if err != nil { From 6bdf2394abb3e0d858195db0223a67579d150fc7 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Wed, 25 Oct 2023 09:47:30 -0600 Subject: [PATCH 21/49] Support public&private non-tagged fields in choice struct --- request.go | 7 +++++++ request_test.go | 10 ++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/request.go b/request.go index a204b16..b063373 100644 --- a/request.go +++ b/request.go @@ -193,11 +193,18 @@ func choiceStructMapping(choice reflect.Type) (result map[string]structFieldInde for i := 0; i < choice.NumField(); i++ { fieldType := choice.Field(i) + // Must be a pointer if fieldType.Type.Kind() != reflect.Ptr { continue } subtype := fieldType.Type.Elem() + + // Must be a pointer to struct + if subtype.Kind() != reflect.Struct { + continue + } + if t, err := jsonapiTypeOfModel(subtype); err == nil { result[t] = structFieldIndex{ Type: subtype, diff --git a/request_test.go b/request_test.go index e175814..7a2979f 100644 --- a/request_test.go +++ b/request_test.go @@ -618,8 +618,10 @@ type Video struct { } type OneOfMedia struct { - Image *Image - Video *Video + Image *Image + random int + Video *Video + RandomStuff *string } func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) { @@ -774,8 +776,8 @@ func Test_choiceStructMapping(t *testing.T) { t.Errorf("expected \"images\" to be the first field, but got %d", imageField.FieldNum) } videoField, ok := result["videos"] - if !ok || videoField.FieldNum != 1 { - t.Errorf("expected \"videos\" to be the second field, but got %d", videoField.FieldNum) + if !ok || videoField.FieldNum != 2 { + t.Errorf("expected \"videos\" to be the third field, but got %d", videoField.FieldNum) } } } From 43b65c2b70dae0f85d0854b22f71bc7a0cc87618 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Fri, 3 Nov 2023 16:40:16 -0600 Subject: [PATCH 22/49] Move test models to models_test.go --- models_test.go | 24 ++++++++++++++++++++++++ request_test.go | 28 ++-------------------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/models_test.go b/models_test.go index 4be2306..889142a 100644 --- a/models_test.go +++ b/models_test.go @@ -212,3 +212,27 @@ type CustomAttributeTypes struct { Float CustomFloatType `jsonapi:"attr,float"` String CustomStringType `jsonapi:"attr,string"` } + +type Image struct { + ID string `jsonapi:"primary,images"` + Src string `jsonapi:"attr,src"` +} + +type Video struct { + ID string `jsonapi:"primary,videos"` + Captions string `jsonapi:"attr,captions"` +} + +type OneOfMedia struct { + Image *Image + random int + Video *Video + RandomStuff *string +} + +type BlogPostWithPoly struct { + ID string `jsonapi:"primary,blogs"` + Title string `jsonapi:"attr,title"` + Hero *OneOfMedia `jsonapi:"polyrelation,hero-media,omitempty"` + Media []*OneOfMedia `jsonapi:"polyrelation,media,omitempty"` +} diff --git a/request_test.go b/request_test.go index 7a2979f..a58df7f 100644 --- a/request_test.go +++ b/request_test.go @@ -607,31 +607,7 @@ func TestUnmarshalRelationships(t *testing.T) { } } -type Image struct { - ID int `jsonapi:"primary,images"` - Src string `jsonapi:"attr,src"` -} - -type Video struct { - ID int `jsonapi:"primary,videos"` - Captions string `jsonapi:"attr,captions"` -} - -type OneOfMedia struct { - Image *Image - random int - Video *Video - RandomStuff *string -} - func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) { - type pointerToOne struct { - ID int `jsonapi:"primary,blogs"` - Title string `jsonapi:"attr,title"` - Hero *OneOfMedia `jsonapi:"polyrelation,hero-media,omitempty"` - Media []*OneOfMedia `jsonapi:"polyrelation,media,omitempty"` - } - in := bytes.NewReader([]byte(`{ "data": { "type": "blogs", @@ -684,7 +660,7 @@ func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) { } ] }`)) - out := new(pointerToOne) + out := new(BlogPostWithPoly) if err := UnmarshalPayload(in, out); err != nil { t.Fatal(err) @@ -714,7 +690,7 @@ func Test_UnmarshalPayload_polymorphicRelations(t *testing.T) { func Test_UnmarshalPayload_polymorphicRelations_no_choice(t *testing.T) { type pointerToOne struct { - ID int `jsonapi:"primary,blogs"` + ID string `jsonapi:"primary,blogs"` Title string `jsonapi:"attr,title"` Hero *OneOfMedia `jsonapi:"polyrelation,hero-media,omitempty"` } From 4ebf344b6487c363a6492e16e5eac8f9e4b1b836 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Fri, 3 Nov 2023 16:41:33 -0600 Subject: [PATCH 23/49] Fixes panic when a has-many relation contains nil --- response.go | 9 ++++++++- response_test.go | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/response.go b/response.go index 94ccf8c..fc394c2 100644 --- a/response.go +++ b/response.go @@ -26,6 +26,8 @@ var ( // ErrUnexpectedType is returned when marshalling an interface; the interface // had to be a pointer or a slice; otherwise this error is returned. ErrUnexpectedType = errors.New("models should be a struct pointer or slice of struct pointers") + // ErrUnexpectedNil is returned when a slice of relation structs contains nil values + ErrUnexpectedNil = errors.New("slice of struct pointers cannot contain nil") ) // MarshalPayload writes a jsonapi response for one or many records. The @@ -498,7 +500,12 @@ func visitModelNodeRelationships(models reflect.Value, included *map[string]*Nod nodes := []*Node{} for i := 0; i < models.Len(); i++ { - n := models.Index(i).Interface() + model := models.Index(i) + if !model.IsValid() || model.IsNil() { + return nil, ErrUnexpectedNil + } + + n := model.Interface() node, err := visitModelNode(n, included, sideload) if err != nil { diff --git a/response_test.go b/response_test.go index a95bb39..5d0822e 100644 --- a/response_test.go +++ b/response_test.go @@ -3,6 +3,7 @@ package jsonapi import ( "bytes" "encoding/json" + "errors" "fmt" "reflect" "sort" @@ -40,6 +41,26 @@ func TestMarshalPayload(t *testing.T) { func TestMarshalPayloadWithNulls(t *testing.T) { +func TestMarshalPayloadWithManyRelationWithNils(t *testing.T) { + blog := &Blog{ + ID: 1, + Title: "Hello, World", + Posts: []*Post{ + nil, + { + ID: 2, + }, + nil, + }, + } + + out := bytes.NewBuffer(nil) + if err := MarshalPayload(out, blog); !errors.Is(err, ErrUnexpectedNil) { + t.Fatal("expected error but got none") + } +} + +func TestMarshalPayloadWithNulls(t *testing.T) { books := []*Book{nil, {ID: 101}, nil} var jsonData map[string]interface{} From 6c08cda837ce93bd90bb35e66efac685d628307e Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Fri, 3 Nov 2023 16:43:32 -0600 Subject: [PATCH 24/49] Marshal polyrelation polyrelation fields are marshaled as JSON to the first non-nil field within a choice type --- response.go | 88 ++++++++++++++++++++++++++++++++++- response_test.go | 116 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 201 insertions(+), 3 deletions(-) diff --git a/response.go b/response.go index fc394c2..cf0476b 100644 --- a/response.go +++ b/response.go @@ -193,6 +193,31 @@ func MarshalOnePayloadEmbedded(w io.Writer, model interface{}) error { return json.NewEncoder(w).Encode(payload) } +func chooseFirstNonNilFieldValue(structValue reflect.Value) (reflect.Value, error) { + for i := 0; i < structValue.NumField(); i++ { + choiceFieldValue := structValue.Field(i) + choiceTypeField := choiceFieldValue.Type() + + // Must be a pointer + if choiceTypeField.Kind() != reflect.Ptr { + continue + } + + // Must not be nil + if choiceFieldValue.IsNil() { + continue + } + + subtype := choiceTypeField.Elem() + _, err := jsonapiTypeOfModel(subtype) + if err == nil { + return choiceFieldValue, nil + } + } + + return reflect.Value{}, errors.New("no non-nil choice field was found in the specified struct") +} + func visitModelNode(model interface{}, included *map[string]*Node, sideload bool) (*Node, error) { node := new(Node) @@ -207,13 +232,13 @@ func visitModelNode(model interface{}, included *map[string]*Node, modelType := value.Type().Elem() for i := 0; i < modelValue.NumField(); i++ { + fieldValue := modelValue.Field(i) structField := modelValue.Type().Field(i) tag := structField.Tag.Get(annotationJSONAPI) if tag == "" { continue } - fieldValue := modelValue.Field(i) fieldType := modelType.Field(i) args := strings.Split(tag, annotationSeparator) @@ -356,7 +381,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, node.Attributes[args[1]] = fieldValue.Interface() } } - } else if annotation == annotationRelation { + } else if annotation == annotationRelation || annotation == annotationPolyRelation { var omitEmpty bool //add support for 'omitempty' struct tag for marshaling as absent @@ -371,6 +396,65 @@ func visitModelNode(model interface{}, included *map[string]*Node, 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.Pointer { + er = ErrUnexpectedType + break + } + + if choiceValue.IsNil() { + fieldValue = reflect.ValueOf(nil) + } + + structValue := choiceValue.Elem() + if found, err := chooseFirstNonNilFieldValue(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.Pointer { + er = ErrUnexpectedType + break + } + + if itemValue.IsNil() { + er = ErrUnexpectedNil + break + } + + structValue := itemValue.Elem() + + if found, err := chooseFirstNonNilFieldValue(structValue); err == nil { + collection = append(collection, found.Interface()) + } + } + + if er != nil { + break + } + + fieldValue = reflect.ValueOf(collection) + } + } + if node.Relationships == nil { node.Relationships = make(map[string]interface{}) } diff --git a/response_test.go b/response_test.go index 5d0822e..4eb54c0 100644 --- a/response_test.go +++ b/response_test.go @@ -39,7 +39,121 @@ func TestMarshalPayload(t *testing.T) { } } -func TestMarshalPayloadWithNulls(t *testing.T) { +func TestMarshalPayloadWithOnePolyrelation(t *testing.T) { + blog := &BlogPostWithPoly{ + ID: "1", + Title: "Hello, World", + Hero: &OneOfMedia{ + Image: &Image{ + ID: "2", + }, + }, + } + + out := bytes.NewBuffer(nil) + if err := MarshalPayload(out, blog); err != nil { + t.Fatal(err) + } + + var jsonData map[string]interface{} + if err := json.Unmarshal(out.Bytes(), &jsonData); err != nil { + t.Fatal(err) + } + + relationships := jsonData["data"].(map[string]interface{})["relationships"].(map[string]interface{}) + if relationships == nil { + t.Fatal("No relationships defined in unmarshaled JSON") + } + heroMedia := relationships["hero-media"].(map[string]interface{})["data"].(map[string]interface{}) + if heroMedia == nil { + t.Fatal("No hero-media relationship defined in unmarshaled JSON") + } + + if heroMedia["id"] != "2" { + t.Fatal("Expected ID \"2\" in unmarshaled JSON") + } + + if heroMedia["type"] != "images" { + t.Fatal("Expected type \"images\" in unmarshaled JSON") + } +} + +func TestMarshalPayloadWithManyPolyrelation(t *testing.T) { + blog := &BlogPostWithPoly{ + ID: "1", + Title: "Hello, World", + Media: []*OneOfMedia{ + { + Image: &Image{ + ID: "2", + }, + }, + { + Video: &Video{ + ID: "3", + }, + }, + }, + } + + out := bytes.NewBuffer(nil) + if err := MarshalPayload(out, blog); err != nil { + t.Fatal(err) + } + + var jsonData map[string]interface{} + if err := json.Unmarshal(out.Bytes(), &jsonData); err != nil { + t.Fatal(err) + } + + relationships := jsonData["data"].(map[string]interface{})["relationships"].(map[string]interface{}) + if relationships == nil { + t.Fatal("No relationships defined in unmarshaled JSON") + } + + heroMedia := relationships["media"].(map[string]interface{}) + if heroMedia == nil { + t.Fatal("No hero-media relationship defined in unmarshaled JSON") + } + + heroMediaData := heroMedia["data"].([]interface{}) + + if len(heroMediaData) != 2 { + t.Fatal("Expected 2 items in unmarshaled JSON") + } + + imageData := heroMediaData[0].(map[string]interface{}) + videoData := heroMediaData[1].(map[string]interface{}) + + if imageData["id"] != "2" || imageData["type"] != "images" { + t.Fatal("Expected images ID \"2\" in unmarshaled JSON") + } + + if videoData["id"] != "3" || videoData["type"] != "videos" { + t.Fatal("Expected videos ID \"3\" in unmarshaled JSON") + } +} + +func TestMarshalPayloadWithManyPolyrelationWithNils(t *testing.T) { + blog := &BlogPostWithPoly{ + ID: "1", + Title: "Hello, World", + Media: []*OneOfMedia{ + nil, + { + Image: &Image{ + ID: "2", + }, + }, + nil, + }, + } + + out := bytes.NewBuffer(nil) + if err := MarshalPayload(out, blog); !errors.Is(err, ErrUnexpectedNil) { + t.Fatal("expected error but got none") + } +} func TestMarshalPayloadWithManyRelationWithNils(t *testing.T) { blog := &Blog{ From e9893b80dcfa67a22967746cab2c81effbd08a46 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Sat, 4 Nov 2023 14:06:21 -0600 Subject: [PATCH 25/49] Fix go compatibility for <= 1.11 --- response.go | 4 ++-- response_test.go | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/response.go b/response.go index cf0476b..5398064 100644 --- a/response.go +++ b/response.go @@ -406,7 +406,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, choiceValue := fieldValue // must be a pointer type - if choiceValue.Type().Kind() != reflect.Pointer { + if choiceValue.Type().Kind() != reflect.Ptr { er = ErrUnexpectedType break } @@ -430,7 +430,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, for i := 0; i < fieldValue.Len(); i++ { itemValue := fieldValue.Index(i) // Once again, must be a pointer type - if itemValue.Type().Kind() != reflect.Pointer { + if itemValue.Type().Kind() != reflect.Ptr { er = ErrUnexpectedType break } diff --git a/response_test.go b/response_test.go index 4eb54c0..e4b9b1a 100644 --- a/response_test.go +++ b/response_test.go @@ -3,7 +3,6 @@ package jsonapi import ( "bytes" "encoding/json" - "errors" "fmt" "reflect" "sort" @@ -150,7 +149,7 @@ func TestMarshalPayloadWithManyPolyrelationWithNils(t *testing.T) { } out := bytes.NewBuffer(nil) - if err := MarshalPayload(out, blog); !errors.Is(err, ErrUnexpectedNil) { + if err := MarshalPayload(out, blog); err != ErrUnexpectedNil { t.Fatal("expected error but got none") } } @@ -169,7 +168,7 @@ func TestMarshalPayloadWithManyRelationWithNils(t *testing.T) { } out := bytes.NewBuffer(nil) - if err := MarshalPayload(out, blog); !errors.Is(err, ErrUnexpectedNil) { + if err := MarshalPayload(out, blog); err != ErrUnexpectedNil { t.Fatal("expected error but got none") } } From 1037764ea855d5830e9e85b9bb62c9b4a1de5ff2 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Mon, 6 Nov 2023 09:57:08 -0700 Subject: [PATCH 26/49] refactor: rename/document chooseFirstNonNilFieldValue Adds test for nil hasOne polyrelation --- response.go | 9 ++++++--- response_test.go | 21 +++++++++++++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/response.go b/response.go index 5398064..e23dd9c 100644 --- a/response.go +++ b/response.go @@ -193,7 +193,10 @@ func MarshalOnePayloadEmbedded(w io.Writer, model interface{}) error { return json.NewEncoder(w).Encode(payload) } -func chooseFirstNonNilFieldValue(structValue reflect.Value) (reflect.Value, error) { +// selectChoiceTypeStructField returns the first non-nil struct pointer field in the +// specified struct value that has a jsonapi type field defined within it. +// An error is returned if there are no fields matching that definition. +func selectChoiceTypeStructField(structValue reflect.Value) (reflect.Value, error) { for i := 0; i < structValue.NumField(); i++ { choiceFieldValue := structValue.Field(i) choiceTypeField := choiceFieldValue.Type() @@ -416,7 +419,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, } structValue := choiceValue.Elem() - if found, err := chooseFirstNonNilFieldValue(structValue); err == nil { + if found, err := selectChoiceTypeStructField(structValue); err == nil { fieldValue = found } } else { @@ -442,7 +445,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, structValue := itemValue.Elem() - if found, err := chooseFirstNonNilFieldValue(structValue); err == nil { + if found, err := selectChoiceTypeStructField(structValue); err == nil { collection = append(collection, found.Interface()) } } diff --git a/response_test.go b/response_test.go index e4b9b1a..da10dee 100644 --- a/response_test.go +++ b/response_test.go @@ -38,7 +38,7 @@ func TestMarshalPayload(t *testing.T) { } } -func TestMarshalPayloadWithOnePolyrelation(t *testing.T) { +func TestMarshalPayloadWithHasOnePolyrelation(t *testing.T) { blog := &BlogPostWithPoly{ ID: "1", Title: "Hello, World", @@ -77,7 +77,7 @@ func TestMarshalPayloadWithOnePolyrelation(t *testing.T) { } } -func TestMarshalPayloadWithManyPolyrelation(t *testing.T) { +func TestMarshalPayloadWithHasManyPolyrelation(t *testing.T) { blog := &BlogPostWithPoly{ ID: "1", Title: "Hello, World", @@ -133,7 +133,7 @@ func TestMarshalPayloadWithManyPolyrelation(t *testing.T) { } } -func TestMarshalPayloadWithManyPolyrelationWithNils(t *testing.T) { +func TestMarshalPayloadWithHasManyPolyrelationWithNils(t *testing.T) { blog := &BlogPostWithPoly{ ID: "1", Title: "Hello, World", @@ -154,7 +154,20 @@ func TestMarshalPayloadWithManyPolyrelationWithNils(t *testing.T) { } } -func TestMarshalPayloadWithManyRelationWithNils(t *testing.T) { +func TestMarshalPayloadWithHasOneNilPolyrelation(t *testing.T) { + blog := &BlogPostWithPoly{ + ID: "1", + Title: "Hello, World", + Hero: nil, + } + + out := bytes.NewBuffer(nil) + if err := MarshalPayload(out, blog); err != nil { + t.Fatalf("expected no error but got %s", err) + } +} + +func TestMarshalPayloadWithHasOneNilRelation(t *testing.T) { blog := &Blog{ ID: 1, Title: "Hello, World", From 960294d717d7b4c82e6b15f5451b57dc6b93965e Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Mon, 6 Nov 2023 11:37:57 -0700 Subject: [PATCH 27/49] remove unused error return, enhance docs, style fixes --- request.go | 40 ++++++++++++++++++++++++++++++---------- request_test.go | 5 +---- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/request.go b/request.go index b063373..a12c2b1 100644 --- a/request.go +++ b/request.go @@ -32,6 +32,8 @@ var ( ErrUnknownFieldNumberType = errors.New("The struct field was not of a known number type") // ErrInvalidType is returned when the given type is incompatible with the expected type. ErrInvalidType = errors.New("Invalid type provided") // I wish we used punctuation. + // ErrTypeNotFound is returned when the given type not found on the model. + ErrTypeNotFound = errors.New("no primary type annotation found on model") ) // ErrUnsupportedPtrType is returned when the Struct field was a pointer but @@ -155,7 +157,13 @@ func jsonapiTypeOfModel(structModel reflect.Type) (string, error) { for i := 0; i < structModel.NumField(); i++ { fieldType := structModel.Field(i) args, err := getStructTags(fieldType) - if err != nil || len(args) < 2 { + + // A jsonapi tag was found, but it was improperly structured + if err != nil { + return "", err + } + + if len(args) < 2 { continue } @@ -164,7 +172,7 @@ func jsonapiTypeOfModel(structModel reflect.Type) (string, error) { } } - return "", errors.New("no primary annotation found on model") + return "", ErrTypeNotFound } // structFieldIndex holds a bit of information about a type found at a struct field index @@ -175,7 +183,7 @@ type structFieldIndex struct { // choiceStructMapping reflects on a value that may be a slice // of choice type structs or a choice type struct. A choice type -// struct is a struct comprising of pointers to other jsonapi models, +// struct is a struct comprised of pointers to other jsonapi models, // only one of which is populated with a value by the decoder. // // The specified type is probed and a map is generated that maps the @@ -183,7 +191,23 @@ type structFieldIndex struct { // within the choice type struct. This data can then be used to correctly // assign each data relationship node to the correct choice type // struct field. -func choiceStructMapping(choice reflect.Type) (result map[string]structFieldIndex, err error) { +// +// For example, if the `choice` type was +// +// type OneOfMedia struct { +// Video *Video +// Image *Image +// } +// +// then the resulting map would be +// +// { +// "videos" => {Video, 0} +// "images" => {Image, 1} +// } +// +// where `"videos"` is the value of the `primary` annotation on the `Video` model +func choiceStructMapping(choice reflect.Type) (result map[string]structFieldIndex) { result = make(map[string]structFieldIndex) for choice.Kind() != reflect.Struct { @@ -213,7 +237,7 @@ func choiceStructMapping(choice reflect.Type) (result map[string]structFieldInde } } - return result, nil + return result } func getStructTags(field reflect.StructField) ([]string, error) { @@ -395,11 +419,7 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) // struct type field. var choiceMapping map[string]structFieldIndex = nil if annotation == annotationPolyRelation { - choiceMapping, err = choiceStructMapping(fieldValue.Type()) - if err != nil { - er = err - break - } + choiceMapping = choiceStructMapping(fieldValue.Type()) } if isSlice { diff --git a/request_test.go b/request_test.go index a58df7f..5a1b24a 100644 --- a/request_test.go +++ b/request_test.go @@ -743,10 +743,7 @@ func Test_choiceStructMapping(t *testing.T) { } for _, c := range cases { - result, err := choiceStructMapping(c.val) - if err != nil { - t.Fatal(err) - } + result := choiceStructMapping(c.val) imageField, ok := result["images"] if !ok || imageField.FieldNum != 0 { t.Errorf("expected \"images\" to be the first field, but got %d", imageField.FieldNum) From bb4d09f5b4612c774fe7bbe457c57eb7f9b7ccc0 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Wed, 15 Nov 2023 10:49:51 -0700 Subject: [PATCH 28/49] Fix omitted model value for polyrelation fields --- request_test.go | 31 +++++++++++++++++++++++++++++++ response.go | 7 ++++++- response_test.go | 12 ++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/request_test.go b/request_test.go index 5a1b24a..7eb9bda 100644 --- a/request_test.go +++ b/request_test.go @@ -734,6 +734,37 @@ func Test_UnmarshalPayload_polymorphicRelations_no_choice(t *testing.T) { } } +func Test_UnmarshalPayload_polymorphicRelations_omitted(t *testing.T) { + type pointerToOne struct { + ID string `jsonapi:"primary,blogs"` + Title string `jsonapi:"attr,title"` + Hero *OneOfMedia `jsonapi:"polyrelation,hero-media"` + } + + in := bytes.NewReader([]byte(`{ + "data": { + "type": "blogs", + "id": "3", + "attributes": { + "title": "Hello, World" + } + } + }`)) + out := new(pointerToOne) + + if err := UnmarshalPayload(in, out); err != nil { + t.Fatal(err) + } + + if out.Title != "Hello, World" { + t.Errorf("expected Title %q but got %q", "Hello, World", out.Title) + } + + if out.Hero != nil { + t.Fatalf("expected Hero to be nil, but got %+v", out.Hero) + } +} + func Test_choiceStructMapping(t *testing.T) { cases := []struct { val reflect.Type diff --git a/response.go b/response.go index e23dd9c..602b16b 100644 --- a/response.go +++ b/response.go @@ -417,8 +417,13 @@ func visitModelNode(model interface{}, included *map[string]*Node, 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 } diff --git a/response_test.go b/response_test.go index da10dee..599d999 100644 --- a/response_test.go +++ b/response_test.go @@ -167,6 +167,18 @@ func TestMarshalPayloadWithHasOneNilPolyrelation(t *testing.T) { } } +func TestMarshalPayloadWithHasOneOmittedPolyrelation(t *testing.T) { + blog := &BlogPostWithPoly{ + ID: "1", + Title: "Hello, World", + } + + out := bytes.NewBuffer(nil) + if err := MarshalPayload(out, blog); err != nil { + t.Fatalf("expected no error but got %s", err) + } +} + func TestMarshalPayloadWithHasOneNilRelation(t *testing.T) { blog := &Blog{ ID: 1, From 4d8a31fcec17b86a0a6a6f4bc55b9370d0910f23 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Wed, 15 Nov 2023 14:34:40 -0700 Subject: [PATCH 29/49] Update README.md --- README.md | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 7b86d7d..a2ba413 100644 --- a/README.md +++ b/README.md @@ -1,19 +1,19 @@ # jsonapi -[![Build Status](https://travis-ci.org/google/jsonapi.svg?branch=master)](https://travis-ci.org/google/jsonapi) -[![Go Report Card](https://goreportcard.com/badge/github.com/google/jsonapi)](https://goreportcard.com/report/github.com/google/jsonapi) -[![GoDoc](https://godoc.org/github.com/google/jsonapi?status.svg)](http://godoc.org/github.com/google/jsonapi) -[![No Maintenance Intended](http://unmaintained.tech/badge.svg)](http://unmaintained.tech/) +![Build Status](https://github.com/hashicorp/jsonapi/actions/workflows/ci.yml/badge.svg?main) +![Go Report Card](https://goreportcard.com/badge/github.com/hashicorp/jsonapi) +![GoDoc](https://godoc.org/github.com/hashicorp/jsonapi?status.svg) A serializer/deserializer for JSON payloads that comply to the -[JSON API - jsonapi.org](http://jsonapi.org) spec in go. - +[JSON API - jsonapi.org](http://jsonapi.org) v1.1 spec in go. +This package was forked from [google/jsonapi](https://github.com/google/jsonapi) and +adds several enhancements such as [links](#links) and [polymorphic relationships](#polyrelation). ## Installation ``` -go get -u github.com/google/jsonapi +go get -u github.com/hashicorp/jsonapi ``` Or, see [Alternative Installation](#alternative-installation). @@ -91,9 +91,9 @@ To run, * Make sure you have [Go installed](https://golang.org/doc/install) * Create the following directories or similar: `~/go` * Set `GOPATH` to `PWD` in your shell session, `export GOPATH=$PWD` -* `go get github.com/google/jsonapi`. (Append `-u` after `get` if you +* `go get github.com/hashicorp/jsonapi`. (Append `-u` after `get` if you are updating.) -* `cd $GOPATH/src/github.com/google/jsonapi/examples` +* `cd $GOPATH/src/github.com/hashicorp/jsonapi/examples` * `go build && ./examples` ## `jsonapi` Tag Reference @@ -234,9 +234,6 @@ struct. When accepting input values on this type of choice type, it would a good and check that the value is set on only one field. #### `links` - -*Note: This annotation is an added feature independent of the canonical google/jsonapi package* - ``` `jsonapi:"links,omitempty"` ``` @@ -256,7 +253,7 @@ about the rest? ### Create Record Example You can Unmarshal a JSON API payload using -[jsonapi.UnmarshalPayload](http://godoc.org/github.com/google/jsonapi#UnmarshalPayload). +[jsonapi.UnmarshalPayload](http://godoc.org/github.com/hashicorp/jsonapi#UnmarshalPayload). It reads from an [io.Reader](https://golang.org/pkg/io/#Reader) containing a JSON API payload for one record (but can have related records). Then, it materializes a struct that you created and passed in @@ -265,7 +262,7 @@ the top level, in request payloads at the moment. Bulk creates and updates are not supported yet. After saving your record, you can use, -[MarshalOnePayload](http://godoc.org/github.com/google/jsonapi#MarshalOnePayload), +[MarshalOnePayload](http://godoc.org/github.com/hashicorp/jsonapi#MarshalOnePayload), to write the JSON API response to an [io.Writer](https://golang.org/pkg/io/#Writer). @@ -275,7 +272,7 @@ to write the JSON API response to an UnmarshalPayload(in io.Reader, model interface{}) ``` -Visit [godoc](http://godoc.org/github.com/google/jsonapi#UnmarshalPayload) +Visit [godoc](http://godoc.org/github.com/hashicorp/jsonapi#UnmarshalPayload) #### `MarshalPayload` @@ -283,7 +280,7 @@ Visit [godoc](http://godoc.org/github.com/google/jsonapi#UnmarshalPayload) MarshalPayload(w io.Writer, models interface{}) error ``` -Visit [godoc](http://godoc.org/github.com/google/jsonapi#MarshalPayload) +Visit [godoc](http://godoc.org/github.com/hashicorp/jsonapi#MarshalPayload) Writes a JSON API response, with related records sideloaded, into an `included` array. This method encodes a response for either a single record or @@ -319,7 +316,7 @@ func CreateBlog(w http.ResponseWriter, r *http.Request) { UnmarshalManyPayload(in io.Reader, t reflect.Type) ([]interface{}, error) ``` -Visit [godoc](http://godoc.org/github.com/google/jsonapi#UnmarshalManyPayload) +Visit [godoc](http://godoc.org/github.com/hashicorp/jsonapi#UnmarshalManyPayload) Takes an `io.Reader` and a `reflect.Type` representing the uniform type contained within the `"data"` JSON API member. @@ -485,7 +482,7 @@ if err := validate(&myStructToValidate); err != nil { MarshalOnePayloadEmbedded(w io.Writer, model interface{}) error ``` -Visit [godoc](http://godoc.org/github.com/google/jsonapi#MarshalOnePayloadEmbedded) +Visit [godoc](http://godoc.org/github.com/hashicorp/jsonapi#MarshalOnePayloadEmbedded) This method is not strictly meant to for use in implementation code, although feel free. It was mainly created for use in tests; in most cases, From 0044c38e42e94f45979a056ae2991b4331c70309 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Wed, 15 Nov 2023 14:53:50 -0700 Subject: [PATCH 30/49] Adds links to README badges --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index a2ba413..1c870c4 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ # jsonapi -![Build Status](https://github.com/hashicorp/jsonapi/actions/workflows/ci.yml/badge.svg?main) -![Go Report Card](https://goreportcard.com/badge/github.com/hashicorp/jsonapi) -![GoDoc](https://godoc.org/github.com/hashicorp/jsonapi?status.svg) +[![Build Status](https://github.com/hashicorp/jsonapi/actions/workflows/ci.yml/badge.svg?main)](https://github.com/hashicorp/jsonapi/actions/workflows/ci.yml?query=branch%3Amain) +[![Go Report Card](https://goreportcard.com/badge/github.com/hashicorp/jsonapi)](https://goreportcard.com/report/github.com/hashicorp/jsonapi) +[![GoDoc](https://godoc.org/github.com/hashicorp/jsonapi?status.svg)](http://godoc.org/github.com/hashicorp/jsonapi) A serializer/deserializer for JSON payloads that comply to the [JSON API - jsonapi.org](http://jsonapi.org) v1.1 spec in go. From 0bf163a5b485660690ae8a112fb12ea4a24d490e Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Thu, 4 Jan 2024 15:17:30 -0800 Subject: [PATCH 31/49] feat: introduce nullable types --- go.mod | 2 + models_test.go | 9 +++ nullable.go | 113 ++++++++++++++++++++++++++++++++++++ request.go | 35 ++++++++++- request_test.go | 97 +++++++++++++++++++++++++++++++ response.go | 16 +++++ response_test.go | 148 +++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 419 insertions(+), 1 deletion(-) create mode 100644 nullable.go diff --git a/go.mod b/go.mod index f8ae3f6..4f90467 100644 --- a/go.mod +++ b/go.mod @@ -1 +1,3 @@ module github.com/hashicorp/jsonapi + +go 1.18 diff --git a/models_test.go b/models_test.go index 889142a..9144cf4 100644 --- a/models_test.go +++ b/models_test.go @@ -35,6 +35,15 @@ type TimestampModel struct { RFC3339P *time.Time `jsonapi:"attr,rfc3339p,rfc3339"` } +type WithNullables struct { + ID int `jsonapi:"primary,with-nullables"` + Name string `jsonapi:"attr,name"` + IntTime Nullable[time.Time] `jsonapi:"attr,int_time,omitempty"` + RFC3339Time Nullable[time.Time] `jsonapi:"attr,rfc3339_time,rfc3339,omitempty"` + ISO8601Time Nullable[time.Time] `jsonapi:"attr,iso8601_time,iso8601,omitempty"` + Bool Nullable[bool] `jsonapi:"attr,bool,omitempty"` +} + type Car struct { ID *string `jsonapi:"primary,cars"` Make *string `jsonapi:"attr,make,omitempty"` diff --git a/nullable.go b/nullable.go new file mode 100644 index 0000000..a8ff331 --- /dev/null +++ b/nullable.go @@ -0,0 +1,113 @@ +package jsonapi + +import ( + "errors" + "reflect" + "time" +) + +var supportedNullableTypes = map[string]reflect.Value{ + "bool": reflect.ValueOf(false), + "time.Time": reflect.ValueOf(time.Time{}), +} + +// Nullable is a generic type, which implements a field that can be one of three states: +// +// - field is not set in the request +// - field is explicitly set to `null` in the request +// - field is explicitly set to a valid value in the request +// +// Nullable is intended to be used with JSON marshalling and unmarshalling. +// This is generally useful for PATCH requests, where attributes 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 field is expected to be optional, add the `omitempty` JSON tags. Do NOT use `*Nullable`! +// +// Adapted from https://www.jvt.me/posts/2024/01/09/go-json-nullable/ + +type Nullable[T any] map[bool]T + +// NewNullableWithValue is a convenience helper to allow constructing a +// Nullable with a given value, for instance to construct a field inside a +// struct without introducing an intermediate variable. +func NewNullableWithValue[T any](t T) Nullable[T] { + var n Nullable[T] + n.Set(t) + return n +} + +// NewNullNullable is a convenience helper to allow constructing a Nullable with +// an explicit `null`, for instance to construct a field inside a struct +// without introducing an intermediate variable +func NewNullNullable[T any]() Nullable[T] { + var n Nullable[T] + n.SetNull() + return n +} + +// Get retrieves the underlying value, if present, and returns an error if the value was not present +func (t Nullable[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 *Nullable[T]) Set(value T) { + *t = map[bool]T{true: value} +} + +// Set sets the underlying value to a given value +func (t *Nullable[T]) SetInterface(value interface{}) { + t.Set(value.(T)) +} + +// IsNull indicate whether the field was sent, and had a value of `null` +func (t Nullable[T]) IsNull() bool { + _, foundNull := t[false] + return foundNull +} + +// SetNull indicate that the field was sent, and had a value of `null` +func (t *Nullable[T]) SetNull() { + var empty T + *t = map[bool]T{false: empty} +} + +// IsSpecified indicates whether the field was sent +func (t Nullable[T]) IsSpecified() bool { + return len(t) != 0 +} + +// SetUnspecified indicate whether the field was sent +func (t *Nullable[T]) SetUnspecified() { + *t = map[bool]T{} +} + +func NullableBool(v bool) Nullable[bool] { + return NewNullableWithValue[bool](v) +} + +func NullBool() Nullable[bool] { + return NewNullNullable[bool]() +} + +func NullableTime(v time.Time) Nullable[time.Time] { + return NewNullableWithValue[time.Time](v) +} + +func NullTime() Nullable[time.Time] { + return NewNullNullable[time.Time]() +} diff --git a/request.go b/request.go index a12c2b1..96bce66 100644 --- a/request.go +++ b/request.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "reflect" + "regexp" "strconv" "strings" "time" @@ -589,6 +590,12 @@ func unmarshalAttribute( value = reflect.ValueOf(attribute) fieldType := structField.Type + // Handle Nullable[T] + if strings.HasPrefix(fieldValue.Type().Name(), "Nullable[") { + value, err = handleNullable(attribute, args, structField, fieldValue) + return + } + // Handle field of type []string if fieldValue.Type() == reflect.TypeOf([]string{}) { value, err = handleStringSlice(attribute) @@ -656,6 +663,32 @@ func handleStringSlice(attribute interface{}) (reflect.Value, error) { return reflect.ValueOf(values), nil } +func handleNullable( + attribute interface{}, + args []string, + structField reflect.StructField, + fieldValue reflect.Value) (reflect.Value, error) { + + if a, ok := attribute.(string); ok { + if bytes.Equal([]byte(a), []byte("null")) { + return reflect.ValueOf(nil), nil + } + } + + var rgx = regexp.MustCompile(`\[(.*)\]`) + rs := rgx.FindStringSubmatch(fieldValue.Type().Name()) + + attrVal, err := unmarshalAttribute(attribute, args, structField, supportedNullableTypes[rs[1]]) + if err != nil { + return reflect.ValueOf(nil), err + } + + fieldValue.Set(reflect.MakeMap(fieldValue.Type())) + fieldValue.SetMapIndex(reflect.ValueOf(true), attrVal) + + return fieldValue, nil +} + func handleTime(attribute interface{}, args []string, fieldValue reflect.Value) (reflect.Value, error) { var isISO8601, isRFC3339 bool v := reflect.ValueOf(attribute) @@ -714,7 +747,7 @@ func handleTime(attribute interface{}, args []string, fieldValue reflect.Value) return reflect.ValueOf(time.Now()), ErrInvalidTime } - t := time.Unix(at, 0) + t := time.Unix(at, 0).UTC() return reflect.ValueOf(t), nil } diff --git a/request_test.go b/request_test.go index 7eb9bda..b9b857d 100644 --- a/request_test.go +++ b/request_test.go @@ -300,6 +300,88 @@ func TestStringPointerField(t *testing.T) { } } +func TestUnmarshalNullableTime(t *testing.T) { + aTime := time.Date(2016, 8, 17, 8, 27, 12, 23849, time.UTC) + + out := new(WithNullables) + + attrs := map[string]interface{}{ + "name": "Name", + "int_time": aTime.Unix(), + "rfc3339_time": aTime.Format(time.RFC3339), + "iso8601_time": aTime.Format(iso8601TimeFormat), + } + + if err := UnmarshalPayload(samplePayloadWithNullables(attrs), out); err != nil { + t.Fatal(err) + } + + if out.IntTime == nil { + t.Fatal("Was not expecting a nil pointer for out.IntTime") + } + + timeVal, err := out.IntTime.Get() + if err != nil { + t.Fatal(err) + } + + if expected, actual := aTime, timeVal; expected.Equal(actual) { + t.Fatalf("Was expecting int_time to be `%s`, got `%s`", expected, actual) + } + + timeVal, err = out.IntTime.Get() + if err != nil { + t.Fatal(err) + } + + if out.RFC3339Time == nil { + t.Fatal("Was not expecting a nil pointer for out.RFC3339Time") + } + if expected, actual := aTime, timeVal; expected.Equal(actual) { + t.Fatalf("Was expecting descript to be `%s`, got `%s`", expected, actual) + } + + timeVal, err = out.IntTime.Get() + if err != nil { + t.Fatal(err) + } + + if out.ISO8601Time == nil { + t.Fatal("Was not expecting a nil pointer for out.ISO8601Time") + } + if expected, actual := aTime, timeVal; expected.Equal(actual) { + t.Fatalf("Was expecting descript to be `%s`, got `%s`", expected, actual) + } +} + +func TestUnmarshalNullableBool(t *testing.T) { + out := new(WithNullables) + + aBool := false + + attrs := map[string]interface{}{ + "name": "Name", + "bool": aBool, + } + + if err := UnmarshalPayload(samplePayloadWithNullables(attrs), out); err != nil { + t.Fatal(err) + } + + if out.Bool == nil { + t.Fatal("Was not expecting a nil pointer for out.Bool") + } + + boolVal, err := out.Bool.Get() + if err != nil { + t.Fatal(err) + } + + if expected, actual := aBool, boolVal; expected != actual { + t.Fatalf("Was expecting bool to be `%t`, got `%t`", expected, actual) + } +} + func TestMalformedTag(t *testing.T) { out := new(BadModel) err := UnmarshalPayload(samplePayload(), out) @@ -1426,6 +1508,21 @@ func sampleWithPointerPayload(m map[string]interface{}) io.Reader { return out } +func samplePayloadWithNullables(m map[string]interface{}) io.Reader { + payload := &OnePayload{ + Data: &Node{ + ID: "5", + Type: "with-nullables", + Attributes: m, + }, + } + + out := bytes.NewBuffer(nil) + json.NewEncoder(out).Encode(payload) + + return out +} + func testModel() *Blog { return &Blog{ ID: 5, diff --git a/response.go b/response.go index 602b16b..85e38e4 100644 --- a/response.go +++ b/response.go @@ -331,6 +331,22 @@ func visitModelNode(model interface{}, included *map[string]*Node, node.Attributes = make(map[string]interface{}) } + // Handle Nullable[T] + if strings.HasPrefix(fieldValue.Type().Name(), "Nullable[") { + // handle unspecified + if fieldValue.IsNil() { + continue + } + + // handle null + if fieldValue.MapIndex(reflect.ValueOf(false)).IsValid() { + continue + } + + // handle value + fieldValue = fieldValue.MapIndex(reflect.ValueOf(true)) + } + if fieldValue.Type() == reflect.TypeOf(time.Time{}) { t := fieldValue.Interface().(time.Time) diff --git a/response_test.go b/response_test.go index 599d999..74f0148 100644 --- a/response_test.go +++ b/response_test.go @@ -820,6 +820,154 @@ func TestMarshal_Times(t *testing.T) { } } +func TestCustomMarshal_Time(t *testing.T) { + aTime := time.Date(2016, 8, 17, 8, 27, 12, 23849, time.UTC) + + for _, tc := range []struct { + desc string + input *WithNullables + verification func(data map[string]interface{}) error + }{ + { + desc: "time_nil", + input: &WithNullables{ + ID: 5, + RFC3339Time: nil, + }, + verification: func(root map[string]interface{}) error { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["rfc3339_time"] + if got, want := v, (interface{})(nil); got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + return nil + }, + }, + { + desc: "time_value_rfc3339", + input: &WithNullables{ + ID: 5, + RFC3339Time: NullableTime(aTime), + }, + verification: func(root map[string]interface{}) error { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["rfc3339_time"].(string) + if got, want := v, aTime.Format(time.RFC3339); got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + return nil + }, + }, + { + desc: "time_value_iso8601", + input: &WithNullables{ + ID: 5, + ISO8601Time: NullableTime(aTime), + }, + verification: func(root map[string]interface{}) error { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["iso8601_time"].(string) + if got, want := v, aTime.Format(iso8601TimeFormat); got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + return nil + }, + }, + { + desc: "time_null_integer", + input: &WithNullables{ + ID: 5, + IntTime: NullTime(), + }, + verification: func(root map[string]interface{}) error { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["int_time"] + if got, want := v, (interface{})(nil); 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 TestCustomMarshal_Bool(t *testing.T) { + aBool := true + + for _, tc := range []struct { + desc string + input *WithNullables + verification func(data map[string]interface{}) error + }{ + { + desc: "bool_nil", + input: &WithNullables{ + ID: 5, + Bool: nil, + }, + verification: func(root map[string]interface{}) error { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"] + if got, want := v, (interface{})(nil); got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + return nil + }, + }, + { + desc: "unsetable_value_present", + input: &WithNullables{ + ID: 5, + Bool: NullableBool(aBool), + }, + verification: func(root map[string]interface{}) error { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"].(bool) + if got, want := v, aBool; got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + return nil + }, + }, + { + desc: "unsetable_nil_value", + input: &WithNullables{ + ID: 5, + Bool: NullBool(), + }, + verification: func(root map[string]interface{}) error { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"] + if got, want := v, (interface{})(nil); 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 61c1233838ef2f46513af00edf19eda2c9b3b2ae Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Thu, 11 Jan 2024 17:57:27 -0800 Subject: [PATCH 32/49] enforce go >=1.18 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8df71bd..e652795 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go: [ '1.21', '1.20', '1.19', '1.18', '1.17', '1.11' ] + go: [ '1.21', '1.20', '1.19', '1.18'] steps: - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 From 3414f84788e53a3a3fb876a0162548651dc17127 Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Thu, 11 Jan 2024 17:59:08 -0800 Subject: [PATCH 33/49] chore: allow overriding of supported nullable type map --- nullable.go | 2 +- request.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nullable.go b/nullable.go index a8ff331..1b18428 100644 --- a/nullable.go +++ b/nullable.go @@ -6,7 +6,7 @@ import ( "time" ) -var supportedNullableTypes = map[string]reflect.Value{ +var SupportedNullableTypes = map[string]reflect.Value{ "bool": reflect.ValueOf(false), "time.Time": reflect.ValueOf(time.Time{}), } diff --git a/request.go b/request.go index 96bce66..a4d3bf0 100644 --- a/request.go +++ b/request.go @@ -678,7 +678,7 @@ func handleNullable( var rgx = regexp.MustCompile(`\[(.*)\]`) rs := rgx.FindStringSubmatch(fieldValue.Type().Name()) - attrVal, err := unmarshalAttribute(attribute, args, structField, supportedNullableTypes[rs[1]]) + attrVal, err := unmarshalAttribute(attribute, args, structField, SupportedNullableTypes[rs[1]]) if err != nil { return reflect.ValueOf(nil), err } From 93a15272e02cc96f1004be2ca6b2631d81ddc8d4 Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Fri, 12 Jan 2024 09:44:54 -0800 Subject: [PATCH 34/49] chore: incorporate review feedback --- nullable.go | 6 ------ request.go | 7 +++---- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/nullable.go b/nullable.go index 1b18428..23e2ed7 100644 --- a/nullable.go +++ b/nullable.go @@ -2,15 +2,9 @@ package jsonapi import ( "errors" - "reflect" "time" ) -var SupportedNullableTypes = map[string]reflect.Value{ - "bool": reflect.ValueOf(false), - "time.Time": reflect.ValueOf(time.Time{}), -} - // Nullable is a generic type, which implements a field that can be one of three states: // // - field is not set in the request diff --git a/request.go b/request.go index a4d3bf0..b617d95 100644 --- a/request.go +++ b/request.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "reflect" - "regexp" "strconv" "strings" "time" @@ -675,10 +674,10 @@ func handleNullable( } } - var rgx = regexp.MustCompile(`\[(.*)\]`) - rs := rgx.FindStringSubmatch(fieldValue.Type().Name()) + innerType := fieldValue.Type().Elem() + zeroValue := reflect.Zero(innerType) - attrVal, err := unmarshalAttribute(attribute, args, structField, SupportedNullableTypes[rs[1]]) + attrVal, err := unmarshalAttribute(attribute, args, structField, zeroValue) if err != nil { return reflect.ValueOf(nil), err } From 2acfcfe21d5a1138fd43246f883f87e15e5efbd0 Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Fri, 12 Jan 2024 13:53:53 -0800 Subject: [PATCH 35/49] tests: update tests --- models_test.go | 14 +++++----- nullable.go | 53 ++++++++++++++++++------------------ request.go | 6 ++-- request_test.go | 10 +++---- response.go | 2 +- response_test.go | 71 +++++++++++++++++++++++++++++------------------- 6 files changed, 85 insertions(+), 71 deletions(-) diff --git a/models_test.go b/models_test.go index 9144cf4..1b6a5ac 100644 --- a/models_test.go +++ b/models_test.go @@ -35,13 +35,13 @@ type TimestampModel struct { RFC3339P *time.Time `jsonapi:"attr,rfc3339p,rfc3339"` } -type WithNullables struct { - ID int `jsonapi:"primary,with-nullables"` - Name string `jsonapi:"attr,name"` - IntTime Nullable[time.Time] `jsonapi:"attr,int_time,omitempty"` - RFC3339Time Nullable[time.Time] `jsonapi:"attr,rfc3339_time,rfc3339,omitempty"` - ISO8601Time Nullable[time.Time] `jsonapi:"attr,iso8601_time,iso8601,omitempty"` - Bool Nullable[bool] `jsonapi:"attr,bool,omitempty"` +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"` } type Car struct { diff --git a/nullable.go b/nullable.go index 23e2ed7..73b3406 100644 --- a/nullable.go +++ b/nullable.go @@ -5,13 +5,13 @@ import ( "time" ) -// Nullable is a generic type, which implements a field that can be one of three states: +// NullableAttr is a generic type, which implements a field that can be one of three states: // // - field is not set in the request // - field is explicitly set to `null` in the request // - field is explicitly set to a valid value in the request // -// Nullable is intended to be used with JSON marshalling and unmarshalling. +// NullableAttr is intended to be used with JSON marshalling and unmarshalling. // This is generally useful for PATCH requests, where attributes with zero // values are intentionally not marshaled into the request payload so that // existing attribute values are not overwritten. @@ -22,32 +22,31 @@ import ( // - map[false]T means an explicit null was provided // - nil or zero map means the field was not provided // -// If the field is expected to be optional, add the `omitempty` JSON tags. Do NOT use `*Nullable`! +// If the field is expected to be optional, add the `omitempty` JSON tags. Do NOT use `*NullableAttr`! // // Adapted from https://www.jvt.me/posts/2024/01/09/go-json-nullable/ +type NullableAttr[T any] map[bool]T -type Nullable[T any] map[bool]T - -// NewNullableWithValue is a convenience helper to allow constructing a -// Nullable with a given value, for instance to construct a field inside a +// 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 NewNullableWithValue[T any](t T) Nullable[T] { - var n Nullable[T] +func NewNullableAttrWithValue[T any](t T) NullableAttr[T] { + var n NullableAttr[T] n.Set(t) return n } -// NewNullNullable is a convenience helper to allow constructing a Nullable with +// 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 NewNullNullable[T any]() Nullable[T] { - var n Nullable[T] +func NewNullNullableAttr[T any]() NullableAttr[T] { + var n NullableAttr[T] n.SetNull() return n } // Get retrieves the underlying value, if present, and returns an error if the value was not present -func (t Nullable[T]) Get() (T, error) { +func (t NullableAttr[T]) Get() (T, error) { var empty T if t.IsNull() { return empty, errors.New("value is null") @@ -59,49 +58,49 @@ func (t Nullable[T]) Get() (T, error) { } // Set sets the underlying value to a given value -func (t *Nullable[T]) Set(value T) { +func (t *NullableAttr[T]) Set(value T) { *t = map[bool]T{true: value} } // Set sets the underlying value to a given value -func (t *Nullable[T]) SetInterface(value interface{}) { +func (t *NullableAttr[T]) SetInterface(value interface{}) { t.Set(value.(T)) } // IsNull indicate whether the field was sent, and had a value of `null` -func (t Nullable[T]) IsNull() bool { +func (t NullableAttr[T]) IsNull() bool { _, foundNull := t[false] return foundNull } // SetNull indicate that the field was sent, and had a value of `null` -func (t *Nullable[T]) SetNull() { +func (t *NullableAttr[T]) SetNull() { var empty T *t = map[bool]T{false: empty} } // IsSpecified indicates whether the field was sent -func (t Nullable[T]) IsSpecified() bool { +func (t NullableAttr[T]) IsSpecified() bool { return len(t) != 0 } // SetUnspecified indicate whether the field was sent -func (t *Nullable[T]) SetUnspecified() { +func (t *NullableAttr[T]) SetUnspecified() { *t = map[bool]T{} } -func NullableBool(v bool) Nullable[bool] { - return NewNullableWithValue[bool](v) +func NullableBool(v bool) NullableAttr[bool] { + return NewNullableAttrWithValue[bool](v) } -func NullBool() Nullable[bool] { - return NewNullNullable[bool]() +func NullBool() NullableAttr[bool] { + return NewNullNullableAttr[bool]() } -func NullableTime(v time.Time) Nullable[time.Time] { - return NewNullableWithValue[time.Time](v) +func NullableTime(v time.Time) NullableAttr[time.Time] { + return NewNullableAttrWithValue[time.Time](v) } -func NullTime() Nullable[time.Time] { - return NewNullNullable[time.Time]() +func NullTime() NullableAttr[time.Time] { + return NewNullNullableAttr[time.Time]() } diff --git a/request.go b/request.go index b617d95..ad0f5f2 100644 --- a/request.go +++ b/request.go @@ -589,8 +589,8 @@ func unmarshalAttribute( value = reflect.ValueOf(attribute) fieldType := structField.Type - // Handle Nullable[T] - if strings.HasPrefix(fieldValue.Type().Name(), "Nullable[") { + // Handle NullableAttr[T] + if strings.HasPrefix(fieldValue.Type().Name(), "NullableAttr[") { value, err = handleNullable(attribute, args, structField, fieldValue) return } @@ -746,7 +746,7 @@ func handleTime(attribute interface{}, args []string, fieldValue reflect.Value) return reflect.ValueOf(time.Now()), ErrInvalidTime } - t := time.Unix(at, 0).UTC() + t := time.Unix(at, 0) return reflect.ValueOf(t), nil } diff --git a/request_test.go b/request_test.go index b9b857d..350ba6e 100644 --- a/request_test.go +++ b/request_test.go @@ -303,7 +303,7 @@ func TestStringPointerField(t *testing.T) { func TestUnmarshalNullableTime(t *testing.T) { aTime := time.Date(2016, 8, 17, 8, 27, 12, 23849, time.UTC) - out := new(WithNullables) + out := new(WithNullableAttrs) attrs := map[string]interface{}{ "name": "Name", @@ -312,7 +312,7 @@ func TestUnmarshalNullableTime(t *testing.T) { "iso8601_time": aTime.Format(iso8601TimeFormat), } - if err := UnmarshalPayload(samplePayloadWithNullables(attrs), out); err != nil { + if err := UnmarshalPayload(samplePayloadWithNullableAttrs(attrs), out); err != nil { t.Fatal(err) } @@ -355,7 +355,7 @@ func TestUnmarshalNullableTime(t *testing.T) { } func TestUnmarshalNullableBool(t *testing.T) { - out := new(WithNullables) + out := new(WithNullableAttrs) aBool := false @@ -364,7 +364,7 @@ func TestUnmarshalNullableBool(t *testing.T) { "bool": aBool, } - if err := UnmarshalPayload(samplePayloadWithNullables(attrs), out); err != nil { + if err := UnmarshalPayload(samplePayloadWithNullableAttrs(attrs), out); err != nil { t.Fatal(err) } @@ -1508,7 +1508,7 @@ func sampleWithPointerPayload(m map[string]interface{}) io.Reader { return out } -func samplePayloadWithNullables(m map[string]interface{}) io.Reader { +func samplePayloadWithNullableAttrs(m map[string]interface{}) io.Reader { payload := &OnePayload{ Data: &Node{ ID: "5", diff --git a/response.go b/response.go index 85e38e4..32be6d9 100644 --- a/response.go +++ b/response.go @@ -332,7 +332,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, } // Handle Nullable[T] - if strings.HasPrefix(fieldValue.Type().Name(), "Nullable[") { + if strings.HasPrefix(fieldValue.Type().Name(), "NullableAttr[") { // handle unspecified if fieldValue.IsNil() { continue diff --git a/response_test.go b/response_test.go index 74f0148..38a5891 100644 --- a/response_test.go +++ b/response_test.go @@ -820,17 +820,17 @@ func TestMarshal_Times(t *testing.T) { } } -func TestCustomMarshal_Time(t *testing.T) { +func TestNullableAttr_Time(t *testing.T) { aTime := time.Date(2016, 8, 17, 8, 27, 12, 23849, time.UTC) for _, tc := range []struct { desc string - input *WithNullables + input *WithNullableAttrs verification func(data map[string]interface{}) error }{ { - desc: "time_nil", - input: &WithNullables{ + desc: "time_unspecified", + input: &WithNullableAttrs{ ID: 5, RFC3339Time: nil, }, @@ -843,8 +843,22 @@ func TestCustomMarshal_Time(t *testing.T) { }, }, { - desc: "time_value_rfc3339", - input: &WithNullables{ + desc: "time_null", + input: &WithNullableAttrs{ + ID: 5, + RFC3339Time: NullTime(), + }, + verification: func(root map[string]interface{}) error { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["rfc3339_time"] + if got, want := v, (interface{})(nil); got != want { + return fmt.Errorf("got %v, want %v", got, want) + } + return nil + }, + }, + { + desc: "time_not_null_rfc3339", + input: &WithNullableAttrs{ ID: 5, RFC3339Time: NullableTime(aTime), }, @@ -857,8 +871,8 @@ func TestCustomMarshal_Time(t *testing.T) { }, }, { - desc: "time_value_iso8601", - input: &WithNullables{ + desc: "time_not_null_iso8601", + input: &WithNullableAttrs{ ID: 5, ISO8601Time: NullableTime(aTime), }, @@ -871,14 +885,14 @@ func TestCustomMarshal_Time(t *testing.T) { }, }, { - desc: "time_null_integer", - input: &WithNullables{ + desc: "time_not_null_int", + input: &WithNullableAttrs{ ID: 5, - IntTime: NullTime(), + IntTime: NullableTime(aTime), }, verification: func(root map[string]interface{}) error { - v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["int_time"] - if got, want := v, (interface{})(nil); got != want { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["int_time"].(float64) + if got, want := int64(v), aTime.Unix(); got != want { return fmt.Errorf("got %v, want %v", got, want) } return nil @@ -901,17 +915,17 @@ func TestCustomMarshal_Time(t *testing.T) { } } -func TestCustomMarshal_Bool(t *testing.T) { +func TestNullableAttr_Bool(t *testing.T) { aBool := true for _, tc := range []struct { desc string - input *WithNullables + input *WithNullableAttrs verification func(data map[string]interface{}) error }{ { - desc: "bool_nil", - input: &WithNullables{ + desc: "bool_unspecified", + input: &WithNullableAttrs{ ID: 5, Bool: nil, }, @@ -924,33 +938,34 @@ func TestCustomMarshal_Bool(t *testing.T) { }, }, { - desc: "unsetable_value_present", - input: &WithNullables{ + desc: "bool_null", + input: &WithNullableAttrs{ ID: 5, - Bool: NullableBool(aBool), + Bool: NullBool(), }, verification: func(root map[string]interface{}) error { - v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"].(bool) - if got, want := v, aBool; got != want { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"] + if got, want := v, (interface{})(nil); got != want { return fmt.Errorf("got %v, want %v", got, want) } return nil }, }, { - desc: "unsetable_nil_value", - input: &WithNullables{ + desc: "bool_not_null", + input: &WithNullableAttrs{ ID: 5, - Bool: NullBool(), + Bool: NullableBool(aBool), }, verification: func(root map[string]interface{}) error { - v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"] - if got, want := v, (interface{})(nil); got != want { + v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"].(bool) + if got, want := v, aBool; 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 { From eae9fb62ee1b61877fef3abc0aea3e0c7a8afd39 Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Fri, 12 Jan 2024 14:50:31 -0800 Subject: [PATCH 36/49] docs: document NullableAttr and provide example usage --- README.md | 66 ++++++++++++++++++++++++++++++++++++++++++++ examples/app.go | 22 +++++++++++++++ examples/fixtures.go | 4 ++- examples/handler.go | 25 +++++++++++++++++ examples/models.go | 15 +++++----- 5 files changed, 124 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 1c870c4..8def4a0 100644 --- a/README.md +++ b/README.md @@ -409,6 +409,72 @@ func (post Post) JSONAPIRelationshipMeta(relation string) *Meta { } ``` +### Nullable attributes + +Certain APIs may interpret the meaning of `null` attribute values as significantly +different from unspecified values (those that do not show up in the request). +The default use of the `omitempty` struct tag does not allow for sending +significant `null`s. + +A type is provided for this purpose if needed: `NullableAttr[T]`. This type +provides an API for sending and receiving significant `null` values for +attribute values of any type. + +In the example below, a payload is presented for a fictitious API that makes use +of significant `null` values. Once enabled, the `UnsettableTime` setting can +only be disabled by updating it to a `null` value. + +The payload struct below makes use of a `NullableAttr` with an inner `time.Time` +to allow this behavior: + +```go +type Settings struct { + ID int `jsonapi:"primary,videos"` + UnsettableTime jsonapi.NullableAttr[time.Time] `jsonapi:"attr,unsettable_time,rfc3339,omitempty"` +} +``` + +To enable the setting as described above, an non-null `time.Time` value is +sent to the API. This is done by using the exported +`NewNullableAttrWithValue[T]()` method: + +```go +s := Settings{ + ID: 1, + UnsettableTime: jsonapi.NewNullableAttrWithValue[time.Time](time.Now()), +} +``` + +To disable the setting, a `null` value needs to be sent to the API. This is done +by using the exported `NewNullNullableAttr[T]()` method: + +```go +s := Settings{ + ID: 1, + UnsettableTime: jsonapi.NewNullNullableAttr[time.Time](), +} +``` + +Once a payload has been marshaled, the attribute value is flattened to a +primitive value: +``` + "unsettable_time": "2021-01-01T02:07:14Z", +``` + +Significant nulls are also included and flattened, even when specifying `omitempty`: +``` + "unsettable_time": null, +``` + +Once a payload is unmarshaled, the target attribute field is hydrated with +the value in the payload and can be retrieved with the `Get()` method: +```go +t, err := s.UnsettableTime.Get() +``` + +All other struct tags used in the attribute definition will be honored when +marshaling and unmarshaling non-null values for the inner type. + ### Custom types Custom types are supported for primitive types, only, as attributes. Examples, diff --git a/examples/app.go b/examples/app.go index e94a101..cbd15d4 100644 --- a/examples/app.go +++ b/examples/app.go @@ -96,6 +96,28 @@ func exerciseHandler() { fmt.Println(buf.String()) fmt.Println("============== end raw jsonapi response =============") + // update + blog.UnsettableTime = jsonapi.NewNullableAttrWithValue[time.Time](time.Now()) + in = bytes.NewBuffer(nil) + jsonapi.MarshalOnePayloadEmbedded(in, blog) + + req, _ = http.NewRequest(http.MethodPatch, "/blogs", in) + + req.Header.Set(headerAccept, jsonapi.MediaType) + + w = httptest.NewRecorder() + + fmt.Println("============ start update ===========") + http.DefaultServeMux.ServeHTTP(w, req) + fmt.Println("============ stop update ===========") + + buf = bytes.NewBuffer(nil) + io.Copy(buf, w.Body) + + fmt.Println("============ jsonapi response from update ===========") + fmt.Println(buf.String()) + fmt.Println("============== end raw jsonapi response =============") + // echo blogs := []interface{}{ fixtureBlogCreate(1), diff --git a/examples/fixtures.go b/examples/fixtures.go index 7d0402d..6c87983 100644 --- a/examples/fixtures.go +++ b/examples/fixtures.go @@ -1,6 +1,8 @@ package main -import "time" +import ( + "time" +) func fixtureBlogCreate(i int) *Blog { return &Blog{ diff --git a/examples/handler.go b/examples/handler.go index 4500ca8..f01a360 100644 --- a/examples/handler.go +++ b/examples/handler.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "net/http" "strconv" @@ -25,6 +26,8 @@ func (h *ExampleHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodPost: methodHandler = h.createBlog + case http.MethodPatch: + methodHandler = h.updateBlog case http.MethodPut: methodHandler = h.echoBlogs case http.MethodGet: @@ -61,6 +64,28 @@ func (h *ExampleHandler) createBlog(w http.ResponseWriter, r *http.Request) { } } +func (h *ExampleHandler) updateBlog(w http.ResponseWriter, r *http.Request) { + jsonapiRuntime := jsonapi.NewRuntime().Instrument("blogs.update") + + blog := new(Blog) + + if err := jsonapiRuntime.UnmarshalPayload(r.Body, blog); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + fmt.Println(blog) + + // ...do stuff with your blog... + + w.WriteHeader(http.StatusCreated) + w.Header().Set(headerContentType, jsonapi.MediaType) + + if err := jsonapiRuntime.MarshalPayload(w, blog); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } +} + func (h *ExampleHandler) echoBlogs(w http.ResponseWriter, r *http.Request) { jsonapiRuntime := jsonapi.NewRuntime().Instrument("blogs.list") // ...fetch your blogs, filter, offset, limit, etc... diff --git a/examples/models.go b/examples/models.go index 4842361..8334725 100644 --- a/examples/models.go +++ b/examples/models.go @@ -9,13 +9,14 @@ import ( // Blog is a model representing a blog site type Blog struct { - ID int `jsonapi:"primary,blogs"` - Title string `jsonapi:"attr,title"` - Posts []*Post `jsonapi:"relation,posts"` - CurrentPost *Post `jsonapi:"relation,current_post"` - CurrentPostID int `jsonapi:"attr,current_post_id"` - CreatedAt time.Time `jsonapi:"attr,created_at"` - ViewCount int `jsonapi:"attr,view_count"` + ID int `jsonapi:"primary,blogs"` + Title string `jsonapi:"attr,title"` + Posts []*Post `jsonapi:"relation,posts"` + CurrentPost *Post `jsonapi:"relation,current_post"` + CurrentPostID int `jsonapi:"attr,current_post_id"` + CreatedAt time.Time `jsonapi:"attr,created_at"` + UnsettableTime jsonapi.NullableAttr[time.Time] `jsonapi:"attr,unsettable_time,rfc3339,omitempty"` + ViewCount int `jsonapi:"attr,view_count"` } // Post is a model representing a post on a blog From cf85dabfcf3623b1299014f617a4d0291267833f Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Fri, 12 Jan 2024 15:00:19 -0800 Subject: [PATCH 37/49] chore: incorporate review feedback Co-authored-by: Nick Fagerlund --- nullable.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nullable.go b/nullable.go index 73b3406..cd1b7b2 100644 --- a/nullable.go +++ b/nullable.go @@ -73,7 +73,7 @@ func (t NullableAttr[T]) IsNull() bool { return foundNull } -// SetNull indicate that the field was sent, and had a value of `null` +// SetNull sets the value to an explicit `null` func (t *NullableAttr[T]) SetNull() { var empty T *t = map[bool]T{false: empty} @@ -84,7 +84,7 @@ func (t NullableAttr[T]) IsSpecified() bool { return len(t) != 0 } -// SetUnspecified indicate whether the field was sent +// SetUnspecified sets the value to be absent from the serialized payload func (t *NullableAttr[T]) SetUnspecified() { *t = map[bool]T{} } From 2dbeecf8ee04330bce81c815cbb253dd994dd564 Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Tue, 30 Jan 2024 13:02:19 -0800 Subject: [PATCH 38/49] chore: incorporate review feedback --- nullable.go | 17 ----------------- request.go | 8 +++----- response_test.go | 12 ++++++------ 3 files changed, 9 insertions(+), 28 deletions(-) diff --git a/nullable.go b/nullable.go index cd1b7b2..68910f6 100644 --- a/nullable.go +++ b/nullable.go @@ -2,7 +2,6 @@ package jsonapi import ( "errors" - "time" ) // NullableAttr is a generic type, which implements a field that can be one of three states: @@ -88,19 +87,3 @@ func (t NullableAttr[T]) IsSpecified() bool { func (t *NullableAttr[T]) SetUnspecified() { *t = map[bool]T{} } - -func NullableBool(v bool) NullableAttr[bool] { - return NewNullableAttrWithValue[bool](v) -} - -func NullBool() NullableAttr[bool] { - return NewNullNullableAttr[bool]() -} - -func NullableTime(v time.Time) NullableAttr[time.Time] { - return NewNullableAttrWithValue[time.Time](v) -} - -func NullTime() NullableAttr[time.Time] { - return NewNullNullableAttr[time.Time]() -} diff --git a/request.go b/request.go index ad0f5f2..e9ea55b 100644 --- a/request.go +++ b/request.go @@ -668,10 +668,8 @@ func handleNullable( structField reflect.StructField, fieldValue reflect.Value) (reflect.Value, error) { - if a, ok := attribute.(string); ok { - if bytes.Equal([]byte(a), []byte("null")) { - return reflect.ValueOf(nil), nil - } + if a, ok := attribute.(string); ok && a == "null" { + return reflect.ValueOf(nil), nil } innerType := fieldValue.Type().Elem() @@ -682,7 +680,7 @@ func handleNullable( return reflect.ValueOf(nil), err } - fieldValue.Set(reflect.MakeMap(fieldValue.Type())) + fieldValue.Set(reflect.MakeMapWithSize(fieldValue.Type(), 1)) fieldValue.SetMapIndex(reflect.ValueOf(true), attrVal) return fieldValue, nil diff --git a/response_test.go b/response_test.go index 38a5891..7fc9ecb 100644 --- a/response_test.go +++ b/response_test.go @@ -846,7 +846,7 @@ func TestNullableAttr_Time(t *testing.T) { desc: "time_null", input: &WithNullableAttrs{ ID: 5, - RFC3339Time: NullTime(), + RFC3339Time: NewNullNullableAttr[time.Time](), }, verification: func(root map[string]interface{}) error { v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["rfc3339_time"] @@ -860,7 +860,7 @@ func TestNullableAttr_Time(t *testing.T) { desc: "time_not_null_rfc3339", input: &WithNullableAttrs{ ID: 5, - RFC3339Time: NullableTime(aTime), + RFC3339Time: NewNullableAttrWithValue[time.Time](aTime), }, verification: func(root map[string]interface{}) error { v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["rfc3339_time"].(string) @@ -874,7 +874,7 @@ func TestNullableAttr_Time(t *testing.T) { desc: "time_not_null_iso8601", input: &WithNullableAttrs{ ID: 5, - ISO8601Time: NullableTime(aTime), + ISO8601Time: NewNullableAttrWithValue[time.Time](aTime), }, verification: func(root map[string]interface{}) error { v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["iso8601_time"].(string) @@ -888,7 +888,7 @@ func TestNullableAttr_Time(t *testing.T) { desc: "time_not_null_int", input: &WithNullableAttrs{ ID: 5, - IntTime: NullableTime(aTime), + IntTime: NewNullableAttrWithValue[time.Time](aTime), }, verification: func(root map[string]interface{}) error { v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["int_time"].(float64) @@ -941,7 +941,7 @@ func TestNullableAttr_Bool(t *testing.T) { desc: "bool_null", input: &WithNullableAttrs{ ID: 5, - Bool: NullBool(), + Bool: NewNullNullableAttr[bool](), }, verification: func(root map[string]interface{}) error { v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"] @@ -955,7 +955,7 @@ func TestNullableAttr_Bool(t *testing.T) { desc: "bool_not_null", input: &WithNullableAttrs{ ID: 5, - Bool: NullableBool(aBool), + Bool: NewNullableAttrWithValue[bool](aBool), }, verification: func(root map[string]interface{}) error { v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"].(bool) From f45a8739f3067fe0f8f43c2f613d6a36185223b2 Mon Sep 17 00:00:00 2001 From: Chris Trombley Date: Tue, 30 Jan 2024 17:12:06 -0800 Subject: [PATCH 39/49] fix: nullable null marshaling --- response.go | 8 +++++--- response_test.go | 20 ++++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/response.go b/response.go index 32be6d9..dea77d8 100644 --- a/response.go +++ b/response.go @@ -340,11 +340,13 @@ func visitModelNode(model interface{}, included *map[string]*Node, // handle null if fieldValue.MapIndex(reflect.ValueOf(false)).IsValid() { + node.Attributes[args[1]] = json.RawMessage("null") continue - } + } else { - // handle value - fieldValue = fieldValue.MapIndex(reflect.ValueOf(true)) + // handle value + fieldValue = fieldValue.MapIndex(reflect.ValueOf(true)) + } } if fieldValue.Type() == reflect.TypeOf(time.Time{}) { diff --git a/response_test.go b/response_test.go index 7fc9ecb..d79d64f 100644 --- a/response_test.go +++ b/response_test.go @@ -835,8 +835,9 @@ func TestNullableAttr_Time(t *testing.T) { RFC3339Time: nil, }, verification: func(root map[string]interface{}) error { - v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["rfc3339_time"] - if got, want := v, (interface{})(nil); got != want { + _, ok := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["rfc3339_time"] + + if got, want := ok, false; got != want { return fmt.Errorf("got %v, want %v", got, want) } return nil @@ -849,8 +850,9 @@ func TestNullableAttr_Time(t *testing.T) { RFC3339Time: NewNullNullableAttr[time.Time](), }, verification: func(root map[string]interface{}) error { - v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["rfc3339_time"] - if got, want := v, (interface{})(nil); got != want { + _, ok := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["rfc3339_time"] + + if got, want := ok, true; got != want { return fmt.Errorf("got %v, want %v", got, want) } return nil @@ -930,8 +932,9 @@ func TestNullableAttr_Bool(t *testing.T) { Bool: nil, }, verification: func(root map[string]interface{}) error { - v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"] - if got, want := v, (interface{})(nil); got != want { + _, ok := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"] + + if got, want := ok, false; got != want { return fmt.Errorf("got %v, want %v", got, want) } return nil @@ -944,8 +947,9 @@ func TestNullableAttr_Bool(t *testing.T) { Bool: NewNullNullableAttr[bool](), }, verification: func(root map[string]interface{}) error { - v := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"] - if got, want := v, (interface{})(nil); got != want { + _, ok := root["data"].(map[string]interface{})["attributes"].(map[string]interface{})["bool"] + + if got, want := ok, true; got != want { return fmt.Errorf("got %v, want %v", got, want) } return nil From a31b22b8f097de87bfd881ecca2d5550892ea4e1 Mon Sep 17 00:00:00 2001 From: Jason Harley Date: Mon, 29 Jul 2024 14:48:57 -0400 Subject: [PATCH 40/49] add 'source' to error object --- errors.go | 15 +++++++++++++++ errors_test.go | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/errors.go b/errors.go index 798fed0..67a69fa 100644 --- a/errors.go +++ b/errors.go @@ -42,10 +42,25 @@ type ErrorObject struct { // Code is an application-specific error code, expressed as a string value. Code string `json:"code,omitempty"` + // Source is an object containing references to the primary source of the error. + Source *ErrorSource `json:"source,omitempty"` + // Meta is an object containing non-standard meta-information about the error. Meta *map[string]interface{} `json:"meta,omitempty"` } +// ErrorSource is an object containing references to the primary source of the error. +type ErrorSource struct { + // Pointer is a string indicating the value in the request document that caused the error. + Pointer string `json:"pointer,omitempty"` + + // Parameter is a string indicating which query or path parameter caused the error. + Parameter string `json:"parameter,omitempty"` + + // Header is a string indicating the name of a single request header which caused the error. + Header string `json:"header,omitempty"` +} + // Error implements the `Error` interface. func (e *ErrorObject) Error() string { return fmt.Sprintf("Error: %s %s\n", e.Title, e.Detail) diff --git a/errors_test.go b/errors_test.go index 683a1d1..ef07359 100644 --- a/errors_test.go +++ b/errors_test.go @@ -28,9 +28,9 @@ func TestMarshalErrorsWritesTheExpectedPayload(t *testing.T) { }{ { Title: "TestFieldsAreSerializedAsNeeded", - In: []*ErrorObject{{ID: "0", Title: "Test title.", Detail: "Test detail", Status: "400", Code: "E1100"}}, + In: []*ErrorObject{{ID: "0", Title: "Test title.", Detail: "Test detail", Status: "400", Code: "E1100", Source: &ErrorSource{Pointer: "title"}}}, Out: map[string]interface{}{"errors": []interface{}{ - map[string]interface{}{"id": "0", "title": "Test title.", "detail": "Test detail", "status": "400", "code": "E1100"}, + map[string]interface{}{"id": "0", "title": "Test title.", "detail": "Test detail", "status": "400", "code": "E1100", "source": map[string]interface{}{"pointer": "title"}}, }}, }, { From 3526b7bd08dca98be0d034d66bf4005cbd509cd9 Mon Sep 17 00:00:00 2001 From: Jason Harley Date: Fri, 2 Aug 2024 13:07:00 -0400 Subject: [PATCH 41/49] Apply doc. suggestions from code review Co-authored-by: Brandon Croft --- errors.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/errors.go b/errors.go index 67a69fa..4eab742 100644 --- a/errors.go +++ b/errors.go @@ -50,8 +50,9 @@ type ErrorObject struct { } // ErrorSource is an object containing references to the primary source of the error. +// Only one field should be populated depending on the source of the error. type ErrorSource struct { - // Pointer is a string indicating the value in the request document that caused the error. + // Pointer is a JSON Pointer (RFC6901) indicating the value in the request document that caused the error. Pointer string `json:"pointer,omitempty"` // Parameter is a string indicating which query or path parameter caused the error. From d846bbedd37785e9a0fde275ebdd0af7c0b5e86c Mon Sep 17 00:00:00 2001 From: Mukesh Choudhari Date: Mon, 18 Nov 2024 09:34:23 +0530 Subject: [PATCH 42/49] Add CODEOWNERS file in .github/CODEOWNERS --- .github/CODEOWNERS | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..3da71e0 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,13 @@ +# Each line is a file pattern followed by one or more owners. +# More on CODEOWNERS files: https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners + +# Default owner +* @hashicorp/team-ip-compliance + +# Add override rules below. Each line is a file/folder pattern followed by one or more owners. +# Being an owner means those groups or individuals will be added as reviewers to PRs affecting +# those areas of the code. +# Examples: +# /docs/ @docs-team +# *.js @js-team +# *.go @go-team \ No newline at end of file From b0c6a5b7edd8ae4e19c7ad6d47763d6b0cdd6cf7 Mon Sep 17 00:00:00 2001 From: Taylor Chaparro Date: Mon, 23 Dec 2024 13:32:50 -0800 Subject: [PATCH 43/49] fix: handle deprecating relation for polyrelation --- request.go | 28 ++++++++++++++++++++++++++++ request_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/request.go b/request.go index e9ea55b..64fc7f1 100644 --- a/request.go +++ b/request.go @@ -309,9 +309,33 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) modelValue := model.Elem() modelType := modelValue.Type() + polyrelationFields := map[string]reflect.Type{} var er error + // preprocess the model to find polyrelation fields + for i := 0; i < modelValue.NumField(); i++ { + fieldValue := modelValue.Field(i) + fieldType := modelType.Field(i) + + args, err := getStructTags(fieldType) + if err != nil { + er = err + break + } + + if len(args) < 2 { + continue + } + + annotation := args[0] + name := args[1] + + if annotation == annotationPolyRelation { + polyrelationFields[name] = fieldValue.Type() + } + } + for i := 0; i < modelValue.NumField(); i++ { fieldValue := modelValue.Field(i) fieldType := modelType.Field(i) @@ -474,6 +498,10 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) continue } + if pFieldType, ok := polyrelationFields[args[1]]; ok && fieldValue.Type() != pFieldType { + 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()) diff --git a/request_test.go b/request_test.go index 350ba6e..1408ad9 100644 --- a/request_test.go +++ b/request_test.go @@ -847,6 +847,50 @@ func Test_UnmarshalPayload_polymorphicRelations_omitted(t *testing.T) { } } +func Test_UnmarshalPayload_polymorphicRelations_deprecatedRelation(t *testing.T) { + type withDeprecatedRelation struct { + ID string `jsonapi:"primary,blogs"` + Title string `jsonapi:"attr,title"` + Media *OneOfMedia `jsonapi:"polyrelation,media"` + Image *Image `jsonapi:"relation,media"` // Deprecated + } + + in := bytes.NewReader([]byte(`{ + "data": [{ + "type": "blogs", + "id": "3", + "attributes": { + "title": "Hello, World" + }, + "relationships": { + "media": { + "data": { + "type": "videos", + "id": "123" + } + } + } + }] + }`)) + + model := reflect.TypeOf(new(withDeprecatedRelation)) + + out, err := UnmarshalManyPayload(in, model) + if err != nil { + t.Fatal(err) + } + + result := out[0].(*withDeprecatedRelation) + + if result.Title != "Hello, World" { + t.Errorf("expected Title %q but got %q", "Hello, World", result.Title) + } + + if result.Media.Video.ID != "123" { + t.Fatalf("expected Video to be \"123\", but got %+v", result.Media.Video) + } +} + func Test_choiceStructMapping(t *testing.T) { cases := []struct { val reflect.Type From 9e3a973460c42c1241e7276724429f675a33360e Mon Sep 17 00:00:00 2001 From: Taylor Chaparro Date: Tue, 7 Jan 2025 09:48:25 -0800 Subject: [PATCH 44/49] chore: comment describing when relation fields are skipped --- request.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/request.go b/request.go index 64fc7f1..27f628e 100644 --- a/request.go +++ b/request.go @@ -498,6 +498,9 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) continue } + // If the field is also a polyrelation field, then prefer the polyrelation. + // Otherwise stop processing this node. + // This is to allow relation and polyrelation fields to coexist, supporting deprecation for consumers if pFieldType, ok := polyrelationFields[args[1]]; ok && fieldValue.Type() != pFieldType { continue } From 9333e5c660c7bc7bf2f360d430936a6f767573a2 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Fri, 10 Jan 2025 17:14:59 -0700 Subject: [PATCH 45/49] Support nested objects within attributes when Marshaling 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. --- models_test.go | 1 + response.go | 33 +++++++++++++++++++++-------- response_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 9 deletions(-) diff --git a/models_test.go b/models_test.go index 1b6a5ac..0accce4 100644 --- a/models_test.go +++ b/models_test.go @@ -184,6 +184,7 @@ type Company struct { ID string `jsonapi:"primary,companies"` Name string `jsonapi:"attr,name"` Boss Employee `jsonapi:"attr,boss"` + Manager *Employee `jsonapi:"attr,manager"` Teams []Team `jsonapi:"attr,teams"` People []*People `jsonapi:"attr,people"` FoundedAt time.Time `jsonapi:"attr,founded-at,iso8601"` diff --git a/response.go b/response.go index dea77d8..3293314 100644 --- a/response.go +++ b/response.go @@ -226,14 +226,20 @@ func visitModelNode(model interface{}, included *map[string]*Node, node := new(Node) var er error + var modelValue reflect.Value + var modelType reflect.Type value := reflect.ValueOf(model) - if value.IsNil() { - return nil, nil + if value.Type().Kind() == reflect.Pointer { + if value.IsNil() { + return nil, nil + } + modelValue = value.Elem() + modelType = value.Type().Elem() + } else { + modelValue = value + modelType = value.Type() } - modelValue := value.Elem() - modelType := value.Type().Elem() - for i := 0; i < modelValue.NumField(); i++ { fieldValue := modelValue.Field(i) structField := modelValue.Type().Field(i) @@ -395,11 +401,20 @@ func visitModelNode(model interface{}, included *map[string]*Node, continue } - strAttr, ok := fieldValue.Interface().(string) - if ok { - node.Attributes[args[1]] = strAttr + if fieldValue.Type().Kind() == reflect.Struct || (fieldValue.Type().Kind() == reflect.Pointer && fieldValue.Elem().Kind() == reflect.Struct) { + nested, err := visitModelNode(fieldValue.Interface(), nil, false) + if err != nil { + er = fmt.Errorf("failed to marshal nested attribute %q: %w", args[1], err) + break + } + node.Attributes[args[1]] = nested.Attributes } else { - node.Attributes[args[1]] = fieldValue.Interface() + strAttr, ok := fieldValue.Interface().(string) + if ok { + node.Attributes[args[1]] = strAttr + } else { + node.Attributes[args[1]] = fieldValue.Interface() + } } } } else if annotation == annotationRelation || annotation == annotationPolyRelation { diff --git a/response_test.go b/response_test.go index d79d64f..efef68a 100644 --- a/response_test.go +++ b/response_test.go @@ -682,6 +682,60 @@ func TestSupportsAttributes(t *testing.T) { } } +func TestMarshalObjectAttribute(t *testing.T) { + now := time.Now() + testModel := &Company{ + ID: "5", + Name: "test", + Boss: Employee{ + HiredAt: &now, + }, + Manager: &Employee{ + Firstname: "Dave", + HiredAt: &now, + }, + } + + out := bytes.NewBuffer(nil) + if err := MarshalPayload(out, testModel); err != nil { + t.Fatal(err) + } + + resp := new(OnePayload) + if err := json.NewDecoder(out).Decode(resp); err != nil { + t.Fatal(err) + } + + data := resp.Data + + if data.Attributes == nil { + t.Fatalf("Expected attributes") + } + + boss, ok := data.Attributes["boss"].(map[string]interface{}) + if !ok { + t.Fatalf("Expected boss attribute, got %v", data.Attributes) + } + + hiredAt, ok := boss["hired-at"] + if !ok { + t.Fatalf("Expected boss attribute to contain a \"hired-at\" property, got %v", boss) + } + + if hiredAt != now.UTC().Format(iso8601TimeFormat) { + t.Fatalf("Expected hired-at to be %s, got %s", now.UTC().Format(iso8601TimeFormat), hiredAt) + } + + manager, ok := data.Attributes["manager"].(map[string]interface{}) + if !ok { + t.Fatalf("Expected manager attribute, got %v", data.Attributes) + } + + if manager["firstname"] != "Dave" { + t.Fatalf("Expected manager.firstname to be \"Dave\", got %v", manager) + } +} + func TestOmitsZeroTimes(t *testing.T) { testModel := &Blog{ ID: 5, From e03a6d4bac106558ad7b752d80def01b7dcbbc7b Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Mon, 13 Jan 2025 12:54:04 -0700 Subject: [PATCH 46/49] Add support to Marshal slices of nested objects --- models_test.go | 10 +++++++ response.go | 26 ++++++++++++++++-- response_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 2 deletions(-) diff --git a/models_test.go b/models_test.go index 0accce4..df3b43b 100644 --- a/models_test.go +++ b/models_test.go @@ -190,6 +190,16 @@ type Company struct { FoundedAt time.Time `jsonapi:"attr,founded-at,iso8601"` } +type CompanyOmitEmpty struct { + ID string `jsonapi:"primary,companies"` + Name string `jsonapi:"attr,name,omitempty"` + Boss Employee `jsonapi:"attr,boss,omitempty"` + Manager *Employee `jsonapi:"attr,manager,omitempty"` + Teams []Team `jsonapi:"attr,teams,omitempty"` + People []*People `jsonapi:"attr,people,omitempty"` + FoundedAt time.Time `jsonapi:"attr,founded-at,iso8601,omitempty"` +} + type People struct { Name string `jsonapi:"attr,name"` Age int `jsonapi:"attr,age"` diff --git a/response.go b/response.go index 3293314..836ce39 100644 --- a/response.go +++ b/response.go @@ -401,7 +401,28 @@ func visitModelNode(model interface{}, included *map[string]*Node, continue } - if fieldValue.Type().Kind() == reflect.Struct || (fieldValue.Type().Kind() == reflect.Pointer && fieldValue.Elem().Kind() == reflect.Struct) { + isStruct := fieldValue.Type().Kind() == reflect.Struct + isPointerToStruct := fieldValue.Type().Kind() == reflect.Pointer && fieldValue.Elem().Kind() == reflect.Struct + isSliceOfStruct := fieldValue.Type().Kind() == reflect.Slice && fieldValue.Type().Elem().Kind() == reflect.Struct + isSliceOfPointerToStruct := fieldValue.Type().Kind() == reflect.Slice && fieldValue.Type().Elem().Kind() == reflect.Pointer && fieldValue.Type().Elem().Elem().Kind() == reflect.Struct + + if isSliceOfStruct || isSliceOfPointerToStruct { + if fieldValue.Len() == 0 && omitEmpty { + continue + } + // Nested slice of object attributes + manyNested, err := visitModelNodeRelationships(fieldValue, nil, false) + if err != nil { + er = fmt.Errorf("failed to marshal slice of nested attribute %q: %w", args[1], err) + break + } + nestedNodes := make([]any, len(manyNested.Data)) + for i, n := range manyNested.Data { + nestedNodes[i] = n.Attributes + } + node.Attributes[args[1]] = nestedNodes + } else if isStruct || isPointerToStruct { + // Nested object attribute nested, err := visitModelNode(fieldValue.Interface(), nil, false) if err != nil { er = fmt.Errorf("failed to marshal nested attribute %q: %w", args[1], err) @@ -409,6 +430,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, } node.Attributes[args[1]] = nested.Attributes } else { + // Primative attribute strAttr, ok := fieldValue.Interface().(string) if ok { node.Attributes[args[1]] = strAttr @@ -626,7 +648,7 @@ func visitModelNodeRelationships(models reflect.Value, included *map[string]*Nod for i := 0; i < models.Len(); i++ { model := models.Index(i) - if !model.IsValid() || model.IsNil() { + if !model.IsValid() || (model.Kind() == reflect.Pointer && model.IsNil()) { return nil, ErrUnexpectedNil } diff --git a/response_test.go b/response_test.go index efef68a..2691a1f 100644 --- a/response_test.go +++ b/response_test.go @@ -694,6 +694,14 @@ func TestMarshalObjectAttribute(t *testing.T) { Firstname: "Dave", HiredAt: &now, }, + Teams: []Team{ + {Name: "Team 1"}, + {Name: "Team-2"}, + }, + People: []*People{ + {Name: "Person-1"}, + {Name: "Person-2"}, + }, } out := bytes.NewBuffer(nil) @@ -734,6 +742,69 @@ func TestMarshalObjectAttribute(t *testing.T) { if manager["firstname"] != "Dave" { t.Fatalf("Expected manager.firstname to be \"Dave\", got %v", manager) } + + people, ok := data.Attributes["people"].([]interface{}) + if !ok { + t.Fatalf("Expected people attribute, got %v", data.Attributes) + } + if len(people) != 2 { + t.Fatalf("Expected 2 people, got %v", people) + } + + teams, ok := data.Attributes["teams"].([]interface{}) + if !ok { + t.Fatalf("Expected teams attribute, got %v", data.Attributes) + } + if len(teams) != 2 { + t.Fatalf("Expected 2 teams, got %v", teams) + } +} + +func TestMarshalObjectAttributeWithEmptyNested(t *testing.T) { + testModel := &CompanyOmitEmpty{ + ID: "5", + Name: "test", + Boss: Employee{}, + Manager: nil, + Teams: []Team{}, + People: nil, + } + + out := bytes.NewBuffer(nil) + if err := MarshalPayload(out, testModel); err != nil { + t.Fatal(err) + } + + resp := new(OnePayload) + if err := json.NewDecoder(out).Decode(resp); err != nil { + t.Fatal(err) + } + + data := resp.Data + + if data.Attributes == nil { + t.Fatalf("Expected attributes") + } + + _, ok := data.Attributes["boss"].(map[string]interface{}) + if ok { + t.Fatalf("Expected omitted boss attribute, got %v", data.Attributes) + } + + _, ok = data.Attributes["manager"].(map[string]interface{}) + if ok { + t.Fatalf("Expected omitted manager attribute, got %v", data.Attributes) + } + + _, ok = data.Attributes["people"].([]interface{}) + if ok { + t.Fatalf("Expected omitted people attribute, got %v", data.Attributes) + } + + _, ok = data.Attributes["teams"].([]interface{}) + if ok { + t.Fatalf("Expected omitted teams attribute, got %v", data.Attributes) + } } func TestOmitsZeroTimes(t *testing.T) { From 656e9ed5bb9fe2a1ea223b67089ebe629d9f4335 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Tue, 14 Jan 2025 13:14:58 -0700 Subject: [PATCH 47/49] refactor: visitModelNode --- response.go | 556 ++++++++++++++++++++++++++-------------------------- 1 file changed, 282 insertions(+), 274 deletions(-) diff --git a/response.go b/response.go index 836ce39..cc989f2 100644 --- a/response.go +++ b/response.go @@ -221,6 +221,281 @@ func selectChoiceTypeStructField(structValue reflect.Value) (reflect.Value, erro return reflect.Value{}, errors.New("no non-nil choice field was found in the specified struct") } +func visitModelNodeAttribute(args []string, node *Node, fieldValue reflect.Value) error { + var omitEmpty, iso8601, rfc3339 bool + + if len(args) > 2 { + for _, arg := range args[2:] { + switch arg { + case annotationOmitEmpty: + omitEmpty = true + case annotationISO8601: + iso8601 = true + case annotationRFC3339: + rfc3339 = true + } + } + } + + if node.Attributes == nil { + node.Attributes = make(map[string]interface{}) + } + + // Handle Nullable[T] + if strings.HasPrefix(fieldValue.Type().Name(), "NullableAttr[") { + // handle unspecified + if fieldValue.IsNil() { + return nil + } + + // handle null + if fieldValue.MapIndex(reflect.ValueOf(false)).IsValid() { + node.Attributes[args[1]] = json.RawMessage("null") + return nil + } else { + + // handle value + fieldValue = fieldValue.MapIndex(reflect.ValueOf(true)) + } + } + + if fieldValue.Type() == reflect.TypeOf(time.Time{}) { + t := fieldValue.Interface().(time.Time) + + if t.IsZero() { + return nil + } + + if iso8601 { + node.Attributes[args[1]] = t.UTC().Format(iso8601TimeFormat) + } else if rfc3339 { + node.Attributes[args[1]] = t.UTC().Format(time.RFC3339) + } else { + node.Attributes[args[1]] = t.Unix() + } + } else if fieldValue.Type() == reflect.TypeOf(new(time.Time)) { + // A time pointer may be nil + if fieldValue.IsNil() { + if omitEmpty { + return nil + } + + node.Attributes[args[1]] = nil + } else { + tm := fieldValue.Interface().(*time.Time) + + if tm.IsZero() && omitEmpty { + return nil + } + + if iso8601 { + node.Attributes[args[1]] = tm.UTC().Format(iso8601TimeFormat) + } else if rfc3339 { + node.Attributes[args[1]] = tm.UTC().Format(time.RFC3339) + } else { + node.Attributes[args[1]] = tm.Unix() + } + } + } else { + // Dealing with a fieldValue that is not a time + emptyValue := reflect.Zero(fieldValue.Type()) + + // See if we need to omit this field + if omitEmpty && reflect.DeepEqual(fieldValue.Interface(), emptyValue.Interface()) { + return nil + } + + isStruct := fieldValue.Type().Kind() == reflect.Struct + isPointerToStruct := fieldValue.Type().Kind() == reflect.Pointer && fieldValue.Elem().Kind() == reflect.Struct + isSliceOfStruct := fieldValue.Type().Kind() == reflect.Slice && fieldValue.Type().Elem().Kind() == reflect.Struct + isSliceOfPointerToStruct := fieldValue.Type().Kind() == reflect.Slice && fieldValue.Type().Elem().Kind() == reflect.Pointer && fieldValue.Type().Elem().Elem().Kind() == reflect.Struct + + if isSliceOfStruct || isSliceOfPointerToStruct { + if fieldValue.Len() == 0 && omitEmpty { + return nil + } + // Nested slice of object attributes + manyNested, err := visitModelNodeRelationships(fieldValue, nil, false) + if err != nil { + return fmt.Errorf("failed to marshal slice of nested attribute %q: %w", args[1], err) + } + nestedNodes := make([]any, len(manyNested.Data)) + for i, n := range manyNested.Data { + nestedNodes[i] = n.Attributes + } + node.Attributes[args[1]] = nestedNodes + } else if isStruct || isPointerToStruct { + // Nested object attribute + nested, err := visitModelNode(fieldValue.Interface(), nil, false) + if err != nil { + return fmt.Errorf("failed to marshal nested attribute %q: %w", args[1], err) + } + node.Attributes[args[1]] = nested.Attributes + } else { + // Primitive attribute + strAttr, ok := fieldValue.Interface().(string) + if ok { + node.Attributes[args[1]] = strAttr + } else { + node.Attributes[args[1]] = fieldValue.Interface() + } + } + } + + return nil +} + +func visitModelNodeRelation(model any, annotation string, args []string, node *Node, fieldValue reflect.Value, included *map[string]*Node, sideload bool) error { + var omitEmpty bool + + //add support for 'omitempty' struct tag for marshaling as absent + if len(args) > 2 { + omitEmpty = args[2] == annotationOmitEmpty + } + + isSlice := fieldValue.Type().Kind() == reflect.Slice + if omitEmpty && + (isSlice && fieldValue.Len() < 1 || + (!isSlice && fieldValue.IsNil())) { + return nil + } + + 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 { + return ErrUnexpectedType + } + + if choiceValue.IsNil() { + fieldValue = reflect.ValueOf(nil) + } + structValue := choiceValue.Elem() + + // Short circuit if field is omitted from model + if !structValue.IsValid() { + return nil + } + + 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 { + return ErrUnexpectedType + } + + if itemValue.IsNil() { + return ErrUnexpectedNil + } + + structValue := itemValue.Elem() + + if found, err := selectChoiceTypeStructField(structValue); err == nil { + collection = append(collection, found.Interface()) + } + } + + fieldValue = reflect.ValueOf(collection) + } + } + + if node.Relationships == nil { + node.Relationships = make(map[string]interface{}) + } + + 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 { + return err + } + 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} + return nil + } + + relationship, err := visitModelNode( + fieldValue.Interface(), + included, + sideload, + ) + + if err != nil { + return err + } + + 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, + } + } + } + return nil +} + func visitModelNode(model interface{}, included *map[string]*Node, sideload bool) (*Node, error) { node := new(Node) @@ -229,6 +504,7 @@ func visitModelNode(model interface{}, included *map[string]*Node, var modelValue reflect.Value var modelType reflect.Type value := reflect.ValueOf(model) + if value.Type().Kind() == reflect.Pointer { if value.IsNil() { return nil, nil @@ -318,282 +594,14 @@ func visitModelNode(model interface{}, included *map[string]*Node, node.ClientID = clientID } } else if annotation == annotationAttribute { - var omitEmpty, iso8601, rfc3339 bool - - if len(args) > 2 { - for _, arg := range args[2:] { - switch arg { - case annotationOmitEmpty: - omitEmpty = true - case annotationISO8601: - iso8601 = true - case annotationRFC3339: - rfc3339 = true - } - } - } - - if node.Attributes == nil { - node.Attributes = make(map[string]interface{}) - } - - // Handle Nullable[T] - if strings.HasPrefix(fieldValue.Type().Name(), "NullableAttr[") { - // handle unspecified - if fieldValue.IsNil() { - continue - } - - // handle null - if fieldValue.MapIndex(reflect.ValueOf(false)).IsValid() { - node.Attributes[args[1]] = json.RawMessage("null") - continue - } else { - - // handle value - fieldValue = fieldValue.MapIndex(reflect.ValueOf(true)) - } - } - - if fieldValue.Type() == reflect.TypeOf(time.Time{}) { - t := fieldValue.Interface().(time.Time) - - if t.IsZero() { - continue - } - - if iso8601 { - node.Attributes[args[1]] = t.UTC().Format(iso8601TimeFormat) - } else if rfc3339 { - node.Attributes[args[1]] = t.UTC().Format(time.RFC3339) - } else { - node.Attributes[args[1]] = t.Unix() - } - } else if fieldValue.Type() == reflect.TypeOf(new(time.Time)) { - // A time pointer may be nil - if fieldValue.IsNil() { - if omitEmpty { - continue - } - - node.Attributes[args[1]] = nil - } else { - tm := fieldValue.Interface().(*time.Time) - - if tm.IsZero() && omitEmpty { - continue - } - - if iso8601 { - node.Attributes[args[1]] = tm.UTC().Format(iso8601TimeFormat) - } else if rfc3339 { - node.Attributes[args[1]] = tm.UTC().Format(time.RFC3339) - } else { - node.Attributes[args[1]] = tm.Unix() - } - } - } else { - // Dealing with a fieldValue that is not a time - emptyValue := reflect.Zero(fieldValue.Type()) - - // See if we need to omit this field - if omitEmpty && reflect.DeepEqual(fieldValue.Interface(), emptyValue.Interface()) { - continue - } - - isStruct := fieldValue.Type().Kind() == reflect.Struct - isPointerToStruct := fieldValue.Type().Kind() == reflect.Pointer && fieldValue.Elem().Kind() == reflect.Struct - isSliceOfStruct := fieldValue.Type().Kind() == reflect.Slice && fieldValue.Type().Elem().Kind() == reflect.Struct - isSliceOfPointerToStruct := fieldValue.Type().Kind() == reflect.Slice && fieldValue.Type().Elem().Kind() == reflect.Pointer && fieldValue.Type().Elem().Elem().Kind() == reflect.Struct - - if isSliceOfStruct || isSliceOfPointerToStruct { - if fieldValue.Len() == 0 && omitEmpty { - continue - } - // Nested slice of object attributes - manyNested, err := visitModelNodeRelationships(fieldValue, nil, false) - if err != nil { - er = fmt.Errorf("failed to marshal slice of nested attribute %q: %w", args[1], err) - break - } - nestedNodes := make([]any, len(manyNested.Data)) - for i, n := range manyNested.Data { - nestedNodes[i] = n.Attributes - } - node.Attributes[args[1]] = nestedNodes - } else if isStruct || isPointerToStruct { - // Nested object attribute - nested, err := visitModelNode(fieldValue.Interface(), nil, false) - if err != nil { - er = fmt.Errorf("failed to marshal nested attribute %q: %w", args[1], err) - break - } - node.Attributes[args[1]] = nested.Attributes - } else { - // Primative attribute - strAttr, ok := fieldValue.Interface().(string) - if ok { - node.Attributes[args[1]] = strAttr - } else { - node.Attributes[args[1]] = fieldValue.Interface() - } - } + er = visitModelNodeAttribute(args, node, fieldValue) + if er != nil { + 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 - } - - 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) - } - } - - if node.Relationships == nil { - node.Relationships = make(map[string]interface{}) - } - - 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. From abbc3c7a91984572165cbcda8f47ba9ff397dded Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Wed, 22 Jan 2025 08:57:55 -0700 Subject: [PATCH 48/49] Update CODEOWNERS --- .github/CODEOWNERS | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 3da71e0..bf95d33 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,7 +1,8 @@ # Each line is a file pattern followed by one or more owners. # More on CODEOWNERS files: https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners -# Default owner +# Default owners +* @hashicorp/tf-core-cloud * @hashicorp/team-ip-compliance # Add override rules below. Each line is a file/folder pattern followed by one or more owners. @@ -10,4 +11,4 @@ # Examples: # /docs/ @docs-team # *.js @js-team -# *.go @go-team \ No newline at end of file +# *.go @go-team From a19fc77c39d20301c4c0ed90764a8a7878de32d0 Mon Sep 17 00:00:00 2001 From: Netra Mali Date: Fri, 24 Jan 2025 15:28:22 -0500 Subject: [PATCH 49/49] 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 df3b43b..28b56e6 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 27f628e..9864e78 100644 --- a/request.go +++ b/request.go @@ -483,10 +483,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 + 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 @@ -495,6 +515,12 @@ 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 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 } @@ -505,9 +531,6 @@ 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 { @@ -515,7 +538,12 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) 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..7fa0d3e 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 cc989f2..c749aeb 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]) 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)