From 90f9c5547e70d552ee335c84f87fc1d5ababb823 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 2 Dec 2024 11:48:50 -0300 Subject: [PATCH] =?UTF-8?q?Implement=20=E2=80=9Casync=20room=20get?= =?UTF-8?q?=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That is, CHA-RC1f and CHA-RC1g. Note that I’ve taken a slightly different approach to "one operation waits for another" here than that which I took in the room lifecyle manager. I’m using Task instances to represent the operation’s work instead of using subscriptions; this new way feels easier to work with and seems more intuitive, but I guess it means that I maybe create Tasks where I wouldn’t need to otherwise; let’s see which we prefer working with over time. Resolves #152. --- Sources/AblyChat/Errors.swift | 14 +- Sources/AblyChat/Rooms.swift | 267 ++++++++++++++++-- Tests/AblyChatTests/DefaultRoomsTests.swift | 296 +++++++++++++++++++- Tests/AblyChatTests/Mocks/MockRoom.swift | 9 + 4 files changed, 551 insertions(+), 35 deletions(-) diff --git a/Sources/AblyChat/Errors.swift b/Sources/AblyChat/Errors.swift index ba036c46..38a27a79 100644 --- a/Sources/AblyChat/Errors.swift +++ b/Sources/AblyChat/Errors.swift @@ -32,6 +32,8 @@ public enum ErrorCode: Int { case roomIsReleasing = 102_102 case roomIsReleased = 102_103 + case roomReleasedBeforeOperationCompleted = 102_106 + case roomInInvalidState = 102_107 /// Has a case for each of the ``ErrorCode`` cases that imply a fixed status code. @@ -50,6 +52,7 @@ public enum ErrorCode: Int { case roomInFailedState case roomIsReleasing case roomIsReleased + case roomReleasedBeforeOperationCompleted internal var toNumericErrorCode: ErrorCode { switch self { @@ -81,6 +84,8 @@ public enum ErrorCode: Int { .roomIsReleasing case .roomIsReleased: .roomIsReleased + case .roomReleasedBeforeOperationCompleted: + .roomReleasedBeforeOperationCompleted } } @@ -91,7 +96,8 @@ public enum ErrorCode: Int { case .inconsistentRoomOptions, .roomInFailedState, .roomIsReleasing, - .roomIsReleased: + .roomIsReleased, + .roomReleasedBeforeOperationCompleted: 400 case .messagesAttachmentFailed, @@ -162,6 +168,7 @@ internal enum ChatError { case roomInFailedState case roomIsReleasing case roomIsReleased + case roomReleasedBeforeOperationCompleted case presenceOperationRequiresRoomAttach(feature: RoomFeature) case roomTransitionedToInvalidStateForPresenceOperation(cause: ARTErrorInfo?) @@ -201,6 +208,8 @@ internal enum ChatError { .fixedStatusCode(.roomIsReleasing) case .roomIsReleased: .fixedStatusCode(.roomIsReleased) + case .roomReleasedBeforeOperationCompleted: + .fixedStatusCode(.roomReleasedBeforeOperationCompleted) case .roomTransitionedToInvalidStateForPresenceOperation: // CHA-RL9c .variableStatusCode(.roomInInvalidState, statusCode: 500) @@ -260,6 +269,8 @@ 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 .roomReleasedBeforeOperationCompleted: + "Room was released before the operation could complete." case let .presenceOperationRequiresRoomAttach(feature): "To perform this \(Self.descriptionOfFeature(feature)) operation, you must first attach the room." case .roomTransitionedToInvalidStateForPresenceOperation: @@ -280,6 +291,7 @@ internal enum ChatError { .roomInFailedState, .roomIsReleasing, .roomIsReleased, + .roomReleasedBeforeOperationCompleted, .presenceOperationRequiresRoomAttach: nil } diff --git a/Sources/AblyChat/Rooms.swift b/Sources/AblyChat/Rooms.swift index f6691e92..877a897d 100644 --- a/Sources/AblyChat/Rooms.swift +++ b/Sources/AblyChat/Rooms.swift @@ -21,8 +21,55 @@ internal actor DefaultRooms: Rooms { private let logger: InternalLogger private let roomFactory: RoomFactory - /// The set of rooms, keyed by room ID. - private var rooms: [String: RoomFactory.Room] = [:] + /// All the state that a `DefaultRooms` instance might hold for a given room ID. + private enum RoomState { + /// There is no room map entry (see ``RoomMapEntry`` for meaning of this term) for this room ID, but a CHA-RC1g release operation is in progress. + case releaseOperationInProgress(releaseTask: Task) + + /// There is a room map entry for this room ID. + case roomMapEntry(RoomMapEntry) + } + + /// An entry in the “room map” that CHA-RC1f and CHA-RC1g refer to. + private enum RoomMapEntry { + /// The room has been requested, but is awaiting the completion of a CHA-RC1g release operation. + case requestAwaitingRelease( + // A task which provides the result of the pending release operation. + releaseTask: Task, + // The options with which the room was requested. + requestedOptions: RoomOptions, + // A task that will return the result of this room fetch request. + creationTask: Task, + // Calling this function will cause `creationTask` to fail with the given error. + failCreation: @Sendable (Error) -> Void + ) + + /// The room has been created. + case created(room: RoomFactory.Room) + + /// The room options that correspond to this room map entry (either the options that were passed to the pending room fetch request, or the options of the created room). + var roomOptions: RoomOptions { + switch self { + case let .requestAwaitingRelease(_, requestedOptions: options, _, _): + options + case let .created(room): + room.options + } + } + + /// Returns the room which this room map entry corresponds to. If the room map entry represents a pending request, it will return or throw with the result of this request. + func waitForRoom() async throws -> RoomFactory.Room { + switch self { + case let .requestAwaitingRelease(_, _, creationTask: creationTask, _): + try await creationTask.value + case let .created(room): + room + } + } + } + + /// The value for a given room ID is the state that corresponds to that room ID. + private var roomStates: [String: RoomState] = [:] internal init(realtime: RealtimeClient, clientOptions: ClientOptions, logger: InternalLogger, roomFactory: RoomFactory) { self.realtime = realtime @@ -32,40 +79,222 @@ internal actor DefaultRooms: Rooms { chatAPI = ChatAPI(realtime: realtime) } + /// The types of operation that this instance can perform. + internal enum OperationType { + /// A call to ``get(roomID:options:)``. + case get + /// A call to ``release(roomID:)``. + case release + } + + #if DEBUG + internal struct OperationWaitEvent { + internal var waitingOperationType: OperationType + internal var waitedOperationType: OperationType + } + + // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36) + /// Supports the ``testsOnly_subscribeToOperationWaitEvents()`` method. + private var operationWaitEventSubscriptions: [Subscription] = [] + + /// Returns a subscription which emits an event each time one operation is going to wait for another to complete. + internal func testsOnly_subscribeToOperationWaitEvents() -> Subscription { + let subscription = Subscription(bufferingPolicy: .unbounded) + operationWaitEventSubscriptions.append(subscription) + return subscription + } + + private func emitOperationWaitEvent(waitingOperationType: OperationType, waitedOperationType: OperationType) { + let operationWaitEvent = OperationWaitEvent(waitingOperationType: waitingOperationType, waitedOperationType: waitedOperationType) + for subscription in operationWaitEventSubscriptions { + subscription.emit(operationWaitEvent) + } + } + #endif + internal func get(roomID: String, options: RoomOptions) async throws -> any Room { - // CHA-RC1b - if let existingRoom = rooms[roomID] { - // CHA-RC1c - if existingRoom.options != options { - throw ARTErrorInfo( - chatError: .inconsistentRoomOptions(requested: options, existing: existingRoom.options) + if let existingRoomState = roomStates[roomID] { + switch existingRoomState { + case let .roomMapEntry(existingRoomMapEntry): + // CHA-RC1f1 + if existingRoomMapEntry.roomOptions != options { + throw ARTErrorInfo( + chatError: .inconsistentRoomOptions(requested: options, existing: existingRoomMapEntry.roomOptions) + ) + } + + // CHA-RC1f2 + logger.log(message: "Waiting for room from existing room map entry \(existingRoomMapEntry)", level: .debug) + + #if DEBUG + emitOperationWaitEvent(waitingOperationType: .get, waitedOperationType: .get) + #endif + + do { + let room = try await existingRoomMapEntry.waitForRoom() + logger.log(message: "Completed waiting for room from existing room map entry \(existingRoomMapEntry)", level: .debug) + return room + } catch { + logger.log(message: "Got error \(error) waiting for room from existing room map entry \(existingRoomMapEntry)", level: .debug) + throw error + } + case let .releaseOperationInProgress(releaseTask: releaseTask): + let creationFailureFunctions = makeCreationFailureFunctions() + + let creationTask = Task { + logger.log(message: "At start of room creation task", level: .debug) + + // We wait for the first of the following events: + // + // - a creation failure is externally signalled, in which case we throw the corresponding error + // - the in-progress release operation completes + try await withThrowingTaskGroup(of: Void.self) { group in + group.addTask { + try await creationFailureFunctions.throwAnySignalledCreationFailure() + } + + group.addTask { [logger] in + // This task is rather messy but its aim can be summarised as the following: + // + // - if releaseTask completes, then complete + // - if the task is cancelled, then do not propagate the cancellation to releaseTask (because we haven’t properly thought through whether it can handle task cancellation; see existing TODO: https://github.com/ably/ably-chat-swift/issues/29), and do not wait for releaseTask to complete (because the CHA-RC1g4 failure is meant to happen immediately, not only once the release operation completes) + + logger.log(message: "Room creation waiting for completion of release operation", level: .debug) + #if DEBUG + await self.emitOperationWaitEvent(waitingOperationType: .get, waitedOperationType: .release) + #endif + + let (stream, continuation) = AsyncStream.makeStream() + Task.detached { // detached so as not to propagate task cancellation + // CHA-RC1f4 + await releaseTask.value + continuation.yield(()) + continuation.finish() + } + + if await (stream.contains { _ in true }) { + logger.log(message: "Room creation completed waiting for completion of release operation", level: .debug) + } else { + // Task was cancelled + logger.log(message: "Room creation stopped waiting for completion of release operation", level: .debug) + } + } + + // This pattern for waiting for the first of multiple tasks to complete is taken from here: + // https://forums.swift.org/t/accept-the-first-task-to-complete/54386 + defer { group.cancelAll() } + try await group.next() + } + + return try await createRoom(roomID: roomID, options: options) + } + + roomStates[roomID] = .roomMapEntry( + .requestAwaitingRelease( + releaseTask: releaseTask, + requestedOptions: options, + creationTask: creationTask, + failCreation: creationFailureFunctions.failCreation + ) ) - } - return existingRoom + return try await creationTask.value + } } + // CHA-RC1f3 + return try await createRoom(roomID: roomID, options: options) + } + + /// Creates two functions, `failCreation` and `throwAnySignalledCreationFailure`. The latter is an async function that waits until the former is called with an error as an argument; it then throws this error. + private func makeCreationFailureFunctions() -> (failCreation: @Sendable (Error) -> Void, throwAnySignalledCreationFailure: @Sendable () async throws -> Void) { + let (stream, continuation) = AsyncThrowingStream.makeStream(of: Void.self, throwing: Error.self) + + return ( + failCreation: { @Sendable [logger] (error: Error) in + logger.log(message: "Recieved request to fail room creation with error \(error)", level: .debug) + continuation.finish(throwing: error) + }, + throwAnySignalledCreationFailure: { @Sendable [logger] in + logger.log(message: "Waiting for room creation failure request", level: .debug) + do { + try await stream.first { _ in true } + } catch { + logger.log(message: "Wait for room creation failure request gave error \(error)", level: .debug) + throw error + } + logger.log(message: "Wait for room creation failure request completed without error", level: .debug) + } + ) + } + + private func waitForOperation(_ operationTask: Task, waitingOperationType: OperationType, waitedOperationType: OperationType) async { + logger.log(message: "\(waitingOperationType) operation waiting for in-progress \(waitedOperationType) operation to complete", level: .debug) + #if DEBUG + emitOperationWaitEvent(waitingOperationType: waitingOperationType, waitedOperationType: waitedOperationType) + #endif + await operationTask.value + logger.log(message: "\(waitingOperationType) operation completed waiting for in-progress \(waitedOperationType) operation to complete", level: .debug) + } + + private func createRoom(roomID: String, options: RoomOptions) async throws -> RoomFactory.Room { + logger.log(message: "Creating room with ID \(roomID), options \(options)", level: .debug) let room = try await roomFactory.createRoom(realtime: realtime, chatAPI: chatAPI, roomID: roomID, options: options, logger: logger) - rooms[roomID] = room + roomStates[roomID] = .roomMapEntry(.created(room: room)) return room } #if DEBUG - internal func testsOnly_hasExistingRoomWithID(_ roomID: String) -> Bool { - rooms[roomID] != nil + internal func testsOnly_hasRoomMapEntryWithID(_ roomID: String) -> Bool { + guard let roomState = roomStates[roomID] else { + return false + } + + return if case .roomMapEntry = roomState { + true + } else { + false + } } #endif internal func release(roomID: String) async throws { - guard let room = rooms[roomID] else { - // TODO: what to do here? (https://github.com/ably/specification/pull/200/files#r1837154563) — Andy replied that it’s a no-op but that this is going to be specified in an upcoming PR when we make room-getting async + guard let roomState = roomStates[roomID] else { + // CHA-RC1g2 (no-op) return } - // CHA-RC1d - rooms.removeValue(forKey: roomID) + switch roomState { + case let .releaseOperationInProgress(releaseTask): + // CHA-RC1g3 + await waitForOperation(releaseTask, waitingOperationType: .release, waitedOperationType: .release) + case let .roomMapEntry( + .requestAwaitingRelease( + releaseTask: releaseTask, + _, + _, + failCreation: failCreation + ) + ): + // CHA-RC1g4 + logger.log(message: "Release operation requesting failure of in-progress room creation request", level: .debug) + failCreation(ARTErrorInfo(chatError: .roomReleasedBeforeOperationCompleted)) + await waitForOperation(releaseTask, waitingOperationType: .release, waitedOperationType: .release) + case let .roomMapEntry(.created(room: room)): + let releaseTask = Task { + logger.log(message: "Release operation waiting for room release operation to complete", level: .debug) + // Clear the `.releaseOperationInProgress` state (written in a `defer` in case `room.release()` becomes throwing in the future) + defer { roomStates.removeValue(forKey: roomID) } + await room.release() + logger.log(message: "Release operation completed waiting for room release operation to complete", level: .debug) + } + + // Note that, since we’re in an actor, we expect `releaseTask` to always be executed _after_ this synchronous code section, meaning that the `roomStates` mutations happen in the correct order + + // This also achieves CHA-RC1g5 (remove room from room map) + roomStates[roomID] = .releaseOperationInProgress(releaseTask: releaseTask) - // CHA-RL1e - await room.release() + await releaseTask.value + } } } diff --git a/Tests/AblyChatTests/DefaultRoomsTests.swift b/Tests/AblyChatTests/DefaultRoomsTests.swift index 934ecb1c..a30731be 100644 --- a/Tests/AblyChatTests/DefaultRoomsTests.swift +++ b/Tests/AblyChatTests/DefaultRoomsTests.swift @@ -3,11 +3,38 @@ import Testing // The channel name of basketball::$chat::$chatMessages is passed in to these tests due to `DefaultRoom` kicking off the `DefaultMessages` initialization. This in turn needs a valid `roomId` or else the `MockChannels` class will throw an error as it would be expecting a channel with the name \(roomID)::$chat::$chatMessages to exist (where `roomId` is the property passed into `rooms.get`). struct DefaultRoomsTests { + // MARK: - Test helpers + + /// A mock implementation of an `InternalRoom`’s `release` operation. Its ``complete()`` method allows you to signal to the mock that the release should complete. + final class SignallableReleaseOperation: Sendable { + private let continuation: AsyncStream.Continuation + + /// When this function is set as a ``MockRoom``’s `releaseImplementation`, calling ``complete()`` will cause the corresponding `release()` to complete with the result passed to that method. + /// + /// ``release`` will respond to task cancellation by throwing `CancellationError`. + let releaseImplementation: @Sendable () async -> Void + + init() { + let (stream, continuation) = AsyncStream.makeStream(of: Void.self) + self.continuation = continuation + + releaseImplementation = { @Sendable () async in + await (stream.first { _ in true }) // this will return if we yield to the continuation or if the Task is cancelled + } + } + + /// Causes the async function embedded in ``releaseImplementation`` to return. + func complete() { + continuation.yield(()) + } + } + // MARK: - Get a room - // @spec CHA-RC1a + // @spec CHA-RC1f + // @spec CHA-RC1f3 @Test - func get_returnsRoomWithGivenID() async throws { + func get_returnsRoomWithGivenIDAndOptions() async throws { // Given: an instance of DefaultRooms let realtime = MockRealtime.create(channels: .init(channels: [ .init(name: "basketball::$chat::$chatMessages"), @@ -21,20 +48,22 @@ struct DefaultRoomsTests { let roomID = "basketball" let room = try await rooms.get(roomID: roomID, options: options) - // Then: It returns a room that uses the same Realtime instance, with the given ID and options + // Then: It returns a room that uses the same Realtime instance, with the given ID and options, and it creates a room map entry for that ID let mockRoom = try #require(room as? MockRoom) #expect(mockRoom === roomToReturn) + #expect(await rooms.testsOnly_hasRoomMapEntryWithID(roomID)) + let createRoomArguments = try #require(await roomFactory.createRoomArguments) #expect(createRoomArguments.realtime === realtime) #expect(createRoomArguments.roomID == roomID) #expect(createRoomArguments.options == options) } - // @spec CHA-RC1b + // @specOneOf(1/2) CHA-RC1f2 - Tests the case where there is already a room in the room map @Test - func get_returnsExistingRoomWithGivenID() async throws { - // Given: an instance of DefaultRooms, on which get(roomID:options:) has already been called with a given ID + func get_whenRoomExistsInRoomMap_returnsExistingRoomWithGivenID() async throws { + // Given: an instance of DefaultRooms, which has, per CHA-RC1f3, a room in the room map with a given ID let realtime = MockRealtime.create(channels: .init(channels: [ .init(name: "basketball::$chat::$chatMessages"), ])) @@ -46,7 +75,7 @@ struct DefaultRoomsTests { let roomID = "basketball" let firstRoom = try await rooms.get(roomID: roomID, options: options) - // When: get(roomID:options:) is called with the same room ID + // When: get(roomID:options:) is called with the same room ID and options let secondRoom = try await rooms.get(roomID: roomID, options: options) // Then: It does not create another room, and returns the same room object @@ -54,14 +83,59 @@ struct DefaultRoomsTests { #expect(secondRoom === firstRoom) } - // @spec CHA-RC1c + // @specOneOf(2/2) CHA-RC1f2 - Tests the case where, per CHA-RC1f4, there is, in the spec’s language, a _future_ in the room map + @Test + func get_whenFutureExistsInRoomMap_returnsExistingRoomWithGivenID() async throws { + // Given: an instance of DefaultRooms, for which, per CHA-RC1f4, a previous call to get(roomID:options:) with a given ID is waiting for a CHA-RC1g release operation to complete + let realtime = MockRealtime.create(channels: .init(channels: [ + .init(name: "basketball::$chat::$chatMessages"), + ])) + let options = RoomOptions() + + let roomReleaseOperation = SignallableReleaseOperation() + let roomToReturn = MockRoom(options: options, releaseImplementation: roomReleaseOperation.releaseImplementation) + + let roomFactory = MockRoomFactory(room: roomToReturn) + let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: roomFactory) + + let roomID = "basketball" + + // Get a room so that we can release it + _ = try await rooms.get(roomID: roomID, options: options) + let roomReleaseCalls = await roomToReturn.releaseCallsAsyncSequence + async let _ = rooms.release(roomID: roomID) + // Wait for `release` to be called on the room so that we know that the CHA-RC1g release operation is in progress + _ = await roomReleaseCalls.first { _ in true } + + let operationWaitSubscription = await rooms.testsOnly_subscribeToOperationWaitEvents() + // This is the "Given"’s "previous call to get(roomID:options:)" + async let firstRoom = try await rooms.get(roomID: roomID, options: options) + // Wait for the `firstRoom` fetch to start waiting for the CHA-RC1g release operation, to know that we’ve fulfilled the conditions of the "Given" + _ = await operationWaitSubscription.first { $0.waitingOperationType == .get && $0.waitedOperationType == .release } + + // When: get(roomID:options:) is called with the same room ID + async let secondRoom = try await rooms.get(roomID: roomID, options: options) + + // Then: The second call to `get` waits for the first call, and when the CHA-RC1g release operation completes, the second call to get(roomID:options:) does not create another room and returns the same room object as the first call + _ = await operationWaitSubscription.first { $0.waitingOperationType == .get && $0.waitedOperationType == .get } + + // Allow the CHA-RC1g release operation to complete + roomReleaseOperation.complete() + + #expect(await roomFactory.createRoomCallCount == 1) + #expect(try await firstRoom === roomToReturn) + #expect(try await secondRoom === roomToReturn) + } + + // @specOneOf(1/2) CHA-RC1f1 - Tests the case where there is already a room in the room map @Test - func get_throwsErrorWhenOptionsDoNotMatch() async throws { - // Given: an instance of DefaultRooms, on which get(roomID:options:) has already been called with a given ID and options + func get_whenRoomExistsInRoomMap_throwsErrorWhenOptionsDoNotMatch() async throws { + // Given: an instance of DefaultRooms, which has, per CHA-RC1f3, a room in the room map with a given ID and options let realtime = MockRealtime.create(channels: .init(channels: [ .init(name: "basketball::$chat::$chatMessages"), ])) let options = RoomOptions() + let roomToReturn = MockRoom(options: options) let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: MockRoomFactory(room: roomToReturn)) @@ -79,13 +153,205 @@ struct DefaultRoomsTests { } } + // @specOneOf(2/2) CHA-RC1f1 - Tests the case where, per CHA-RC1f4, there is, in the spec’s language, a _future_ in the room map + @Test + func get_whenFutureExistsInRoomMap_throwsErrorWhenOptionsDoNotMatch() async throws { + // Given: an instance of DefaultRooms, for which, per CHA-RC1f4, a previous call to get(roomID:options:) with a given ID and options is waiting for a CHA-RC1g release operation to complete + let realtime = MockRealtime.create(channels: .init(channels: [ + .init(name: "basketball::$chat::$chatMessages"), + ])) + let options = RoomOptions() + + let roomReleaseOperation = SignallableReleaseOperation() + let roomToReturn = MockRoom(options: options, releaseImplementation: roomReleaseOperation.releaseImplementation) + + let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: MockRoomFactory(room: roomToReturn)) + + let roomID = "basketball" + + // Get a room so that we can release it + _ = try await rooms.get(roomID: roomID, options: options) + let roomReleaseCalls = await roomToReturn.releaseCallsAsyncSequence + async let _ = rooms.release(roomID: roomID) + // Wait for `release` to be called on the room so that we know that the CHA-RC1g release operation is in progress + _ = await roomReleaseCalls.first { _ in true } + + let operationWaitSubscription = await rooms.testsOnly_subscribeToOperationWaitEvents() + // This is the "Given"’s "previous call to get(roomID:options:)" + async let _ = try await rooms.get(roomID: roomID, options: options) + // Wait for the `firstRoom` fetch to start waiting for the CHA-RC1g release operation, to know that we’ve fulfilled the conditions of the "Given" + _ = await operationWaitSubscription.first { $0.waitingOperationType == .get && $0.waitedOperationType == .release } + + // When: get(roomID:options:) is called with the same ID but different options + // Then: The second call to get(roomID:options:) throws an inconsistentRoomOptions error + let differentOptions = RoomOptions(presence: .init(subscribe: false)) + + await #expect { + try await rooms.get(roomID: roomID, options: differentOptions) + } throws: { error in + isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.inconsistentRoomOptions)) + } + + // Post-test: Allow the CHA-RC1g release operation to complete + roomReleaseOperation.complete() + } + + // @spec CHA-RC1f4 + @Test + func get_whenReleaseInProgress() async throws { + // Given: an instance of DefaultRooms, for which a CHA-RC1g release operation is in progrss + let realtime = MockRealtime.create(channels: .init(channels: [ + .init(name: "basketball::$chat::$chatMessages"), + ])) + + let roomReleaseOperation = SignallableReleaseOperation() + let options = RoomOptions() + let roomToReturn = MockRoom(options: options, releaseImplementation: roomReleaseOperation.releaseImplementation) + + let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: MockRoomFactory(room: roomToReturn)) + + let roomID = "basketball" + + // Get a room so that we can release it + _ = try await rooms.get(roomID: roomID, options: options) + let roomReleaseCalls = await roomToReturn.releaseCallsAsyncSequence + async let _ = rooms.release(roomID: roomID) + // Wait for `release` to be called on the room so that we know that the CHA-RC1g release operation is in progress + _ = await roomReleaseCalls.first { _ in true } + + // When: `get(roomID:options:)` is called on the room + let operationWaitSubscription = await rooms.testsOnly_subscribeToOperationWaitEvents() + async let fetchedRoom = rooms.get(roomID: roomID, options: options) + + // Then: The call to `get(roomID:options:)` creates a room map entry and waits for the CHA-RC1g release operation to complete + _ = await operationWaitSubscription.first { $0.waitingOperationType == .get && $0.waitedOperationType == .release } + #expect(await rooms.testsOnly_hasRoomMapEntryWithID(roomID)) + + // and When: The CHA-RC1g release operation completes + + // Allow the CHA-RC1g release operation to complete + roomReleaseOperation.complete() + + // Then: The call to `get(roomID:options:)` completes + _ = try await fetchedRoom + } + // MARK: - Release a room - // @spec CHA-RC1d - // @spec CHA-RC1e + // @spec CHA-RC1g2 + @Test + func release_withNoRoomMapEntry_andNoReleaseInProgress() async throws { + // Given: An instance of DefaultRooms, with neither a room map entry nor a release operation in progress for a given room ID + let realtime = MockRealtime.create(channels: .init(channels: [ + .init(name: "basketball::$chat::$chatMessages"), + ])) + let roomFactory = MockRoomFactory() + let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: roomFactory) + + // When: `release(roomID:)` is called with this room ID + // Then: The call to `release(roomID:)` completes (this is as much as I can do to test the spec’s “no-op”; i.e. check it doesn’t seem to wait for anything or have any obvious side effects) + let roomID = "basketball" + try await rooms.release(roomID: roomID) + } + + // @spec CHA-RC1g3 + @Test + func release_withNoRoomMapEntry_andReleaseInProgress() async throws { + // Given: an instance of DefaultRooms, for which a release operation is in progress + let realtime = MockRealtime.create(channels: .init(channels: [ + .init(name: "basketball::$chat::$chatMessages"), + ])) + + let roomReleaseOperation = SignallableReleaseOperation() + let options = RoomOptions() + let roomToReturn = MockRoom(options: options, releaseImplementation: roomReleaseOperation.releaseImplementation) + + let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: MockRoomFactory(room: roomToReturn)) + + let roomID = "basketball" + + // Get a room so that we can release it + _ = try await rooms.get(roomID: roomID, options: options) + let roomReleaseCalls = await roomToReturn.releaseCallsAsyncSequence + async let _ = rooms.release(roomID: roomID) + // Wait for `release` to be called on the room so that we know that the release operation is in progress + _ = await roomReleaseCalls.first { _ in true } + + // When: `release(roomID:)` is called with this room ID + let operationWaitSubscription = await rooms.testsOnly_subscribeToOperationWaitEvents() + async let secondReleaseResult: Void = rooms.release(roomID: roomID) + + // Then: The call to `release(roomID:)` waits for the previous release operation to complete + _ = await operationWaitSubscription.first { $0.waitingOperationType == .release && $0.waitedOperationType == .release } + + // and When: The previous CHA-RC1g release operation completes + + // Allow the previous release operation to complete + roomReleaseOperation.complete() + + // Then: The second call to `release(roomID:)` completes, and this second release call does not trigger a CHA-RL3 room release operation (i.e. in the language of the spec it reuses the “future” of the existing CHA-RC1g release operation) + try await secondReleaseResult + #expect(await roomToReturn.releaseCallCount == 1) + } + + // @spec CHA-RC1g4 + @Test + func release_withReleaseInProgress_failsPendingGetOperations() async throws { + // Given: an instance of DefaultRooms, for which there is a release operation already in progress, and a CHA-RC1f4 future in the room map awaiting the completion of this release operation + let realtime = MockRealtime.create(channels: .init(channels: [ + .init(name: "basketball::$chat::$chatMessages"), + ])) + + let roomReleaseOperation = SignallableReleaseOperation() + let options = RoomOptions() + let roomToReturn = MockRoom(options: options, releaseImplementation: roomReleaseOperation.releaseImplementation) + + let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: MockRoomFactory(room: roomToReturn)) + + let roomID = "basketball" + + // Get a room so that we can release it + _ = try await rooms.get(roomID: roomID, options: options) + let roomReleaseCalls = await roomToReturn.releaseCallsAsyncSequence + async let _ = rooms.release(roomID: roomID) + // Wait for `release` to be called on the room so that we know that the release operation is in progress + _ = await roomReleaseCalls.first { _ in true } + + let operationWaitSubscription = await rooms.testsOnly_subscribeToOperationWaitEvents() + // This is the “CHA-RC1f future” of the “Given” + async let fetchedRoom = rooms.get(roomID: roomID, options: options) + + // Wait for the call to `get(roomID:options:)` to start waiting for the CHA-RC1g release operation to complete + _ = await operationWaitSubscription.first { $0.waitingOperationType == .get && $0.waitedOperationType == .release } + + // When: `release(roomID:)` is called on the room, with the same room ID + async let secondReleaseResult: Void = rooms.release(roomID: roomID) + + // Then: The pending call to `get(roomID:options:)` that is waiting for the “CHA-RC1f future” of the “Given” fails with a RoomReleasedBeforeOperationCompleted error + let roomGetError: Error? + do { + _ = try await fetchedRoom + roomGetError = nil + } catch { + roomGetError = error + } + + #expect(isChatError(roomGetError, withCodeAndStatusCode: .fixedStatusCode(.roomReleasedBeforeOperationCompleted))) + + // and When: The previous CHA-RC1g release operation completes + + // Allow the previous release operation to complete + roomReleaseOperation.complete() + + // Then: The second call to `release(roomID:)` completes, and this second release call does not trigger a CHA-RL3 room release operation (i.e. in the language of the spec it reuses the “future” of the existing CHA-RC1g release operation) + try await secondReleaseResult + #expect(await roomToReturn.releaseCallCount == 1) + } + + // @spec CHA-RC1g5 @Test func release() async throws { - // Given: an instance of DefaultRooms, on which get(roomID:options:) has already been called with a given ID + // Given: an instance of DefaultRooms, which has a room map entry for a given room ID and has no release operation in progress for that room ID let realtime = MockRealtime.create(channels: .init(channels: [ .init(name: "basketball::$chat::$chatMessages"), ])) @@ -97,12 +363,12 @@ struct DefaultRoomsTests { let roomID = "basketball" let roomToReturn = MockRoom(options: options) { - await hasExistingRoomAtMomentRoomReleaseCalledStreamComponents.continuation.yield(rooms.testsOnly_hasExistingRoomWithID(roomID)) + await hasExistingRoomAtMomentRoomReleaseCalledStreamComponents.continuation.yield(rooms.testsOnly_hasRoomMapEntryWithID(roomID)) } await roomFactory.setRoom(roomToReturn) _ = try await rooms.get(roomID: roomID, options: .init()) - try #require(await rooms.testsOnly_hasExistingRoomWithID(roomID)) + try #require(await rooms.testsOnly_hasRoomMapEntryWithID(roomID)) // When: `release(roomID:)` is called with this room ID _ = try await rooms.release(roomID: roomID) diff --git a/Tests/AblyChatTests/Mocks/MockRoom.swift b/Tests/AblyChatTests/Mocks/MockRoom.swift index 7039e218..f5e539d3 100644 --- a/Tests/AblyChatTests/Mocks/MockRoom.swift +++ b/Tests/AblyChatTests/Mocks/MockRoom.swift @@ -8,6 +8,7 @@ actor MockRoom: InternalRoom { init(options: RoomOptions, releaseImplementation: (@Sendable () async -> Void)? = nil) { self.options = options self.releaseImplementation = releaseImplementation + _releaseCallsAsyncSequence = AsyncStream.makeStream() } nonisolated var roomID: String { @@ -52,9 +53,17 @@ actor MockRoom: InternalRoom { func release() async { releaseCallCount += 1 + _releaseCallsAsyncSequence.continuation.yield(()) guard let releaseImplementation else { fatalError("releaseImplementation must be set before calling `release`") } await releaseImplementation() } + + /// Emits an element each time ``release()`` is called. + var releaseCallsAsyncSequence: AsyncStream { + _releaseCallsAsyncSequence.stream + } + + private let _releaseCallsAsyncSequence: (stream: AsyncStream, continuation: AsyncStream.Continuation) }