From 18ae5bd473ec0858e11f4860f41a1d5d17db9ac6 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sat, 1 Nov 2025 13:59:50 +0000 Subject: [PATCH 1/2] Remove transient disconnect handling from Connection. --- Sources/AblyChat/DefaultConnection.swift | 34 ++---------------------- 1 file changed, 2 insertions(+), 32 deletions(-) diff --git a/Sources/AblyChat/DefaultConnection.swift b/Sources/AblyChat/DefaultConnection.swift index 454330ea..efa5031c 100644 --- a/Sources/AblyChat/DefaultConnection.swift +++ b/Sources/AblyChat/DefaultConnection.swift @@ -2,7 +2,6 @@ import Ably internal final class DefaultConnection: Connection { private let realtime: any InternalRealtimeClientProtocol - private let timerManager = TimerManager(clock: SystemClock()) // (CHA-CS2a) The chat client must expose its current connection status. internal var status: ConnectionStatus { @@ -24,7 +23,7 @@ internal final class DefaultConnection: Connection { internal func onStatusChange(_ callback: @escaping @MainActor (ConnectionStatusChange) -> Void) -> some StatusSubscription { // (CHA-CS5) The chat client must monitor the underlying realtime connection for connection status changes. let eventListener = realtime.connection.on { [weak self] stateChange in - guard let self else { + guard self != nil else { return } let currentState = ConnectionStatus.fromRealtimeConnectionState(stateChange.current) @@ -41,35 +40,7 @@ internal final class DefaultConnection: Connection { retryIn: stateChange.retryIn, ) - let isTimerRunning = timerManager.hasRunningTask() - // (CHA-CS5a) The chat client must suppress transient disconnection events. It is not uncommon for Ably servers to perform connection shedding to balance load, or due to retiring. Clients should not need to concern themselves with transient events. - - // (CHA-CS5a2) If a transient disconnect timer is active and the realtime connection status changes to `DISCONNECTED` or `CONNECTING`, the library must not emit a status change. - if isTimerRunning, currentState == .disconnected || currentState == .connecting { - return - } - - // (CHA-CS5a3) If a transient disconnect timer is active and the realtime connections status changes to `CONNECTED`, `SUSPENDED` or `FAILED`, the library shall cancel the transient disconnect timer. The superseding status change shall be emitted. - if isTimerRunning, currentState == .connected || currentState == .suspended || currentState == .failed { - timerManager.cancelTimer() - callback(statusChange) - } - - // (CHA-CS5a1) If the realtime connection status transitions from `CONNECTED` to `DISCONNECTED`, the chat client connection status must not change. A 5 second transient disconnect timer shall be started. - if previousState == .connected, currentState == .disconnected, !isTimerRunning { - timerManager.setTimer(interval: 5.0) { [timerManager] in - // (CHA-CS5a4) If a transient disconnect timer expires the library shall emit a connection status change event. This event must contain the current status of of timer expiry, along with the original error that initiated the transient disconnect timer. - timerManager.cancelTimer() - callback(statusChange) - } - return - } - - if isTimerRunning { - timerManager.cancelTimer() - } - - // (CHA-CS5b) Not withstanding CHA-CS5a. If a connection state event is observed from the underlying realtime library, the client must emit a status change event. The current status of that event shall reflect the status change in the underlying realtime library, along with the accompanying error. + // (CHA-CS5c) The current status of that event shall reflect the status change in the underlying realtime library, along with the accompanying error. callback(statusChange) } @@ -77,7 +48,6 @@ internal final class DefaultConnection: Connection { guard let self else { return } - timerManager.cancelTimer() realtime.connection.off(eventListener) } } From 5311848e32f9e47361ce974c0286644fb06fabbd Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sat, 1 Nov 2025 14:00:00 +0000 Subject: [PATCH 2/2] Add tests for Connection. --- Sources/AblyChat/Connection.swift | 4 +- .../DefaultConnectionTests.swift | 150 ++++++++++++++++++ .../AblyChatTests/Mocks/MockConnection.swift | 42 ++++- 3 files changed, 188 insertions(+), 8 deletions(-) create mode 100644 Tests/AblyChatTests/DefaultConnectionTests.swift diff --git a/Sources/AblyChat/Connection.swift b/Sources/AblyChat/Connection.swift index 9d0a318d..6e998f94 100644 --- a/Sources/AblyChat/Connection.swift +++ b/Sources/AblyChat/Connection.swift @@ -108,7 +108,7 @@ public enum ConnectionStatus: Sendable { */ case failed - // (CHA-CS1g) The @CLOSING@ status is used when a user has called @close@ on the underlying Realtime client and it is attempting to close the connection with Ably. The library will not attempt to reconnect. + // (CHA-CS1g) The CLOSING status is used when a user has called close on the underlying Realtime client and it is attempting to close the connection with Ably. The library will not attempt to reconnect. /** * An explicit request by the developer to close the connection has been sent to the Ably service. @@ -117,7 +117,7 @@ public enum ConnectionStatus: Sendable { */ case closing - // (CHA-CS1h) The @CLOSED@ status is used when the @close@ call on the underlying Realtime client has succeeded, either via mutual agreement with the server or forced after a time out. The library will not attempt to reconnect. + // (CHA-CS1h) The CLOSED status is used when the close call on the underlying Realtime client has succeeded, either via mutual agreement with the server or forced after a time out. The library will not attempt to reconnect. /** * The connection has been explicitly closed by the client. diff --git a/Tests/AblyChatTests/DefaultConnectionTests.swift b/Tests/AblyChatTests/DefaultConnectionTests.swift new file mode 100644 index 00000000..9ca95049 --- /dev/null +++ b/Tests/AblyChatTests/DefaultConnectionTests.swift @@ -0,0 +1,150 @@ +import Ably +@testable import AblyChat +import Testing + +@MainActor +struct DefaultConnectionTests { + // MARK: - CHA-CS1: Connection Status Values + + // These specs are not marked as `[Testable]`, but lets have this basic check anyway: + // CHA-CS1a + // CHA-CS1b + // CHA-CS1c + // CHA-CS1d + // CHA-CS1e + // CHA-CS1f + // CHA-CS1g + // CHA-CS1h + @Test + func chatConnectionStatusReflectsAllRealtimeConnectionStates() async throws { + // Test all possible realtime connection state mappings to chat connection status + let testCases: [(ARTRealtimeConnectionState, ConnectionStatus, String)] = [ + // CHA-CS1a: INITIALIZED status + (.initialized, .initialized, "initialized"), + // CHA-CS1b: CONNECTING status + (.connecting, .connecting, "connecting"), + // CHA-CS1c: CONNECTED status + (.connected, .connected, "connected"), + // CHA-CS1d: DISCONNECTED status + (.disconnected, .disconnected, "disconnected"), + // CHA-CS1e: SUSPENDED status + (.suspended, .suspended, "suspended"), + // CHA-CS1f: FAILED status + (.failed, .failed, "failed"), + // CHA-CS1g: CLOSING status + (.closing, .closing, "closing"), + // CHA-CS1h: CLOSED status + (.closed, .closed, "closed"), + ] + + for (realtimeState, expectedChatStatus, description) in testCases { + // Given: A connection in a specific realtime state + let mockConnection = MockConnection(state: realtimeState) + let mockRealtime = MockRealtime(connection: mockConnection) + let connection = DefaultConnection(realtime: mockRealtime) + + // When: The connection status is checked + let status = connection.status + + // Then: Status should match the expected chat connection status + #expect(status == expectedChatStatus, "Realtime state \(description) should map to \(expectedChatStatus)") + } + } + + // MARK: - CHA-CS2: Exposing Connection Status and Error + + // @spec CHA-CS2a + // @spec CHA-CS2b + // @spec CHA-CS3 + @Test + func chatClientMustExposeItsCurrentStatusAndError() async throws { + // Given: An instance of ChatClient with initialized connection and no error + let options = ARTClientOptions(key: "fake:key") + options.autoConnect = false + let realtime = ARTRealtime(options: options) + let client = ChatClient(realtime: realtime, clientOptions: nil) + + // When: The connection status and error are checked + let status = client.connection.status + let error = client.connection.error + + // Then: Status should be initialized and error should be nil (CHA-CS3) + #expect(status == .initialized) + #expect(error == nil) + } + + // @spec CHA-CS2b + @Test + func chatClientMustExposeLatestError() async throws { + // Given: A connection with an error + let connectionError = ErrorInfo( + code: 40142, + message: "Connection failed", + statusCode: 401, + ) + let mockConnection = MockConnection(state: .failed, errorReason: connectionError) + let mockRealtime = MockRealtime(connection: mockConnection) + let connection = DefaultConnection(realtime: mockRealtime) + + // When: The error is checked + let error = connection.error + + // Then: The error should match the connection error + #expect(error?.code == connectionError.code) + #expect(error?.message == connectionError.message) + #expect(error?.statusCode == connectionError.statusCode) + } + + // MARK: - CHA-CS4: Observing Connection Status + + // @spec CHA-CS4a + // @spec CHA-CS4b + // @spec CHA-CS4c + // @spec CHA-CS4d + // @spec CHA-CS4e + // @spec CHA-CS5c - mocks are the same as for CHA-CS4, so CHA-CS5c is covered by this test + @Test + func clientsCanRegisterListenerForConnectionStatusEvents() async throws { + // Given: A connection and a listener + let mockConnection = MockConnection(state: .connecting) + let mockRealtime = MockRealtime(connection: mockConnection) + let connection = DefaultConnection(realtime: mockRealtime) + let connectionError = ErrorInfo( + code: 80003, + message: "Connection lost", + statusCode: 500, + ) + + var receivedStatusChanges: [ConnectionStatusChange] = [] + + // When: Register a listener (CHA-CS4d) + let subscription = connection.onStatusChange { statusChange in + receivedStatusChanges.append(statusChange) + } + + // And: Emit a state change + mockConnection.emit(.disconnected, event: .disconnected, error: connectionError) + + // Then: The listener should receive the event with correct information + #expect(receivedStatusChanges.count == 1) + + let statusChange = receivedStatusChanges[0] + // (CHA-CS4a) Contains newly entered connection status + #expect(statusChange.current == .disconnected) + // (CHA-CS4b) Contains previous connection status + #expect(statusChange.previous == .connecting) + // (CHA-CS4c) Contains connection error + #expect(statusChange.error?.code == connectionError.code) + #expect(statusChange.error?.message == connectionError.message) + + // When: Unregister the listener (CHA-CS4e) + subscription.off() + receivedStatusChanges.removeAll() + + // And: Emit a state change + mockConnection.emit(.disconnected, event: .disconnected) + + // Then: The listener should not receive any events + #expect(receivedStatusChanges.isEmpty) + } +} diff --git a/Tests/AblyChatTests/Mocks/MockConnection.swift b/Tests/AblyChatTests/Mocks/MockConnection.swift index 22f22522..a5e2bb5d 100644 --- a/Tests/AblyChatTests/Mocks/MockConnection.swift +++ b/Tests/AblyChatTests/Mocks/MockConnection.swift @@ -2,20 +2,50 @@ import Ably @testable import AblyChat final class MockConnection: InternalConnectionProtocol { - let state: ARTRealtimeConnectionState + var state: ARTRealtimeConnectionState - let errorReason: ErrorInfo? + var errorReason: ErrorInfo? + + private var listeners: [(ARTEventListener, @MainActor (ConnectionStateChange) -> Void)] = [] init(state: ARTRealtimeConnectionState = .initialized, errorReason: ErrorInfo? = nil) { self.state = state self.errorReason = errorReason } - func on(_: @escaping @MainActor (ConnectionStateChange) -> Void) -> ARTEventListener { - fatalError("Not implemented") + func on(_ callback: @escaping @MainActor (ConnectionStateChange) -> Void) -> ARTEventListener { + let listener = ARTEventListener() + listeners.append((listener, callback)) + return listener + } + + func off(_ listener: ARTEventListener) { + listeners.removeAll { $0.0 === listener } } - func off(_: ARTEventListener) { - fatalError("Not implemented") + // Helper method to emit state changes for testing + func emit( + _ newState: ARTRealtimeConnectionState, + event: ARTRealtimeConnectionEvent, + error: ErrorInfo? = nil, + retryIn: TimeInterval? = nil, + ) { + let previousState = state + state = newState + if let error { + errorReason = error + } + + let stateChange = ConnectionStateChange( + current: newState, + previous: previousState, + event: event, + reason: error, + retryIn: retryIn ?? 0, + ) + + for (_, callback) in listeners { + callback(stateChange) + } } }