Skip to content

Commit

Permalink
Add method for waiting to be able to do non-attach presence ops
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@lawrence-forooghian
lawrence-forooghian committed Nov 18, 2024
1 parent b58f9d7 commit 698f8b5
Show file tree
Hide file tree
Showing 8 changed files with 310 additions and 31 deletions.
44 changes: 30 additions & 14 deletions Sources/AblyChat/Errors.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ public let errorDomain = "AblyChatErrorDomain"
The error codes for errors in the ``errorDomain`` error domain.
*/
public enum ErrorCode: Int {
case nonspecific = 40000

/// ``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:)``.
///
/// TODO this code is a guess, revisit in https://github.com/ably-labs/ably-chat-swift/issues/32
Expand All @@ -36,7 +38,8 @@ public enum ErrorCode: Int {
internal var statusCode: Int {
// 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
switch self {
case .inconsistentRoomOptions,
case .nonspecific,
.inconsistentRoomOptions,
.messagesDetachmentFailed,
.presenceDetachmentFailed,
.reactionsDetachmentFailed,
Expand Down Expand Up @@ -69,6 +72,8 @@ internal enum ChatError {
case roomInFailedState
case roomIsReleasing
case roomIsReleased
case presenceOperationRequiresRoomAttach(feature: RoomFeature)
case presenceOperationDisallowedForCurrentRoomStatus(feature: RoomFeature)

/// The ``ARTErrorInfo.code`` that should be returned for this error.
internal var code: ErrorCode {
Expand Down Expand Up @@ -107,20 +112,14 @@ internal enum ChatError {
.roomIsReleasing
case .roomIsReleased:
.roomIsReleased
case .presenceOperationRequiresRoomAttach,
.presenceOperationDisallowedForCurrentRoomStatus:
.nonspecific
}
}

/// A helper type for parameterising the construction of error messages.
private enum AttachOrDetach {
case attach
case detach
}

private static func localizedDescription(
forFailureOfOperation operation: AttachOrDetach,
feature: RoomFeature
) -> String {
let featureDescription = switch feature {
private static func descriptionOfFeature(_ feature: RoomFeature) -> String {
switch feature {
case .messages:
"messages"
case .occupancy:
Expand All @@ -132,15 +131,26 @@ internal enum ChatError {
case .typing:
"typing"
}
}

/// A helper type for parameterising the construction of error messages.
private enum AttachOrDetach {
case attach
case detach
}

private static func localizedDescription(
forFailureOfOperation operation: AttachOrDetach,
feature: RoomFeature
) -> String {
let operationDescription = switch operation {
case .attach:
"attach"
case .detach:
"detach"
}

return "The \(featureDescription) feature failed to \(operationDescription)."
return "The \(descriptionOfFeature(feature)) feature failed to \(operationDescription)."
}

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

Expand All @@ -171,7 +185,9 @@ internal enum ChatError {
case .inconsistentRoomOptions,
.roomInFailedState,
.roomIsReleasing,
.roomIsReleased:
.roomIsReleased,
.presenceOperationRequiresRoomAttach,
.presenceOperationDisallowedForCurrentRoomStatus:
nil
}
}
Expand Down
25 changes: 21 additions & 4 deletions Sources/AblyChat/Room.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,17 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
throw ARTErrorInfo.create(withCode: 40000, message: "Ensure your Realtime instance is initialized with a clientId.")
}

let featureChannels = Self.createFeatureChannels(roomID: roomID, realtime: realtime)
channels = featureChannels.mapValues(\.channel)
let contributors = featureChannels.values.map(\.contributor)
let featureChannelPartialDependencies = Self.createFeatureChannelPartialDependencies(roomID: roomID, realtime: realtime)
channels = featureChannelPartialDependencies.mapValues(\.channel)
let contributors = featureChannelPartialDependencies.values.map(\.contributor)

lifecycleManager = await lifecycleManagerFactory.createManager(
contributors: contributors,
logger: logger
)

let featureChannels = Self.createFeatureChannels(partialDependencies: featureChannelPartialDependencies, lifecycleManager: lifecycleManager)

// TODO: Address force unwrapping of `channels` within feature initialisation below: https://github.com/ably-labs/ably-chat-swift/issues/105

messages = await DefaultMessages(
Expand All @@ -108,7 +110,12 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
) : nil
}

private static func createFeatureChannels(roomID: String, realtime: RealtimeClient) -> [RoomFeature: DefaultFeatureChannel] {
private struct FeatureChannelPartialDependencies {
internal var channel: RealtimeChannelProtocol
internal var contributor: DefaultRoomLifecycleContributor
}

private static func createFeatureChannelPartialDependencies(roomID: String, realtime: RealtimeClient) -> [RoomFeature: FeatureChannelPartialDependencies] {
.init(uniqueKeysWithValues: [RoomFeature.messages, RoomFeature.reactions].map { feature in
let channel = realtime.getChannel(feature.channelNameForRoomID(roomID))
let contributor = DefaultRoomLifecycleContributor(channel: .init(underlyingChannel: channel), feature: feature)
Expand All @@ -117,6 +124,16 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
})
}

private static func createFeatureChannels(partialDependencies: [RoomFeature: FeatureChannelPartialDependencies], lifecycleManager: RoomLifecycleManager) -> [RoomFeature: DefaultFeatureChannel] {
partialDependencies.mapValues { partialDependencies in
.init(
channel: partialDependencies.channel,
contributor: partialDependencies.contributor,
roomLifecycleManager: lifecycleManager
)
}
}

public nonisolated var presence: any Presence {
fatalError("Not yet implemented")
}
Expand Down
21 changes: 20 additions & 1 deletion Sources/AblyChat/RoomFeature.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,38 @@ internal enum RoomFeature {

/// Provides all of the channel-related functionality that a room feature (e.g. an implementation of ``Messages``) needs.
///
/// This mishmash exists to give a room feature access to both:
/// This mishmash exists to give a room feature access to:
///
/// - a `RealtimeChannelProtocol` object (this is the interface that our features are currently written against, as opposed to, say, `RoomLifecycleContributorChannel`)
/// - the discontinuities emitted by the room lifecycle
/// - the presence-readiness wait mechanism supplied by the room lifecycle
internal protocol FeatureChannel: Sendable, EmitsDiscontinuities {
var channel: RealtimeChannelProtocol { get }

/// Waits until we can perform presence operations on the contributors of this room without triggering an implicit attach.
///
/// 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:
///
/// - 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.
/// - CHA-PR3e, CHA-PR11e, CHA-PR6d, CHA-T2d: If the room is in the ATTACHED status, it returns immediately.
/// - 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:)``.
/// - // 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:)``.
///
/// - Parameters:
/// - requester: The room feature that wishes to perform a presence operation. This is only used for customising the message of the thrown error.
func waitToBeAbleToPerformPresenceOperations(requestedByFeature requester: RoomFeature) async throws(ARTErrorInfo)
}

internal struct DefaultFeatureChannel: FeatureChannel {
internal var channel: RealtimeChannelProtocol
internal var contributor: DefaultRoomLifecycleContributor
internal var roomLifecycleManager: RoomLifecycleManager

internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
await contributor.subscribeToDiscontinuities()
}

internal func waitToBeAbleToPerformPresenceOperations(requestedByFeature requester: RoomFeature) async throws(ARTErrorInfo) {
try await roomLifecycleManager.waitToBeAbleToPerformPresenceOperations(requestedByFeature: requester)
}
}
69 changes: 60 additions & 9 deletions Sources/AblyChat/RoomLifecycleManager.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ internal protocol RoomLifecycleManager: Sendable {
func performReleaseOperation() async
var roomStatus: RoomStatus { get async }
func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription<RoomStatusChange>
func waitToBeAbleToPerformPresenceOperations(requestedByFeature requester: RoomFeature) async throws(ARTErrorInfo)
}

internal protocol RoomLifecycleManagerFactory: Sendable {
Expand Down Expand Up @@ -556,7 +557,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
/// 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``.
internal struct OperationWaitEvent: Equatable {
/// The ID of the operation which initiated the wait. It is waiting for the operation with ID ``waitedOperationID`` to complete.
internal var waitingOperationID: UUID
internal var waitingOperationID: UUID?
/// The ID of the operation whose completion will be awaited.
internal var waitedOperationID: UUID
}
Expand All @@ -573,6 +574,29 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
}
#endif

private enum OperationWaitRequester {
case anotherOperation(operationID: UUID)
case waitToBeAbleToPerformPresenceOperations

internal var loggingDescription: String {
switch self {
case let .anotherOperation(operationID):
"Operation \(operationID)"
case .waitToBeAbleToPerformPresenceOperations:
"waitToBeAbleToPerformPresenceOperations"
}
}

internal var waitingOperationID: UUID? {
switch self {
case let .anotherOperation(operationID):
operationID
case .waitToBeAbleToPerformPresenceOperations:
nil
}
}
}

/// Waits for the operation with ID `waitedOperationID` to complete, re-throwing any error thrown by that operation.
///
/// 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.)
Expand All @@ -581,20 +605,20 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
///
/// - Parameters:
/// - waitedOperationID: The ID of the operation whose completion will be awaited.
/// - waitingOperationID: The ID of the operation which is awaiting this result. Only used for logging.
/// - requester: A description of who is awaiting this result. Only used for logging.
private func waitForCompletionOfOperationWithID(
_ waitedOperationID: UUID,
waitingOperationID: UUID
requester: OperationWaitRequester
) async throws(ARTErrorInfo) {
logger.log(message: "Operation \(waitingOperationID) started waiting for result of operation \(waitedOperationID)", level: .debug)
logger.log(message: "\(requester.loggingDescription) started waiting for result of operation \(waitedOperationID)", level: .debug)

do {
let result = await withCheckedContinuation { (continuation: OperationResultContinuations.Continuation) in
// 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.)
operationResultContinuations.addContinuation(continuation, forResultOfOperationWithID: waitedOperationID)

#if DEBUG
let operationWaitEvent = OperationWaitEvent(waitingOperationID: waitingOperationID, waitedOperationID: waitedOperationID)
let operationWaitEvent = OperationWaitEvent(waitingOperationID: requester.waitingOperationID, waitedOperationID: waitedOperationID)
for subscription in operationWaitEventSubscriptions {
subscription.emit(operationWaitEvent)
}
Expand All @@ -603,9 +627,9 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor

try result.get()

logger.log(message: "Operation \(waitingOperationID) completed waiting for result of operation \(waitedOperationID), which completed successfully", level: .debug)
logger.log(message: "\(requester.loggingDescription) completed waiting for result of operation \(waitedOperationID), which completed successfully", level: .debug)
} catch {
logger.log(message: "Operation \(waitingOperationID) completed waiting for result of operation \(waitedOperationID), which threw error \(error)", level: .debug)
logger.log(message: "\(requester.loggingDescription) completed waiting for result of operation \(waitedOperationID), which threw error \(error)", level: .debug)
throw error
}
}
Expand Down Expand Up @@ -686,7 +710,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor

// CHA-RL1d
if let currentOperationID = status.operationID {
try? await waitForCompletionOfOperationWithID(currentOperationID, waitingOperationID: operationID)
try? await waitForCompletionOfOperationWithID(currentOperationID, requester: .anotherOperation(operationID: operationID))
}

// CHA-RL1e
Expand Down Expand Up @@ -903,7 +927,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
// CHA-RL3c
// See note on waitForCompletionOfOperationWithID for the current need for this force try
// swiftlint:disable:next force_try
return try! await waitForCompletionOfOperationWithID(releaseOperationID, waitingOperationID: operationID)
return try! await waitForCompletionOfOperationWithID(releaseOperationID, requester: .anotherOperation(operationID: operationID))
case .initialized, .attached, .attachingDueToAttachOperation, .attachingDueToContributorStateChange, .detaching, .suspended, .failed:
break
}
Expand Down Expand Up @@ -942,4 +966,31 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
// CHA-RL3g
changeStatus(to: .released)
}

// MARK: - Waiting to be able to perform presence operations

internal func waitToBeAbleToPerformPresenceOperations(requestedByFeature requester: RoomFeature) async throws(ARTErrorInfo) {
switch status {
case let .attachingDueToAttachOperation(attachOperationID):
// CHA-PR3d, CHA-PR10d, CHA-PR6c, CHA-T2c
try await waitForCompletionOfOperationWithID(attachOperationID, requester: .waitToBeAbleToPerformPresenceOperations)
case .attachingDueToContributorStateChange:
// TODO: Spec doesn’t say what to do in this situation; asked in https://github.com/ably/specification/issues/228
fatalError("waitToBeAbleToPerformPresenceOperations doesn’t currently handle attachingDueToContributorStateChange")
case .attached:
// CHA-PR3e, CHA-PR11e, CHA-PR6d, CHA-T2d
break
case .detached:
// CHA-PR3f, CHA-PR11f, CHA-PR6e, CHA-T2e
throw .init(chatError: .presenceOperationRequiresRoomAttach(feature: requester))
case .detaching,
.failed,
.initialized,
.released,
.releasing,
.suspended:
// CHA-PR3g, CHA-PR11g, CHA-PR6f, CHA-T2f
throw .init(chatError: .presenceOperationDisallowedForCurrentRoomStatus(feature: requester))
}
}
}
Loading

0 comments on commit 698f8b5

Please sign in to comment.