From e2888e17e6701c7d5b1827faf31b94372af3d552 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 5 Dec 2024 14:55:54 -0300 Subject: [PATCH 1/5] Rename mis-named `asQueryItems` methods These are not like the `Message.asQueryItems` method, since, unlike that one, their result is not intended to be used as the query params of an HTTP request. --- Sources/AblyChat/DefaultPresence.swift | 6 +++--- Sources/AblyChat/DefaultRoomReactions.swift | 2 +- Sources/AblyChat/Presence.swift | 5 ++++- Sources/AblyChat/RoomReactions.swift | 6 ++++-- Tests/AblyChatTests/DefaultRoomReactionsTests.swift | 2 +- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/Sources/AblyChat/DefaultPresence.swift b/Sources/AblyChat/DefaultPresence.swift index 1f1a05b..682e054 100644 --- a/Sources/AblyChat/DefaultPresence.swift +++ b/Sources/AblyChat/DefaultPresence.swift @@ -104,7 +104,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { throw error } return try await withCheckedThrowingContinuation { continuation in - channel.presence.enterClient(clientID, data: data?.asQueryItems()) { [logger] error in + channel.presence.enterClient(clientID, data: data?.asJSONObject()) { [logger] error in if let error { logger.log(message: "Error entering presence: \(error)", level: .error) continuation.resume(throwing: error) @@ -128,7 +128,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { } return try await withCheckedThrowingContinuation { continuation in - channel.presence.update(data?.asQueryItems()) { [logger] error in + channel.presence.update(data?.asJSONObject()) { [logger] error in if let error { logger.log(message: "Error updating presence: \(error)", level: .error) continuation.resume(throwing: error) @@ -151,7 +151,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { throw error } return try await withCheckedThrowingContinuation { continuation in - channel.presence.leave(data?.asQueryItems()) { [logger] error in + channel.presence.leave(data?.asJSONObject()) { [logger] error in if let error { logger.log(message: "Error leaving presence: \(error)", level: .error) continuation.resume(throwing: error) diff --git a/Sources/AblyChat/DefaultRoomReactions.swift b/Sources/AblyChat/DefaultRoomReactions.swift index 4acabf3..ee9276c 100644 --- a/Sources/AblyChat/DefaultRoomReactions.swift +++ b/Sources/AblyChat/DefaultRoomReactions.swift @@ -24,7 +24,7 @@ internal final class DefaultRoomReactions: RoomReactions, EmitsDiscontinuities { 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.asQueryItems(), extras: extras) + channel.publish(RoomReactionEvents.reaction.rawValue, data: params.asJSONObject(), extras: extras) } // (CHA-ER4) A user may subscribe to reaction events in Realtime. diff --git a/Sources/AblyChat/Presence.swift b/Sources/AblyChat/Presence.swift index 1652ba6..c6b4552 100644 --- a/Sources/AblyChat/Presence.swift +++ b/Sources/AblyChat/Presence.swift @@ -47,7 +47,10 @@ public struct PresenceData: Codable, Sendable { } internal extension PresenceData { - func asQueryItems() -> [String: Any] { + /// Returns a dictionary that `JSONSerialization` can serialize to a JSON "object" value. + /// + /// Suitable to pass as the `data` argument of an ably-cocoa presence operation. + func asJSONObject() -> [String: Any] { // Return an empty userCustomData string if no custom data is available guard let userCustomData else { return ["userCustomData": ""] diff --git a/Sources/AblyChat/RoomReactions.swift b/Sources/AblyChat/RoomReactions.swift index 58bdb6f..b804d0f 100644 --- a/Sources/AblyChat/RoomReactions.swift +++ b/Sources/AblyChat/RoomReactions.swift @@ -29,8 +29,10 @@ public struct SendReactionParams: Sendable { } internal extension SendReactionParams { - // Same as `ARTDataQuery.asQueryItems` from ably-cocoa. - func asQueryItems() -> [String: String] { + /// 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 ?? [:])" diff --git a/Tests/AblyChatTests/DefaultRoomReactionsTests.swift b/Tests/AblyChatTests/DefaultRoomReactionsTests.swift index c49c629..f5141c4 100644 --- a/Tests/AblyChatTests/DefaultRoomReactionsTests.swift +++ b/Tests/AblyChatTests/DefaultRoomReactionsTests.swift @@ -39,7 +39,7 @@ struct DefaultRoomReactionsTests { // Then #expect(channel.lastMessagePublishedName == RoomReactionEvents.reaction.rawValue) - #expect(channel.lastMessagePublishedData as? [String: String] == sendReactionParams.asQueryItems()) + #expect(channel.lastMessagePublishedData as? [String: String] == sendReactionParams.asJSONObject()) #expect(channel.lastMessagePublishedExtras as? Dictionary == ["headers": sendReactionParams.headers]) } From 111404c8b6854a310da013a6131256710923b6fc Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 9 Dec 2024 16:55:28 -0300 Subject: [PATCH 2/5] Switch to data-less presence ops in integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes it clearer that we don’t care about data in these calls. I should have done this in 4ee16bd. --- Tests/AblyChatTests/IntegrationTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/AblyChatTests/IntegrationTests.swift b/Tests/AblyChatTests/IntegrationTests.swift index 68a94f8..75850d6 100644 --- a/Tests/AblyChatTests/IntegrationTests.swift +++ b/Tests/AblyChatTests/IntegrationTests.swift @@ -176,7 +176,7 @@ struct IntegrationTests { try await txRoom.attach() // (4) Enter presence on the other client and check that we receive the updated occupancy on the subscription - try await txRoom.presence.enter(data: nil) + try await txRoom.presence.enter() // (5) Check that we received an updated presence count on the subscription _ = try #require(await rxOccupancySubscription.first { occupancyEvent in @@ -188,7 +188,7 @@ struct IntegrationTests { #expect(rxOccupancyAfterTxEnter.presenceMembers == 1) // 1 for txClient entering presence // (7) Leave presence on the other client and check that we receive the updated occupancy on the subscription - try await txRoom.presence.leave(data: nil) + try await txRoom.presence.leave() // (8) Check that we received an updated presence count on the subscription _ = try #require(await rxOccupancySubscription.first { occupancyEvent in From d7b5e53dc5e24cf339f85ab7912049509f00bfd5 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 10 Dec 2024 13:04:23 -0300 Subject: [PATCH 3/5] Fix signature of PresenceMember initializer This inconsistency with the `data` property was a mistake in 20e7f5f. --- Example/AblyChatExample/Mocks/MockClients.swift | 4 ++-- Sources/AblyChat/Presence.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Example/AblyChatExample/Mocks/MockClients.swift b/Example/AblyChatExample/Mocks/MockClients.swift index 513c2d5..320e221 100644 --- a/Example/AblyChatExample/Mocks/MockClients.swift +++ b/Example/AblyChatExample/Mocks/MockClients.swift @@ -277,7 +277,7 @@ actor MockPresence: Presence { MockStrings.names.shuffled().map { name in PresenceMember( clientID: name, - data: PresenceData(userCustomData: nil), + data: nil, action: .present, extras: nil, updatedAt: Date() @@ -289,7 +289,7 @@ actor MockPresence: Presence { MockStrings.names.shuffled().map { name in PresenceMember( clientID: name, - data: PresenceData(userCustomData: nil), + data: nil, action: .present, extras: nil, updatedAt: Date() diff --git a/Sources/AblyChat/Presence.swift b/Sources/AblyChat/Presence.swift index c6b4552..445ee99 100644 --- a/Sources/AblyChat/Presence.swift +++ b/Sources/AblyChat/Presence.swift @@ -152,7 +152,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: (any Sendable)?, updatedAt: Date) { self.clientID = clientID self.data = data self.action = action From f11e0a1c1b74690a077caeb6aa7df2d79cfa284a Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 10 Dec 2024 15:47:15 -0300 Subject: [PATCH 4/5] Make handling of presence data consistent There was one place where we throw and one where we fail silently (even though the surrounding code throws for a bunch of other reasons of seemingly similar severity). So throw consistently. --- Sources/AblyChat/DefaultPresence.swift | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/Sources/AblyChat/DefaultPresence.swift b/Sources/AblyChat/DefaultPresence.swift index 682e054..b11676f 100644 --- a/Sources/AblyChat/DefaultPresence.swift +++ b/Sources/AblyChat/DefaultPresence.swift @@ -198,13 +198,15 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { await featureChannel.onDiscontinuity(bufferingPolicy: bufferingPolicy) } - private func decodePresenceData(from data: Any?) -> PresenceData? { - guard let userData = data as? [String: Any] else { - return nil + private func decodePresenceData(from data: Any?) throws -> PresenceData? { + guard let data = data as? [String: Any] else { + let error = ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without data") + logger.log(message: error.message, level: .error) + throw error } do { - let jsonData = try JSONSerialization.data(withJSONObject: userData, options: []) + let jsonData = try JSONSerialization.data(withJSONObject: data, options: []) let presenceData = try JSONDecoder().decode(PresenceData.self, from: jsonData) return presenceData } catch { @@ -221,11 +223,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { throw error } let presenceMembers = try members.map { member in - guard let data = member.data as? [String: Any] else { - let error = ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without data") - logger.log(message: error.message, level: .error) - throw error - } + let userCustomData = try decodePresenceData(from: member.data) guard let clientID = member.clientId else { let error = ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without clientId") @@ -239,8 +237,6 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { throw error } - let userCustomData = decodePresenceData(from: data) - // 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() @@ -271,7 +267,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { throw error } - let userCustomDataDecoded = decodePresenceData(from: message.data) + let userCustomDataDecoded = try decodePresenceData(from: message.data) let presenceEvent = PresenceEvent( action: event, From 80e85856184c5c2f5c1afeb9edba9baadc59765d Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 9 Dec 2024 16:55:28 -0300 Subject: [PATCH 5/5] Improve API and internals for presence data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Public API improvements: - You can now pass any JSON value as presence data (previously, the top-level value had to be an object, and you could not pass arrays or nested objects). This brings us in line with CHA-PR2a. - Do not expose the `userCustomData` property of the presence data object in the public API; it’s an implementation detail. - Conform to the `ExpressibleBy*Literal` protocols, making it easier to create a JSON value. I have deliberately chosen to make the data-arg variants of the presence operation methods take a non-optional PresenceData. This is to minimise confusion between the absence of presence data and a presence data with JSON value `null`. This is how I wrote this API in 20e7f5f, but I didn’t restore it properly in 4ee16bd. The JSONValue type introduced here is based on the example given in [1]. Internals improvements: - Fix the presence object that gets passed to ably-cocoa when the user specifies no presence data; we were previously passing an empty string as the presence data in this case. (See below for where I got the “correct” behaviour from.) - Simplify the way in which we decode the presence data received from ably-cocoa (i.e. don’t do a round-trip of JSON serialization and deserialization); this comes at the cost of not getting a little bit for free from Swift’s serialization mechanism, but I think it’s worth it The behaviour of how to map the chat presence data public API to the object exchanged with the core SDK is not currently fully specified. So, the behaviour that I’ve implemented here is based on the behaviour of the JS Chat SDK at 69ea478. I’ve created spec issue [2] in order to specify this stuff properly, but I’m in a slight rush to get this public API fixed before we release our first beta, so I’ll address this later. Resolves #178. [1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-literals/ [2] https://github.com/ably/specification/issues/256 --- Example/AblyChatExample/ContentView.swift | 4 +- .../AblyChatExample/Mocks/MockClients.swift | 36 +++- Sources/AblyChat/DefaultPresence.swift | 67 +++++-- Sources/AblyChat/JSONValue.swift | 168 ++++++++++++++++++ Sources/AblyChat/Presence.swift | 99 +---------- Sources/AblyChat/PresenceDataDTO.swift | 34 ++++ Tests/AblyChatTests/IntegrationTests.swift | 24 +-- Tests/AblyChatTests/JSONValueTests.swift | 145 +++++++++++++++ .../AblyChatTests/PresenceDataDTOTests.swift | 41 +++++ 9 files changed, 486 insertions(+), 132 deletions(-) create mode 100644 Sources/AblyChat/JSONValue.swift create mode 100644 Sources/AblyChat/PresenceDataDTO.swift create mode 100644 Tests/AblyChatTests/JSONValueTests.swift create mode 100644 Tests/AblyChatTests/PresenceDataDTOTests.swift diff --git a/Example/AblyChatExample/ContentView.swift b/Example/AblyChatExample/ContentView.swift index efa5c64..39a949c 100644 --- a/Example/AblyChatExample/ContentView.swift +++ b/Example/AblyChatExample/ContentView.swift @@ -202,13 +202,13 @@ struct ContentView: View { } func showPresence() async throws { - try await room().presence.enter(data: .init(userCustomData: ["status": .string("📱 Online")])) + try await room().presence.enter(data: ["status": "📱 Online"]) // Continue listening for new presence events on a background task so this function can return Task { for await event in try await room().presence.subscribe(events: [.enter, .leave, .update]) { withAnimation { - let status = event.data?.userCustomData?["status"]?.value as? String + let status = event.data?.objectValue?["status"]?.stringValue let clientPresenceChangeMessage = "\(event.clientID) \(event.action.displayedText)" let presenceMessage = status != nil ? "\(clientPresenceChangeMessage) with status: \(status!)" : clientPresenceChangeMessage diff --git a/Example/AblyChatExample/Mocks/MockClients.swift b/Example/AblyChatExample/Mocks/MockClients.swift index 320e221..5a1825b 100644 --- a/Example/AblyChatExample/Mocks/MockClients.swift +++ b/Example/AblyChatExample/Mocks/MockClients.swift @@ -301,40 +301,64 @@ actor MockPresence: Presence { fatalError("Not yet implemented") } - func enter(data: PresenceData? = nil) async throws { + func enter() async throws { + try await enter(dataForEvent: nil) + } + + func enter(data: PresenceData) async throws { + try await enter(dataForEvent: data) + } + + private func enter(dataForEvent: PresenceData?) async throws { for subscription in mockSubscriptions { subscription.emit( PresenceEvent( action: .enter, clientID: clientID, timestamp: Date(), - data: data + data: dataForEvent ) ) } } - func update(data: PresenceData? = nil) async throws { + func update() async throws { + try await update(dataForEvent: nil) + } + + func update(data: PresenceData) async throws { + try await update(dataForEvent: data) + } + + private func update(dataForEvent: PresenceData? = nil) async throws { for subscription in mockSubscriptions { subscription.emit( PresenceEvent( action: .update, clientID: clientID, timestamp: Date(), - data: data + data: dataForEvent ) ) } } - func leave(data: PresenceData? = nil) async throws { + func leave() async throws { + try await leave(dataForEvent: nil) + } + + func leave(data: PresenceData) async throws { + try await leave(dataForEvent: data) + } + + func leave(dataForEvent: PresenceData? = nil) async throws { for subscription in mockSubscriptions { subscription.emit( PresenceEvent( action: .leave, clientID: clientID, timestamp: Date(), - data: data + data: dataForEvent ) ) } diff --git a/Sources/AblyChat/DefaultPresence.swift b/Sources/AblyChat/DefaultPresence.swift index b11676f..3b97483 100644 --- a/Sources/AblyChat/DefaultPresence.swift +++ b/Sources/AblyChat/DefaultPresence.swift @@ -92,8 +92,16 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { } } + internal func enter(data: PresenceData) async throws { + try await enter(optionalData: data) + } + + internal func enter() async throws { + try await enter(optionalData: nil) + } + // (CHA-PR3a) Users may choose to enter presence, optionally providing custom data to enter with. The overall presence data must retain the format specified in CHA-PR2. - internal func enter(data: PresenceData? = nil) async throws { + private func enter(optionalData data: PresenceData?) async throws { logger.log(message: "Entering presence", level: .debug) // CHA-PR3c to CHA-PR3g @@ -103,8 +111,11 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { logger.log(message: "Error waiting to be able to perform presence enter operation: \(error)", level: .error) throw error } + + let dto = PresenceDataDTO(userCustomData: data) + return try await withCheckedThrowingContinuation { continuation in - channel.presence.enterClient(clientID, data: data?.asJSONObject()) { [logger] error in + channel.presence.enterClient(clientID, data: JSONValue.object(dto.toJSONObjectValue).toAblyCocoaPresenceData) { [logger] error in if let error { logger.log(message: "Error entering presence: \(error)", level: .error) continuation.resume(throwing: error) @@ -115,8 +126,16 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { } } + internal func update(data: PresenceData) async throws { + try await update(optionalData: data) + } + + internal func update() async throws { + try await update(optionalData: nil) + } + // (CHA-PR10a) Users may choose to update their presence data, optionally providing custom data to update with. The overall presence data must retain the format specified in CHA-PR2. - internal func update(data: PresenceData? = nil) async throws { + private func update(optionalData data: PresenceData?) async throws { logger.log(message: "Updating presence", level: .debug) // CHA-PR10c to CHA-PR10g @@ -127,8 +146,10 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { throw error } + let dto = PresenceDataDTO(userCustomData: data) + return try await withCheckedThrowingContinuation { continuation in - channel.presence.update(data?.asJSONObject()) { [logger] error in + channel.presence.update(JSONValue.object(dto.toJSONObjectValue).toAblyCocoaPresenceData) { [logger] error in if let error { logger.log(message: "Error updating presence: \(error)", level: .error) continuation.resume(throwing: error) @@ -139,8 +160,16 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { } } + internal func leave(data: PresenceData) async throws { + try await leave(optionalData: data) + } + + internal func leave() async throws { + try await leave(optionalData: nil) + } + // (CHA-PR4a) Users may choose to leave presence, which results in them being removed from the Realtime presence set. - internal func leave(data: PresenceData? = nil) async throws { + internal func leave(optionalData data: PresenceData?) async throws { logger.log(message: "Leaving presence", level: .debug) // CHA-PR6b to CHA-PR6f @@ -150,8 +179,11 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { logger.log(message: "Error waiting to be able to perform presence leave operation: \(error)", level: .error) throw error } + + let dto = PresenceDataDTO(userCustomData: data) + return try await withCheckedThrowingContinuation { continuation in - channel.presence.leave(data?.asJSONObject()) { [logger] error in + channel.presence.leave(JSONValue.object(dto.toJSONObjectValue).toAblyCocoaPresenceData) { [logger] error in if let error { logger.log(message: "Error leaving presence: \(error)", level: .error) continuation.resume(throwing: error) @@ -198,21 +230,20 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { await featureChannel.onDiscontinuity(bufferingPolicy: bufferingPolicy) } - private func decodePresenceData(from data: Any?) throws -> PresenceData? { - guard let data = data as? [String: Any] else { + private func decodePresenceDataDTO(from ablyCocoaPresenceData: Any?) throws -> PresenceDataDTO { + guard let ablyCocoaPresenceData else { let error = ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without data") logger.log(message: error.message, level: .error) throw error } + let jsonValue = JSONValue(ablyCocoaPresenceData: ablyCocoaPresenceData) + do { - let jsonData = try JSONSerialization.data(withJSONObject: data, options: []) - let presenceData = try JSONDecoder().decode(PresenceData.self, from: jsonData) - return presenceData + return try PresenceDataDTO(jsonValue: jsonValue) } catch { - print("Failed to decode PresenceData: \(error)") - logger.log(message: "Failed to decode PresenceData: \(error)", level: .error) - return nil + logger.log(message: "Failed to decode presence data DTO from \(jsonValue), error \(error)", level: .error) + throw error } } @@ -223,7 +254,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { throw error } let presenceMembers = try members.map { member in - let userCustomData = try decodePresenceData(from: member.data) + let presenceDataDTO = try decodePresenceDataDTO(from: member.data) guard let clientID = member.clientId else { let error = ARTErrorInfo.create(withCode: 50000, status: 500, message: "Received incoming message without clientId") @@ -242,7 +273,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { let presenceMember = PresenceMember( clientID: clientID, - data: userCustomData ?? .init(), + data: presenceDataDTO.userCustomData, action: PresenceMember.Action(from: member.action), extras: extras, updatedAt: timestamp @@ -267,13 +298,13 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { throw error } - let userCustomDataDecoded = try decodePresenceData(from: message.data) + let presenceDataDTO = try decodePresenceDataDTO(from: message.data) let presenceEvent = PresenceEvent( action: event, clientID: clientID, timestamp: timestamp, - data: userCustomDataDecoded ?? .init() + data: presenceDataDTO.userCustomData ) logger.log(message: "Returning presence event: \(presenceEvent)", level: .debug) diff --git a/Sources/AblyChat/JSONValue.swift b/Sources/AblyChat/JSONValue.swift new file mode 100644 index 0000000..2bb383c --- /dev/null +++ b/Sources/AblyChat/JSONValue.swift @@ -0,0 +1,168 @@ +import Foundation + +/// A JSON value (where "value" has the meaning defined by the [JSON specification](https://www.json.org)). +/// +/// `JSONValue` provides a type-safe API for working with JSON values. It implements Swift’s `ExpressibleBy*Literal` protocols. This allows you to write type-safe JSON values using familiar syntax. For example: +/// +/// ```swift +/// let jsonValue: JSONValue = [ +/// "someArray": [ +/// [ +/// "someStringKey": "someString", +/// "someIntegerKey": 123, +/// "someFloatKey": 123.456, +/// "someTrueKey": true, +/// "someFalseKey": false, +/// "someNullKey": .null, +/// ], +/// "someOtherArrayElement", +/// ], +/// "someNestedObject": [ +/// "someOtherKey": "someOtherValue", +/// ], +/// ] +/// ``` +/// +/// > Note: To write a `JSONValue` that corresponds to the `null` JSON value, you must explicitly write `.null`. `JSONValue` deliberately does not implement the `ExpressibleByNilLiteral` protocol in order to avoid confusion between a value of type `JSONValue?` and a `JSONValue` with case `.null`. +public indirect enum JSONValue: Sendable, Equatable { + case object([String: JSONValue]) + case array([JSONValue]) + case string(String) + case number(Double) + case bool(Bool) + case null + + // MARK: - Convenience getters for associated values + + /// If this `JSONValue` has case `object`, this returns the associated value. Else, it returns `nil`. + public var objectValue: [String: JSONValue]? { + if case let .object(objectValue) = self { + objectValue + } else { + nil + } + } + + /// If this `JSONValue` has case `array`, this returns the associated value. Else, it returns `nil`. + public var arrayValue: [JSONValue]? { + if case let .array(arrayValue) = self { + arrayValue + } else { + nil + } + } + + /// If this `JSONValue` has case `string`, this returns the associated value. Else, it returns `nil`. + public var stringValue: String? { + if case let .string(stringValue) = self { + stringValue + } else { + nil + } + } + + /// If this `JSONValue` has case `number`, this returns the associated value. Else, it returns `nil`. + public var numberValue: Double? { + if case let .number(numberValue) = self { + numberValue + } else { + nil + } + } + + /// If this `JSONValue` has case `bool`, this returns the associated value. Else, it returns `nil`. + public var boolValue: Bool? { + if case let .bool(boolValue) = self { + boolValue + } else { + nil + } + } + + /// Returns true if and only if this `JSONValue` has case `null`. + public var isNull: Bool { + if case .null = self { + true + } else { + false + } + } +} + +extension JSONValue: ExpressibleByDictionaryLiteral { + public init(dictionaryLiteral elements: (String, JSONValue)...) { + self = .object(.init(uniqueKeysWithValues: elements)) + } +} + +extension JSONValue: ExpressibleByArrayLiteral { + public init(arrayLiteral elements: JSONValue...) { + self = .array(elements) + } +} + +extension JSONValue: ExpressibleByStringLiteral { + public init(stringLiteral value: String) { + self = .string(value) + } +} + +extension JSONValue: ExpressibleByIntegerLiteral { + public init(integerLiteral value: Int) { + self = .number(Double(value)) + } +} + +extension JSONValue: ExpressibleByFloatLiteral { + public init(floatLiteral value: Double) { + self = .number(value) + } +} + +extension JSONValue: ExpressibleByBooleanLiteral { + public init(booleanLiteral value: Bool) { + self = .bool(value) + } +} + +// MARK: - Bridging with ably-cocoa + +internal extension JSONValue { + init(ablyCocoaPresenceData: Any) { + switch ablyCocoaPresenceData { + case let dictionary as [String: Any]: + self = .object(dictionary.mapValues { .init(ablyCocoaPresenceData: $0) }) + case let array as [Any]: + self = .array(array.map { .init(ablyCocoaPresenceData: $0) }) + case let string as String: + self = .string(string) + // The order here is important, since a Bool can satisfy the NSNumber check + case let bool as Bool: + self = .bool(bool) + case let number as NSNumber: + self = .number(number.doubleValue) + case is NSNull: + 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)") + } + } + + var toAblyCocoaPresenceData: Any { + switch self { + case let .object(underlying): + underlying.mapValues(\.toAblyCocoaPresenceData) + case let .array(underlying): + underlying.map(\.toAblyCocoaPresenceData) + case let .string(underlying): + underlying + case let .number(underlying): + underlying + case let .bool(underlying): + underlying + case .null: + NSNull() + } + } +} diff --git a/Sources/AblyChat/Presence.swift b/Sources/AblyChat/Presence.swift index 445ee99..bed24de 100644 --- a/Sources/AblyChat/Presence.swift +++ b/Sources/AblyChat/Presence.swift @@ -1,92 +1,16 @@ import Ably -// TODO: (https://github.com/ably-labs/ably-chat-swift/issues/13): try to improve this type -public enum PresenceCustomData: Sendable, Codable, Equatable { - case string(String) - case number(Int) // Changed from NSNumber to Int to conform to Codable. Address in linked issue above. - case bool(Bool) - case null - - public var value: Any? { - switch self { - case let .string(value): - value - case let .number(value): - value - case let .bool(value): - value - case .null: - nil - } - } - - public init(from decoder: Decoder) throws { - let container = try decoder.singleValueContainer() - - if let value = try? container.decode(String.self) { - self = .string(value) - } else if let value = try? container.decode(Int.self) { - self = .number(value) - } else if let value = try? container.decode(Bool.self) { - self = .bool(value) - } else { - self = .null - } - } -} - -public typealias UserCustomData = [String: PresenceCustomData] - -// (CHA-PR2a) The presence data format is a JSON object as described below. Customers may specify content of an arbitrary type to be placed in the userCustomData field. -public struct PresenceData: Codable, Sendable { - public var userCustomData: UserCustomData? - - public init(userCustomData: UserCustomData? = nil) { - self.userCustomData = userCustomData - } -} - -internal extension PresenceData { - /// Returns a dictionary that `JSONSerialization` can serialize to a JSON "object" value. - /// - /// Suitable to pass as the `data` argument of an ably-cocoa presence operation. - func asJSONObject() -> [String: Any] { - // Return an empty userCustomData string if no custom data is available - guard let userCustomData else { - return ["userCustomData": ""] - } - - // Create a dictionary for userCustomData - var userCustomDataDict: [String: Any] = [:] - - // Iterate over the custom data and handle different PresenceCustomData cases - for (key, value) in userCustomData { - switch value { - case let .string(stringValue): - userCustomDataDict[key] = stringValue - case let .number(numberValue): - userCustomDataDict[key] = numberValue - case let .bool(boolValue): - userCustomDataDict[key] = boolValue - case .null: - userCustomDataDict[key] = NSNull() // Use NSNull to represent null in the dictionary - } - } - - // Return the final dictionary - return ["userCustomData": userCustomDataDict] - } -} +public typealias PresenceData = JSONValue public protocol Presence: AnyObject, Sendable, EmitsDiscontinuities { func get() async throws -> [PresenceMember] func get(params: PresenceQuery) async throws -> [PresenceMember] func isUserPresent(clientID: String) async throws -> Bool - func enter(data: PresenceData?) async throws + func enter(data: PresenceData) async throws func enter() async throws - func update(data: PresenceData?) async throws + func update(data: PresenceData) async throws func update() async throws - func leave(data: PresenceData?) async throws + func leave(data: PresenceData) async throws func leave() async throws func subscribe(event: PresenceEventType, bufferingPolicy: BufferingPolicy) async -> Subscription /// Same as calling ``subscribe(event:bufferingPolicy:)`` with ``BufferingPolicy.unbounded``. @@ -100,20 +24,6 @@ public protocol Presence: AnyObject, Sendable, EmitsDiscontinuities { func subscribe(events: [PresenceEventType]) async -> Subscription } -public extension Presence { - func enter() async throws { - try await enter(data: nil) - } - - func update() async throws { - try await update(data: nil) - } - - func leave() async throws { - try await leave(data: nil) - } -} - public extension Presence { func subscribe(event: PresenceEventType) async -> Subscription { await subscribe(event: event, bufferingPolicy: .unbounded) @@ -161,6 +71,7 @@ public struct PresenceMember: Sendable { } public var clientID: String + // `nil` means that there is no presence data; this is different to a `JSONValue` of case `.null` public var data: PresenceData? public var action: Action // TODO: (https://github.com/ably-labs/ably-chat-swift/issues/13): try to improve this type diff --git a/Sources/AblyChat/PresenceDataDTO.swift b/Sources/AblyChat/PresenceDataDTO.swift new file mode 100644 index 0000000..48192a1 --- /dev/null +++ b/Sources/AblyChat/PresenceDataDTO.swift @@ -0,0 +1,34 @@ +// (CHA-PR2a) The presence data format is a JSON object as described below. Customers may specify content of an arbitrary type to be placed in the userCustomData field. +internal struct PresenceDataDTO: Equatable { + internal var userCustomData: PresenceData? +} + +// MARK: - Conversion to and from JSONValue + +internal extension PresenceDataDTO { + enum JSONKey: String { + case userCustomData + } + + enum DecodingError: Error { + case valueHasWrongType(key: JSONKey) + } + + init(jsonValue: JSONValue) throws { + guard case let .object(jsonObject) = jsonValue else { + throw DecodingError.valueHasWrongType(key: .userCustomData) + } + + userCustomData = jsonObject[JSONKey.userCustomData.rawValue] + } + + var toJSONObjectValue: [String: JSONValue] { + var result: [String: JSONValue] = [:] + + if let userCustomData { + result[JSONKey.userCustomData.rawValue] = userCustomData + } + + return result + } +} diff --git a/Tests/AblyChatTests/IntegrationTests.swift b/Tests/AblyChatTests/IntegrationTests.swift index 75850d6..7526c5d 100644 --- a/Tests/AblyChatTests/IntegrationTests.swift +++ b/Tests/AblyChatTests/IntegrationTests.swift @@ -205,40 +205,40 @@ struct IntegrationTests { let rxPresenceSubscription = await rxRoom.presence.subscribe(events: [.enter, .leave, .update]) // (2) Send `.enter` presence event with custom data on the other client and check that we receive it on the subscription - try await txRoom.presence.enter(data: .init(userCustomData: ["randomData": .string("randomValue")])) + try await txRoom.presence.enter(data: ["randomData": "randomValue"]) let rxPresenceEnterTxEvent = try #require(await rxPresenceSubscription.first { _ in true }) #expect(rxPresenceEnterTxEvent.action == .enter) - #expect(rxPresenceEnterTxEvent.data?.userCustomData?["randomData"]?.value as? String == "randomValue") + #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 - try await txRoom.presence.update(data: .init(userCustomData: ["randomData": .string("randomValue")])) + try await txRoom.presence.update(data: ["randomData": "randomValue"]) let rxPresenceUpdateTxEvent = try #require(await rxPresenceSubscription.first { _ in true }) #expect(rxPresenceUpdateTxEvent.action == .update) - #expect(rxPresenceUpdateTxEvent.data?.userCustomData?["randomData"]?.value as? String == "randomValue") + #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 - try await txRoom.presence.leave(data: .init(userCustomData: ["randomData": .string("randomValue")])) + try await txRoom.presence.leave(data: ["randomData": "randomValue"]) let rxPresenceLeaveTxEvent = try #require(await rxPresenceSubscription.first { _ in true }) #expect(rxPresenceLeaveTxEvent.action == .leave) - #expect(rxPresenceLeaveTxEvent.data?.userCustomData?["randomData"]?.value as? String == "randomValue") + #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 - try await txRoom.presence.enter(data: .init(userCustomData: ["randomData": .string("randomValue")])) + try await txRoom.presence.enter(data: ["randomData": "randomValue"]) let rxPresenceEnterRxEvent = try #require(await rxPresenceSubscription.first { _ in true }) #expect(rxPresenceEnterRxEvent.action == .enter) - #expect(rxPresenceEnterRxEvent.data?.userCustomData?["randomData"]?.value as? String == "randomValue") + #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 - try await txRoom.presence.update(data: .init(userCustomData: ["randomData": .string("randomValue")])) + try await txRoom.presence.update(data: ["randomData": "randomValue"]) let rxPresenceUpdateRxEvent = try #require(await rxPresenceSubscription.first { _ in true }) #expect(rxPresenceUpdateRxEvent.action == .update) - #expect(rxPresenceUpdateRxEvent.data?.userCustomData?["randomData"]?.value as? String == "randomValue") + #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 - try await txRoom.presence.leave(data: .init(userCustomData: ["randomData": .string("randomValue")])) + try await txRoom.presence.leave(data: ["randomData": "randomValue"]) let rxPresenceLeaveRxEvent = try #require(await rxPresenceSubscription.first { _ in true }) #expect(rxPresenceLeaveRxEvent.action == .leave) - #expect(rxPresenceLeaveRxEvent.data?.userCustomData?["randomData"]?.value as? String == "randomValue") + #expect(rxPresenceLeaveRxEvent.data == ["randomData": "randomValue"]) // MARK: - Typing Indicators diff --git a/Tests/AblyChatTests/JSONValueTests.swift b/Tests/AblyChatTests/JSONValueTests.swift new file mode 100644 index 0000000..8572524 --- /dev/null +++ b/Tests/AblyChatTests/JSONValueTests.swift @@ -0,0 +1,145 @@ +@testable import AblyChat +import Foundation +import Testing + +struct JSONValueTests { + // MARK: Conversion from ably-cocoa presence data + + @Test(arguments: [ + // object + (ablyCocoaPresenceData: ["someKey": "someValue"], expectedResult: ["someKey": "someValue"]), + // array + (ablyCocoaPresenceData: ["someElement"], expectedResult: ["someElement"]), + // string + (ablyCocoaPresenceData: "someString", expectedResult: "someString"), + // number + (ablyCocoaPresenceData: NSNumber(value: 123), expectedResult: 123), + (ablyCocoaPresenceData: NSNumber(value: 123.456), expectedResult: 123.456), + // bool + (ablyCocoaPresenceData: NSNumber(value: true), expectedResult: true), + (ablyCocoaPresenceData: NSNumber(value: false), expectedResult: false), + // null + (ablyCocoaPresenceData: NSNull(), expectedResult: .null), + ] as[(ablyCocoaPresenceData: Sendable, expectedResult: JSONValue?)]) + func initWithAblyCocoaPresenceData(ablyCocoaPresenceData: Sendable, expectedResult: JSONValue?) { + #expect(JSONValue(ablyCocoaPresenceData: ablyCocoaPresenceData) == expectedResult) + } + + // Tests that it correctly handles an object deserialized by `JSONSerialization` (which is what ably-cocoa uses for deserialization). + @Test + func initWithAblyCocoaPresenceData_endToEnd() throws { + let jsonString = """ + { + "someArray": [ + { + "someStringKey": "someString", + "someIntegerKey": 123, + "someFloatKey": 123.456, + "someTrueKey": true, + "someFalseKey": false, + "someNullKey": null + }, + "someOtherArrayElement" + ], + "someNestedObject": { + "someOtherKey": "someOtherValue" + } + } + """ + + let ablyCocoaPresenceData = try JSONSerialization.jsonObject(with: #require(jsonString.data(using: .utf8))) + + let expected: JSONValue = [ + "someArray": [ + [ + "someStringKey": "someString", + "someIntegerKey": 123, + "someFloatKey": 123.456, + "someTrueKey": true, + "someFalseKey": false, + "someNullKey": .null, + ], + "someOtherArrayElement", + ], + "someNestedObject": [ + "someOtherKey": "someOtherValue", + ], + ] + + #expect(JSONValue(ablyCocoaPresenceData: ablyCocoaPresenceData) == expected) + } + + // MARK: Conversion to ably-cocoa presence data + + @Test(arguments: [ + // object + (value: ["someKey": "someValue"], expectedResult: ["someKey": "someValue"]), + // array + (value: ["someElement"], expectedResult: ["someElement"]), + // string + (value: "someString", expectedResult: "someString"), + // number + (value: 123, expectedResult: NSNumber(value: 123)), + (value: 123.456, expectedResult: NSNumber(value: 123.456)), + // bool + (value: true, expectedResult: NSNumber(value: true)), + (value: false, expectedResult: NSNumber(value: false)), + // 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) + 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 { + let value: JSONValue = [ + "someArray": [ + [ + "someStringKey": "someString", + "someIntegerKey": 123, + "someFloatKey": 123.456, + "someTrueKey": true, + "someFalseKey": false, + "someNullKey": .null, + ], + "someOtherArrayElement", + ], + "someNestedObject": [ + "someOtherKey": "someOtherValue", + ], + ] + + let expectedJSONString = """ + { + "someArray": [ + { + "someStringKey": "someString", + "someIntegerKey": 123, + "someFloatKey": 123.456, + "someTrueKey": true, + "someFalseKey": false, + "someNullKey": null + }, + "someOtherArrayElement" + ], + "someNestedObject": { + "someOtherKey": "someOtherValue" + } + } + """ + + let jsonSerializationOptions: JSONSerialization.WritingOptions = [.sortedKeys] + + let valueData = try JSONSerialization.data(withJSONObject: value.toAblyCocoaPresenceData, options: jsonSerializationOptions) + let expectedData = try { + let serialized = try JSONSerialization.jsonObject(with: #require(expectedJSONString.data(using: .utf8))) + return try JSONSerialization.data(withJSONObject: serialized, options: jsonSerializationOptions) + }() + + #expect(valueData == expectedData) + } +} diff --git a/Tests/AblyChatTests/PresenceDataDTOTests.swift b/Tests/AblyChatTests/PresenceDataDTOTests.swift new file mode 100644 index 0000000..943d507 --- /dev/null +++ b/Tests/AblyChatTests/PresenceDataDTOTests.swift @@ -0,0 +1,41 @@ +@testable import AblyChat +import Testing + +struct PresenceDataDTOTests { + // MARK: - Creating from JSON value + + @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 + (jsonValue: [:], expectedResult: .init(userCustomData: nil)), + // Confirm that an arbitrary non-`.null` userCustomData is extracted correctly + (jsonValue: ["userCustomData": "hello"], expectedResult: .init(userCustomData: "hello")), + // Confirm that `.null` userCustomData is treated like any other JSON value + (jsonValue: ["userCustomData": .null], expectedResult: .init(userCustomData: .null)), + ] as[(jsonValue: JSONValue, expectedResult: PresenceDataDTO)]) + func initWithJSONValue(jsonValue: JSONValue, expectedResult: PresenceDataDTO) throws { + #expect(try PresenceDataDTO(jsonValue: jsonValue) == expectedResult) + } + + func initWithJSONValue_failsIfNotObject() { + #expect(throws: PresenceDataDTO.DecodingError.self) { + try PresenceDataDTO(jsonValue: "hello") + } + } + + // MARK: - Conversion to JSON object value + + @Test( + arguments: [ + // If user doesn’t pass any data to the presence operation, the resulting JSON object should contain no `userCustomData` key + (userCustomData: nil, expectedJSONObject: [:]), + // Confirm that an arbitrary non-`.null` JSON value is treated correctly + (userCustomData: "hello", expectedJSONObject: ["userCustomData": "hello"]), + // Confirm that `.null` is treated like any other JSON value; i.e. if the user passes `.null` as the data of a presence operation, then the resulting JSON object has `"userCustomData": .null` + (userCustomData: .null, expectedJSONObject: ["userCustomData": .null]), + ] as[(userCustomData: PresenceData?, expectedJSONObject: [String: JSONValue])] + ) + func toJSONObject(userCustomData: PresenceData?, expectedJSONObject: [String: JSONValue]) { + let dto = PresenceDataDTO(userCustomData: userCustomData) + #expect(dto.toJSONObjectValue == expectedJSONObject) + } +}