Skip to content

Commit 9c97883

Browse files
Add method for waiting to be able to do non-attach presence ops
This is needed for Umair to be able to implement the presence and typing indicators spec points that I’ve mentioned in the code. Based on spec at f7ca465. Resolves #112.
1 parent 0f11c84 commit 9c97883

File tree

8 files changed

+310
-31
lines changed

8 files changed

+310
-31
lines changed

Sources/AblyChat/Errors.swift

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ public let errorDomain = "AblyChatErrorDomain"
1111
The error codes for errors in the ``errorDomain`` error domain.
1212
*/
1313
public enum ErrorCode: Int {
14+
case nonspecific = 40000
15+
1416
/// ``Rooms.get(roomID:options:)`` was called with a different set of room options than was used on a previous call. You must first release the existing room instance using ``Rooms.release(roomID:)``.
1517
///
1618
/// TODO this code is a guess, revisit in https://github.com/ably-labs/ably-chat-swift/issues/32
@@ -36,7 +38,8 @@ public enum ErrorCode: Int {
3638
internal var statusCode: Int {
3739
// TODO: These are currently a guess, revisit once outstanding spec question re status codes is answered (https://github.com/ably/specification/pull/200#discussion_r1755222945), and also revisit in https://github.com/ably-labs/ably-chat-swift/issues/32
3840
switch self {
39-
case .inconsistentRoomOptions,
41+
case .nonspecific,
42+
.inconsistentRoomOptions,
4043
.messagesDetachmentFailed,
4144
.presenceDetachmentFailed,
4245
.reactionsDetachmentFailed,
@@ -69,6 +72,8 @@ internal enum ChatError {
6972
case roomInFailedState
7073
case roomIsReleasing
7174
case roomIsReleased
75+
case presenceOperationRequiresRoomAttach(feature: RoomFeature)
76+
case presenceOperationDisallowedForCurrentRoomStatus(feature: RoomFeature)
7277

7378
/// The ``ARTErrorInfo.code`` that should be returned for this error.
7479
internal var code: ErrorCode {
@@ -107,20 +112,14 @@ internal enum ChatError {
107112
.roomIsReleasing
108113
case .roomIsReleased:
109114
.roomIsReleased
115+
case .presenceOperationRequiresRoomAttach,
116+
.presenceOperationDisallowedForCurrentRoomStatus:
117+
.nonspecific
110118
}
111119
}
112120

113-
/// A helper type for parameterising the construction of error messages.
114-
private enum AttachOrDetach {
115-
case attach
116-
case detach
117-
}
118-
119-
private static func localizedDescription(
120-
forFailureOfOperation operation: AttachOrDetach,
121-
feature: RoomFeature
122-
) -> String {
123-
let featureDescription = switch feature {
121+
private static func descriptionOfFeature(_ feature: RoomFeature) -> String {
122+
switch feature {
124123
case .messages:
125124
"messages"
126125
case .occupancy:
@@ -132,15 +131,26 @@ internal enum ChatError {
132131
case .typing:
133132
"typing"
134133
}
134+
}
135+
136+
/// A helper type for parameterising the construction of error messages.
137+
private enum AttachOrDetach {
138+
case attach
139+
case detach
140+
}
135141

142+
private static func localizedDescription(
143+
forFailureOfOperation operation: AttachOrDetach,
144+
feature: RoomFeature
145+
) -> String {
136146
let operationDescription = switch operation {
137147
case .attach:
138148
"attach"
139149
case .detach:
140150
"detach"
141151
}
142152

143-
return "The \(featureDescription) feature failed to \(operationDescription)."
153+
return "The \(descriptionOfFeature(feature)) feature failed to \(operationDescription)."
144154
}
145155

146156
/// The ``ARTErrorInfo.localizedDescription`` that should be returned for this error.
@@ -158,6 +168,10 @@ internal enum ChatError {
158168
"Cannot perform operation because the room is in a releasing state."
159169
case .roomIsReleased:
160170
"Cannot perform operation because the room is in a released state."
171+
case let .presenceOperationRequiresRoomAttach(feature):
172+
"To perform this \(Self.descriptionOfFeature(feature)) operation, you must first attach the room."
173+
case let .presenceOperationDisallowedForCurrentRoomStatus(feature):
174+
"This \(Self.descriptionOfFeature(feature)) operation can not be performed given the current room status."
161175
}
162176
}
163177

@@ -171,7 +185,9 @@ internal enum ChatError {
171185
case .inconsistentRoomOptions,
172186
.roomInFailedState,
173187
.roomIsReleasing,
174-
.roomIsReleased:
188+
.roomIsReleased,
189+
.presenceOperationRequiresRoomAttach,
190+
.presenceOperationDisallowedForCurrentRoomStatus:
175191
nil
176192
}
177193
}

Sources/AblyChat/Room.swift

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,17 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
8484
throw ARTErrorInfo.create(withCode: 40000, message: "Ensure your Realtime instance is initialized with a clientId.")
8585
}
8686

87-
let featureChannels = Self.createFeatureChannels(roomID: roomID, roomOptions: options, realtime: realtime)
88-
channels = featureChannels.mapValues(\.channel)
89-
let contributors = featureChannels.values.map(\.contributor)
87+
let featureChannelPartialDependencies = Self.createFeatureChannelPartialDependencies(roomID: roomID, roomOptions: options, realtime: realtime)
88+
channels = featureChannelPartialDependencies.mapValues(\.channel)
89+
let contributors = featureChannelPartialDependencies.values.map(\.contributor)
9090

9191
lifecycleManager = await lifecycleManagerFactory.createManager(
9292
contributors: contributors,
9393
logger: logger
9494
)
9595

96+
let featureChannels = Self.createFeatureChannels(partialDependencies: featureChannelPartialDependencies, lifecycleManager: lifecycleManager)
97+
9698
// TODO: Address force unwrapping of `channels` within feature initialisation below: https://github.com/ably-labs/ably-chat-swift/issues/105
9799

98100
messages = await DefaultMessages(
@@ -124,7 +126,12 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
124126
) : nil
125127
}
126128

127-
private static func createFeatureChannels(roomID: String, roomOptions: RoomOptions, realtime: RealtimeClient) -> [RoomFeature: DefaultFeatureChannel] {
129+
private struct FeatureChannelPartialDependencies {
130+
internal var channel: RealtimeChannelProtocol
131+
internal var contributor: DefaultRoomLifecycleContributor
132+
}
133+
134+
private static func createFeatureChannelPartialDependencies(roomID: String, roomOptions: RoomOptions, realtime: RealtimeClient) -> [RoomFeature: FeatureChannelPartialDependencies] {
128135
.init(uniqueKeysWithValues: [
129136
RoomFeature.messages,
130137
RoomFeature.reactions,
@@ -156,6 +163,16 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
156163
})
157164
}
158165

166+
private static func createFeatureChannels(partialDependencies: [RoomFeature: FeatureChannelPartialDependencies], lifecycleManager: RoomLifecycleManager) -> [RoomFeature: DefaultFeatureChannel] {
167+
partialDependencies.mapValues { partialDependencies in
168+
.init(
169+
channel: partialDependencies.channel,
170+
contributor: partialDependencies.contributor,
171+
roomLifecycleManager: lifecycleManager
172+
)
173+
}
174+
}
175+
159176
public nonisolated var presence: any Presence {
160177
guard let _presence else {
161178
fatalError("Presence is not enabled for this room")

Sources/AblyChat/RoomFeature.swift

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,38 @@ internal enum RoomFeature {
3131

3232
/// Provides all of the channel-related functionality that a room feature (e.g. an implementation of ``Messages``) needs.
3333
///
34-
/// This mishmash exists to give a room feature access to both:
34+
/// This mishmash exists to give a room feature access to:
3535
///
3636
/// - a `RealtimeChannelProtocol` object (this is the interface that our features are currently written against, as opposed to, say, `RoomLifecycleContributorChannel`)
3737
/// - the discontinuities emitted by the room lifecycle
38+
/// - the presence-readiness wait mechanism supplied by the room lifecycle
3839
internal protocol FeatureChannel: Sendable, EmitsDiscontinuities {
3940
var channel: RealtimeChannelProtocol { get }
41+
42+
/// Waits until we can perform presence operations on the contributors of this room without triggering an implicit attach.
43+
///
44+
/// Implements the checks described by CHA-PR3d, CHA-PR3e, CHA-PR3f, and CHA-PR3g (and similar ones described by other functionality that performs contributor presence operations). Namely:
45+
///
46+
/// - CHA-PR3d, CHA-PR10d, CHA-PR6c, CHA-T2c: If the room is in the ATTACHING status, it waits for the current ATTACH to complete and then returns. If the current ATTACH fails, then it re-throws that operation’s error.
47+
/// - CHA-PR3e, CHA-PR11e, CHA-PR6d, CHA-T2d: If the room is in the ATTACHED status, it returns immediately.
48+
/// - CHA-PR3f, CHA-PR11f, CHA-PR6e, CHA-T2e: If the room is in the DETACHED status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationRequiresRoomAttach(feature:)``.
49+
/// - // CHA-PR3g, CHA-PR11g, CHA-PR6f, CHA-T2f: If the room is in any other status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationDisallowedForCurrentRoomStatus(feature:)``.
50+
///
51+
/// - Parameters:
52+
/// - requester: The room feature that wishes to perform a presence operation. This is only used for customising the message of the thrown error.
53+
func waitToBeAbleToPerformPresenceOperations(requestedByFeature requester: RoomFeature) async throws(ARTErrorInfo)
4054
}
4155

4256
internal struct DefaultFeatureChannel: FeatureChannel {
4357
internal var channel: RealtimeChannelProtocol
4458
internal var contributor: DefaultRoomLifecycleContributor
59+
internal var roomLifecycleManager: RoomLifecycleManager
4560

4661
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
4762
await contributor.subscribeToDiscontinuities()
4863
}
64+
65+
internal func waitToBeAbleToPerformPresenceOperations(requestedByFeature requester: RoomFeature) async throws(ARTErrorInfo) {
66+
try await roomLifecycleManager.waitToBeAbleToPerformPresenceOperations(requestedByFeature: requester)
67+
}
4968
}

Sources/AblyChat/RoomLifecycleManager.swift

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ internal protocol RoomLifecycleManager: Sendable {
4646
func performReleaseOperation() async
4747
var roomStatus: RoomStatus { get async }
4848
func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription<RoomStatusChange>
49+
func waitToBeAbleToPerformPresenceOperations(requestedByFeature requester: RoomFeature) async throws(ARTErrorInfo)
4950
}
5051

5152
internal protocol RoomLifecycleManagerFactory: Sendable {
@@ -556,7 +557,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
556557
/// The manager emits an `OperationWaitEvent` each time one room lifecycle operation is going to wait for another to complete. These events are emitted to support testing of the manager; see ``testsOnly_subscribeToOperationWaitEvents``.
557558
internal struct OperationWaitEvent: Equatable {
558559
/// The ID of the operation which initiated the wait. It is waiting for the operation with ID ``waitedOperationID`` to complete.
559-
internal var waitingOperationID: UUID
560+
internal var waitingOperationID: UUID?
560561
/// The ID of the operation whose completion will be awaited.
561562
internal var waitedOperationID: UUID
562563
}
@@ -573,6 +574,29 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
573574
}
574575
#endif
575576

577+
private enum OperationWaitRequester {
578+
case anotherOperation(operationID: UUID)
579+
case waitToBeAbleToPerformPresenceOperations
580+
581+
internal var loggingDescription: String {
582+
switch self {
583+
case let .anotherOperation(operationID):
584+
"Operation \(operationID)"
585+
case .waitToBeAbleToPerformPresenceOperations:
586+
"waitToBeAbleToPerformPresenceOperations"
587+
}
588+
}
589+
590+
internal var waitingOperationID: UUID? {
591+
switch self {
592+
case let .anotherOperation(operationID):
593+
operationID
594+
case .waitToBeAbleToPerformPresenceOperations:
595+
nil
596+
}
597+
}
598+
}
599+
576600
/// Waits for the operation with ID `waitedOperationID` to complete, re-throwing any error thrown by that operation.
577601
///
578602
/// Note that this method currently treats all waited operations as throwing. If you wish to wait for an operation that you _know_ to be non-throwing (which the RELEASE operation currently is) then you’ll need to call this method with `try!` or equivalent. (It might be possible to improve this in the future, but I didn’t want to put much time into figuring it out.)
@@ -581,20 +605,20 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
581605
///
582606
/// - Parameters:
583607
/// - waitedOperationID: The ID of the operation whose completion will be awaited.
584-
/// - waitingOperationID: The ID of the operation which is awaiting this result. Only used for logging.
608+
/// - requester: A description of who is awaiting this result. Only used for logging.
585609
private func waitForCompletionOfOperationWithID(
586610
_ waitedOperationID: UUID,
587-
waitingOperationID: UUID
611+
requester: OperationWaitRequester
588612
) async throws(ARTErrorInfo) {
589-
logger.log(message: "Operation \(waitingOperationID) started waiting for result of operation \(waitedOperationID)", level: .debug)
613+
logger.log(message: "\(requester.loggingDescription) started waiting for result of operation \(waitedOperationID)", level: .debug)
590614

591615
do {
592616
let result = await withCheckedContinuation { (continuation: OperationResultContinuations.Continuation) in
593617
// My “it is guaranteed” in the documentation for this method is really more of an “I hope that”, because it’s based on my pretty vague understanding of Swift concurrency concepts; namely, I believe that if you call this manager-isolated `async` method from another manager-isolated method, the initial synchronous part of this method — in particular the call to `addContinuation` below — will occur _before_ the call to this method suspends. (I think this can be roughly summarised as “calls to async methods on self don’t do actor hopping” but I could be completely misusing a load of Swift concurrency vocabulary there.)
594618
operationResultContinuations.addContinuation(continuation, forResultOfOperationWithID: waitedOperationID)
595619

596620
#if DEBUG
597-
let operationWaitEvent = OperationWaitEvent(waitingOperationID: waitingOperationID, waitedOperationID: waitedOperationID)
621+
let operationWaitEvent = OperationWaitEvent(waitingOperationID: requester.waitingOperationID, waitedOperationID: waitedOperationID)
598622
for subscription in operationWaitEventSubscriptions {
599623
subscription.emit(operationWaitEvent)
600624
}
@@ -603,9 +627,9 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
603627

604628
try result.get()
605629

606-
logger.log(message: "Operation \(waitingOperationID) completed waiting for result of operation \(waitedOperationID), which completed successfully", level: .debug)
630+
logger.log(message: "\(requester.loggingDescription) completed waiting for result of operation \(waitedOperationID), which completed successfully", level: .debug)
607631
} catch {
608-
logger.log(message: "Operation \(waitingOperationID) completed waiting for result of operation \(waitedOperationID), which threw error \(error)", level: .debug)
632+
logger.log(message: "\(requester.loggingDescription) completed waiting for result of operation \(waitedOperationID), which threw error \(error)", level: .debug)
609633
throw error
610634
}
611635
}
@@ -686,7 +710,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
686710

687711
// CHA-RL1d
688712
if let currentOperationID = status.operationID {
689-
try? await waitForCompletionOfOperationWithID(currentOperationID, waitingOperationID: operationID)
713+
try? await waitForCompletionOfOperationWithID(currentOperationID, requester: .anotherOperation(operationID: operationID))
690714
}
691715

692716
// CHA-RL1e
@@ -903,7 +927,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
903927
// CHA-RL3c
904928
// See note on waitForCompletionOfOperationWithID for the current need for this force try
905929
// swiftlint:disable:next force_try
906-
return try! await waitForCompletionOfOperationWithID(releaseOperationID, waitingOperationID: operationID)
930+
return try! await waitForCompletionOfOperationWithID(releaseOperationID, requester: .anotherOperation(operationID: operationID))
907931
case .initialized, .attached, .attachingDueToAttachOperation, .attachingDueToContributorStateChange, .detaching, .suspended, .failed:
908932
break
909933
}
@@ -942,4 +966,31 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
942966
// CHA-RL3g
943967
changeStatus(to: .released)
944968
}
969+
970+
// MARK: - Waiting to be able to perform presence operations
971+
972+
internal func waitToBeAbleToPerformPresenceOperations(requestedByFeature requester: RoomFeature) async throws(ARTErrorInfo) {
973+
switch status {
974+
case let .attachingDueToAttachOperation(attachOperationID):
975+
// CHA-PR3d, CHA-PR10d, CHA-PR6c, CHA-T2c
976+
try await waitForCompletionOfOperationWithID(attachOperationID, requester: .waitToBeAbleToPerformPresenceOperations)
977+
case .attachingDueToContributorStateChange:
978+
// TODO: Spec doesn’t say what to do in this situation; asked in https://github.com/ably/specification/issues/228
979+
fatalError("waitToBeAbleToPerformPresenceOperations doesn’t currently handle attachingDueToContributorStateChange")
980+
case .attached:
981+
// CHA-PR3e, CHA-PR11e, CHA-PR6d, CHA-T2d
982+
break
983+
case .detached:
984+
// CHA-PR3f, CHA-PR11f, CHA-PR6e, CHA-T2e
985+
throw .init(chatError: .presenceOperationRequiresRoomAttach(feature: requester))
986+
case .detaching,
987+
.failed,
988+
.initialized,
989+
.released,
990+
.releasing,
991+
.suspended:
992+
// CHA-PR3g, CHA-PR11g, CHA-PR6f, CHA-T2f
993+
throw .init(chatError: .presenceOperationDisallowedForCurrentRoomStatus(feature: requester))
994+
}
995+
}
945996
}

0 commit comments

Comments
 (0)