Skip to content

Commit

Permalink
Implement room release
Browse files Browse the repository at this point in the history
We use the implementation of the RELEASE operation provided by the room
lifecycle manager, and implement the spec points relating to room map
bookkeeping and releasing the underlying realtime channels.

Based on [1] at 6f0740a.

I have not implemented the spec points that relate to making sure that a
room fetch waits for any previous room with the same ID to finish
releasing; this is a part of the spec which is in flux (currently
implemented via the INITIALIZING status, which was added to the spec
after we started implementing the room lifecycle manager and hasn’t been
implemented in this SDK yet, and soon to be further changed in the spec
by making room-getting async). We can look at the current state of
things when we come to do #66.

Part of #47.

[1] ably/specification#200
  • Loading branch information
lawrence-forooghian committed Nov 12, 2024
1 parent c51fd92 commit fb0f60c
Show file tree
Hide file tree
Showing 10 changed files with 161 additions and 12 deletions.
21 changes: 18 additions & 3 deletions Sources/AblyChat/Room.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ public protocol Room: AnyObject, Sendable {
var options: RoomOptions { get }
}

/// A ``Room`` that exposes additional functionality for use within the SDK.
internal protocol InternalRoom: Room {
func release() async
}

public struct RoomStatusChange: Sendable, Equatable {
public var current: RoomStatus
public var previous: RoomStatus
Expand All @@ -30,7 +35,7 @@ public struct RoomStatusChange: Sendable, Equatable {
}

internal protocol RoomFactory: Sendable {
associatedtype Room: AblyChat.Room
associatedtype Room: AblyChat.InternalRoom

func createRoom(realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: InternalLogger) async throws -> Room
}
Expand All @@ -50,7 +55,7 @@ internal final class DefaultRoomFactory: Sendable, RoomFactory {
}
}

internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>: Room where LifecycleManagerFactory.Contributor == DefaultRoomLifecycleContributor {
internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>: InternalRoom where LifecycleManagerFactory.Contributor == DefaultRoomLifecycleContributor {
internal nonisolated let roomID: String
internal nonisolated let options: RoomOptions
private let chatAPI: ChatAPI
Expand All @@ -61,6 +66,7 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
private nonisolated let realtime: RealtimeClient

private let lifecycleManager: any RoomLifecycleManager
private let channels: [RoomFeature: any RealtimeChannelProtocol]

private let logger: InternalLogger

Expand All @@ -75,7 +81,7 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
throw ARTErrorInfo.create(withCode: 40000, message: "Ensure your Realtime instance is initialized with a clientId.")
}

let channels = Self.createChannels(roomID: roomID, realtime: realtime)
channels = Self.createChannels(roomID: roomID, realtime: realtime)
let contributors = Self.createContributors(channels: channels)

lifecycleManager = await lifecycleManagerFactory.createManager(
Expand Down Expand Up @@ -130,6 +136,15 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
try await lifecycleManager.performDetachOperation()
}

internal func release() async {
await lifecycleManager.performReleaseOperation()

// CHA-RL3h
for channel in channels.values {
realtime.channels.release(channel.name)
}
}

// MARK: - Room status

internal func onStatusChange(bufferingPolicy: BufferingPolicy) async -> Subscription<RoomStatusChange> {
Expand Down
11 changes: 10 additions & 1 deletion Sources/AblyChat/RoomLifecycleManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ internal protocol RoomLifecycleContributor: Identifiable, Sendable {
internal protocol RoomLifecycleManager: Sendable {
func performAttachOperation() async throws
func performDetachOperation() async throws
func performReleaseOperation() async
var roomStatus: RoomStatus { get async }
func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription<RoomStatusChange>
}
Expand Down Expand Up @@ -864,11 +865,19 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor

// MARK: - RELEASE operation

internal func performReleaseOperation() async {
await _performReleaseOperation(forcingOperationID: nil)
}

internal func performReleaseOperation(testsOnly_forcingOperationID forcedOperationID: UUID? = nil) async {
await _performReleaseOperation(forcingOperationID: forcedOperationID)
}

/// Implements CHA-RL3’s RELEASE operation.
///
/// - Parameters:
/// - forcedOperationID: Allows tests to force the operation to have a given ID. In combination with the ``testsOnly_subscribeToOperationWaitEvents`` API, this allows tests to verify that one test-initiated operation is waiting for another test-initiated operation.
internal func performReleaseOperation(testsOnly_forcingOperationID forcedOperationID: UUID? = nil) async {
internal func _performReleaseOperation(forcingOperationID forcedOperationID: UUID? = nil) async {
await performAnOperation(forcingOperationID: forcedOperationID) { operationID in
await bodyOfReleaseOperation(operationID: operationID)
}
Expand Down
19 changes: 17 additions & 2 deletions Sources/AblyChat/Rooms.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,22 @@ internal actor DefaultRooms<RoomFactory: AblyChat.RoomFactory>: Rooms {
}
}

internal func release(roomID _: String) async throws {
fatalError("Not yet implemented")
#if DEBUG
internal func testsOnly_hasExistingRoomWithID(_ roomID: String) -> Bool {
rooms[roomID] != nil
}
#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
return
}

// CHA-RC1d
rooms.removeValue(forKey: roomID)

// CHA-RL1e
await room.release()
}
}
27 changes: 27 additions & 0 deletions Tests/AblyChatTests/DefaultRoomTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,33 @@ struct DefaultRoomTests {
#expect(await lifecycleManager.detachCallCount == 1)
}

// MARK: - Release

// @spec CHA-RL3h - I haven’t explicitly tested that `performReleaseOperation()` happens _before_ releasing the channels (i.e. the “upon operation completion” part of the spec point), because it would require me to spend extra time on mock-writing which I can’t really afford to spend right now. I think we can live with it at least for the time being; I’m pretty sure there are other tests where the spec mentions or requires an order where I also haven’t tested the order.
@Test
func release() async throws {
// Given: a DefaultRoom instance
let channelsList = [
MockRealtimeChannel(name: "basketball::$chat::$chatMessages"),
]
let channels = MockChannels(channels: channelsList)
let realtime = MockRealtime.create(channels: channels)

let lifecycleManager = MockRoomLifecycleManager()
let lifecycleManagerFactory = MockRoomLifecycleManagerFactory(manager: lifecycleManager)

let room = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: .init(), logger: TestLogger(), lifecycleManagerFactory: lifecycleManagerFactory)

// When: `release()` is called on the room
await room.release()

// Then: It:
// 1. calls `performReleaseOperation()` on the room lifecycle manager
// 2. calls `channels.release()` with the name of each of the features’ channels
#expect(await lifecycleManager.releaseCallCount == 1)
#expect(Set(channels.releaseArguments) == Set(channelsList.map(\.name)))
}

// MARK: - Room status

@Test
Expand Down
39 changes: 39 additions & 0 deletions Tests/AblyChatTests/DefaultRoomsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ 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: - Get a room

// @spec CHA-RC1a
@Test
func get_returnsRoomWithGivenID() async throws {
Expand Down Expand Up @@ -78,4 +80,41 @@ struct DefaultRoomsTests {
// Then: It throws an inconsistentRoomOptions error
#expect(isChatError(caughtError, withCode: .inconsistentRoomOptions))
}

// MARK: - Release a room

// @spec CHA-RC1d
// @spec CHA-RC1e
@Test
func release() async throws {
// Given: an instance of DefaultRooms, on which get(roomID:options:) has already been called with a given ID
let realtime = MockRealtime.create(channels: .init(channels: [
.init(name: "basketball::$chat::$chatMessages"),
]))
let options = RoomOptions()
let hasExistingRoomAtMomentRoomReleaseCalledStreamComponents = AsyncStream.makeStream(of: Bool.self)
let roomFactory = MockRoomFactory()
let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: roomFactory)

let roomID = "basketball"

let roomToReturn = MockRoom(options: options) {
await hasExistingRoomAtMomentRoomReleaseCalledStreamComponents.continuation.yield(rooms.testsOnly_hasExistingRoomWithID(roomID))
}
await roomFactory.setRoom(roomToReturn)

_ = try await rooms.get(roomID: roomID, options: .init())
try #require(await rooms.testsOnly_hasExistingRoomWithID(roomID))

// When: `release(roomID:)` is called with this room ID
_ = try await rooms.release(roomID: roomID)

// Then:
// 1. first, the room is removed from the room map
// 2. next, `release` is called on the room
let hasExistingRoomAtMomentRoomReleaseCalled = await hasExistingRoomAtMomentRoomReleaseCalledStreamComponents.stream.first { _ in true }
#expect(try !#require(hasExistingRoomAtMomentRoomReleaseCalled))

#expect(await roomToReturn.releaseCallCount == 1)
}
}
11 changes: 11 additions & 0 deletions Tests/AblyChatTests/IntegrationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,16 @@ struct IntegrationTests {
// (11) Check that we received a DETACHED status change as a result of detaching the room
_ = try #require(await rxRoomStatusSubscription.first { $0.current == .detached })
#expect(await rxRoom.status == .detached)

// (12) Release the room
try await rxClient.rooms.release(roomID: roomID)

// (13) Check that we received a RELEASED status change as a result of releasing the room
_ = try #require(await rxRoomStatusSubscription.first { $0.current == .released })
#expect(await rxRoom.status == .released)

// (14) Fetch the room we just released and check it’s a new object
let postReleaseRxRoom = try await rxClient.rooms.get(roomID: roomID, options: .init())
#expect(postReleaseRxRoom !== rxRoom)
}
}
19 changes: 16 additions & 3 deletions Tests/AblyChatTests/Mocks/MockChannels.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import Ably
import AblyChat

final class MockChannels: RealtimeChannelsProtocol, Sendable {
final class MockChannels: RealtimeChannelsProtocol, @unchecked Sendable {
private let channels: [MockRealtimeChannel]
private let mutex = NSLock()
/// Access must be synchronized via ``mutex``.
private(set) var _releaseArguments: [String] = []

init(channels: [MockRealtimeChannel]) {
self.channels = channels
Expand All @@ -24,7 +27,17 @@ final class MockChannels: RealtimeChannelsProtocol, Sendable {
fatalError("Not implemented")
}

func release(_: String) {
fatalError("Not implemented")
func release(_ name: String) {
mutex.lock()
_releaseArguments.append(name)
mutex.unlock()
}

var releaseArguments: [String] {
let result: [String]
mutex.lock()
result = _releaseArguments
mutex.unlock()
return result
}
}
15 changes: 13 additions & 2 deletions Tests/AblyChatTests/Mocks/MockRoom.swift
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
@testable import AblyChat

actor MockRoom: Room {
actor MockRoom: InternalRoom {
let options: RoomOptions
private(set) var releaseCallCount = 0
let releaseImplementation: (@Sendable () async -> Void)?

init(options: RoomOptions) {
init(options: RoomOptions, releaseImplementation: (@Sendable () async -> Void)? = nil) {
self.options = options
self.releaseImplementation = releaseImplementation
}

nonisolated var roomID: String {
Expand Down Expand Up @@ -46,4 +49,12 @@ actor MockRoom: Room {
func detach() async throws {
fatalError("Not implemented")
}

func release() async {
releaseCallCount += 1
guard let releaseImplementation else {
fatalError("releaseImplementation must be set before calling `release`")
}
await releaseImplementation()
}
}
6 changes: 5 additions & 1 deletion Tests/AblyChatTests/Mocks/MockRoomFactory.swift
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
@testable import AblyChat

actor MockRoomFactory: RoomFactory {
private let room: MockRoom?
private var room: MockRoom?
private(set) var createRoomArguments: (realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: any InternalLogger)?

init(room: MockRoom? = nil) {
self.room = room
}

func setRoom(_ room: MockRoom) {
self.room = room
}

func createRoom(realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: any InternalLogger) async throws -> MockRoom {
createRoomArguments = (realtime: realtime, chatAPI: chatAPI, roomID: roomID, options: options, logger: logger)

Expand Down
5 changes: 5 additions & 0 deletions Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ actor MockRoomLifecycleManager: RoomLifecycleManager {
private(set) var attachCallCount = 0
private let detachResult: Result<Void, ARTErrorInfo>?
private(set) var detachCallCount = 0
private(set) var releaseCallCount = 0
private let _roomStatus: RoomStatus?
// TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36)
private var subscriptions: [Subscription<RoomStatusChange>] = []
Expand All @@ -32,6 +33,10 @@ actor MockRoomLifecycleManager: RoomLifecycleManager {
try detachResult.get()
}

func performReleaseOperation() async {
releaseCallCount += 1
}

var roomStatus: RoomStatus {
guard let roomStatus = _roomStatus else {
fatalError("In order to call roomStatus, roomStatus must be passed to the initializer")
Expand Down

0 comments on commit fb0f60c

Please sign in to comment.