diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 9c31fe91..0e5a5e6b 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -301,9 +301,10 @@ internal actor RoomLifecycleManager { /// A contributor state change is considered handled once the manager has performed all of the side effects that it will perform as a result of receiving this state change. Specifically, once: /// /// - the manager has recorded all pending discontinuity events provoked by the state change (you can retrieve these using ``testsOnly_pendingDiscontinuityEventsForContributor(at:)``) - /// - the manager has performed all status changes provoked by the state change + /// - the manager has performed all status changes provoked by the state change (this does _not_ include the case in which the state change provokes the creation of a transient disconnect timeout which subsequently provokes a status change; use ``testsOnly_subscribeToHandledTransientDisconnectTimeouts()`` to find out about those) /// - the manager has performed all contributor actions provoked by the state change, namely calls to ``RoomLifecycleContributorChannel.detach()`` or ``RoomLifecycleContributor.emitDiscontinuity(_:)`` /// - the manager has recorded all transient disconnect timeouts provoked by the state change (you can retrieve this information using ``testsOnly_hasTransientDisconnectTimeout(for:) or ``testsOnly_idOfTransientDisconnectTimeout(for:)``) + /// - the manager has performed all transient disconnect timeout cancellations provoked by the state change (you can retrieve this information using ``testsOnly_hasTransientDisconnectTimeout(for:) or ``testsOnly_idOfTransientDisconnectTimeout(for:)``) internal func testsOnly_subscribeToHandledContributorStateChanges() -> Subscription { let subscription = Subscription(bufferingPolicy: .unbounded) stateChangeHandledSubscriptions.append(subscription) @@ -318,9 +319,28 @@ internal actor RoomLifecycleManager { contributorAnnotations[contributor].hasTransientDisconnectTimeout } + internal var testsOnly_hasTransientDisconnectTimeoutForAnyContributor: Bool { + contributors.contains { testsOnly_hasTransientDisconnectTimeout(for: $0) } + } + internal func testsOnly_idOfTransientDisconnectTimeout(for contributor: Contributor) -> UUID? { contributorAnnotations[contributor].transientDisconnectTimeout?.id } + + // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36) + /// Supports the ``testsOnly_subscribeToHandledTransientDisconnectTimeouts()`` method. + private var transientDisconnectTimeoutHandledSubscriptions: [Subscription] = [] + + /// Returns a subscription which emits the IDs of the transient disconnect timeouts that have been handled by the manager. + /// + /// A transient disconnect timeout is considered handled once the manager has performed all of the side effects that it will perform as a result of creating this timeout. Specifically, once: + /// + /// - the manager has performed all status changes provoked by the completion of this timeout (which may be none, if the timeout gets cancelled) + internal func testsOnly_subscribeToHandledTransientDisconnectTimeouts() -> Subscription { + let subscription = Subscription(bufferingPolicy: .unbounded) + transientDisconnectTimeoutHandledSubscriptions.append(subscription) + return subscription + } #endif /// Implements CHA-RL4b’s contributor state change handling. @@ -364,11 +384,16 @@ internal actor RoomLifecycleManager { contributorAnnotations[contributor].pendingDiscontinuityEvents.append(reason) } - } else if status != .attached { - if await (contributors.async.map { await $0.channel.state }.allSatisfy { $0 == .attached }) { - // CHA-RL4b8 - logger.log(message: "Now that all contributors are ATTACHED, transitioning room to ATTACHED", level: .info) - changeStatus(to: .attached) + } else { + // CHA-RL4b10 + clearTransientDisconnectTimeouts(for: contributor) + + if status != .attached { + if await (contributors.async.map { await $0.channel.state }.allSatisfy { $0 == .attached }) { + // CHA-RL4b8 + logger.log(message: "Now that all contributors are ATTACHED, transitioning room to ATTACHED", level: .info) + changeStatus(to: .attached) + } } } case .failed: @@ -379,6 +404,7 @@ internal actor RoomLifecycleManager { preconditionFailure("FAILED state change event should have a reason") } + clearTransientDisconnectTimeouts() 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) @@ -398,6 +424,8 @@ internal actor RoomLifecycleManager { preconditionFailure("SUSPENDED state change event should have a reason") } + clearTransientDisconnectTimeouts() + changeStatus(to: .suspended(error: reason)) } case .attaching: @@ -411,10 +439,20 @@ internal actor RoomLifecycleManager { try await clock.sleep(timeInterval: 5) } catch { logger.log(message: "Transient disconnect timeout \(transientDisconnectTimeout.id) for \(contributor) was interrupted, error \(error)", level: .debug) + + #if DEBUG + emitTransientDisconnectTimeoutHandledEventForTimeoutWithID(transientDisconnectTimeout.id) + #endif + + return } logger.log(message: "Transient disconnect timeout \(transientDisconnectTimeout.id) for \(contributor) completed", level: .debug) contributorAnnotations[contributor].transientDisconnectTimeout = nil changeStatus(to: .attachingDueToContributorStateChange(error: stateChange.reason)) + + #if DEBUG + emitTransientDisconnectTimeoutHandledEventForTimeoutWithID(transientDisconnectTimeout.id) + #endif } } default: @@ -429,6 +467,31 @@ internal actor RoomLifecycleManager { #endif } + #if DEBUG + private func emitTransientDisconnectTimeoutHandledEventForTimeoutWithID(_ id: UUID) { + logger.log(message: "Emitting transient disconnect timeout handled event for \(id)", level: .debug) + for subscription in transientDisconnectTimeoutHandledSubscriptions { + subscription.emit(id) + } + } + #endif + + private func clearTransientDisconnectTimeouts(for contributor: Contributor) { + guard let transientDisconnectTimeout = contributorAnnotations[contributor].transientDisconnectTimeout else { + return + } + + logger.log(message: "Clearing transient disconnect timeout \(transientDisconnectTimeout.id) for \(contributor)", level: .debug) + transientDisconnectTimeout.task?.cancel() + contributorAnnotations[contributor].transientDisconnectTimeout = nil + } + + private func clearTransientDisconnectTimeouts() { + for contributor in contributors { + clearTransientDisconnectTimeouts(for: contributor) + } + } + // MARK: - Operation handling /// Whether the room lifecycle manager currently has a room lifecycle operation in progress. @@ -614,6 +677,9 @@ internal actor RoomLifecycleManager { } } + // CHA-RL1g3 + clearTransientDisconnectTimeouts() + // CHA-RL1g1 changeStatus(to: .attached) @@ -684,6 +750,7 @@ internal actor RoomLifecycleManager { } // CHA-RL2e + clearTransientDisconnectTimeouts() changeStatus(to: .detaching(detachOperationID: operationID)) // CHA-RL2f @@ -774,6 +841,7 @@ internal actor RoomLifecycleManager { } // CHA-RL3l + clearTransientDisconnectTimeouts() changeStatus(to: .releasing(releaseOperationID: operationID)) // CHA-RL3d diff --git a/Tests/AblyChatTests/Mocks/MockSimpleClock.swift b/Tests/AblyChatTests/Mocks/MockSimpleClock.swift index a070bc35..2106e40c 100644 --- a/Tests/AblyChatTests/Mocks/MockSimpleClock.swift +++ b/Tests/AblyChatTests/Mocks/MockSimpleClock.swift @@ -7,7 +7,7 @@ actor MockSimpleClock: SimpleClock { enum SleepBehavior { case success - case fromFunction(@Sendable () async -> Void) + case fromFunction(@Sendable () async throws -> Void) } init(sleepBehavior: SleepBehavior? = nil) { @@ -32,7 +32,7 @@ actor MockSimpleClock: SimpleClock { case .success: break case let .fromFunction(function): - await function() + try await function() } } } diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index 727200a6..6ba5bb31 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -32,6 +32,8 @@ struct RoomLifecycleManagerTests { private let continuation: AsyncStream.Continuation /// When this behavior is set as a ``MockSimpleClock``’s `sleepBehavior`, calling ``complete(result:)`` will cause the corresponding `sleep(timeInterval:)` to complete with the result passed to that method. + /// + /// ``sleep(timeInterval:)`` will respond to task cancellation by throwing `CancellationError`. let behavior: MockSimpleClock.SleepBehavior init() { @@ -39,7 +41,8 @@ struct RoomLifecycleManagerTests { self.continuation = continuation behavior = .fromFunction { - await (stream.first { _ in true })! + await (stream.first { _ in true }) // this will return if we yield to the continuation or if the Task is cancelled + try Task.checkCancellation() } } @@ -90,6 +93,14 @@ struct RoomLifecycleManagerTests { _ = await handledSignal } + /// Given a room lifecycle manager and the ID of a transient disconnect timeout, this method will return once the manager has performed all of the side effects that it will perform as a result of creating that timeout. You can provide a function which will be called after ``waitForManager`` has started listening for the manager’s “transient disconnect timeout handled” notifications. + func waitForManager(_ manager: RoomLifecycleManager, toHandleTransientDisconnectTimeoutWithID id: UUID, during action: () async -> Void) async { + let subscription = await manager.testsOnly_subscribeToHandledTransientDisconnectTimeouts() + async let handledSignal = subscription.first { $0 == id } + await action() + _ = await handledSignal + } + // MARK: - Initial state // @spec CHA-RS2a @@ -276,6 +287,23 @@ struct RoomLifecycleManagerTests { } } + // @spec CHA-RL1g3 + @Test + func attach_uponSuccess_clearsTransientDisconnectTimeouts() async throws { + // Given: A RoomLifecycleManager, all of whose contributors’ calls to `attach` succeed + let contributors = (1 ... 3).map { _ in createContributor(attachBehavior: .complete(.success)) } + let manager = await createManager( + forTestingWhatHappensWhenHasTransientDisconnectTimeoutForTheseContributorIDs: [contributors[1].id], + contributors: contributors + ) + + // When: `performAttachOperation()` is called on the lifecycle manager + try await manager.performAttachOperation() + + // Then: It clears all transient disconnect timeouts + #expect(await !manager.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) + } + // @spec CHA-RL1h2 // @specOneOf(1/2) CHA-RL1h1 - tests that an error gets thrown when channel attach fails due to entering SUSPENDED (TODO: but I don’t yet fully understand the meaning of CHA-RL1h1; outstanding question https://github.com/ably/specification/pull/200/files#r1765476610) // @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) @@ -511,22 +539,29 @@ 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) + // @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) let contributorDetachOperation = SignallableChannelOperation() - let manager = await createManager(contributors: [createContributor(detachBehavior: contributorDetachOperation.behavior)]) + let contributor = createContributor(detachBehavior: contributorDetachOperation.behavior) + + let manager = await createManager( + // We set a transient disconnect timeout, just so we can check that it gets cleared, as the spec point specifies + forTestingWhatHappensWhenHasTransientDisconnectTimeoutForTheseContributorIDs: [contributor.id], + contributors: [contributor] + ) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let statusChange = statusChangeSubscription.first { _ in true } // 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, its current state 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 contributorDetachOperation.complete(result: .success) @@ -720,22 +755,29 @@ struct RoomLifecycleManagerTests { #expect(await contributor.channel.detachCallCount == 1) } - // @specPartial CHA-RL3l - Haven’t implemented the part that refers to "transient disconnect timeouts"; TODO do this (https://github.com/ably-labs/ably-chat-swift/issues/48) + // @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) let contributorDetachOperation = SignallableChannelOperation() - let manager = await createManager(contributors: [createContributor(detachBehavior: contributorDetachOperation.behavior)]) + let contributor = createContributor(detachBehavior: contributorDetachOperation.behavior) + + let manager = await createManager( + // We set a transient disconnect timeout, just so we can check that it gets cleared, as the spec point specifies + forTestingWhatHappensWhenHasTransientDisconnectTimeoutForTheseContributorIDs: [contributor.id], + contributors: [contributor] + ) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let statusChange = statusChangeSubscription.first { _ in true } // 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, its current state 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 contributorDetachOperation.complete(result: .success) @@ -959,7 +1001,7 @@ struct RoomLifecycleManagerTests { #expect(pendingDiscontinuityEvent === contributorStateChange.reason) } - // @specPartial CHA-RL4b5 - Haven’t implemented the part that refers to "transient disconnect timeouts"; TODO do this (https://github.com/ably-labs/ably-chat-swift/issues/48) + // @spec CHA-RL4b5 @Test func contributorFailedEvent_withNoOperationInProgress() async throws { // Given: A RoomLifecycleManager, with no room lifecycle operation in progress @@ -967,9 +1009,15 @@ struct RoomLifecycleManagerTests { // TODO: The .success is currently arbitrary since the spec doesn’t say what to do if detach fails (have asked in https://github.com/ably/specification/pull/200#discussion_r1777471810) createContributor(detachBehavior: .success), createContributor(detachBehavior: .success), + createContributor(detachBehavior: .success), ] let manager = await createManager( forTestingWhatHappensWhenCurrentlyIn: .initialized, // case arbitrary, just care that no operation is in progress + forTestingWhatHappensWhenHasTransientDisconnectTimeoutForTheseContributorIDs: [ + // Give 2 of the 3 contributors a transient disconnect timeout, so we can test that _all_ such timeouts get cleared (as the spec point specifies), not just those for the FAILED contributor + contributors[0].id, + contributors[1].id, + ], contributors: contributors ) @@ -992,12 +1040,15 @@ struct RoomLifecycleManagerTests { // Then: // - 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 + // - it clears all transient disconnect timeouts _ = try #require(await failedStatusChange) #expect(await manager.current.isFailed) for contributor in contributors { #expect(await contributor.channel.detachCallCount == 1) } + + #expect(await !manager.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) } // @spec CHA-RL4b6 @@ -1081,6 +1132,89 @@ struct RoomLifecycleManagerTests { #expect(await !manager.testsOnly_hasTransientDisconnectTimeout(for: contributor)) } + // @specOneOf(1/2) CHA-RL4b10 + @Test + func contributorAttachedEvent_withNoOperationInProgress_clearsTransientDisconnectTimeouts() async throws { + // Given: A RoomLifecycleManager, with no room lifecycle operation in progress + let contributorThatWillEmitAttachedStateChange = createContributor() + let contributors = [ + contributorThatWillEmitAttachedStateChange, + createContributor(), + createContributor(), + ] + let manager = await createManager( + forTestingWhatHappensWhenCurrentlyIn: .initialized, // case arbitrary, just care that no operation is in progress + forTestingWhatHappensWhenHasTransientDisconnectTimeoutForTheseContributorIDs: [ + // Give 2 of the 3 contributors a transient disconnect timeout, so we can test that only the timeout for the ATTACHED contributor gets cleared, not all of them + contributorThatWillEmitAttachedStateChange.id, + contributors[1].id, + ], + contributors: contributors + ) + + // When: A contributor emits a state change to ATTACHED + let contributorAttachedStateChange = ARTChannelStateChange( + current: .attached, + previous: .attaching, // arbitrary + event: .attached, + reason: nil // arbitrary + ) + + await waitForManager(manager, toHandleContributorStateChange: contributorAttachedStateChange) { + await contributorThatWillEmitAttachedStateChange.channel.emitStateChange(contributorAttachedStateChange) + } + + // Then: The manager clears any transient disconnect timeout for that contributor + #expect(await !manager.testsOnly_hasTransientDisconnectTimeout(for: contributorThatWillEmitAttachedStateChange)) + // check the timeout for the other contributors didn’t get cleared + #expect(await manager.testsOnly_hasTransientDisconnectTimeout(for: contributors[1])) + } + + // @specOneOf(2/2) CHA-RL4b10 - This test is more elaborate than contributorAttachedEvent_withNoOperationInProgress_clearsTransientDisconnectTimeouts; instead of telling the manager to pretend that it has a transient disconnect timeout, we create a proper one by fulfilling the conditions of CHA-RL4b7, and we then fulfill the conditions of CHA-RL4b10 and check that the _side effects_ of the transient disconnect timeout (i.e. the state change) do not get performed. This is the _only_ test in which we go to these lengths to confirm that a transient disconnect timeout is truly cancelled; I think it’s enough to check it properly only once and then use simpler ways of checking it in other tests. + @Test + func contributorAttachedEvent_withNoOperationInProgress_clearsTransientDisconnectTimeouts_checkThatSideEffectsNotPerformed() async throws { + // Given: A RoomLifecycleManager, with no operation in progress, with a transient disconnect timeout + let contributor = createContributor() + let sleepOperation = SignallableSleepOperation() + let clock = MockSimpleClock(sleepBehavior: sleepOperation.behavior) + let initialManagerStatus = RoomLifecycleManager.Status.initialized // arbitrary no-operation-in-progress + let manager = await createManager( + forTestingWhatHappensWhenCurrentlyIn: initialManagerStatus, + contributors: [contributor], + clock: clock + ) + let contributorStateChange = ARTChannelStateChange( + current: .attaching, + previous: .detached, // arbitrary + event: .attaching, + reason: nil // arbitrary + ) + async let maybeClockSleepArgument = clock.sleepCallArgumentsAsyncSequence.first { _ in true } + // We create a transient disconnect timeout by fulfilling the conditions of CHA-RL4b7 + await waitForManager(manager, toHandleContributorStateChange: contributorStateChange) { + await contributor.channel.emitStateChange(contributorStateChange) + } + try #require(await maybeClockSleepArgument != nil) + + let transientDisconnectTimeoutID = try #require(await manager.testsOnly_idOfTransientDisconnectTimeout(for: contributor)) + + // When: A contributor emits a state change to ATTACHED, and we wait for the manager to inform us that any side effects that the transient disconnect timeout may cause have taken place + let contributorAttachedStateChange = ARTChannelStateChange( + current: .attached, + previous: .attaching, // arbitrary + event: .attached, + reason: nil // arbitrary + ) + + await waitForManager(manager, toHandleTransientDisconnectTimeoutWithID: transientDisconnectTimeoutID) { + await contributor.channel.emitStateChange(contributorAttachedStateChange) + } + + // 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.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) + } + // @specOneOf(1/2) CHA-RL4b8 @Test func contributorAttachedEvent_withNoOperationInProgress_roomNotAttached_allContributorsAttached() async throws { @@ -1146,14 +1280,24 @@ struct RoomLifecycleManagerTests { #expect(await manager.current == initialManagerStatus.toRoomLifecycle) } - // @specPartial CHA-RL4b9 - Haven’t implemented the part that refers to "transient disconnect timeouts"; TODO do this (https://github.com/ably-labs/ably-chat-swift/issues/48). Nor have I implemented "the room enters the RETRY loop"; TODO do this (https://github.com/ably-labs/ably-chat-swift/issues/51) + // @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) @Test func contributorSuspendedEvent_withNoOperationInProgress() async throws { // Given: A RoomLifecycleManager with no lifecycle operation in progress - let contributor = createContributor() + let contributorThatWillEmitStateChange = createContributor() + let contributors = [ + contributorThatWillEmitStateChange, + createContributor(), + createContributor(), + ] let manager = await createManager( forTestingWhatHappensWhenCurrentlyIn: .initialized, // case arbitrary, just care that no operation is in progress - contributors: [contributor] + // Give 2 of the 3 contributors a transient disconnect timeout, so we can test that _all_ such timeouts get cleared (as the spec point specifies), not just those for the SUSPENDED contributor + forTestingWhatHappensWhenHasTransientDisconnectTimeoutForTheseContributorIDs: [ + contributorThatWillEmitStateChange.id, + contributors[1].id, + ], + contributors: [contributorThatWillEmitStateChange] ) let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) @@ -1169,12 +1313,18 @@ struct RoomLifecycleManagerTests { resumed: false // arbitrary ) - await contributor.channel.emitStateChange(contributorStateChange) + await waitForManager(manager, toHandleContributorStateChange: contributorStateChange) { + await contributorThatWillEmitStateChange.channel.emitStateChange(contributorStateChange) + } - // Then: The room transitions to SUSPENDED, and this state change has error equal to the contributor state change’s `reason` + // Then: + // - The room transitions to SUSPENDED, and this state 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) #expect(await manager.current == .suspended(error: contributorStateChangeReason)) + + #expect(await !manager.testsOnly_hasTransientDisconnectTimeoutForAnyContributor) } }