Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECO-4982] Implement room release #107

Merged
merged 2 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Sources/AblyChat/ChatClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ public actor DefaultChatClient: ChatClient {
self.realtime = realtime
self.clientOptions = clientOptions ?? .init()
logger = DefaultInternalLogger(logHandler: self.clientOptions.logHandler, logLevel: self.clientOptions.logLevel)
let roomLifecycleManagerFactory = DefaultRoomLifecycleManagerFactory()
rooms = DefaultRooms(realtime: realtime, clientOptions: self.clientOptions, logger: logger, lifecycleManagerFactory: roomLifecycleManagerFactory)
let roomFactory = DefaultRoomFactory()
rooms = DefaultRooms(realtime: realtime, clientOptions: self.clientOptions, logger: logger, roomFactory: roomFactory)
}

public nonisolated var connection: any Connection {
Expand Down
46 changes: 38 additions & 8 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 @@ -29,7 +34,28 @@ public struct RoomStatusChange: Sendable, Equatable {
}
}

internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>: Room where LifecycleManagerFactory.Contributor == DefaultRoomLifecycleContributor {
internal protocol RoomFactory: Sendable {
associatedtype Room: AblyChat.InternalRoom

func createRoom(realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: InternalLogger) async throws -> Room
}

internal final class DefaultRoomFactory: Sendable, RoomFactory {
private let lifecycleManagerFactory = DefaultRoomLifecycleManagerFactory()

internal func createRoom(realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: InternalLogger) async throws -> DefaultRoom<DefaultRoomLifecycleManagerFactory> {
try await DefaultRoom(
realtime: realtime,
chatAPI: chatAPI,
roomID: roomID,
options: options,
logger: logger,
lifecycleManagerFactory: lifecycleManagerFactory
)
}
}
lawrence-forooghian marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -40,12 +66,7 @@ internal actor DefaultRoom<LifecycleManagerFactory: RoomLifecycleManagerFactory>
private nonisolated let realtime: RealtimeClient

private let lifecycleManager: any RoomLifecycleManager

#if DEBUG
internal nonisolated var testsOnly_realtime: RealtimeClient {
realtime
}
#endif
private let channels: [RoomFeature: any RealtimeChannelProtocol]

private let logger: InternalLogger

Expand All @@ -60,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 @@ -115,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)
maratal marked this conversation as resolved.
Show resolved Hide resolved
}
}

// 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
31 changes: 23 additions & 8 deletions Sources/AblyChat/Rooms.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ public protocol Rooms: AnyObject, Sendable {
var clientOptions: ClientOptions { get }
}

internal actor DefaultRooms<LifecycleManagerFactory: RoomLifecycleManagerFactory>: Rooms where LifecycleManagerFactory.Contributor == DefaultRoomLifecycleContributor {
internal actor DefaultRooms<RoomFactory: AblyChat.RoomFactory>: Rooms {
private nonisolated let realtime: RealtimeClient
private let chatAPI: ChatAPI

Expand All @@ -19,16 +19,16 @@ internal actor DefaultRooms<LifecycleManagerFactory: RoomLifecycleManagerFactory
internal nonisolated let clientOptions: ClientOptions

private let logger: InternalLogger
private let lifecycleManagerFactory: LifecycleManagerFactory
private let roomFactory: RoomFactory

/// The set of rooms, keyed by room ID.
private var rooms: [String: DefaultRoom<LifecycleManagerFactory>] = [:]
private var rooms: [String: RoomFactory.Room] = [:]

internal init(realtime: RealtimeClient, clientOptions: ClientOptions, logger: InternalLogger, lifecycleManagerFactory: LifecycleManagerFactory) {
internal init(realtime: RealtimeClient, clientOptions: ClientOptions, logger: InternalLogger, roomFactory: RoomFactory) {
self.realtime = realtime
self.clientOptions = clientOptions
self.logger = logger
self.lifecycleManagerFactory = lifecycleManagerFactory
self.roomFactory = roomFactory
chatAPI = ChatAPI(realtime: realtime)
}

Expand All @@ -43,13 +43,28 @@ internal actor DefaultRooms<LifecycleManagerFactory: RoomLifecycleManagerFactory

return existingRoom
} else {
let room = try await DefaultRoom(realtime: realtime, chatAPI: chatAPI, roomID: roomID, options: options, logger: logger, lifecycleManagerFactory: lifecycleManagerFactory)
let room = try await roomFactory.createRoom(realtime: realtime, chatAPI: chatAPI, roomID: roomID, options: options, logger: logger)
rooms[roomID] = room
return room
}
}

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
}
lawrence-forooghian marked this conversation as resolved.
Show resolved Hide resolved

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

// CHA-RL1e
await room.release()
}
}
2 changes: 1 addition & 1 deletion Tests/AblyChatTests/DefaultChatClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ struct DefaultChatClientTests {
// Then: Its `rooms` property returns an instance of DefaultRooms with the same realtime client and client options
let rooms = client.rooms

let defaultRooms = try #require(rooms as? DefaultRooms<DefaultRoomLifecycleManagerFactory>)
let defaultRooms = try #require(rooms as? DefaultRooms<DefaultRoomFactory>)
#expect(defaultRooms.testsOnly_realtime === realtime)
#expect(defaultRooms.clientOptions.isEqualForTestPurposes(options))
}
Expand Down
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
70 changes: 59 additions & 11 deletions Tests/AblyChatTests/DefaultRoomsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,32 @@ 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 {
// Given: an instance of DefaultRooms
let realtime = MockRealtime.create(channels: .init(channels: [
.init(name: "basketball::$chat::$chatMessages"),
]))
let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), lifecycleManagerFactory: MockRoomLifecycleManagerFactory())
let options = RoomOptions()
let roomToReturn = MockRoom(options: options)
let roomFactory = MockRoomFactory(room: roomToReturn)
let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: roomFactory)

// When: get(roomID:options:) is called
let roomID = "basketball"
let options = RoomOptions()
let room = try await rooms.get(roomID: roomID, options: options)

// Then: It returns a DefaultRoom instance that uses the same Realtime instance, with the given ID and options
let defaultRoom = try #require(room as? DefaultRoom<MockRoomLifecycleManagerFactory>)
#expect(defaultRoom.testsOnly_realtime === realtime)
#expect(defaultRoom.roomID == roomID)
#expect(defaultRoom.options == options)
// Then: It returns a room that uses the same Realtime instance, with the given ID and options
let mockRoom = try #require(room as? MockRoom)
#expect(mockRoom === roomToReturn)

let createRoomArguments = try #require(await roomFactory.createRoomArguments)
#expect(createRoomArguments.realtime === realtime)
#expect(createRoomArguments.roomID == roomID)
#expect(createRoomArguments.options == options)
}

// @spec CHA-RC1b
Expand All @@ -31,10 +38,11 @@ struct DefaultRoomsTests {
let realtime = MockRealtime.create(channels: .init(channels: [
.init(name: "basketball::$chat::$chatMessages"),
]))
let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), lifecycleManagerFactory: MockRoomLifecycleManagerFactory())
let options = RoomOptions()
let roomToReturn = MockRoom(options: options)
let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: MockRoomFactory(room: roomToReturn))

let roomID = "basketball"
let options = RoomOptions()
let firstRoom = try await rooms.get(roomID: roomID, options: options)

// When: get(roomID:options:) is called with the same room ID
Expand All @@ -51,10 +59,11 @@ struct DefaultRoomsTests {
let realtime = MockRealtime.create(channels: .init(channels: [
.init(name: "basketball::$chat::$chatMessages"),
]))
let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), lifecycleManagerFactory: MockRoomLifecycleManagerFactory())
let options = RoomOptions()
let roomToReturn = MockRoom(options: options)
let rooms = DefaultRooms(realtime: realtime, clientOptions: .init(), logger: TestLogger(), roomFactory: MockRoomFactory(room: roomToReturn))

let roomID = "basketball"
let options = RoomOptions()
_ = try await rooms.get(roomID: roomID, options: options)

// When: get(roomID:options:) is called with the same ID but different options
Expand All @@ -71,4 +80,43 @@ 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

// These two lines are convoluted because the #require macro has a hard time with stuff of type Bool? and emits warnings about ambiguity unless you jump through the hoops it tells you to
let hasExistingRoomAtMomentRoomReleaseCalled = await hasExistingRoomAtMomentRoomReleaseCalledStreamComponents.stream.first { _ in true }
#expect(try !#require(hasExistingRoomAtMomentRoomReleaseCalled as Bool?))

#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()
defer { mutex.unlock() }
_releaseArguments.append(name)
}

var releaseArguments: [String] {
let result: [String]
mutex.lock()
result = _releaseArguments
mutex.unlock()
return result
}
}
Loading