Merge pull request #103 from ably-labs/47-groundwork

[ECO-4982] Some groundwork for integrating room lifecycle manager
ably · Nov 11, 2024 · ac67cea · ac67cea
2 parents 26b38a4 + 628b85a
commit ac67cea
Show file tree

Hide file tree

Showing 14 changed files with 146 additions and 148 deletions.
diff --git a/Example/AblyChatExample/Mocks/MockRealtime.swift b/Example/AblyChatExample/Mocks/MockRealtime.swift
@@ -18,10 +18,6 @@ final class MockRealtime: NSObject, RealtimeClientProtocol, Sendable {
             fatalError("Not implemented")
         }
 
-        func get(_: String) -> Channel {
-            fatalError("Not implemented")
-        }
-
         func exists(_: String) -> Bool {
             fatalError("Not implemented")
         }

diff --git a/Sources/AblyChat/AblyCocoaExtensions/Ably+Concurrency.swift b/Sources/AblyChat/AblyCocoaExtensions/Ably+Concurrency.swift
@@ -4,27 +4,27 @@ import Ably
 // TODO: remove once we improve this experience in ably-cocoa (https://github.com/ably/ably-cocoa/issues/1967)
 
 internal extension ARTRealtimeChannelProtocol {
-    func attachAsync() async throws {
-        try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, _>) in
+    func attachAsync() async throws(ARTErrorInfo) {
+        try await withCheckedContinuation { (continuation: CheckedContinuation<Result<Void, ARTErrorInfo>, _>) in
             attach { error in
                 if let error {
-                    continuation.resume(throwing: error)
+                    continuation.resume(returning: .failure(error))
                 } else {
-                    continuation.resume()
+                    continuation.resume(returning: .success(()))
                 }
             }
-        }
+        }.get()
     }
 
-    func detachAsync() async throws {
-        try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, _>) in
+    func detachAsync() async throws(ARTErrorInfo) {
+        try await withCheckedContinuation { (continuation: CheckedContinuation<Result<Void, ARTErrorInfo>, _>) in
             detach { error in
                 if let error {
-                    continuation.resume(throwing: error)
+                    continuation.resume(returning: .failure(error))
                 } else {
-                    continuation.resume()
+                    continuation.resume(returning: .success(()))
                 }
             }
-        }
+        }.get()
     }
 }
diff --git a/Sources/AblyChat/ChatAPI.swift b/Sources/AblyChat/ChatAPI.swift
@@ -8,10 +8,6 @@ internal final class ChatAPI: Sendable {
         self.realtime = realtime
     }
 
-    internal func getChannel(_ name: String) -> any RealtimeChannelProtocol {
-        realtime.getChannel(name)
-    }
-
     // (CHA-M6) Messages should be queryable from a paginated REST API.
     internal func getMessages(roomId: String, params: QueryOptions) async throws -> any PaginatedResult<Message> {
         let endpoint = "\(apiVersion)/rooms/\(roomId)/messages"

diff --git a/Sources/AblyChat/DefaultMessages.swift b/Sources/AblyChat/DefaultMessages.swift
@@ -13,23 +13,20 @@ private struct MessageSubscriptionWrapper {
 @MainActor
 internal final class DefaultMessages: Messages, EmitsDiscontinuities {
     private let roomID: String
-    public let channel: RealtimeChannelProtocol
+    public nonisolated let channel: RealtimeChannelProtocol
     private let chatAPI: ChatAPI
     private let clientID: String
 
     // TODO: https://github.com/ably-labs/ably-chat-swift/issues/36 - Handle unsubscribing in line with CHA-M4b
     // UUID acts as a unique identifier for each listener/subscription. MessageSubscriptionWrapper houses the subscription and the timeserial of when it was attached or resumed.
     private var subscriptionPoints: [UUID: MessageSubscriptionWrapper] = [:]
 
-    internal nonisolated init(chatAPI: ChatAPI, roomID: String, clientID: String) async {
+    internal nonisolated init(channel: RealtimeChannelProtocol, chatAPI: ChatAPI, roomID: String, clientID: String) async {
+        self.channel = channel
         self.chatAPI = chatAPI
         self.roomID = roomID
         self.clientID = clientID
 
-        // (CHA-M1) Chat messages for a Room are sent on a corresponding realtime channel <roomId>::$chat::$chatMessages. For example, if your room id is my-room then the messages channel will be my-room::$chat::$chatMessages.
-        let messagesChannelName = "\(roomID)::$chat::$chatMessages"
-        channel = chatAPI.getChannel(messagesChannelName)
-
         // Implicitly handles channel events and therefore listners within this class. Alternative is to explicitly call something like `DefaultMessages.start()` which makes the SDK more cumbersome to interact with. This class is useless without kicking off this flow so I think leaving it here is suitable.
         // "Calls to instance method 'handleChannelEvents(roomId:)' from outside of its actor context are implicitly asynchronous" hence the `await` here.
         await handleChannelEvents(roomId: roomID)

diff --git a/Sources/AblyChat/Dependencies.swift b/Sources/AblyChat/Dependencies.swift
@@ -14,9 +14,8 @@ public protocol RealtimeClientProtocol: ARTRealtimeProtocol, Sendable {
 public protocol RealtimeChannelsProtocol: ARTRealtimeChannelsProtocol, Sendable {
     associatedtype Channel: RealtimeChannelProtocol
 
-    // It’s not clear to me why ARTRealtimeChannelsProtocol doesn’t include this property (https://github.com/ably/ably-cocoa/issues/1968).
+    // It’s not clear to me why ARTRealtimeChannelsProtocol doesn’t include this function (https://github.com/ably/ably-cocoa/issues/1968).
     func get(_ name: String, options: ARTRealtimeChannelOptions) -> Channel
-    func get(_ name: String) -> Channel
 }
 
 /// Expresses the requirements of the object returned by ``RealtimeChannelsProtocol.get(_:)``.

diff --git a/Sources/AblyChat/Room.swift b/Sources/AblyChat/Room.swift
@@ -39,6 +39,9 @@ internal actor DefaultRoom: Room {
     // Exposed for testing.
     private nonisolated let realtime: RealtimeClient
 
+    /// The channels that contribute to this room.
+    private let channels: [RoomFeature: RealtimeChannelProtocol]
+
     #if DEBUG
         internal nonisolated var testsOnly_realtime: RealtimeClient {
             realtime
@@ -61,13 +64,23 @@ internal actor DefaultRoom: Room {
             throw ARTErrorInfo.create(withCode: 40000, message: "Ensure your Realtime instance is initialized with a clientId.")
         }
 
+        channels = Self.createChannels(roomID: roomID, realtime: realtime)
+
         messages = await DefaultMessages(
+            channel: channels[.messages]!,
             chatAPI: chatAPI,
             roomID: roomID,
             clientID: clientId
         )
     }
 
+    private static func createChannels(roomID: String, realtime: RealtimeClient) -> [RoomFeature: RealtimeChannelProtocol] {
+        .init(uniqueKeysWithValues: [RoomFeature.messages, RoomFeature.typing, RoomFeature.reactions].map { feature in
+            let channel = realtime.getChannel(feature.channelNameForRoomID(roomID))
+            return (feature, channel)
+        })
+    }
+
     public nonisolated var presence: any Presence {
         fatalError("Not yet implemented")
     }
@@ -84,19 +97,8 @@ internal actor DefaultRoom: Room {
         fatalError("Not yet implemented")
     }
 
-    /// Fetches the channels that contribute to this room.
-    private func channels() -> [any RealtimeChannelProtocol] {
-        [
-            "chatMessages",
-            "typingIndicators",
-            "reactions",
-        ].map { suffix in
-            realtime.channels.get("\(roomID)::$chat::$\(suffix)")
-        }
-    }
-
     public func attach() async throws {
-        for channel in channels() {
+        for channel in channels.map(\.value) {
             do {
                 try await channel.attachAsync()
             } catch {
@@ -108,7 +110,7 @@ internal actor DefaultRoom: Room {
     }
 
     public func detach() async throws {
-        for channel in channels() {
+        for channel in channels.map(\.value) {
             do {
                 try await channel.detachAsync()
             } catch {

diff --git a/Sources/AblyChat/RoomFeature.swift b/Sources/AblyChat/RoomFeature.swift
@@ -5,4 +5,23 @@ internal enum RoomFeature {
     case reactions
     case occupancy
     case typing
+
+    internal func channelNameForRoomID(_ roomID: String) -> String {
+        "\(roomID)::$chat::$\(channelNameSuffix)"
+    }
+
+    private var channelNameSuffix: String {
+        switch self {
+        case .messages:
+            // (CHA-M1) Chat messages for a Room are sent on a corresponding realtime channel <roomId>::$chat::$chatMessages. For example, if your room id is my-room then the messages channel will be my-room::$chat::$chatMessages.
+            "chatMessages"
+        case .typing:
+            "typingIndicators"
+        case .reactions:
+            "reactions"
+        case .presence, .occupancy:
+            // We’ll add these, with reference to the relevant spec points, as we implement these features
+            fatalError("Don’t know channel name suffix for room feature \(self)")
+        }
+    }
 }
diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift
@@ -40,7 +40,9 @@ internal protocol RoomLifecycleContributor: Identifiable, Sendable {
     func emitDiscontinuity(_ error: ARTErrorInfo) async
 }
 
-internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
+internal protocol RoomLifecycleManager: Sendable {}
+
+internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor>: RoomLifecycleManager {
     // MARK: - Constant properties
 
     private let logger: InternalLogger
@@ -583,7 +585,7 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
 
     /// Executes a function that represents a room lifecycle operation.
     ///
-    /// - Note: Note that `RoomLifecycleManager` does not implement any sort of mutual exclusion mechanism that _enforces_ that one room lifecycle operation must wait for another (e.g. it is _not_ a queue); each operation needs to implement its own logic for whether it should proceed in the presence of other in-progress operations.
+    /// - Note: Note that `DefaultRoomLifecycleManager` does not implement any sort of mutual exclusion mechanism that _enforces_ that one room lifecycle operation must wait for another (e.g. it is _not_ a queue); each operation needs to implement its own logic for whether it should proceed in the presence of other in-progress operations.
     ///
     /// - Parameters:
     ///   - forcedOperationID: Forces the operation to have a given ID. In combination with the ``testsOnly_subscribeToOperationWaitEvents`` API, this allows tests to verify that one test-initiated operation is waiting for another test-initiated operation.

diff --git a/Tests/AblyChatTests/ChatAPITests.swift b/Tests/AblyChatTests/ChatAPITests.swift
@@ -3,24 +3,6 @@ import Ably
 import Testing
 
 struct ChatAPITests {
-    // MARK: getChannel Tests
-
-    // @spec CHA-M1
-    @Test
-    func getChannel_returnsChannel() {
-        // Given
-        let realtime = MockRealtime.create(
-            channels: .init(channels: [.init(name: "basketball::$chat::$chatMessages")])
-        )
-        let chatAPI = ChatAPI(realtime: realtime)
-
-        // When
-        let channel = chatAPI.getChannel("basketball::$chat::$chatMessages")
-
-        // Then
-        #expect(channel.name == "basketball::$chat::$chatMessages")
-    }
-
     // MARK: sendMessage Tests
 
     // @spec CHA-M3c

diff --git a/Tests/AblyChatTests/DefaultMessagesTests.swift b/Tests/AblyChatTests/DefaultMessagesTests.swift
@@ -3,30 +3,15 @@ import Ably
 import Testing
 
 struct DefaultMessagesTests {
-    // @spec CHA-M1
-    @Test
-    func init_channelNameIsSetAsMessagesChannelName() async throws {
-        // clientID value is arbitrary
-
-        // Given
-        let realtime = MockRealtime.create(channels: .init(channels: [.init(name: "basketball::$chat::$chatMessages")]))
-        let chatAPI = ChatAPI(realtime: realtime)
-
-        // When
-        let defaultMessages = await DefaultMessages(chatAPI: chatAPI, roomID: "basketball", clientID: "clientId")
-
-        // Then
-        await #expect(defaultMessages.channel.name == "basketball::$chat::$chatMessages")
-    }
-
     @Test
     func subscribe_whenChannelIsAttachedAndNoChannelSerial_throwsError() async throws {
         // roomId and clientId values are arbitrary
 
         // Given
-        let realtime = MockRealtime.create(channels: .init(channels: [.init(name: "basketball::$chat::$chatMessages")]))
+        let realtime = MockRealtime.create()
         let chatAPI = ChatAPI(realtime: realtime)
-        let defaultMessages = await DefaultMessages(chatAPI: chatAPI, roomID: "basketball", clientID: "clientId")
+        let channel = MockRealtimeChannel()
+        let defaultMessages = await DefaultMessages(channel: channel, chatAPI: chatAPI, roomID: "basketball", clientID: "clientId")
 
         // Then
         await #expect(throws: ARTErrorInfo.create(withCode: 40000, status: 400, message: "channel is attached, but channelSerial is not defined"), performing: {
@@ -40,11 +25,10 @@ struct DefaultMessagesTests {
         // Message response of succcess with no items, and roomId are arbitrary
 
         // Given
-        let realtime = MockRealtime.create(
-            channels: .init(channels: [.init(name: "basketball::$chat::$chatMessages")])
-        ) { (MockHTTPPaginatedResponse.successGetMessagesWithNoItems, nil) }
+        let realtime = MockRealtime.create { (MockHTTPPaginatedResponse.successGetMessagesWithNoItems, nil) }
         let chatAPI = ChatAPI(realtime: realtime)
-        let defaultMessages = await DefaultMessages(chatAPI: chatAPI, roomID: "basketball", clientID: "clientId")
+        let channel = MockRealtimeChannel()
+        let defaultMessages = await DefaultMessages(channel: channel, chatAPI: chatAPI, roomID: "basketball", clientID: "clientId")
 
         // Then
         await #expect(throws: Never.self, performing: {
@@ -60,21 +44,15 @@ struct DefaultMessagesTests {
         // all setup values here are arbitrary
 
         // Given
-        let realtime = MockRealtime.create(
-            channels: .init(
-                channels: [
-                    .init(
-                        name: "basketball::$chat::$chatMessages",
-                        properties: .init(
-                            attachSerial: "001",
-                            channelSerial: "001"
-                        )
-                    ),
-                ]
-            )
-        ) { (MockHTTPPaginatedResponse.successGetMessagesWithNoItems, nil) }
+        let realtime = MockRealtime.create { (MockHTTPPaginatedResponse.successGetMessagesWithNoItems, nil) }
         let chatAPI = ChatAPI(realtime: realtime)
-        let defaultMessages = await DefaultMessages(chatAPI: chatAPI, roomID: "basketball", clientID: "clientId")
+        let channel = MockRealtimeChannel(
+            properties: .init(
+                attachSerial: "001",
+                channelSerial: "001"
+            )
+        )
+        let defaultMessages = await DefaultMessages(channel: channel, chatAPI: chatAPI, roomID: "basketball", clientID: "clientId")
         let subscription = try await defaultMessages.subscribe(bufferingPolicy: .unbounded)
         let expectedPaginatedResult = PaginatedResultWrapper<Message>(
             paginatedResponse: MockHTTPPaginatedResponse.successGetMessagesWithNoItems,