Skip to content

Commit

Permalink
Rename public API for room lifecycle and status
Browse files Browse the repository at this point in the history
We:

- rename the RoomLifecycle enum to RoomLifecycleStatus (which makes more
  sense);
- rename the RoomStatus protocol to RoomLifecycle and rename its
  `current` property to `status` ("current" what?; it wasn’t clear, and
  was inconsistent with the core SDKs, which only use `current` for state
  _changes_)

The net effect is that instead of writing `room.status.current` you now
write `room.lifecycle.status`, which still seems pretty readable and
gives the benefits listed above.

This change has been agreed with the Chat team and the decision is
recorded in [1]; they will make this change in JS.

As part of this, I’ve also corrected references to room “state” to the
correct terminology of “status” (in [2] Andy explains why they used the
word “status”, choosing to reserve “state” for some larger composite
state).

Outstanding spec question about whether to rename the RoomInFailedState
error [3]; that can wait.

[1] https://ably.atlassian.net/wiki/spaces/CHA/pages/3307405320/SDK+Divergence+Decision+Log
[2] #73 (comment)
[3] https://github.com/ably/specification/pull/200/files#r1783542331
  • Loading branch information
lawrence-forooghian committed Oct 1, 2024
1 parent be38b87 commit 64c0315
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 94 deletions.
14 changes: 7 additions & 7 deletions Sources/AblyChat/Room.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public protocol Room: AnyObject, Sendable {
var typing: any Typing { get }
// To access this property if occupancy is not enabled for the room is a programmer error, and will lead to `fatalError` being called.
var occupancy: any Occupancy { get }
var status: any RoomStatus { get }
var lifecycle: any RoomLifecycle { get }
func attach() async throws
func detach() async throws
var options: RoomOptions { get }
Expand All @@ -30,15 +30,15 @@ internal actor DefaultRoom: Room {
}
#endif

private let _status: DefaultRoomStatus
private let _lifecycle: DefaultRoomLifecycle
private let logger: InternalLogger

internal init(realtime: RealtimeClient, roomID: String, options: RoomOptions, logger: InternalLogger) {
self.realtime = realtime
self.roomID = roomID
self.options = options
self.logger = logger
_status = .init(logger: logger)
_lifecycle = .init(logger: logger)
}

public nonisolated var messages: any Messages {
Expand All @@ -61,8 +61,8 @@ internal actor DefaultRoom: Room {
fatalError("Not yet implemented")
}

internal nonisolated var status: any RoomStatus {
_status
internal nonisolated var lifecycle: any RoomLifecycle {
_lifecycle
}

/// Fetches the channels that contribute to this room.
Expand All @@ -85,7 +85,7 @@ internal actor DefaultRoom: Room {
throw error
}
}
await _status.transition(to: .attached)
await _lifecycle.transition(to: .attached)
}

public func detach() async throws {
Expand All @@ -97,6 +97,6 @@ internal actor DefaultRoom: Room {
throw error
}
}
await _status.transition(to: .detached)
await _lifecycle.transition(to: .detached)
}
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import Ably

public protocol RoomStatus: AnyObject, Sendable {
var current: RoomLifecycle { get async }
public protocol RoomLifecycle: AnyObject, Sendable {
var status: RoomStatus { get async }
// TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap
var error: ARTErrorInfo? { get async }
func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription<RoomStatusChange>
}

public enum RoomLifecycle: Sendable {
public enum RoomStatus: Sendable {
case initialized
case attaching
case attached
Expand All @@ -20,20 +20,20 @@ public enum RoomLifecycle: Sendable {
}

public struct RoomStatusChange: Sendable {
public var current: RoomLifecycle
public var previous: RoomLifecycle
public var current: RoomStatus
public var previous: RoomStatus
// TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap
public var error: ARTErrorInfo?

public init(current: RoomLifecycle, previous: RoomLifecycle, error: ARTErrorInfo? = nil) {
public init(current: RoomStatus, previous: RoomStatus, error: ARTErrorInfo? = nil) {
self.current = current
self.previous = previous
self.error = error
}
}

internal actor DefaultRoomStatus: RoomStatus {
internal private(set) var current: RoomLifecycle = .initialized
internal actor DefaultRoomLifecycle: RoomLifecycle {
internal private(set) var status: RoomStatus = .initialized
// TODO: populate this (https://github.com/ably-labs/ably-chat-swift/issues/28)
internal private(set) var error: ARTErrorInfo?

Expand All @@ -52,11 +52,11 @@ internal actor DefaultRoomStatus: RoomStatus {
return subscription
}

/// Sets ``current`` to the given state, and emits a status change to all subscribers added via ``onChange(bufferingPolicy:)``.
internal func transition(to newState: RoomLifecycle) {
logger.log(message: "Transitioning to \(newState)", level: .debug)
let statusChange = RoomStatusChange(current: newState, previous: current)
current = newState
/// Sets ``status`` to the given status, and emits a status change to all subscribers added via ``onChange(bufferingPolicy:)``.
internal func transition(to newStatus: RoomStatus) {
logger.log(message: "Transitioning to \(newStatus)", level: .debug)
let statusChange = RoomStatusChange(current: newStatus, previous: status)
status = newStatus
for subscription in subscriptions {
subscription.emit(statusChange)
}
Expand Down
8 changes: 4 additions & 4 deletions Sources/AblyChat/RoomLifecycleManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ internal actor RoomLifecycleManager<Channel: RoomLifecycleContributorChannel> {
internal var channel: Channel
}

internal private(set) var current: RoomLifecycle
internal private(set) var current: RoomStatus
internal private(set) var error: ARTErrorInfo?

private let logger: InternalLogger
Expand All @@ -44,7 +44,7 @@ internal actor RoomLifecycleManager<Channel: RoomLifecycleContributorChannel> {

#if DEBUG
internal init(
testsOnly_current current: RoomLifecycle? = nil,
testsOnly_current current: RoomStatus? = nil,
contributors: [Contributor],
logger: InternalLogger,
clock: SimpleClock
Expand All @@ -59,7 +59,7 @@ internal actor RoomLifecycleManager<Channel: RoomLifecycleContributorChannel> {
#endif

private init(
current: RoomLifecycle?,
current: RoomStatus?,
contributors: [Contributor],
logger: InternalLogger,
clock: SimpleClock
Expand All @@ -80,7 +80,7 @@ internal actor RoomLifecycleManager<Channel: RoomLifecycleContributorChannel> {
}

/// Updates ``current`` and ``error`` and emits a status change event.
private func changeStatus(to new: RoomLifecycle, error: ARTErrorInfo? = nil) {
private func changeStatus(to new: RoomStatus, error: ARTErrorInfo? = nil) {
logger.log(message: "Transitioning from \(current) to \(new), error \(String(describing: error))", level: .info)
let previous = current
current = new
Expand Down
41 changes: 41 additions & 0 deletions Tests/AblyChatTests/DefaultRoomLifecycleTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
@testable import AblyChat
import Testing

struct DefaultRoomLifecycleTests {
@Test
func current_startsAsInitialized() async {
let lifecycle = DefaultRoomLifecycle(logger: TestLogger())
#expect(await lifecycle.status == .initialized)
}

@Test()
func error_startsAsNil() async {
let lifecycle = DefaultRoomLifecycle(logger: TestLogger())
#expect(await lifecycle.error == nil)
}

@Test
func transition() async throws {
// Given: A DefaultRoomLifecycle
let lifecycle = DefaultRoomLifecycle(logger: TestLogger())
let originalStatus = await lifecycle.status
let newStatus = RoomStatus.attached // arbitrary

let subscription1 = await lifecycle.onChange(bufferingPolicy: .unbounded)
let subscription2 = await lifecycle.onChange(bufferingPolicy: .unbounded)

async let statusChange1 = subscription1.first { $0.current == newStatus }
async let statusChange2 = subscription2.first { $0.current == newStatus }

// When: transition(to:) is called
await lifecycle.transition(to: newStatus)

// Then: It emits a status change to all subscribers added via onChange(bufferingPolicy:), and updates its `status` property to the new status
for statusChange in try await [#require(statusChange1), #require(statusChange2)] {
#expect(statusChange.previous == originalStatus)
#expect(statusChange.current == newStatus)
}

#expect(await lifecycle.status == .attached)
}
}
41 changes: 0 additions & 41 deletions Tests/AblyChatTests/DefaultRoomStatusTests.swift

This file was deleted.

8 changes: 4 additions & 4 deletions Tests/AblyChatTests/DefaultRoomTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ struct DefaultRoomTests {
let realtime = MockRealtime.create(channels: channels)
let room = DefaultRoom(realtime: realtime, roomID: "basketball", options: .init(), logger: TestLogger())

let subscription = await room.status.onChange(bufferingPolicy: .unbounded)
let subscription = await room.lifecycle.onChange(bufferingPolicy: .unbounded)
async let attachedStatusChange = subscription.first { $0.current == .attached }

// When: `attach` is called on the room
Expand All @@ -30,7 +30,7 @@ struct DefaultRoomTests {
#expect(channel.attachCallCounter.isNonZero)
}

#expect(await room.status.current == .attached)
#expect(await room.lifecycle.status == .attached)
#expect(try #require(await attachedStatusChange).current == .attached)
}

Expand Down Expand Up @@ -81,7 +81,7 @@ struct DefaultRoomTests {
let realtime = MockRealtime.create(channels: channels)
let room = DefaultRoom(realtime: realtime, roomID: "basketball", options: .init(), logger: TestLogger())

let subscription = await room.status.onChange(bufferingPolicy: .unbounded)
let subscription = await room.lifecycle.onChange(bufferingPolicy: .unbounded)
async let detachedStatusChange = subscription.first { $0.current == .detached }

// When: `detach` is called on the room
Expand All @@ -92,7 +92,7 @@ struct DefaultRoomTests {
#expect(channel.detachCallCounter.isNonZero)
}

#expect(await room.status.current == .detached)
#expect(await room.lifecycle.status == .detached)
#expect(try #require(await detachedStatusChange).current == .detached)
}

Expand Down
Loading

0 comments on commit 64c0315

Please sign in to comment.