Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject nonunique resources #59

Merged
merged 5 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
Expand Down
67 changes: 48 additions & 19 deletions jsonapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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)
}
}
Expand All @@ -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]struct{})
DQSevilla marked this conversation as resolved.
Show resolved Hide resolved

for _, ro := range append(d.getResourceObjectSlice(), d.Included...) {
rid := ro.getIdentifier()
if _, ok := topLevelSeen[rid]; ro.ID != "" && ok {
return false
}
topLevelSeen[rid] = struct{}{}

relSeen := make(map[string]struct{})
for _, rel := range ro.Relationships {
for _, relRo := range rel.getResourceObjectSlice() {
relRid := relRo.getIdentifier()
if _, ok := relSeen[relRid]; relRo.ID != "" && ok {
return false
}
relSeen[relRid] = struct{}{}
}
}
}

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
Expand Down
22 changes: 13 additions & 9 deletions jsonapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"}}}`
Expand All @@ -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"}}}}}`
Expand Down
3 changes: 3 additions & 0 deletions marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 12 additions & 1 deletion marshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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%"},
}, {
Expand Down
5 changes: 4 additions & 1 deletion unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"}}
}
Expand Down
22 changes: 21 additions & 1 deletion unmarshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}`,
Expand Down Expand Up @@ -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
Expand Down
Loading