Merge pull request #199 from ably/198-reaction-headers-and-metadata

[ECO-5181] Fix sending and receiving of reaction metadata and headers

Author: lawrence-forooghian

Sources/AblyChat/DefaultMessages.swift

@@ -77,12 +77,12 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities {
                     throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without clientId")
                 }
 
-                let metadata: Metadata? = if let metadataJSONObject = data["metadata"]?.objectValue {
+                let metadata: Metadata? = if let metadataJSONObject = try data.optionalObjectValueForKey("metadata") {
                     try metadataJSONObject.mapValues { try MetadataValue(jsonValue: $0) }
                 } else {
                     nil
                 }
-                let headers: Headers? = if let headersJSONObject = extras["headers"]?.objectValue {
+                let headers: Headers? = if let headersJSONObject = try extras.optionalObjectValueForKey("headers") {
                     try headersJSONObject.mapValues { try HeadersValue(jsonValue: $0) }
                 } else {
                     nil

Sources/AblyChat/DefaultRoomReactions.swift

@@ -23,8 +23,14 @@ internal final class DefaultRoomReactions: RoomReactions, EmitsDiscontinuities {
     // (CHA-ER3a) Reactions are sent on the channel using a message in a particular format - see spec for format.
     internal func send(params: SendReactionParams) async throws {
         logger.log(message: "Sending reaction with params: \(params)", level: .debug)
-        let extras = ["headers": params.headers ?? [:]] as ARTJsonCompatible
-        channel.publish(RoomReactionEvents.reaction.rawValue, data: params.asJSONObject(), extras: extras)
+
+        let dto = RoomReactionDTO(type: params.type, metadata: params.metadata, headers: params.headers)
+
+        channel.publish(
+            RoomReactionEvents.reaction.rawValue,
+            data: dto.data.toJSONValue.toAblyCocoaData,
+            extras: dto.extras.toJSONObject.toARTJsonCompatible
+        )
     }
 
     // (CHA-ER4) A user may subscribe to reaction events in Realtime.
@@ -38,10 +44,8 @@ internal final class DefaultRoomReactions: RoomReactions, EmitsDiscontinuities {
             logger.log(message: "Received roomReaction message: \(message)", level: .debug)
             Task {
                 do {
-                    guard let data = message.data as? [String: Any],
-                          let reactionType = data["type"] as? String
-                    else {
-                        throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without data or text")
+                    guard let ablyCocoaData = message.data else {
+                        throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without data")
                     }
 
                     guard let messageClientID = message.clientId else {
@@ -52,18 +56,20 @@ internal final class DefaultRoomReactions: RoomReactions, EmitsDiscontinuities {
                         throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without timestamp")
                     }
 
-                    guard let extras = try message.extras?.toJSON() else {
+                    guard let ablyCocoaExtras = try message.extras?.toJSON() else {
                         throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without extras")
                     }
 
-                    let metadata = data["metadata"] as? Metadata
-                    let headers = extras["headers"] as? Headers
+                    let dto = try RoomReactionDTO(
+                        data: .init(jsonValue: .init(ablyCocoaData: ablyCocoaData)),
+                        extras: .init(jsonValue: .init(ablyCocoaData: ablyCocoaExtras))
+                    )
 
                     // (CHA-ER4d) Realtime events that are malformed (unknown fields should be ignored) shall not be emitted to listeners.
                     let reaction = Reaction(
-                        type: reactionType,
-                        metadata: metadata ?? .init(),
-                        headers: headers ?? .init(),
+                        type: dto.type,
+                        metadata: dto.metadata ?? [:],
+                        headers: dto.headers ?? [:],
                         createdAt: timestamp,
                         clientID: messageClientID,
                         isSelf: messageClientID == clientID

Sources/AblyChat/JSONCodable.swift

@@ -7,3 +7,224 @@ internal protocol JSONDecodable {
 }
 
 internal typealias JSONCodable = JSONDecodable & JSONEncodable
+
+internal protocol JSONObjectEncodable: JSONEncodable {
+    var toJSONObject: [String: JSONValue] { get }
+}
+
+// Default implementation of `JSONEncodable` conformance for `JSONObjectEncodable`
+internal extension JSONObjectEncodable {
+    var toJSONValue: JSONValue {
+        .object(toJSONObject)
+    }
+}
+
+internal protocol JSONObjectDecodable: JSONDecodable {
+    init(jsonObject: [String: JSONValue]) throws
+}
+
+internal enum JSONValueDecodingError: Error {
+    case valueIsNotObject
+    case noValueForKey(String)
+    case wrongTypeForKey(String, actualValue: JSONValue)
+}
+
+// Default implementation of `JSONDecodable` conformance for `JSONObjectDecodable`
+internal extension JSONObjectDecodable {
+    init(jsonValue: JSONValue) throws {
+        guard case let .object(jsonObject) = jsonValue else {
+            throw JSONValueDecodingError.valueIsNotObject
+        }
+
+        self = try .init(jsonObject: jsonObject)
+    }
+}
+
+internal typealias JSONObjectCodable = JSONObjectDecodable & JSONObjectEncodable
+
+// MARK: - Extracting 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] {
+    /// If this dictionary contains a value for `key`, and this value has case `object`, this returns the associated value.
+    ///
+    /// - Throws:
+    ///   - `JSONValueDecodingError.noValueForKey` if the key is absent
+    ///   - `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `object`
+    func objectValueForKey(_ key: String) throws -> [String: JSONValue] {
+        guard let value = self[key] else {
+            throw JSONValueDecodingError.noValueForKey(key)
+        }
+
+        guard case let .object(objectValue) = value else {
+            throw JSONValueDecodingError.wrongTypeForKey(key, actualValue: value)
+        }
+
+        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`.
+    ///
+    /// - Throws: `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `object` or `null`
+    func optionalObjectValueForKey(_ key: String) throws -> [String: JSONValue]? {
+        guard let value = self[key] else {
+            return nil
+        }
+
+        if case .null = value {
+            return nil
+        }
+
+        guard case let .object(objectValue) = value else {
+            throw JSONValueDecodingError.wrongTypeForKey(key, actualValue: value)
+        }
+
+        return objectValue
+    }
+
+    /// If this dictionary contains a value for `key`, and this value has case `array`, this returns the associated value.
+    ///
+    /// - Throws:
+    ///   - `JSONValueDecodingError.noValueForKey` if the key is absent
+    ///   - `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `array`
+    func arrayValueForKey(_ key: String) throws -> [JSONValue] {
+        guard let value = self[key] else {
+            throw JSONValueDecodingError.noValueForKey(key)
+        }
+
+        guard case let .array(arrayValue) = value else {
+            throw JSONValueDecodingError.wrongTypeForKey(key, actualValue: value)
+        }
+
+        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`.
+    ///
+    /// - Throws: `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `array` or `null`
+    func optionalArrayValueForKey(_ key: String) throws -> [JSONValue]? {
+        guard let value = self[key] else {
+            return nil
+        }
+
+        if case .null = value {
+            return nil
+        }
+
+        guard case let .array(arrayValue) = value else {
+            throw JSONValueDecodingError.wrongTypeForKey(key, actualValue: value)
+        }
+
+        return arrayValue
+    }
+
+    /// If this dictionary contains a value for `key`, and this value has case `string`, this returns the associated value.
+    ///
+    /// - Throws:
+    ///   - `JSONValueDecodingError.noValueForKey` if the key is absent
+    ///   - `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `string`
+    func stringValueForKey(_ key: String) throws -> String {
+        guard let value = self[key] else {
+            throw JSONValueDecodingError.noValueForKey(key)
+        }
+
+        guard case let .string(stringValue) = value else {
+            throw JSONValueDecodingError.wrongTypeForKey(key, actualValue: value)
+        }
+
+        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`.
+    ///
+    /// - Throws: `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `string` or `null`
+    func optionalStringValueForKey(_ key: String) throws -> String? {
+        guard let value = self[key] else {
+            return nil
+        }
+
+        if case .null = value {
+            return nil
+        }
+
+        guard case let .string(stringValue) = value else {
+            throw JSONValueDecodingError.wrongTypeForKey(key, actualValue: value)
+        }
+
+        return stringValue
+    }
+
+    /// If this dictionary contains a value for `key`, and this value has case `number`, this returns the associated value.
+    ///
+    /// - Throws:
+    ///   - `JSONValueDecodingError.noValueForKey` if the key is absent
+    ///   - `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `number`
+    func numberValueForKey(_ key: String) throws -> Double {
+        guard let value = self[key] else {
+            throw JSONValueDecodingError.noValueForKey(key)
+        }
+
+        guard case let .number(numberValue) = value else {
+            throw JSONValueDecodingError.wrongTypeForKey(key, actualValue: value)
+        }
+
+        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`.
+    ///
+    /// - Throws: `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `number` or `null`
+    func optionalNumberValueForKey(_ key: String) throws -> Double? {
+        guard let value = self[key] else {
+            return nil
+        }
+
+        if case .null = value {
+            return nil
+        }
+
+        guard case let .number(numberValue) = value else {
+            throw JSONValueDecodingError.wrongTypeForKey(key, actualValue: value)
+        }
+
+        return numberValue
+    }
+
+    /// If this dictionary contains a value for `key`, and this value has case `bool`, this returns the associated value.
+    ///
+    /// - Throws:
+    ///   - `JSONValueDecodingError.noValueForKey` if the key is absent
+    ///   - `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `bool`
+    func boolValueForKey(_ key: String) throws -> Bool {
+        guard let value = self[key] else {
+            throw JSONValueDecodingError.noValueForKey(key)
+        }
+
+        guard case let .bool(boolValue) = value else {
+            throw JSONValueDecodingError.wrongTypeForKey(key, actualValue: value)
+        }
+
+        return boolValue
+    }
+
+    /// If this dictionary contains a value for `key`, and this value has case `bool`, this returns the associated value.
+    ///
+    /// - Throws:
+    ///   - `JSONValueDecodingError.noValueForKey` if the key is absent
+    ///   - `JSONValueDecodingError.wrongTypeForKey` if the value does not have case `bool`
+    func optionalBoolValueForKey(_ key: String) throws -> Bool? {
+        guard let value = self[key] else {
+            return nil
+        }
+
+        if case .null = value {
+            return nil
+        }
+
+        guard case let .bool(boolValue) = value else {
+            throw JSONValueDecodingError.wrongTypeForKey(key, actualValue: value)
+        }
+
+        return boolValue
+    }
+}

Sources/AblyChat/JSONValue.swift

@@ -1,3 +1,4 @@
+import Ably
 import Foundation
 
 /// A JSON value (where "value" has the meaning defined by the [JSON specification](https://www.json.org)).
@@ -166,10 +167,11 @@ internal extension JSONValue {
     ///
     /// - `ARTPresenceMessage`’s `data` property
     /// - the `data` argument that’s passed to `ARTRealtime`’s `request(…)` method
+    /// - the `data` argument that’s passed to `ARTRealtime`’s `publish(…)` method
     var toAblyCocoaData: Any {
         switch self {
         case let .object(underlying):
-            underlying.mapValues(\.toAblyCocoaData)
+            underlying.toAblyCocoaDataDictionary
         case let .array(underlying):
             underlying.map(\.toAblyCocoaData)
         case let .string(underlying):
@@ -183,3 +185,21 @@ internal extension JSONValue {
         }
     }
 }
+
+internal extension [String: JSONValue] {
+    /// Creates an ably-cocoa deserialized JSON object from a dictionary that has string keys and `JSONValue` values.
+    ///
+    /// Specifically, the value of this property can be used as:
+    ///
+    /// - `ARTPresenceMessage`’s `data` property
+    /// - the `data` argument that’s passed to `ARTRealtime`’s `request(…)` method
+    /// - the `data` argument that’s passed to `ARTRealtime`’s `publish(…)` method
+    var toAblyCocoaDataDictionary: [String: Any] {
+        mapValues(\.toAblyCocoaData)
+    }
+
+    /// Creates an ably-cocoa `ARTJsonCompatible` object from a dictionary that has string keys and `JSONValue` values.
+    var toARTJsonCompatible: ARTJsonCompatible {
+        toAblyCocoaDataDictionary as ARTJsonCompatible
+    }
+}

Sources/AblyChat/PresenceDataDTO.swift

@@ -3,32 +3,24 @@ internal struct PresenceDataDTO: Equatable {
     internal var userCustomData: PresenceData?
 }
 
-// MARK: - JSONCodable
+// MARK: - JSONObjectCodable
 
-extension PresenceDataDTO: JSONCodable {
+extension PresenceDataDTO: JSONObjectCodable {
     internal enum JSONKey: String {
         case userCustomData
     }
 
-    internal enum DecodingError: Error {
-        case valueHasWrongType(key: JSONKey)
-    }
-
-    internal init(jsonValue: JSONValue) throws {
-        guard case let .object(jsonObject) = jsonValue else {
-            throw DecodingError.valueHasWrongType(key: .userCustomData)
-        }
-
+    internal init(jsonObject: [String: JSONValue]) throws {
         userCustomData = jsonObject[JSONKey.userCustomData.rawValue]
     }
 
-    internal var toJSONValue: JSONValue {
+    internal var toJSONObject: [String: JSONValue] {
         var result: [String: JSONValue] = [:]
 
         if let userCustomData {
             result[JSONKey.userCustomData.rawValue] = userCustomData
         }
 
-        return .object(result)
+        return result
     }
 }