From 983281a8d12b78edc966fcd2e0ada7e99b733bd1 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 29 Aug 2024 10:22:00 +0100 Subject: [PATCH] Implement the ability to fetch a room MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of #19. References to spec are based on [1] at commit aa7455d. I’ve decided to, for now, throw ably-cocoa’s ARTErrorInfo for consistency with JS; created #32 to revisit this later. We have decided (see [3]) that we’re going to try using actors as our mechanism for concurrency-safe management of mutable state. We accept that this will lead to more of the public API needing to be annotated as `async` (as has happened to Rooms.get here), which in some cases might lead to weird-looking API, and have chosen to accept this compromise in order to get the safety checking offered to us by the compiler, and because of developers’ aversion to writing "@unchecked Sendable". We might not have needed to make this compromise if we had access to Swift 6 / iOS 18’s Mutex type, which allows for synchronous management of mutable state in a way that the compiler is happy with. But, none of the decisions here need to be final; we can see how we feel about the API as it evolves and as our knowledge of the language grows. [1] https://github.com/ably/specification/pull/200 [2] https://github.com/ably/ably-cocoa/issues/1962 [3] https://github.com/ably-labs/ably-chat-swift/pull/33#discussion_r1736054462 --- Sources/AblyChat/Errors.swift | 67 +++++++++++++++++++++ Sources/AblyChat/Room.swift | 48 +++++++++++++++ Sources/AblyChat/RoomOptions.swift | 10 +-- Sources/AblyChat/Rooms.swift | 24 ++++++-- Tests/AblyChatTests/DefaultRoomsTests.swift | 64 ++++++++++++++++++++ Tests/AblyChatTests/Helpers/Helpers.swift | 15 +++++ 6 files changed, 219 insertions(+), 9 deletions(-) create mode 100644 Sources/AblyChat/Errors.swift create mode 100644 Tests/AblyChatTests/DefaultRoomsTests.swift create mode 100644 Tests/AblyChatTests/Helpers/Helpers.swift diff --git a/Sources/AblyChat/Errors.swift b/Sources/AblyChat/Errors.swift new file mode 100644 index 00000000..8a3070cf --- /dev/null +++ b/Sources/AblyChat/Errors.swift @@ -0,0 +1,67 @@ +import Ably + +/** + The error domain used for the ``Ably.ARTErrorInfo`` error instances thrown by the Ably Chat SDK. + + See ``ErrorCode`` for the possible ``ARTErrorInfo.code`` values. + */ +public let errorDomain = "AblyChatErrorDomain" + +/** + The error codes for errors in the ``errorDomain`` error domain. + */ +public enum ErrorCode: Int { + /// ``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 + case inconsistentRoomOptions = 1 + + /// The ``ARTErrorInfo.statusCode`` that should be returned for this error. + internal var statusCode: Int { + // TODO: These are currently a guess, revisit in https://github.com/ably-labs/ably-chat-swift/issues/32 + switch self { + case .inconsistentRoomOptions: + 400 + } + } +} + +/** + The errors thrown by the Chat SDK. + + This type exists in addition to ``ErrorCode`` to allow us to attach metadata which can be incorporated into the error’s `localizedDescription`. + */ +internal enum ChatError { + case inconsistentRoomOptions(requested: RoomOptions, existing: RoomOptions) + + /// The ``ARTErrorInfo.code`` that should be returned for this error. + internal var code: ErrorCode { + switch self { + case .inconsistentRoomOptions: + .inconsistentRoomOptions + } + } + + /// The ``ARTErrorInfo.localizedDescription`` that should be returned for this error. + internal var localizedDescription: String { + switch self { + case let .inconsistentRoomOptions(requested, existing): + "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:). Requested options: \(requested), existing options: \(existing)" + } + } +} + +internal extension ARTErrorInfo { + convenience init(chatError: ChatError) { + var userInfo: [String: Any] = [:] + // TODO: copied and pasted from implementation of -[ARTErrorInfo createWithCode:status:message:requestId:] because there’s no way to pass domain; revisit in https://github.com/ably-labs/ably-chat-swift/issues/32. Also the ARTErrorInfoStatusCode variable in ably-cocoa is not public. + userInfo["ARTErrorInfoStatusCode"] = chatError.code.statusCode + userInfo[NSLocalizedDescriptionKey] = chatError.localizedDescription + + self.init( + domain: errorDomain, + code: chatError.code.rawValue, + userInfo: userInfo + ) + } +} diff --git a/Sources/AblyChat/Room.swift b/Sources/AblyChat/Room.swift index 286821ad..f5155c4f 100644 --- a/Sources/AblyChat/Room.swift +++ b/Sources/AblyChat/Room.swift @@ -1,3 +1,5 @@ +import Ably + public protocol Room: AnyObject, Sendable { var roomID: String { get } var messages: any Messages { get } @@ -14,3 +16,49 @@ public protocol Room: AnyObject, Sendable { func detach() async throws var options: RoomOptions { get } } + +internal actor DefaultRoom: Room { + internal nonisolated let roomID: String + internal nonisolated let options: RoomOptions + + // Exposed for testing. + internal nonisolated let realtime: any ARTRealtimeProtocol + + internal init(realtime: any ARTRealtimeProtocol, roomID: String, options: RoomOptions) { + self.realtime = realtime + self.roomID = roomID + self.options = options + } + + public nonisolated var messages: any Messages { + fatalError("Not yet implemented") + } + + public nonisolated var presence: any Presence { + fatalError("Not yet implemented") + } + + public nonisolated var reactions: any RoomReactions { + fatalError("Not yet implemented") + } + + public nonisolated var typing: any Typing { + fatalError("Not yet implemented") + } + + public nonisolated var occupancy: any Occupancy { + fatalError("Not yet implemented") + } + + public nonisolated var status: any RoomStatus { + fatalError("Not yet implemented") + } + + public func attach() async throws { + fatalError("Not yet implemented") + } + + public func detach() async throws { + fatalError("Not yet implemented") + } +} diff --git a/Sources/AblyChat/RoomOptions.swift b/Sources/AblyChat/RoomOptions.swift index a927bfc8..ca7b5973 100644 --- a/Sources/AblyChat/RoomOptions.swift +++ b/Sources/AblyChat/RoomOptions.swift @@ -1,6 +1,6 @@ import Foundation -public struct RoomOptions: Sendable { +public struct RoomOptions: Sendable, Equatable { public var presence: PresenceOptions? public var typing: TypingOptions? public var reactions: RoomReactionsOptions? @@ -14,7 +14,7 @@ public struct RoomOptions: Sendable { } } -public struct PresenceOptions: Sendable { +public struct PresenceOptions: Sendable, Equatable { public var enter = true public var subscribe = true @@ -24,7 +24,7 @@ public struct PresenceOptions: Sendable { } } -public struct TypingOptions: Sendable { +public struct TypingOptions: Sendable, Equatable { public var timeout: TimeInterval = 10 public init(timeout: TimeInterval = 10) { @@ -32,10 +32,10 @@ public struct TypingOptions: Sendable { } } -public struct RoomReactionsOptions: Sendable { +public struct RoomReactionsOptions: Sendable, Equatable { public init() {} } -public struct OccupancyOptions: Sendable { +public struct OccupancyOptions: Sendable, Equatable { public init() {} } diff --git a/Sources/AblyChat/Rooms.swift b/Sources/AblyChat/Rooms.swift index c3e48e5b..9577a9ba 100644 --- a/Sources/AblyChat/Rooms.swift +++ b/Sources/AblyChat/Rooms.swift @@ -1,7 +1,7 @@ -import Ably +@preconcurrency import Ably public protocol Rooms: AnyObject, Sendable { - func get(roomID: String, options: RoomOptions) throws -> any Room + func get(roomID: String, options: RoomOptions) async throws -> any Room func release(roomID: String) async throws var clientOptions: ClientOptions { get } } @@ -11,13 +11,29 @@ internal actor DefaultRooms: Rooms { internal nonisolated let realtime: ARTRealtimeProtocol internal nonisolated let clientOptions: ClientOptions + /// The set of rooms, keyed by room ID. + private var rooms: [String: DefaultRoom] = [:] + internal init(realtime: ARTRealtimeProtocol, clientOptions: ClientOptions) { self.realtime = realtime self.clientOptions = clientOptions } - internal nonisolated func get(roomID _: String, options _: RoomOptions) throws -> any Room { - fatalError("Not yet implemented") + internal func get(roomID: String, options: RoomOptions) throws -> any Room { + // CHA-RC1b + if let existingRoom = rooms[roomID] { + if existingRoom.options != options { + throw ARTErrorInfo( + chatError: .inconsistentRoomOptions(requested: options, existing: existingRoom.options) + ) + } + + return existingRoom + } else { + let room = DefaultRoom(realtime: realtime, roomID: roomID, options: options) + rooms[roomID] = room + return room + } } internal func release(roomID _: String) async throws { diff --git a/Tests/AblyChatTests/DefaultRoomsTests.swift b/Tests/AblyChatTests/DefaultRoomsTests.swift new file mode 100644 index 00000000..31fc83dd --- /dev/null +++ b/Tests/AblyChatTests/DefaultRoomsTests.swift @@ -0,0 +1,64 @@ +@testable import AblyChat +import XCTest + +class DefaultRoomsTests: XCTestCase { + // @spec CHA-RC1a + func test_get_returnsRoomWithGivenID() async throws { + // Given: an instance of DefaultRooms + let realtime = MockRealtime.create() + let rooms = DefaultRooms(realtime: realtime, clientOptions: .init()) + + // 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 XCTUnwrap(room as? DefaultRoom) + XCTAssertIdentical(defaultRoom.realtime, realtime) + XCTAssertEqual(defaultRoom.roomID, roomID) + XCTAssertEqual(defaultRoom.options, options) + } + + // @spec CHA-RC1b + func test_get_returnsExistingRoomWithGivenID() async throws { + // Given: an instance of DefaultRooms, on which get(roomID:options:) has already been called with a given ID + let realtime = MockRealtime.create() + let rooms = DefaultRooms(realtime: realtime, clientOptions: .init()) + + 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 + let secondRoom = try await rooms.get(roomID: roomID, options: options) + + // Then: It returns the same room object + XCTAssertIdentical(secondRoom, firstRoom) + } + + // @spec CHA-RC1c + func test_get_throwsErrorWhenOptionsDoNotMatch() async throws { + // Given: an instance of DefaultRooms, on which get(roomID:options:) has already been called with a given ID and options + let realtime = MockRealtime.create() + let rooms = DefaultRooms(realtime: realtime, clientOptions: .init()) + + 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 + let differentOptions = RoomOptions(presence: .init(subscribe: false)) + + let caughtError: Error? + do { + _ = try await rooms.get(roomID: roomID, options: differentOptions) + caughtError = nil + } catch { + caughtError = error + } + + // Then: It throws an inconsistentRoomOptions error + try assertIsChatError(caughtError, withCode: .inconsistentRoomOptions) + } +} diff --git a/Tests/AblyChatTests/Helpers/Helpers.swift b/Tests/AblyChatTests/Helpers/Helpers.swift new file mode 100644 index 00000000..669c23ed --- /dev/null +++ b/Tests/AblyChatTests/Helpers/Helpers.swift @@ -0,0 +1,15 @@ +import Ably +@testable import AblyChat +import XCTest + +/** + Asserts that a given optional `Error` is an `ARTErrorInfo` in the chat error domain with a given code. + */ +func assertIsChatError(_ maybeError: (any Error)?, withCode code: AblyChat.ErrorCode, file: StaticString = #filePath, line: UInt = #line) throws { + let error = try XCTUnwrap(maybeError, "Expected an error", file: file, line: line) + let ablyError = try XCTUnwrap(error as? ARTErrorInfo, "Expected an ARTErrorInfo", file: file, line: line) + + XCTAssertEqual(ablyError.domain, AblyChat.errorDomain as String, file: file, line: line) + XCTAssertEqual(ablyError.code, code.rawValue, file: file, line: line) + XCTAssertEqual(ablyError.statusCode, code.statusCode, file: file, line: line) +}