From 2a23687ef87c7f8d687b1fa4dc221e5943f78b11 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 31 Oct 2024 16:35:40 -0300 Subject: [PATCH] Implement RETRY room lifecycle operation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on spec at 8ff947d. The internal triggering of the RETRY operation (as specified by CHA-RL1h3 and CHA-RL4b9) will come in #50. Note that, since the RETRY operation does not currently cause a transition to SUSPENDED, the `hasOperationInProgress` manager property does not return `true` if there’s a RETRY in progress. (As mentioned in cdab387, we’ll address this in #50.) Resolves #51. --- Sources/AblyChat/RoomLifecycleManager.swift | 153 ++++++- .../DefaultRoomLifecycleManagerTests.swift | 402 ++++++++++++++++++ .../MockRoomLifecycleContributorChannel.swift | 2 + 3 files changed, 546 insertions(+), 11 deletions(-) diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 682df6f9..fab81682 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -182,10 +182,12 @@ internal actor DefaultRoomLifecycleManager [Contributor] { + switch trigger { + case .detachOperation: + // CHA-RL2f + contributors + case let .retryOperation(_, triggeringContributor): + // CHA-RL5a + contributors.filter { $0.id != triggeringContributor.id } + } } // MARK: - RELEASE operation @@ -930,7 +966,7 @@ internal actor DefaultRoomLifecycleManager MockRoomLifecycleContributorChannel.AttachOrDetachBehavior in + if callCount == 1 { + return .completeAndChangeState(.failure(.createUnknownError() /* arbitrary */ ), newState: .attached /* this is what CHA-RL2h3 mentions as being the only non-FAILED state that would happen in this situation in reality */ ) + } else { + return .success + } + } + + let contributors = [ + createContributor( + attachBehavior: .success, // Not related to this test, just so that the subsequent CHA-RL5f attachment cycle completes + + // Not related to this test, just so that the subsequent CHA-RL5d wait-for-ATTACHED completes + subscribeToStateBehavior: .addSubscriptionAndEmitStateChange( + .init( + current: .attached, + previous: .attaching, // arbitrary + event: .attached, + reason: nil + ) + ) + ), + createContributor( + attachBehavior: .success, // Not related to this test, just so that the subsequent CHA-RL5f attachment cycle completes + detachBehavior: .fromFunction(detachImpl) + ), + createContributor( + attachBehavior: .success, // Not related to this test, just so that the subsequent CHA-RL5f attachment cycle completes + detachBehavior: .success + ), + ] + + let clock = MockSimpleClock() + + let manager = await createManager(contributors: contributors, clock: clock) + + // When: `performRetryOperation(triggeredByContributor:)` is called on the manager, triggered by a contributor that isn’t that at index 1 + await manager.performRetryOperation(triggeredByContributor: contributors[0]) + + // Then: The manager calls `detach` in sequence on all contributors except that which triggered the RETRY, stopping upon one of these `detach` calls throwing an error, then sleeps for 250ms, then performs these `detach` calls again + + // (Note that for simplicity of the test I’m not actually making assertions about the sequence in which events happen here) + #expect(await contributors[0].channel.detachCallCount == 0) + #expect(await contributors[1].channel.detachCallCount == 2) + #expect(await contributors[2].channel.detachCallCount == 1) + #expect(await clock.sleepCallArguments == [0.25]) + } + + // @spec CHA-RL5c + @Test + func retry_ifDetachFailsDueToFailedChannelState_transitionsToFailed() async throws { + // Given: A RoomLifecycleManager, whose contributor at index 1 has an implementation of `detach()` which throws an error and transitions the contributor to the FAILED state + let contributor1DetachError = ARTErrorInfo.createUnknownError() // arbitrary + + let contributors = [ + // Features arbitrarily chosen, just need to be distinct in order to make assertions about errors later + createContributor(feature: .messages), + createContributor( + feature: .presence, + detachBehavior: .completeAndChangeState(.failure(contributor1DetachError), newState: .failed) + ), + createContributor( + feature: .typing, + detachBehavior: .success + ), + ] + + let manager = await createManager(contributors: contributors) + + let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) + async let maybeFailedStatusChange = roomStatusSubscription.failedElements().first { _ in true } + + // When: `performRetryOperation(triggeredByContributor:)` is called on the manager, triggered by the contributor at index 0 + await manager.performRetryOperation(triggeredByContributor: contributors[0]) + + // Then: + // 1. (This is basically just testing the behaviour of CHA-RL2h1 again, plus the fact that we don’t try to detach the channel that triggered the RETRY) The manager calls `detach` in sequence on all contributors except that which triggered the RETRY, and enters the FAILED status, the associated error for this status being the *DetachmentFailed code corresponding to contributor 1’s feature, and its `cause` is contributor 1’s `errorReason` + // 2. the RETRY operation is terminated (which we confirm here by checking that it has not attempted to attach any of the contributors, which shows us that CHA-RL5f didn’t happen) + #expect(await contributors[0].channel.detachCallCount == 0) + #expect(await contributors[1].channel.detachCallCount == 1) + #expect(await contributors[2].channel.detachCallCount == 1) + + let roomStatus = await manager.roomStatus + let failedStatusChange = try #require(await maybeFailedStatusChange) + + #expect(roomStatus.isFailed) + for error in [roomStatus.error, failedStatusChange.error] { + #expect(isChatError(error, withCode: .presenceDetachmentFailed, cause: contributor1DetachError)) + } + + for contributor in contributors { + #expect(await contributor.channel.attachCallCount == 0) + } + } + + // swiftlint:disable type_name + /// Describes how a contributor will end up in one of the states that CHA-RL5d expects it to end up in. + enum CHA_RL5d_JourneyToState { + // swiftlint:enable type_name + case transitionsToState + case alreadyInState + } + + // @spec CHA-RL5e - We also test how it behaves if _already_ in FAILED, which is not specified but seems like the intended behaviour to me (TODO: confirm, https://github.com/ably/specification/issues/221) + @Test(arguments: [CHA_RL5d_JourneyToState.transitionsToState, .alreadyInState]) + func retry_whenTriggeringContributorEndsUpFailed_terminatesOperation(journeyToState: CHA_RL5d_JourneyToState) async throws { + // Given: A RoomLifecycleManager, with a contributor that: + // - (if test argument journeyToState is .transitionsToState) emits a state change to FAILED after subscribeToState() is called on it + // - (if test argument journeyToState is .alreadyInState) is already FAILED (it would be more realistic for it to become FAILED _during_ the CHA-RL5a detachment cycle, but that would be more awkward to mock, so this will do) + let contributorFailedReason = ARTErrorInfo.createUnknownError() // arbitrary + + let contributors = [ + createContributor( + initialState: ({ + switch journeyToState { + case .alreadyInState: + .failed + case .transitionsToState: + .suspended // arbitrary + } + })(), + initialErrorReason: ({ + switch journeyToState { + case .alreadyInState: + contributorFailedReason + case .transitionsToState: + nil + } + })(), + + detachBehavior: .success, // Not related to this test, just so that the CHA-RL5a detachment cycle completes + + subscribeToStateBehavior: ({ + switch journeyToState { + case .alreadyInState: + return nil + case .transitionsToState: + let contributorFailedStateChange = ARTChannelStateChange( + current: .failed, + previous: .attaching, // arbitrary + event: .failed, + reason: contributorFailedReason + ) + + return .addSubscriptionAndEmitStateChange(contributorFailedStateChange) + } + })() + ), + createContributor( + detachBehavior: .success // Not related to this test, just so that the CHA-RL5a detachment cycle completes + ), + ] + + let manager = await createManager( + // This status is not related to the test, it’s just there so that the manager believes it has an operation in progress and hence doesn’t attempt any CHA-RL4 behaviours upon receiving the contributor state change. In https://github.com/ably-labs/ably-chat-swift/issues/50 we’ll make sure that the manager is _actually_ in the SUSPENDED status when there’s a RETRY happening. + forTestingWhatHappensWhenCurrentlyIn: .suspended(retryOperationID: UUID(), error: .createUnknownError()), + + contributors: contributors + ) + + let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) + async let maybeFailedStatusChange = roomStatusSubscription.failedElements().first { _ in true } + + // When: `performRetryOperation(triggeredByContributor:)` is called on the manager, triggered by the aforementioned contributor + await manager.performRetryOperation(triggeredByContributor: contributors[0]) + + // Then: + // 1. The room transitions to (and remains in) FAILED, and its error is: + // - (if test argument journeyToState is .transitionsToState) that associated with the contributor state change + // - (if test argument journeyToState is .alreadyInState) the channel’s `errorReason` + // 2. The RETRY operation terminates (to confirm this, we check that the room has not attempted to attach any of the contributors, indicating that we have not proceeded to the CHA-RL5f attachment cycle) + + let failedStatusChange = try #require(await maybeFailedStatusChange) + + let roomStatus = await manager.roomStatus + #expect(roomStatus.isFailed) + + for error in [failedStatusChange.error, roomStatus.error] { + #expect(error === contributorFailedReason) + } + + for contributor in contributors { + #expect(await contributor.channel.attachCallCount == 0) + } + } + + // @specOneOf(1/3) CHA-RL5f - Tests the first part of CHA-RL5f, namely the transition to ATTACHING once the triggering channel becomes ATTACHED. (We also test how it behaves if _already_ in ATTACHED, which is not specified but seems like the intended behaviour to me (TODO: confirm, https://github.com/ably/specification/issues/221)) + @Test(arguments: [CHA_RL5d_JourneyToState.transitionsToState, .alreadyInState]) + func retry_whenTriggeringContributorEndsUpAttached_proceedsToAttachmentCycle(journeyToState: CHA_RL5d_JourneyToState) async throws { + // Given: A RoomLifecycleManager, with a contributor that: + // - (if test argument journeyToState is .transitionsToState) emits a state change to ATTACHED after subscribeToState() is called on it + // - (if test argument journeyToState is .alreadyInState) is already ATTACHED (it would be more realistic for it to become ATTACHED _during_ the CHA-RL5a detachment cycle, but that would be more awkward to mock, so this will do) + let contributors = [ + createContributor( + initialState: ({ + switch journeyToState { + case .alreadyInState: + .attached + case .transitionsToState: + .suspended // arbitrary + } + })(), + + attachBehavior: .success, // Not related to this test, just so that the subsequent CHA-RL5f attachment cycle completes + + subscribeToStateBehavior: ({ + switch journeyToState { + case .alreadyInState: + return nil + case .transitionsToState: + let contributorAttachedStateChange = ARTChannelStateChange( + current: .attached, + previous: .attaching, // arbitrary + event: .attached, + reason: nil + ) + + return .addSubscriptionAndEmitStateChange(contributorAttachedStateChange) + } + })() + ), + createContributor( + attachBehavior: .success, // Not related to this test, just so that the subsequent CHA-RL5f attachment cycle completes + detachBehavior: .success // Not related to this test, just so that the CHA-RL5a detachment cycle completes + ), + ] + + let manager = await createManager( + // This status is not related to the test, it’s just there so that the manager believes it has an operation in progress and hence doesn’t attempt any CHA-RL4 behaviours upon receiving the contributor state change. In https://github.com/ably-labs/ably-chat-swift/issues/50 we’ll make sure that the manager is _actually_ in the SUSPENDED status when there’s a RETRY happening. + forTestingWhatHappensWhenCurrentlyIn: .suspended(retryOperationID: UUID(), error: .createUnknownError()), + + contributors: contributors + ) + + let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) + async let maybeAttachingStatusChange = roomStatusSubscription.attachingElements().first { _ in true } + + // When: `performRetryOperation(triggeredByContributor:)` is called on the manager, triggered by the aforementioned contributor + await manager.performRetryOperation(triggeredByContributor: contributors[0]) + + // Then: The room transitions to ATTACHING, and the RETRY operation continues (to confirm this, we check that the room has attempted to attach all of the contributors, indicating that we have proceeded to the CHA-RL5f attachment cycle) + let attachingStatusChange = try #require(await maybeAttachingStatusChange) + // The spec doesn’t mention an error for the ATTACHING status change, so I’ll assume there shouldn’t be one + #expect(attachingStatusChange.error == nil) + + for contributor in contributors { + #expect(await contributor.channel.attachCallCount == 1) + } + } + + // @specOneOf(2/3) CHA-RL5f - Tests the case where the CHA-RL1e attachment cycle succeeds + @Test + func retry_whenAttachmentCycleSucceeds() async throws { + // Given: A RoomLifecycleManager, with contributors on all of whom calling `attach()` succeeds (i.e. conditions for a CHA-RL1e attachment cycle to succeed) + let contributors = [ + createContributor( + attachBehavior: .success, + subscribeToStateBehavior: .addSubscriptionAndEmitStateChange( + .init( + current: .attached, + previous: .attaching, // arbitrary + event: .attached, + reason: nil + ) + ) // Not related to this test, just so that the CHA-RL5d wait completes + ), + createContributor( + attachBehavior: .success, + detachBehavior: .success // Not related to this test, just so that the CHA-RL5a detachment cycle completes + ), + createContributor( + attachBehavior: .success, + detachBehavior: .success // Not related to this test, just so that the CHA-RL5a detachment cycle completes + ), + ] + + let manager = await createManager( + // This status is not related to the test, it’s just there so that the manager believes it has an operation in progress and hence doesn’t attempt any CHA-RL4 behaviours upon receiving the contributor state change. In https://github.com/ably-labs/ably-chat-swift/issues/50 we’ll make sure that the manager is _actually_ in the SUSPENDED status when there’s a RETRY happening. + forTestingWhatHappensWhenCurrentlyIn: .suspended(retryOperationID: UUID(), error: .createUnknownError()), + + contributors: contributors + ) + + let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) + async let attachedStatusChange = roomStatusSubscription.first { $0.current == .attached } + + // When: `performRetryOperation(triggeredByContributor:)` is called on the manager + await manager.performRetryOperation(triggeredByContributor: contributors[0]) + + // Then: The room performs a CHA-RL1e attachment cycle (we sufficiently satisfy ourselves of this fact by checking that it’s attempted to attach all of the channels), transitions to ATTACHED, and the RETRY operation completes + for contributor in contributors { + #expect(await contributor.channel.attachCallCount == 1) + } + + _ = try #require(await attachedStatusChange) + #expect(await manager.roomStatus == .attached) + } + + // @specOneOf(3/3) CHA-RL5f - Tests the case where the CHA-RL1e attachment cycle fails + @Test + func retry_whenAttachmentCycleFails() async throws { + // Given: A RoomLifecycleManager, with a contributor on whom calling `attach()` fails, putting the contributor into the FAILED state (i.e. conditions for a CHA-RL1e attachment cycle to fail) + let contributorAttachError = ARTErrorInfo.createUnknownError() // arbitrary + + let contributors = [ + createContributor( + attachBehavior: .success, + detachBehavior: .success, // So that the detach performed by CHA-RL1h5’s rollback of the attachment cycle succeeds + subscribeToStateBehavior: .addSubscriptionAndEmitStateChange( + .init( + current: .attached, + previous: .attaching, // arbitrary + event: .attached, + reason: nil + ) + ) // Not related to this test, just so that the CHA-RL5d wait completes + ), + createContributor( + attachBehavior: .completeAndChangeState(.failure(contributorAttachError), newState: .failed), + detachBehavior: .success // Not related to this test, just so that the CHA-RL5a detachment cycle completes + ), + createContributor( + attachBehavior: .success, + detachBehavior: .success // So that the CHA-RL5a detachment cycle completes (not related to this test) and so that the detach performed by CHA-RL1h5’s rollback of the attachment cycle succeeds + ), + ] + + let manager = await createManager( + // This status is not related to the test, it’s just there so that the manager believes it has an operation in progress and hence doesn’t attempt any CHA-RL4 behaviours upon receiving the contributor state change. In https://github.com/ably-labs/ably-chat-swift/issues/50 we’ll make sure that the manager is _actually_ in the SUSPENDED status when there’s a RETRY happening. + forTestingWhatHappensWhenCurrentlyIn: .suspended(retryOperationID: UUID(), error: .createUnknownError()), + + contributors: contributors + ) + + let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded) + async let failedStatusChange = roomStatusSubscription.failedElements().first { _ in true } + + // When: `performRetryOperation(triggeredByContributor:)` is called on the manager + await manager.performRetryOperation(triggeredByContributor: contributors[0]) + + // Then: The room performs a CHA-RL1e attachment cycle (we sufficiently satisfy ourselves of this fact by checking that it’s attempted to attach all of the channels up to and including the one for which attachment fails, and subsequently detached all channels except for the FAILED one), transitions to FAILED, and the RETRY operation completes + #expect(await contributors[0].channel.attachCallCount == 1) + #expect(await contributors[1].channel.attachCallCount == 1) + #expect(await contributors[2].channel.attachCallCount == 0) + + #expect(await contributors[0].channel.detachCallCount == 1) // from CHA-RL1h5’s rollback of the attachment cycle + #expect(await contributors[1].channel.detachCallCount == 1) // from CHA-RL5a detachment cycle + #expect(await contributors[2].channel.detachCallCount == 2) // from CHA-RL5a detachment cycle and CHA-RL1h5’s rollback of the attachment cycle + + _ = try #require(await failedStatusChange) + #expect(await manager.roomStatus.isFailed) + } + // MARK: - Handling contributor UPDATE events // @spec CHA-RL4a1 diff --git a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift index b363f875..ceb431c6 100644 --- a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift +++ b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributorChannel.swift @@ -16,11 +16,13 @@ final actor MockRoomLifecycleContributorChannel: RoomLifecycleContributorChannel init( initialState: ARTRealtimeChannelState, + initialErrorReason: ARTErrorInfo?, attachBehavior: AttachOrDetachBehavior?, detachBehavior: AttachOrDetachBehavior?, subscribeToStateBehavior: SubscribeToStateBehavior? ) { state = initialState + errorReason = initialErrorReason self.attachBehavior = attachBehavior self.detachBehavior = detachBehavior self.subscribeToStateBehavior = subscribeToStateBehavior ?? .justAddSubscription