From 9b6392ac0cd1c0aa2eb323a70d022301ea1b9c30 Mon Sep 17 00:00:00 2001 From: Vitalii Budnik Date: Tue, 19 Sep 2023 18:28:17 +0300 Subject: [PATCH 1/6] chore: allow absent `data` in ToManyRelationship --- Sources/JSONAPI/Resource/Relationship.swift | 5 +++++ Tests/JSONAPITests/Relationships/RelationshipTests.swift | 8 ++++++++ .../Relationships/stubs/RelationshipStubs.swift | 8 ++++++++ 3 files changed, 21 insertions(+) diff --git a/Sources/JSONAPI/Resource/Relationship.swift b/Sources/JSONAPI/Resource/Relationship.swift index fc03c4e..a2ada1e 100644 --- a/Sources/JSONAPI/Resource/Relationship.swift +++ b/Sources/JSONAPI/Resource/Relationship.swift @@ -401,6 +401,11 @@ extension ToManyRelationship: Codable { links = try container.decode(LinksType.self, forKey: .links) } + guard container.contains(.data) else { + idsWithMeta = [] + return + } + var identifiers: UnkeyedDecodingContainer do { identifiers = try container.nestedUnkeyedContainer(forKey: .data) diff --git a/Tests/JSONAPITests/Relationships/RelationshipTests.swift b/Tests/JSONAPITests/Relationships/RelationshipTests.swift index 4a9a5fe..6cb99c9 100644 --- a/Tests/JSONAPITests/Relationships/RelationshipTests.swift +++ b/Tests/JSONAPITests/Relationships/RelationshipTests.swift @@ -235,6 +235,14 @@ extension RelationshipTests { test_DecodeEncodeEquality(type: ToManyWithMetaAndLinks.self, data: to_many_relationship_with_meta_and_links) } + + func test_ToManyRelationshipWithMetaNoData() { + let relationship = decoded(type: ToManyWithMeta.self, + data: to_many_relationship_with_meta_no_data) + + XCTAssertEqual(relationship.ids, []) + XCTAssertEqual(relationship.meta.a, "hello") + } } // MARK: Nullable diff --git a/Tests/JSONAPITests/Relationships/stubs/RelationshipStubs.swift b/Tests/JSONAPITests/Relationships/stubs/RelationshipStubs.swift index 434667f..18bd179 100644 --- a/Tests/JSONAPITests/Relationships/stubs/RelationshipStubs.swift +++ b/Tests/JSONAPITests/Relationships/stubs/RelationshipStubs.swift @@ -241,3 +241,11 @@ let to_many_relationship_type_mismatch = """ ] } """.data(using: .utf8)! + +let to_many_relationship_with_meta_no_data = """ +{ + "meta": { + "a": "hello" + } +} +""".data(using: .utf8)! From 33ee3fa21e0751cd9d668c2d1c4e1672fa8b057c Mon Sep 17 00:00:00 2001 From: Vitalii Budnik Date: Wed, 20 Sep 2023 16:04:22 +0300 Subject: [PATCH 2/6] chore: add ResourceObjectWithOptionalDataInRelationships protocol --- Sources/JSONAPI/Resource/Relationship.swift | 13 +++++--- .../Resource Object/ResourceObject.swift | 20 ++++++++++++ .../Relationships/RelationshipTests.swift | 31 +++++++++++++++++-- .../Test Helpers/EncodeDecode.swift | 4 +++ 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/Sources/JSONAPI/Resource/Relationship.swift b/Sources/JSONAPI/Resource/Relationship.swift index a2ada1e..401ca0a 100644 --- a/Sources/JSONAPI/Resource/Relationship.swift +++ b/Sources/JSONAPI/Resource/Relationship.swift @@ -222,7 +222,7 @@ extension Optional: JSONAPIIdentifiable, OptionalRelatable, JSONTyped where Wrap } // MARK: Codable -private enum ResourceLinkageCodingKeys: String, CodingKey { +enum ResourceLinkageCodingKeys: String, CodingKey { case data = "data" case meta = "meta" case links = "links" @@ -401,9 +401,14 @@ extension ToManyRelationship: Codable { links = try container.decode(LinksType.self, forKey: .links) } - guard container.contains(.data) else { - idsWithMeta = [] - return + let hasData = container.contains(.data) + var canHaveNoDataInRelationships: Bool = false + if let relatableType = Relatable.self as? ResourceObjectWithOptionalDataInRelationships.Type { + canHaveNoDataInRelationships = relatableType.canHaveNoDataInRelationships + } + guard hasData || !canHaveNoDataInRelationships else { + idsWithMeta = [] + return } var identifiers: UnkeyedDecodingContainer diff --git a/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift b/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift index 77d367e..2f6ce38 100644 --- a/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift +++ b/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift @@ -58,6 +58,20 @@ public protocol ResourceObjectProxyDescription: JSONTyped { associatedtype Relationships: Equatable } +/// A flagging protocol for `ResourceObjectProxyDescription` objects. +/// Indicates an object with varying behavior when it's being decoded from resources and the `data` key is missing. +public protocol ResourceObjectWithOptionalDataInRelationships { + /// A Boolean flag indicating that instances while decoding from relationships can be decoded without `data` + /// key and have only links and meta information. + /// + /// Default value: `false`. + static var canHaveNoDataInRelationships: Bool { get } +} + +extension ResourceObjectWithOptionalDataInRelationships { + public static var canHaveNoDataInRelationships: Bool { false } +} + /// A `ResourceObjectDescription` describes a JSON API /// Resource Object. The Resource Object /// itself is encoded and decoded as an @@ -244,6 +258,12 @@ public extension ResourceObject where EntityRawIdType: CreatableRawIdType { } } +// Conformance to the protocol so we can access the `canHaveNoDataInRelationships` flag from +// `ToManyRelationship` in type-erasured `Relatable`. +extension ResourceObject: ResourceObjectWithOptionalDataInRelationships where Description: ResourceObjectWithOptionalDataInRelationships { + public static var canHaveNoDataInRelationships: Bool { Description.canHaveNoDataInRelationships } +} + // MARK: - Attribute Access public extension ResourceObjectProxy { // MARK: Dynaminc Member Keypath Lookup diff --git a/Tests/JSONAPITests/Relationships/RelationshipTests.swift b/Tests/JSONAPITests/Relationships/RelationshipTests.swift index 6cb99c9..7923a60 100644 --- a/Tests/JSONAPITests/Relationships/RelationshipTests.swift +++ b/Tests/JSONAPITests/Relationships/RelationshipTests.swift @@ -6,7 +6,7 @@ // import XCTest -import JSONAPI +@testable import JSONAPI class RelationshipTests: XCTestCase { @@ -236,12 +236,35 @@ extension RelationshipTests { data: to_many_relationship_with_meta_and_links) } - func test_ToManyRelationshipWithMetaNoData() { + func test_ToManyRelationshipWithMetaNoDataOmittable() { + TestEntityType1.canHaveNoDataInRelationships = true + let relationship = decoded(type: ToManyWithMeta.self, data: to_many_relationship_with_meta_no_data) XCTAssertEqual(relationship.ids, []) XCTAssertEqual(relationship.meta.a, "hello") + + TestEntityType1.canHaveNoDataInRelationships = false + } + + func test_ToManyRelationshipWithMetaNoDataNotOmittable() { + TestEntityType1.canHaveNoDataInRelationships = false + + do { + _ = try decodedThrows(type: ToManyWithMeta.self, + data: to_many_relationship_with_meta_no_data) + XCTFail("Expected decoding to fail.") + } catch let error as DecodingError { + switch error { + case .keyNotFound(ResourceLinkageCodingKeys.data, _): + break + default: + XCTFail("Expected error to be DecodingError.keyNotFound(.data), but got \(error)") + } + } catch { + XCTFail("Expected to have DecodingError.keyNotFound(.data), but got \(error)") + } } } @@ -285,12 +308,14 @@ extension RelationshipTests { // MARK: - Test types extension RelationshipTests { - enum TestEntityType1: ResourceObjectDescription { + enum TestEntityType1: ResourceObjectDescription, ResourceObjectWithOptionalDataInRelationships { typealias Attributes = NoAttributes typealias Relationships = NoRelationships public static var jsonType: String { return "test_entity1" } + + static var canHaveNoDataInRelationships: Bool = false } typealias TestEntity1 = BasicEntity diff --git a/Tests/JSONAPITests/Test Helpers/EncodeDecode.swift b/Tests/JSONAPITests/Test Helpers/EncodeDecode.swift index 75a93e7..2b59137 100644 --- a/Tests/JSONAPITests/Test Helpers/EncodeDecode.swift +++ b/Tests/JSONAPITests/Test Helpers/EncodeDecode.swift @@ -24,6 +24,10 @@ func decoded(type: T.Type, data: Data) -> T { return try! testDecoder.decode(T.self, from: data) } +func decodedThrows(type: T.Type, data: Data) throws -> T { + return try testDecoder.decode(T.self, from: data) +} + func encoded(value: T) -> Data { return try! testEncoder.encode(value) } From fd514b100153433bc4ce31e411b643b2419da8e4 Mon Sep 17 00:00:00 2001 From: Vitalii Budnik Date: Wed, 20 Sep 2023 18:59:41 +0300 Subject: [PATCH 3/6] chore: add default ResourceDescription conformance to ResourceObjectWithOptionalDataInRelationships --- Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift b/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift index 2f6ce38..42eb345 100644 --- a/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift +++ b/Sources/JSONAPI/Resource/Resource Object/ResourceObject.swift @@ -53,7 +53,7 @@ public protocol JSONTyped { /// A `ResourceObjectProxyDescription` is an `ResourceObjectDescription` /// without Codable conformance. -public protocol ResourceObjectProxyDescription: JSONTyped { +public protocol ResourceObjectProxyDescription: JSONTyped, ResourceObjectWithOptionalDataInRelationships { associatedtype Attributes: Equatable associatedtype Relationships: Equatable } From 93efa69c9d4cd905e37cfd6112d84149500e4881 Mon Sep 17 00:00:00 2001 From: Mathew Polzin Date: Fri, 1 Mar 2024 12:03:25 -0600 Subject: [PATCH 4/6] make coding keys private again and test the error description instead of the error enum --- Sources/JSONAPI/Resource/Relationship.swift | 2 +- .../Relationships/RelationshipTests.swift | 20 ++++++------------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/Sources/JSONAPI/Resource/Relationship.swift b/Sources/JSONAPI/Resource/Relationship.swift index 401ca0a..e8ec896 100644 --- a/Sources/JSONAPI/Resource/Relationship.swift +++ b/Sources/JSONAPI/Resource/Relationship.swift @@ -222,7 +222,7 @@ extension Optional: JSONAPIIdentifiable, OptionalRelatable, JSONTyped where Wrap } // MARK: Codable -enum ResourceLinkageCodingKeys: String, CodingKey { +private enum ResourceLinkageCodingKeys: String, CodingKey { case data = "data" case meta = "meta" case links = "links" diff --git a/Tests/JSONAPITests/Relationships/RelationshipTests.swift b/Tests/JSONAPITests/Relationships/RelationshipTests.swift index 7923a60..d600637 100644 --- a/Tests/JSONAPITests/Relationships/RelationshipTests.swift +++ b/Tests/JSONAPITests/Relationships/RelationshipTests.swift @@ -6,7 +6,7 @@ // import XCTest -@testable import JSONAPI +import JSONAPI class RelationshipTests: XCTestCase { @@ -251,19 +251,11 @@ extension RelationshipTests { func test_ToManyRelationshipWithMetaNoDataNotOmittable() { TestEntityType1.canHaveNoDataInRelationships = false - do { - _ = try decodedThrows(type: ToManyWithMeta.self, - data: to_many_relationship_with_meta_no_data) - XCTFail("Expected decoding to fail.") - } catch let error as DecodingError { - switch error { - case .keyNotFound(ResourceLinkageCodingKeys.data, _): - break - default: - XCTFail("Expected error to be DecodingError.keyNotFound(.data), but got \(error)") - } - } catch { - XCTFail("Expected to have DecodingError.keyNotFound(.data), but got \(error)") + XCTAssertThrowsError( + try decodedThrows(type: ToManyWithMeta.self, + data: to_many_relationship_with_meta_no_data) + ) { error in + XCTAssertEqual(error.localizedDescription, "The data couldn’t be read because it is missing.") } } } From cae14e9a03264bed6805b10b1bc5a44c5b14b66a Mon Sep 17 00:00:00 2001 From: Mathew Polzin Date: Fri, 1 Mar 2024 12:08:46 -0600 Subject: [PATCH 5/6] prefer let for variable that does not need to be mutated. --- Sources/JSONAPI/Resource/Relationship.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Sources/JSONAPI/Resource/Relationship.swift b/Sources/JSONAPI/Resource/Relationship.swift index e8ec896..6d6ca43 100644 --- a/Sources/JSONAPI/Resource/Relationship.swift +++ b/Sources/JSONAPI/Resource/Relationship.swift @@ -402,9 +402,11 @@ extension ToManyRelationship: Codable { } let hasData = container.contains(.data) - var canHaveNoDataInRelationships: Bool = false + let canHaveNoDataInRelationships: Bool if let relatableType = Relatable.self as? ResourceObjectWithOptionalDataInRelationships.Type { canHaveNoDataInRelationships = relatableType.canHaveNoDataInRelationships + } else { + canHaveNoDataInRelationships = false } guard hasData || !canHaveNoDataInRelationships else { idsWithMeta = [] From 74c9230a2b9cce14631aadb99d9e6b4d8f758c3a Mon Sep 17 00:00:00 2001 From: Mathew Polzin Date: Fri, 1 Mar 2024 12:30:31 -0600 Subject: [PATCH 6/6] admit in test that older open source foundation implementations of the localized error are not so good --- Tests/JSONAPITests/Relationships/RelationshipTests.swift | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Tests/JSONAPITests/Relationships/RelationshipTests.swift b/Tests/JSONAPITests/Relationships/RelationshipTests.swift index d600637..3decfae 100644 --- a/Tests/JSONAPITests/Relationships/RelationshipTests.swift +++ b/Tests/JSONAPITests/Relationships/RelationshipTests.swift @@ -255,7 +255,14 @@ extension RelationshipTests { try decodedThrows(type: ToManyWithMeta.self, data: to_many_relationship_with_meta_no_data) ) { error in - XCTAssertEqual(error.localizedDescription, "The data couldn’t be read because it is missing.") + let oldLinuxFoundationMsg = "The operation could not be completed. (SwiftError error 0.)" + let newLinuxFoundationMsg = "The operation could not be completed. The data is missing." + let newDesirableMsg = "The data couldn’t be read because it is missing." + XCTAssert( + error.localizedDescription == newDesirableMsg + || error.localizedDescription == newLinuxFoundationMsg + || error.localizedDescription == oldLinuxFoundationMsg + ) } } }