Switch to a custom HomeserverReachability instead of different publishers

ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift

@@ -275,7 +275,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
         let mediaProvider = authenticationService.classicAppAccount.map { account in
             MediaProvider(mediaLoader: ClassicAppMediaLoader(classicAppAccount: account),
                           imageCache: .onlyInMemory,
-                          homeserverReachabilityPublisher: appMediator.networkMonitor.reachabilityPublisher) // Close enough approximation
+                          homeserverReachabilityPublisher: appMediator.networkMonitor.reachabilityPublisher.map(HomeserverReachability.init)) // Close enough approximation
         }
         
         let parameters = AuthenticationStartScreenParameters(authenticationService: authenticationService,

ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift

@@ -285,7 +285,7 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
             .store(in: &cancellables)
         
         let reachabilityNotificationID = "io.element.elementx.reachability.notification"
-        userSession.clientProxy.homeserverConnectivityPublisher.removeDuplicates()
+        userSession.clientProxy.homeserverReachabilityPublisher.removeDuplicates()
             .combineLatest(flowParameters.appMediator.networkMonitor.reachabilityPublisher.removeDuplicates())
             .receive(on: DispatchQueue.main)
             .sink { [weak self] homeserverReachability, networkReachability in
@@ -293,16 +293,17 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
                 
                 guard let self else { return }
                 switch (networkReachability, homeserverReachability) {
-                case (.reachable, .reachable):
-                    flowParameters.userIndicatorController.retractIndicatorWithId(reachabilityNotificationID)
-                case (.reachable, .unreachable):
-                    flowParameters.userIndicatorController.submitIndicator(.init(id: reachabilityNotificationID,
-                                                                                 title: L10n.commonServerUnreachable,
-                                                                                 persistent: true))
                 case (.unreachable, _):
                     flowParameters.userIndicatorController.submitIndicator(.init(id: reachabilityNotificationID,
                                                                                  title: L10n.commonOffline,
                                                                                  persistent: true))
+                case (.reachable, .unreachable):
+                    flowParameters.userIndicatorController.submitIndicator(.init(id: reachabilityNotificationID,
+                                                                                 title: L10n.commonServerUnreachable,
+                                                                                 persistent: true))
+                // Don't alarm the user while we've intentionally suspended the client.
+                case (.reachable, .reachable), (.reachable, .suspended):
+                    flowParameters.userIndicatorController.retractIndicatorWithId(reachabilityNotificationID)
                 }
             }
             .store(in: &cancellables)

ElementX/Sources/Mocks/ClientProxyMock.swift

@@ -63,7 +63,6 @@ extension ClientProxyMock {
         loadingStatePublisher = .init(.notLoading)
         verificationStatePublisher = .init(.unknown)
         homeserverReachabilityPublisher = .init(.reachable)
-        homeserverConnectivityPublisher = .init(.reachable)
         
         userAvatarURLPublisher = .init(nil)
         userDisplayNamePublisher = .init("User display name")

ElementX/Sources/Mocks/Generated/GeneratedMocks.swift

@@ -2227,16 +2227,11 @@ nonisolated class ClientProxyMock: ClientProxyProtocol, @unchecked Sendable {
         set(value) { underlyingVerificationStatePublisher = value }
     }
     nonisolated(unsafe) var underlyingVerificationStatePublisher: CurrentValuePublisher<SessionVerificationState, Never>!
-    var homeserverReachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never> {
+    var homeserverReachabilityPublisher: CurrentValuePublisher<HomeserverReachability, Never> {
         get { return underlyingHomeserverReachabilityPublisher }
         set(value) { underlyingHomeserverReachabilityPublisher = value }
     }
-    nonisolated(unsafe) var underlyingHomeserverReachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never>!
-    var homeserverConnectivityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never> {
-        get { return underlyingHomeserverConnectivityPublisher }
-        set(value) { underlyingHomeserverConnectivityPublisher = value }
-    }
-    nonisolated(unsafe) var underlyingHomeserverConnectivityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never>!
+    nonisolated(unsafe) var underlyingHomeserverReachabilityPublisher: CurrentValuePublisher<HomeserverReachability, Never>!
     var userID: String {
         get { return underlyingUserID }
         set(value) { underlyingUserID = value }

ElementX/Sources/Other/CurrentValuePublisher.swift

@@ -14,9 +14,12 @@ import Combine
 /// `CurrentValueSubject` is documented as thread-safe but is not formally `Sendable`, hence `@unchecked`.
 nonisolated struct CurrentValuePublisher<Output, Failure: Error>: Publisher, @unchecked Sendable {
     private let subject: CurrentValueSubject<Output, Failure>
+    /// Retains the upstream subscription feeding ``subject`` when this publisher is derived via ``map(_:)``.
+    private let cancellable: AnyCancellable?
     
-    init(_ subject: CurrentValueSubject<Output, Failure>) {
+    init(_ subject: CurrentValueSubject<Output, Failure>, cancellable: AnyCancellable? = nil) {
         self.subject = subject
+        self.cancellable = cancellable
     }
     
     init(_ value: Output) {
@@ -32,6 +35,14 @@ nonisolated struct CurrentValuePublisher<Output, Failure: Error>: Publisher, @un
     }
 }
 
+nonisolated extension CurrentValuePublisher where Failure == Never {
+    func map<T>(_ transform: @escaping (Output) -> T) -> CurrentValuePublisher<T, Never> {
+        let subject = CurrentValueSubject<T, Never>(transform(value))
+        let cancellable = sink { subject.send(transform($0)) }
+        return .init(subject, cancellable: cancellable)
+    }
+}
+
 nonisolated extension CurrentValueSubject {
     func asCurrentValuePublisher() -> CurrentValuePublisher<Output, Failure> {
         .init(self)

ElementX/Sources/Other/NetworkMonitor/NetworkMonitorProtocol.swift

@@ -13,6 +13,19 @@ enum NetworkMonitorReachability {
     case unreachable
 }
 
+enum HomeserverReachability {
+    /// The store is open and the homeserver is reachable.
+    case reachable
+    /// The homeserver can't be reached even though we're trying to sync (i.e. offline).
+    case unreachable
+    /// The client is suspended, so the store is closed and we're not trying to reach the homeserver.
+    case suspended
+    
+    init(_ reachability: NetworkMonitorReachability) {
+        self = reachability == .reachable ? .reachable : .unreachable
+    }
+}
+
 protocol NetworkMonitorProtocol {
     var reachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never> { get }
 }

ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/SecureBackupLogoutConfirmationScreenCoordinator.swift

@@ -11,7 +11,7 @@ import SwiftUI
 
 struct SecureBackupLogoutConfirmationScreenCoordinatorParameters {
     let secureBackupController: SecureBackupControllerProtocol
-    let homeserverReachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never>
+    let homeserverReachabilityPublisher: CurrentValuePublisher<HomeserverReachability, Never>
 }
 
 enum SecureBackupLogoutConfirmationScreenCoordinatorAction {

ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/SecureBackupLogoutConfirmationScreenViewModel.swift

@@ -13,7 +13,7 @@ typealias SecureBackupLogoutConfirmationScreenViewModelType = StateStoreViewMode
 
 class SecureBackupLogoutConfirmationScreenViewModel: SecureBackupLogoutConfirmationScreenViewModelType, SecureBackupLogoutConfirmationScreenViewModelProtocol {
     private let secureBackupController: SecureBackupControllerProtocol
-    private let homeserverReachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never>
+    private let homeserverReachabilityPublisher: CurrentValuePublisher<HomeserverReachability, Never>
     
     private let backupUploadStateSubject: CurrentValueSubject<SecureBackupSteadyState, Never> = .init(.waiting)
     
@@ -28,7 +28,7 @@ class SecureBackupLogoutConfirmationScreenViewModel: SecureBackupLogoutConfirmat
         actionsSubject.eraseToAnyPublisher()
     }
     
-    init(secureBackupController: SecureBackupControllerProtocol, homeserverReachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never>) {
+    init(secureBackupController: SecureBackupControllerProtocol, homeserverReachabilityPublisher: CurrentValuePublisher<HomeserverReachability, Never>) {
         self.secureBackupController = secureBackupController
         self.homeserverReachabilityPublisher = homeserverReachabilityPublisher
         
@@ -90,7 +90,7 @@ class SecureBackupLogoutConfirmationScreenViewModel: SecureBackupLogoutConfirmat
         }
     }
     
-    private func updateMode(backupState: SecureBackupSteadyState, reachability: NetworkMonitorReachability) {
+    private func updateMode(backupState: SecureBackupSteadyState, reachability: HomeserverReachability) {
         switch (backupState, reachability) {
         case (.waiting, .reachable):
             state.mode = .waitingToStart(hasStalled: false)
@@ -101,7 +101,7 @@ class SecureBackupLogoutConfirmationScreenViewModel: SecureBackupLogoutConfirmat
             break // Nothing to do here, it will be handled with the result.
         case (.done, .reachable):
             state.mode = .backupOngoing(progress: 1.0)
-        case (_, .unreachable):
+        case (_, .unreachable), (_, .suspended):
             state.mode = .offline
         }
     }

ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/View/SecureBackupLogoutConfirmationScreen.swift

@@ -160,7 +160,7 @@ struct SecureBackupLogoutConfirmationScreen_Previews: PreviewProvider, TestableP
             return .success(())
         }
         
-        let reachability: NetworkMonitorReachability = mode == .offline ? .unreachable : .reachable
+        let reachability: HomeserverReachability = mode == .offline ? .unreachable : .reachable
         
         let viewModel = SecureBackupLogoutConfirmationScreenViewModel(secureBackupController: secureBackupController,
                                                                       homeserverReachabilityPublisher: .init(reachability))

ElementX/Sources/Services/Client/ClientProxy.swift

@@ -180,21 +180,11 @@ class ClientProxy: ClientProxyProtocol {
         verificationStateSubject.asCurrentValuePublisher()
     }
     
-    /// Reports unreachable while suspended (the store is closed) as well as when offline. Use this to gate
-    /// work that needs an open store, such as media loading.
-    private let homeserverReachabilitySubject = CurrentValueSubject<NetworkMonitorReachability, Never>(.reachable)
-    var homeserverReachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never> {
+    private let homeserverReachabilitySubject = CurrentValueSubject<HomeserverReachability, Never>(.reachable)
+    var homeserverReachabilityPublisher: CurrentValuePublisher<HomeserverReachability, Never> {
         homeserverReachabilitySubject.asCurrentValuePublisher()
     }
     
-    /// The homeserver's reachability derived from the sync service alone, i.e. it stays reachable while the
-    /// client is suspended. Use this for user-facing messaging so we don't claim the server is unreachable
-    /// when we've simply paused syncing.
-    private let homeserverConnectivitySubject = CurrentValueSubject<NetworkMonitorReachability, Never>(.reachable)
-    var homeserverConnectivityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never> {
-        homeserverConnectivitySubject.asCurrentValuePublisher()
-    }
-    
     private let timelineMediaVisibilitySubject = CurrentValueSubject<TimelineMediaVisibility, Never>(.always)
     var timelineMediaVisibilityPublisher: CurrentValuePublisher<TimelineMediaVisibility, Never> {
         timelineMediaVisibilitySubject.asCurrentValuePublisher()
@@ -1210,12 +1200,12 @@ class ClientProxy: ClientProxyProtocol {
     }
     
     private func updateHomeserverReachability() {
-        homeserverConnectivitySubject.send(syncServiceState == .offline ? .unreachable : .reachable)
-        
-        let reachability: NetworkMonitorReachability = if desiredServiceState == .running, syncServiceState != .offline {
-            .reachable
-        } else {
+        let reachability: HomeserverReachability = if desiredServiceState != .running {
+            .suspended
+        } else if syncServiceState == .offline {
             .unreachable
+        } else {
+            .reachable
         }
         
         homeserverReachabilitySubject.send(reachability)

ElementX/Sources/Services/Client/ClientProxyProtocol.swift

@@ -108,9 +108,7 @@ protocol ClientProxyProtocol: AnyObject {
     
     var verificationStatePublisher: CurrentValuePublisher<SessionVerificationState, Never> { get }
     
-    var homeserverReachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never> { get }
-    
-    var homeserverConnectivityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never> { get }
+    var homeserverReachabilityPublisher: CurrentValuePublisher<HomeserverReachability, Never> { get }
     
     var userID: String { get }
     

ElementX/Sources/Services/Media/Provider/MediaProvider.swift

@@ -13,11 +13,11 @@ import UIKit
 nonisolated struct MediaProvider: MediaProviderProtocol {
     private let mediaLoader: MediaLoaderProtocol
     private let imageCache: Kingfisher.ImageCache
-    private let homeserverReachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never>?
+    private let homeserverReachabilityPublisher: CurrentValuePublisher<HomeserverReachability, Never>?
     
     init(mediaLoader: MediaLoaderProtocol,
          imageCache: Kingfisher.ImageCache,
-         homeserverReachabilityPublisher: CurrentValuePublisher<NetworkMonitorReachability, Never>?) {
+         homeserverReachabilityPublisher: CurrentValuePublisher<HomeserverReachability, Never>?) {
         self.mediaLoader = mediaLoader
         self.imageCache = imageCache
         self.homeserverReachabilityPublisher = homeserverReachabilityPublisher

UnitTests/Sources/MediaProvider/MediaProviderTests.swift

@@ -16,7 +16,7 @@ import Testing
 struct MediaProviderTests {
     private var mediaLoader: MediaLoaderMock
     private var imageCache: MockImageCache
-    private var reachabilitySubject = CurrentValueSubject<NetworkMonitorReachability, Never>(.reachable)
+    private var reachabilitySubject = CurrentValueSubject<HomeserverReachability, Never>(.reachable)
     
     var mediaProvider: MediaProvider!
     
@@ -40,7 +40,7 @@ struct MediaProviderTests {
         
         mediaLoader.loadMediaContentForSourceClosure = { [reachabilitySubject] _ in
             switch reachabilitySubject.value {
-            case .unreachable:
+            case .unreachable, .suspended:
                 reachabilitySubject.send(.reachable)
                 throw MediaProviderTestsError.error
             case .reachable:

UnitTests/Sources/SecureBackupLogoutConfirmationScreenViewModelTests.swift

@@ -18,13 +18,13 @@ struct SecureBackupLogoutConfirmationScreenViewModelTests {
     }
     
     private var secureBackupController: SecureBackupControllerMock
-    private var reachabilitySubject: CurrentValueSubject<NetworkMonitorReachability, Never>
+    private var reachabilitySubject: CurrentValueSubject<HomeserverReachability, Never>
     
     init() {
         secureBackupController = SecureBackupControllerMock()
         secureBackupController.keyBackupState = CurrentValueSubject<SecureBackupKeyBackupState, Never>(.enabled).asCurrentValuePublisher()
         
-        reachabilitySubject = CurrentValueSubject<NetworkMonitorReachability, Never>(.reachable)
+        reachabilitySubject = CurrentValueSubject<HomeserverReachability, Never>(.reachable)
         
         viewModel = SecureBackupLogoutConfirmationScreenViewModel(secureBackupController: secureBackupController,
                                                                   homeserverReachabilityPublisher: reachabilitySubject.asCurrentValuePublisher())

UnitTests/Sources/UserSessionFlowCoordinatorTests.swift

@@ -19,7 +19,7 @@ struct UserSessionFlowCoordinatorTests {
     private let stateMachineFactory = PublishedStateMachineFactory()
     
     private let networkReachabilitySubject: CurrentValueSubject<NetworkMonitorReachability, Never> = .init(.reachable)
-    private let homeserverReachabilitySubject: CurrentValueSubject<NetworkMonitorReachability, Never> = .init(.reachable)
+    private let homeserverReachabilitySubject: CurrentValueSubject<HomeserverReachability, Never> = .init(.reachable)
     private var cancellables = Set<AnyCancellable>()
     
     private var tabCoordinator: NavigationTabCoordinator<UserSessionFlowCoordinator.HomeTab>? {
@@ -228,6 +228,14 @@ struct UserSessionFlowCoordinatorTests {
         // Then the indicator should be hidden now as everything is back to normal
         #expect(userIndicatorController.submitIndicatorDelayCallsCount == 3)
         #expect(retractReachabilityIndicatorCallsCount == 2)
+        
+        // When the client is suspended.
+        homeserverReachabilitySubject.send(.suspended)
+        try await Task.sleep(for: .milliseconds(100))
+        
+        // Then no unreachable indicator should be shown as the pause is intentional.
+        #expect(userIndicatorController.submitIndicatorDelayCallsCount == 3)
+        #expect(retractReachabilityIndicatorCallsCount == 3)
     }
     
     // MARK: - Helpers