From 09f07e6ac8de8d40ba38f2a9dd5b516888388855 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 1 Oct 2024 15:58:49 -0300 Subject: [PATCH 1/5] Fix comment Mistake in 25e5052. --- Sources/AblyChat/RoomLifecycleManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 0bc540a..ff64859 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -2,7 +2,7 @@ import Ably /// The interface that the lifecycle manager expects its contributing realtime channels to conform to. /// -/// We use this instead of the ``RealtimeChannel`` interface as its ``attach`` and ``detach`` methods are `async` instead of using callbacks. This makes it easier to write mocks for (since ``RealtimeChannel`` doesn’t express to the type system that the callbacks it receives need to be `Sendable`, it’s hard to, for example, create a mock that creates a `Task` and then calls the callback from inside this task). +/// We use this instead of the ``RealtimeChannelProtocol`` interface as its ``attach`` and ``detach`` methods are `async` instead of using callbacks. This makes it easier to write mocks for (since ``RealtimeChannelProtocol`` doesn’t express to the type system that the callbacks it receives need to be `Sendable`, it’s hard to, for example, create a mock that creates a `Task` and then calls the callback from inside this task). /// /// We choose to also mark the channel’s mutable state as `async`. This is a way of highlighting at the call site of accessing this state that, since `ARTRealtimeChannel` mutates this state on a separate thread, it’s possible for this state to have changed since the last time you checked it, or since the last time you performed an operation that might have mutated it, or since the last time you recieved an event informing you that it changed. To be clear, marking these as `async` doesn’t _solve_ these issues; it just makes them a bit more visible. We’ll decide how to address them in https://github.com/ably-labs/ably-chat-swift/issues/49. internal protocol RoomLifecycleContributorChannel: Sendable { From e3342c592c4d652733b467ac4d97e704fbf1b8ca Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 26 Sep 2024 15:44:10 -0300 Subject: [PATCH 2/5] Make RoomLifecycleManager initializer async MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In an upcoming commit I’ll need to perform some async work (subscribing to contributor state changes) which needs to complete before the manager can be used. --- Sources/AblyChat/RoomLifecycleManager.swift | 10 ++-- .../RoomLifecycleManagerTests.swift | 54 +++++++++---------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index ff64859..845de19 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -33,8 +33,8 @@ internal actor RoomLifecycleManager { contributors: [Contributor], logger: InternalLogger, clock: SimpleClock - ) { - self.init( + ) async { + await self.init( current: nil, contributors: contributors, logger: logger, @@ -48,8 +48,8 @@ internal actor RoomLifecycleManager { contributors: [Contributor], logger: InternalLogger, clock: SimpleClock - ) { - self.init( + ) async { + await self.init( current: current, contributors: contributors, logger: logger, @@ -63,7 +63,7 @@ internal actor RoomLifecycleManager { contributors: [Contributor], logger: InternalLogger, clock: SimpleClock - ) { + ) async { self.current = current ?? .initialized self.contributors = contributors self.logger = logger diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index 6ac1e54..e69f711 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -31,8 +31,8 @@ struct RoomLifecycleManagerTests { forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle? = nil, contributors: [RoomLifecycleManager.Contributor] = [], clock: SimpleClock = MockSimpleClock() - ) -> RoomLifecycleManager { - .init( + ) async -> RoomLifecycleManager { + await .init( testsOnly_current: current, contributors: contributors, logger: TestLogger(), @@ -62,14 +62,14 @@ struct RoomLifecycleManagerTests { // @spec CHA-RS3 @Test func current_startsAsInitialized() async { - let manager = createManager() + let manager = await createManager() #expect(await manager.current == .initialized) } @Test func error_startsAsNil() async { - let manager = createManager() + let manager = await createManager() #expect(await manager.error == nil) } @@ -81,7 +81,7 @@ struct RoomLifecycleManagerTests { func attach_whenAlreadyAttached() async throws { // Given: A RoomLifecycleManager in the ATTACHED state let contributor = createContributor() - let manager = createManager(forTestingWhatHappensWhenCurrentlyIn: .attached, contributors: [contributor]) + let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .attached, contributors: [contributor]) // When: `performAttachOperation()` is called on the lifecycle manager try await manager.performAttachOperation() @@ -94,7 +94,7 @@ struct RoomLifecycleManagerTests { @Test func attach_whenReleasing() async throws { // Given: A RoomLifecycleManager in the RELEASING state - let manager = createManager(forTestingWhatHappensWhenCurrentlyIn: .releasing) + let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .releasing) // When: `performAttachOperation()` is called on the lifecycle manager // Then: It throws a roomIsReleasing error @@ -109,7 +109,7 @@ struct RoomLifecycleManagerTests { @Test func attach_whenReleased() async throws { // Given: A RoomLifecycleManager in the RELEASED state - let manager = createManager(forTestingWhatHappensWhenCurrentlyIn: .released) + let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .released) // When: `performAttachOperation()` is called on the lifecycle manager // Then: It throws a roomIsReleased error @@ -126,7 +126,7 @@ struct RoomLifecycleManagerTests { // 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) let contributorAttachOperation = SignallableChannelOperation() - let manager = createManager(contributors: [createContributor(attachBehavior: contributorAttachOperation.behavior)]) + let manager = await createManager(contributors: [createContributor(attachBehavior: contributorAttachOperation.behavior)]) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let statusChange = statusChangeSubscription.first { _ in true } @@ -148,7 +148,7 @@ struct RoomLifecycleManagerTests { func attach_attachesAllContributors_andWhenTheyAllAttachSuccessfully_transitionsToAttached() 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 = createManager(contributors: contributors) + let manager = await createManager(contributors: contributors) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let attachedStatusChange = statusChangeSubscription.first { $0.current == .attached } @@ -180,7 +180,7 @@ struct RoomLifecycleManagerTests { } } - let manager = createManager(contributors: contributors) + let manager = await createManager(contributors: contributors) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let maybeSuspendedStatusChange = statusChangeSubscription.first { $0.current == .suspended } @@ -231,7 +231,7 @@ struct RoomLifecycleManagerTests { } } - let manager = createManager(contributors: contributors) + let manager = await createManager(contributors: contributors) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let maybeFailedStatusChange = statusChangeSubscription.first { $0.current == .failed } @@ -282,7 +282,7 @@ struct RoomLifecycleManagerTests { ), ] - let manager = createManager(contributors: contributors) + let manager = await createManager(contributors: contributors) // When: `performAttachOperation()` is called on the lifecycle manager try? await manager.performAttachOperation() @@ -324,7 +324,7 @@ struct RoomLifecycleManagerTests { ), ] - let manager = createManager(contributors: contributors) + let manager = await createManager(contributors: contributors) // When: `performAttachOperation()` is called on the lifecycle manager try? await manager.performAttachOperation() @@ -340,7 +340,7 @@ struct RoomLifecycleManagerTests { func detach_whenAlreadyDetached() async throws { // Given: A RoomLifecycleManager in the DETACHED state let contributor = createContributor() - let manager = createManager(forTestingWhatHappensWhenCurrentlyIn: .detached, contributors: [contributor]) + let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .detached, contributors: [contributor]) // When: `performDetachOperation()` is called on the lifecycle manager try await manager.performDetachOperation() @@ -353,7 +353,7 @@ struct RoomLifecycleManagerTests { @Test func detach_whenReleasing() async throws { // Given: A RoomLifecycleManager in the RELEASING state - let manager = createManager(forTestingWhatHappensWhenCurrentlyIn: .releasing) + let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .releasing) // When: `performDetachOperation()` is called on the lifecycle manager // Then: It throws a roomIsReleasing error @@ -368,7 +368,7 @@ struct RoomLifecycleManagerTests { @Test func detach_whenReleased() async throws { // Given: A RoomLifecycleManager in the RELEASED state - let manager = createManager(forTestingWhatHappensWhenCurrentlyIn: .released) + let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .released) // When: `performAttachOperation()` is called on the lifecycle manager // Then: It throws a roomIsReleased error @@ -383,7 +383,7 @@ struct RoomLifecycleManagerTests { @Test func detach_whenFailed() async throws { // Given: A RoomLifecycleManager in the FAILED state - let manager = createManager(forTestingWhatHappensWhenCurrentlyIn: .failed) + let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .failed) // When: `performAttachOperation()` is called on the lifecycle manager // Then: It throws a roomInFailedState error @@ -400,7 +400,7 @@ struct RoomLifecycleManagerTests { // 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 = createManager(contributors: [createContributor(detachBehavior: contributorDetachOperation.behavior)]) + let manager = await createManager(contributors: [createContributor(detachBehavior: contributorDetachOperation.behavior)]) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let statusChange = statusChangeSubscription.first { _ in true } @@ -421,7 +421,7 @@ struct RoomLifecycleManagerTests { func detach_detachesAllContributors_andWhenTheyAllDetachSuccessfully_transitionsToDetached() async throws { // Given: A RoomLifecycleManager, all of whose contributors’ calls to `detach` succeed let contributors = (1 ... 3).map { _ in createContributor(detachBehavior: .complete(.success)) } - let manager = createManager(contributors: contributors) + let manager = await createManager(contributors: contributors) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let detachedStatusChange = statusChangeSubscription.first { $0.current == .detached } @@ -458,7 +458,7 @@ struct RoomLifecycleManagerTests { createContributor(feature: .typing, detachBehavior: .success), ] - let manager = createManager(contributors: contributors) + let manager = await createManager(contributors: contributors) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let maybeFailedStatusChange = statusChangeSubscription.first { $0.current == .failed } @@ -505,7 +505,7 @@ struct RoomLifecycleManagerTests { let contributor = createContributor(initialState: .attached, detachBehavior: .fromFunction(detachImpl)) let clock = MockSimpleClock() - let manager = createManager(contributors: [contributor], clock: clock) + let manager = await createManager(contributors: [contributor], clock: clock) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let asyncLetStatusChanges = Array(statusChangeSubscription.prefix(2)) @@ -529,7 +529,7 @@ struct RoomLifecycleManagerTests { func release_whenAlreadyReleased() async { // Given: A RoomLifecycleManager in the RELEASED state let contributor = createContributor() - let manager = createManager(forTestingWhatHappensWhenCurrentlyIn: .released, contributors: [contributor]) + let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .released, contributors: [contributor]) // When: `performReleaseOperation()` is called on the lifecycle manager await manager.performReleaseOperation() @@ -543,7 +543,7 @@ struct RoomLifecycleManagerTests { func release_whenDetached() async throws { // Given: A RoomLifecycleManager in the DETACHED state let contributor = createContributor() - let manager = createManager(forTestingWhatHappensWhenCurrentlyIn: .detached, contributors: [contributor]) + let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .detached, contributors: [contributor]) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let statusChange = statusChangeSubscription.first { _ in true } @@ -563,7 +563,7 @@ struct RoomLifecycleManagerTests { // 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 = createManager(contributors: [createContributor(detachBehavior: contributorDetachOperation.behavior)]) + let manager = await createManager(contributors: [createContributor(detachBehavior: contributorDetachOperation.behavior)]) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let statusChange = statusChangeSubscription.first { _ in true } @@ -593,7 +593,7 @@ struct RoomLifecycleManagerTests { createContributor(initialState: .detached /* arbitrary non-FAILED */, detachBehavior: .complete(.success)), ] - let manager = createManager(contributors: contributors) + let manager = await createManager(contributors: contributors) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let releasedStatusChange = statusChangeSubscription.first { $0.current == .released } @@ -633,7 +633,7 @@ struct RoomLifecycleManagerTests { let clock = MockSimpleClock() - let manager = createManager(contributors: [contributor], clock: clock) + let manager = await createManager(contributors: [contributor], clock: clock) // Then: When `performReleaseOperation()` is called on the manager await manager.performReleaseOperation() @@ -653,7 +653,7 @@ struct RoomLifecycleManagerTests { let clock = MockSimpleClock() - let manager = createManager(contributors: [contributor], clock: clock) + let manager = await createManager(contributors: [contributor], clock: clock) let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded) async let releasedStatusChange = statusChangeSubscription.first { $0.current == .released } From cba991da07d86f4e8ab61b827df07fd674b2d80e Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 1 Oct 2024 11:17:03 -0300 Subject: [PATCH 3/5] Change room lifecycle contributor into a protocol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For an upcoming feature, I’m going to want to add methods to the contributor itself, and hence want to be able to mock it. --- Sources/AblyChat/RoomLifecycleManager.swift | 15 ++++++++------- .../Mocks/MockRoomLifecycleContributor.swift | 11 +++++++++++ .../AblyChatTests/RoomLifecycleManagerTests.swift | 6 +++--- 3 files changed, 22 insertions(+), 10 deletions(-) create mode 100644 Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 845de19..84c656e 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -13,15 +13,16 @@ internal protocol RoomLifecycleContributorChannel: Sendable { var errorReason: ARTErrorInfo? { get async } } -internal actor RoomLifecycleManager { - /// A realtime channel that contributes to the room lifecycle. - internal struct Contributor { - /// The room feature that this contributor corresponds to. Used only for choosing which error to throw when a contributor operation fails. - internal var feature: RoomFeature +/// A realtime channel that contributes to the room lifecycle. +internal protocol RoomLifecycleContributor: Sendable { + associatedtype Channel: RoomLifecycleContributorChannel - internal var channel: Channel - } + /// The room feature that this contributor corresponds to. Used only for choosing which error to throw when a contributor operation fails. + var feature: RoomFeature { get } + var channel: Channel { get } +} +internal actor RoomLifecycleManager { internal private(set) var current: RoomLifecycle internal private(set) var error: ARTErrorInfo? diff --git a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift new file mode 100644 index 0000000..d1a0d2a --- /dev/null +++ b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift @@ -0,0 +1,11 @@ +@testable import AblyChat + +actor MockRoomLifecycleContributor: RoomLifecycleContributor { + nonisolated let feature: RoomFeature + nonisolated let channel: MockRoomLifecycleContributorChannel + + init(feature: RoomFeature, channel: MockRoomLifecycleContributorChannel) { + self.feature = feature + self.channel = channel + } +} diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index e69f711..e2c477b 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -29,9 +29,9 @@ struct RoomLifecycleManagerTests { private func createManager( forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle? = nil, - contributors: [RoomLifecycleManager.Contributor] = [], + contributors: [MockRoomLifecycleContributor] = [], clock: SimpleClock = MockSimpleClock() - ) async -> RoomLifecycleManager { + ) async -> RoomLifecycleManager { await .init( testsOnly_current: current, contributors: contributors, @@ -45,7 +45,7 @@ struct RoomLifecycleManagerTests { feature: RoomFeature = .messages, // Arbitrarily chosen, its value only matters in test cases where we check which error is thrown attachBehavior: MockRoomLifecycleContributorChannel.AttachOrDetachBehavior? = nil, detachBehavior: MockRoomLifecycleContributorChannel.AttachOrDetachBehavior? = nil - ) -> RoomLifecycleManager.Contributor { + ) -> MockRoomLifecycleContributor { .init( feature: feature, channel: .init( From b6c3ddb775ec51ab1de78eb9b3b59072613edd93 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 24 Sep 2024 16:55:07 -0300 Subject: [PATCH 4/5] Implement (some of) Room Lifecycle Monitoring spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements points CHA-RL4* from same spec as referenced in e70ee44. In addition to the TODOs added in the code (all of which refer either to existing GitHub issues or questions on the spec, for which we have #66 as a catch-all issue), I’ve also not done: - CHA-RL4a2 — I don’t understand the meaning of “has not yet successfully managed to attach its Realtime Channel”, asked about it in [1] - CHA-RL4b2 — seems redundant, asked about it in [2] - CHA-RL4b3, CHA-RL4b4 — seem redundant, asked about it in [3] - CHA-RL4b5, CHA-RL4b6, CHA-RL4b7 — these relate to transient disconnect timeouts, so will do them in #48 Something which I didn’t think about in 25e5052, and which I haven’t thought about here, is how actor reentrancy (i.e. the fact that when an actor-isolated method suspends via `await`, another actor-isolated method can be scheduled instead, which can potentially cause issues like state updates being interleaved in unexpected ways) might affect the room lifecycle manager. I would like to first of all implement the whole thing, specifically all of the spec points that might provide us with a mutual exclusion mechanism (i.e. the ones that tell us to wait until the current operation is complete), before assessing the situation. Have created #75 for this. Created [4] to address the `@preconcurrency import Ably` introduced by this commit. Aside: I have not been consistent with the way that I’ve named the tests; the existing lifecycle manager test names have a part that describes the expected side effects. But I haven’t done that here because some of the spec points tested here have multiple side effects and the test names would become really long and hard to read. So for those ones I’ve only described the expected side effects inside the tests. I think we can live with the inconsistency for now. Part of #53. [1] https://github.com/ably/specification/pull/200/files#r1775552624 [2] https://github.com/ably/specification/pull/200/files#r1777212960 [3] https://github.com/ably/specification/pull/200/files#r1777365677 [4] https://github.com/ably/ably-cocoa/issues/1987 --- Package.swift | 4 + Sources/AblyChat/RoomLifecycleManager.swift | 217 ++++++++++++++- .../Mocks/MockRoomLifecycleContributor.swift | 7 + .../MockRoomLifecycleContributorChannel.swift | 16 +- .../RoomLifecycleManagerTests.swift | 257 +++++++++++++++++- 5 files changed, 496 insertions(+), 5 deletions(-) diff --git a/Package.swift b/Package.swift index 14f5e28..74824c3 100644 --- a/Package.swift +++ b/Package.swift @@ -40,6 +40,10 @@ let package = Package( name: "Ably", package: "ably-cocoa" ), + .product( + name: "AsyncAlgorithms", + package: "swift-async-algorithms" + ), ] ), .testTarget( diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 84c656e..4d3ae5c 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -1,8 +1,14 @@ -import Ably +@preconcurrency import Ably +import AsyncAlgorithms /// The interface that the lifecycle manager expects its contributing realtime channels to conform to. /// -/// We use this instead of the ``RealtimeChannelProtocol`` interface as its ``attach`` and ``detach`` methods are `async` instead of using callbacks. This makes it easier to write mocks for (since ``RealtimeChannelProtocol`` doesn’t express to the type system that the callbacks it receives need to be `Sendable`, it’s hard to, for example, create a mock that creates a `Task` and then calls the callback from inside this task). +/// We use this instead of the ``RealtimeChannelProtocol`` interface as: +/// +/// - its ``attach`` and ``detach`` methods are `async` instead of using callbacks +/// - it uses `AsyncSequence` to emit state changes instead of using callbacks +/// +/// This makes it easier to write mocks for (since ``RealtimeChannelProtocol`` doesn’t express to the type system that the callbacks it receives need to be `Sendable`, it’s hard to, for example, create a mock that creates a `Task` and then calls the callback from inside this task). /// /// We choose to also mark the channel’s mutable state as `async`. This is a way of highlighting at the call site of accessing this state that, since `ARTRealtimeChannel` mutates this state on a separate thread, it’s possible for this state to have changed since the last time you checked it, or since the last time you performed an operation that might have mutated it, or since the last time you recieved an event informing you that it changed. To be clear, marking these as `async` doesn’t _solve_ these issues; it just makes them a bit more visible. We’ll decide how to address them in https://github.com/ably-labs/ably-chat-swift/issues/49. internal protocol RoomLifecycleContributorChannel: Sendable { @@ -11,24 +17,80 @@ internal protocol RoomLifecycleContributorChannel: Sendable { var state: ARTRealtimeChannelState { get async } var errorReason: ARTErrorInfo? { get async } + + /// Equivalent to subscribing to a `RealtimeChannelProtocol` object’s state changes via its `on(_:)` method. The subscription should use the ``BufferingPolicy.unbounded`` buffering policy. + /// + /// It is marked as `async` purely to make it easier to write mocks for this method (i.e. to use an actor as a mock). + func subscribeToState() async -> Subscription } /// A realtime channel that contributes to the room lifecycle. -internal protocol RoomLifecycleContributor: Sendable { +/// +/// The identity implied by the `Identifiable` conformance must distinguish each of the contributors passed to a given ``RoomLifecycleManager`` instance. +internal protocol RoomLifecycleContributor: Identifiable, Sendable { associatedtype Channel: RoomLifecycleContributorChannel /// The room feature that this contributor corresponds to. Used only for choosing which error to throw when a contributor operation fails. var feature: RoomFeature { get } var channel: Channel { get } + + /// Informs the contributor that there has been a break in channel continuity, which it should inform library users about. + /// + /// It is marked as `async` purely to make it easier to write mocks for this method (i.e. to use an actor as a mock). + func emitDiscontinuity(_ error: ARTErrorInfo) async } internal actor RoomLifecycleManager { + /// Stores manager state relating to a given contributor. + private struct ContributorAnnotation { + // TODO: Not clear whether there can be multiple or just one (asked in https://github.com/ably/specification/pull/200/files#r1781927850) + var pendingDiscontinuityEvents: [ARTErrorInfo] = [] + } + 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. + private var contributorAnnotations: ContributorAnnotations + + /// Provides a `Dictionary`-like interface for storing manager state about individual contributors. + private struct ContributorAnnotations { + private var storage: [Contributor.ID: ContributorAnnotation] + + init(contributors: [Contributor]) { + storage = contributors.reduce(into: [:]) { result, contributor in + result[contributor.id] = .init() + } + } + + /// It is a programmer error to call this subscript getter with a contributor that was not one of those passed to ``init(contributors:pendingDiscontinuityEvents)``. + subscript(_ contributor: Contributor) -> ContributorAnnotation { + get { + guard let annotation = storage[contributor.id] else { + preconditionFailure("Expected annotation for \(contributor)") + } + return annotation + } + + set { + storage[contributor.id] = newValue + } + } + + mutating func clearPendingDiscontinuityEvents() { + storage = storage.mapValues { annotation in + var newAnnotation = annotation + newAnnotation.pendingDiscontinuityEvents = [] + return newAnnotation + } + } + } private let logger: InternalLogger private let clock: SimpleClock private let contributors: [Contributor] + private var listenForStateChangesTask: Task! internal init( contributors: [Contributor], @@ -37,6 +99,7 @@ internal actor RoomLifecycleManager { ) async { await self.init( current: nil, + hasOperationInProgress: nil, contributors: contributors, logger: logger, clock: clock @@ -46,12 +109,14 @@ internal actor RoomLifecycleManager { #if DEBUG internal init( testsOnly_current current: RoomLifecycle? = nil, + testsOnly_hasOperationInProgress hasOperationInProgress: Bool? = nil, contributors: [Contributor], logger: InternalLogger, clock: SimpleClock ) async { await self.init( current: current, + hasOperationInProgress: hasOperationInProgress, contributors: contributors, logger: logger, clock: clock @@ -61,16 +126,55 @@ internal actor RoomLifecycleManager { private init( current: RoomLifecycle?, + hasOperationInProgress: Bool?, contributors: [Contributor], logger: InternalLogger, clock: SimpleClock ) async { self.current = current ?? .initialized + self.hasOperationInProgress = hasOperationInProgress ?? false self.contributors = contributors + contributorAnnotations = .init(contributors: contributors) self.logger = logger self.clock = clock + + // The idea here is to make sure that, before the initializer completes, we are already listening for state changes, so that e.g. tests don’t miss a state change. + let subscriptions = await withTaskGroup(of: (contributor: Contributor, subscription: Subscription).self) { group in + for contributor in contributors { + group.addTask { + await (contributor: contributor, subscription: contributor.channel.subscribeToState()) + } + } + + return await Array(group) + } + + // CHA-RL4: listen for state changes from our contributors + // TODO: Understand what happens when this task gets cancelled by `deinit`; I’m not convinced that the for-await loops will exit (https://github.com/ably-labs/ably-chat-swift/issues/29) + listenForStateChangesTask = Task { + await withTaskGroup(of: Void.self) { group in + for (contributor, subscription) in subscriptions { + // This `@Sendable` is to make the compiler error "'self'-isolated value of type '() async -> Void' passed as a strongly transferred parameter; later accesses could race" go away. I don’t hugely understand what it means, but given the "'self'-isolated value" I guessed it was something vaguely to do with the fact that `async` actor initializers are actor-isolated and thought that marking it as `@Sendable` would sever this isolation and make the error go away, which it did 🤷. But there are almost certainly consequences that I am incapable of reasoning about with my current level of Swift concurrency knowledge. + group.addTask { @Sendable [weak self] in + for await stateChange in subscription { + await self?.didReceiveStateChange(stateChange, forContributor: contributor) + } + } + } + } + } + } + + deinit { + listenForStateChangesTask.cancel() } + #if DEBUG + internal func testsOnly_pendingDiscontinuityEvents(for contributor: Contributor) -> [ARTErrorInfo] { + contributorAnnotations[contributor].pendingDiscontinuityEvents + } + #endif + // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36) private var subscriptions: [Subscription] = [] @@ -80,6 +184,113 @@ internal actor RoomLifecycleManager { return subscription } + #if DEBUG + // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36) + /// Supports the ``testsOnly_subscribeToHandledContributorStateChanges()`` method. + private var stateChangeHandledSubscriptions: [Subscription] = [] + + /// Returns a subscription which emits the contributor state changes that have been handled by the manager. + /// + /// 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 contributor actions provoked by the state change, namely calls to ``RoomLifecycleContributorChannel.detach()`` or ``RoomLifecycleContributor.emitDiscontinuity(_:)`` + internal func testsOnly_subscribeToHandledContributorStateChanges() -> Subscription { + let subscription = Subscription(bufferingPolicy: .unbounded) + stateChangeHandledSubscriptions.append(subscription) + return subscription + } + #endif + + /// Implements CHA-RL4b’s contributor state change handling. + private func didReceiveStateChange(_ stateChange: ARTChannelStateChange, forContributor contributor: Contributor) async { + logger.log(message: "Got state change \(stateChange) for contributor \(contributor)", level: .info) + + // TODO: The spec, which is written for a single-threaded environment, is presumably operating on the assumption that the channel is currently in the state given by `stateChange.current` (https://github.com/ably-labs/ably-chat-swift/issues/49) + switch stateChange.event { + case .update: + // CHA-RL4a1 — if RESUMED then no-op + guard !stateChange.resumed else { + break + } + + guard let reason = stateChange.reason else { + // TODO: Decide the right thing to do here (https://github.com/ably-labs/ably-chat-swift/issues/74) + preconditionFailure("State change event with resumed == false should have a reason") + } + + if hasOperationInProgress { + // CHA-RL4a3 + logger.log(message: "Recording pending discontinuity event for contributor \(contributor)", level: .info) + + contributorAnnotations[contributor].pendingDiscontinuityEvents.append(reason) + } else { + // CHA-RL4a4 + logger.log(message: "Emitting discontinuity event for contributor \(contributor)", level: .info) + + await contributor.emitDiscontinuity(reason) + } + case .attached: + if hasOperationInProgress { + if !stateChange.resumed { + // CHA-RL4b1 + logger.log(message: "Recording pending discontinuity event for contributor \(contributor)", level: .info) + + guard let reason = stateChange.reason else { + // TODO: Decide the right thing to do here (https://github.com/ably-labs/ably-chat-swift/issues/74) + preconditionFailure("State change event with resumed == false should have a reason") + } + + contributorAnnotations[contributor].pendingDiscontinuityEvents.append(reason) + } + } else if current != .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: + if !hasOperationInProgress { + // CHA-RL4b5 + guard let reason = stateChange.reason else { + // TODO: Decide the right thing to do here (https://github.com/ably-labs/ably-chat-swift/issues/74) + preconditionFailure("FAILED state change event should have a 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 { + do { + try await contributor.channel.detach() + } catch { + logger.log(message: "Failed to detach contributor \(contributor), error \(error)", level: .info) + } + } + } + case .suspended: + if !hasOperationInProgress { + // CHA-RL4b9 + guard let reason = stateChange.reason else { + // TODO: Decide the right thing to do here (https://github.com/ably-labs/ably-chat-swift/issues/74) + preconditionFailure("SUSPENDED state change event should have a reason") + } + + changeStatus(to: .suspended, error: reason) + } + default: + break + } + + #if DEBUG + for subscription in stateChangeHandledSubscriptions { + subscription.emit(stateChange) + } + #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) diff --git a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift index d1a0d2a..e008fd2 100644 --- a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift +++ b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift @@ -1,11 +1,18 @@ +import Ably @testable import AblyChat actor MockRoomLifecycleContributor: RoomLifecycleContributor { nonisolated let feature: RoomFeature nonisolated let channel: MockRoomLifecycleContributorChannel + private(set) var emitDiscontinuityArguments: [ARTErrorInfo] = [] + init(feature: RoomFeature, channel: MockRoomLifecycleContributorChannel) { self.feature = feature self.channel = channel } + + func emitDiscontinuity(_ error: ARTErrorInfo) async { + emitDiscontinuityArguments.append(error) + } } diff --git a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift index d23539a..3ef81e5 100644 --- a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift +++ b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift @@ -1,4 +1,4 @@ -import Ably +@preconcurrency import Ably @testable import AblyChat final actor MockRoomLifecycleContributorChannel: RoomLifecycleContributorChannel { @@ -7,6 +7,8 @@ final actor MockRoomLifecycleContributorChannel: RoomLifecycleContributorChannel var state: ARTRealtimeChannelState var errorReason: ARTErrorInfo? + // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36) + private var subscriptions: [Subscription] = [] private(set) var attachCallCount = 0 private(set) var detachCallCount = 0 @@ -92,4 +94,16 @@ final actor MockRoomLifecycleContributorChannel: RoomLifecycleContributorChannel throw error } } + + func subscribeToState() -> Subscription { + let subscription = Subscription(bufferingPolicy: .unbounded) + subscriptions.append(subscription) + return subscription + } + + func emitStateChange(_ stateChange: ARTChannelStateChange) { + for subscription in subscriptions { + subscription.emit(stateChange) + } + } } diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index e2c477b..c7f93c2 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -1,4 +1,4 @@ -import Ably +@preconcurrency import Ably @testable import AblyChat import Testing @@ -29,11 +29,13 @@ struct RoomLifecycleManagerTests { private func createManager( forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle? = nil, + forTestingWhatHappensWhenHasOperationInProgress hasOperationInProgress: Bool? = nil, contributors: [MockRoomLifecycleContributor] = [], clock: SimpleClock = MockSimpleClock() ) async -> RoomLifecycleManager { await .init( testsOnly_current: current, + testsOnly_hasOperationInProgress: hasOperationInProgress, contributors: contributors, logger: TestLogger(), clock: clock @@ -56,6 +58,14 @@ struct RoomLifecycleManagerTests { ) } + /// Given a room lifecycle manager and a channel state change, this method will return once the manager has performed all of the side effects that it will perform as a result of receiving this state change. You can provide a function which will be called after ``waitForManager`` has started listening for the manager’s “state change handled” notifications. + func waitForManager(_ manager: RoomLifecycleManager, toHandleContributorStateChange stateChange: ARTChannelStateChange, during action: () async -> Void) async { + let subscription = await manager.testsOnly_subscribeToHandledContributorStateChanges() + async let handledSignal = subscription.first { $0 === stateChange } + await action() + _ = await handledSignal + } + // MARK: - Initial state // @spec CHA-RS2a @@ -675,4 +685,249 @@ struct RoomLifecycleManagerTests { #expect(await manager.current == .released) } + + // MARK: - Handling contributor UPDATE events + + // @spec CHA-RL4a1 + @Test + func contributorUpdate_withResumedTrue_doesNothing() async throws { + // Given: A RoomLifecycleManager + let contributor = createContributor() + let manager = await createManager(contributors: [contributor]) + + // When: A contributor emits an UPDATE event with `resumed` flag set to true + let contributorStateChange = ARTChannelStateChange( + current: .attached, // arbitrary + previous: .attached, // arbitrary + event: .update, + reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary + resumed: true + ) + + await waitForManager(manager, toHandleContributorStateChange: contributorStateChange) { + await contributor.channel.emitStateChange(contributorStateChange) + } + + // Then: The manager does not record a pending discontinuity event for this contributor, nor does it call `emitDiscontinuity` on the contributor (this is my interpretation of "no action should be taken" in CHA-RL4a1; i.e. that the actions described in CHA-RL4a2 and CHA-RL4a3 shouldn’t happen) (TODO: get clarification; have asked in https://github.com/ably/specification/pull/200#discussion_r1777385499) + #expect(await manager.testsOnly_pendingDiscontinuityEvents(for: contributor).isEmpty) + #expect(await contributor.emitDiscontinuityArguments.isEmpty) + } + + // @spec CHA-RL4a3 + @Test + func contributorUpdate_withResumedFalse_withOperationInProgress_recordsPendingDiscontinuityEvent() async throws { + // Given: A RoomLifecycleManager, with a room lifecycle operation in progress + let contributor = createContributor() + let manager = await createManager(forTestingWhatHappensWhenHasOperationInProgress: true, contributors: [contributor]) + + // When: A contributor emits an UPDATE event with `resumed` flag set to false + let contributorStateChange = ARTChannelStateChange( + current: .attached, // arbitrary + previous: .attached, // arbitrary + event: .update, + reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary + resumed: false + ) + + await waitForManager(manager, toHandleContributorStateChange: contributorStateChange) { + await contributor.channel.emitStateChange(contributorStateChange) + } + + // Then: The manager records a pending discontinuity event for this contributor, and this discontinuity event has error equal to the contributor UPDATE event’s `reason` + let pendingDiscontinuityEvents = await manager.testsOnly_pendingDiscontinuityEvents(for: contributor) + try #require(pendingDiscontinuityEvents.count == 1) + + let pendingDiscontinuityEvent = pendingDiscontinuityEvents[0] + #expect(pendingDiscontinuityEvent === contributorStateChange.reason) + } + + // @spec CHA-RL4a4 + @Test + func contributorUpdate_withResumedTrue_withNoOperationInProgress_emitsDiscontinuityEvent() async throws { + // Given: A RoomLifecycleManager, with no room lifecycle operation in progress + let contributor = createContributor() + let manager = await createManager(forTestingWhatHappensWhenHasOperationInProgress: false, contributors: [contributor]) + + // When: A contributor emits an UPDATE event with `resumed` flag set to false + let contributorStateChange = ARTChannelStateChange( + current: .attached, // arbitrary + previous: .attached, // arbitrary + event: .update, + reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary + resumed: false + ) + + await waitForManager(manager, toHandleContributorStateChange: contributorStateChange) { + await contributor.channel.emitStateChange(contributorStateChange) + } + + // Then: The manager calls `emitDiscontinuity` on the contributor, with error equal to the contributor UPDATE event’s `reason` + let emitDiscontinuityArguments = await contributor.emitDiscontinuityArguments + try #require(emitDiscontinuityArguments.count == 1) + + let discontinuity = emitDiscontinuityArguments[0] + #expect(discontinuity === contributorStateChange.reason) + } + + // @specPartial CHA-RL4b1 - I don’t know the meaning of "and the particular contributor has been attached previously" so haven’t implemented that part of the spec point (TODO: asked in https://github.com/ably/specification/pull/200/files#r1775552624) + @Test + func contributorAttachEvent_withResumeFalse_withOperationInProgress_recordsPendingDiscontinuityEvent() async throws { + // Given: A RoomLifecycleManager, with a room lifecycle operation in progress + let contributor = createContributor() + let manager = await createManager(forTestingWhatHappensWhenHasOperationInProgress: true, contributors: [contributor]) + + // When: A contributor emits an ATTACHED event with `resumed` flag set to false + let contributorStateChange = ARTChannelStateChange( + current: .attached, + previous: .attaching, // arbitrary + event: .attached, + reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary + resumed: false + ) + + await waitForManager(manager, toHandleContributorStateChange: contributorStateChange) { + await contributor.channel.emitStateChange(contributorStateChange) + } + + // Then: The manager records a pending discontinuity event for this contributor, and this discontinuity event has error equal to the contributor ATTACHED event’s `reason` + let pendingDiscontinuityEvents = await manager.testsOnly_pendingDiscontinuityEvents(for: contributor) + try #require(pendingDiscontinuityEvents.count == 1) + + let pendingDiscontinuityEvent = pendingDiscontinuityEvents[0] + #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) + @Test + func contributorFailedEvent_withNoOperationInProgress() async throws { + // Given: A RoomLifecycleManager, with no room lifecycle operation in progress + let contributors = [ + // 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), + ] + let manager = await createManager(forTestingWhatHappensWhenHasOperationInProgress: false, contributors: contributors) + + let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) + async let failedStatusChange = roomStatusSubscription.first { $0.current == .failed } + + // When: A contributor emits an FAILED event + let contributorStateChange = ARTChannelStateChange( + current: .failed, + previous: .attached, // arbitrary + event: .failed, + reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary + resumed: false // arbitrary + ) + + await waitForManager(manager, toHandleContributorStateChange: contributorStateChange) { + await contributors[0].channel.emitStateChange(contributorStateChange) + } + + // 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 + _ = try #require(await failedStatusChange) + #expect(await manager.current == .failed) + + for contributor in contributors { + #expect(await contributor.channel.detachCallCount == 1) + } + } + + // @specOneOf(1/2) CHA-RL4b8 + @Test + func contributorAttachedEvent_withNoOperationInProgress_roomNotAttached_allContributorsAttached() async throws { + // Given: A RoomLifecycleManager, 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) + let contributors = [ + createContributor(initialState: .attached), + createContributor(initialState: .attached), + ] + + let manager = await createManager( + forTestingWhatHappensWhenCurrentlyIn: .initialized, // arbitrary non-ATTACHED + forTestingWhatHappensWhenHasOperationInProgress: false, + contributors: contributors + ) + + let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) + async let maybeAttachedRoomStatusChange = roomStatusSubscription.first { $0.current == .attached } + + // When: A contributor emits a state change to ATTACHED + let contributorStateChange = ARTChannelStateChange( + current: .attached, + previous: .attaching, // arbitrary + event: .attached, + reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary + resumed: false // arbitrary + ) + + await contributors[0].channel.emitStateChange(contributorStateChange) + + // Then: The room status transitions to ATTACHED + _ = try #require(await maybeAttachedRoomStatusChange) + #expect(await manager.current == .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 + @Test + func contributorAttachedEvent_withNoOperationInProgress_roomNotAttached_notAllContributorsAttached() async throws { + // Given: A RoomLifecycleManager, 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) + let contributors = [ + createContributor(initialState: .attached), + createContributor(initialState: .detached), + ] + + let initialManagerState = RoomLifecycle.initialized // arbitrary non-ATTACHED + let manager = await createManager( + forTestingWhatHappensWhenCurrentlyIn: initialManagerState, + forTestingWhatHappensWhenHasOperationInProgress: false, + contributors: contributors + ) + + // When: A contributor emits a state change to ATTACHED + let contributorStateChange = ARTChannelStateChange( + current: .attached, + previous: .attaching, // arbitrary + event: .attached, + reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary + resumed: false // arbitrary + ) + + await waitForManager(manager, toHandleContributorStateChange: contributorStateChange) { + await contributors[0].channel.emitStateChange(contributorStateChange) + } + + // Then: The room status does not change + #expect(await manager.current == initialManagerState) + } + + // @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) + @Test + func contributorSuspendedEvent_withNoOperationInProgress() async throws { + // Given: A RoomLifecycleManager with no lifecycle operation in progress + let contributor = createContributor() + let manager = await createManager(forTestingWhatHappensWhenHasOperationInProgress: false, contributors: [contributor]) + + let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) + async let maybeSuspendedRoomStatusChange = roomStatusSubscription.first { $0.current == .suspended } + + // When: A contributor emits a state change to SUSPENDED + let contributorStateChange = ARTChannelStateChange( + current: .suspended, + previous: .attached, // arbitrary + event: .suspended, + reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary + resumed: false // arbitrary + ) + + await contributor.channel.emitStateChange(contributorStateChange) + + // 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(await manager.current == .suspended) + #expect(await manager.error === contributorStateChange.reason) + } } From 05836a64df087336c54291bf3ef6237addd912db Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 30 Sep 2024 14:12:44 -0300 Subject: [PATCH 5/5] Implement CHA-RL1g2 Resolves #53. --- Sources/AblyChat/RoomLifecycleManager.swift | 27 ++++++++++++-- .../RoomLifecycleManagerTests.swift | 36 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 4d3ae5c..4204160 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -58,9 +58,9 @@ internal actor RoomLifecycleManager { private struct ContributorAnnotations { private var storage: [Contributor.ID: ContributorAnnotation] - init(contributors: [Contributor]) { + init(contributors: [Contributor], pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]) { storage = contributors.reduce(into: [:]) { result, contributor in - result[contributor.id] = .init() + result[contributor.id] = .init(pendingDiscontinuityEvents: pendingDiscontinuityEvents[contributor.id] ?? []) } } @@ -100,6 +100,7 @@ internal actor RoomLifecycleManager { await self.init( current: nil, hasOperationInProgress: nil, + pendingDiscontinuityEvents: [:], contributors: contributors, logger: logger, clock: clock @@ -110,6 +111,7 @@ internal actor RoomLifecycleManager { internal init( testsOnly_current current: RoomLifecycle? = nil, testsOnly_hasOperationInProgress hasOperationInProgress: Bool? = nil, + testsOnly_pendingDiscontinuityEvents pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]? = nil, contributors: [Contributor], logger: InternalLogger, clock: SimpleClock @@ -117,6 +119,7 @@ internal actor RoomLifecycleManager { await self.init( current: current, hasOperationInProgress: hasOperationInProgress, + pendingDiscontinuityEvents: pendingDiscontinuityEvents, contributors: contributors, logger: logger, clock: clock @@ -127,6 +130,7 @@ internal actor RoomLifecycleManager { private init( current: RoomLifecycle?, hasOperationInProgress: Bool?, + pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]?, contributors: [Contributor], logger: InternalLogger, clock: SimpleClock @@ -134,7 +138,7 @@ internal actor RoomLifecycleManager { self.current = current ?? .initialized self.hasOperationInProgress = hasOperationInProgress ?? false self.contributors = contributors - contributorAnnotations = .init(contributors: contributors) + contributorAnnotations = .init(contributors: contributors, pendingDiscontinuityEvents: pendingDiscontinuityEvents ?? [:]) self.logger = logger self.clock = clock @@ -362,6 +366,23 @@ internal actor RoomLifecycleManager { // CHA-RL1g1 changeStatus(to: .attached) + + // CHA-RL1g2 + await emitPendingDiscontinuityEvents() + } + + /// Implements CHA-RL1g2’s emitting of pending discontinuity events. + private func emitPendingDiscontinuityEvents() async { + // Emit all pending discontinuity events + logger.log(message: "Emitting pending discontinuity events", level: .info) + for contributor in contributors { + for pendingDiscontinuityEvent in contributorAnnotations[contributor].pendingDiscontinuityEvents { + logger.log(message: "Emitting pending discontinuity event \(pendingDiscontinuityEvent) to contributor \(contributor)", level: .info) + await contributor.emitDiscontinuity(pendingDiscontinuityEvent) + } + } + + contributorAnnotations.clearPendingDiscontinuityEvents() } /// Implements CHA-RL1h5’s "detach all channels that are not in the FAILED state". diff --git a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift index c7f93c2..14804d2 100644 --- a/Tests/AblyChatTests/RoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/RoomLifecycleManagerTests.swift @@ -30,12 +30,14 @@ struct RoomLifecycleManagerTests { private func createManager( forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle? = nil, forTestingWhatHappensWhenHasOperationInProgress hasOperationInProgress: Bool? = nil, + forTestingWhatHappensWhenHasPendingDiscontinuityEvents pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo]]? = nil, contributors: [MockRoomLifecycleContributor] = [], clock: SimpleClock = MockSimpleClock() ) async -> RoomLifecycleManager { await .init( testsOnly_current: current, testsOnly_hasOperationInProgress: hasOperationInProgress, + testsOnly_pendingDiscontinuityEvents: pendingDiscontinuityEvents, contributors: contributors, logger: TestLogger(), clock: clock @@ -175,6 +177,40 @@ struct RoomLifecycleManagerTests { try #require(await manager.current == .attached) } + // @spec CHA-RL1g2 + @Test + func attach_uponSuccess_emitsPendingDiscontinuityEvents() async throws { + // Given: A RoomLifecycleManager, all of whose contributors’ calls to `attach` succeed + let contributors = (1 ... 3).map { _ in createContributor(attachBehavior: .complete(.success)) } + let pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo]] = [ + contributors[1].id: [.init(domain: "SomeDomain", code: 123) /* arbitrary */ ], + contributors[2].id: [.init(domain: "SomeDomain", code: 456) /* arbitrary */ ], + ] + let manager = await createManager( + forTestingWhatHappensWhenHasPendingDiscontinuityEvents: pendingDiscontinuityEvents, + contributors: contributors + ) + + // When: `performAttachOperation()` is called on the lifecycle manager + try await manager.performAttachOperation() + + // Then: It: + // - emits all pending discontinuities to its contributors + // - clears all pending discontinuity events (TODO: I assume this is the intended behaviour, but confirm; have asked in https://github.com/ably/specification/pull/200/files#r1781917231) + for contributor in contributors { + let expectedPendingDiscontinuityEvents = pendingDiscontinuityEvents[contributor.id] ?? [] + let emitDiscontinuityArguments = await contributor.emitDiscontinuityArguments + try #require(emitDiscontinuityArguments.count == expectedPendingDiscontinuityEvents.count) + for (emitDiscontinuityArgument, expectedArgument) in zip(emitDiscontinuityArguments, expectedPendingDiscontinuityEvents) { + #expect(emitDiscontinuityArgument === expectedArgument) + } + } + + for contributor in contributors { + #expect(await manager.testsOnly_pendingDiscontinuityEvents(for: contributor).isEmpty) + } + } + // @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)