From 4309df1a67e7e0223769c8efd788fd2e494c0889 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 12 Dec 2024 10:51:20 -0300 Subject: [PATCH 1/6] Turn conversion to and from JSONValue into a protocol --- Sources/AblyChat/JSONCodable.swift | 9 +++++++++ Sources/AblyChat/PresenceDataDTO.swift | 12 ++++++------ Tests/AblyChatTests/PresenceDataDTOTests.swift | 4 ++-- 3 files changed, 17 insertions(+), 8 deletions(-) create mode 100644 Sources/AblyChat/JSONCodable.swift diff --git a/Sources/AblyChat/JSONCodable.swift b/Sources/AblyChat/JSONCodable.swift new file mode 100644 index 0000000..676c220 --- /dev/null +++ b/Sources/AblyChat/JSONCodable.swift @@ -0,0 +1,9 @@ +internal protocol JSONEncodable { + var toJSONObjectValue: [String: JSONValue] { get } +} + +internal protocol JSONDecodable { + init(jsonValue: JSONValue) throws +} + +internal typealias JSONCodable = JSONDecodable & JSONEncodable diff --git a/Sources/AblyChat/PresenceDataDTO.swift b/Sources/AblyChat/PresenceDataDTO.swift index 48192a1..8d76ccc 100644 --- a/Sources/AblyChat/PresenceDataDTO.swift +++ b/Sources/AblyChat/PresenceDataDTO.swift @@ -3,18 +3,18 @@ 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) } @@ -22,7 +22,7 @@ internal extension PresenceDataDTO { userCustomData = jsonObject[JSONKey.userCustomData.rawValue] } - var toJSONObjectValue: [String: JSONValue] { + internal var toJSONObjectValue: [String: JSONValue] { var result: [String: JSONValue] = [:] if let userCustomData { diff --git a/Tests/AblyChatTests/PresenceDataDTOTests.swift b/Tests/AblyChatTests/PresenceDataDTOTests.swift index 943d507..c26a1d4 100644 --- a/Tests/AblyChatTests/PresenceDataDTOTests.swift +++ b/Tests/AblyChatTests/PresenceDataDTOTests.swift @@ -2,7 +2,7 @@ import Testing struct PresenceDataDTOTests { - // MARK: - Creating from JSON value + // MARK: - JSONDecodable @Test(arguments: [ // If the `userCustomData` key is missing (indicating that no data was passed when performing the presence operation), then the DTO’s `userCustomData` should be nil @@ -22,7 +22,7 @@ struct PresenceDataDTOTests { } } - // MARK: - Conversion to JSON object value + // MARK: - JSONCodable @Test( arguments: [ From 6024a22b6947aebaca1d8b660cb149e1afd342f5 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 12 Dec 2024 13:33:50 -0300 Subject: [PATCH 2/6] Make JSONEncodable return value more general Will be helpful when I add JSONEncodable support to other types. --- Sources/AblyChat/DefaultPresence.swift | 6 +++--- Sources/AblyChat/JSONCodable.swift | 2 +- Sources/AblyChat/PresenceDataDTO.swift | 4 ++-- Tests/AblyChatTests/PresenceDataDTOTests.swift | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Sources/AblyChat/DefaultPresence.swift b/Sources/AblyChat/DefaultPresence.swift index 3b97483..6eeb42d 100644 --- a/Sources/AblyChat/DefaultPresence.swift +++ b/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.toAblyCocoaPresenceData) { [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.toAblyCocoaPresenceData) { [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.toAblyCocoaPresenceData) { [logger] error in if let error { logger.log(message: "Error leaving presence: \(error)", level: .error) continuation.resume(throwing: error) diff --git a/Sources/AblyChat/JSONCodable.swift b/Sources/AblyChat/JSONCodable.swift index 676c220..0009ab3 100644 --- a/Sources/AblyChat/JSONCodable.swift +++ b/Sources/AblyChat/JSONCodable.swift @@ -1,5 +1,5 @@ internal protocol JSONEncodable { - var toJSONObjectValue: [String: JSONValue] { get } + var toJSONValue: JSONValue { get } } internal protocol JSONDecodable { diff --git a/Sources/AblyChat/PresenceDataDTO.swift b/Sources/AblyChat/PresenceDataDTO.swift index 8d76ccc..d2b1e70 100644 --- a/Sources/AblyChat/PresenceDataDTO.swift +++ b/Sources/AblyChat/PresenceDataDTO.swift @@ -22,13 +22,13 @@ extension PresenceDataDTO: JSONCodable { userCustomData = jsonObject[JSONKey.userCustomData.rawValue] } - internal 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) } } diff --git a/Tests/AblyChatTests/PresenceDataDTOTests.swift b/Tests/AblyChatTests/PresenceDataDTOTests.swift index c26a1d4..47a88fd 100644 --- a/Tests/AblyChatTests/PresenceDataDTOTests.swift +++ b/Tests/AblyChatTests/PresenceDataDTOTests.swift @@ -34,8 +34,8 @@ struct PresenceDataDTOTests { (userCustomData: .null, expectedJSONObject: ["userCustomData": .null]), ] as[(userCustomData: PresenceData?, expectedJSONObject: [String: JSONValue])] ) - func toJSONObject(userCustomData: PresenceData?, expectedJSONObject: [String: JSONValue]) { + func toJSONValue(userCustomData: PresenceData?, expectedJSONObject: [String: JSONValue]) { let dto = PresenceDataDTO(userCustomData: userCustomData) - #expect(dto.toJSONObjectValue == expectedJSONObject) + #expect(dto.toJSONValue == .object(expectedJSONObject)) } } From 35007525fc872fca92ddf5cfe0a7bab46cf21fb3 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 12 Dec 2024 11:59:59 -0300 Subject: [PATCH 3/6] Remove unused param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We (erroneously) never read this argument, but it hasn’t been a problem because we never pass it either. --- Sources/AblyChat/ChatAPI.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/AblyChat/ChatAPI.swift b/Sources/AblyChat/ChatAPI.swift index 9ea9978..0d969d3 100644 --- a/Sources/AblyChat/ChatAPI.swift +++ b/Sources/AblyChat/ChatAPI.swift @@ -93,8 +93,7 @@ internal final class ChatAPI: Sendable { private func makePaginatedRequest( _ url: String, - params: [String: String]? = nil, - body: [String: Any]? = nil + params: [String: String]? = nil ) async throws -> any PaginatedResult { try await withCheckedThrowingContinuation { (continuation: CheckedContinuation, _>) in do { From 4c47a371b18af64487849271744b252416fcff0f Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 12 Dec 2024 15:33:05 -0300 Subject: [PATCH 4/6] Switch HeadersValue and MetadataValue to use Double For consistency with JSONValue, which seems like the right thing to do API-wise and also will make some upcoming work easier. --- Sources/AblyChat/Headers.swift | 2 +- Sources/AblyChat/Metadata.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/AblyChat/Headers.swift b/Sources/AblyChat/Headers.swift index e64a12f..279481d 100644 --- a/Sources/AblyChat/Headers.swift +++ b/Sources/AblyChat/Headers.swift @@ -2,7 +2,7 @@ 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 } diff --git a/Sources/AblyChat/Metadata.swift b/Sources/AblyChat/Metadata.swift index 85bc686..28489b8 100644 --- a/Sources/AblyChat/Metadata.swift +++ b/Sources/AblyChat/Metadata.swift @@ -3,7 +3,7 @@ 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 } From f26e80a27799992096c17f64b5a272ab6702b42b Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 12 Dec 2024 17:50:05 -0300 Subject: [PATCH 5/6] Generalise the names of the JSONValue to ably-cocoa methods In an upcoming commit, I intend to use these methods in a few more situations other than just presence. --- Sources/AblyChat/DefaultPresence.swift | 8 +++---- Sources/AblyChat/JSONValue.swift | 29 +++++++++++++++++------- Tests/AblyChatTests/JSONValueTests.swift | 22 +++++++++--------- 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/Sources/AblyChat/DefaultPresence.swift b/Sources/AblyChat/DefaultPresence.swift index 6eeb42d..7d17082 100644 --- a/Sources/AblyChat/DefaultPresence.swift +++ b/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: dto.toJSONValue.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(dto.toJSONValue.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(dto.toJSONValue.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) diff --git a/Sources/AblyChat/JSONValue.swift b/Sources/AblyChat/JSONValue.swift index 4aba898..1cffea2 100644 --- a/Sources/AblyChat/JSONValue.swift +++ b/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): diff --git a/Tests/AblyChatTests/JSONValueTests.swift b/Tests/AblyChatTests/JSONValueTests.swift index 93b9c00..b95e748 100644 --- a/Tests/AblyChatTests/JSONValueTests.swift +++ b/Tests/AblyChatTests/JSONValueTests.swift @@ -3,7 +3,7 @@ import Foundation import Testing struct JSONValueTests { - // MARK: Conversion from ably-cocoa presence data + // MARK: Conversion from ably-cocoa data @Test(arguments: [ // object @@ -23,13 +23,13 @@ struct JSONValueTests { // null (ablyCocoaPresenceData: NSNull(), expectedResult: .null), ] as[(ablyCocoaPresenceData: Sendable, expectedResult: JSONValue?)]) - func initWithAblyCocoaPresenceData(ablyCocoaPresenceData: Sendable, expectedResult: JSONValue?) { - #expect(JSONValue(ablyCocoaPresenceData: ablyCocoaPresenceData) == expectedResult) + func initWithAblyCocoaPresenceData(ablyCocoaData: Sendable, expectedResult: JSONValue?) { + #expect(JSONValue(ablyCocoaData: ablyCocoaData) == expectedResult) } // Tests that it correctly handles an object deserialized by `JSONSerialization` (which is what ably-cocoa uses for deserialization). @Test - func initWithAblyCocoaPresenceData_endToEnd() throws { + func initWithAblyCocoaData_endToEnd() throws { let jsonString = """ { "someArray": [ @@ -51,7 +51,7 @@ struct JSONValueTests { } """ - let ablyCocoaPresenceData = try JSONSerialization.jsonObject(with: #require(jsonString.data(using: .utf8))) + let ablyCocoaData = try JSONSerialization.jsonObject(with: #require(jsonString.data(using: .utf8))) let expected: JSONValue = [ "someArray": [ @@ -72,10 +72,10 @@ struct JSONValueTests { ], ] - #expect(JSONValue(ablyCocoaPresenceData: ablyCocoaPresenceData) == expected) + #expect(JSONValue(ablyCocoaData: ablyCocoaData) == expected) } - // MARK: Conversion to ably-cocoa presence data + // MARK: Conversion to ably-cocoa data @Test(arguments: [ // object @@ -95,15 +95,15 @@ struct JSONValueTests { // null (value: .null, expectedResult: NSNull()), ] as[(value: JSONValue, expectedResult: Sendable)]) - func toAblyCocoaPresenceData(value: JSONValue, expectedResult: Sendable) throws { - let resultAsNSObject = try #require(value.toAblyCocoaPresenceData as? NSObject) + func toAblyCocoaData(value: JSONValue, expectedResult: Sendable) throws { + let resultAsNSObject = try #require(value.toAblyCocoaData as? NSObject) let expectedResultAsNSObject = try #require(expectedResult as? NSObject) #expect(resultAsNSObject == expectedResultAsNSObject) } // Tests that it creates an object that can be serialized by `JSONSerialization` (which is what ably-cocoa uses for serialization), and that the result of this serialization is what we’d expect. @Test - func toAblyCocoaPresenceData_endToEnd() throws { + func toAblyCocoaData_endToEnd() throws { let value: JSONValue = [ "someArray": [ [ @@ -146,7 +146,7 @@ struct JSONValueTests { let jsonSerializationOptions: JSONSerialization.WritingOptions = [.sortedKeys] - let valueData = try JSONSerialization.data(withJSONObject: value.toAblyCocoaPresenceData, options: jsonSerializationOptions) + let valueData = try JSONSerialization.data(withJSONObject: value.toAblyCocoaData, options: jsonSerializationOptions) let expectedData = try { let serialized = try JSONSerialization.jsonObject(with: #require(expectedJSONString.data(using: .utf8))) return try JSONSerialization.data(withJSONObject: serialized, options: jsonSerializationOptions) From 7fcab5c59941d8d26729a2bfc0b57687c976a09c Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 12 Dec 2024 11:44:54 -0300 Subject: [PATCH 6/6] Fix sending and receiving of message metadata and headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 18 +++-- Sources/AblyChat/DefaultMessages.swift | 21 ++++-- Sources/AblyChat/Headers.swift | 36 ++++++++++ Sources/AblyChat/Metadata.swift | 38 ++++++++++- Tests/AblyChatTests/ChatAPITests.swift | 46 +++++++++++++ .../AblyChatTests/DefaultMessagesTests.swift | 67 +++++++++++++++++++ Tests/AblyChatTests/IntegrationTests.swift | 14 +++- Tests/AblyChatTests/Mocks/MockRealtime.swift | 19 +++++- .../Mocks/MockRealtimeChannel.swift | 27 +++++++- 9 files changed, 267 insertions(+), 19 deletions(-) diff --git a/Sources/AblyChat/ChatAPI.swift b/Sources/AblyChat/ChatAPI.swift index 0d969d3..55bf7db 100644 --- a/Sources/AblyChat/ChatAPI.swift +++ b/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(_ url: String, method: String, body: [String: Any]? = nil) async throws -> Response { - try await withCheckedThrowingContinuation { continuation in + 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 { + 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)) diff --git a/Sources/AblyChat/DefaultMessages.swift b/Sources/AblyChat/DefaultMessages.swift index 9d4cab3..4b36e72 100644 --- a/Sources/AblyChat/DefaultMessages.swift +++ b/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 diff --git a/Sources/AblyChat/Headers.swift b/Sources/AblyChat/Headers.swift index 279481d..f256417 100644 --- a/Sources/AblyChat/Headers.swift +++ b/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 // 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. diff --git a/Sources/AblyChat/Metadata.swift b/Sources/AblyChat/Metadata.swift index 28489b8..7186d5f 100644 --- a/Sources/AblyChat/Metadata.swift +++ b/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 + } + } +} diff --git a/Tests/AblyChatTests/ChatAPITests.swift b/Tests/AblyChatTests/ChatAPITests.swift index e160d3f..e7aca3a 100644 --- a/Tests/AblyChatTests/ChatAPITests.swift +++ b/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 diff --git a/Tests/AblyChatTests/DefaultMessagesTests.swift b/Tests/AblyChatTests/DefaultMessagesTests.swift index 4058642..d2ad536 100644 --- a/Tests/AblyChatTests/DefaultMessagesTests.swift +++ b/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 { diff --git a/Tests/AblyChatTests/IntegrationTests.swift b/Tests/AblyChatTests/IntegrationTests.swift index f9db7c5..c2ca767 100644 --- a/Tests/AblyChatTests/IntegrationTests.swift +++ b/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) diff --git a/Tests/AblyChatTests/Mocks/MockRealtime.swift b/Tests/AblyChatTests/Mocks/MockRealtime.swift index c6c3fd4..44d2a18 100644 --- a/Tests/AblyChatTests/Mocks/MockRealtime.swift +++ b/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 + } } diff --git a/Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift b/Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift index d84900d..f4e7c57 100644 --- a/Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift +++ b/Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift @@ -17,16 +17,27 @@ final class MockRealtimeChannel: NSObject, RealtimeChannelProtocol { nonisolated(unsafe) var lastMessagePublishedData: Any? nonisolated(unsafe) var lastMessagePublishedExtras: (any ARTJsonCompatible)? + // TODO: If we tighten up the types we then we should be able to get rid of the `@unchecked Sendable` here, but I’m in a rush. Revisit in https://github.com/ably/ably-chat-swift/issues/195 + struct MessageToEmit: @unchecked Sendable { + var action: ARTMessageAction + var serial: String + var clientID: String + var data: Any + var extras: NSDictionary + } + init( name: String? = nil, properties: ARTChannelProperties = .init(), state _: ARTRealtimeChannelState = .suspended, attachResult: AttachOrDetachResult? = nil, - detachResult: AttachOrDetachResult? = nil + detachResult: AttachOrDetachResult? = nil, + messageToEmitOnSubscribe: MessageToEmit? = nil ) { _name = name self.attachResult = attachResult self.detachResult = detachResult + self.messageToEmitOnSubscribe = messageToEmitOnSubscribe attachSerial = properties.attachSerial channelSerial = properties.channelSerial } @@ -121,6 +132,8 @@ final class MockRealtimeChannel: NSObject, RealtimeChannelProtocol { detachResult.performCallback(callback) } + let messageToEmitOnSubscribe: MessageToEmit? + func subscribe(_: @escaping ARTMessageCallback) -> ARTEventListener? { fatalError("Not implemented") } @@ -129,8 +142,16 @@ final class MockRealtimeChannel: NSObject, RealtimeChannelProtocol { fatalError("Not implemented") } - func subscribe(_: String, callback _: @escaping ARTMessageCallback) -> ARTEventListener? { - ARTEventListener() + func subscribe(_: String, callback: @escaping ARTMessageCallback) -> ARTEventListener? { + if let messageToEmitOnSubscribe { + let message = ARTMessage(name: nil, data: messageToEmitOnSubscribe.data) + message.action = messageToEmitOnSubscribe.action + message.serial = messageToEmitOnSubscribe.serial + message.clientId = messageToEmitOnSubscribe.clientID + message.extras = messageToEmitOnSubscribe.extras + callback(message) + } + return ARTEventListener() } func subscribe(_: String, onAttach _: ARTCallback?, callback _: @escaping ARTMessageCallback) -> ARTEventListener? {