Merge pull request #196 from ably/193-fix-message-metadata-and-headers

[ECO-5178] Fix sending and receiving of message headers and metadata

Sources/AblyChat/ChatAPI.swift

@@ -28,15 +28,15 @@ internal final class ChatAPI: Sendable {
         }
 
         let endpoint = "\(apiVersionV2)/rooms/\(roomId)/messages"
-        var body: [String: Any] = ["text": params.text]
+        var body: [String: JSONValue] = ["text": .string(params.text)]
 
         // (CHA-M3b) A message may be sent without metadata or headers. When these are not specified by the user, they must be omitted from the REST payload.
         if let metadata = params.metadata {
-            body["metadata"] = metadata
+            body["metadata"] = .object(metadata.mapValues(\.toJSONValue))
         }
 
         if let headers = params.headers {
-            body["headers"] = headers
+            body["headers"] = .object(headers.mapValues(\.toJSONValue))
         }
 
         let response: SendMessageResponse = try await makeRequest(endpoint, method: "POST", body: body)
@@ -63,10 +63,16 @@ internal final class ChatAPI: Sendable {
     }
 
     // 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: Any]? = nil) async throws -> Response {
-        try await withCheckedThrowingContinuation { continuation in
+    private func makeRequest<Response: Codable>(_ url: String, method: String, body: [String: JSONValue]? = nil) async throws -> Response {
+        let ablyCocoaBody: Any? = if let body {
+            JSONValue.object(body).toAblyCocoaData
+        } else {
+            nil
+        }
+
+        return try await withCheckedThrowingContinuation { continuation in
             do {
-                try realtime.request(method, path: url, params: [:], body: body, headers: [:]) { paginatedResponse, error in
+                try realtime.request(method, path: url, params: [:], body: ablyCocoaBody, headers: [:]) { paginatedResponse, error in
                     if let error {
                         // (CHA-M3e) If an error is returned from the REST API, its ErrorInfo representation shall be thrown as the result of the send call.
                         continuation.resume(throwing: ARTErrorInfo.create(from: error))
@@ -93,8 +99,7 @@ internal final class ChatAPI: Sendable {
 
     private func makePaginatedRequest<Response: Codable & Sendable & Equatable>(
         _ url: String,
-        params: [String: String]? = nil,
-        body: [String: Any]? = nil
+        params: [String: String]? = nil
     ) async throws -> any PaginatedResult<Response> {
         try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<PaginatedResultWrapper<Response>, _>) in
             do {

Sources/AblyChat/DefaultMessages.swift

@@ -56,13 +56,16 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities {
         channel.subscribe(RealtimeMessageName.chatMessage.rawValue) { message in
             Task {
                 // TODO: Revisit errors thrown as part of https://github.com/ably-labs/ably-chat-swift/issues/32
-                guard let data = message.data as? [String: Any],
-                      let text = data["text"] as? String
+                guard let ablyCocoaData = message.data,
+                      let data = JSONValue(ablyCocoaData: ablyCocoaData).objectValue,
+                      let text = data["text"]?.stringValue
                 else {
                     throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without data or text")
                 }
 
-                guard let extras = try message.extras?.toJSON() else {
+                guard let ablyCocoaExtras = message.extras,
+                      let extras = try JSONValue(ablyCocoaData: ablyCocoaExtras.toJSON()).objectValue
+                else {
                     throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without extras")
                 }
 
@@ -74,8 +77,16 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities {
                     throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without clientId")
                 }
 
-                let metadata = data["metadata"] as? Metadata
-                let headers = extras["headers"] as? Headers
+                let metadata: Metadata? = if let metadataJSONObject = data["metadata"]?.objectValue {
+                    try metadataJSONObject.mapValues { try MetadataValue(jsonValue: $0) }
+                } else {
+                    nil
+                }
+                let headers: Headers? = if let headersJSONObject = extras["headers"]?.objectValue {
+                    try headersJSONObject.mapValues { try HeadersValue(jsonValue: $0) }
+                } else {
+                    nil
+                }
 
                 guard let action = MessageAction.fromRealtimeAction(message.action) else {
                     return

Sources/AblyChat/DefaultPresence.swift

@@ -115,7 +115,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
         let dto = PresenceDataDTO(userCustomData: data)
 
         return try await withCheckedThrowingContinuation { continuation in
-            channel.presence.enterClient(clientID, data: JSONValue.object(dto.toJSONObjectValue).toAblyCocoaPresenceData) { [logger] error in
+            channel.presence.enterClient(clientID, data: dto.toJSONValue.toAblyCocoaData) { [logger] error in
                 if let error {
                     logger.log(message: "Error entering presence: \(error)", level: .error)
                     continuation.resume(throwing: error)
@@ -149,7 +149,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
         let dto = PresenceDataDTO(userCustomData: data)
 
         return try await withCheckedThrowingContinuation { continuation in
-            channel.presence.update(JSONValue.object(dto.toJSONObjectValue).toAblyCocoaPresenceData) { [logger] error in
+            channel.presence.update(dto.toJSONValue.toAblyCocoaData) { [logger] error in
                 if let error {
                     logger.log(message: "Error updating presence: \(error)", level: .error)
                     continuation.resume(throwing: error)
@@ -183,7 +183,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
         let dto = PresenceDataDTO(userCustomData: data)
 
         return try await withCheckedThrowingContinuation { continuation in
-            channel.presence.leave(JSONValue.object(dto.toJSONObjectValue).toAblyCocoaPresenceData) { [logger] error in
+            channel.presence.leave(dto.toJSONValue.toAblyCocoaData) { [logger] error in
                 if let error {
                     logger.log(message: "Error leaving presence: \(error)", level: .error)
                     continuation.resume(throwing: error)
@@ -237,7 +237,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
             throw error
         }
 
-        let jsonValue = JSONValue(ablyCocoaPresenceData: ablyCocoaPresenceData)
+        let jsonValue = JSONValue(ablyCocoaData: ablyCocoaPresenceData)
 
         do {
             return try PresenceDataDTO(jsonValue: jsonValue)

Sources/AblyChat/Headers.swift

@@ -2,11 +2,47 @@
 
 public enum HeadersValue: Sendable, Codable, Equatable {
     case string(String)
-    case number(Int) // Changed from NSNumber to Int to conform to Codable. Address in linked issue above.
+    case number(Double) // Changed from NSNumber to Double to conform to Codable. Address in linked issue above.
     case bool(Bool)
     case null
 }
 
+extension HeadersValue: JSONDecodable {
+    internal enum JSONDecodingError: Error {
+        case unsupportedJSONValue(JSONValue)
+    }
+
+    internal init(jsonValue: JSONValue) throws {
+        self = switch jsonValue {
+        case let .string(value):
+            .string(value)
+        case let .number(value):
+            .number(value)
+        case let .bool(value):
+            .bool(value)
+        case .null:
+            .null
+        default:
+            throw JSONDecodingError.unsupportedJSONValue(jsonValue)
+        }
+    }
+}
+
+extension HeadersValue: JSONEncodable {
+    internal var toJSONValue: JSONValue {
+        switch self {
+        case let .string(value):
+            .string(value)
+        case let .number(value):
+            .number(Double(value))
+        case let .bool(value):
+            .bool(value)
+        case .null:
+            .null
+        }
+    }
+}
+
 // The corresponding type in TypeScript is
 // Record<string, number | string | boolean | null | undefined>
 // There may be a better way to represent it in Swift; this will do for now. Have omitted `undefined` because I don’t know how that would occur.

Sources/AblyChat/JSONCodable.swift

@@ -0,0 +1,9 @@
+internal protocol JSONEncodable {
+    var toJSONValue: JSONValue { get }
+}
+
+internal protocol JSONDecodable {
+    init(jsonValue: JSONValue) throws
+}
+
+internal typealias JSONCodable = JSONDecodable & JSONEncodable

Sources/AblyChat/JSONValue.swift

@@ -128,12 +128,19 @@ extension JSONValue: ExpressibleByBooleanLiteral {
 // MARK: - Bridging with ably-cocoa
 
 internal extension JSONValue {
-    init(ablyCocoaPresenceData: Any) {
-        switch ablyCocoaPresenceData {
+    /// Creates a `JSONValue` from an ably-cocoa deserialized JSON object.
+    ///
+    /// Specifically, `ablyCocoaData` can be:
+    ///
+    /// - 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
+    init(ablyCocoaData: Any) {
+        switch ablyCocoaData {
         case let dictionary as [String: Any]:
-            self = .object(dictionary.mapValues { .init(ablyCocoaPresenceData: $0) })
+            self = .object(dictionary.mapValues { .init(ablyCocoaData: $0) })
         case let array as [Any]:
-            self = .array(array.map { .init(ablyCocoaPresenceData: $0) })
+            self = .array(array.map { .init(ablyCocoaData: $0) })
         case let string as String:
             self = .string(string)
         case let number as NSNumber:
@@ -149,16 +156,22 @@ internal extension JSONValue {
             self = .null
         default:
             // ably-cocoa is not conforming to our assumptions; either its behaviour is wrong or our assumptions are wrong. Either way, bring this loudly to our attention instead of trying to carry on
-            preconditionFailure("JSONValue(ablyCocoaPresenceData:) was given \(ablyCocoaPresenceData)")
+            preconditionFailure("JSONValue(ablyCocoaData:) was given \(ablyCocoaData)")
         }
     }
 
-    var toAblyCocoaPresenceData: Any {
+    /// Creates an ably-cocoa deserialized JSON object from a `JSONValue`.
+    ///
+    /// 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
+    var toAblyCocoaData: Any {
         switch self {
         case let .object(underlying):
-            underlying.mapValues(\.toAblyCocoaPresenceData)
+            underlying.mapValues(\.toAblyCocoaData)
         case let .array(underlying):
-            underlying.map(\.toAblyCocoaPresenceData)
+            underlying.map(\.toAblyCocoaData)
         case let .string(underlying):
             underlying
         case let .number(underlying):

Sources/AblyChat/Metadata.swift

@@ -3,9 +3,45 @@
 
 public enum MetadataValue: Sendable, Codable, Equatable {
     case string(String)
-    case number(Int) // Changed from NSNumber to Int to conform to Codable. Address in linked issue above.
+    case number(Double)
     case bool(Bool)
     case null
 }
 
-public typealias Metadata = [String: MetadataValue?]
+public typealias Metadata = [String: MetadataValue]
+
+extension MetadataValue: JSONDecodable {
+    internal enum JSONDecodingError: Error {
+        case unsupportedJSONValue(JSONValue)
+    }
+
+    internal init(jsonValue: JSONValue) throws {
+        self = switch jsonValue {
+        case let .string(value):
+            .string(value)
+        case let .number(value):
+            .number(value)
+        case let .bool(value):
+            .bool(value)
+        case .null:
+            .null
+        default:
+            throw JSONDecodingError.unsupportedJSONValue(jsonValue)
+        }
+    }
+}
+
+extension MetadataValue: JSONEncodable {
+    internal var toJSONValue: JSONValue {
+        switch self {
+        case let .string(value):
+            .string(value)
+        case let .number(value):
+            .number(Double(value))
+        case let .bool(value):
+            .bool(value)
+        case .null:
+            .null
+        }
+    }
+}

Sources/AblyChat/PresenceDataDTO.swift

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

Tests/AblyChatTests/ChatAPITests.swift

@@ -52,6 +52,52 @@ struct ChatAPITests {
         #expect(message == expectedMessage)
     }
 
+    @Test
+    func sendMessage_includesHeadersInBody() async throws {
+        // Given
+        let realtime = MockRealtime.create {
+            (MockHTTPPaginatedResponse.successSendMessage, nil)
+        }
+        let chatAPI = ChatAPI(realtime: realtime)
+
+        // When
+        _ = try await chatAPI.sendMessage(
+            roomId: "", // arbitrary
+            params: .init(
+                text: "", // arbitrary
+                // The exact value here is arbitrary, just want to check it gets serialized
+                headers: ["numberKey": .number(10), "stringKey": .string("hello")]
+            )
+        )
+
+        // Then
+        let requestBody = try #require(realtime.requestArguments.first?.body as? NSDictionary)
+        #expect(try #require(requestBody["headers"] as? NSObject) == ["numberKey": 10, "stringKey": "hello"] as NSObject)
+    }
+
+    @Test
+    func sendMessage_includesMetadataInBody() async throws {
+        // Given
+        let realtime = MockRealtime.create {
+            (MockHTTPPaginatedResponse.successSendMessage, nil)
+        }
+        let chatAPI = ChatAPI(realtime: realtime)
+
+        // When
+        _ = try await chatAPI.sendMessage(
+            roomId: "", // arbitrary
+            params: .init(
+                text: "", // arbitrary
+                // The exact value here is arbitrary, just want to check it gets serialized
+                metadata: ["numberKey": .number(10), "stringKey": .string("hello")]
+            )
+        )
+
+        // Then
+        let requestBody = try #require(realtime.requestArguments.first?.body as? NSDictionary)
+        #expect(try #require(requestBody["metadata"] as? NSObject) == ["numberKey": 10, "stringKey": "hello"] as NSObject)
+    }
+
     // MARK: getMessages Tests
 
     // @specOneOf(1/2) CHA-M6