diff --git a/errors.go b/errors.go index 90fa714..197ca0c 100644 --- a/errors.go +++ b/errors.go @@ -37,6 +37,10 @@ var ( // ErrRelationshipMissingRequiredMembers indicates that a relationship does not have at least one required member ErrRelationshipMissingRequiredMembers = errors.New("relationship is missing required top-level members; must have one of: \"data\", \"meta\", \"links\"") + // ErrNonuniqueResource indicates that multiple resource objects across the primary data and included sections share + // the same type & id, or multiple resource linkages with the same type & id exist in a relationship section + ErrNonuniqueResource = errors.New("\"type\" and \"id\" must be unique across resources") + // ErrErrorUnmarshalingNotImplemented indicates that an attempt was made to unmarshal an error document ErrErrorUnmarshalingNotImplemented = errors.New("error unmarshaling is not implemented") ) diff --git a/jsonapi.go b/jsonapi.go index 9dac84d..83e1cab 100644 --- a/jsonapi.go +++ b/jsonapi.go @@ -44,6 +44,10 @@ func (ro *resourceObject) UnmarshalJSON(data []byte) error { return nil } +func (ro *resourceObject) getIdentifier() string { + return fmt.Sprintf("{Type: %v, ID: %v}", ro.Type, ro.ID) +} + // JSONAPI is a JSON:API object as defined by https://jsonapi.org/format/1.0/#document-jsonapi-object. type jsonAPI struct { Version string `json:"version"` @@ -235,6 +239,16 @@ func (d *document) isEmpty() bool { return len(d.DataMany) == 0 && d.DataOne == nil } +func (d *document) getResourceObjectSlice() []*resourceObject { + if d.hasMany { + return d.DataMany + } + if d.DataOne == nil { + return nil + } + return []*resourceObject{d.DataOne} +} + // verifyFullLinkage returns an error if the given compound document is not fully-linked as // described by https://jsonapi.org/format/1.1/#document-compound-documents. That is, there must be // a chain of relationships linking all included data to primary data transitively. @@ -243,20 +257,6 @@ func (d *document) verifyFullLinkage(aliasRelationships bool) error { return nil } - getResourceObjectSlice := func(d *document) []*resourceObject { - if d.hasMany { - return d.DataMany - } - if d.DataOne == nil { - return nil - } - return []*resourceObject{d.DataOne} - } - - resourceIdentifier := func(ro *resourceObject) string { - return fmt.Sprintf("{Type: %v, ID: %v}", ro.Type, ro.ID) - } - // a list of related resource identifiers, and a flag to mark nodes as visited type includeNode struct { included *resourceObject @@ -270,16 +270,16 @@ func (d *document) verifyFullLinkage(aliasRelationships bool) error { relatedTo := make([]*resourceObject, 0) for _, relationship := range included.Relationships { - relatedTo = append(relatedTo, getResourceObjectSlice(relationship)...) + relatedTo = append(relatedTo, relationship.getResourceObjectSlice()...) } - includeGraph[resourceIdentifier(included)] = &includeNode{included, relatedTo, false} + includeGraph[included.getIdentifier()] = &includeNode{included, relatedTo, false} } // helper to traverse the graph from a given key and mark nodes as visited var visit func(ro *resourceObject) visit = func(ro *resourceObject) { - node, ok := includeGraph[resourceIdentifier(ro)] + node, ok := includeGraph[ro.getIdentifier()] if !ok { return } @@ -299,10 +299,10 @@ func (d *document) verifyFullLinkage(aliasRelationships bool) error { } // visit all include nodes that are accessible from the primary data - primaryData := getResourceObjectSlice(d) + primaryData := d.getResourceObjectSlice() for _, data := range primaryData { for _, relationship := range data.Relationships { - for _, ro := range getResourceObjectSlice(relationship) { + for _, ro := range relationship.getResourceObjectSlice() { visit(ro) } } @@ -322,6 +322,35 @@ func (d *document) verifyFullLinkage(aliasRelationships bool) error { return nil } +// verifyResourceUniqueness checks if the given document contains unique resources as described +// by https://jsonapi.org/format/1.1/#document-resource-object-identification. Resource objects +// across primary data and included must be unique, and resource linkages must be unique in +// any given relationship section. +func (d *document) verifyResourceUniqueness() bool { + topLevelSeen := make(map[string]bool) + + for _, ro := range append(d.getResourceObjectSlice(), d.Included...) { + rid := ro.getIdentifier() + if ro.ID != "" && topLevelSeen[rid] { + return false + } + topLevelSeen[rid] = true + + relSeen := make(map[string]bool) + for _, rel := range ro.Relationships { + for _, relRo := range rel.getResourceObjectSlice() { + relRid := relRo.getIdentifier() + if relRo.ID != "" && relSeen[relRid] { + return false + } + relSeen[relRid] = true + } + } + } + + return true +} + // Linkable can be implemented to marshal resource object links as defined by https://jsonapi.org/format/1.0/#document-resource-object-links. type Linkable interface { Link() *Link diff --git a/jsonapi_test.go b/jsonapi_test.go index bdade8c..f9bcd24 100644 --- a/jsonapi_test.go +++ b/jsonapi_test.go @@ -31,6 +31,7 @@ var ( articleANoID = Article{Title: "A"} articleB = Article{ID: "2", Title: "B"} articlesAB = []Article{articleA, articleB} + articlesAA = []Article{articleA, articleA} articlesABPtr = []*Article{&articleA, &articleB} articleComplete = ArticleComplete{ ID: "1", @@ -77,15 +78,16 @@ var ( articleWithoutResourceObjectMeta = ArticleWithResourceObjectMeta{ID: "1", Title: "A"} // articles with relationships - articleRelated = ArticleRelated{ID: "1", Title: "A"} - articleRelatedNoOmitEmpty = ArticleRelatedNoOmitEmpty{ID: "1", Title: "A"} - articleRelatedAuthor = ArticleRelated{ID: "1", Title: "A", Author: &authorA} - articleRelatedAuthorWithMeta = ArticleRelated{ID: "1", Title: "A", Author: &authorAWithMeta} - articleRelatedComments = ArticleRelated{ID: "1", Title: "A", Comments: []*Comment{&commentA}} - articleRelatedCommentsArchived = ArticleRelated{ID: "1", Title: "A", Comments: []*Comment{&commentArchived}} - articleRelatedCommentsNested = ArticleRelated{ID: "1", Title: "A", Comments: []*Comment{&commentAWithAuthor}} - articleRelatedComplete = ArticleRelated{ID: "1", Title: "A", Author: &authorAWithMeta, Comments: commentsAB} - articlesRelatedComplex = []*ArticleRelated{ + articleRelated = ArticleRelated{ID: "1", Title: "A"} + articleRelatedNoOmitEmpty = ArticleRelatedNoOmitEmpty{ID: "1", Title: "A"} + articleRelatedAuthor = ArticleRelated{ID: "1", Title: "A", Author: &authorA} + articleRelatedAuthorWithMeta = ArticleRelated{ID: "1", Title: "A", Author: &authorAWithMeta} + articleRelatedComments = ArticleRelated{ID: "1", Title: "A", Comments: []*Comment{&commentA}} + articleRelatedNonuniqueComments = ArticleRelated{ID: "1", Title: "A", Comments: []*Comment{&commentA, &commentA}} + articleRelatedCommentsArchived = ArticleRelated{ID: "1", Title: "A", Comments: []*Comment{&commentArchived}} + articleRelatedCommentsNested = ArticleRelated{ID: "1", Title: "A", Comments: []*Comment{&commentAWithAuthor}} + articleRelatedComplete = ArticleRelated{ID: "1", Title: "A", Author: &authorAWithMeta, Comments: commentsAB} + articlesRelatedComplex = []*ArticleRelated{ { ID: "1", Title: "Bazel 101", @@ -153,6 +155,7 @@ var ( 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"}}]}` + articlesABNonuniqueData = `{"data":[{"type":"articles","id":"1","attributes":{"title":"A"}},{"type":"articles","id":"1","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"}}}` @@ -178,6 +181,7 @@ var ( articleRelatedCommentsWithIncludeBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"relationships":{"comments":{"data":[{"id":"1","type":"comments"}],"links":{"self":"http://example.com/articles/1/relationships/comments","related":"http://example.com/articles/1/comments"}}}},"included":[{"id":"1","type":"comments","attributes":{"body":"A"},"relationships":{"author":{"data":{"id":"1","type":"author"},"links":{"self":"http://example.com/comments/1/relationships/author","related":"http://example.com/comments/1/author"}}}}]}` articleRelatedCompleteBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"relationships":{"author":{"data":{"id":"1","type":"author"},"meta":{"count":10},"links":{"self":"http://example.com/articles/1/relationships/author","related":"http://example.com/articles/1/author"}},"comments":{"data":[{"id":"1","type":"comments"},{"id":"2","type":"comments"}],"links":{"self":"http://example.com/articles/1/relationships/comments","related":"http://example.com/articles/1/comments"}}}}}` articleRelatedCompleteWithIncludeBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"relationships":{"author":{"data":{"id":"1","type":"author"}},"comments":{"data":[{"id":"1","type":"comments"},{"id":"2","type":"comments"}]}}},"included":[{"id":"1","type":"author","attributes":{"name":"A"}},{"id":"1","type":"comments","attributes":{"body":"A"}},{"id":"2","type":"comments","attributes":{"body":"B"}}]}` + articleRelatedNonuniqueLinkage = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"relationships":{"author":{"data":{"id":"1","type":"author"}},"comments":{"data":[{"id":"1","type":"comments"},{"id":"1","type":"comments"}]}}}}` articleRelatedCommentsNestedWithIncludeBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"relationships":{"comments":{"data":[{"id":"1","type":"comments"}],"links":{"self":"http://example.com/articles/1/relationships/comments","related":"http://example.com/articles/1/comments"}}}},"included":[{"id":"1","type":"comments","attributes":{"body":"A"},"relationships":{"author":{"data":{"id":"1","type":"author"},"links":{"self":"http://example.com/comments/1/relationships/author","related":"http://example.com/comments/1/author"}}}},{"id":"1","type":"author","attributes":{"name":"A"}}]}` articleWithIncludeOnlyBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"}},"included":[{"id":"1","type":"author","attributes":{"name":"A"}}]}` articleRelatedAuthorLinksOnlyBody = `{"data":{"id":"1","type":"articles","attributes":{"title":"A"},"relationships":{"author":{"links":{"self":"http://example.com/articles/1/relationships/author","related":"http://example.com/articles/1/author"}}}}}` diff --git a/marshal.go b/marshal.go index 167441e..48525f9 100644 --- a/marshal.go +++ b/marshal.go @@ -205,6 +205,9 @@ func makeDocument(v any, m *Marshaler, isRelationship bool) (*document, error) { d.Included = append(d.Included, ro) } + if ok := d.verifyResourceUniqueness(); !ok { + return nil, ErrNonuniqueResource + } // if we got any included data, verify full-linkage of this compound document. if err := d.verifyFullLinkage(false); err != nil { return nil, err diff --git a/marshal_test.go b/marshal_test.go index 2a2a86b..ff1d248 100644 --- a/marshal_test.go +++ b/marshal_test.go @@ -81,6 +81,11 @@ func TestMarshal(t *testing.T) { given: &articlesAB, expect: articlesABBody, expectError: nil, + }, { + description: "[]Article (nonunique data)", + given: articlesAA, + expect: "", + expectError: ErrNonuniqueResource, }, { description: "[]*Article", given: articlesABPtr, @@ -584,6 +589,12 @@ func TestMarshalRelationships(t *testing.T) { marshalOptions: nil, expect: articleRelatedCommentsBody, expectError: nil, + }, { + description: "with related nonunique comments", + given: &articleRelatedNonuniqueComments, + marshalOptions: nil, + expect: "", + expectError: ErrNonuniqueResource, }, { description: "with related author and comments", given: &articleRelatedComplete, @@ -752,7 +763,7 @@ func TestMarshalMemberNameValidation(t *testing.T) { }, { description: "Articles with one invalid resource meta member name", given: []*ArticleWithGenericMeta{ - {ID: "1"}, {ID: "1", Meta: map[string]any{"foo%": 1}}, + {ID: "1"}, {ID: "2", Meta: map[string]any{"foo%": 1}}, }, expectError: &MemberNameValidationError{"foo%"}, }, { diff --git a/unmarshal.go b/unmarshal.go index c124b84..722b09a 100644 --- a/unmarshal.go +++ b/unmarshal.go @@ -88,6 +88,9 @@ func Unmarshal(data []byte, v any, opts ...UnmarshalOption) (err error) { } func (d *document) unmarshal(v any, m *Unmarshaler) (err error) { + if ok := d.verifyResourceUniqueness(); !ok { + return ErrNonuniqueResource + } // verify full-linkage in-case this is a compound document if err = d.verifyFullLinkage(true); err != nil { return @@ -142,7 +145,7 @@ func unmarshalResourceObjects(ros []*resourceObject, v any, m *Unmarshaler) erro outType := derefType(reflect.TypeOf(v)) outValue := derefValue(reflect.ValueOf(v)) - // first, it must be a struct since we'll be parsing the jsonapi struct tags + // first, it must be a slice since we'll be parsing multiple resource objects if outType.Kind() != reflect.Slice { return &TypeError{Actual: outType.String(), Expected: []string{"slice"}} } diff --git a/unmarshal_test.go b/unmarshal_test.go index 7b8537c..6c29eaf 100644 --- a/unmarshal_test.go +++ b/unmarshal_test.go @@ -69,6 +69,16 @@ func TestUnmarshal(t *testing.T) { }, expect: []Article{articleA, articleB}, expectError: nil, + }, { + description: "[]Article (nonunique data)", + given: articlesABNonuniqueData, + do: func(body []byte) (any, error) { + var a []Article + err := Unmarshal(body, &a) + return a, err + }, + expect: ([]Article)(nil), + expectError: ErrNonuniqueResource, }, { description: "[]Article (empty)", given: emptyManyBody, @@ -415,6 +425,16 @@ func TestUnmarshal(t *testing.T) { }, expect: &articleRelatedCommentsNested, expectError: nil, + }, { + description: "ArticleRelated (nonunique linkage)", + given: articleRelatedNonuniqueLinkage, + do: func(body []byte) (any, error) { + var a ArticleRelated + err := Unmarshal(body, &a) + return a, err + }, + expect: ArticleRelated{}, + expectError: ErrNonuniqueResource, }, { description: "links member only", given: `{"links":null}`, @@ -712,7 +732,7 @@ func TestUnmarshalMemberNameValidation(t *testing.T) { articleWithInvalidToplevelMetaMemberNameBody := `{"data":{"id":"1","type":"articles","attributes":{"title":"A"}},"meta":{"foo%":2}}` articleWithInvalidJSONAPIMetaMemberNameBody := `{"data":{"id":"1","type":"articles","attributes":{"title":"A"}},"jsonapi":{"version":"1.0","meta":{"foo%":1}}}` articleWithInvalidRelationshipAttributeNameNotIncludedBody := `{"data":{"id":"1","type":"articles","relationships":{"author":{"data":{"id":"1","type":"author"}}}}}` - articlesWithOneInvalidResourceMetaMemberName := `{"data":[{"id":"1","type":"articles"},{"id":"1","type":"articles","meta":{"foo%":1}}]}` + articlesWithOneInvalidResourceMetaMemberName := `{"data":[{"id":"1","type":"articles"},{"id":"2","type":"articles","meta":{"foo%":1}}]}` tests := []struct { description string