Skip to content

Commit

Permalink
Fix sending and receiving of reaction metadata and headers
Browse files Browse the repository at this point in the history
Similar issue to the one which was fixed in 7fcab5c.

Haven’t added tests for the handling of the incoming message, since
there were no existing tests to update and I didn’t feel like writing
them; 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.

The spec doesn’t describe how to behave if the user doesn’t pass headers
or metadata; I’ve taken the behaviour of passing an empty object from
the JS Chat SDK at 6d1b04a. Have created spec issue [1] for specifying
this.

Resolves #198.

[1] ably/specification#256
  • Loading branch information
lawrence-forooghian committed Dec 17, 2024
1 parent 8b8a084 commit 6035a1e
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 27 deletions.
30 changes: 18 additions & 12 deletions Sources/AblyChat/DefaultRoomReactions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions Sources/AblyChat/JSONValue.swift
Original file line number Diff line number Diff line change
@@ -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)).
Expand Down Expand Up @@ -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):
Expand All @@ -191,7 +193,13 @@ 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
}
}
70 changes: 70 additions & 0 deletions Sources/AblyChat/RoomReactionDTO.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// CHA-ER3a
internal struct RoomReactionDTO {
internal var data: Data
internal var extras: Extras

internal struct Data: Equatable {
internal var type: String
internal var metadata: ReactionMetadata?
}

internal struct Extras: Equatable {
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] {
[
JSONKey.type.rawValue: .string(type),
JSONKey.metadata.rawValue: .object(metadata?.mapValues(\.toJSONValue) ?? [:]),
]
}
}

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] {
[
JSONKey.headers.rawValue: .object(headers?.mapValues(\.toJSONValue) ?? [:]),
]
}
}
12 changes: 0 additions & 12 deletions Sources/AblyChat/RoomReactions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
4 changes: 2 additions & 2 deletions Tests/AblyChatTests/DefaultRoomReactionsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion Tests/AblyChatTests/IntegrationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
113 changes: 113 additions & 0 deletions Tests/AblyChatTests/RoomReactionDTOTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
@testable import AblyChat
import Testing

enum RoomReactionDTOTests {
struct DataTests {
// MARK: - JSONDecodable

@Test
func initWithJSONValue_failsIfNotObject() {
#expect(throws: JSONValueDecodingError.self) {
try RoomReactionDTO.Data(jsonValue: "hello")
}
}

@Test
func initWithJSONValue_withNoTypeKey() {
#expect(throws: JSONValueDecodingError.self) {
try RoomReactionDTO.Data(jsonValue: [:])
}
}

@Test
func initWithJSONValue_withNoMetadataKey() throws {
#expect(try RoomReactionDTO.Data(jsonValue: ["type": "" /* arbitrary */ ]).metadata == nil)
}

@Test
func initWithJSONValue() throws {
let data = try RoomReactionDTO.Data(
jsonValue: [
"type": "someType",
"metadata": [
"someStringKey": "someStringValue",
"someNumberKey": 123,
],
]
)

#expect(data == .init(type: "someType", metadata: ["someStringKey": .string("someStringValue"), "someNumberKey": .number(123)]))
}

// MARK: - JSONCodable

@Test
func toJSONValue_withNilMetadata() {
// i.e. should create an empty object for metadata
#expect(RoomReactionDTO.Data(type: "" /* arbitrary */, metadata: nil).toJSONValue == .object(["type": "", "metadata": .object([:])]))
}

@Test
func toJSONValue() {
let data = RoomReactionDTO.Data(type: "someType", metadata: ["someStringKey": .string("someStringValue"), "someNumberKey": .number(123)])

#expect(data.toJSONValue == [
"type": "someType",
"metadata": [
"someStringKey": "someStringValue",
"someNumberKey": 123,
],
])
}
}

struct ExtrasTests {
// MARK: - JSONDecodable

@Test
func initWithJSONValue_failsIfNotObject() {
#expect(throws: JSONValueDecodingError.self) {
try RoomReactionDTO.Extras(jsonValue: "hello")
}
}

@Test
func initWithJSONValue_withNoHeadersKey() throws {
#expect(try RoomReactionDTO.Extras(jsonValue: [:]).headers == nil)
}

@Test
func initWithJSONValue() throws {
let data = try RoomReactionDTO.Extras(
jsonValue: [
"headers": [
"someStringKey": "someStringValue",
"someNumberKey": 123,
],
]
)

#expect(data == .init(headers: ["someStringKey": .string("someStringValue"), "someNumberKey": .number(123)]))
}

// MARK: - JSONCodable

@Test
func toJSONValue_withNilHeaders() {
// i.e. should create an empty object for headers
#expect(RoomReactionDTO.Extras(headers: nil).toJSONValue == .object(["headers": .object([:])]))
}

@Test
func toJSONValue() {
let data = RoomReactionDTO.Extras(headers: ["someStringKey": .string("someStringValue"), "someNumberKey": .number(123)])

#expect(data.toJSONValue == [
"headers": [
"someStringKey": "someStringValue",
"someNumberKey": 123,
],
])
}
}
}

0 comments on commit 6035a1e

Please sign in to comment.