From ae278805f8b32c11fcc603f0af181746c07d9c36 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 1 Oct 2024 17:55:06 -0300 Subject: [PATCH 1/5] Fix typo Mistake in a63b22b. --- Tests/AblyChatTests/DefaultRoomStatusTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/AblyChatTests/DefaultRoomStatusTests.swift b/Tests/AblyChatTests/DefaultRoomStatusTests.swift index 89ea6d80..8503b28e 100644 --- a/Tests/AblyChatTests/DefaultRoomStatusTests.swift +++ b/Tests/AblyChatTests/DefaultRoomStatusTests.swift @@ -16,7 +16,7 @@ struct DefaultRoomStatusTests { @Test func transition() async throws { - // Given: A RoomStatus + // Given: A DefaultRoomStatus let status = DefaultRoomStatus(logger: TestLogger()) let originalState = await status.current let newState = RoomLifecycle.attached // arbitrary From b8deb5d8d9be3cd8e82f5f3be094f720a9ed21ae Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 28 Oct 2024 15:07:42 -0300 Subject: [PATCH 2/5] Remove a couple of `error` properties Missed these in d51887c. --- Example/AblyChatExample/Mocks/MockClients.swift | 1 - Sources/AblyChat/RoomStatus.swift | 1 - Tests/AblyChatTests/DefaultRoomStatusTests.swift | 6 ------ 3 files changed, 8 deletions(-) diff --git a/Example/AblyChatExample/Mocks/MockClients.swift b/Example/AblyChatExample/Mocks/MockClients.swift index 53ca9792..8d6620e0 100644 --- a/Example/AblyChatExample/Mocks/MockClients.swift +++ b/Example/AblyChatExample/Mocks/MockClients.swift @@ -400,7 +400,6 @@ actor MockRoomStatus: RoomStatus { let roomID: String var current: RoomLifecycle - var error: ARTErrorInfo? private var mockSubscriptions: [MockSubscription] = [] diff --git a/Sources/AblyChat/RoomStatus.swift b/Sources/AblyChat/RoomStatus.swift index 3fa7d03a..2cedd8d4 100644 --- a/Sources/AblyChat/RoomStatus.swift +++ b/Sources/AblyChat/RoomStatus.swift @@ -58,7 +58,6 @@ public struct RoomStatusChange: Sendable { internal actor DefaultRoomStatus: RoomStatus { internal private(set) var current: RoomLifecycle = .initialized - internal private(set) var error: ARTErrorInfo? private let logger: InternalLogger diff --git a/Tests/AblyChatTests/DefaultRoomStatusTests.swift b/Tests/AblyChatTests/DefaultRoomStatusTests.swift index 8503b28e..33f183aa 100644 --- a/Tests/AblyChatTests/DefaultRoomStatusTests.swift +++ b/Tests/AblyChatTests/DefaultRoomStatusTests.swift @@ -8,12 +8,6 @@ struct DefaultRoomStatusTests { #expect(await status.current == .initialized) } - @Test() - func error_startsAsNil() async { - let status = DefaultRoomStatus(logger: TestLogger()) - #expect(await status.error == nil) - } - @Test func transition() async throws { // Given: A DefaultRoomStatus From 7e4460d0aa3652c849cd13313fad667cf78b5608 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 28 Oct 2024 12:12:10 -0300 Subject: [PATCH 3/5] =?UTF-8?q?Apply=20CHADR-062=E2=80=99s=20public=20API?= =?UTF-8?q?=20changes=20to=20Room?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We apply the changes that we decided upon in [1]; that is, we: - remove the RoomStatus protocol, moving its functionality directly into the Room protocol, and rename the `current` property to `status`; - rename the RoomLifecycle enum to RoomStatus As part of this, I’ve also corrected references to room “state” to the correct terminology of “status” (in [2] Andy explains why they used the word “status”, choosing to reserve “state” for some larger composite state). [1] https://ably.atlassian.net/wiki/spaces/CHA/pages/3410526209/CHADR-062+Renaming+Lifecycle+Types+for+Clarity [2] https://github.com/ably-labs/ably-chat-swift/issues/73#issuecomment-2385770039 --- Example/AblyChatExample/ContentView.swift | 2 +- .../AblyChatExample/Mocks/MockClients.swift | 43 +++++-------- Sources/AblyChat/Room.swift | 45 +++++++++++--- Sources/AblyChat/RoomLifecycleManager.swift | 10 +-- Sources/AblyChat/RoomStatus.swift | 49 +-------------- .../DefaultRoomStatusTests.swift | 35 ----------- Tests/AblyChatTests/DefaultRoomTests.swift | 57 +++++++++++++++-- .../Helpers/RoomLifecycle+Error.swift | 2 +- .../RoomLifecycleManagerTests.swift | 62 +++++++++---------- 9 files changed, 145 insertions(+), 160 deletions(-) delete mode 100644 Tests/AblyChatTests/DefaultRoomStatusTests.swift diff --git a/Example/AblyChatExample/ContentView.swift b/Example/AblyChatExample/ContentView.swift index bae5ddcf..f3a54f14 100644 --- a/Example/AblyChatExample/ContentView.swift +++ b/Example/AblyChatExample/ContentView.swift @@ -170,7 +170,7 @@ struct ContentView: View { } func showRoomStatus() async throws { - for await status in try await room().status.onChange(bufferingPolicy: .unbounded) { + for await status in try await room().onStatusChange(bufferingPolicy: .unbounded) { withAnimation { if status.current.isAttaching { statusInfo = "\(status.current)...".capitalized diff --git a/Example/AblyChatExample/Mocks/MockClients.swift b/Example/AblyChatExample/Mocks/MockClients.swift index 8d6620e0..a373aa4c 100644 --- a/Example/AblyChatExample/Mocks/MockClients.swift +++ b/Example/AblyChatExample/Mocks/MockClients.swift @@ -64,7 +64,9 @@ actor MockRoom: Room { nonisolated lazy var occupancy: any Occupancy = MockOccupancy(clientID: clientID, roomID: roomID) - nonisolated lazy var status: any RoomStatus = MockRoomStatus(clientID: clientID, roomID: roomID) + var status: RoomStatus = .initialized + + private var mockSubscriptions: [MockSubscription] = [] func attach() async throws { fatalError("Not yet implemented") @@ -73,6 +75,18 @@ actor MockRoom: Room { func detach() async throws { fatalError("Not yet implemented") } + + private func createSubscription() -> MockSubscription { + let subscription = MockSubscription(randomElement: { + RoomStatusChange(current: [.attached, .attached, .attached, .attached, .attaching(error: nil), .attaching(error: nil), .suspended(error: .createUnknownError())].randomElement()!, previous: .attaching(error: nil)) + }, interval: 8) + mockSubscriptions.append(subscription) + return subscription + } + + func onStatusChange(bufferingPolicy _: BufferingPolicy) async -> Subscription { + .init(mockAsyncSequence: createSubscription()) + } } actor MockMessages: Messages { @@ -394,30 +408,3 @@ actor MockOccupancy: Occupancy { fatalError("Not yet implemented") } } - -actor MockRoomStatus: RoomStatus { - let clientID: String - let roomID: String - - var current: RoomLifecycle - - private var mockSubscriptions: [MockSubscription] = [] - - init(clientID: String, roomID: String) { - self.clientID = clientID - self.roomID = roomID - current = .initialized - } - - private func createSubscription() -> MockSubscription { - let subscription = MockSubscription(randomElement: { - RoomStatusChange(current: [.attached, .attached, .attached, .attached, .attaching(error: nil), .attaching(error: nil), .suspended(error: .createUnknownError())].randomElement()!, previous: .attaching(error: nil)) - }, interval: 8) - mockSubscriptions.append(subscription) - return subscription - } - - func onChange(bufferingPolicy _: BufferingPolicy) async -> Subscription { - .init(mockAsyncSequence: createSubscription()) - } -} diff --git a/Sources/AblyChat/Room.swift b/Sources/AblyChat/Room.swift index 72352e22..3364ebeb 100644 --- a/Sources/AblyChat/Room.swift +++ b/Sources/AblyChat/Room.swift @@ -11,12 +11,24 @@ public protocol Room: AnyObject, Sendable { var typing: any Typing { get } // To access this property if occupancy is not enabled for the room is a programmer error, and will lead to `fatalError` being called. var occupancy: any Occupancy { get } - var status: any RoomStatus { get } + // TODO: change to `status` + var status: RoomStatus { get async } + func onStatusChange(bufferingPolicy: BufferingPolicy) async -> Subscription func attach() async throws func detach() async throws var options: RoomOptions { get } } +public struct RoomStatusChange: Sendable { + public var current: RoomStatus + public var previous: RoomStatus + + public init(current: RoomStatus, previous: RoomStatus) { + self.current = current + self.previous = previous + } +} + internal actor DefaultRoom: Room { internal nonisolated let roomID: String internal nonisolated let options: RoomOptions @@ -33,7 +45,9 @@ internal actor DefaultRoom: Room { } #endif - private let _status: DefaultRoomStatus + internal private(set) var status: RoomStatus = .initialized + // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36) + private var statusSubscriptions: [Subscription] = [] private let logger: InternalLogger internal init(realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: InternalLogger) async throws { @@ -41,7 +55,6 @@ internal actor DefaultRoom: Room { self.roomID = roomID self.options = options self.logger = logger - _status = .init(logger: logger) self.chatAPI = chatAPI guard let clientId = realtime.clientId else { @@ -71,10 +84,6 @@ internal actor DefaultRoom: Room { fatalError("Not yet implemented") } - internal nonisolated var status: any RoomStatus { - _status - } - /// Fetches the channels that contribute to this room. private func channels() -> [any RealtimeChannelProtocol] { [ @@ -95,7 +104,7 @@ internal actor DefaultRoom: Room { throw error } } - await _status.transition(to: .attached) + transition(to: .attached) } public func detach() async throws { @@ -107,6 +116,24 @@ internal actor DefaultRoom: Room { throw error } } - await _status.transition(to: .detached) + transition(to: .detached) + } + + // MARK: - Room status + + internal func onStatusChange(bufferingPolicy: BufferingPolicy) -> Subscription { + let subscription: Subscription = .init(bufferingPolicy: bufferingPolicy) + statusSubscriptions.append(subscription) + return subscription + } + + /// Sets ``status`` to the given status, and emits a status change to all subscribers added via ``onStatusChange(bufferingPolicy:)``. + internal func transition(to newStatus: RoomStatus) { + logger.log(message: "Transitioning to \(newStatus)", level: .debug) + let statusChange = RoomStatusChange(current: newStatus, previous: status) + status = newStatus + for subscription in statusSubscriptions { + subscription.emit(statusChange) + } } } diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 0e5a5e6b..c820222e 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -157,7 +157,7 @@ internal actor RoomLifecycleManager { case releasing(releaseOperationID: UUID) case released - internal var toRoomLifecycle: RoomLifecycle { + internal var toRoomStatus: RoomStatus { switch self { case .initialized: .initialized @@ -264,8 +264,8 @@ internal actor RoomLifecycleManager { // MARK: - Room status and its changes - internal var current: RoomLifecycle { - status.toRoomLifecycle + internal var current: RoomStatus { + status.toRoomStatus } internal func onChange(bufferingPolicy: BufferingPolicy) -> Subscription { @@ -279,7 +279,7 @@ internal actor RoomLifecycleManager { logger.log(message: "Transitioning from \(status) to \(new)", level: .info) let previous = status status = new - let statusChange = RoomStatusChange(current: status.toRoomLifecycle, previous: previous.toRoomLifecycle) + let statusChange = RoomStatusChange(current: status.toRoomStatus, previous: previous.toRoomStatus) emitStatusChange(statusChange) } @@ -779,7 +779,7 @@ internal actor RoomLifecycleManager { } // This check is CHA-RL2h2 - if !status.toRoomLifecycle.isFailed { + if !status.toRoomStatus.isFailed { changeStatus(to: .failed(error: error)) } default: diff --git a/Sources/AblyChat/RoomStatus.swift b/Sources/AblyChat/RoomStatus.swift index 2cedd8d4..ac9c93fb 100644 --- a/Sources/AblyChat/RoomStatus.swift +++ b/Sources/AblyChat/RoomStatus.swift @@ -1,11 +1,7 @@ import Ably -public protocol RoomStatus: AnyObject, Sendable { - var current: RoomLifecycle { get async } - func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription -} - -public enum RoomLifecycle: Sendable, Equatable { +// TODO: rename +public enum RoomStatus: Sendable, Equatable { case initialized case attaching(error: ARTErrorInfo?) case attached @@ -16,7 +12,7 @@ public enum RoomLifecycle: Sendable, Equatable { case releasing case released - // Helpers to allow us to test whether a `RoomLifecycle` value has a certain case, without caring about the associated value. These are useful for in contexts where we want to use a `Bool` to communicate a case. For example: + // Helpers to allow us to test whether a `RoomStatus` value has a certain case, without caring about the associated value. These are useful for in contexts where we want to use a `Bool` to communicate a case. For example: // // 1. testing (e.g. `#expect(status.isFailed)`) // 2. testing that a status does _not_ have a particular case (e.g. if !status.isFailed), which a `case` statement cannot succinctly express @@ -45,42 +41,3 @@ public enum RoomLifecycle: Sendable, Equatable { } } } - -public struct RoomStatusChange: Sendable { - public var current: RoomLifecycle - public var previous: RoomLifecycle - - public init(current: RoomLifecycle, previous: RoomLifecycle) { - self.current = current - self.previous = previous - } -} - -internal actor DefaultRoomStatus: RoomStatus { - internal private(set) var current: RoomLifecycle = .initialized - - private let logger: InternalLogger - - internal init(logger: InternalLogger) { - self.logger = logger - } - - // 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) { - logger.log(message: "Transitioning to \(newState)", level: .debug) - 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 deleted file mode 100644 index 33f183aa..00000000 --- a/Tests/AblyChatTests/DefaultRoomStatusTests.swift +++ /dev/null @@ -1,35 +0,0 @@ -@testable import AblyChat -import Testing - -struct DefaultRoomStatusTests { - @Test - func current_startsAsInitialized() async { - let status = DefaultRoomStatus(logger: TestLogger()) - #expect(await status.current == .initialized) - } - - @Test - func transition() async throws { - // Given: A DefaultRoomStatus - let status = DefaultRoomStatus(logger: TestLogger()) - 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 - for statusChange in try await [#require(statusChange1), #require(statusChange2)] { - #expect(statusChange.previous == originalState) - #expect(statusChange.current == newState) - } - - #expect(await status.current == .attached) - } -} diff --git a/Tests/AblyChatTests/DefaultRoomTests.swift b/Tests/AblyChatTests/DefaultRoomTests.swift index d797e6f8..55580c5a 100644 --- a/Tests/AblyChatTests/DefaultRoomTests.swift +++ b/Tests/AblyChatTests/DefaultRoomTests.swift @@ -3,6 +3,8 @@ import Ably import Testing struct DefaultRoomTests { + // MARK: - Attach + @Test func attach_attachesAllChannels_andSucceedsIfAllSucceed() async throws { // Given: a DefaultRoom instance with ID "basketball", with a Realtime client for which `attach(_:)` completes successfully if called on the following channels: @@ -19,7 +21,7 @@ struct DefaultRoomTests { let realtime = MockRealtime.create(channels: channels) let room = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: .init(), logger: TestLogger()) - let subscription = await room.status.onChange(bufferingPolicy: .unbounded) + let subscription = await room.onStatusChange(bufferingPolicy: .unbounded) async let attachedStatusChange = subscription.first { $0.current == .attached } // When: `attach` is called on the room @@ -30,7 +32,7 @@ struct DefaultRoomTests { #expect(channel.attachCallCounter.isNonZero) } - #expect(await room.status.current == .attached) + #expect(await room.status == .attached) #expect(try #require(await attachedStatusChange).current == .attached) } @@ -65,6 +67,8 @@ struct DefaultRoomTests { #expect(try #require(roomAttachError as? ARTErrorInfo) === channelAttachError) } + // MARK: - Detach + @Test func detach_detachesAllChannels_andSucceedsIfAllSucceed() async throws { // Given: a DefaultRoom instance with ID "basketball", with a Realtime client for which `detach(_:)` completes successfully if called on the following channels: @@ -81,7 +85,7 @@ struct DefaultRoomTests { let realtime = MockRealtime.create(channels: channels) let room = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: .init(), logger: TestLogger()) - let subscription = await room.status.onChange(bufferingPolicy: .unbounded) + let subscription = await room.onStatusChange(bufferingPolicy: .unbounded) async let detachedStatusChange = subscription.first { $0.current == .detached } // When: `detach` is called on the room @@ -92,7 +96,7 @@ struct DefaultRoomTests { #expect(channel.detachCallCounter.isNonZero) } - #expect(await room.status.current == .detached) + #expect(await room.status == .detached) #expect(try #require(await detachedStatusChange).current == .detached) } @@ -126,4 +130,49 @@ struct DefaultRoomTests { // Then: the room `detach` call fails with the same error as the channel `detach(_:)` call #expect(try #require(roomDetachError as? ARTErrorInfo) === channelDetachError) } + + // MARK: - Room status + + @Test + func current_startsAsInitialized() async throws { + let channelsList = [ + MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), + MockRealtimeChannel(name: "basketball::$chat::$typingIndicators"), + MockRealtimeChannel(name: "basketball::$chat::$reactions"), + ] + let realtime = MockRealtime.create(channels: .init(channels: channelsList)) + let room = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: .init(), logger: TestLogger()) + #expect(await room.status == .initialized) + } + + @Test + func transition() async throws { + // Given: A DefaultRoom + let channelsList = [ + MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), + MockRealtimeChannel(name: "basketball::$chat::$typingIndicators"), + MockRealtimeChannel(name: "basketball::$chat::$reactions"), + ] + let realtime = MockRealtime.create(channels: .init(channels: channelsList)) + let room = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: .init(), logger: TestLogger()) + let originalStatus = await room.status + let newStatus = RoomStatus.attached // arbitrary + + let subscription1 = await room.onStatusChange(bufferingPolicy: .unbounded) + let subscription2 = await room.onStatusChange(bufferingPolicy: .unbounded) + + async let statusChange1 = subscription1.first { $0.current == newStatus } + async let statusChange2 = subscription2.first { $0.current == newStatus } + + // When: transition(to:) is called + await room.transition(to: newStatus) + + // Then: It emits a status change to all subscribers added via onChange(bufferingPolicy:), and updates its `status` property to the new state + for statusChange in try await [#require(statusChange1), #require(statusChange2)] { + #expect(statusChange.previous == originalStatus) + #expect(statusChange.current == newStatus) + } + + #expect(await room.status == .attached) + } } diff --git a/Tests/AblyChatTests/Helpers/RoomLifecycle+Error.swift b/Tests/AblyChatTests/Helpers/RoomLifecycle+Error.swift index f2f33df2..bbd86298 100644 --- a/Tests/AblyChatTests/Helpers/RoomLifecycle+Error.swift +++ b/Tests/AblyChatTests/Helpers/RoomLifecycle+Error.swift @@ -1,7 +1,7 @@ import Ably import AblyChat -extension RoomLifecycle { +extension RoomStatus { var error: ARTErrorInfo? { switch self { case let .failed(error): diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index 6ba5bb31..d18176d8 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -117,7 +117,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL1a @Test func attach_whenAlreadyAttached() async throws { - // Given: A RoomLifecycleManager in the ATTACHED state + // Given: A RoomLifecycleManager in the ATTACHED status let contributor = createContributor() let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .attached, contributors: [contributor]) @@ -131,7 +131,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL1b @Test func attach_whenReleasing() async throws { - // Given: A RoomLifecycleManager in the RELEASING state + // Given: A RoomLifecycleManager in the RELEASING status let manager = await createManager( forTestingWhatHappensWhenCurrentlyIn: .releasing(releaseOperationID: UUID() /* arbitrary */ ) ) @@ -148,7 +148,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL1c @Test func attach_whenReleased() async throws { - // Given: A RoomLifecycleManager in the RELEASED state + // Given: A RoomLifecycleManager in the RELEASED status let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .released) // When: `performAttachOperation()` is called on the lifecycle manager @@ -211,7 +211,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL1e @Test func attach_transitionsToAttaching() async throws { - // Given: A RoomLifecycleManager, with a contributor on whom calling `attach()` will not complete until after the "Then" part of this test (the motivation for this is to suppress the room from transitioning to ATTACHED, so that we can assert its current state as being ATTACHING) + // Given: A RoomLifecycleManager, with a contributor on whom calling `attach()` will not complete until after the "Then" part of this test (the motivation for this is to suppress the room from transitioning to ATTACHED, so that we can assert its current status as being ATTACHING) let contributorAttachOperation = SignallableChannelOperation() let manager = await createManager(contributors: [createContributor(attachBehavior: contributorAttachOperation.behavior)]) @@ -221,12 +221,12 @@ struct RoomLifecycleManagerTests { // When: `performAttachOperation()` is called on the lifecycle manager async let _ = try await manager.performAttachOperation() - // Then: It emits a status change to ATTACHING, and its current state is ATTACHING + // Then: It emits a status change to ATTACHING, and its current status is ATTACHING #expect(try #require(await statusChange).current == .attaching(error: nil)) #expect(await manager.current == .attaching(error: nil)) - // Post-test: Now that we’ve seen the ATTACHING state, allow the contributor `attach` call to complete + // Post-test: Now that we’ve seen the ATTACHING status, allow the contributor `attach` call to complete contributorAttachOperation.complete(result: .success) } @@ -244,7 +244,7 @@ struct RoomLifecycleManagerTests { // When: `performAttachOperation()` is called on the lifecycle manager try await manager.performAttachOperation() - // Then: It calls `attach` on all the contributors, the room attach operation succeeds, it emits a status change to ATTACHED, and its current state is ATTACHED + // Then: It calls `attach` on all the contributors, the room attach operation succeeds, it emits a status change to ATTACHED, and its current status is ATTACHED for contributor in contributors { #expect(await contributor.channel.attachCallCount > 0) } @@ -309,7 +309,7 @@ struct RoomLifecycleManagerTests { // @specPartial CHA-RL1h3 - Have tested the failure of the operation and the error that’s thrown. Have not yet implemented the "enter the recovery loop" (TODO: https://github.com/ably-labs/ably-chat-swift/issues/50) @Test func attach_whenContributorFailsToAttachAndEntersSuspended_transitionsToSuspended() async throws { - // Given: A RoomLifecycleManager, one of whose contributors’ call to `attach` fails causing it to enter the SUSPENDED state + // Given: A RoomLifecycleManager, one of whose contributors’ call to `attach` fails causing it to enter the SUSPENDED status let contributorAttachError = ARTErrorInfo(domain: "SomeDomain", code: 123) let contributors = (1 ... 3).map { i in if i == 1 { @@ -329,7 +329,7 @@ struct RoomLifecycleManagerTests { // Then: // - // 1. the room status transitions to SUSPENDED, with the state change’s `error` having the AttachmentFailed code corresponding to the feature of the failed contributor, `cause` equal to the error thrown by the contributor `attach` call + // 1. the room status transitions to SUSPENDED, with the status change’s `error` having the AttachmentFailed code corresponding to the feature of the failed contributor, `cause` equal to the error thrown by the contributor `attach` call // 2. the manager’s `error` is set to this same error // 3. the room attach operation fails with this same error let suspendedStatusChange = try #require(await maybeSuspendedStatusChange) @@ -379,7 +379,7 @@ struct RoomLifecycleManagerTests { async let roomAttachResult: Void = manager.performAttachOperation() // Then: - // 1. the room status transitions to FAILED, with the state change’s `error` having the AttachmentFailed code corresponding to the feature of the failed contributor, `cause` equal to the error thrown by the contributor `attach` call + // 1. the room status transitions to FAILED, with the status change’s `error` having the AttachmentFailed code corresponding to the feature of the failed contributor, `cause` equal to the error thrown by the contributor `attach` call // 2. the manager’s `error` is set to this same error // 3. the room attach operation fails with this same error let failedStatusChange = try #require(await maybeFailedStatusChange) @@ -477,7 +477,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL2a @Test func detach_whenAlreadyDetached() async throws { - // Given: A RoomLifecycleManager in the DETACHED state + // Given: A RoomLifecycleManager in the DETACHED status let contributor = createContributor() let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .detached, contributors: [contributor]) @@ -491,7 +491,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL2b @Test func detach_whenReleasing() async throws { - // Given: A RoomLifecycleManager in the RELEASING state + // Given: A RoomLifecycleManager in the RELEASING status let manager = await createManager( forTestingWhatHappensWhenCurrentlyIn: .releasing(releaseOperationID: UUID() /* arbitrary */ ) ) @@ -508,7 +508,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL2c @Test func detach_whenReleased() async throws { - // Given: A RoomLifecycleManager in the RELEASED state + // Given: A RoomLifecycleManager in the RELEASED status let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .released) // When: `performAttachOperation()` is called on the lifecycle manager @@ -523,7 +523,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL2d @Test func detach_whenFailed() async throws { - // Given: A RoomLifecycleManager in the FAILED state + // Given: A RoomLifecycleManager in the FAILED status let manager = await createManager( forTestingWhatHappensWhenCurrentlyIn: .failed( error: .createUnknownError() /* arbitrary */ @@ -542,7 +542,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL2e @Test func detach_transitionsToDetaching() async throws { - // Given: A RoomLifecycleManager, with a contributor on whom calling `detach()` will not complete until after the "Then" part of this test (the motivation for this is to suppress the room from transitioning to DETACHED, so that we can assert its current state as being DETACHING) + // Given: A RoomLifecycleManager, with a contributor on whom calling `detach()` will not complete until after the "Then" part of this test (the motivation for this is to suppress the room from transitioning to DETACHED, so that we can assert its current status as being DETACHING) let contributorDetachOperation = SignallableChannelOperation() let contributor = createContributor(detachBehavior: contributorDetachOperation.behavior) @@ -558,12 +558,12 @@ struct RoomLifecycleManagerTests { // When: `performDetachOperation()` is called on the lifecycle manager async let _ = try await manager.performDetachOperation() - // Then: It emits a status change to DETACHING, its current state is DETACHING, and it clears transient disconnect timeouts + // Then: It emits a status change to DETACHING, its current status is DETACHING, and it clears transient disconnect timeouts #expect(try #require(await statusChange).current == .detaching) #expect(await manager.current == .detaching) #expect(await !manager.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) - // Post-test: Now that we’ve seen the DETACHING state, allow the contributor `detach` call to complete + // Post-test: Now that we’ve seen the DETACHING status, allow the contributor `detach` call to complete contributorDetachOperation.complete(result: .success) } @@ -581,7 +581,7 @@ struct RoomLifecycleManagerTests { // When: `performDetachOperation()` is called on the lifecycle manager try await manager.performDetachOperation() - // Then: It calls `detach` on all the contributors, the room detach operation succeeds, it emits a status change to DETACHED, and its current state is DETACHED + // Then: It calls `detach` on all the contributors, the room detach operation succeeds, it emits a status change to DETACHED, and its current status is DETACHED for contributor in contributors { #expect(await contributor.channel.detachCallCount > 0) } @@ -626,7 +626,7 @@ struct RoomLifecycleManagerTests { // Then: It: // - calls `detach` on all of the contributors - // - emits a state change to FAILED and the call to `performDetachOperation()` fails; the error associated with the state change and the `performDetachOperation()` has the *DetachmentFailed code corresponding to contributor 1’s feature, and its `cause` is contributor 1’s `errorReason` (contributor 1 because it’s the "first feature to fail" as the spec says) + // - emits a status change to FAILED and the call to `performDetachOperation()` fails; the error associated with the status change and the `performDetachOperation()` has the *DetachmentFailed code corresponding to contributor 1’s feature, and its `cause` is contributor 1’s `errorReason` (contributor 1 because it’s the "first feature to fail" as the spec says) // TODO: Understand whether it’s `errorReason` or the contributor `detach` thrown error that’s meant to be use (outstanding question https://github.com/ably/specification/pull/200/files#r1763792152) for contributor in contributors { #expect(await contributor.channel.detachCallCount > 0) @@ -639,7 +639,7 @@ struct RoomLifecycleManagerTests { } } - // @specUntested CHA-RL2h2 - I was unable to find a way to test this spec point in an environment in which concurrency is being used; there is no obvious moment at which to stop observing the emitted state changes in order to be sure that FAILED has not been emitted twice. + // @specUntested CHA-RL2h2 - I was unable to find a way to test this spec point in an environment in which concurrency is being used; there is no obvious moment at which to stop observing the emitted status changes in order to be sure that FAILED has not been emitted twice. // @spec CHA-RL2h3 @Test @@ -679,7 +679,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL3a @Test func release_whenAlreadyReleased() async { - // Given: A RoomLifecycleManager in the RELEASED state + // Given: A RoomLifecycleManager in the RELEASED status let contributor = createContributor() let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .released, contributors: [contributor]) @@ -693,7 +693,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL3b @Test func release_whenDetached() async throws { - // Given: A RoomLifecycleManager in the DETACHED state + // Given: A RoomLifecycleManager in the DETACHED status let contributor = createContributor() let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .detached, contributors: [contributor]) @@ -712,7 +712,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL3c @Test func release_whenReleasing() async throws { - // Given: A RoomLifecycleManager with a RELEASE lifecycle operation in progress, and hence in the RELEASING state + // Given: A RoomLifecycleManager with a RELEASE lifecycle operation in progress, and hence in the RELEASING status let contributorDetachOperation = SignallableChannelOperation() let contributor = createContributor( // This allows us to prolong the execution of the RELEASE triggered in (1) @@ -758,7 +758,7 @@ struct RoomLifecycleManagerTests { // @spec CHA-RL3l @Test func release_transitionsToReleasing() async throws { - // Given: A RoomLifecycleManager, with a contributor on whom calling `detach()` will not complete until after the "Then" part of this test (the motivation for this is to suppress the room from transitioning to RELEASED, so that we can assert its current state as being RELEASING) + // Given: A RoomLifecycleManager, with a contributor on whom calling `detach()` will not complete until after the "Then" part of this test (the motivation for this is to suppress the room from transitioning to RELEASED, so that we can assert its current status as being RELEASING) let contributorDetachOperation = SignallableChannelOperation() let contributor = createContributor(detachBehavior: contributorDetachOperation.behavior) @@ -774,12 +774,12 @@ struct RoomLifecycleManagerTests { // When: `performReleaseOperation()` is called on the lifecycle manager async let _ = await manager.performReleaseOperation() - // Then: It emits a status change to RELEASING, its current state is RELEASING, and it clears transient disconnect timeouts + // Then: It emits a status change to RELEASING, its current status is RELEASING, and it clears transient disconnect timeouts #expect(try #require(await statusChange).current == .releasing) #expect(await manager.current == .releasing) #expect(await !manager.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) - // Post-test: Now that we’ve seen the RELEASING state, allow the contributor `detach` call to complete + // Post-test: Now that we’ve seen the RELEASING status, allow the contributor `detach` call to complete contributorDetachOperation.complete(result: .success) } @@ -1211,14 +1211,14 @@ struct RoomLifecycleManagerTests { } // Then: The manager’s status remains unchanged. In particular, it has not changed to ATTACHING, meaning that the CHA-RL4b7 side effect has not happened and hence that the transient disconnect timeout was properly cancelled - #expect(await manager.current == initialManagerStatus.toRoomLifecycle) + #expect(await manager.current == initialManagerStatus.toRoomStatus) #expect(await !manager.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) } // @specOneOf(1/2) CHA-RL4b8 @Test func contributorAttachedEvent_withNoOperationInProgress_roomNotAttached_allContributorsAttached() async throws { - // Given: A RoomLifecycleManager, with no operation in progress and not in the ATTACHED state, all of whose contributors are in the ATTACHED state (to satisfy the condition of CHA-RL4b8; for the purposes of this test I don’t care that they’re in this state even _before_ the state change of the When) + // Given: A RoomLifecycleManager, with no operation in progress and not in the ATTACHED status, all of whose contributors are in the ATTACHED state (to satisfy the condition of CHA-RL4b8; for the purposes of this test I don’t care that they’re in this state even _before_ the state change of the When) let contributors = [ createContributor(initialState: .attached), createContributor(initialState: .attached), @@ -1251,7 +1251,7 @@ struct RoomLifecycleManagerTests { // @specOneOf(2/2) CHA-RL4b8 - Tests that the specified side effect doesn’t happen if part of the condition (i.e. all contributors now being ATTACHED) is not met @Test func contributorAttachedEvent_withNoOperationInProgress_roomNotAttached_notAllContributorsAttached() async throws { - // Given: A RoomLifecycleManager, with no operation in progress and not in the ATTACHED state, one of whose contributors is not in the ATTACHED state state (to simulate the condition of CHA-RL4b8 not being met; for the purposes of this test I don’t care that they’re in this state even _before_ the state change of the When) + // Given: A RoomLifecycleManager, with no operation in progress and not in the ATTACHED status, one of whose contributors is not in the ATTACHED state state (to simulate the condition of CHA-RL4b8 not being met; for the purposes of this test I don’t care that they’re in this state even _before_ the state change of the When) let contributors = [ createContributor(initialState: .attached), createContributor(initialState: .detached), @@ -1277,7 +1277,7 @@ struct RoomLifecycleManagerTests { } // Then: The room status does not change - #expect(await manager.current == initialManagerStatus.toRoomLifecycle) + #expect(await manager.current == initialManagerStatus.toRoomStatus) } // @specPartial CHA-RL4b9 - Haven’t implemented "the room enters the RETRY loop"; TODO do this (https://github.com/ably-labs/ably-chat-swift/issues/51) @@ -1318,7 +1318,7 @@ struct RoomLifecycleManagerTests { } // Then: - // - The room transitions to SUSPENDED, and this state change has error equal to the contributor state change’s `reason` + // - The room transitions to SUSPENDED, and this status change has error equal to the contributor state change’s `reason` // - All transient disconnect timeouts are cancelled let suspendedRoomStatusChange = try #require(await maybeSuspendedRoomStatusChange) #expect(suspendedRoomStatusChange.error === contributorStateChangeReason) From b39621cf1157d10a8c3ba731ae63ce758dda9d18 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 28 Oct 2024 15:25:23 -0300 Subject: [PATCH 4/5] =?UTF-8?q?Rename=20RoomLifecycleManager=E2=80=99s=20`?= =?UTF-8?q?current`=20property?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was called `current` in 25e5052 to match the naming used in the public API at the time. Post-7e4460d, calling it `roomStatus` makes more sense (or even `status` but that’s already taken by a private property). --- Sources/AblyChat/RoomLifecycleManager.swift | 2 +- .../RoomLifecycleManagerTests.swift | 36 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index c820222e..c8d8d125 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -264,7 +264,7 @@ internal actor RoomLifecycleManager { // MARK: - Room status and its changes - internal var current: RoomStatus { + internal var roomStatus: RoomStatus { status.toRoomStatus } diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index d18176d8..5b6d5e7a 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -109,7 +109,7 @@ struct RoomLifecycleManagerTests { func current_startsAsInitialized() async { let manager = await createManager() - #expect(await manager.current == .initialized) + #expect(await manager.roomStatus == .initialized) } // MARK: - ATTACH operation @@ -224,7 +224,7 @@ struct RoomLifecycleManagerTests { // Then: It emits a status change to ATTACHING, and its current status is ATTACHING #expect(try #require(await statusChange).current == .attaching(error: nil)) - #expect(await manager.current == .attaching(error: nil)) + #expect(await manager.roomStatus == .attaching(error: nil)) // Post-test: Now that we’ve seen the ATTACHING status, allow the contributor `attach` call to complete contributorAttachOperation.complete(result: .success) @@ -250,7 +250,7 @@ struct RoomLifecycleManagerTests { } _ = try #require(await attachedStatusChange, "Expected status change to ATTACHED") - try #require(await manager.current == .attached) + try #require(await manager.roomStatus == .attached) } // @spec CHA-RL1g2 @@ -334,7 +334,7 @@ struct RoomLifecycleManagerTests { // 3. the room attach operation fails with this same error let suspendedStatusChange = try #require(await maybeSuspendedStatusChange) - #expect(await manager.current.isSuspended) + #expect(await manager.roomStatus.isSuspended) var roomAttachError: Error? do { @@ -343,7 +343,7 @@ struct RoomLifecycleManagerTests { roomAttachError = error } - for error in await [suspendedStatusChange.error, manager.current.error, roomAttachError] { + for error in await [suspendedStatusChange.error, manager.roomStatus.error, roomAttachError] { #expect(isChatError(error, withCode: .messagesAttachmentFailed, cause: contributorAttachError)) } } @@ -384,7 +384,7 @@ struct RoomLifecycleManagerTests { // 3. the room attach operation fails with this same error let failedStatusChange = try #require(await maybeFailedStatusChange) - #expect(await manager.current.isFailed) + #expect(await manager.roomStatus.isFailed) var roomAttachError: Error? do { @@ -393,7 +393,7 @@ struct RoomLifecycleManagerTests { roomAttachError = error } - for error in await [failedStatusChange.error, manager.current.error, roomAttachError] { + for error in await [failedStatusChange.error, manager.roomStatus.error, roomAttachError] { #expect(isChatError(error, withCode: .messagesAttachmentFailed, cause: contributorAttachError)) } } @@ -560,7 +560,7 @@ struct RoomLifecycleManagerTests { // Then: It emits a status change to DETACHING, its current status is DETACHING, and it clears transient disconnect timeouts #expect(try #require(await statusChange).current == .detaching) - #expect(await manager.current == .detaching) + #expect(await manager.roomStatus == .detaching) #expect(await !manager.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) // Post-test: Now that we’ve seen the DETACHING status, allow the contributor `detach` call to complete @@ -587,7 +587,7 @@ struct RoomLifecycleManagerTests { } _ = try #require(await detachedStatusChange, "Expected status change to DETACHED") - #expect(await manager.current == .detached) + #expect(await manager.roomStatus == .detached) } // @spec CHA-RL2h1 @@ -705,7 +705,7 @@ struct RoomLifecycleManagerTests { // Then: The room release operation succeeds, the room transitions to RELEASED, and no attempt is made to detach a contributor (which we’ll consider as satisfying the spec’s requirement that the transition be "immediate") #expect(try #require(await statusChange).current == .released) - #expect(await manager.current == .released) + #expect(await manager.roomStatus == .released) #expect(await contributor.channel.detachCallCount == 0) } @@ -776,7 +776,7 @@ struct RoomLifecycleManagerTests { // Then: It emits a status change to RELEASING, its current status is RELEASING, and it clears transient disconnect timeouts #expect(try #require(await statusChange).current == .releasing) - #expect(await manager.current == .releasing) + #expect(await manager.roomStatus == .releasing) #expect(await !manager.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) // Post-test: Now that we’ve seen the RELEASING status, allow the contributor `detach` call to complete @@ -819,7 +819,7 @@ struct RoomLifecycleManagerTests { _ = await releasedStatusChange - #expect(await manager.current == .released) + #expect(await manager.roomStatus == .released) } // @spec CHA-RL3f @@ -878,7 +878,7 @@ struct RoomLifecycleManagerTests { _ = await releasedStatusChange - #expect(await manager.current == .released) + #expect(await manager.roomStatus == .released) } // MARK: - Handling contributor UPDATE events @@ -1042,7 +1042,7 @@ struct RoomLifecycleManagerTests { // - and it calls `detach` on all contributors // - it clears all transient disconnect timeouts _ = try #require(await failedStatusChange) - #expect(await manager.current.isFailed) + #expect(await manager.roomStatus.isFailed) for contributor in contributors { #expect(await contributor.channel.detachCallCount == 1) @@ -1211,7 +1211,7 @@ struct RoomLifecycleManagerTests { } // Then: The manager’s status remains unchanged. In particular, it has not changed to ATTACHING, meaning that the CHA-RL4b7 side effect has not happened and hence that the transient disconnect timeout was properly cancelled - #expect(await manager.current == initialManagerStatus.toRoomStatus) + #expect(await manager.roomStatus == initialManagerStatus.toRoomStatus) #expect(await !manager.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) } @@ -1245,7 +1245,7 @@ struct RoomLifecycleManagerTests { // Then: The room status transitions to ATTACHED _ = try #require(await maybeAttachedRoomStatusChange) - #expect(await manager.current == .attached) + #expect(await manager.roomStatus == .attached) } // @specOneOf(2/2) CHA-RL4b8 - Tests that the specified side effect doesn’t happen if part of the condition (i.e. all contributors now being ATTACHED) is not met @@ -1277,7 +1277,7 @@ struct RoomLifecycleManagerTests { } // Then: The room status does not change - #expect(await manager.current == initialManagerStatus.toRoomStatus) + #expect(await manager.roomStatus == initialManagerStatus.toRoomStatus) } // @specPartial CHA-RL4b9 - Haven’t implemented "the room enters the RETRY loop"; TODO do this (https://github.com/ably-labs/ably-chat-swift/issues/51) @@ -1323,7 +1323,7 @@ struct RoomLifecycleManagerTests { let suspendedRoomStatusChange = try #require(await maybeSuspendedRoomStatusChange) #expect(suspendedRoomStatusChange.error === contributorStateChangeReason) - #expect(await manager.current == .suspended(error: contributorStateChangeReason)) + #expect(await manager.roomStatus == .suspended(error: contributorStateChangeReason)) #expect(await !manager.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) } From b771238b225b7c8e21e082be7ff6258b1b3a5c98 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 28 Oct 2024 15:31:08 -0300 Subject: [PATCH 5/5] =?UTF-8?q?Apply=20CHADR-062=E2=80=99s=20public=20API?= =?UTF-8?q?=20changes=20to=20Connection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As we did for Room in 7e4460d (a lot fewer changes here, though, because we haven’t implemented Connection yet). --- Sources/AblyChat/Connection.swift | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/Sources/AblyChat/Connection.swift b/Sources/AblyChat/Connection.swift index c53e954e..5efa165e 100644 --- a/Sources/AblyChat/Connection.swift +++ b/Sources/AblyChat/Connection.swift @@ -1,17 +1,13 @@ import Ably public protocol Connection: AnyObject, Sendable { - var status: any ConnectionStatus { get } -} - -public protocol ConnectionStatus: AnyObject, Sendable { - var current: ConnectionLifecycle { get } + var status: ConnectionStatus { get } // 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 + func onStatusChange(bufferingPolicy: BufferingPolicy) -> Subscription } -public enum ConnectionLifecycle: Sendable { +public enum ConnectionStatus: Sendable { case initialized case connecting case connected @@ -21,13 +17,13 @@ public enum ConnectionLifecycle: Sendable { } public struct ConnectionStatusChange: Sendable { - public var current: ConnectionLifecycle - public var previous: ConnectionLifecycle + public var current: ConnectionStatus + public var previous: ConnectionStatus // TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap public var error: ARTErrorInfo? public var retryIn: TimeInterval - public init(current: ConnectionLifecycle, previous: ConnectionLifecycle, error: ARTErrorInfo? = nil, retryIn: TimeInterval) { + public init(current: ConnectionStatus, previous: ConnectionStatus, error: ARTErrorInfo? = nil, retryIn: TimeInterval) { self.current = current self.previous = previous self.error = error