Merge pull request #86 from mattpolzin/clearer-include-failure-errors

Clearer include failure errors

Author: mattpolzin

Sources/JSONAPI/Document/Includes.swift

@@ -76,14 +76,15 @@ extension Includes: Decodable where I: Decodable {
                         }
                 }
                 guard errors.count == error.individualTypeFailures.count else {
-                    throw IncludesDecodingError(error: error, idx: idx)
+                    throw IncludesDecodingError(error: error, idx: idx, totalIncludesCount: container.count ?? 0)
                 }
                 throw IncludesDecodingError(
                     error: IncludeDecodingError(failures: errors),
-                    idx: idx
+                    idx: idx,
+                    totalIncludesCount: container.count ?? 0
                 )
             } catch let error {
-                throw IncludesDecodingError(error: error, idx: idx)
+                throw IncludesDecodingError(error: error, idx: idx, totalIncludesCount: container.count ?? 0)
             }
         }
 
@@ -208,7 +209,13 @@ extension Includes where I: _Poly11 {
 // MARK: - DecodingError
 public struct IncludesDecodingError: Swift.Error, Equatable {
     public let error: Swift.Error
+    /// The zero-based index of the include that failed to decode.
     public let idx: Int
+    /// The total count of includes in the document that failed to decode.
+    ///
+    /// In other words, "of `totalIncludesCount` includes, the `(idx + 1)`th
+    /// include failed to decode.
+    public let totalIncludesCount: Int
 
     public static func ==(lhs: Self, rhs: Self) -> Bool {
         return lhs.idx == rhs.idx
@@ -218,18 +225,55 @@ public struct IncludesDecodingError: Swift.Error, Equatable {
 
 extension IncludesDecodingError: CustomStringConvertible {
     public var description: String {
-        return "Include \(idx + 1) failed to parse: \(error)"
+        let ordinalSuffix: String
+        if (idx % 100) + 1 > 9 && (idx % 100) + 1 < 20 {
+            // the teens
+            ordinalSuffix = "th"
+        } else {
+            switch ((idx % 10) + 1) {
+            case 1:
+                ordinalSuffix = "st"
+            case 2:
+                ordinalSuffix = "nd"
+            case 3:
+                ordinalSuffix = "rd"
+            default:
+                ordinalSuffix = "th"
+            }
+        }
+        let ordinalDescription = "\(idx + 1)\(ordinalSuffix)"
+
+        return "Out of the \(totalIncludesCount) includes in the document, the \(ordinalDescription) one failed to parse: \(error)"
     }
 }
 
 public struct IncludeDecodingError: Swift.Error, Equatable, CustomStringConvertible {
     public let failures: [ResourceObjectDecodingError]
 
     public var description: String {
+        // concise error when all failures are mismatched JSON:API types:
+        if case let .jsonTypeMismatch(foundType: foundType)? = failures.first?.cause,
+           failures.allSatisfy({ $0.cause.isTypeMismatch }) {
+            let expectedTypes = failures
+                .compactMap { "'\($0.resourceObjectJsonAPIType)'" }
+                .joined(separator: ", ")
+
+            return "Found JSON:API type '\(foundType)' but expected one of \(expectedTypes)"
+        }
+
+        // concise error when all but failures but one are type mismatches because
+        // we can assume the correct type was found but there was some other error:
+        let nonTypeMismatches = failures.filter({ !$0.cause.isTypeMismatch})
+        if nonTypeMismatches.count == 1, let nonTypeMismatch = nonTypeMismatches.first {
+            return String(describing: nonTypeMismatch)
+        }
+
+        // fall back to just describing all of the reasons it could not have been any of the available
+        // types:
         return failures
             .enumerated()
             .map {
-                "\nCould not have been Include Type \($0.offset + 1) because:\n\($0.element)"
+                "\nCould not have been Include Type `\($0.element.resourceObjectJsonAPIType)` because:\n\($0.element)"
         }.joined(separator: "\n")
     }
 }

Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift

@@ -398,7 +398,7 @@ public extension ResourceObject {
         do {
             type = try container.decode(String.self, forKey: .type)
         } catch let error as DecodingError {
-            throw ResourceObjectDecodingError(error)
+            throw ResourceObjectDecodingError(error, jsonAPIType: Self.jsonType)
                 ?? error
         }
 
@@ -417,13 +417,14 @@ public extension ResourceObject {
                 ?? container.decodeIfPresent(Description.Attributes.self, forKey: .attributes)
                 ?? Description.Attributes(from: EmptyObjectDecoder())
         } catch let decodingError as DecodingError {
-            throw ResourceObjectDecodingError(decodingError)
+            throw ResourceObjectDecodingError(decodingError, jsonAPIType: Self.jsonType)
                 ?? decodingError
         } catch _ as EmptyObjectDecodingError {
             throw ResourceObjectDecodingError(
                 subjectName: ResourceObjectDecodingError.entireObject,
                 cause: .keyNotFound,
-                location: .attributes
+                location: .attributes,
+                jsonAPIType: Self.jsonType
             )
         }
 
@@ -432,16 +433,17 @@ public extension ResourceObject {
                 ?? container.decodeIfPresent(Description.Relationships.self, forKey: .relationships)
                 ?? Description.Relationships(from: EmptyObjectDecoder())
         } catch let decodingError as DecodingError {
-            throw ResourceObjectDecodingError(decodingError)
+            throw ResourceObjectDecodingError(decodingError, jsonAPIType: Self.jsonType)
                 ?? decodingError
         } catch let decodingError as JSONAPICodingError {
-            throw ResourceObjectDecodingError(decodingError)
+            throw ResourceObjectDecodingError(decodingError, jsonAPIType: Self.jsonType)
                 ?? decodingError
         } catch _ as EmptyObjectDecodingError {
             throw ResourceObjectDecodingError(
                 subjectName: ResourceObjectDecodingError.entireObject,
                 cause: .keyNotFound,
-                location: .relationships
+                location: .relationships,
+                jsonAPIType: Self.jsonType
             )
         }
 

Sources/JSONAPI/Resource/Resource Object/ResourceObjectDecodingError.swift

@@ -6,6 +6,7 @@
 //
 
 public struct ResourceObjectDecodingError: Swift.Error, Equatable {
+    public let resourceObjectJsonAPIType: String
     public let subjectName: String
     public let cause: Cause
     public let location: Location
@@ -16,8 +17,13 @@ public struct ResourceObjectDecodingError: Swift.Error, Equatable {
         case keyNotFound
         case valueNotFound
         case typeMismatch(expectedTypeName: String)
-        case jsonTypeMismatch(expectedType: String, foundType: String)
+        case jsonTypeMismatch(foundType: String)
         case quantityMismatch(expected: JSONAPICodingError.Quantity)
+
+        internal var isTypeMismatch: Bool {
+            guard case .jsonTypeMismatch = self else { return false}
+            return true
+        }
     }
 
     public enum Location: String, Equatable {
@@ -38,7 +44,8 @@ public struct ResourceObjectDecodingError: Swift.Error, Equatable {
         }
     }
 
-    init?(_ decodingError: DecodingError) {
+    init?(_ decodingError: DecodingError, jsonAPIType: String) {
+        self.resourceObjectJsonAPIType = jsonAPIType
         switch decodingError {
         case .typeMismatch(let expectedType, let ctx):
             (location, subjectName) = Self.context(ctx)
@@ -67,11 +74,12 @@ public struct ResourceObjectDecodingError: Swift.Error, Equatable {
         }
     }
 
-    init?(_ jsonAPIError: JSONAPICodingError) {
+    init?(_ jsonAPIError: JSONAPICodingError, jsonAPIType: String) {
+        self.resourceObjectJsonAPIType = jsonAPIType
         switch jsonAPIError {
         case .typeMismatch(expected: let expected, found: let found, path: let path):
             (location, subjectName) = Self.context(path: path)
-            cause = .jsonTypeMismatch(expectedType: expected, foundType: found)
+            cause = .jsonTypeMismatch(foundType: found)
         case .quantityMismatch(expected: let expected, path: let path):
             (location, subjectName) = Self.context(path: path)
             cause = .quantityMismatch(expected: expected)
@@ -81,12 +89,14 @@ public struct ResourceObjectDecodingError: Swift.Error, Equatable {
     }
 
     init(expectedJSONAPIType: String, found: String) {
+        resourceObjectJsonAPIType = expectedJSONAPIType
         location = .type
         subjectName = "self"
-        cause = .jsonTypeMismatch(expectedType: expectedJSONAPIType, foundType: found)
+        cause = .jsonTypeMismatch(foundType: found)
     }
 
-    init(subjectName: String, cause: Cause, location: Location) {
+    init(subjectName: String, cause: Cause, location: Location, jsonAPIType: String) {
+        self.resourceObjectJsonAPIType = jsonAPIType
         self.subjectName = subjectName
         self.cause = cause
         self.location = location
@@ -135,10 +145,10 @@ extension ResourceObjectDecodingError: CustomStringConvertible {
             return "'\(location.singular)' (a.k.a. the JSON:API type name) is not a \(expected) as expected."
         case .typeMismatch(expectedTypeName: let expected):
             return "'\(subjectName)' \(location.singular) is not a \(expected) as expected."
-        case .jsonTypeMismatch(expectedType: let expected, foundType: let found) where location == .type:
-            return "found JSON:API type \"\(found)\" but expected \"\(expected)\""
-        case .jsonTypeMismatch(expectedType: let expected, foundType: let found):
-            return "'\(subjectName)' \(location.singular) is of JSON:API type \"\(found)\" but it was expected to be \"\(expected)\""
+        case .jsonTypeMismatch(foundType: let found) where location == .type:
+            return "found JSON:API type \"\(found)\" but expected \"\(resourceObjectJsonAPIType)\""
+        case .jsonTypeMismatch(foundType: let found):
+            return "'\(subjectName)' \(location.singular) is of JSON:API type \"\(found)\" but it was expected to be \"\(resourceObjectJsonAPIType)\""
         case .quantityMismatch(expected: let expected):
             let expecation: String = {
                 switch expected {

Tests/JSONAPITests/Document/DocumentDecodingErrorTests.swift

@@ -79,6 +79,7 @@ final class DocumentDecodingErrorTests: XCTestCase {
     }
 
     func test_include_failure() {
+        // test that if there is only one possible include, we just find out on one line what expecation failed.
         XCTAssertThrowsError(
             try testDecoder.decode(
                 Document<SingleResourceBody<Article>, NoMetadata, NoLinks, Include1<Author>, NoAPIDescription, UnknownJSONAPIError>.self,
@@ -91,11 +92,12 @@ final class DocumentDecodingErrorTests: XCTestCase {
                     return
             }
 
-            XCTAssertEqual(String(describing: error), #"Include 3 failed to parse: found JSON:API type "not_an_author" but expected "authors""#)
+            XCTAssertEqual(String(describing: error), #"Out of the 3 includes in the document, the 3rd one failed to parse: found JSON:API type "not_an_author" but expected "authors""#)
         }
     }
 
     func test_include_failure2() {
+        // test that if there are two possiblie includes, we find out why each of them was not possible to decode.
         XCTAssertThrowsError(
             try testDecoder.decode(
                 Document<SingleResourceBody<Article>, NoMetadata, NoLinks, Include2<Article, Author>, NoAPIDescription, UnknownJSONAPIError>.self,
@@ -108,15 +110,30 @@ final class DocumentDecodingErrorTests: XCTestCase {
                     return
             }
 
-            XCTAssertEqual(String(describing: error),
-#"""
-Include 3 failed to parse: 
-Could not have been Include Type 1 because:
-found JSON:API type "not_an_author" but expected "articles"
+            XCTAssertEqual(
+                String(describing: error),
+                "Out of the 3 includes in the document, the 3rd one failed to parse: Found JSON:API type 'not_an_author' but expected one of 'articles', 'authors'"
+            )
+        }
+    }
+
+    func test_include_failure3() {
+        // test that if the failed include is at a different index, the other index is reported correctly.
+        XCTAssertThrowsError(
+            try testDecoder.decode(
+                Document<SingleResourceBody<Article>, NoMetadata, NoLinks, Include2<Article, Author>, NoAPIDescription, UnknownJSONAPIError>.self,
+                from: single_document_some_includes_wrong_type2
+            )
+        ) { error in
+            guard let docError = error as? DocumentDecodingError,
+                  case .includes = docError else {
+                XCTFail("Expected primary resource document error. Got \(error)")
+                return
+            }
 
-Could not have been Include Type 2 because:
-found JSON:API type "not_an_author" but expected "authors"
-"""#
+            XCTAssertEqual(
+                String(describing: error),
+                "Out of the 3 includes in the document, the 2nd one failed to parse: Found JSON:API type 'not_an_author' but expected one of 'articles', 'authors'"
             )
         }
     }

Tests/JSONAPITests/Document/stubs/DocumentStubs.swift

@@ -289,6 +289,37 @@ let single_document_some_includes_wrong_type = """
 }
 """.data(using: .utf8)!
 
+let single_document_some_includes_wrong_type2 = """
+{
+    "data": {
+        "id": "1",
+        "type": "articles",
+        "relationships": {
+            "author": {
+                "data": {
+                    "type": "authors",
+                    "id": "33"
+                }
+            }
+        }
+    },
+    "included": [
+        {
+            "id": "30",
+            "type": "authors"
+        },
+        {
+            "id": "31",
+            "type": "not_an_author"
+        },
+        {
+            "id": "33",
+            "type": "authors"
+        }
+    ]
+}
+""".data(using: .utf8)!
+
 let single_document_some_includes_with_api_description = """
 {
 	"data": {

Tests/JSONAPITests/Includes/IncludesDecodingErrorTests.swift

@@ -10,7 +10,6 @@ import JSONAPI
 
 final class IncludesDecodingErrorTests: XCTestCase {
     func test_unexpectedIncludeType() {
-        var error1: Error!
         XCTAssertThrowsError(try testDecoder.decode(Includes<Include2<TestEntity, TestEntity2>>.self, from: three_different_type_includes)) { (error: Error) -> Void in
             XCTAssertEqual(
                 (error as? IncludesDecodingError)?.idx,
@@ -19,23 +18,53 @@ final class IncludesDecodingErrorTests: XCTestCase {
 
             XCTAssertEqual(
                 (error as? IncludesDecodingError).map(String.init(describing:)),
-"""
-Include 3 failed to parse: \nCould not have been Include Type 1 because:
-found JSON:API type "test_entity4" but expected "test_entity1"
+                "Out of the 3 includes in the document, the 3rd one failed to parse: Found JSON:API type 'test_entity4' but expected one of 'test_entity1', 'test_entity2'"
+            )
+        }
 
-Could not have been Include Type 2 because:
-found JSON:API type "test_entity4" but expected "test_entity2"
-"""
+        // now test that we get the same error with a different total include count from a different test stub
+        XCTAssertThrowsError(try testDecoder.decode(Includes<Include2<TestEntity, TestEntity2>>.self, from: four_different_type_includes)) { (error2: Error) -> Void in
+            XCTAssertEqual(
+                (error2 as? IncludesDecodingError).map(String.init(describing:)),
+                "Out of the 4 includes in the document, the 3rd one failed to parse: Found JSON:API type 'test_entity4' but expected one of 'test_entity1', 'test_entity2'"
             )
+        }
 
-            error1 = error
+        // and with six total includes
+        XCTAssertThrowsError(try testDecoder.decode(Includes<Include2<TestEntity, TestEntity2>>.self, from: six_includes_one_bad_type)) { (error2: Error) -> Void in
+            XCTAssertEqual(
+                (error2 as? IncludesDecodingError).map(String.init(describing:)),
+                "Out of the 6 includes in the document, the 5th one failed to parse: Found JSON:API type 'test_entity4' but expected one of 'test_entity1', 'test_entity2'"
+            )
         }
 
-        // now test that we get the same error from a different test stub
-        XCTAssertThrowsError(try testDecoder.decode(Includes<Include2<TestEntity, TestEntity2>>.self, from: four_different_type_includes)) { (error2: Error) -> Void in
+        // and with a number of total includes between 10 and 19
+        XCTAssertThrowsError(try testDecoder.decode(Includes<Include2<TestEntity, TestEntity2>>.self, from: eleven_includes_one_bad_type)) { (error2: Error) -> Void in
             XCTAssertEqual(
-                error1 as? IncludesDecodingError,
-                error2 as? IncludesDecodingError
+                (error2 as? IncludesDecodingError).map(String.init(describing:)),
+                "Out of the 11 includes in the document, the 10th one failed to parse: Found JSON:API type 'test_entity4' but expected one of 'test_entity1', 'test_entity2'"
+            )
+        }
+
+        // and finally with a larger number of total includes
+        XCTAssertThrowsError(try testDecoder.decode(Includes<Include2<TestEntity, TestEntity2>>.self, from: twenty_two_includes_one_bad_type)) { (error2: Error) -> Void in
+            XCTAssertEqual(
+                (error2 as? IncludesDecodingError).map(String.init(describing:)),
+                "Out of the 22 includes in the document, the 21st one failed to parse: Found JSON:API type 'test_entity4' but expected one of 'test_entity1', 'test_entity2'"
+            )
+        }
+    }
+
+    func test_missingProperty() {
+        XCTAssertThrowsError(
+            try testDecoder.decode(
+                Includes<Include3<TestEntity, TestEntity2, TestEntity4>>.self,
+                from: three_includes_one_missing_attributes
+            )
+        ) { (error: Error) -> Void in
+            XCTAssertEqual(
+                (error as? IncludesDecodingError).map(String.init(describing:)),
+                "Out of the 3 includes in the document, the 3rd one failed to parse: 'foo' attribute is required and missing."
             )
         }
     }