Skip to content

Commit

Permalink
Track whether a room lifecycle operation is in progress
Browse files Browse the repository at this point in the history
Remove the temporary hasOperationInProgress stored property added in
b6c3ddb.

It seems to me, at least based on what I’ve implemented so far, that you
have an operation in progress if and only if you’re in a certain status
case, so I’ve encoded this constraint in the Status type. Might turn out
this is false and needs to be revisited later.

TODO note it’s your responsibility to put yourself back into a
non-in-progress state (i.e. same as it would be to unlock a mutex)

TODO this is probably wrong — it's not clear where the emit
discontinuities stuff fits into this
  • Loading branch information
lawrence-forooghian committed Oct 15, 2024
1 parent cc53b28 commit 8203f81
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 25 deletions.
50 changes: 37 additions & 13 deletions Sources/AblyChat/RoomLifecycleManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ internal protocol RoomLifecycleContributor: Identifiable, Sendable {
internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
// MARK: - Constant properties

// 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
private let logger: InternalLogger
private let clock: SimpleClock
private let contributors: [Contributor]
Expand All @@ -67,7 +65,6 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
) async {
await self.init(
status: nil,
hasOperationInProgress: nil,
pendingDiscontinuityEvents: [:],
contributors: contributors,
logger: logger,
Expand All @@ -78,15 +75,13 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
#if DEBUG
internal init(
testsOnly_status status: Status? = nil,
testsOnly_hasOperationInProgress hasOperationInProgress: Bool? = nil,
testsOnly_pendingDiscontinuityEvents pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]? = nil,
contributors: [Contributor],
logger: InternalLogger,
clock: SimpleClock
) async {
await self.init(
status: status,
hasOperationInProgress: hasOperationInProgress,
pendingDiscontinuityEvents: pendingDiscontinuityEvents,
contributors: contributors,
logger: logger,
Expand All @@ -97,14 +92,12 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {

private init(
status: Status?,
hasOperationInProgress: Bool?,
pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]?,
contributors: [Contributor],
logger: InternalLogger,
clock: SimpleClock
) async {
self.status = status ?? .initialized
self.hasOperationInProgress = hasOperationInProgress ?? false
self.contributors = contributors
contributorAnnotations = .init(contributors: contributors, pendingDiscontinuityEvents: pendingDiscontinuityEvents ?? [:])
self.logger = logger
Expand Down Expand Up @@ -145,13 +138,13 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {

internal enum Status: Equatable {
case initialized
case attaching
case attaching(attachOperationID: UUID)
case attached
case detaching
case detaching(detachOperationID: UUID)
case detached
case suspended(error: ARTErrorInfo)
case failed(error: ARTErrorInfo)
case releasing
case releasing(releaseOperationID: UUID)
case released

internal var toRoomLifecycle: RoomLifecycle {
Expand All @@ -176,6 +169,24 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
.released
}
}

fileprivate var operationID: UUID? {
switch self {
case let .attaching(attachOperationID):
attachOperationID
case let .detaching(detachOperationID):
detachOperationID
case let .releasing(releaseOperationID):
releaseOperationID
case .suspended,
.initialized,
.attached,
.detached,
.failed,
.released:
nil
}
}
}

// MARK: - Types for contributor annotations
Expand Down Expand Up @@ -359,10 +370,18 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
#endif
}

// MARK: - Operation handling

private var hasOperationInProgress: Bool {
status.operationID != nil
}

// MARK: - ATTACH operation

/// Implements CHA-RL1’s `ATTACH` operation.
internal func performAttachOperation() async throws {
let operationID = UUID()

switch status {
case .attached:
// CHA-RL1a
Expand All @@ -378,7 +397,7 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
}

// CHA-RL1e
changeStatus(to: .attaching)
changeStatus(to: .attaching(attachOperationID: operationID))

// CHA-RL1f
for contributor in contributors {
Expand Down Expand Up @@ -418,6 +437,7 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
changeStatus(to: .attached)

// CHA-RL1g2
// TODO: this is not great, we're pretending that the operation has finished but it hasn't
await emitPendingDiscontinuityEvents()
}

Expand Down Expand Up @@ -456,6 +476,8 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {

/// Implements CHA-RL2’s DETACH operation.
internal func performDetachOperation() async throws {
let operationID = UUID()

switch status {
case .detached:
// CHA-RL2a
Expand All @@ -474,7 +496,7 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
}

// CHA-RL2e
changeStatus(to: .detaching)
changeStatus(to: .detaching(detachOperationID: operationID))

// CHA-RL2f
var firstDetachError: Error?
Expand Down Expand Up @@ -537,6 +559,8 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {

/// Implements CHA-RL3’s RELEASE operation.
internal func performReleaseOperation() async {
let operationID = UUID()

switch status {
case .released:
// CHA-RL3a
Expand All @@ -550,7 +574,7 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
}

// CHA-RL3c
changeStatus(to: .releasing)
changeStatus(to: .releasing(releaseOperationID: operationID))

// CHA-RL3d
for contributor in contributors {
Expand Down
39 changes: 27 additions & 12 deletions Tests/AblyChatTests/RoomLifecycleManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,12 @@ struct RoomLifecycleManagerTests {

private func createManager(
forTestingWhatHappensWhenCurrentlyIn status: RoomLifecycleManager<MockRoomLifecycleContributor>.Status? = nil,
forTestingWhatHappensWhenHasOperationInProgress hasOperationInProgress: Bool? = nil,
forTestingWhatHappensWhenHasPendingDiscontinuityEvents pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo]]? = nil,
contributors: [MockRoomLifecycleContributor] = [],
clock: SimpleClock = MockSimpleClock()
) async -> RoomLifecycleManager<MockRoomLifecycleContributor> {
await .init(
testsOnly_status: status,
testsOnly_hasOperationInProgress: hasOperationInProgress,
testsOnly_pendingDiscontinuityEvents: pendingDiscontinuityEvents,
contributors: contributors,
logger: TestLogger(),
Expand Down Expand Up @@ -99,7 +97,9 @@ struct RoomLifecycleManagerTests {
@Test
func attach_whenReleasing() async throws {
// Given: A RoomLifecycleManager in the RELEASING state
let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .releasing)
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .releasing(releaseOperationID: UUID() /* arbitrary */ )
)

// When: `performAttachOperation()` is called on the lifecycle manager
// Then: It throws a roomIsReleasing error
Expand Down Expand Up @@ -392,7 +392,9 @@ struct RoomLifecycleManagerTests {
@Test
func detach_whenReleasing() async throws {
// Given: A RoomLifecycleManager in the RELEASING state
let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .releasing)
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .releasing(releaseOperationID: UUID() /* arbitrary */ )
)

// When: `performDetachOperation()` is called on the lifecycle manager
// Then: It throws a roomIsReleasing error
Expand Down Expand Up @@ -751,7 +753,10 @@ struct RoomLifecycleManagerTests {
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])
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .attaching(attachOperationID: UUID()), // case and ID arbitrary, just care that an operation is in progress
contributors: [contributor]
)

// When: A contributor emits an UPDATE event with `resumed` flag set to false
let contributorStateChange = ARTChannelStateChange(
Expand Down Expand Up @@ -779,7 +784,10 @@ struct RoomLifecycleManagerTests {
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])
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .initialized, // case arbitrary, just care that no operation is in progress
contributors: [contributor]
)

// When: A contributor emits an UPDATE event with `resumed` flag set to false
let contributorStateChange = ARTChannelStateChange(
Expand Down Expand Up @@ -807,7 +815,10 @@ struct RoomLifecycleManagerTests {
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])
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .attaching(attachOperationID: UUID()), // case and ID arbitrary, just care that an operation is in progress
contributors: [contributor]
)

// When: A contributor emits an ATTACHED event with `resumed` flag set to false
let contributorStateChange = ARTChannelStateChange(
Expand Down Expand Up @@ -839,7 +850,10 @@ struct RoomLifecycleManagerTests {
createContributor(detachBehavior: .success),
createContributor(detachBehavior: .success),
]
let manager = await createManager(forTestingWhatHappensWhenHasOperationInProgress: false, contributors: contributors)
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .initialized, // case arbitrary, just care that no operation is in progress
contributors: contributors
)

let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded)
async let failedStatusChange = roomStatusSubscription.failedElements().first { _ in true }
Expand Down Expand Up @@ -879,7 +893,6 @@ struct RoomLifecycleManagerTests {

let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .initialized, // arbitrary non-ATTACHED
forTestingWhatHappensWhenHasOperationInProgress: false,
contributors: contributors
)

Expand Down Expand Up @@ -911,10 +924,9 @@ struct RoomLifecycleManagerTests {
createContributor(initialState: .detached),
]

let initialManagerStatus = RoomLifecycleManager<MockRoomLifecycleContributor>.Status.initialized // arbitrary non-ATTACHED
let initialManagerStatus = RoomLifecycleManager<MockRoomLifecycleContributor>.Status.detached // arbitrary non-ATTACHED, no-operation-in-progress
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: initialManagerStatus,
forTestingWhatHappensWhenHasOperationInProgress: false,
contributors: contributors
)

Expand All @@ -940,7 +952,10 @@ struct RoomLifecycleManagerTests {
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 manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .initialized, // case arbitrary, just care that no operation is in progress
contributors: [contributor]
)

let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded)
async let maybeSuspendedRoomStatusChange = roomStatusSubscription.suspendedElements().first { _ in true }
Expand Down

0 comments on commit 8203f81

Please sign in to comment.