Skip to content

Commit

Permalink
Merge pull request #104 from ably-labs/implement-previously-attached-…
Browse files Browse the repository at this point in the history
…check

Implement “previously attached” part of CHA-RL4b1
  • Loading branch information
lawrence-forooghian authored Nov 11, 2024
2 parents ac67cea + a1703dc commit 82d660a
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 8 deletions.
8 changes: 6 additions & 2 deletions Sources/AblyChat/RoomLifecycleManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
// 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] = []
var transientDisconnectTimeout: TransientDisconnectTimeout?
/// Whether a CHA-RL1f call to `attach()` on the contributor has previously succeeded.
var hasBeenAttached: Bool

var hasTransientDisconnectTimeout: Bool {
transientDisconnectTimeout != nil
Expand All @@ -236,7 +238,8 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
storage = contributors.reduce(into: [:]) { result, contributor in
result[contributor.id] = .init(
pendingDiscontinuityEvents: pendingDiscontinuityEvents[contributor.id] ?? [],
transientDisconnectTimeout: idsOfContributorsWithTransientDisconnectTimeout.contains(contributor.id) ? .init() : nil
transientDisconnectTimeout: idsOfContributorsWithTransientDisconnectTimeout.contains(contributor.id) ? .init() : nil,
hasBeenAttached: false
)
}
}
Expand Down Expand Up @@ -375,7 +378,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
}
case .attached:
if hasOperationInProgress {
if !stateChange.resumed {
if !stateChange.resumed, contributorAnnotations[contributor].hasBeenAttached {
// CHA-RL4b1
logger.log(message: "Recording pending discontinuity event for contributor \(contributor)", level: .info)

Expand Down Expand Up @@ -650,6 +653,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
do {
logger.log(message: "Attaching contributor \(contributor)", level: .info)
try await contributor.channel.attach()
contributorAnnotations[contributor].hasBeenAttached = true
} catch let contributorAttachError {
let contributorState = await contributor.channel.state
logger.log(message: "Failed to attach contributor \(contributor), which is now in state \(contributorState), error \(contributorAttachError)", level: .info)
Expand Down
48 changes: 42 additions & 6 deletions Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -970,17 +970,23 @@ struct DefaultRoomLifecycleManagerTests {
#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)
// @specOneOf(1/2) CHA-RL4b1 - Tests the case where the contributor has been attached previously
@Test
func contributorAttachEvent_withResumeFalse_withOperationInProgress_recordsPendingDiscontinuityEvent() async throws {
// Given: A DefaultRoomLifecycleManager, with a room lifecycle operation in progress
let contributor = createContributor()
func contributorAttachEvent_withResumeFalse_withOperationInProgress_withContributorAttachedPreviously_recordsPendingDiscontinuityEvent() async throws {
// Given: A DefaultRoomLifecycleManager, with a room lifecycle operation in progress, and which has a contributor for which a CHA-RL1f call to `attach()` has succeeded
let contributorDetachOperation = SignallableChannelOperation()
let contributor = createContributor(attachBehavior: .success, detachBehavior: contributorDetachOperation.behavior)
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .attachingDueToAttachOperation(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
// This is to satisfy "a CHA-RL1f call to `attach()` has succeeded"
try await manager.performAttachOperation()

// This is to put the manager into the DETACHING state, to satisfy "with a room lifecycle operation in progress"
async let _ = manager.performDetachOperation()

// When: The aforementioned contributor emits an ATTACHED event with `resumed` flag set to false
let contributorStateChange = ARTChannelStateChange(
current: .attached,
previous: .attaching, // arbitrary
Expand All @@ -999,6 +1005,36 @@ struct DefaultRoomLifecycleManagerTests {

let pendingDiscontinuityEvent = pendingDiscontinuityEvents[0]
#expect(pendingDiscontinuityEvent === contributorStateChange.reason)

// Teardown: Allow performDetachOperation() call to complete
contributorDetachOperation.complete(result: .success)
}

// @specOneOf(2/2) CHA-RL4b1 - Tests the case where the contributor has not been attached previously
@Test
func contributorAttachEvent_withResumeFalse_withOperationInProgress_withContributorNotAttachedPreviously_doesNotRecordPendingDiscontinuityEvent() async throws {
// Given: A DefaultRoomLifecycleManager, with a room lifecycle operation in progress, and which has a contributor for which a CHA-RL1f call to `attach()` has not previously succeeded
let contributor = createContributor()
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .attachingDueToAttachOperation(attachOperationID: UUID()), // case and ID arbitrary, just care that an operation is in progress
contributors: [contributor]
)

// When: The aforementioned 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 does not record a pending discontinuity event for this contributor
#expect(await manager.testsOnly_pendingDiscontinuityEvents(for: contributor).isEmpty)
}

// @spec CHA-RL4b5
Expand Down

0 comments on commit 82d660a

Please sign in to comment.