Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECO-5035] Remove usage of Decodable for decoding data recieved from Realtime #200

Merged
merged 2 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 10 additions & 28 deletions Sources/AblyChat/ChatAPI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<Response: Codable>(_ url: String, method: String, body: [String: JSONValue]? = nil) async throws -> Response {
private func makeRequest<Response: JSONDecodable>(_ url: String, method: String, body: [String: JSONValue]? = nil) async throws -> Response {
let ablyCocoaBody: Any? = if let body {
JSONValue.object(body).toAblyCocoaData
} else {
Expand All @@ -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)
Expand All @@ -97,7 +102,7 @@ internal final class ChatAPI: Sendable {
}
}

private func makePaginatedRequest<Response: Codable & Sendable & Equatable>(
private func makePaginatedRequest<Response: JSONDecodable & Sendable & Equatable>(
_ url: String,
params: [String: String]? = nil
) async throws -> any PaginatedResult<Response> {
Expand All @@ -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: Decodable>(_: 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: Decodable>(_: T.Type, from dictionary: [NSDictionary]) throws -> T {
let data = try JSONSerialization.data(withJSONObject: dictionary)
return try decoder.decode(T.self, from: data)
}
}
2 changes: 1 addition & 1 deletion Sources/AblyChat/Events.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
4 changes: 2 additions & 2 deletions Sources/AblyChat/Headers.swift
Original file line number Diff line number Diff line change
@@ -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
}
Expand Down
80 changes: 75 additions & 5 deletions Sources/AblyChat/JSONCodable.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import Foundation

internal protocol JSONEncodable {
var toJSONValue: JSONValue { get }
}
Expand Down Expand Up @@ -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`
Expand All @@ -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] {
Expand All @@ -63,7 +66,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]? {
Expand Down Expand Up @@ -99,7 +102,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]? {
Expand Down Expand Up @@ -135,7 +138,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? {
Expand Down Expand Up @@ -171,7 +174,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? {
Expand Down Expand Up @@ -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<T: RawRepresentable>(_ 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<T: RawRepresentable>(_ 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<T: RawRepresentable>(_ 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
}
}
1 change: 1 addition & 0 deletions Sources/AblyChat/JSONValue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
24 changes: 14 additions & 10 deletions Sources/AblyChat/Message.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down Expand Up @@ -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) }
)
}
}
3 changes: 1 addition & 2 deletions Sources/AblyChat/Metadata.swift
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
11 changes: 10 additions & 1 deletion Sources/AblyChat/Occupancy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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"))
)
}
}
9 changes: 5 additions & 4 deletions Sources/AblyChat/PaginatedResult.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public protocol PaginatedResult<T>: 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<Response: Codable & Sendable & Equatable> {
internal struct ARTHTTPPaginatedCallbackWrapper<Response: JSONDecodable & Sendable & Equatable> {
internal let callbackResult: (ARTHTTPPaginatedResponse?, ARTErrorInfo?)

internal func handleResponse(continuation: CheckedContinuation<PaginatedResultWrapper<Response>, any Error>) {
Expand All @@ -32,7 +32,8 @@ internal struct ARTHTTPPaginatedCallbackWrapper<Response: Codable & Sendable & E
}

do {
let decodedResponse = try DictionaryDecoder().decode([Response].self, from: paginatedResponse.items)
let jsonValues = paginatedResponse.items.map { JSONValue(ablyCocoaData: $0) }
let decodedResponse = try jsonValues.map { try Response(jsonValue: $0) }
let result = paginatedResponse.toPaginatedResult(items: decodedResponse)
continuation.resume(returning: result)
} catch {
Expand All @@ -46,7 +47,7 @@ internal struct ARTHTTPPaginatedCallbackWrapper<Response: Codable & Sendable & E
}

/// `PaginatedResult` protocol implementation allowing access to the underlying items from a lower level paginated response object e.g. `ARTHTTPPaginatedResponse`, whilst succinctly handling errors through the use of `ARTHTTPPaginatedCallbackWrapper`.
internal final class PaginatedResultWrapper<T: Codable & Sendable & Equatable>: PaginatedResult {
internal final class PaginatedResultWrapper<T: JSONDecodable & Sendable & Equatable>: PaginatedResult {
internal let items: [T]
internal let hasNext: Bool
internal let isLast: Bool
Expand Down Expand Up @@ -96,7 +97,7 @@ internal final class PaginatedResultWrapper<T: Codable & Sendable & Equatable>:

private extension ARTHTTPPaginatedResponse {
/// Converts an `ARTHTTPPaginatedResponse` to a `PaginatedResultWrapper` allowing for access to operations as per conformance to `PaginatedResult`.
func toPaginatedResult<T: Codable & Sendable>(items: [T]) -> PaginatedResultWrapper<T> {
func toPaginatedResult<T: JSONDecodable & Sendable>(items: [T]) -> PaginatedResultWrapper<T> {
PaginatedResultWrapper(paginatedResponse: self, items: items)
}
}
Loading