From a63b22b75793f7359494959d940355110a98a5c6 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 2 Sep 2024 15:27:43 +0100 Subject: [PATCH] Implement room status change upon attach or detach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on the simplified requirements described in #19. The decision from 7d6acde to use actors as the mechanism for managing mutable state means that I’ve had to make RoomStatus.{ current, error, onChange(bufferingPolicy:) } async. As mentioned there, if later on we decide this is too weird an API, then we can think of alternatives. I really wanted to avoid making DefaultRoomStatus an actor; I tried to find a way to isolate this state to the DefaultRoom actor (who logically owns this state), by trying to make the DefaultRoomStatus access the DefaultRoom-managed state, but I was not successful and didn’t want to sink much time into it. It means that DefaultRoom has suspension points in order to access its current state, which I am not happy about. But we can revisit later, perhaps armed with more knowledge of concurrency and in less of a rush to get some initial functionality implemented. Resolves #19. --- Sources/AblyChat/Room.swift | 8 +++- Sources/AblyChat/RoomStatus.swift | 30 ++++++++++-- .../DefaultRoomStatusTests.swift | 46 +++++++++++++++++++ Tests/AblyChatTests/DefaultRoomTests.swift | 26 ++++++++++- 4 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 Tests/AblyChatTests/DefaultRoomStatusTests.swift diff --git a/Sources/AblyChat/Room.swift b/Sources/AblyChat/Room.swift index e7f1f3c0..c7ef5db9 100644 --- a/Sources/AblyChat/Room.swift +++ b/Sources/AblyChat/Room.swift @@ -24,6 +24,8 @@ internal actor DefaultRoom: Room { // Exposed for testing. internal nonisolated let realtime: RealtimeClient + private let _status = DefaultRoomStatus() + internal init(realtime: RealtimeClient, roomID: String, options: RoomOptions) { self.realtime = realtime self.roomID = roomID @@ -50,8 +52,8 @@ internal actor DefaultRoom: Room { fatalError("Not yet implemented") } - public nonisolated var status: any RoomStatus { - fatalError("Not yet implemented") + internal nonisolated var status: any RoomStatus { + _status } /// Fetches the channels that contribute to this room. @@ -69,11 +71,13 @@ internal actor DefaultRoom: Room { for channel in channels() { try await channel.attachAsync() } + await _status.transition(to: .attached) } public func detach() async throws { for channel in channels() { try await channel.detachAsync() } + await _status.transition(to: .detached) } } diff --git a/Sources/AblyChat/RoomStatus.swift b/Sources/AblyChat/RoomStatus.swift index 24eee9d7..87a39783 100644 --- a/Sources/AblyChat/RoomStatus.swift +++ b/Sources/AblyChat/RoomStatus.swift @@ -1,10 +1,10 @@ import Ably public protocol RoomStatus: AnyObject, Sendable { - var current: RoomLifecycle { get } + var current: RoomLifecycle { get async } // TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap - var error: ARTErrorInfo? { get } - func onChange(bufferingPolicy: BufferingPolicy) -> Subscription + var error: ARTErrorInfo? { get async } + func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription } public enum RoomLifecycle: Sendable { @@ -31,3 +31,27 @@ public struct RoomStatusChange: Sendable { self.error = error } } + +internal actor DefaultRoomStatus: RoomStatus { + internal private(set) var current: RoomLifecycle = .initialized + // TODO: populate this (https://github.com/ably-labs/ably-chat-swift/issues/28) + internal private(set) var error: ARTErrorInfo? + + // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36) + private var subscriptions: [Subscription] = [] + + internal func onChange(bufferingPolicy: BufferingPolicy) -> Subscription { + let subscription: Subscription = .init(bufferingPolicy: bufferingPolicy) + subscriptions.append(subscription) + return subscription + } + + /// Sets ``current`` to the given state, and emits a status change to all subscribers added via ``onChange(bufferingPolicy:)``. + internal func transition(to newState: RoomLifecycle) { + let statusChange = RoomStatusChange(current: newState, previous: current) + current = newState + for subscription in subscriptions { + subscription.emit(statusChange) + } + } +} diff --git a/Tests/AblyChatTests/DefaultRoomStatusTests.swift b/Tests/AblyChatTests/DefaultRoomStatusTests.swift new file mode 100644 index 00000000..75f58dad --- /dev/null +++ b/Tests/AblyChatTests/DefaultRoomStatusTests.swift @@ -0,0 +1,46 @@ +@testable import AblyChat +import XCTest + +class DefaultRoomStatusTests: XCTestCase { + func test_current_startsAsInitialized() async { + let status = DefaultRoomStatus() + let current = await status.current + XCTAssertEqual(current, .initialized) + } + + func test_error_startsAsNil() async { + let status = DefaultRoomStatus() + let error = await status.error + XCTAssertNil(error) + } + + func test_transition() async { + // Given: A RoomStatus + let status = DefaultRoomStatus() + let originalState = await status.current + let newState = RoomLifecycle.attached // arbitrary + + let subscription1 = await status.onChange(bufferingPolicy: .unbounded) + let subscription2 = await status.onChange(bufferingPolicy: .unbounded) + + async let statusChange1 = subscription1.first { $0.current == newState } + async let statusChange2 = subscription2.first { $0.current == newState } + + // When: transition(to:) is called + await status.transition(to: newState) + + // Then: It emits a status change to all subscribers added via onChange(bufferingPolicy:), and updates its `current` property to the new state + guard let statusChange1 = await statusChange1, let statusChange2 = await statusChange2 else { + XCTFail("Expected status changes to be emitted") + return + } + + for statusChange in [statusChange1, statusChange2] { + XCTAssertEqual(statusChange.previous, originalState) + XCTAssertEqual(statusChange.current, newState) + } + + let current = await status.current + XCTAssertEqual(current, .attached) + } +} diff --git a/Tests/AblyChatTests/DefaultRoomTests.swift b/Tests/AblyChatTests/DefaultRoomTests.swift index 624fdf2b..7f600ba7 100644 --- a/Tests/AblyChatTests/DefaultRoomTests.swift +++ b/Tests/AblyChatTests/DefaultRoomTests.swift @@ -18,13 +18,24 @@ class DefaultRoomTests: XCTestCase { let realtime = MockRealtime.create(channels: channels) let room = DefaultRoom(realtime: realtime, roomID: "basketball", options: .init()) + let subscription = await room.status.onChange(bufferingPolicy: .unbounded) + async let attachedStatusChange = subscription.first { $0.current == .attached } + // When: `attach` is called on the room try await room.attach() - // Then: `attach(_:)` is called on each of the channels, and the room `attach` call succeeds + // Then: `attach(_:)` is called on each of the channels, the room `attach` call succeeds, and the room transitions to ATTACHED for channel in channelsList { XCTAssertTrue(channel.attachCallCounter.isNonZero) } + + guard let attachedStatusChange = await attachedStatusChange else { + XCTFail("Expected status change to ATTACHED but didn't get one") + return + } + let currentStatus = await room.status.current + XCTAssertEqual(currentStatus, .attached) + XCTAssertEqual(attachedStatusChange.current, .attached) } func test_attach_attachesAllChannels_andFailsIfOneFails() async throws { @@ -73,13 +84,24 @@ class DefaultRoomTests: XCTestCase { let realtime = MockRealtime.create(channels: channels) let room = DefaultRoom(realtime: realtime, roomID: "basketball", options: .init()) + let subscription = await room.status.onChange(bufferingPolicy: .unbounded) + async let detachedStatusChange = subscription.first { $0.current == .detached } + // When: `detach` is called on the room try await room.detach() - // Then: `detach(_:)` is called on each of the channels, and the room `detach` call succeeds + // Then: `detach(_:)` is called on each of the channels, the room `detach` call succeeds, and the room transitions to DETACHED for channel in channelsList { XCTAssertTrue(channel.detachCallCounter.isNonZero) } + + guard let detachedStatusChange = await detachedStatusChange else { + XCTFail("Expected status change to DETACHED but didn't get one") + return + } + let currentStatus = await room.status.current + XCTAssertEqual(currentStatus, .detached) + XCTAssertEqual(detachedStatusChange.current, .detached) } func test_detach_detachesAllChannels_andFailsIfOneFails() async throws {