diff --git a/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift index b5105b786..157e04b5c 100644 --- a/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift +++ b/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, diff --git a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift index a513f49e6..e0f3f9d7f 100644 --- a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift +++ b/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) diff --git a/ElementX/Sources/Mocks/ClientProxyMock.swift b/ElementX/Sources/Mocks/ClientProxyMock.swift index 4866f7ebb..a34a79401 100644 --- a/ElementX/Sources/Mocks/ClientProxyMock.swift +++ b/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") diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift index ce32a3806..c56e9480f 100644 --- a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift +++ b/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! - var homeserverReachabilityPublisher: CurrentValuePublisher { + var homeserverReachabilityPublisher: CurrentValuePublisher { get { return underlyingHomeserverReachabilityPublisher } set(value) { underlyingHomeserverReachabilityPublisher = value } } - nonisolated(unsafe) var underlyingHomeserverReachabilityPublisher: CurrentValuePublisher! - var homeserverConnectivityPublisher: CurrentValuePublisher { - get { return underlyingHomeserverConnectivityPublisher } - set(value) { underlyingHomeserverConnectivityPublisher = value } - } - nonisolated(unsafe) var underlyingHomeserverConnectivityPublisher: CurrentValuePublisher! + nonisolated(unsafe) var underlyingHomeserverReachabilityPublisher: CurrentValuePublisher! var userID: String { get { return underlyingUserID } set(value) { underlyingUserID = value } diff --git a/ElementX/Sources/Other/CurrentValuePublisher.swift b/ElementX/Sources/Other/CurrentValuePublisher.swift index b4cb6a4f5..c469aadac 100644 --- a/ElementX/Sources/Other/CurrentValuePublisher.swift +++ b/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: Publisher, @unchecked Sendable { private let subject: CurrentValueSubject + /// Retains the upstream subscription feeding ``subject`` when this publisher is derived via ``map(_:)``. + private let cancellable: AnyCancellable? - init(_ subject: CurrentValueSubject) { + init(_ subject: CurrentValueSubject, cancellable: AnyCancellable? = nil) { self.subject = subject + self.cancellable = cancellable } init(_ value: Output) { @@ -32,6 +35,14 @@ nonisolated struct CurrentValuePublisher: Publisher, @un } } +nonisolated extension CurrentValuePublisher where Failure == Never { + func map(_ transform: @escaping (Output) -> T) -> CurrentValuePublisher { + let subject = CurrentValueSubject(transform(value)) + let cancellable = sink { subject.send(transform($0)) } + return .init(subject, cancellable: cancellable) + } +} + nonisolated extension CurrentValueSubject { func asCurrentValuePublisher() -> CurrentValuePublisher { .init(self) diff --git a/ElementX/Sources/Other/NetworkMonitor/NetworkMonitorProtocol.swift b/ElementX/Sources/Other/NetworkMonitor/NetworkMonitorProtocol.swift index b8e92992d..0f1ae5b89 100644 --- a/ElementX/Sources/Other/NetworkMonitor/NetworkMonitorProtocol.swift +++ b/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 { get } } diff --git a/ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/SecureBackupLogoutConfirmationScreenCoordinator.swift b/ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/SecureBackupLogoutConfirmationScreenCoordinator.swift index f554afa55..83649834d 100644 --- a/ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/SecureBackupLogoutConfirmationScreenCoordinator.swift +++ b/ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/SecureBackupLogoutConfirmationScreenCoordinator.swift @@ -11,7 +11,7 @@ import SwiftUI struct SecureBackupLogoutConfirmationScreenCoordinatorParameters { let secureBackupController: SecureBackupControllerProtocol - let homeserverReachabilityPublisher: CurrentValuePublisher + let homeserverReachabilityPublisher: CurrentValuePublisher } enum SecureBackupLogoutConfirmationScreenCoordinatorAction { diff --git a/ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/SecureBackupLogoutConfirmationScreenViewModel.swift b/ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/SecureBackupLogoutConfirmationScreenViewModel.swift index 985915fe7..89c3d3cb4 100644 --- a/ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/SecureBackupLogoutConfirmationScreenViewModel.swift +++ b/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 + private let homeserverReachabilityPublisher: CurrentValuePublisher private let backupUploadStateSubject: CurrentValueSubject = .init(.waiting) @@ -28,7 +28,7 @@ class SecureBackupLogoutConfirmationScreenViewModel: SecureBackupLogoutConfirmat actionsSubject.eraseToAnyPublisher() } - init(secureBackupController: SecureBackupControllerProtocol, homeserverReachabilityPublisher: CurrentValuePublisher) { + init(secureBackupController: SecureBackupControllerProtocol, homeserverReachabilityPublisher: CurrentValuePublisher) { 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 } } diff --git a/ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/View/SecureBackupLogoutConfirmationScreen.swift b/ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/View/SecureBackupLogoutConfirmationScreen.swift index f8464233a..52d6051b3 100644 --- a/ElementX/Sources/Screens/SecureBackup/SecureBackupLogoutConfirmationScreen/View/SecureBackupLogoutConfirmationScreen.swift +++ b/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)) diff --git a/ElementX/Sources/Services/Client/ClientProxy.swift b/ElementX/Sources/Services/Client/ClientProxy.swift index ef7b21fa8..41b0288e2 100644 --- a/ElementX/Sources/Services/Client/ClientProxy.swift +++ b/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(.reachable) - var homeserverReachabilityPublisher: CurrentValuePublisher { + private let homeserverReachabilitySubject = CurrentValueSubject(.reachable) + var homeserverReachabilityPublisher: CurrentValuePublisher { 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(.reachable) - var homeserverConnectivityPublisher: CurrentValuePublisher { - homeserverConnectivitySubject.asCurrentValuePublisher() - } - private let timelineMediaVisibilitySubject = CurrentValueSubject(.always) var timelineMediaVisibilityPublisher: CurrentValuePublisher { 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) diff --git a/ElementX/Sources/Services/Client/ClientProxyProtocol.swift b/ElementX/Sources/Services/Client/ClientProxyProtocol.swift index 08e8a1af5..f2023f2d5 100644 --- a/ElementX/Sources/Services/Client/ClientProxyProtocol.swift +++ b/ElementX/Sources/Services/Client/ClientProxyProtocol.swift @@ -108,9 +108,7 @@ protocol ClientProxyProtocol: AnyObject { var verificationStatePublisher: CurrentValuePublisher { get } - var homeserverReachabilityPublisher: CurrentValuePublisher { get } - - var homeserverConnectivityPublisher: CurrentValuePublisher { get } + var homeserverReachabilityPublisher: CurrentValuePublisher { get } var userID: String { get } diff --git a/ElementX/Sources/Services/Media/Provider/MediaProvider.swift b/ElementX/Sources/Services/Media/Provider/MediaProvider.swift index 2b8a7ad46..208a11d5f 100644 --- a/ElementX/Sources/Services/Media/Provider/MediaProvider.swift +++ b/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? + private let homeserverReachabilityPublisher: CurrentValuePublisher? init(mediaLoader: MediaLoaderProtocol, imageCache: Kingfisher.ImageCache, - homeserverReachabilityPublisher: CurrentValuePublisher?) { + homeserverReachabilityPublisher: CurrentValuePublisher?) { self.mediaLoader = mediaLoader self.imageCache = imageCache self.homeserverReachabilityPublisher = homeserverReachabilityPublisher diff --git a/UnitTests/Sources/MediaProvider/MediaProviderTests.swift b/UnitTests/Sources/MediaProvider/MediaProviderTests.swift index 744102eec..32595a778 100644 --- a/UnitTests/Sources/MediaProvider/MediaProviderTests.swift +++ b/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(.reachable) + private var reachabilitySubject = CurrentValueSubject(.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: diff --git a/UnitTests/Sources/SecureBackupLogoutConfirmationScreenViewModelTests.swift b/UnitTests/Sources/SecureBackupLogoutConfirmationScreenViewModelTests.swift index 7284e2b16..1e6bac312 100644 --- a/UnitTests/Sources/SecureBackupLogoutConfirmationScreenViewModelTests.swift +++ b/UnitTests/Sources/SecureBackupLogoutConfirmationScreenViewModelTests.swift @@ -18,13 +18,13 @@ struct SecureBackupLogoutConfirmationScreenViewModelTests { } private var secureBackupController: SecureBackupControllerMock - private var reachabilitySubject: CurrentValueSubject + private var reachabilitySubject: CurrentValueSubject init() { secureBackupController = SecureBackupControllerMock() secureBackupController.keyBackupState = CurrentValueSubject(.enabled).asCurrentValuePublisher() - reachabilitySubject = CurrentValueSubject(.reachable) + reachabilitySubject = CurrentValueSubject(.reachable) viewModel = SecureBackupLogoutConfirmationScreenViewModel(secureBackupController: secureBackupController, homeserverReachabilityPublisher: reachabilitySubject.asCurrentValuePublisher()) diff --git a/UnitTests/Sources/UserSessionFlowCoordinatorTests.swift b/UnitTests/Sources/UserSessionFlowCoordinatorTests.swift index f3c54691f..6a00fba63 100644 --- a/UnitTests/Sources/UserSessionFlowCoordinatorTests.swift +++ b/UnitTests/Sources/UserSessionFlowCoordinatorTests.swift @@ -19,7 +19,7 @@ struct UserSessionFlowCoordinatorTests { private let stateMachineFactory = PublishedStateMachineFactory() private let networkReachabilitySubject: CurrentValueSubject = .init(.reachable) - private let homeserverReachabilitySubject: CurrentValueSubject = .init(.reachable) + private let homeserverReachabilitySubject: CurrentValueSubject = .init(.reachable) private var cancellables = Set() private var tabCoordinator: NavigationTabCoordinator? { @@ -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