From 670cc381232b395dcda39947da9c13f6ab287884 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 19 Nov 2024 11:20:28 -0300 Subject: [PATCH 1/4] Make version-related stuff internal None of this stuff is public in JS, from where it was copied. --- Sources/AblyChat/Version.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/AblyChat/Version.swift b/Sources/AblyChat/Version.swift index 98dab3cd..3beefbe4 100644 --- a/Sources/AblyChat/Version.swift +++ b/Sources/AblyChat/Version.swift @@ -4,13 +4,13 @@ import Ably // Update this when you release a new version // Version information -public let version = "0.1.0" +internal let version = "0.1.0" // Channel options agent string -public let channelOptionsAgentString = "chat-ios/\(version)" +internal let channelOptionsAgentString = "chat-ios/\(version)" // Default channel options -public var defaultChannelOptions: ARTRealtimeChannelOptions { +internal var defaultChannelOptions: ARTRealtimeChannelOptions { let options = ARTRealtimeChannelOptions() options.params = ["agent": channelOptionsAgentString] return options From b2f39b523e33d209650f20b561a778377e6c386e Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 19 Nov 2024 11:21:26 -0300 Subject: [PATCH 2/4] Remove some comments that add no information --- Sources/AblyChat/Version.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Sources/AblyChat/Version.swift b/Sources/AblyChat/Version.swift index 3beefbe4..ef37c3dc 100644 --- a/Sources/AblyChat/Version.swift +++ b/Sources/AblyChat/Version.swift @@ -6,10 +6,8 @@ import Ably // Version information internal let version = "0.1.0" -// Channel options agent string internal let channelOptionsAgentString = "chat-ios/\(version)" -// Default channel options internal var defaultChannelOptions: ARTRealtimeChannelOptions { let options = ARTRealtimeChannelOptions() options.params = ["agent": channelOptionsAgentString] From 334420641c6fcc69552e9341bf4e4d51091a33d9 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 19 Nov 2024 15:13:45 -0300 Subject: [PATCH 3/4] Create a channel options type with value semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We shouldn’t be mutating the channel options that are passed in to `getChannel(…)` (violates caller’s expectations), and ably-cocoa offers us no mechanism for copying an ARTRealtimeChannelOptions object. --- Sources/AblyChat/Dependencies.swift | 45 +++++++++++++++++++---------- Sources/AblyChat/Room.swift | 2 +- Sources/AblyChat/Version.swift | 6 +--- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/Sources/AblyChat/Dependencies.swift b/Sources/AblyChat/Dependencies.swift index 38533818..e1da254f 100644 --- a/Sources/AblyChat/Dependencies.swift +++ b/Sources/AblyChat/Dependencies.swift @@ -21,25 +21,38 @@ public protocol RealtimeChannelsProtocol: ARTRealtimeChannelsProtocol, Sendable /// Expresses the requirements of the object returned by ``RealtimeChannelsProtocol.get(_:)``. public protocol RealtimeChannelProtocol: ARTRealtimeChannelProtocol, Sendable {} +/// Like (a subset of) `ARTRealtimeChannelOptions` but with value semantics. (It’s unfortunate that `ARTRealtimeChannelOptions` doesn’t have a `-copy` method.) +internal struct RealtimeChannelOptions { + internal var modes: ARTChannelMode + internal var params: [String: String]? + + internal init() { + // Get our default values from ably-cocoa + let artRealtimeChannelOptions = ARTRealtimeChannelOptions() + modes = artRealtimeChannelOptions.modes + params = artRealtimeChannelOptions.params + } + + internal var toARTRealtimeChannelOptions: ARTRealtimeChannelOptions { + let result = ARTRealtimeChannelOptions() + result.modes = modes + result.params = params + return result + } +} + internal extension RealtimeClientProtocol { // Function to get the channel with merged options - func getChannel(_ name: String, opts: ARTRealtimeChannelOptions? = nil) -> any RealtimeChannelProtocol { - // Create a new instance of ARTRealtimeChannelOptions if opts is nil - let resolvedOptions = ARTRealtimeChannelOptions() - - // Merge params if available, using opts first, then defaultChannelOptions as fallback - resolvedOptions.params = (opts?.params ?? [:]).merging( - defaultChannelOptions.params ?? [:] - ) { _, new in new } - - // Apply other options from `opts` if necessary - if let customOpts = opts { - resolvedOptions.modes = customOpts.modes - resolvedOptions.cipher = customOpts.cipher - resolvedOptions.attachOnSubscribe = customOpts.attachOnSubscribe + func getChannel(_ name: String, opts: RealtimeChannelOptions? = nil) -> any RealtimeChannelProtocol { + var resolvedOptions = opts ?? .init() + + // Add in the default params + resolvedOptions.params = (resolvedOptions.params ?? [:]).merging( + defaultChannelParams + ) { _, new + in new } - // Return the resolved channel - return channels.get(name, options: resolvedOptions) + return channels.get(name, options: resolvedOptions.toARTRealtimeChannelOptions) } } diff --git a/Sources/AblyChat/Room.swift b/Sources/AblyChat/Room.swift index 0b151664..e0c724b2 100644 --- a/Sources/AblyChat/Room.swift +++ b/Sources/AblyChat/Room.swift @@ -138,7 +138,7 @@ internal actor DefaultRoom RoomFeature.presence, RoomFeature.occupancy, ].map { feature in - let channelOptions = ARTRealtimeChannelOptions() + var channelOptions = RealtimeChannelOptions() // channel setup for presence and occupancy if feature == .presence { diff --git a/Sources/AblyChat/Version.swift b/Sources/AblyChat/Version.swift index ef37c3dc..ca79481c 100644 --- a/Sources/AblyChat/Version.swift +++ b/Sources/AblyChat/Version.swift @@ -8,8 +8,4 @@ internal let version = "0.1.0" internal let channelOptionsAgentString = "chat-ios/\(version)" -internal var defaultChannelOptions: ARTRealtimeChannelOptions { - let options = ARTRealtimeChannelOptions() - options.params = ["agent": channelOptionsAgentString] - return options -} +internal let defaultChannelParams = ["agent": channelOptionsAgentString] From cf5b20db0be56d215a02d16a9d067c2b3fd2a55c Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 19 Nov 2024 11:48:48 -0300 Subject: [PATCH 4/4] Implement CHA-GP2a (disable implicit attach) Resolves #102. --- Sources/AblyChat/Dependencies.swift | 6 ++++++ Tests/AblyChatTests/DefaultRoomTests.swift | 20 ++++++++++++++++++++ Tests/AblyChatTests/Mocks/MockChannels.swift | 16 +++++++++++++++- 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/Sources/AblyChat/Dependencies.swift b/Sources/AblyChat/Dependencies.swift index e1da254f..7b0ae8e1 100644 --- a/Sources/AblyChat/Dependencies.swift +++ b/Sources/AblyChat/Dependencies.swift @@ -25,18 +25,21 @@ public protocol RealtimeChannelProtocol: ARTRealtimeChannelProtocol, Sendable {} internal struct RealtimeChannelOptions { internal var modes: ARTChannelMode internal var params: [String: String]? + internal var attachOnSubscribe: Bool internal init() { // Get our default values from ably-cocoa let artRealtimeChannelOptions = ARTRealtimeChannelOptions() modes = artRealtimeChannelOptions.modes params = artRealtimeChannelOptions.params + attachOnSubscribe = artRealtimeChannelOptions.attachOnSubscribe } internal var toARTRealtimeChannelOptions: ARTRealtimeChannelOptions { let result = ARTRealtimeChannelOptions() result.modes = modes result.params = params + result.attachOnSubscribe = attachOnSubscribe return result } } @@ -53,6 +56,9 @@ internal extension RealtimeClientProtocol { in new } + // CHA-GP2a + resolvedOptions.attachOnSubscribe = false + return channels.get(name, options: resolvedOptions.toARTRealtimeChannelOptions) } } diff --git a/Tests/AblyChatTests/DefaultRoomTests.swift b/Tests/AblyChatTests/DefaultRoomTests.swift index 36fa48a2..c3f85e84 100644 --- a/Tests/AblyChatTests/DefaultRoomTests.swift +++ b/Tests/AblyChatTests/DefaultRoomTests.swift @@ -3,6 +3,26 @@ import Ably import Testing struct DefaultRoomTests { + // MARK: - Fetching channels + + // @spec CHA-GP2a + @Test + func disablesImplicitAttach() async throws { + // Given: A DefaultRoom instance + let channelsList = [ + MockRealtimeChannel(name: "basketball::$chat::$chatMessages", attachResult: .success), + MockRealtimeChannel(name: "basketball::$chat::$reactions", attachResult: .success), + ] + let channels = MockChannels(channels: channelsList) + let realtime = MockRealtime.create(channels: channels) + _ = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: .init(), logger: TestLogger(), lifecycleManagerFactory: MockRoomLifecycleManagerFactory()) + + // Then: When it fetches a channel, it does so with the `attachOnSubscribe` channel option set to false + let channelsGetArguments = channels.getArguments + #expect(!channelsGetArguments.isEmpty) + #expect(channelsGetArguments.allSatisfy { $0.options.attachOnSubscribe == false }) + } + // MARK: - Features // @spec CHA-M1 diff --git a/Tests/AblyChatTests/Mocks/MockChannels.swift b/Tests/AblyChatTests/Mocks/MockChannels.swift index 8afb6f00..dbd12b00 100644 --- a/Tests/AblyChatTests/Mocks/MockChannels.swift +++ b/Tests/AblyChatTests/Mocks/MockChannels.swift @@ -5,13 +5,19 @@ final class MockChannels: RealtimeChannelsProtocol, @unchecked Sendable { private let channels: [MockRealtimeChannel] private let mutex = NSLock() /// Access must be synchronized via ``mutex``. + private(set) var _getArguments: [(name: String, options: ARTRealtimeChannelOptions)] = [] + /// Access must be synchronized via ``mutex``. private(set) var _releaseArguments: [String] = [] init(channels: [MockRealtimeChannel]) { self.channels = channels } - func get(_ name: String, options _: ARTRealtimeChannelOptions) -> MockRealtimeChannel { + func get(_ name: String, options: ARTRealtimeChannelOptions) -> MockRealtimeChannel { + mutex.lock() + _getArguments.append((name: name, options: options)) + mutex.unlock() + guard let channel = (channels.first { $0.name == name }) else { fatalError("There is no mock channel with name \(name)") } @@ -19,6 +25,14 @@ final class MockChannels: RealtimeChannelsProtocol, @unchecked Sendable { return channel } + var getArguments: [(name: String, options: ARTRealtimeChannelOptions)] { + let result: [(name: String, options: ARTRealtimeChannelOptions)] + mutex.lock() + result = _getArguments + mutex.unlock() + return result + } + func exists(_: String) -> Bool { fatalError("Not implemented") }