diff --git a/Example/AblyChatExample/Mocks/MockClients.swift b/Example/AblyChatExample/Mocks/MockClients.swift index 3e8d1bc8..14a31d71 100644 --- a/Example/AblyChatExample/Mocks/MockClients.swift +++ b/Example/AblyChatExample/Mocks/MockClients.swift @@ -412,7 +412,7 @@ actor MockRoomStatus: RoomStatus { private func createSubscription() -> MockSubscription { let subscription = MockSubscription(randomElement: { - RoomStatusChange(current: [.attached, .attached, .attached, .attached, .attaching, .attaching, .suspended].randomElement()!, previous: .attaching) + RoomStatusChange(current: [.attached, .attached, .attached, .attached, .attaching, .attaching, .suspended(error: .createUnknownError())].randomElement()!, previous: .attaching) }, interval: 8) mockSubscriptions.append(subscription) return subscription diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 42041609..dbbf34ee 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -48,7 +48,6 @@ internal actor RoomLifecycleManager { } internal private(set) var current: RoomLifecycle - internal private(set) var error: ARTErrorInfo? // TODO: This currently allows the the tests to inject a value in order to test the spec points that are predicated on whether “a channel lifecycle operation is in progress”. In https://github.com/ably-labs/ably-chat-swift/issues/52 we’ll set this property based on whether there actually is a lifecycle operation in progress. private let hasOperationInProgress: Bool /// Manager state that relates to individual contributors, keyed by contributors’ ``Contributor.id``. Stored separately from ``contributors`` so that the latter can be a `let`, to make it clear that the contributors remain fixed for the lifetime of the manager. @@ -263,7 +262,7 @@ internal actor RoomLifecycleManager { preconditionFailure("FAILED state change event should have a reason") } - changeStatus(to: .failed, error: reason) + changeStatus(to: .failed(error: reason)) // TODO: CHA-RL4b5 is a bit unclear about how to handle failure, and whether they can be detached concurrently (asked in https://github.com/ably/specification/pull/200/files#r1777471810) for contributor in contributors { @@ -282,7 +281,7 @@ internal actor RoomLifecycleManager { preconditionFailure("SUSPENDED state change event should have a reason") } - changeStatus(to: .suspended, error: reason) + changeStatus(to: .suspended(error: reason)) } default: break @@ -295,13 +294,12 @@ internal actor RoomLifecycleManager { #endif } - /// Updates ``current`` and ``error`` and emits a status change event. - private func changeStatus(to new: RoomLifecycle, error: ARTErrorInfo? = nil) { - logger.log(message: "Transitioning from \(current) to \(new), error \(String(describing: error))", level: .info) + /// Updates ``current`` and emits a status change event. + private func changeStatus(to new: RoomLifecycle) { + logger.log(message: "Transitioning from \(current) to \(new)", level: .info) let previous = current current = new - self.error = error - let statusChange = RoomStatusChange(current: current, previous: previous, error: error) + let statusChange = RoomStatusChange(current: current, previous: previous) emitStatusChange(statusChange) } @@ -343,14 +341,14 @@ internal actor RoomLifecycleManager { case .suspended: // CHA-RL1h2 let error = ARTErrorInfo(chatError: .attachmentFailed(feature: contributor.feature, underlyingError: contributorAttachError)) - changeStatus(to: .suspended, error: error) + changeStatus(to: .suspended(error: error)) // CHA-RL1h3 throw error case .failed: // CHA-RL1h4 let error = ARTErrorInfo(chatError: .attachmentFailed(feature: contributor.feature, underlyingError: contributorAttachError)) - changeStatus(to: .failed, error: error) + changeStatus(to: .failed(error: error)) // CHA-RL1h5 // TODO: Implement the "asynchronously with respect to CHA-RL1h4" part of CHA-RL1h5 (https://github.com/ably-labs/ably-chat-swift/issues/50) @@ -450,8 +448,8 @@ internal actor RoomLifecycleManager { } // This check is CHA-RL2h2 - if current != .failed { - changeStatus(to: .failed, error: error) + if !current.isFailed { + changeStatus(to: .failed(error: error)) } default: // CHA-RL2h3: Retry until detach succeeds, with a pause before each attempt diff --git a/Sources/AblyChat/RoomStatus.swift b/Sources/AblyChat/RoomStatus.swift index b94488ae..d152b544 100644 --- a/Sources/AblyChat/RoomStatus.swift +++ b/Sources/AblyChat/RoomStatus.swift @@ -2,33 +2,49 @@ import Ably public protocol RoomStatus: AnyObject, Sendable { 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 async } func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription } -public enum RoomLifecycle: Sendable { +public enum RoomLifecycle: Sendable, Equatable { case initialized case attaching case attached case detaching case detached - case suspended - case failed + case suspended(error: ARTErrorInfo) + case failed(error: ARTErrorInfo) 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: + // + // 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 + + public var isSuspended: Bool { + if case .suspended = self { + true + } else { + false + } + } + + public var isFailed: Bool { + if case .failed = self { + true + } else { + false + } + } } public struct RoomStatusChange: Sendable { public var current: RoomLifecycle public var previous: RoomLifecycle - // 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: RoomLifecycle, previous: RoomLifecycle) { self.current = current self.previous = previous - self.error = error } } diff --git a/Tests/AblyChatTests/Helpers/RoomLifecycle+Error.swift b/Tests/AblyChatTests/Helpers/RoomLifecycle+Error.swift new file mode 100644 index 00000000..f2f33df2 --- /dev/null +++ b/Tests/AblyChatTests/Helpers/RoomLifecycle+Error.swift @@ -0,0 +1,21 @@ +import Ably +import AblyChat + +extension RoomLifecycle { + var error: ARTErrorInfo? { + switch self { + case let .failed(error): + error + case let .suspended(error): + error + case .initialized, + .attached, + .attaching, + .detached, + .detaching, + .releasing, + .released: + nil + } + } +} diff --git a/Tests/AblyChatTests/Helpers/Subscription+RoomStatusChange.swift b/Tests/AblyChatTests/Helpers/Subscription+RoomStatusChange.swift new file mode 100644 index 00000000..2bc30bb6 --- /dev/null +++ b/Tests/AblyChatTests/Helpers/Subscription+RoomStatusChange.swift @@ -0,0 +1,34 @@ +import Ably +import AblyChat + +/// Extensions for filtering a subscription by a given case, and then providing access to the values associated with these cases. +/// +/// This provides better ergonomics than writing e.g. `failedStatusChange = await subscription.first { $0.isSuspended }`, because it means that you don’t have to write another `if case` (or equivalent) to get access to the associated value of `failedStatusChange.current`. +extension Subscription where Element == RoomStatusChange { + struct StatusChangeWithError { + /// A status change whose `current` has an associated error; ``error`` provides access to this error. + var statusChange: RoomStatusChange + /// The error associated with `statusChange.current`. + var error: ARTErrorInfo + } + + func suspendedElements() async -> AsyncCompactMapSequence, Subscription.StatusChangeWithError> { + compactMap { statusChange in + if case let .suspended(error) = statusChange.current { + StatusChangeWithError(statusChange: statusChange, error: error) + } else { + nil + } + } + } + + func failedElements() async -> AsyncCompactMapSequence, Subscription.StatusChangeWithError> { + compactMap { statusChange in + if case let .failed(error) = statusChange.current { + StatusChangeWithError(statusChange: statusChange, error: error) + } else { + nil + } + } + } +} diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index 14804d27..2cb0558b 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -79,13 +79,6 @@ struct RoomLifecycleManagerTests { #expect(await manager.current == .initialized) } - @Test - func error_startsAsNil() async { - let manager = await createManager() - - #expect(await manager.error == nil) - } - // MARK: - ATTACH operation // @spec CHA-RL1a @@ -229,7 +222,7 @@ struct RoomLifecycleManagerTests { let manager = await createManager(contributors: contributors) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) - async let maybeSuspendedStatusChange = statusChangeSubscription.first { $0.current == .suspended } + async let maybeSuspendedStatusChange = statusChangeSubscription.suspendedElements().first { _ in true } // When: `performAttachOperation()` is called on the lifecycle manager async let roomAttachResult: Void = manager.performAttachOperation() @@ -241,7 +234,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.current.isSuspended) var roomAttachError: Error? do { @@ -250,7 +243,7 @@ struct RoomLifecycleManagerTests { roomAttachError = error } - for error in await [suspendedStatusChange.error, manager.error, roomAttachError] { + for error in await [suspendedStatusChange.error, manager.current.error, roomAttachError] { #expect(isChatError(error, withCode: .messagesAttachmentFailed, cause: contributorAttachError)) } } @@ -280,7 +273,7 @@ struct RoomLifecycleManagerTests { let manager = await createManager(contributors: contributors) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) - async let maybeFailedStatusChange = statusChangeSubscription.first { $0.current == .failed } + async let maybeFailedStatusChange = statusChangeSubscription.failedElements().first { _ in true } // When: `performAttachOperation()` is called on the lifecycle manager async let roomAttachResult: Void = manager.performAttachOperation() @@ -291,7 +284,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.current.isFailed) var roomAttachError: Error? do { @@ -300,7 +293,7 @@ struct RoomLifecycleManagerTests { roomAttachError = error } - for error in await [failedStatusChange.error, manager.error, roomAttachError] { + for error in await [failedStatusChange.error, manager.current.error, roomAttachError] { #expect(isChatError(error, withCode: .messagesAttachmentFailed, cause: contributorAttachError)) } } @@ -429,7 +422,11 @@ struct RoomLifecycleManagerTests { @Test func detach_whenFailed() async throws { // Given: A RoomLifecycleManager in the FAILED state - let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .failed) + let manager = await createManager( + forTestingWhatHappensWhenCurrentlyIn: .failed( + error: .createUnknownError() /* arbitrary */ + ) + ) // When: `performAttachOperation()` is called on the lifecycle manager // Then: It throws a roomInFailedState error @@ -507,7 +504,7 @@ struct RoomLifecycleManagerTests { let manager = await createManager(contributors: contributors) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) - async let maybeFailedStatusChange = statusChangeSubscription.first { $0.current == .failed } + async let maybeFailedStatusChange = statusChangeSubscription.failedElements().first { _ in true } // When: `performDetachOperation()` is called on the lifecycle manager let maybeRoomDetachError: Error? @@ -845,7 +842,7 @@ struct RoomLifecycleManagerTests { let manager = await createManager(forTestingWhatHappensWhenHasOperationInProgress: false, contributors: contributors) let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) - async let failedStatusChange = roomStatusSubscription.first { $0.current == .failed } + async let failedStatusChange = roomStatusSubscription.failedElements().first { _ in true } // When: A contributor emits an FAILED event let contributorStateChange = ARTChannelStateChange( @@ -864,7 +861,7 @@ struct RoomLifecycleManagerTests { // - the room status transitions to failed, with the error of the status change being the `reason` of the contributor FAILED event // - and it calls `detach` on all contributors _ = try #require(await failedStatusChange) - #expect(await manager.current == .failed) + #expect(await manager.current.isFailed) for contributor in contributors { #expect(await contributor.channel.detachCallCount == 1) @@ -946,14 +943,15 @@ struct RoomLifecycleManagerTests { let manager = await createManager(forTestingWhatHappensWhenHasOperationInProgress: false, contributors: [contributor]) let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) - async let maybeSuspendedRoomStatusChange = roomStatusSubscription.first { $0.current == .suspended } + async let maybeSuspendedRoomStatusChange = roomStatusSubscription.suspendedElements().first { _ in true } // When: A contributor emits a state change to SUSPENDED + let contributorStateChangeReason = ARTErrorInfo(domain: "SomeDomain", code: 123) // arbitrary let contributorStateChange = ARTChannelStateChange( current: .suspended, previous: .attached, // arbitrary event: .suspended, - reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary + reason: contributorStateChangeReason, resumed: false // arbitrary ) @@ -961,9 +959,8 @@ struct RoomLifecycleManagerTests { // Then: The room transitions to SUSPENDED, and this state change has error equal to the contributor state change’s `reason` let suspendedRoomStatusChange = try #require(await maybeSuspendedRoomStatusChange) - #expect(suspendedRoomStatusChange.error === contributorStateChange.reason) + #expect(suspendedRoomStatusChange.error === contributorStateChangeReason) - #expect(await manager.current == .suspended) - #expect(await manager.error === contributorStateChange.reason) + #expect(await manager.current == .suspended(error: contributorStateChangeReason)) } }