Skip to content

Commit

Permalink
Support empty array unmarshaling (#27)
Browse files Browse the repository at this point in the history
- Fixes Empty array unmarshaling fails #26 with a partial json.Unmarshal step
- Adds changes from Create Marshal and Unmarshal benchmarks #32:
  - Clarify the names of some test jsonapi bodies (ones with a top-level meta object)
  - Add Marshal and Unmarshal benchmarks

The addition of a second json.Unmarshal in the (*document).UnmarshalJSON has actually reduced the time spent by Unmarshal, as we can determine whether there is more than one primary data object earlier in the process, without having to re-try the jsonapi.Unmarshal on the entire document.
  • Loading branch information
kpurdon authored May 4, 2023
1 parent 8f803cc commit 5726dcb
Show file tree
Hide file tree
Showing 6 changed files with 263 additions and 71 deletions.
15 changes: 5 additions & 10 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,13 @@ var (

// ErrMissingLinkFields indicates that a LinkObject is not valid.
ErrMissingLinkFields = errors.New("at least one of Links.Self or Links.Related must be set to a nonempty string or *LinkObject")
)

// RequestBodyError indicates that a given request body is invalid.
// TODO: should this replace ErrInvalidBody?
type RequestBodyError struct {
t any
}
// ErrMissingDataField indicates that a *jsonapi.document is missing data in an invalid way
ErrMissingDataField = errors.New("document is missing a required top-level or relationship-level data member")

// Error implements the error interface.
func (e *RequestBodyError) Error() string {
return fmt.Sprintf("body is not a json:api representation of %T", e.t)
}
// ErrEmptyDataObject indicates that a primary or relationship data member is incorrectly represented by an empty JSON object {}
ErrEmptyDataObject = errors.New("resource \"data\" members may not be represented by an empty object {}")
)

// TypeError indicates that an unexpected type was encountered.
type TypeError struct {
Expand Down
44 changes: 26 additions & 18 deletions jsonapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"reflect"
"strings"
)

// ResourceObject is a JSON:API resource object as defined by https://jsonapi.org/format/1.0/#document-resource-objects
Expand Down Expand Up @@ -161,34 +162,41 @@ func (d *document) MarshalJSON() ([]byte, error) {
}

// UnmarshalJSON implements the json.Unmarshaler interface.
func (d *document) UnmarshalJSON(data []byte) (err error) {
func (d *document) UnmarshalJSON(data []byte) error {
type alias document

// Since there is no simple regular expression to capture only that the primary data is an
// array, try unmarshaling both ways
auxMany := &struct {
Data []*resourceObject `json:"data"`
auxRaw := &struct {
Data json.RawMessage `json:"data,omitempty"`
*alias
}{
alias: (*alias)(d),
}
if err = json.Unmarshal(data, &auxMany); err == nil {
d.hasMany = true
d.DataMany = auxMany.Data
return
if err := json.Unmarshal(data, &auxRaw); err != nil {
return err
}

auxOne := &struct {
Data *resourceObject `json:"data"`
*alias
}{
alias: (*alias)(d),
}
if err = json.Unmarshal(data, &auxOne); err == nil {
d.DataOne = auxOne.Data
rawData := string(auxRaw.Data)
switch rawData {
case "":
// no "data" member
if auxRaw.Errors == nil && auxRaw.Meta == nil {
// missing required top-level fields
return ErrMissingDataField
}
return nil
case "{}":
// {"data":{}, ...} is invalid
return ErrEmptyDataObject
case "null":
// {"data":null, ...} is valid
return nil
}

return
if strings.HasPrefix(rawData, "[") {
d.hasMany = true
return json.Unmarshal(auxRaw.Data, &auxRaw.DataMany)
}
return json.Unmarshal(auxRaw.Data, &auxRaw.DataOne)
}

// isEmpty returns true if there is no primary data in the given document (i.e. null or []).
Expand Down
32 changes: 19 additions & 13 deletions jsonapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,21 +94,27 @@ var (
}

// article bodies
emptyBody = `{"data":[]}`
articleABody = `{"data":{"type":"articles","id":"1","attributes":{"title":"A"}}}`
articleANoIDBody = `{"data":{"type":"articles","attributes":{"title":"A"}}}`
articleAInvalidTypeBody = `{"data":{"type":"not-articles","id":"1","attributes":{"title":"A"}}}`
articleOmitTitleFullBody = `{"data":{"type":"articles","id":"1"}}`
articleOmitTitlePartialBody = `{"data":{"type":"articles","id":"1","attributes":{"subtitle":"A"}}}`
articlesABBody = `{"data":[{"type":"articles","id":"1","attributes":{"title":"A"}},{"type":"articles","id":"2","attributes":{"title":"B"}}]}`
articleCompleteBody = `{"data":{"id":"1","type":"articles","attributes":{"info":{"publishDate":"1989-06-15T00:00:00Z","tags":["a","b"],"isPublic":true,"metrics":{"views":10,"reads":4}},"title":"A","subtitle":"AA"}}}`
articleALinkedBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"links":{"self":"https://example.com/articles/1","related":{"href":"https://example.com/articles/1/comments","meta":{"count":10}}}}}`
articleLinkedOnlySelfBody = `{"data":{"id":"1","type":"articles","links":{"self":"https://example.com/articles/1"}}}`
articleWithResourceObjectMetaBody = `{"data":{"type":"articles","id":"1","attributes":{"title":"A"},"meta":{"count":10}}}`
articleAWithMetaBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"meta":{"views":10,"reads":4}}}`
articleEmbeddedBody = `{"data":{"type":"articles","id":"1","attributes":{"title":"A","lastModified":"1989-06-15T00:00:00Z"}}}`
nullDataBody = `{"data":null}`
emptySingleBody = `{"data":{}}`
emptyManyBody = `{"data":[]}`
articleABody = `{"data":{"type":"articles","id":"1","attributes":{"title":"A"}}}`
articleANoIDBody = `{"data":{"type":"articles","attributes":{"title":"A"}}}`
articleAInvalidTypeBody = `{"data":{"type":"not-articles","id":"1","attributes":{"title":"A"}}}`
articleOmitTitleFullBody = `{"data":{"type":"articles","id":"1"}}`
articleOmitTitlePartialBody = `{"data":{"type":"articles","id":"1","attributes":{"subtitle":"A"}}}`
articlesABBody = `{"data":[{"type":"articles","id":"1","attributes":{"title":"A"}},{"type":"articles","id":"2","attributes":{"title":"B"}}]}`
articleCompleteBody = `{"data":{"id":"1","type":"articles","attributes":{"info":{"publishDate":"1989-06-15T00:00:00Z","tags":["a","b"],"isPublic":true,"metrics":{"views":10,"reads":4}},"title":"A","subtitle":"AA"}}}`
articleALinkedBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"links":{"self":"https://example.com/articles/1","related":{"href":"https://example.com/articles/1/comments","meta":{"count":10}}}}}`
articleLinkedOnlySelfBody = `{"data":{"id":"1","type":"articles","links":{"self":"https://example.com/articles/1"}}}`
articleWithResourceObjectMetaBody = `{"data":{"type":"articles","id":"1","attributes":{"title":"A"},"meta":{"count":10}}}`
articleAToplevelMetaBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"}},"meta":{"foo":"bar"}}`
articleAWithMetaBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"meta":{"views":10,"reads":4}}}`
articleNullWithToplevelMetaBody = `{"data":null,"meta":{"foo":"bar"}}`
articleEmptyArrayWithToplevelMetaBody = `{"data":[],"meta":{"foo":"bar"}}`
articleEmbeddedBody = `{"data":{"type":"articles","id":"1","attributes":{"title":"A","lastModified":"1989-06-15T00:00:00Z"}}}`

// articles with relationships bodies
articleRelatedInvalidEmptyRelationshipBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"relationships":{"author":{}}}}`
articleRelatedNoOmitEmptyBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"relationships":{"author":{"data":null},"comments":{"data":[]}}}}`
articleRelatedAuthorBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"relationships":{"author":{"data":{"id":"1","type":"author"},"links":{"self":"http://example.com/articles/1/relationships/author","related":"http://example.com/articles/1/author"}}}}}`
articleRelatedAuthorTwiceBody = `{"data":[{"id":"1","type":"articles","attributes":{"title":"A"},"relationships":{"author":{"data":{"id":"1","type":"author"}}}},{"id":"2","type":"articles","attributes":{"title":"B"},"relationships":{"author":{"data":{"id":"1","type":"author"}}}}]}`
Expand Down
67 changes: 53 additions & 14 deletions marshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ func TestMarshal(t *testing.T) {
{
description: "nil",
given: nil,
expect: `{"data":null}`,
expect: nullDataBody,
expectError: nil,
}, {
description: "Article (empty)",
given: Article{},
expect: `{"data":null}`,
expect: nullDataBody,
expectError: nil,
}, {
description: "*Article (empty)",
Expand All @@ -39,12 +39,12 @@ func TestMarshal(t *testing.T) {
}, {
description: "[]*Article (nil)",
given: []*Article(nil),
expect: emptyBody,
expect: emptyManyBody,
expectError: nil,
}, {
description: "[]*Article (empty)",
given: make([]*Article, 0),
expect: emptyBody,
expect: emptyManyBody,
expectError: nil,
}, {
description: "Article (missing ID)",
Expand Down Expand Up @@ -299,10 +299,7 @@ func TestMarshal(t *testing.T) {
func TestMarshalMeta(t *testing.T) {
t.Parallel()

articleAMetaBody := `{"data":{"id":"1","type":"articles","attributes":{"title":"A"}},"meta":{"foo":"bar"}}`
articleAMetaNullBody := `{"data": null,"meta":{"foo":"bar"}}`
articleAMetaEmptyBody := `{"data":[],"meta":{"foo":"bar"}}`
errorsObjectMetaBody := `{"meta":{"foo":"bar"},"errors":[{"title":"T"}]}`
errorsObjectToplevelMetaBody := `{"meta":{"foo":"bar"},"errors":[{"title":"T"}]}`

tests := []struct {
description string
Expand All @@ -315,15 +312,15 @@ func TestMarshalMeta(t *testing.T) {
description: "map[string]any",
given: &articleA,
givenMeta: map[string]any{"foo": "bar"},
expect: articleAMetaBody,
expect: articleAToplevelMetaBody,
expectError: nil,
}, {
description: "struct",
given: &articleA,
givenMeta: &struct {
Foo string `json:"foo"`
}{Foo: "bar"},
expect: articleAMetaBody,
expect: articleAToplevelMetaBody,
expectError: nil,
}, {
description: "non-object type",
Expand All @@ -335,25 +332,25 @@ func TestMarshalMeta(t *testing.T) {
description: "map[string]any errors object",
given: errorsSimpleSliceSinglePtr,
givenMeta: map[string]any{"foo": "bar"},
expect: errorsObjectMetaBody,
expect: errorsObjectToplevelMetaBody,
expectError: nil,
}, {
description: "map[string]any with nil body",
given: nil,
givenMeta: map[string]any{"foo": "bar"},
expect: articleAMetaNullBody,
expect: articleNullWithToplevelMetaBody,
expectError: nil,
}, {
description: "map[string]any with Article (empty)",
given: Article{},
givenMeta: map[string]any{"foo": "bar"},
expect: articleAMetaNullBody,
expect: articleNullWithToplevelMetaBody,
expectError: nil,
}, {
description: "map[string]any with body []*Article (empty)",
given: []*Article(nil),
givenMeta: map[string]any{"foo": "bar"},
expect: articleAMetaEmptyBody,
expect: articleEmptyArrayWithToplevelMetaBody,
expectError: nil,
},
}
Expand Down Expand Up @@ -767,3 +764,45 @@ func TestMarshalMemberNameValidation(t *testing.T) {
})
}
}

func BenchmarkMarshal(b *testing.B) {
benchmarks := []struct {
name string
given any
opts []MarshalOption
}{
{
name: "ArticleSimple",
given: articleA,
opts: nil,
}, {
name: "ArticleSimpleWithToplevelMeta",
given: articleA,
opts: []MarshalOption{
MarshalMeta(map[string]any{"foo": "bar"}),
},
}, {
name: "ArticleComplex",
given: articleRelatedComments,
opts: []MarshalOption{
MarshalInclude(&commentAWithAuthor, &authorA),
},
}, {
name: "ArticleComplexDisableNameValidation",
given: articleRelatedComments,
opts: []MarshalOption{
MarshalInclude(&commentAWithAuthor, &authorA),
MarshalDisableNameValidation(),
},
},
}

for _, bm := range benchmarks {
bm := bm
b.Run(bm.name, func(b *testing.B) {
for n := 0; n < b.N; n++ {
_, _ = Marshal(bm.given, bm.opts...)
}
})
}
}
14 changes: 7 additions & 7 deletions unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,6 @@ func Unmarshal(data []byte, v any, opts ...UnmarshalOption) (err error) {
}

func (d *document) unmarshal(v any, m *Unmarshaler) (err error) {
// this means we couldn't decode anything (e.g. {}, [], ...)
if d.isEmpty() {
err = &RequestBodyError{t: v}
return
}

// verify full-linkage in-case this is a compound document
if err = d.verifyFullLinkage(true); err != nil {
return
Expand All @@ -108,7 +102,7 @@ func (d *document) unmarshal(v any, m *Unmarshaler) (err error) {
if err != nil {
return
}
} else {
} else if d.DataOne != nil {
err = d.DataOne.unmarshal(v, m)
if err != nil {
return
Expand Down Expand Up @@ -150,6 +144,12 @@ func unmarshalResourceObjects(ros []*resourceObject, v any, m *Unmarshaler) erro
return &TypeError{Actual: outType.String(), Expected: []string{"slice"}}
}

// allocate an empty slice of the outType if there are no resource objects to unmarshal,
// because the main loop cannot construct run and construct one.
if len(ros) == 0 {
outValue = reflect.MakeSlice(outType, 0, 0)
}

for _, ro := range ros {
// unmarshal the resource object into an empty value of the slices element type
outElem := reflect.New(derefType(outType.Elem())).Interface()
Expand Down
Loading

0 comments on commit 5726dcb

Please sign in to comment.