From ace67f5eb2479b91d2e9c9176d2689ef7fbb65a5 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 17 Dec 2024 15:14:47 -0300 Subject: [PATCH 1/2] Fix documentation comments Missed some formatting in 8b8a084. --- Sources/AblyChat/JSONCodable.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/AblyChat/JSONCodable.swift b/Sources/AblyChat/JSONCodable.swift index 2704ab2..665359f 100644 --- a/Sources/AblyChat/JSONCodable.swift +++ b/Sources/AblyChat/JSONCodable.swift @@ -63,7 +63,7 @@ internal extension [String: JSONValue] { return objectValue } - /// If this dictionary contains a value for `key`, and this value has case `object`, this returns the associated value. If this dictionary does not contain a value for `key`, or if the value for key has case `null`, it returns `nil`. + /// If this dictionary contains a value for `key`, and this value has case `object`, this returns the associated value. If this dictionary does not contain a value for `key`, or if the value for `key` has case `null`, it returns `nil`. /// /// - Throws: `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `object` or `null` func optionalObjectValueForKey(_ key: String) throws -> [String: JSONValue]? { @@ -99,7 +99,7 @@ internal extension [String: JSONValue] { return arrayValue } - /// If this dictionary contains a value for `key`, and this value has case `array`, this returns the associated value. If this dictionary does not contain a value for `key`, or if the value for key has case `null`, it returns `nil`. + /// If this dictionary contains a value for `key`, and this value has case `array`, this returns the associated value. If this dictionary does not contain a value for `key`, or if the value for `key` has case `null`, it returns `nil`. /// /// - Throws: `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `array` or `null` func optionalArrayValueForKey(_ key: String) throws -> [JSONValue]? { @@ -135,7 +135,7 @@ internal extension [String: JSONValue] { return stringValue } - /// If this dictionary contains a value for `key`, and this value has case `string`, this returns the associated value. If this dictionary does not contain a value for `key`, or if the value for key has case `null`, it returns `nil`. + /// If this dictionary contains a value for `key`, and this value has case `string`, this returns the associated value. If this dictionary does not contain a value for `key`, or if the value for `key` has case `null`, it returns `nil`. /// /// - Throws: `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `string` or `null` func optionalStringValueForKey(_ key: String) throws -> String? { @@ -171,7 +171,7 @@ internal extension [String: JSONValue] { return numberValue } - /// If this dictionary contains a value for `key`, and this value has case `number`, this returns the associated value. If this dictionary does not contain a value for `key`, or if the value for key has case `null`, it returns `nil`. + /// If this dictionary contains a value for `key`, and this value has case `number`, this returns the associated value. If this dictionary does not contain a value for `key`, or if the value for `key` has case `null`, it returns `nil`. /// /// - Throws: `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `number` or `null` func optionalNumberValueForKey(_ key: String) throws -> Double? { From 526a220a523434b162f1979f650444bb0bc77558 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 12 Dec 2024 11:12:55 -0300 Subject: [PATCH 2/2] Remove usage of Decodable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch to using JSONDecodable internally. This allows us to avoid the dance of encoding an ably-cocoa object to a Data just so that we can decode it using JSONDecoder. It also gives us consistency in how we handle data objects received from ably-cocoa. This means that we now have to write a bit more code in order to decode objects manually, but I think we can live with it. (It might also be that we can revisit in the future and either automatically generate the JSONDecodable conformance using macros, or that we can find some other, better way of bridging with the compiler’s synthesised Decodable conformance.) Resolves #84. --- Sources/AblyChat/ChatAPI.swift | 38 ++++---------- Sources/AblyChat/Events.swift | 2 +- Sources/AblyChat/Headers.swift | 4 +- Sources/AblyChat/JSONCodable.swift | 72 +++++++++++++++++++++++++- Sources/AblyChat/JSONValue.swift | 1 + Sources/AblyChat/Message.swift | 24 +++++---- Sources/AblyChat/Metadata.swift | 3 +- Sources/AblyChat/Occupancy.swift | 11 +++- Sources/AblyChat/PaginatedResult.swift | 9 ++-- 9 files changed, 115 insertions(+), 49 deletions(-) diff --git a/Sources/AblyChat/ChatAPI.swift b/Sources/AblyChat/ChatAPI.swift index 55bf7db..e439dcc 100644 --- a/Sources/AblyChat/ChatAPI.swift +++ b/Sources/AblyChat/ChatAPI.swift @@ -15,9 +15,14 @@ internal final class ChatAPI: Sendable { return try await makePaginatedRequest(endpoint, params: params.asQueryItems()) } - internal struct SendMessageResponse: Codable { + internal struct SendMessageResponse: JSONObjectDecodable { internal let serial: String internal let createdAt: Int64 + + internal init(jsonObject: [String: JSONValue]) throws { + serial = try jsonObject.stringValueForKey("serial") + createdAt = try Int64(jsonObject.numberValueForKey("createdAt")) + } } // (CHA-M3) Messages are sent to Ably via the Chat REST API, using the send method. @@ -62,8 +67,7 @@ internal final class ChatAPI: Sendable { return try await makeRequest(endpoint, method: "GET") } - // TODO: https://github.com/ably-labs/ably-chat-swift/issues/84 - Improve how we're decoding via `JSONSerialization` within the `DictionaryDecoder` - private func makeRequest(_ url: String, method: String, body: [String: JSONValue]? = nil) async throws -> Response { + private func makeRequest(_ url: String, method: String, body: [String: JSONValue]? = nil) async throws -> Response { let ablyCocoaBody: Any? = if let body { JSONValue.object(body).toAblyCocoaData } else { @@ -85,7 +89,8 @@ internal final class ChatAPI: Sendable { } do { - let decodedResponse = try DictionaryDecoder().decode(Response.self, from: firstItem) + let jsonValue = JSONValue(ablyCocoaData: firstItem) + let decodedResponse = try Response(jsonValue: jsonValue) continuation.resume(returning: decodedResponse) } catch { continuation.resume(throwing: error) @@ -97,7 +102,7 @@ internal final class ChatAPI: Sendable { } } - private func makePaginatedRequest( + private func makePaginatedRequest( _ url: String, params: [String: String]? = nil ) async throws -> any PaginatedResult { @@ -116,26 +121,3 @@ internal final class ChatAPI: Sendable { case noItemInResponse } } - -internal struct DictionaryDecoder { - private let decoder = { - var decoder = JSONDecoder() - - // Ably’s REST APIs always serialise dates as milliseconds since Unix epoch - decoder.dateDecodingStrategy = .millisecondsSince1970 - - return decoder - }() - - // Function to decode from a dictionary - internal func decode(_: T.Type, from dictionary: NSDictionary) throws -> T { - let data = try JSONSerialization.data(withJSONObject: dictionary) - return try decoder.decode(T.self, from: data) - } - - // Function to decode from a dictionary array - internal func decode(_: T.Type, from dictionary: [NSDictionary]) throws -> T { - let data = try JSONSerialization.data(withJSONObject: dictionary) - return try decoder.decode(T.self, from: data) - } -} diff --git a/Sources/AblyChat/Events.swift b/Sources/AblyChat/Events.swift index 56c182c..9f87bbc 100644 --- a/Sources/AblyChat/Events.swift +++ b/Sources/AblyChat/Events.swift @@ -3,7 +3,7 @@ import Ably /** * Chat Message Actions. */ -public enum MessageAction: String, Codable, Sendable { +public enum MessageAction: String, Sendable { /** * Action applied to a new message. */ diff --git a/Sources/AblyChat/Headers.swift b/Sources/AblyChat/Headers.swift index bc52cd6..edf5400 100644 --- a/Sources/AblyChat/Headers.swift +++ b/Sources/AblyChat/Headers.swift @@ -1,8 +1,8 @@ // TODO: https://github.com/ably-labs/ably-chat-swift/issues/13 - try to improve this type -public enum HeadersValue: Sendable, Codable, Equatable { +public enum HeadersValue: Sendable, Equatable { case string(String) - case number(Double) // Changed from NSNumber to Double to conform to Codable. Address in linked issue above. + case number(Double) case bool(Bool) case null } diff --git a/Sources/AblyChat/JSONCodable.swift b/Sources/AblyChat/JSONCodable.swift index 665359f..cbda266 100644 --- a/Sources/AblyChat/JSONCodable.swift +++ b/Sources/AblyChat/JSONCodable.swift @@ -1,3 +1,5 @@ +import Foundation + internal protocol JSONEncodable { var toJSONValue: JSONValue { get } } @@ -27,6 +29,7 @@ internal enum JSONValueDecodingError: Error { case valueIsNotObject case noValueForKey(String) case wrongTypeForKey(String, actualValue: JSONValue) + case failedToDecodeFromRawValue(String) } // Default implementation of `JSONDecodable` conformance for `JSONObjectDecodable` @@ -42,7 +45,7 @@ internal extension JSONObjectDecodable { internal typealias JSONObjectCodable = JSONObjectDecodable & JSONObjectEncodable -// MARK: - Extracting values from a dictionary +// MARK: - Extracting primitive values from a dictionary /// This extension adds some helper methods for extracting values from a dictionary of `JSONValue` values; you may find them helpful when implementing `JSONCodable`. internal extension [String: JSONValue] { @@ -228,3 +231,70 @@ internal extension [String: JSONValue] { return boolValue } } + +// MARK: - Extracting dates from a dictionary + +internal extension [String: JSONValue] { + /// If this dictionary contains a value for `key`, and this value has case `number`, this returns a date created by interpreting this value as the number of milliseconds since the Unix epoch (which is the format used by Ably). + /// + /// - Throws: + /// - `JSONValueDecodingError.noValueForKey` if the key is absent + /// - `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `number` + func ablyProtocolDateValueForKey(_ key: String) throws -> Date { + let millisecondsSinceEpoch = try numberValueForKey(key) + + return dateFromMillisecondsSinceEpoch(millisecondsSinceEpoch) + } + + /// If this dictionary contains a value for `key`, and this value has case `number`, this returns a date created by interpreting this value as the number of milliseconds since the Unix epoch (which is the format used by Ably). If this dictionary does not contain a value for `key`, or if the value for `key` has case `null`, it returns `nil`. + /// + /// - Throws: `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `number` or `null` + func optionalAblyProtocolDateValueForKey(_ key: String) throws -> Date? { + guard let millisecondsSinceEpoch = try optionalNumberValueForKey(key) else { + return nil + } + + return dateFromMillisecondsSinceEpoch(millisecondsSinceEpoch) + } + + private func dateFromMillisecondsSinceEpoch(_ millisecondsSinceEpoch: Double) -> Date { + .init(timeIntervalSince1970: millisecondsSinceEpoch / 1000) + } +} + +// MARK: - Extracting RawRepresentable values from a dictionary + +internal extension [String: JSONValue] { + /// If this dictionary contains a value for `key`, and this value has case `string`, this creates an instance of `T` using its `init(rawValue:)` initializer. + /// + /// - Throws: + /// - `JSONValueDecodingError.noValueForKey` if the key is absent + /// - `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `string` + /// - `JSONValueDecodingError.failedToDecodeFromRawValue` if `init(rawValue:)` returns `nil` + func rawRepresentableValueForKey(_ key: String, type: T.Type = T.self) throws -> T where T.RawValue == String { + let rawValue = try stringValueForKey(key) + + return try rawRepresentableValueFromRawValue(rawValue, type: T.self) + } + + /// If this dictionary contains a value for `key`, and this value has case `string`, this creates an instance of `T` using its `init(rawValue:)` initializer. If this dictionary does not contain a value for `key`, or if the value for `key` has case `null`, it returns `nil`. + /// + /// - Throws: + /// - `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `string` or `null` + /// - `JSONValueDecodingError.failedToDecodeFromRawValue` if `init(rawValue:)` returns `nil` + func optionalRawRepresentableValueForKey(_ key: String, type: T.Type = T.self) throws -> T? where T.RawValue == String { + guard let rawValue = try optionalStringValueForKey(key) else { + return nil + } + + return try rawRepresentableValueFromRawValue(rawValue, type: T.self) + } + + private func rawRepresentableValueFromRawValue(_ rawValue: String, type _: T.Type = T.self) throws -> T where T.RawValue == String { + guard let value = T(rawValue: rawValue) else { + throw JSONValueDecodingError.failedToDecodeFromRawValue(rawValue) + } + + return value + } +} diff --git a/Sources/AblyChat/JSONValue.swift b/Sources/AblyChat/JSONValue.swift index 843dcf3..8d53417 100644 --- a/Sources/AblyChat/JSONValue.swift +++ b/Sources/AblyChat/JSONValue.swift @@ -136,6 +136,7 @@ internal extension JSONValue { /// - a non-`nil` value of `ARTPresenceMessage`’s `data` property /// - a non-`nil` value of `ARTMessage`’s `data` property /// - the return value of the `toJSON()` method of a non-`nil` value of `ARTMessage`’s `extras` property + /// - an element of `ARTHTTPPaginatedResult`’s `items` array init(ablyCocoaData: Any) { switch ablyCocoaData { case let dictionary as [String: Any]: diff --git a/Sources/AblyChat/Message.swift b/Sources/AblyChat/Message.swift index 9017313..d1dc21d 100644 --- a/Sources/AblyChat/Message.swift +++ b/Sources/AblyChat/Message.swift @@ -13,7 +13,7 @@ public typealias MessageMetadata = Metadata /** * Represents a single message in a chat room. */ -public struct Message: Sendable, Codable, Identifiable, Equatable { +public struct Message: Sendable, Identifiable, Equatable { // id to meet Identifiable conformance. 2 messages in the same channel cannot have the same serial. public var id: String { serial } @@ -87,15 +87,19 @@ public struct Message: Sendable, Codable, Identifiable, Equatable { self.metadata = metadata self.headers = headers } +} - internal enum CodingKeys: String, CodingKey { - case serial - case action - case clientID = "clientId" - case roomID = "roomId" - case text - case createdAt - case metadata - case headers +extension Message: JSONObjectDecodable { + internal init(jsonObject: [String: JSONValue]) throws { + try self.init( + serial: jsonObject.stringValueForKey("serial"), + action: jsonObject.rawRepresentableValueForKey("action"), + clientID: jsonObject.stringValueForKey("clientId"), + roomID: jsonObject.stringValueForKey("roomId"), + text: jsonObject.stringValueForKey("text"), + createdAt: jsonObject.optionalAblyProtocolDateValueForKey("createdAt"), + metadata: jsonObject.objectValueForKey("metadata").mapValues { try .init(jsonValue: $0) }, + headers: jsonObject.objectValueForKey("headers").mapValues { try .init(jsonValue: $0) } + ) } } diff --git a/Sources/AblyChat/Metadata.swift b/Sources/AblyChat/Metadata.swift index e78e83e..591c0a1 100644 --- a/Sources/AblyChat/Metadata.swift +++ b/Sources/AblyChat/Metadata.swift @@ -1,7 +1,6 @@ // TODO: https://github.com/ably-labs/ably-chat-swift/issues/13 - try to improve this type -// I attempted to address this issue by making a struct conforming to Codable which would at least give us some safety in knowing items can be encoded and decoded. Gave up on it due to fixing other protocol requirements so gone for the same approach as Headers for now, we can investigate whether we need to be open to more types than this later. -public enum MetadataValue: Sendable, Codable, Equatable { +public enum MetadataValue: Sendable, Equatable { case string(String) case number(Double) case bool(Bool) diff --git a/Sources/AblyChat/Occupancy.swift b/Sources/AblyChat/Occupancy.swift index 3779fd4..8d1bb75 100644 --- a/Sources/AblyChat/Occupancy.swift +++ b/Sources/AblyChat/Occupancy.swift @@ -48,7 +48,7 @@ public extension Occupancy { /** * Represents the occupancy of a chat room. */ -public struct OccupancyEvent: Sendable, Encodable, Decodable { +public struct OccupancyEvent: Sendable { /** * The number of connections to the chat room. */ @@ -64,3 +64,12 @@ public struct OccupancyEvent: Sendable, Encodable, Decodable { self.presenceMembers = presenceMembers } } + +extension OccupancyEvent: JSONObjectDecodable { + internal init(jsonObject: [String: JSONValue]) throws { + try self.init( + connections: Int(jsonObject.numberValueForKey("connections")), + presenceMembers: Int(jsonObject.numberValueForKey("presenceMembers")) + ) + } +} diff --git a/Sources/AblyChat/PaginatedResult.swift b/Sources/AblyChat/PaginatedResult.swift index ce7491a..ff603a1 100644 --- a/Sources/AblyChat/PaginatedResult.swift +++ b/Sources/AblyChat/PaginatedResult.swift @@ -13,7 +13,7 @@ public protocol PaginatedResult: AnyObject, Sendable, Equatable { } /// Used internally to reduce the amount of duplicate code when interacting with `ARTHTTPPaginatedCallback`'s. The wrapper takes in the callback result from the caller e.g. `realtime.request` and either throws the appropriate error, or decodes and returns the response. -internal struct ARTHTTPPaginatedCallbackWrapper { +internal struct ARTHTTPPaginatedCallbackWrapper { internal let callbackResult: (ARTHTTPPaginatedResponse?, ARTErrorInfo?) internal func handleResponse(continuation: CheckedContinuation, any Error>) { @@ -32,7 +32,8 @@ internal struct ARTHTTPPaginatedCallbackWrapper: PaginatedResult { +internal final class PaginatedResultWrapper: PaginatedResult { internal let items: [T] internal let hasNext: Bool internal let isLast: Bool @@ -96,7 +97,7 @@ internal final class PaginatedResultWrapper: private extension ARTHTTPPaginatedResponse { /// Converts an `ARTHTTPPaginatedResponse` to a `PaginatedResultWrapper` allowing for access to operations as per conformance to `PaginatedResult`. - func toPaginatedResult(items: [T]) -> PaginatedResultWrapper { + func toPaginatedResult(items: [T]) -> PaginatedResultWrapper { PaginatedResultWrapper(paginatedResponse: self, items: items) } }