From 2eecf959953f18d8cd1dffb068a2dc60dbcbed30 Mon Sep 17 00:00:00 2001 From: Mathew Polzin Date: Sun, 10 Nov 2019 23:02:26 -0800 Subject: [PATCH] Adding Document Decoding Errors for some common problems --- Sources/JSONAPI/Document/Document.swift | 26 ++- .../Document/DocumentDecodingError.swift | 68 ++++++- Sources/JSONAPI/Document/Includes.swift | 25 ++- Sources/JSONAPI/Document/ResourceBody.swift | 17 +- Sources/JSONAPI/Resource/Id.swift | 7 +- .../Resource Object/ResourceObject.swift | 10 +- .../ResourceObjectDecodingError.swift | 21 +- .../Document/DocumentDecodingErrorTests.swift | 189 ++++++++++++++++++ .../Document/stubs/DocumentStubs.swift | 85 ++++++++ .../ResourceObjectDecodingErrorTests.swift | 24 +++ .../stubs/ResourceObjectStubs.swift | 11 + Tests/JSONAPITests/XCTestManifests.swift | 15 ++ 12 files changed, 483 insertions(+), 15 deletions(-) create mode 100644 Tests/JSONAPITests/Document/DocumentDecodingErrorTests.swift diff --git a/Sources/JSONAPI/Document/Document.swift b/Sources/JSONAPI/Document/Document.swift index 7c387be..34c32a4 100644 --- a/Sources/JSONAPI/Document/Document.swift +++ b/Sources/JSONAPI/Document/Document.swift @@ -349,8 +349,8 @@ extension Document: Decodable, CodableJSONAPIDocument where PrimaryResourceBody: public init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: RootCodingKeys.self) - if let noData = NoAPIDescription() as? APIDescription { - apiDescription = noData + if let noAPIDescription = NoAPIDescription() as? APIDescription { + apiDescription = noAPIDescription } else { apiDescription = try container.decode(APIDescription.self, forKey: .jsonapi) } @@ -389,10 +389,24 @@ extension Document: Decodable, CodableJSONAPIDocument where PrimaryResourceBody: if let noData = NoResourceBody() as? PrimaryResourceBody { data = noData } else { - data = try container.decode(PrimaryResourceBody.self, forKey: .data) + do { + data = try container.decode(PrimaryResourceBody.self, forKey: .data) + } catch let error as ResourceObjectDecodingError { + throw DocumentDecodingError(error) + } catch let error as ManyResourceBodyDecodingError { + throw DocumentDecodingError(error) + } catch let error as DecodingError { + throw DocumentDecodingError(error) + ?? error + } } - let maybeIncludes = try container.decodeIfPresent(Includes.self, forKey: .included) + let maybeIncludes: Includes? + do { + maybeIncludes = try container.decodeIfPresent(Includes.self, forKey: .included) + } catch let error as IncludesDecodingError { + throw DocumentDecodingError(error) + } // TODO come back to this and make robust @@ -569,7 +583,7 @@ extension Document.ErrorDocument: Decodable, CodableJSONAPIDocument document = try container.decode(Document.self) guard document.body.isError else { - throw JSONAPIDocumentDecodingError.foundSuccessDocumentWhenExpectingError + throw DocumentDecodingError.foundSuccessDocumentWhenExpectingError } } } @@ -582,7 +596,7 @@ extension Document.SuccessDocument: Decodable, CodableJSONAPIDocument document = try container.decode(Document.self) guard !document.body.isError else { - throw JSONAPIDocumentDecodingError.foundErrorDocumentWhenExpectingSuccess + throw DocumentDecodingError.foundErrorDocumentWhenExpectingSuccess } } } diff --git a/Sources/JSONAPI/Document/DocumentDecodingError.swift b/Sources/JSONAPI/Document/DocumentDecodingError.swift index 912a8f8..0e58fe3 100644 --- a/Sources/JSONAPI/Document/DocumentDecodingError.swift +++ b/Sources/JSONAPI/Document/DocumentDecodingError.swift @@ -1,11 +1,75 @@ // -// DocumentDecodingErro.swift +// DocumentDecodingError.swift // // // Created by Mathew Polzin on 10/20/19. // -public enum JSONAPIDocumentDecodingError: Swift.Error { +public enum DocumentDecodingError: Swift.Error, Equatable { + case primaryResource(error: ResourceObjectDecodingError, idx: Int?) + case primaryResourceMissing + case primaryResourcesMissing + + case includes(error: IncludesDecodingError) + case foundErrorDocumentWhenExpectingSuccess case foundSuccessDocumentWhenExpectingError + + init(_ decodingError: ResourceObjectDecodingError) { + self = .primaryResource(error: decodingError, idx: nil) + } + + init(_ decodingError: ManyResourceBodyDecodingError) { + self = .primaryResource(error: decodingError.error, idx: decodingError.idx) + } + + init(_ decodingError: IncludesDecodingError) { + self = .includes(error: decodingError) + } + + init?(_ decodingError: DecodingError) { + switch decodingError { + case .valueNotFound(let type, let context) where Location(context) == .data && type is AbstractResourceObject.Type: + self = .primaryResourceMissing + case .valueNotFound(let type, let context) where Location(context) == .data && type == UnkeyedDecodingContainer.self: + self = .primaryResourcesMissing + default: + return nil + } + } + + private enum Location: Equatable { + case data + case other + + init(_ context: DecodingError.Context) { + if context.codingPath.contains(where: { $0.stringValue == "data" }) { + self = .data + } else { + self = .other + } + } + } +} + +extension DocumentDecodingError: CustomStringConvertible { + public var description: String { + switch self { + case .primaryResource(error: let error, idx: let idx): + let idxString = idx.map { " \($0 + 1)" } ?? "" + return "Primary Resource\(idxString) failed to parse because \(error)" + case .primaryResourceMissing: + return "Primary Resource missing." + case .primaryResourcesMissing: + return "Primary Resources array missing." + + case .includes(error: let error): + return "\(error)" + + case .foundErrorDocumentWhenExpectingSuccess: + return "Expected a success document with a 'data' property but found an error document." + case .foundSuccessDocumentWhenExpectingError: + return "Expected an error document but found a success document with a 'data' property." + } + } } diff --git a/Sources/JSONAPI/Document/Includes.swift b/Sources/JSONAPI/Document/Includes.swift index e7a701b..2174c67 100644 --- a/Sources/JSONAPI/Document/Includes.swift +++ b/Sources/JSONAPI/Document/Includes.swift @@ -56,8 +56,14 @@ extension Includes: Decodable where I: Decodable { } var valueAggregator = [I]() + var idx = 0 while !container.isAtEnd { - valueAggregator.append(try container.decode(I.self)) + do { + valueAggregator.append(try container.decode(I.self)) + idx = idx + 1 + } catch let error { + throw IncludesDecodingError(error: error, idx: idx) + } } values = valueAggregator @@ -177,3 +183,20 @@ extension Includes where I: _Poly11 { return values.compactMap { $0.k } } } + +// MARK: - DecodingError +public struct IncludesDecodingError: Swift.Error, Equatable { + public let error: Swift.Error + public let idx: Int + + public static func ==(lhs: Self, rhs: Self) -> Bool { + return lhs.idx == rhs.idx + && String(describing: lhs) == String(describing: rhs) + } +} + +extension IncludesDecodingError: CustomStringConvertible { + public var description: String { + return "Include \(idx + 1) failed to parse: \(error)" + } +} diff --git a/Sources/JSONAPI/Document/ResourceBody.swift b/Sources/JSONAPI/Document/ResourceBody.swift index 25090c8..5a7c96f 100644 --- a/Sources/JSONAPI/Document/ResourceBody.swift +++ b/Sources/JSONAPI/Document/ResourceBody.swift @@ -125,8 +125,17 @@ extension ManyResourceBody: Decodable, CodableResourceBody where PrimaryResource public init(from decoder: Decoder) throws { var container = try decoder.unkeyedContainer() var valueAggregator = [PrimaryResource]() + var idx = 0 while !container.isAtEnd { - valueAggregator.append(try container.decode(PrimaryResource.self)) + do { + valueAggregator.append(try container.decode(PrimaryResource.self)) + } catch let error as ResourceObjectDecodingError { + throw ManyResourceBodyDecodingError( + error: error, + idx: idx + ) + } + idx = idx + 1 } values = valueAggregator } @@ -145,3 +154,9 @@ extension ManyResourceBody: CustomStringConvertible { return "PrimaryResourceBody(\(String(describing: values)))" } } + +// MARK: - DecodingError +public struct ManyResourceBodyDecodingError: Swift.Error, Equatable { + public let error: ResourceObjectDecodingError + public let idx: Int +} diff --git a/Sources/JSONAPI/Resource/Id.swift b/Sources/JSONAPI/Resource/Id.swift index d66e3d9..1957b87 100644 --- a/Sources/JSONAPI/Resource/Id.swift +++ b/Sources/JSONAPI/Resource/Id.swift @@ -45,7 +45,10 @@ public protocol OptionalId: Codable { init(rawValue: RawType) } -public protocol IdType: OptionalId, CustomStringConvertible, Hashable where RawType: RawIdType {} +/// marker protocol +public protocol AbstractId {} + +public protocol IdType: AbstractId, OptionalId, CustomStringConvertible, Hashable where RawType: RawIdType {} extension Optional: MaybeRawId where Wrapped: Codable & Equatable {} extension Optional: OptionalId where Wrapped: IdType { @@ -94,7 +97,7 @@ public struct Id: Equa } } -extension Id: Hashable, CustomStringConvertible, IdType where RawType: RawIdType { +extension Id: Hashable, CustomStringConvertible, AbstractId, IdType where RawType: RawIdType { public static func id(from rawValue: RawType) -> Id { return Id(rawValue: rawValue) } diff --git a/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift b/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift index e85d8f0..0604b00 100644 --- a/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift +++ b/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift @@ -96,10 +96,13 @@ extension ResourceObjectProxy { public static var jsonType: String { return Description.jsonType } } +/// A marker protocol. +public protocol AbstractResourceObject {} + /// ResourceObjectType is the protocol that ResourceObject conforms to. This /// protocol lets other types accept any ResourceObject as a generic /// specialization. -public protocol ResourceObjectType: ResourceObjectProxy, CodablePrimaryResource where Description: ResourceObjectDescription { +public protocol ResourceObjectType: AbstractResourceObject, ResourceObjectProxy, CodablePrimaryResource where Description: ResourceObjectDescription { associatedtype Meta: JSONAPI.Meta associatedtype Links: JSONAPI.Links @@ -414,7 +417,10 @@ public extension ResourceObject { let type = try container.decode(String.self, forKey: .type) guard ResourceObject.jsonType == type else { - throw JSONAPICodingError.typeMismatch(expected: Description.jsonType, found: type, path: decoder.codingPath) + throw ResourceObjectDecodingError( + expectedJSONAPIType: ResourceObject.jsonType, + found: type + ) } let maybeUnidentified = Unidentified() as? EntityRawIdType diff --git a/Sources/JSONAPI/Resource/Resource Object/ResourceObjectDecodingError.swift b/Sources/JSONAPI/Resource/Resource Object/ResourceObjectDecodingError.swift index d1ac49c..b644ce6 100644 --- a/Sources/JSONAPI/Resource/Resource Object/ResourceObjectDecodingError.swift +++ b/Sources/JSONAPI/Resource/Resource Object/ResourceObjectDecodingError.swift @@ -23,11 +23,13 @@ public struct ResourceObjectDecodingError: Swift.Error, Equatable { public enum Location: String, Equatable { case attributes case relationships + case type var singular: String { switch self { case .attributes: return "attribute" case .relationships: return "relationship" + case .type: return "type" } } } @@ -63,6 +65,12 @@ public struct ResourceObjectDecodingError: Swift.Error, Equatable { } } + init(expectedJSONAPIType: String, found: String) { + location = .type + subjectName = "self" + cause = .jsonTypeMismatch(expectedType: expectedJSONAPIType, foundType: found) + } + init(subjectName: String, cause: Cause, location: Location) { self.subjectName = subjectName self.cause = cause @@ -75,8 +83,17 @@ public struct ResourceObjectDecodingError: Swift.Error, Equatable { } static func context(path: [CodingKey]) -> (Location, name: String) { + let location: Location + if path.contains(where: { $0.stringValue == "attributes" }) { + location = .attributes + } else if path.contains(where: { $0.stringValue == "relationships" }) { + location = .relationships + } else { + location = .type + } + return ( - path.contains { $0.stringValue == "attributes" } ? .attributes : .relationships, + location, name: path.last?.stringValue ?? "unnamed" ) } @@ -94,6 +111,8 @@ extension ResourceObjectDecodingError: CustomStringConvertible { return "'\(subjectName)' \(location.singular) is not nullable but null." 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 .quantityMismatch(expected: let expected): diff --git a/Tests/JSONAPITests/Document/DocumentDecodingErrorTests.swift b/Tests/JSONAPITests/Document/DocumentDecodingErrorTests.swift new file mode 100644 index 0000000..055e007 --- /dev/null +++ b/Tests/JSONAPITests/Document/DocumentDecodingErrorTests.swift @@ -0,0 +1,189 @@ +// +// DocumentDecodingErrorTests.swift +// +// +// Created by Mathew Polzin on 11/10/19. +// + +import XCTest +import JSONAPI +import Poly + +final class DocumentDecodingErrorTests: XCTestCase { + func test_singlePrimaryResource_missing() { + XCTAssertThrowsError( + try testDecoder.decode( + Document, NoMetadata, NoLinks, NoIncludes, NoAPIDescription, UnknownJSONAPIError>.self, + from: single_document_null + ) + ) { error in + guard let docError = error as? DocumentDecodingError, + case .primaryResourceMissing = docError else { + XCTFail("Expected primary resource missing error. Got \(error)") + return + } + + XCTAssertEqual(String(describing: error), "Primary Resource missing.") + } + } + + func test_singlePrimaryResource_failure() { + XCTAssertThrowsError( + try testDecoder.decode( + Document, NoMetadata, NoLinks, NoIncludes, NoAPIDescription, UnknownJSONAPIError>.self, + from: single_document_no_includes_missing_relationship + ) + ) { error in + guard let docError = error as? DocumentDecodingError, + case .primaryResource = docError else { + XCTFail("Expected primary resource document error. Got \(error)") + return + } + + XCTAssertEqual(String(describing: error), "Primary Resource failed to parse because 'author' relationship is required and missing.") + } + } + + func test_manyPrimaryResource_missing() { + XCTAssertThrowsError( + try testDecoder.decode( + Document, NoMetadata, NoLinks, NoIncludes, NoAPIDescription, UnknownJSONAPIError>.self, + from: many_document_no_includes_data_is_null + ) + ) { error in + guard let docError = error as? DocumentDecodingError, + case .primaryResourcesMissing = docError else { + XCTFail("Expected primary resource missing error. Got \(error)") + return + } + + XCTAssertEqual(String(describing: error), "Primary Resources array missing.") + } + } + + func test_manyPrimaryResource_failure() { + XCTAssertThrowsError( + try testDecoder.decode( + Document, NoMetadata, NoLinks, NoIncludes, NoAPIDescription, UnknownJSONAPIError>.self, + from: many_document_no_includes_missing_relationship + ) + ) { error in + guard let docError = error as? DocumentDecodingError, + case .primaryResource = docError else { + XCTFail("Expected primary resource document error. Got \(error)") + return + } + + XCTAssertEqual(String(describing: error), "Primary Resource 2 failed to parse because 'author' relationship is required and missing.") + } + } + + func test_include_failure() { + XCTAssertThrowsError( + try testDecoder.decode( + Document, NoMetadata, NoLinks, Include1, NoAPIDescription, UnknownJSONAPIError>.self, + from: single_document_some_includes_wrong_type + ) + ) { error in + guard let docError = error as? DocumentDecodingError, + case .includes = docError else { + XCTFail("Expected primary resource document error. Got \(error)") + return + } + + XCTAssertEqual(String(describing: error), #"Include 3 failed to parse: found JSON:API type "not_an_author" but expected "authors""#) + } + } +} + +// MARK: - Test Types +extension DocumentDecodingErrorTests { + enum AuthorType: ResourceObjectDescription { + static var jsonType: String { return "authors" } + + typealias Attributes = NoAttributes + typealias Relationships = NoRelationships + } + + typealias Author = BasicEntity + + enum ArticleType: ResourceObjectDescription { + static var jsonType: String { return "articles" } + + typealias Attributes = NoAttributes + + struct Relationships: JSONAPI.Relationships { + let author: ToOneRelationship + } + } + + typealias Article = BasicEntity + + enum BookType: ResourceObjectDescription { + static var jsonType: String { return "books" } + + struct Attributes: JSONAPI.SparsableAttributes { + let pageCount: Attribute + + enum CodingKeys: String, SparsableCodingKey { + case pageCount + } + } + + struct Relationships: JSONAPI.Relationships { + let author: ToOneRelationship + let series: ToManyRelationship + } + } + + typealias Book = BasicEntity + + struct TestPageMetadata: JSONAPI.Meta { + let total: Int + let limit: Int + let offset: Int + } + + struct TestLinks: JSONAPI.Links { + let link: Link + let link2: Link + + struct TestMetadata: JSONAPI.Meta { + let hello: String + } + } + + typealias TestAPIDescription = APIDescription + + enum TestError: JSONAPIError { + case unknownError + case basic(BasicError) + + struct BasicError: Codable, Equatable { + let code: Int + let description: String + } + + public init(from decoder: Decoder) throws { + let container = try decoder.singleValueContainer() + + self = (try? .basic(container.decode(BasicError.self))) ?? .unknown + } + + public func encode(to encoder: Encoder) throws { + var container = encoder.singleValueContainer() + + switch self { + case .unknownError: + try container.encode("unknown") + case .basic(let error): + try container.encode(error) + } + } + + public static var unknown: Self { + return .unknownError + } + } +} + diff --git a/Tests/JSONAPITests/Document/stubs/DocumentStubs.swift b/Tests/JSONAPITests/Document/stubs/DocumentStubs.swift index 7455a0c..ed83ebf 100644 --- a/Tests/JSONAPITests/Document/stubs/DocumentStubs.swift +++ b/Tests/JSONAPITests/Document/stubs/DocumentStubs.swift @@ -37,6 +37,17 @@ let single_document_no_includes = """ } """.data(using: .utf8)! +let single_document_no_includes_missing_relationship = """ +{ + "data": { + "id": "1", + "type": "articles", + "relationships": { + } + } +} +""".data(using: .utf8)! + let single_document_no_includes_with_api_description = """ { "data": { @@ -247,6 +258,37 @@ let single_document_some_includes = """ } """.data(using: .utf8)! +let single_document_some_includes_wrong_type = """ +{ + "data": { + "id": "1", + "type": "articles", + "relationships": { + "author": { + "data": { + "type": "authors", + "id": "33" + } + } + } + }, + "included": [ + { + "id": "30", + "type": "authors" + }, + { + "id": "31", + "type": "authors" + }, + { + "id": "33", + "type": "not_an_author" + } + ] +} +""".data(using: .utf8)! + let single_document_some_includes_with_api_description = """ { "data": { @@ -452,6 +494,49 @@ let many_document_no_includes = """ } """.data(using: .utf8)! +let many_document_no_includes_data_is_null = """ +{ + "data": null +} +""".data(using: .utf8)! + +let many_document_no_includes_missing_relationship = """ +{ + "data": [ + { + "id": "1", + "type": "articles", + "relationships": { + "author": { + "data": { + "type": "authors", + "id": "33" + } + } + } + }, + { + "id": "2", + "type": "articles", + "relationships": { + } + }, + { + "id": "3", + "type": "articles", + "relationships": { + "author": { + "data": { + "type": "authors", + "id": "11" + } + } + } + } + ] +} +""".data(using: .utf8)! + let many_document_no_includes_with_api_description = """ { "data": [ diff --git a/Tests/JSONAPITests/ResourceObject/ResourceObjectDecodingErrorTests.swift b/Tests/JSONAPITests/ResourceObject/ResourceObjectDecodingErrorTests.swift index 0891d23..d0abb6e 100644 --- a/Tests/JSONAPITests/ResourceObject/ResourceObjectDecodingErrorTests.swift +++ b/Tests/JSONAPITests/ResourceObject/ResourceObjectDecodingErrorTests.swift @@ -287,6 +287,30 @@ extension ResourceObjectDecodingErrorTests { } } +// MARK: - JSON:API Type +extension ResourceObjectDecodingErrorTests { + func test_wrongType() { + XCTAssertThrowsError(try testDecoder.decode( + TestEntity2.self, + from: entity_is_wrong_type + )) { error in + XCTAssertEqual( + error as? ResourceObjectDecodingError, + ResourceObjectDecodingError( + subjectName: "self", + cause: .jsonTypeMismatch(expectedType: "fourteenth_test_entities", foundType: "not_correct_type"), + location: .type + ) + ) + + XCTAssertEqual( + (error as? ResourceObjectDecodingError)?.description, + #"found JSON:API type "not_correct_type" but expected "fourteenth_test_entities""# + ) + } + } +} + // MARK: - Test Types extension ResourceObjectDecodingErrorTests { enum TestEntityType: ResourceObjectDescription { diff --git a/Tests/JSONAPITests/ResourceObject/stubs/ResourceObjectStubs.swift b/Tests/JSONAPITests/ResourceObject/stubs/ResourceObjectStubs.swift index 9fe7355..861ea1c 100644 --- a/Tests/JSONAPITests/ResourceObject/stubs/ResourceObjectStubs.swift +++ b/Tests/JSONAPITests/ResourceObject/stubs/ResourceObjectStubs.swift @@ -533,6 +533,17 @@ let entity_attribute_is_wrong_type3 = """ } """.data(using: .utf8)! +let entity_is_wrong_type = """ +{ + "id": "1", + "type": "not_correct_type", + "attributes": { + "required": "hello", + "yetAnother": 101 + } +} +""".data(using: .utf8)! + let entity_attributes_entirely_missing = """ { "id": "1", diff --git a/Tests/JSONAPITests/XCTestManifests.swift b/Tests/JSONAPITests/XCTestManifests.swift index 0b2c004..0768089 100644 --- a/Tests/JSONAPITests/XCTestManifests.swift +++ b/Tests/JSONAPITests/XCTestManifests.swift @@ -85,6 +85,19 @@ extension CustomAttributesTests { ] } +extension DocumentDecodingErrorTests { + // DO NOT MODIFY: This is autogenerated, use: + // `swift test --generate-linuxmain` + // to regenerate. + static let __allTests__DocumentDecodingErrorTests = [ + ("test_include_failure", test_include_failure), + ("test_manyPrimaryResource_failure", test_manyPrimaryResource_failure), + ("test_manyPrimaryResource_missing", test_manyPrimaryResource_missing), + ("test_singlePrimaryResource_failure", test_singlePrimaryResource_failure), + ("test_singlePrimaryResource_missing", test_singlePrimaryResource_missing), + ] +} + extension DocumentTests { // DO NOT MODIFY: This is autogenerated, use: // `swift test --generate-linuxmain` @@ -379,6 +392,7 @@ extension ResourceObjectDecodingErrorTests { ("test_required_attribute", test_required_attribute), ("test_required_relationship", test_required_relationship), ("test_twoOneVsToMany_relationship", test_twoOneVsToMany_relationship), + ("test_wrongType", test_wrongType), ] } @@ -542,6 +556,7 @@ public func __allTests() -> [XCTestCaseEntry] { testCase(BasicJSONAPIErrorTests.__allTests__BasicJSONAPIErrorTests), testCase(ComputedPropertiesTests.__allTests__ComputedPropertiesTests), testCase(CustomAttributesTests.__allTests__CustomAttributesTests), + testCase(DocumentDecodingErrorTests.__allTests__DocumentDecodingErrorTests), testCase(DocumentTests.__allTests__DocumentTests), testCase(EmptyObjectDecoderTests.__allTests__EmptyObjectDecoderTests), testCase(GenericJSONAPIErrorTests.__allTests__GenericJSONAPIErrorTests),