Reject nonunique resources (#59)

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")
 )

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

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"}}}}}`

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

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%"},
 		}, {

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"}}
 	}

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