Fix sending and receiving of message metadata and headers

We were not serializing or deserializing the Chat SDK’s Metadata and
Headers types. This was a mistake in 8b6e56a.

I was surprised to find that there were no existing tests for the
request that ChatAPI makes when you call sendMessage, nor for
DefaultMessages’s handling of messages received over a realtime channel,
which meant that there wasn’t an obvious place to slot in the tests for
the fixes introduced by this commit, nor did the mocks have any support
for writing such tests. I’ve added tests for my fixes but, given my rush
to get this fix done, the changes to the mocks aren’t great. Have
created issue #195 for us come back and write the missing tests and tidy
mine up.

Note that I’ve removed the optionality of Metadata’s values. This
optionality came from 20e7f5f when it was unclear what Metadata would
be. We probably should have removed it in 8b6e56a when we introduced
`null` as a MetadataValue case.

Resolves #193.

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))

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/Headers.swift

@@ -7,6 +7,42 @@ public enum HeadersValue: Sendable, Codable, Equatable {
     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/Metadata.swift

@@ -8,4 +8,40 @@ public enum MetadataValue: Sendable, Codable, Equatable {
     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
+        }
+    }
+}

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

Tests/AblyChatTests/DefaultMessagesTests.swift

@@ -69,6 +69,73 @@ struct DefaultMessagesTests {
         #expect(previousMessages == expectedPaginatedResult)
     }
 
+    @Test
+    func subscribe_extractsHeadersFromChannelMessage() async throws {
+        // Given
+        let realtime = MockRealtime.create()
+        let chatAPI = ChatAPI(realtime: realtime)
+
+        let channel = MockRealtimeChannel(
+            properties: .init(
+                attachSerial: "001",
+                channelSerial: "001"
+            ),
+            messageToEmitOnSubscribe: .init(
+                action: .create, // arbitrary
+                serial: "", // arbitrary
+                clientID: "", // arbitrary
+                data: [
+                    "text": "", // arbitrary
+                ],
+                extras: [
+                    "headers": ["numberKey": 10, "stringKey": "hello"],
+                ]
+            )
+        )
+        let featureChannel = MockFeatureChannel(channel: channel)
+        let defaultMessages = await DefaultMessages(featureChannel: featureChannel, chatAPI: chatAPI, roomID: "basketball", clientID: "clientId", logger: TestLogger())
+
+        // When
+        let messagesSubscription = try await defaultMessages.subscribe()
+
+        // Then
+        let receivedMessage = try #require(await messagesSubscription.first { _ in true })
+        #expect(receivedMessage.headers == ["numberKey": .number(10), "stringKey": .string("hello")])
+    }
+
+    @Test
+    func subscribe_extractsMetadataFromChannelMessage() async throws {
+        // Given
+        let realtime = MockRealtime.create()
+        let chatAPI = ChatAPI(realtime: realtime)
+
+        let channel = MockRealtimeChannel(
+            properties: .init(
+                attachSerial: "001",
+                channelSerial: "001"
+            ),
+            messageToEmitOnSubscribe: .init(
+                action: .create, // arbitrary
+                serial: "", // arbitrary
+                clientID: "", // arbitrary
+                data: [
+                    "text": "", // arbitrary
+                    "metadata": ["numberKey": 10, "stringKey": "hello"],
+                ],
+                extras: [:]
+            )
+        )
+        let featureChannel = MockFeatureChannel(channel: channel)
+        let defaultMessages = await DefaultMessages(featureChannel: featureChannel, chatAPI: chatAPI, roomID: "basketball", clientID: "clientId", logger: TestLogger())
+
+        // When
+        let messagesSubscription = try await defaultMessages.subscribe()
+
+        // Then
+        let receivedMessage = try #require(await messagesSubscription.first { _ in true })
+        #expect(receivedMessage.metadata == ["numberKey": .number(10), "stringKey": .string("hello")])
+    }
+
     // @spec CHA-M7
     @Test
     func onDiscontinuity() async throws {

Tests/AblyChatTests/IntegrationTests.swift

@@ -101,7 +101,11 @@ struct IntegrationTests {
         let throwawayRxMessageSubscription = try await rxRoom.messages.subscribe()
 
         // (3) Send the message
-        let txMessageBeforeRxSubscribe = try await txRoom.messages.send(params: .init(text: "Hello from txRoom, before rxRoom subscribe"))
+        let txMessageBeforeRxSubscribe = try await txRoom.messages.send(
+            params: .init(
+                text: "Hello from txRoom, before rxRoom subscribe"
+            )
+        )
 
         // (4) Wait for rxRoom to see the message we just sent
         let throwawayRxMessage = try #require(await throwawayRxMessageSubscription.first { _ in true })
@@ -111,7 +115,13 @@ struct IntegrationTests {
         let rxMessageSubscription = try await rxRoom.messages.subscribe()
 
         // (6) Now that we’re subscribed to messages, send a message on the other client and check that we receive it on the subscription
-        let txMessageAfterRxSubscribe = try await txRoom.messages.send(params: .init(text: "Hello from txRoom, after rxRoom subscribe"))
+        let txMessageAfterRxSubscribe = try await txRoom.messages.send(
+            params: .init(
+                text: "Hello from txRoom, after rxRoom subscribe",
+                metadata: ["someMetadataKey": .number(123), "someOtherMetadataKey": .string("foo")],
+                headers: ["someHeadersKey": .number(456), "someOtherHeadersKey": .string("bar")]
+            )
+        )
         let rxMessageFromSubscription = try #require(await rxMessageSubscription.first { _ in true })
         #expect(rxMessageFromSubscription == txMessageAfterRxSubscribe)
 

Tests/AblyChatTests/Mocks/MockRealtime.swift

@@ -3,11 +3,15 @@ import AblyChat
 import Foundation
 
 /// A mock implementation of `ARTRealtimeProtocol`. We’ll figure out how to do mocking in tests properly in https://github.com/ably-labs/ably-chat-swift/issues/5.
-final class MockRealtime: NSObject, RealtimeClientProtocol, Sendable {
+final class MockRealtime: NSObject, RealtimeClientProtocol, @unchecked Sendable {
     let connection: MockConnection
     let channels: MockChannels
     let paginatedCallback: (@Sendable () -> (ARTHTTPPaginatedResponse?, ARTErrorInfo?))?
 
+    private let mutex = NSLock()
+    /// Access must be synchronized via ``mutex``.
+    private(set) var _requestArguments: [(method: String, path: String, params: [String: String]?, body: Any?, headers: [String: String]?, callback: ARTHTTPPaginatedCallback)] = []
+
     var device: ARTLocalDevice {
         fatalError("Not implemented")
     }
@@ -81,11 +85,22 @@ final class MockRealtime: NSObject, RealtimeClientProtocol, Sendable {
         fatalError("Not implemented")
     }
 
-    func request(_: String, path _: String, params _: [String: String]?, body _: Any?, headers _: [String: String]?, callback: @escaping ARTHTTPPaginatedCallback) throws {
+    func request(_ method: String, path: String, params: [String: String]?, body: Any?, headers: [String: String]?, callback: @escaping ARTHTTPPaginatedCallback) throws {
+        mutex.lock()
+        _requestArguments.append((method: method, path: path, params: params, body: body, headers: headers, callback: callback))
+        mutex.unlock()
         guard let paginatedCallback else {
             fatalError("Paginated callback not set")
         }
         let (paginatedResponse, error) = paginatedCallback()
         callback(paginatedResponse, error)
     }
+
+    var requestArguments: [(method: String, path: String, params: [String: String]?, body: Any?, headers: [String: String]?, callback: ARTHTTPPaginatedCallback)] {
+        let result: [(method: String, path: String, params: [String: String]?, body: Any?, headers: [String: String]?, callback: ARTHTTPPaginatedCallback)]
+        mutex.lock()
+        result = _requestArguments
+        mutex.unlock()
+        return result
+    }
 }