Merge pull request #107 from ably-labs/47-implement-release

[ECO-4982] Implement room release

Author: lawrence-forooghian

Sources/AblyChat/ChatClient.swift

@@ -20,8 +20,8 @@ public actor DefaultChatClient: ChatClient {
         self.realtime = realtime
         self.clientOptions = clientOptions ?? .init()
         logger = DefaultInternalLogger(logHandler: self.clientOptions.logHandler, logLevel: self.clientOptions.logLevel)
-        let roomLifecycleManagerFactory = DefaultRoomLifecycleManagerFactory()
-        rooms = DefaultRooms(realtime: realtime, clientOptions: self.clientOptions, logger: logger, lifecycleManagerFactory: roomLifecycleManagerFactory)
+        let roomFactory = DefaultRoomFactory()
+        rooms = DefaultRooms(realtime: realtime, clientOptions: self.clientOptions, logger: logger, roomFactory: roomFactory)
     }
 
     public nonisolated var connection: any Connection {

Sources/AblyChat/Room.swift

@@ -19,6 +19,11 @@ public protocol Room: AnyObject, Sendable {
     var options: RoomOptions { get }
 }
 
+/// A ``Room`` that exposes additional functionality for use within the SDK.
+internal protocol InternalRoom: Room {
+    func release() async
+}
+
 public struct RoomStatusChange: Sendable, Equatable {
     public var current: RoomStatus
     public var previous: RoomStatus
@@ -29,7 +34,28 @@ public struct RoomStatusChange: Sendable, Equatable {
     }
 }
 
-internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>: Room where LifecycleManagerFactory.Contributor == DefaultRoomLifecycleContributor {
+internal protocol RoomFactory: Sendable {
+    associatedtype Room: AblyChat.InternalRoom
+
+    func createRoom(realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: InternalLogger) async throws -> Room
+}
+
+internal final class DefaultRoomFactory: Sendable, RoomFactory {
+    private let lifecycleManagerFactory = DefaultRoomLifecycleManagerFactory()
+
+    internal func createRoom(realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: InternalLogger) async throws -> DefaultRoom<DefaultRoomLifecycleManagerFactory> {
+        try await DefaultRoom(
+            realtime: realtime,
+            chatAPI: chatAPI,
+            roomID: roomID,
+            options: options,
+            logger: logger,
+            lifecycleManagerFactory: lifecycleManagerFactory
+        )
+    }
+}
+
+internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>: InternalRoom where LifecycleManagerFactory.Contributor == DefaultRoomLifecycleContributor {
     internal nonisolated let roomID: String
     internal nonisolated let options: RoomOptions
     private let chatAPI: ChatAPI
@@ -40,12 +66,7 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
     private nonisolated let realtime: RealtimeClient
 
     private let lifecycleManager: any RoomLifecycleManager
-
-    #if DEBUG
-        internal nonisolated var testsOnly_realtime: RealtimeClient {
-            realtime
-        }
-    #endif
+    private let channels: [RoomFeature: any RealtimeChannelProtocol]
 
     private let logger: InternalLogger
 
@@ -60,7 +81,7 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
             throw ARTErrorInfo.create(withCode: 40000, message: "Ensure your Realtime instance is initialized with a clientId.")
         }
 
-        let channels = Self.createChannels(roomID: roomID, realtime: realtime)
+        channels = Self.createChannels(roomID: roomID, realtime: realtime)
         let contributors = Self.createContributors(channels: channels)
 
         lifecycleManager = await lifecycleManagerFactory.createManager(
@@ -115,6 +136,15 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
         try await lifecycleManager.performDetachOperation()
     }
 
+    internal func release() async {
+        await lifecycleManager.performReleaseOperation()
+
+        // CHA-RL3h
+        for channel in channels.values {
+            realtime.channels.release(channel.name)
+        }
+    }
+
     // MARK: - Room status
 
     internal func onStatusChange(bufferingPolicy: BufferingPolicy) async -> Subscription<RoomStatusChange> {

Sources/AblyChat/RoomLifecycleManager.swift

@@ -43,6 +43,7 @@ internal protocol RoomLifecycleContributor: Identifiable, Sendable {
 internal protocol RoomLifecycleManager: Sendable {
     func performAttachOperation() async throws
     func performDetachOperation() async throws
+    func performReleaseOperation() async
     var roomStatus: RoomStatus { get async }
     func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription<RoomStatusChange>
 }
@@ -864,11 +865,19 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
 
     // MARK: - RELEASE operation
 
+    internal func performReleaseOperation() async {
+        await _performReleaseOperation(forcingOperationID: nil)
+    }
+
+    internal func performReleaseOperation(testsOnly_forcingOperationID forcedOperationID: UUID? = nil) async {
+        await _performReleaseOperation(forcingOperationID: forcedOperationID)
+    }
+
     /// Implements CHA-RL3’s RELEASE operation.
     ///
     /// - Parameters:
     ///   - forcedOperationID: Allows tests to force 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.
-    internal func performReleaseOperation(testsOnly_forcingOperationID forcedOperationID: UUID? = nil) async {
+    internal func _performReleaseOperation(forcingOperationID forcedOperationID: UUID? = nil) async {
         await performAnOperation(forcingOperationID: forcedOperationID) { operationID in
             await bodyOfReleaseOperation(operationID: operationID)
         }

Sources/AblyChat/Rooms.swift

@@ -6,7 +6,7 @@ public protocol Rooms: AnyObject, Sendable {
     var clientOptions: ClientOptions { get }
 }
 
-internal actor DefaultRooms<LifecycleManagerFactory: RoomLifecycleManagerFactory>: Rooms where LifecycleManagerFactory.Contributor == DefaultRoomLifecycleContributor {
+internal actor DefaultRooms<RoomFactory: AblyChat.RoomFactory>: Rooms {
     private nonisolated let realtime: RealtimeClient
     private let chatAPI: ChatAPI
 
@@ -19,16 +19,16 @@ internal actor DefaultRooms<LifecycleManagerFactory: RoomLifecycleManagerFactory
     internal nonisolated let clientOptions: ClientOptions
 
     private let logger: InternalLogger
-    private let lifecycleManagerFactory: LifecycleManagerFactory
+    private let roomFactory: RoomFactory
 
     /// The set of rooms, keyed by room ID.
-    private var rooms: [String: DefaultRoom<LifecycleManagerFactory>] = [:]
+    private var rooms: [String: RoomFactory.Room] = [:]
 
-    internal init(realtime: RealtimeClient, clientOptions: ClientOptions, logger: InternalLogger, lifecycleManagerFactory: LifecycleManagerFactory) {
+    internal init(realtime: RealtimeClient, clientOptions: ClientOptions, logger: InternalLogger, roomFactory: RoomFactory) {
         self.realtime = realtime
         self.clientOptions = clientOptions
         self.logger = logger
-        self.lifecycleManagerFactory = lifecycleManagerFactory
+        self.roomFactory = roomFactory
         chatAPI = ChatAPI(realtime: realtime)
     }
 
@@ -43,13 +43,28 @@ internal actor DefaultRooms<LifecycleManagerFactory: RoomLifecycleManagerFactory
 
             return existingRoom
         } else {
-            let room = try await DefaultRoom(realtime: realtime, chatAPI: chatAPI, roomID: roomID, options: options, logger: logger, lifecycleManagerFactory: lifecycleManagerFactory)
+            let room = try await roomFactory.createRoom(realtime: realtime, chatAPI: chatAPI, roomID: roomID, options: options, logger: logger)
             rooms[roomID] = room
             return room
         }
     }
 
-    internal func release(roomID _: String) async throws {
-        fatalError("Not yet implemented")
+    #if DEBUG
+        internal func testsOnly_hasExistingRoomWithID(_ roomID: String) -> Bool {
+            rooms[roomID] != nil
+        }
+    #endif
+
+    internal func release(roomID: String) async throws {
+        guard let room = rooms[roomID] else {
+            // TODO: what to do here? (https://github.com/ably/specification/pull/200/files#r1837154563) — Andy replied that it’s a no-op but that this is going to be specified in an upcoming PR when we make room-getting async
+            return
+        }
+
+        // CHA-RC1d
+        rooms.removeValue(forKey: roomID)
+
+        // CHA-RL1e
+        await room.release()
     }
 }

Tests/AblyChatTests/DefaultChatClientTests.swift

@@ -22,7 +22,7 @@ struct DefaultChatClientTests {
         // Then: Its `rooms` property returns an instance of DefaultRooms with the same realtime client and client options
         let rooms = client.rooms
 
-        let defaultRooms = try #require(rooms as? DefaultRooms<DefaultRoomLifecycleManagerFactory>)
+        let defaultRooms = try #require(rooms as? DefaultRooms<DefaultRoomFactory>)
         #expect(defaultRooms.testsOnly_realtime === realtime)
         #expect(defaultRooms.clientOptions.isEqualForTestPurposes(options))
     }

Tests/AblyChatTests/DefaultRoomTests.swift

@@ -92,6 +92,33 @@ struct DefaultRoomTests {
         #expect(await lifecycleManager.detachCallCount == 1)
     }
 
+    // MARK: - Release
+
+    // @spec CHA-RL3h - I haven’t explicitly tested that `performReleaseOperation()` happens _before_ releasing the channels (i.e. the “upon operation completion” part of the spec point), because it would require me to spend extra time on mock-writing which I can’t really afford to spend right now. I think we can live with it at least for the time being; I’m pretty sure there are other tests where the spec mentions or requires an order where I also haven’t tested the order.
+    @Test
+    func release() async throws {
+        // Given: a DefaultRoom instance
+        let channelsList = [
+            MockRealtimeChannel(name: "basketball::$chat::$chatMessages"),
+        ]
+        let channels = MockChannels(channels: channelsList)
+        let realtime = MockRealtime.create(channels: channels)
+
+        let lifecycleManager = MockRoomLifecycleManager()
+        let lifecycleManagerFactory = MockRoomLifecycleManagerFactory(manager: lifecycleManager)
+
+        let room = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: .init(), logger: TestLogger(), lifecycleManagerFactory: lifecycleManagerFactory)
+
+        // When: `release()` is called on the room
+        await room.release()
+
+        // Then: It:
+        // 1. calls `performReleaseOperation()` on the room lifecycle manager
+        // 2. calls `channels.release()` with the name of each of the features’ channels
+        #expect(await lifecycleManager.releaseCallCount == 1)
+        #expect(Set(channels.releaseArguments) == Set(channelsList.map(\.name)))
+    }
+
     // MARK: - Room status
 
     @Test

Tests/AblyChatTests/DefaultRoomsTests.swift

@@ -3,25 +3,32 @@ import Testing
 
 // The channel name of basketball::$chat::$chatMessages is passed in to these tests due to `DefaultRoom` kicking off the `DefaultMessages` initialization. This in turn needs a valid `roomId` or else the `MockChannels` class will throw an error as it would be expecting a channel with the name \(roomID)::$chat::$chatMessages to exist (where `roomId` is the property passed into `rooms.get`).
 struct DefaultRoomsTests {
+    // MARK: - Get a room
+
     // @spec CHA-RC1a
     @Test
     func get_returnsRoomWithGivenID() async throws {
         // Given: an instance of DefaultRooms
         let realtime = MockRealtime.create(channels: .init(channels: [
             .init(name: "basketball::$chat::$chatMessages"),
         ]))
-        let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), lifecycleManagerFactory: MockRoomLifecycleManagerFactory())
+        let options = RoomOptions()
+        let roomToReturn = MockRoom(options: options)
+        let roomFactory = MockRoomFactory(room: roomToReturn)
+        let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: roomFactory)
 
         // When: get(roomID:options:) is called
         let roomID = "basketball"
-        let options = RoomOptions()
         let room = try await rooms.get(roomID: roomID, options: options)
 
-        // Then: It returns a DefaultRoom instance that uses the same Realtime instance, with the given ID and options
-        let defaultRoom = try #require(room as? DefaultRoom<MockRoomLifecycleManagerFactory>)
-        #expect(defaultRoom.testsOnly_realtime === realtime)
-        #expect(defaultRoom.roomID == roomID)
-        #expect(defaultRoom.options == options)
+        // Then: It returns a room that uses the same Realtime instance, with the given ID and options
+        let mockRoom = try #require(room as? MockRoom)
+        #expect(mockRoom === roomToReturn)
+
+        let createRoomArguments = try #require(await roomFactory.createRoomArguments)
+        #expect(createRoomArguments.realtime === realtime)
+        #expect(createRoomArguments.roomID == roomID)
+        #expect(createRoomArguments.options == options)
     }
 
     // @spec CHA-RC1b
@@ -31,10 +38,11 @@ struct DefaultRoomsTests {
         let realtime = MockRealtime.create(channels: .init(channels: [
             .init(name: "basketball::$chat::$chatMessages"),
         ]))
-        let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), lifecycleManagerFactory: MockRoomLifecycleManagerFactory())
+        let options = RoomOptions()
+        let roomToReturn = MockRoom(options: options)
+        let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: MockRoomFactory(room: roomToReturn))
 
         let roomID = "basketball"
-        let options = RoomOptions()
         let firstRoom = try await rooms.get(roomID: roomID, options: options)
 
         // When: get(roomID:options:) is called with the same room ID
@@ -51,10 +59,11 @@ struct DefaultRoomsTests {
         let realtime = MockRealtime.create(channels: .init(channels: [
             .init(name: "basketball::$chat::$chatMessages"),
         ]))
-        let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), lifecycleManagerFactory: MockRoomLifecycleManagerFactory())
+        let options = RoomOptions()
+        let roomToReturn = MockRoom(options: options)
+        let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: MockRoomFactory(room: roomToReturn))
 
         let roomID = "basketball"
-        let options = RoomOptions()
         _ = try await rooms.get(roomID: roomID, options: options)
 
         // When: get(roomID:options:) is called with the same ID but different options
@@ -71,4 +80,43 @@ struct DefaultRoomsTests {
         // Then: It throws an inconsistentRoomOptions error
         #expect(isChatError(caughtError, withCode: .inconsistentRoomOptions))
     }
+
+    // MARK: - Release a room
+
+    // @spec CHA-RC1d
+    // @spec CHA-RC1e
+    @Test
+    func release() 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 options = RoomOptions()
+        let hasExistingRoomAtMomentRoomReleaseCalledStreamComponents = AsyncStream.makeStream(of: Bool.self)
+        let roomFactory = MockRoomFactory()
+        let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: roomFactory)
+
+        let roomID = "basketball"
+
+        let roomToReturn = MockRoom(options: options) {
+            await hasExistingRoomAtMomentRoomReleaseCalledStreamComponents.continuation.yield(rooms.testsOnly_hasExistingRoomWithID(roomID))
+        }
+        await roomFactory.setRoom(roomToReturn)
+
+        _ = try await rooms.get(roomID: roomID, options: .init())
+        try #require(await rooms.testsOnly_hasExistingRoomWithID(roomID))
+
+        // When: `release(roomID:)` is called with this room ID
+        _ = try await rooms.release(roomID: roomID)
+
+        // Then:
+        // 1. first, the room is removed from the room map
+        // 2. next, `release` is called on the room
+
+        // These two lines are convoluted because the #require macro has a hard time with stuff of type Bool? and emits warnings about ambiguity unless you jump through the hoops it tells you to
+        let hasExistingRoomAtMomentRoomReleaseCalled = await hasExistingRoomAtMomentRoomReleaseCalledStreamComponents.stream.first { _ in true }
+        #expect(try !#require(hasExistingRoomAtMomentRoomReleaseCalled as Bool?))
+
+        #expect(await roomToReturn.releaseCallCount == 1)
+    }
 }

Tests/AblyChatTests/IntegrationTests.swift

@@ -74,5 +74,16 @@ struct IntegrationTests {
         // (11) Check that we received a DETACHED status change as a result of detaching the room
         _ = try #require(await rxRoomStatusSubscription.first { $0.current == .detached })
         #expect(await rxRoom.status == .detached)
+
+        // (12) Release the room
+        try await rxClient.rooms.release(roomID: roomID)
+
+        // (13) Check that we received a RELEASED status change as a result of releasing the room
+        _ = try #require(await rxRoomStatusSubscription.first { $0.current == .released })
+        #expect(await rxRoom.status == .released)
+
+        // (14) Fetch the room we just released and check it’s a new object
+        let postReleaseRxRoom = try await rxClient.rooms.get(roomID: roomID, options: .init())
+        #expect(postReleaseRxRoom !== rxRoom)
     }
 }

Tests/AblyChatTests/Mocks/MockChannels.swift

@@ -1,8 +1,11 @@
 import Ably
 import AblyChat
 
-final class MockChannels: RealtimeChannelsProtocol, Sendable {
+final class MockChannels: RealtimeChannelsProtocol, @unchecked Sendable {
     private let channels: [MockRealtimeChannel]
+    private let mutex = NSLock()
+    /// Access must be synchronized via ``mutex``.
+    private(set) var _releaseArguments: [String] = []
 
     init(channels: [MockRealtimeChannel]) {
         self.channels = channels
@@ -24,7 +27,17 @@ final class MockChannels: RealtimeChannelsProtocol, Sendable {
         fatalError("Not implemented")
     }
 
-    func release(_: String) {
-        fatalError("Not implemented")
+    func release(_ name: String) {
+        mutex.lock()
+        defer { mutex.unlock() }
+        _releaseArguments.append(name)
+    }
+
+    var releaseArguments: [String] {
+        let result: [String]
+        mutex.lock()
+        result = _releaseArguments
+        mutex.unlock()
+        return result
     }
 }