From be38b8790febf55bd4aa8f9beee1399f9d7ebf62 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 1 Oct 2024 17:55:06 -0300 Subject: [PATCH 1/4] 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 64c03155cc76aae1912e71416eae9e7d4e834fe9 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 1 Oct 2024 17:36:03 -0300 Subject: [PATCH 2/4] Rename public API for room lifecycle and status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We: - rename the RoomLifecycle enum to RoomLifecycleStatus (which makes more sense); - rename the RoomStatus protocol to RoomLifecycle and rename its `current` property to `status` ("current" what?; it wasn’t clear, and was inconsistent with the core SDKs, which only use `current` for state _changes_) The net effect is that instead of writing `room.status.current` you now write `room.lifecycle.status`, which still seems pretty readable and gives the benefits listed above. This change has been agreed with the Chat team and the decision is recorded in [1]; they will make this change in JS. 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). Outstanding spec question about whether to rename the RoomInFailedState error [3]; that can wait. [1] https://ably.atlassian.net/wiki/spaces/CHA/pages/3307405320/SDK+Divergence+Decision+Log [2] https://github.com/ably-labs/ably-chat-swift/issues/73#issuecomment-2385770039 [3] https://github.com/ably/specification/pull/200/files#r1783542331 --- Sources/AblyChat/Room.swift | 14 +++--- .../{RoomStatus.swift => RoomLifecycle.swift} | 26 +++++----- Sources/AblyChat/RoomLifecycleManager.swift | 8 +-- .../DefaultRoomLifecycleTests.swift | 41 +++++++++++++++ .../DefaultRoomStatusTests.swift | 41 --------------- Tests/AblyChatTests/DefaultRoomTests.swift | 8 +-- .../RoomLifecycleManagerTests.swift | 50 +++++++++---------- 7 files changed, 94 insertions(+), 94 deletions(-) rename Sources/AblyChat/{RoomStatus.swift => RoomLifecycle.swift} (68%) create mode 100644 Tests/AblyChatTests/DefaultRoomLifecycleTests.swift delete mode 100644 Tests/AblyChatTests/DefaultRoomStatusTests.swift diff --git a/Sources/AblyChat/Room.swift b/Sources/AblyChat/Room.swift index e4660c8d..e1afd732 100644 --- a/Sources/AblyChat/Room.swift +++ b/Sources/AblyChat/Room.swift @@ -11,7 +11,7 @@ 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 } + var lifecycle: any RoomLifecycle { get } func attach() async throws func detach() async throws var options: RoomOptions { get } @@ -30,7 +30,7 @@ internal actor DefaultRoom: Room { } #endif - private let _status: DefaultRoomStatus + private let _lifecycle: DefaultRoomLifecycle private let logger: InternalLogger internal init(realtime: RealtimeClient, roomID: String, options: RoomOptions, logger: InternalLogger) { @@ -38,7 +38,7 @@ internal actor DefaultRoom: Room { self.roomID = roomID self.options = options self.logger = logger - _status = .init(logger: logger) + _lifecycle = .init(logger: logger) } public nonisolated var messages: any Messages { @@ -61,8 +61,8 @@ internal actor DefaultRoom: Room { fatalError("Not yet implemented") } - internal nonisolated var status: any RoomStatus { - _status + internal nonisolated var lifecycle: any RoomLifecycle { + _lifecycle } /// Fetches the channels that contribute to this room. @@ -85,7 +85,7 @@ internal actor DefaultRoom: Room { throw error } } - await _status.transition(to: .attached) + await _lifecycle.transition(to: .attached) } public func detach() async throws { @@ -97,6 +97,6 @@ internal actor DefaultRoom: Room { throw error } } - await _status.transition(to: .detached) + await _lifecycle.transition(to: .detached) } } diff --git a/Sources/AblyChat/RoomStatus.swift b/Sources/AblyChat/RoomLifecycle.swift similarity index 68% rename from Sources/AblyChat/RoomStatus.swift rename to Sources/AblyChat/RoomLifecycle.swift index b26c1f48..ed22a93a 100644 --- a/Sources/AblyChat/RoomStatus.swift +++ b/Sources/AblyChat/RoomLifecycle.swift @@ -1,13 +1,13 @@ import Ably -public protocol RoomStatus: AnyObject, Sendable { - var current: RoomLifecycle { get async } +public protocol RoomLifecycle: AnyObject, Sendable { + var status: RoomStatus { 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 async } func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription } -public enum RoomLifecycle: Sendable { +public enum RoomStatus: Sendable { case initialized case attaching case attached @@ -20,20 +20,20 @@ public enum RoomLifecycle: Sendable { } public struct RoomStatusChange: Sendable { - public var current: RoomLifecycle - public var previous: RoomLifecycle + public var current: RoomStatus + public var previous: RoomStatus // 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 init(current: RoomLifecycle, previous: RoomLifecycle, error: ARTErrorInfo? = nil) { + public init(current: RoomStatus, previous: RoomStatus, error: ARTErrorInfo? = nil) { self.current = current self.previous = previous self.error = error } } -internal actor DefaultRoomStatus: RoomStatus { - internal private(set) var current: RoomLifecycle = .initialized +internal actor DefaultRoomLifecycle: RoomLifecycle { + internal private(set) var status: RoomStatus = .initialized // TODO: populate this (https://github.com/ably-labs/ably-chat-swift/issues/28) internal private(set) var error: ARTErrorInfo? @@ -52,11 +52,11 @@ internal actor DefaultRoomStatus: RoomStatus { 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 + /// Sets ``status`` to the given status, and emits a status change to all subscribers added via ``onChange(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 subscriptions { subscription.emit(statusChange) } diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 0bc540a6..eb8f3730 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -22,7 +22,7 @@ internal actor RoomLifecycleManager { internal var channel: Channel } - internal private(set) var current: RoomLifecycle + internal private(set) var current: RoomStatus internal private(set) var error: ARTErrorInfo? private let logger: InternalLogger @@ -44,7 +44,7 @@ internal actor RoomLifecycleManager { #if DEBUG internal init( - testsOnly_current current: RoomLifecycle? = nil, + testsOnly_current current: RoomStatus? = nil, contributors: [Contributor], logger: InternalLogger, clock: SimpleClock @@ -59,7 +59,7 @@ internal actor RoomLifecycleManager { #endif private init( - current: RoomLifecycle?, + current: RoomStatus?, contributors: [Contributor], logger: InternalLogger, clock: SimpleClock @@ -80,7 +80,7 @@ internal actor RoomLifecycleManager { } /// Updates ``current`` and ``error`` and emits a status change event. - private func changeStatus(to new: RoomLifecycle, error: ARTErrorInfo? = nil) { + private func changeStatus(to new: RoomStatus, error: ARTErrorInfo? = nil) { logger.log(message: "Transitioning from \(current) to \(new), error \(String(describing: error))", level: .info) let previous = current current = new diff --git a/Tests/AblyChatTests/DefaultRoomLifecycleTests.swift b/Tests/AblyChatTests/DefaultRoomLifecycleTests.swift new file mode 100644 index 00000000..efb8b4e3 --- /dev/null +++ b/Tests/AblyChatTests/DefaultRoomLifecycleTests.swift @@ -0,0 +1,41 @@ +@testable import AblyChat +import Testing + +struct DefaultRoomLifecycleTests { + @Test + func current_startsAsInitialized() async { + let lifecycle = DefaultRoomLifecycle(logger: TestLogger()) + #expect(await lifecycle.status == .initialized) + } + + @Test() + func error_startsAsNil() async { + let lifecycle = DefaultRoomLifecycle(logger: TestLogger()) + #expect(await lifecycle.error == nil) + } + + @Test + func transition() async throws { + // Given: A DefaultRoomLifecycle + let lifecycle = DefaultRoomLifecycle(logger: TestLogger()) + let originalStatus = await lifecycle.status + let newStatus = RoomStatus.attached // arbitrary + + let subscription1 = await lifecycle.onChange(bufferingPolicy: .unbounded) + let subscription2 = await lifecycle.onChange(bufferingPolicy: .unbounded) + + async let statusChange1 = subscription1.first { $0.current == newStatus } + async let statusChange2 = subscription2.first { $0.current == newStatus } + + // When: transition(to:) is called + await lifecycle.transition(to: newStatus) + + // Then: It emits a status change to all subscribers added via onChange(bufferingPolicy:), and updates its `status` property to the new status + for statusChange in try await [#require(statusChange1), #require(statusChange2)] { + #expect(statusChange.previous == originalStatus) + #expect(statusChange.current == newStatus) + } + + #expect(await lifecycle.status == .attached) + } +} diff --git a/Tests/AblyChatTests/DefaultRoomStatusTests.swift b/Tests/AblyChatTests/DefaultRoomStatusTests.swift deleted file mode 100644 index 8503b28e..00000000 --- a/Tests/AblyChatTests/DefaultRoomStatusTests.swift +++ /dev/null @@ -1,41 +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 error_startsAsNil() async { - let status = DefaultRoomStatus(logger: TestLogger()) - #expect(await status.error == nil) - } - - @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 8956a7e0..70424f9b 100644 --- a/Tests/AblyChatTests/DefaultRoomTests.swift +++ b/Tests/AblyChatTests/DefaultRoomTests.swift @@ -19,7 +19,7 @@ struct DefaultRoomTests { let realtime = MockRealtime.create(channels: channels) let room = DefaultRoom(realtime: realtime, roomID: "basketball", options: .init(), logger: TestLogger()) - let subscription = await room.status.onChange(bufferingPolicy: .unbounded) + let subscription = await room.lifecycle.onChange(bufferingPolicy: .unbounded) async let attachedStatusChange = subscription.first { $0.current == .attached } // When: `attach` is called on the room @@ -30,7 +30,7 @@ struct DefaultRoomTests { #expect(channel.attachCallCounter.isNonZero) } - #expect(await room.status.current == .attached) + #expect(await room.lifecycle.status == .attached) #expect(try #require(await attachedStatusChange).current == .attached) } @@ -81,7 +81,7 @@ struct DefaultRoomTests { let realtime = MockRealtime.create(channels: channels) let room = DefaultRoom(realtime: realtime, roomID: "basketball", options: .init(), logger: TestLogger()) - let subscription = await room.status.onChange(bufferingPolicy: .unbounded) + let subscription = await room.lifecycle.onChange(bufferingPolicy: .unbounded) async let detachedStatusChange = subscription.first { $0.current == .detached } // When: `detach` is called on the room @@ -92,7 +92,7 @@ struct DefaultRoomTests { #expect(channel.detachCallCounter.isNonZero) } - #expect(await room.status.current == .detached) + #expect(await room.lifecycle.status == .detached) #expect(try #require(await detachedStatusChange).current == .detached) } diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index 6ac1e543..5e57ec50 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -28,7 +28,7 @@ struct RoomLifecycleManagerTests { } private func createManager( - forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle? = nil, + forTestingWhatHappensWhenCurrentlyIn current: RoomStatus? = nil, contributors: [RoomLifecycleManager.Contributor] = [], clock: SimpleClock = MockSimpleClock() ) -> RoomLifecycleManager { @@ -79,7 +79,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 = createManager(forTestingWhatHappensWhenCurrentlyIn: .attached, contributors: [contributor]) @@ -93,7 +93,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 = createManager(forTestingWhatHappensWhenCurrentlyIn: .releasing) // When: `performAttachOperation()` is called on the lifecycle manager @@ -108,7 +108,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 = createManager(forTestingWhatHappensWhenCurrentlyIn: .released) // When: `performAttachOperation()` is called on the lifecycle manager @@ -123,7 +123,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 = createManager(contributors: [createContributor(attachBehavior: contributorAttachOperation.behavior)]) @@ -133,12 +133,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) #expect(await manager.current == .attaching) - // 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) } @@ -156,7 +156,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) } @@ -190,7 +190,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) @@ -240,7 +240,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) @@ -338,7 +338,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 = createManager(forTestingWhatHappensWhenCurrentlyIn: .detached, contributors: [contributor]) @@ -352,7 +352,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 = createManager(forTestingWhatHappensWhenCurrentlyIn: .releasing) // When: `performDetachOperation()` is called on the lifecycle manager @@ -367,7 +367,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 = createManager(forTestingWhatHappensWhenCurrentlyIn: .released) // When: `performAttachOperation()` is called on the lifecycle manager @@ -382,7 +382,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 = createManager(forTestingWhatHappensWhenCurrentlyIn: .failed) // When: `performAttachOperation()` is called on the lifecycle manager @@ -397,7 +397,7 @@ struct RoomLifecycleManagerTests { // @specPartial CHA-RL2e - Haven’t implemented the part that refers to "transient disconnect timeouts"; TODO do this (https://github.com/ably-labs/ably-chat-swift/issues/48) @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 manager = createManager(contributors: [createContributor(detachBehavior: contributorDetachOperation.behavior)]) @@ -407,11 +407,11 @@ struct RoomLifecycleManagerTests { // When: `performDetachOperation()` is called on the lifecycle manager async let _ = try await manager.performDetachOperation() - // Then: It emits a status change to DETACHING, and its current state is DETACHING + // Then: It emits a status change to DETACHING, and its current status is DETACHING #expect(try #require(await statusChange).current == .detaching) #expect(await manager.current == .detaching) - // 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) } @@ -429,7 +429,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) } @@ -474,7 +474,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) @@ -487,7 +487,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 @@ -527,7 +527,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 = createManager(forTestingWhatHappensWhenCurrentlyIn: .released, contributors: [contributor]) @@ -541,7 +541,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 = createManager(forTestingWhatHappensWhenCurrentlyIn: .detached, contributors: [contributor]) @@ -560,7 +560,7 @@ struct RoomLifecycleManagerTests { // @specPartial CHA-RL3c - Haven’t implemented the part that refers to "transient disconnect timeouts"; TODO do this (https://github.com/ably-labs/ably-chat-swift/issues/48) @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 manager = createManager(contributors: [createContributor(detachBehavior: contributorDetachOperation.behavior)]) @@ -570,11 +570,11 @@ struct RoomLifecycleManagerTests { // When: `performReleaseOperation()` is called on the lifecycle manager async let _ = await manager.performReleaseOperation() - // Then: It emits a status change to RELEASING, and its current state is RELEASING + // Then: It emits a status change to RELEASING, and its current status is RELEASING #expect(try #require(await statusChange).current == .releasing) #expect(await manager.current == .releasing) - // 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) } From ad529b10f442eecdc33f5517a9ab9c7cc9d24449 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 1 Oct 2024 17:59:32 -0300 Subject: [PATCH 3/4] Rename RoomLifecycleManager.current It was called `current` in 25e5052 to match the naming used in the public API at the time. Post-64c0315, calling it `status` makes more sense. --- Sources/AblyChat/RoomLifecycleManager.swift | 30 +++++++++---------- .../DefaultRoomLifecycleTests.swift | 2 +- .../RoomLifecycleManagerTests.swift | 28 ++++++++--------- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index eb8f3730..390cde65 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -22,7 +22,7 @@ internal actor RoomLifecycleManager { internal var channel: Channel } - internal private(set) var current: RoomStatus + internal private(set) var status: RoomStatus internal private(set) var error: ARTErrorInfo? private let logger: InternalLogger @@ -35,7 +35,7 @@ internal actor RoomLifecycleManager { clock: SimpleClock ) { self.init( - current: nil, + status: nil, contributors: contributors, logger: logger, clock: clock @@ -44,13 +44,13 @@ internal actor RoomLifecycleManager { #if DEBUG internal init( - testsOnly_current current: RoomStatus? = nil, + testsOnly_status status: RoomStatus? = nil, contributors: [Contributor], logger: InternalLogger, clock: SimpleClock ) { self.init( - current: current, + status: status, contributors: contributors, logger: logger, clock: clock @@ -59,12 +59,12 @@ internal actor RoomLifecycleManager { #endif private init( - current: RoomStatus?, + status: RoomStatus?, contributors: [Contributor], logger: InternalLogger, clock: SimpleClock ) { - self.current = current ?? .initialized + self.status = status ?? .initialized self.contributors = contributors self.logger = logger self.clock = clock @@ -79,13 +79,13 @@ internal actor RoomLifecycleManager { return subscription } - /// Updates ``current`` and ``error`` and emits a status change event. + /// Updates ``status`` and ``error`` and emits a status change event. private func changeStatus(to new: RoomStatus, error: ARTErrorInfo? = nil) { - logger.log(message: "Transitioning from \(current) to \(new), error \(String(describing: error))", level: .info) - let previous = current - current = new + logger.log(message: "Transitioning from \(status) to \(new), error \(String(describing: error))", level: .info) + let previous = status + status = new self.error = error - let statusChange = RoomStatusChange(current: current, previous: previous, error: error) + let statusChange = RoomStatusChange(current: status, previous: previous, error: error) emitStatusChange(statusChange) } @@ -97,7 +97,7 @@ internal actor RoomLifecycleManager { /// Implements CHA-RL1’s `ATTACH` operation. internal func performAttachOperation() async throws { - switch current { + switch status { case .attached: // CHA-RL1a return @@ -171,7 +171,7 @@ internal actor RoomLifecycleManager { /// Implements CHA-RL2’s DETACH operation. internal func performDetachOperation() async throws { - switch current { + switch status { case .detached: // CHA-RL2a return @@ -217,7 +217,7 @@ internal actor RoomLifecycleManager { } // This check is CHA-RL2h2 - if current != .failed { + if status != .failed { changeStatus(to: .failed, error: error) } default: @@ -250,7 +250,7 @@ internal actor RoomLifecycleManager { /// Implementes CHA-RL3’s RELEASE operation. internal func performReleaseOperation() async { - switch current { + switch status { case .released: // CHA-RL3a return diff --git a/Tests/AblyChatTests/DefaultRoomLifecycleTests.swift b/Tests/AblyChatTests/DefaultRoomLifecycleTests.swift index efb8b4e3..a3d3f065 100644 --- a/Tests/AblyChatTests/DefaultRoomLifecycleTests.swift +++ b/Tests/AblyChatTests/DefaultRoomLifecycleTests.swift @@ -3,7 +3,7 @@ import Testing struct DefaultRoomLifecycleTests { @Test - func current_startsAsInitialized() async { + func status_startsAsInitialized() async { let lifecycle = DefaultRoomLifecycle(logger: TestLogger()) #expect(await lifecycle.status == .initialized) } diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index 5e57ec50..54b33079 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -28,12 +28,12 @@ struct RoomLifecycleManagerTests { } private func createManager( - forTestingWhatHappensWhenCurrentlyIn current: RoomStatus? = nil, + forTestingWhatHappensWhenCurrentlyIn status: RoomStatus? = nil, contributors: [RoomLifecycleManager.Contributor] = [], clock: SimpleClock = MockSimpleClock() ) -> RoomLifecycleManager { .init( - testsOnly_current: current, + testsOnly_status: status, contributors: contributors, logger: TestLogger(), clock: clock @@ -61,10 +61,10 @@ struct RoomLifecycleManagerTests { // @spec CHA-RS2a // @spec CHA-RS3 @Test - func current_startsAsInitialized() async { + func status_startsAsInitialized() async { let manager = createManager() - #expect(await manager.current == .initialized) + #expect(await manager.status == .initialized) } @Test @@ -136,7 +136,7 @@ struct RoomLifecycleManagerTests { // Then: It emits a status change to ATTACHING, and its current status is ATTACHING #expect(try #require(await statusChange).current == .attaching) - #expect(await manager.current == .attaching) + #expect(await manager.status == .attaching) // Post-test: Now that we’ve seen the ATTACHING status, allow the contributor `attach` call to complete contributorAttachOperation.complete(result: .success) @@ -162,7 +162,7 @@ struct RoomLifecycleManagerTests { } _ = try #require(await attachedStatusChange, "Expected status change to ATTACHED") - try #require(await manager.current == .attached) + try #require(await manager.status == .attached) } // @spec CHA-RL1h2 @@ -195,7 +195,7 @@ struct RoomLifecycleManagerTests { // 3. the room attach operation fails with this same error let suspendedStatusChange = try #require(await maybeSuspendedStatusChange) - #expect(await manager.current == .suspended) + #expect(await manager.status == .suspended) var roomAttachError: Error? do { @@ -245,7 +245,7 @@ struct RoomLifecycleManagerTests { // 3. the room attach operation fails with this same error let failedStatusChange = try #require(await maybeFailedStatusChange) - #expect(await manager.current == .failed) + #expect(await manager.status == .failed) var roomAttachError: Error? do { @@ -409,7 +409,7 @@ struct RoomLifecycleManagerTests { // Then: It emits a status change to DETACHING, and its current status is DETACHING #expect(try #require(await statusChange).current == .detaching) - #expect(await manager.current == .detaching) + #expect(await manager.status == .detaching) // Post-test: Now that we’ve seen the DETACHING status, allow the contributor `detach` call to complete contributorDetachOperation.complete(result: .success) @@ -435,7 +435,7 @@ struct RoomLifecycleManagerTests { } _ = try #require(await detachedStatusChange, "Expected status change to DETACHED") - #expect(await manager.current == .detached) + #expect(await manager.status == .detached) } // @spec CHA-RL2h1 @@ -553,7 +553,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.status == .released) #expect(await contributor.channel.detachCallCount == 0) } @@ -572,7 +572,7 @@ struct RoomLifecycleManagerTests { // Then: It emits a status change to RELEASING, and its current status is RELEASING #expect(try #require(await statusChange).current == .releasing) - #expect(await manager.current == .releasing) + #expect(await manager.status == .releasing) // Post-test: Now that we’ve seen the RELEASING status, allow the contributor `detach` call to complete contributorDetachOperation.complete(result: .success) @@ -614,7 +614,7 @@ struct RoomLifecycleManagerTests { _ = await releasedStatusChange - #expect(await manager.current == .released) + #expect(await manager.status == .released) } // @spec CHA-RL3f @@ -673,6 +673,6 @@ struct RoomLifecycleManagerTests { _ = await releasedStatusChange - #expect(await manager.current == .released) + #expect(await manager.status == .released) } } From 07ffb72e30da914645470934ba1eef3761f9bdc6 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 1 Oct 2024 18:24:15 -0300 Subject: [PATCH 4/4] Perform analogous change to 64c0315 for Connection Same motivation. Resolves #73. --- Sources/AblyChat/Connection.swift | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Sources/AblyChat/Connection.swift b/Sources/AblyChat/Connection.swift index c53e954e..d7b31bf8 100644 --- a/Sources/AblyChat/Connection.swift +++ b/Sources/AblyChat/Connection.swift @@ -1,17 +1,17 @@ import Ably public protocol Connection: AnyObject, Sendable { - var status: any ConnectionStatus { get } + var lifecycle: any ConnectionLifecycle { get } } -public protocol ConnectionStatus: AnyObject, Sendable { - var current: ConnectionLifecycle { get } +public protocol ConnectionLifecycle: AnyObject, Sendable { + 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 } -public enum ConnectionLifecycle: Sendable { +public enum ConnectionStatus: Sendable { case initialized case connecting case connected @@ -21,13 +21,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