Skip to content

Commit

Permalink
Implement CHA-PR3h and similar
Browse files Browse the repository at this point in the history 
Resolves #151.
  • Loading branch information
@lawrence-forooghian
lawrence-forooghian committed Nov 28, 2024
1 parent 770a940 commit 951e66b
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 72 deletions.
45 changes: 18 additions & 27 deletions Sources/AblyChat/Errors.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ 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 @@ -38,7 +36,6 @@ public enum ErrorCode: Int {

/// Has a case for each of the ``ErrorCode`` cases that imply a fixed status code.
internal enum CaseThatImpliesFixedStatusCode {
case nonspecific
case inconsistentRoomOptions
case messagesAttachmentFailed
case presenceAttachmentFailed
Expand All @@ -53,12 +50,9 @@ public enum ErrorCode: Int {
case roomInFailedState
case roomIsReleasing
case roomIsReleased
case roomInInvalidState

internal var toNumericErrorCode: ErrorCode {
switch self {
case .nonspecific:
.nonspecific
case .inconsistentRoomOptions:
.inconsistentRoomOptions
case .messagesAttachmentFailed:
Expand Down Expand Up @@ -87,17 +81,14 @@ public enum ErrorCode: Int {
.roomIsReleasing
case .roomIsReleased:
.roomIsReleased
case .roomInInvalidState:
.roomInInvalidState
}
}

/// The ``ARTErrorInfo.statusCode`` that should be returned for this error.
internal var statusCode: Int {
// These status codes are taken from the "Chat-specific Error Codes" section of the spec.
switch self {
case .nonspecific,
.inconsistentRoomOptions,
case .inconsistentRoomOptions,
.roomInFailedState,
.roomIsReleasing,
.roomIsReleased:
Expand All @@ -112,18 +103,21 @@ public enum ErrorCode: Int {
.presenceDetachmentFailed,
.reactionsDetachmentFailed,
.occupancyDetachmentFailed,
.typingDetachmentFailed,
// CHA-RL9c
.roomInInvalidState:
.typingDetachmentFailed:
500
}
}
}

/// Has a case for each of the ``ErrorCode`` cases that do not imply a fixed status code.
internal enum CaseThatImpliesVariableStatusCode {
case roomInInvalidState

internal var toNumericErrorCode: ErrorCode {
switch self {}
switch self {
case .roomInInvalidState:
.roomInInvalidState
}
}
}
}
Expand Down Expand Up @@ -169,8 +163,7 @@ internal enum ChatError {
case roomIsReleasing
case roomIsReleased
case presenceOperationRequiresRoomAttach(feature: RoomFeature)
case presenceOperationDisallowedForCurrentRoomStatus(feature: RoomFeature)
case roomInInvalidState(cause: ARTErrorInfo?)
case roomTransitionedToInvalidStateForPresenceOperation(cause: ARTErrorInfo?)

internal var codeAndStatusCode: ErrorCodeAndStatusCode {
switch self {
Expand Down Expand Up @@ -208,11 +201,12 @@ internal enum ChatError {
.fixedStatusCode(.roomIsReleasing)
case .roomIsReleased:
.fixedStatusCode(.roomIsReleased)
case .roomInInvalidState:
.fixedStatusCode(.roomInInvalidState)
case .presenceOperationRequiresRoomAttach,
.presenceOperationDisallowedForCurrentRoomStatus:
.fixedStatusCode(.nonspecific)
case .roomTransitionedToInvalidStateForPresenceOperation:
// CHA-RL9c
.variableStatusCode(.roomInInvalidState, statusCode: 500)
case .presenceOperationRequiresRoomAttach:
// CHA-PR3h, CHA-PR10h, CHA-PR6h, CHA-T2g
.variableStatusCode(.roomInInvalidState, statusCode: 400)
}
}

Expand Down Expand Up @@ -268,9 +262,7 @@ internal enum ChatError {
"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."
case .roomInInvalidState:
case .roomTransitionedToInvalidStateForPresenceOperation:
"The room operation failed because the room was in an invalid state."
}
}
Expand All @@ -282,14 +274,13 @@ internal enum ChatError {
underlyingError
case let .detachmentFailed(_, underlyingError):
underlyingError
case let .roomInInvalidState(cause):
case let .roomTransitionedToInvalidStateForPresenceOperation(cause):
cause
case .inconsistentRoomOptions,
.roomInFailedState,
.roomIsReleasing,
.roomIsReleased,
.presenceOperationRequiresRoomAttach,
.presenceOperationDisallowedForCurrentRoomStatus:
.presenceOperationRequiresRoomAttach:
nil
}
}
Expand Down
7 changes: 3 additions & 4 deletions Sources/AblyChat/RoomFeature.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ internal protocol FeatureChannel: Sendable, EmitsDiscontinuities {

/// 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:
/// Implements the checks described by CHA-PR3d, CHA-PR3e, and CHA-PR3h (and similar ones described by other functionality that performs contributor presence operations). Namely:
///
/// - CHA-RL9, which is invoked by CHA-PR3d, CHA-PR10d, CHA-PR6c, CHA-T2c: If the room is in the ATTACHING status, it waits for the next room status change. If the new status is ATTACHED, it returns. Else, it throws an `ARTErrorInfo` derived from ``ChatError.roomInInvalidState(cause:)``.
/// - CHA-RL9, which is invoked by CHA-PR3d, CHA-PR10d, CHA-PR6c, CHA-T2c: If the room is in the ATTACHING status, it waits for the next room status change. If the new status is ATTACHED, it returns. Else, it throws an `ARTErrorInfo` derived from ``ChatError.roomTransitionedToInvalidStateForPresenceOperation(cause:)``.
/// - CHA-PR3e, CHA-PR10e, CHA-PR6d, CHA-T2d: If the room is in the ATTACHED status, it returns immediately.
/// - CHA-PR3f, CHA-PR10f, CHA-PR6e, CHA-T2e: If the room is in the DETACHED status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationRequiresRoomAttach(feature:)``.
/// - // CHA-PR3g, CHA-PR10g, CHA-PR6f, CHA-T2f: If the room is in any other status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationDisallowedForCurrentRoomStatus(feature:)``.
/// - CHA-PR3h, CHA-PR10h, CHA-PR6h, CHA-T2g: If the room is in any other status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationRequiresRoomAttach(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.
Expand Down
9 changes: 3 additions & 6 deletions Sources/AblyChat/RoomLifecycleManager.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1225,17 +1225,14 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
// TODO: decide what to do if nextRoomStatusChange is nil; I believe that this will happen if the current Task is cancelled. For now, will just treat it as an invalid status change. Handle it properly in https://github.com/ably-labs/ably-chat-swift/issues/29
if nextRoomStatusChange?.current != .attached {
// CHA-RL9c
throw .init(chatError: .roomInInvalidState(cause: nextRoomStatusChange?.current.error))
throw .init(chatError: .roomTransitionedToInvalidStateForPresenceOperation(cause: nextRoomStatusChange?.current.error))
}
case .attached:
// CHA-PR3e, CHA-PR10e, CHA-PR6d, CHA-T2d
break
case .detached:
// CHA-PR3f, CHA-PR10f, CHA-PR6e, CHA-T2e
throw .init(chatError: .presenceOperationRequiresRoomAttach(feature: requester))
default:
// CHA-PR3g, CHA-PR10g, CHA-PR6f, CHA-T2f
throw .init(chatError: .presenceOperationDisallowedForCurrentRoomStatus(feature: requester))
// CHA-PR3h, CHA-PR10h, CHA-PR6h, CHA-T2g
throw .init(chatError: .presenceOperationRequiresRoomAttach(feature: requester))
}
}

Expand Down
45 changes: 10 additions & 35 deletions Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2087,7 +2087,7 @@ struct DefaultRoomLifecycleManagerTests {
let contributorAttachError = ARTErrorInfo.createUnknownError() // arbitrary
contributorAttachOperation.complete(behavior: .completeAndChangeState(.failure(contributorAttachError), newState: .failed))

// Then: The call to `waitToBeAbleToPerformPresenceOperations(requestedByFeature:)` fails with a `roomInInvalidState` error, whose cause is the error associated with the room status change
// Then: The call to `waitToBeAbleToPerformPresenceOperations(requestedByFeature:)` fails with a `roomInInvalidState` error with status code 500, whose cause is the error associated with the room status change
var caughtError: Error?
do {
try await waitToBeAbleToPerformPresenceOperationsResult
Expand All @@ -2096,7 +2096,7 @@ struct DefaultRoomLifecycleManagerTests {
}

let expectedCause = ARTErrorInfo(chatError: .attachmentFailed(feature: .messages, underlyingError: contributorAttachError)) // using our knowledge of CHA-RL1h4
#expect(isChatError(caughtError, withCodeAndStatusCode: .fixedStatusCode(.roomInInvalidState), cause: expectedCause))
#expect(isChatError(caughtError, withCodeAndStatusCode: .variableStatusCode(.roomInInvalidState, statusCode: 500), cause: expectedCause))
}

// @specPartial CHA-PR3e - Tests the wait described in the spec point, but not that the feature actually performs this wait nor the side effect. TODO change this to a specOneOf once the feature is implemented
Expand All @@ -2115,40 +2115,15 @@ struct DefaultRoomLifecycleManagerTests {
try await manager.waitToBeAbleToPerformPresenceOperations(requestedByFeature: .messages /* arbitrary */ )
}

// @specPartial CHA-PR3f - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
// @specPartial CHA-PR10f - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
// @specPartial CHA-PR6e - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
// @specPartial CHA-T2e - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
@Test
func waitToBeAbleToPerformPresenceOperations_whenDetached() async throws {
// Given: A DefaultRoomLifecycleManager in the DETACHED status
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .detached
)

// (Note: I wanted to use #expect(…, throws:) below, but for some reason it made the compiler _crash_! No idea why. So, gave up on that.)

// When: `waitToBeAbleToPerformPresenceOperations(requestedByFeature:)` is called on the lifecycle manager
var caughtError: ARTErrorInfo?
do {
try await manager.waitToBeAbleToPerformPresenceOperations(requestedByFeature: .messages /* arbitrary */ )
} catch {
caughtError = error
}

// Then: It throws a presenceOperationRequiresRoomAttach error for that feature (which we just check via its error code, i.e. `.nonspecific`, and its message)
#expect(isChatError(caughtError, withCodeAndStatusCode: .fixedStatusCode(.nonspecific), message: "To perform this messages operation, you must first attach the room."))
}

// @specPartial CHA-PR3f - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
// @specPartial CHA-PR10f - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
// @specPartial CHA-PR6e - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
// @specPartial CHA-T2e - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
// @specPartial CHA-PR3h - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
// @specPartial CHA-PR10h - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
// @specPartial CHA-PR6h - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
// @specPartial CHA-T2g - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
@Test
func waitToBeAbleToPerformPresenceOperations_whenAnyOtherStatus() async throws {
// Given: A DefaultRoomLifecycleManager in a status other than ATTACHING, ATTACHED or DETACHED
// Given: A DefaultRoomLifecycleManager in a status other than ATTACHING or ATTACHED
let manager = await createManager(
forTestingWhatHappensWhenCurrentlyIn: .detaching(detachOperationID: .init()) // arbitrary given the above constraints
forTestingWhatHappensWhenCurrentlyIn: .detached // arbitrary given the above constraints
)

// (Note: I wanted to use #expect(…, throws:) below, but for some reason it made the compiler _crash_! No idea why. So, gave up on that.)
Expand All @@ -2161,7 +2136,7 @@ struct DefaultRoomLifecycleManagerTests {
caughtError = error
}

// Then: It throws a presenceOperationDisallowedForCurrentRoomStatus error for that feature (which we just check via its error code, i.e. `.nonspecific`, and its message)
#expect(isChatError(caughtError, withCodeAndStatusCode: .fixedStatusCode(.nonspecific), message: "This messages operation can not be performed given the current room status."))
// Then: It throws a roomInInvalidState error for that feature, with status code 400, and a message explaining that the room must first be attached
#expect(isChatError(caughtError, withCodeAndStatusCode: .variableStatusCode(.roomInInvalidState, statusCode: 400), message: "To perform this messages operation, you must first attach the room."))
}
}

0 comments on commit 951e66b

Please sign in to comment.