Make room responsible for channel fetching

The room needs to have access to its contributing channels so that, when do we integrate the RoomLifecycleManager into the SDK in #47, it can use them to create a RoomLifecycleManager instance. So, let’s make the room fetch the channels and then pass them to the features. This way, we only fetch each channel once. Also, in an upcoming commit it will be the room who releases the channels when `release()` is called on it, giving another reason why it makes sense for the room to fetch the channels, I think.
ably · Nov 10, 2024 · e8dfc83 · e8dfc83
1 parent 33c4f6c
commit e8dfc83
Show file tree

Hide file tree

Showing 11 changed files with 87 additions and 91 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/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 these functions (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/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,

diff --git a/Tests/AblyChatTests/DefaultRoomTests.swift b/Tests/AblyChatTests/DefaultRoomTests.swift
@@ -3,6 +3,25 @@ import Ably
 import Testing
 
 struct DefaultRoomTests {
+    // MARK: - Features
+
+    // @spec CHA-M1
+    @Test
+    func messagesChannelName() async throws {
+        // Given: a DefaultRoom instance
+        let channelsList = [
+            MockRealtimeChannel(name: "basketball::$chat::$chatMessages", attachResult: .success),
+            MockRealtimeChannel(name: "basketball::$chat::$typingIndicators", attachResult: .success),
+            MockRealtimeChannel(name: "basketball::$chat::$reactions", attachResult: .success),
+        ]
+        let channels = MockChannels(channels: channelsList)
+        let realtime = MockRealtime.create(channels: channels)
+        let room = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: .init(), logger: TestLogger())
+
+        // Then
+        #expect(room.messages.channel.name == "basketball::$chat::$chatMessages")
+    }
+
     // MARK: - Attach
 
     @Test

diff --git a/Tests/AblyChatTests/DefaultRoomsTests.swift b/Tests/AblyChatTests/DefaultRoomsTests.swift
@@ -7,7 +7,11 @@ struct DefaultRoomsTests {
     @Test
     func get_returnsRoomWithGivenID() async throws {
         // Given: an instance of DefaultRooms
-        let realtime = MockRealtime.create(channels: .init(channels: [.init(name: "basketball::$chat::$chatMessages")]))
+        let realtime = MockRealtime.create(channels: .init(channels: [
+            .init(name: "basketball::$chat::$chatMessages"),
+            .init(name: "basketball::$chat::$typingIndicators"),
+            .init(name: "basketball::$chat::$reactions"),
+        ]))
         let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger())
 
         // When: get(roomID:options:) is called
@@ -26,7 +30,11 @@ struct DefaultRoomsTests {
     @Test
     func get_returnsExistingRoomWithGivenID() async throws {
         // Given: an instance of DefaultRooms, on which get(roomID:options:) has already been called with a given ID
-        let realtime = MockRealtime.create(channels: .init(channels: [.init(name: "basketball::$chat::$chatMessages")]))
+        let realtime = MockRealtime.create(channels: .init(channels: [
+            .init(name: "basketball::$chat::$chatMessages"),
+            .init(name: "basketball::$chat::$typingIndicators"),
+            .init(name: "basketball::$chat::$reactions"),
+        ]))
         let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger())
 
         let roomID = "basketball"
@@ -44,7 +52,11 @@ struct DefaultRoomsTests {
     @Test
     func get_throwsErrorWhenOptionsDoNotMatch() async throws {
         // Given: an instance of DefaultRooms, on which get(roomID:options:) has already been called with a given ID and options
-        let realtime = MockRealtime.create(channels: .init(channels: [.init(name: "basketball::$chat::$chatMessages")]))
+        let realtime = MockRealtime.create(channels: .init(channels: [
+            .init(name: "basketball::$chat::$chatMessages"),
+            .init(name: "basketball::$chat::$typingIndicators"),
+            .init(name: "basketball::$chat::$reactions"),
+        ]))
         let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger())
 
         let roomID = "basketball"

diff --git a/Tests/AblyChatTests/Mocks/MockChannels.swift b/Tests/AblyChatTests/Mocks/MockChannels.swift
@@ -8,18 +8,14 @@ final class MockChannels: RealtimeChannelsProtocol, Sendable {
         self.channels = channels
     }
 
-    func get(_ name: String) -> MockRealtimeChannel {
+    func get(_ name: String, options _: ARTRealtimeChannelOptions) -> MockRealtimeChannel {
         guard let channel = (channels.first { $0.name == name }) else {
             fatalError("There is no mock channel with name \(name)")
         }
 
         return channel
     }
 
-    func get(_ name: String, options _: ARTRealtimeChannelOptions) -> MockRealtimeChannel {
-        get(name)
-    }
-
     func exists(_: String) -> Bool {
         fatalError("Not implemented")
     }