Skip to content

Commit

Permalink
Merge pull request #201 from ably/13-fix-PresenceMember-extras-type
Browse files Browse the repository at this point in the history
[ECO-4927] Improve the type of `PresenceMember.extras`
  • Loading branch information
lawrence-forooghian authored Dec 18, 2024
2 parents 58a7082 + c80808f commit a182876
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 19 deletions.
6 changes: 3 additions & 3 deletions Sources/AblyChat/DefaultMessages.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities {
throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without data or text")
}

guard let ablyCocoaExtras = message.extras,
let extras = try JSONValue(ablyCocoaData: ablyCocoaExtras.toJSON()).objectValue
else {
guard let ablyCocoaExtras = message.extras else {
throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without extras")
}

let extras = JSONValue.objectFromAblyCocoaExtras(ablyCocoaExtras)

guard let serial = message.serial else {
throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without serial")
}
Expand Down
7 changes: 5 additions & 2 deletions Sources/AblyChat/DefaultPresence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,11 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
throw error
}

// Seems like we want to just forward on `extras` from the cocoa SDK but that is an `ARTJsonCompatible` type which is not `Sendable`... currently just converting this to a `Sendable` type (`String`) until we know what to do with this.
let extras = member.extras?.toJSONString()
let extras: [String: JSONValue]? = if let ablyCocoaExtras = member.extras {
JSONValue.objectFromAblyCocoaExtras(ablyCocoaExtras)
} else {
nil
}

let presenceMember = PresenceMember(
clientID: clientID,
Expand Down
4 changes: 2 additions & 2 deletions Sources/AblyChat/DefaultRoomReactions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ internal final class DefaultRoomReactions: RoomReactions, EmitsDiscontinuities {
throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without timestamp")
}

guard let ablyCocoaExtras = try message.extras?.toJSON() else {
guard let ablyCocoaExtras = message.extras else {
throw ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without extras")
}

let dto = try RoomReactionDTO(
data: .init(jsonValue: .init(ablyCocoaData: ablyCocoaData)),
extras: .init(jsonValue: .init(ablyCocoaData: ablyCocoaExtras))
extras: .init(jsonObject: JSONValue.objectFromAblyCocoaExtras(ablyCocoaExtras))
)

// (CHA-ER4d) Realtime events that are malformed (unknown fields should be ignored) shall not be emitted to listeners.
Expand Down
21 changes: 16 additions & 5 deletions Sources/AblyChat/JSONValue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,7 @@ internal extension JSONValue {
///
/// 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
/// - a non-`nil` value of `ARTBaseMessage`’s `data` property
/// - an element of `ARTHTTPPaginatedResult`’s `items` array
init(ablyCocoaData: Any) {
switch ablyCocoaData {
Expand All @@ -162,11 +160,24 @@ internal extension JSONValue {
}
}

/// Creates a `JSONValue` from an ably-cocoa deserialized JSON message extras object. Specifically, `ablyCocoaExtras` can be a non-`nil` value of `ARTBaseMessage`’s `extras` property.
static func objectFromAblyCocoaExtras(_ ablyCocoaExtras: any ARTJsonCompatible) -> [String: JSONValue] {
// (This is based on the fact that, in reality, I believe that `extras` is always a JSON object; see https://github.com/ably/ably-cocoa/issues/2002 for improving ably-cocoa’s API to reflect this)

let jsonValue = JSONValue(ablyCocoaData: ablyCocoaExtras)
guard case let .object(jsonObject) = jsonValue else {
// 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.objectFromAblyCocoaExtras(_:) was given \(ablyCocoaExtras)")
}

return jsonObject
}

/// Creates an ably-cocoa deserialized JSON object from a `JSONValue`.
///
/// Specifically, the value of this property can be used as:
///
/// - `ARTPresenceMessage`’s `data` property
/// - `ARTBaseMessage`’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 {
Expand All @@ -192,7 +203,7 @@ internal extension [String: JSONValue] {
///
/// Specifically, the value of this property can be used as:
///
/// - `ARTPresenceMessage`’s `data` property
/// - `ARTBaseMessage`’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] {
Expand Down
4 changes: 2 additions & 2 deletions Sources/AblyChat/Presence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public struct PresenceMember: Sendable {
}
}

public init(clientID: String, data: PresenceData?, action: PresenceMember.Action, extras: (any Sendable)?, updatedAt: Date) {
public init(clientID: String, data: PresenceData?, action: PresenceMember.Action, extras: [String: JSONValue]?, updatedAt: Date) {
self.clientID = clientID
self.data = data
self.action = action
Expand Down Expand Up @@ -195,7 +195,7 @@ public struct PresenceMember: Sendable {
/**
* The extras associated with the presence member.
*/
public var extras: (any Sendable)?
public var extras: [String: JSONValue]?
public var updatedAt: Date
}

Expand Down
16 changes: 11 additions & 5 deletions Tests/AblyChatTests/IntegrationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -228,31 +228,37 @@ struct IntegrationTests {
#expect(rxPresenceEnterTxEvent.action == .enter)
#expect(rxPresenceEnterTxEvent.data == ["randomData": "randomValue"])

// (3) Send `.update` presence event with custom data on the other client and check that we receive it on the subscription
// (3) Fetch rxClient's presence members and check that txClient is there
let rxPresenceMembers = try await rxRoom.presence.get()
#expect(rxPresenceMembers.count == 1)
#expect(rxPresenceMembers[0].action == .present)
#expect(rxPresenceMembers[0].data == ["randomData": "randomValue"])

// (4) Send `.update` presence event with custom data on the other client and check that we receive it on the subscription
try await txRoom.presence.update(data: ["randomData": "randomValue"])
let rxPresenceUpdateTxEvent = try #require(await rxPresenceSubscription.first { _ in true })
#expect(rxPresenceUpdateTxEvent.action == .update)
#expect(rxPresenceUpdateTxEvent.data == ["randomData": "randomValue"])

// (4) Send `.leave` presence event with custom data on the other client and check that we receive it on the subscription
// (5) Send `.leave` presence event with custom data on the other client and check that we receive it on the subscription
try await txRoom.presence.leave(data: ["randomData": "randomValue"])
let rxPresenceLeaveTxEvent = try #require(await rxPresenceSubscription.first { _ in true })
#expect(rxPresenceLeaveTxEvent.action == .leave)
#expect(rxPresenceLeaveTxEvent.data == ["randomData": "randomValue"])

// (5) Send `.enter` presence event with custom data on our client and check that we receive it on the subscription
// (6) Send `.enter` presence event with custom data on our client and check that we receive it on the subscription
try await txRoom.presence.enter(data: ["randomData": "randomValue"])
let rxPresenceEnterRxEvent = try #require(await rxPresenceSubscription.first { _ in true })
#expect(rxPresenceEnterRxEvent.action == .enter)
#expect(rxPresenceEnterRxEvent.data == ["randomData": "randomValue"])

// (6) Send `.update` presence event with custom data on our client and check that we receive it on the subscription
// (7) Send `.update` presence event with custom data on our client and check that we receive it on the subscription
try await txRoom.presence.update(data: ["randomData": "randomValue"])
let rxPresenceUpdateRxEvent = try #require(await rxPresenceSubscription.first { _ in true })
#expect(rxPresenceUpdateRxEvent.action == .update)
#expect(rxPresenceUpdateRxEvent.data == ["randomData": "randomValue"])

// (7) Send `.leave` presence event with custom data on our client and check that we receive it on the subscription
// (8) Send `.leave` presence event with custom data on our client and check that we receive it on the subscription
try await txRoom.presence.leave(data: ["randomData": "randomValue"])
let rxPresenceLeaveRxEvent = try #require(await rxPresenceSubscription.first { _ in true })
#expect(rxPresenceLeaveRxEvent.action == .leave)
Expand Down

0 comments on commit a182876

Please sign in to comment.