From ad6b02b67281112b971efdf5c72c43a1d8544587 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 16 Dec 2024 11:41:55 -0300 Subject: [PATCH] Fix sending and receiving of reaction metadata and headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar issue to the one which was fixed in 7fcab5c. Haven’t added tests for the handling of the incoming message, since there were existing tests to update; will leave this for whoever writes these tests in #88. The approach that I’ve taken here of using a DTO is consistent with the approach that we use for presence data. I should have done this in 7fcab5c too; will do it separately. TODO add tests for DTO Resolves #198. --- Sources/AblyChat/DefaultRoomReactions.swift | 30 +++++--- Sources/AblyChat/JSONValue.swift | 8 ++ Sources/AblyChat/RoomReactionDTO.swift | 77 +++++++++++++++++++ Sources/AblyChat/RoomReactions.swift | 12 --- .../DefaultRoomReactionsTests.swift | 4 +- Tests/AblyChatTests/IntegrationTests.swift | 10 ++- 6 files changed, 114 insertions(+), 27 deletions(-) create mode 100644 Sources/AblyChat/RoomReactionDTO.swift diff --git a/Sources/AblyChat/DefaultRoomReactions.swift b/Sources/AblyChat/DefaultRoomReactions.swift index ee9276c..e3aeed9 100644 --- a/Sources/AblyChat/DefaultRoomReactions.swift +++ b/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 diff --git a/Sources/AblyChat/JSONValue.swift b/Sources/AblyChat/JSONValue.swift index e322a3a..3e91af8 100644 --- a/Sources/AblyChat/JSONValue.swift +++ b/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,6 +167,7 @@ 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): @@ -191,9 +193,15 @@ internal extension [String: 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 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 + } } // MARK: - Extracting values from a dictionary diff --git a/Sources/AblyChat/RoomReactionDTO.swift b/Sources/AblyChat/RoomReactionDTO.swift new file mode 100644 index 0000000..a996e77 --- /dev/null +++ b/Sources/AblyChat/RoomReactionDTO.swift @@ -0,0 +1,77 @@ +// CHA-ER3a +internal struct RoomReactionDTO { + internal var data: Data + internal var extras: Extras + + internal struct Data { + internal var type: String + internal var metadata: ReactionMetadata? + } + + internal struct Extras { + internal var headers: ReactionHeaders? + } +} + +internal extension RoomReactionDTO { + init(type: String, metadata: ReactionMetadata?, headers: ReactionHeaders?) { + data = .init(type: type, metadata: metadata) + extras = .init(headers: headers) + } + + var type: String { + data.type + } + + var metadata: ReactionMetadata? { + data.metadata + } + + var headers: ReactionHeaders? { + extras.headers + } +} + +// MARK: - JSONCodable + +extension RoomReactionDTO.Data: JSONObjectCodable { + internal enum JSONKey: String { + case type + case metadata + } + + internal init(jsonObject: [String: JSONValue]) throws { + type = try jsonObject.stringValueForKey(JSONKey.type.rawValue) + metadata = try jsonObject.optionalObjectValueForKey(JSONKey.metadata.rawValue)?.mapValues { try .init(jsonValue: $0) } + } + + internal var toJSONObject: [String: JSONValue] { + var result: [String: JSONValue] = [JSONKey.type.rawValue: .string(type)] + + if let metadata { + result[JSONKey.metadata.rawValue] = .object(metadata.mapValues(\.toJSONValue)) + } + + return result + } +} + +extension RoomReactionDTO.Extras: JSONObjectCodable { + internal enum JSONKey: String { + case headers + } + + internal init(jsonObject: [String: JSONValue]) throws { + headers = try jsonObject.optionalObjectValueForKey(JSONKey.headers.rawValue)?.mapValues { try .init(jsonValue: $0) } + } + + internal var toJSONObject: [String: JSONValue] { + var result: [String: JSONValue] = [:] + + if let headers { + result[JSONKey.headers.rawValue] = .object(headers.mapValues(\.toJSONValue)) + } + + return result + } +} diff --git a/Sources/AblyChat/RoomReactions.swift b/Sources/AblyChat/RoomReactions.swift index b804d0f..eca8ada 100644 --- a/Sources/AblyChat/RoomReactions.swift +++ b/Sources/AblyChat/RoomReactions.swift @@ -27,15 +27,3 @@ public struct SendReactionParams: Sendable { self.headers = headers } } - -internal extension SendReactionParams { - /// Returns a dictionary that `JSONSerialization` can serialize to a JSON "object" value. - /// - /// Suitable to pass as the `data` argument of an ably-cocoa publish operation. - func asJSONObject() -> [String: String] { - var dict: [String: String] = [:] - dict["type"] = "\(type)" - dict["metadata"] = "\(metadata ?? [:])" - return dict - } -} diff --git a/Tests/AblyChatTests/DefaultRoomReactionsTests.swift b/Tests/AblyChatTests/DefaultRoomReactionsTests.swift index a51d514..0482cbc 100644 --- a/Tests/AblyChatTests/DefaultRoomReactionsTests.swift +++ b/Tests/AblyChatTests/DefaultRoomReactionsTests.swift @@ -39,8 +39,8 @@ struct DefaultRoomReactionsTests { // Then #expect(channel.lastMessagePublishedName == RoomReactionEvents.reaction.rawValue) - #expect(channel.lastMessagePublishedData as? [String: String] == sendReactionParams.asJSONObject()) - #expect(channel.lastMessagePublishedExtras as? Dictionary == ["headers": sendReactionParams.headers]) + #expect(channel.lastMessagePublishedData as? NSObject == ["type": "like", "metadata": ["someMetadataKey": "someMetadataValue"]] as NSObject) + #expect(channel.lastMessagePublishedExtras as? Dictionary == ["headers": ["someHeadersKey": "someHeadersValue"]]) } // @spec CHA-ER4 diff --git a/Tests/AblyChatTests/IntegrationTests.swift b/Tests/AblyChatTests/IntegrationTests.swift index c2ca767..a43db13 100644 --- a/Tests/AblyChatTests/IntegrationTests.swift +++ b/Tests/AblyChatTests/IntegrationTests.swift @@ -165,9 +165,17 @@ struct IntegrationTests { let rxReactionSubscription = await rxRoom.reactions.subscribe() // (2) Now that we’re subscribed to reactions, send a reaction on the other client and check that we receive it on the subscription - try await txRoom.reactions.send(params: .init(type: "heart")) + try await txRoom.reactions.send( + params: .init( + type: "heart", + metadata: ["someMetadataKey": .number(123), "someOtherMetadataKey": .string("foo")], + headers: ["someHeadersKey": .number(456), "someOtherHeadersKey": .string("bar")] + ) + ) let rxReactionFromSubscription = try #require(await rxReactionSubscription.first { _ in true }) #expect(rxReactionFromSubscription.type == "heart") + #expect(rxReactionFromSubscription.metadata == ["someMetadataKey": .number(123), "someOtherMetadataKey": .string("foo")]) + #expect(rxReactionFromSubscription.headers == ["someHeadersKey": .number(456), "someOtherHeadersKey": .string("bar")]) // MARK: - Occupancy