Skip to content

Commit 279ae93

Browse files
Implement “previously attached” part of CHA-RL4b1
Based on [1] at d9af7bf. I’m assuming that the “no call” in this spec point is a typo and means “a call”; have asked in [2]. Needed for when we integrate the room lifecycle manager into the rest of the SDK in #47 (else there will be a crash when running the integration tests, because a channel’s first ATTACHED status change has resumed == false and our precondition will fail). [1] ably/specification#200 [2] ably/specification#200 (comment)
1 parent d320404 commit 279ae93

File tree

2 files changed

+48
-8
lines changed

2 files changed

+48
-8
lines changed

Sources/AblyChat/RoomLifecycleManager.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,8 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
218218
// TODO: Not clear whether there can be multiple or just one (asked in https://github.com/ably/specification/pull/200/files#r1781927850)
219219
var pendingDiscontinuityEvents: [ARTErrorInfo] = []
220220
var transientDisconnectTimeout: TransientDisconnectTimeout?
221+
/// Whether a CHA-RL1f call to `attach()` on the contributor has previously succeeded.
222+
var hasBeenAttached: Bool
221223

222224
var hasTransientDisconnectTimeout: Bool {
223225
transientDisconnectTimeout != nil
@@ -236,7 +238,8 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
236238
storage = contributors.reduce(into: [:]) { result, contributor in
237239
result[contributor.id] = .init(
238240
pendingDiscontinuityEvents: pendingDiscontinuityEvents[contributor.id] ?? [],
239-
transientDisconnectTimeout: idsOfContributorsWithTransientDisconnectTimeout.contains(contributor.id) ? .init() : nil
241+
transientDisconnectTimeout: idsOfContributorsWithTransientDisconnectTimeout.contains(contributor.id) ? .init() : nil,
242+
hasBeenAttached: false
240243
)
241244
}
242245
}
@@ -375,7 +378,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
375378
}
376379
case .attached:
377380
if hasOperationInProgress {
378-
if !stateChange.resumed {
381+
if !stateChange.resumed, contributorAnnotations[contributor].hasBeenAttached {
379382
// CHA-RL4b1
380383
logger.log(message: "Recording pending discontinuity event for contributor \(contributor)", level: .info)
381384

@@ -650,6 +653,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
650653
do {
651654
logger.log(message: "Attaching contributor \(contributor)", level: .info)
652655
try await contributor.channel.attach()
656+
contributorAnnotations[contributor].hasBeenAttached = true
653657
} catch let contributorAttachError {
654658
let contributorState = await contributor.channel.state
655659
logger.log(message: "Failed to attach contributor \(contributor), which is now in state \(contributorState), error \(contributorAttachError)", level: .info)

Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -970,17 +970,23 @@ struct DefaultRoomLifecycleManagerTests {
970970
#expect(discontinuity === contributorStateChange.reason)
971971
}
972972

973-
// @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)
973+
// @specOneOf(1/2) CHA-RL4b1 - Tests the case where the contributor has been attached previously
974974
@Test
975-
func contributorAttachEvent_withResumeFalse_withOperationInProgress_recordsPendingDiscontinuityEvent() async throws {
976-
// Given: A DefaultRoomLifecycleManager, with a room lifecycle operation in progress
977-
let contributor = createContributor()
975+
func contributorAttachEvent_withResumeFalse_withOperationInProgress_withContributorAttachedPreviously_recordsPendingDiscontinuityEvent() async throws {
976+
// 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
977+
let contributorDetachOperation = SignallableChannelOperation()
978+
let contributor = createContributor(attachBehavior: .success, detachBehavior: contributorDetachOperation.behavior)
978979
let manager = await createManager(
979-
forTestingWhatHappensWhenCurrentlyIn: .attachingDueToAttachOperation(attachOperationID: UUID()), // case and ID arbitrary, just care that an operation is in progress
980980
contributors: [contributor]
981981
)
982982

983-
// When: A contributor emits an ATTACHED event with `resumed` flag set to false
983+
// This is to satisfy "a CHA-RL1f call to `attach()` has succeeded"
984+
try await manager.performAttachOperation()
985+
986+
// This is to put the manager into the DETACHING state, to satisfy "with a room lifecycle operation in progress"
987+
async let _ = manager.performDetachOperation()
988+
989+
// When: The aforementioned contributor emits an ATTACHED event with `resumed` flag set to false
984990
let contributorStateChange = ARTChannelStateChange(
985991
current: .attached,
986992
previous: .attaching, // arbitrary
@@ -999,6 +1005,36 @@ struct DefaultRoomLifecycleManagerTests {
9991005

10001006
let pendingDiscontinuityEvent = pendingDiscontinuityEvents[0]
10011007
#expect(pendingDiscontinuityEvent === contributorStateChange.reason)
1008+
1009+
// Teardown: Allow performDetachOperation() call to complete
1010+
contributorDetachOperation.complete(result: .success)
1011+
}
1012+
1013+
// @specOneOf(2/2) CHA-RL4b1 - Tests the case where the contributor has not been attached previously
1014+
@Test
1015+
func contributorAttachEvent_withResumeFalse_withOperationInProgress_withContributorNotAttachedPreviously_recordsPendingDiscontinuityEvent() async throws {
1016+
// 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
1017+
let contributor = createContributor()
1018+
let manager = await createManager(
1019+
forTestingWhatHappensWhenCurrentlyIn: .attachingDueToAttachOperation(attachOperationID: UUID()), // case and ID arbitrary, just care that an operation is in progress
1020+
contributors: [contributor]
1021+
)
1022+
1023+
// When: The aforementioned contributor emits an ATTACHED event with `resumed` flag set to false
1024+
let contributorStateChange = ARTChannelStateChange(
1025+
current: .attached,
1026+
previous: .attaching, // arbitrary
1027+
event: .attached,
1028+
reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary
1029+
resumed: false
1030+
)
1031+
1032+
await waitForManager(manager, toHandleContributorStateChange: contributorStateChange) {
1033+
await contributor.channel.emitStateChange(contributorStateChange)
1034+
}
1035+
1036+
// Then: The manager does not record a pending discontinuity event for this contributor
1037+
#expect(await manager.testsOnly_pendingDiscontinuityEvents(for: contributor).isEmpty)
10021038
}
10031039

10041040
// @spec CHA-RL4b5

0 commit comments

Comments
 (0)