Secure backup tweaks (#2122)

* Show the recovery section if key backup is anything **but** disabled

* Don't show an action on secure backup if its current state is unknown

* Do not show a badge on the home screen avatar while the recovery state is unknown

* Do not assume key backup is disabled when its state is unknown. If the state is unknown, check if a backup exists remotely.

* Dedupe remote backup state requests and handle task cancellations
This commit is contained in:
Stefan Ceriu
2023-11-21 10:59:10 +02:00
committed by GitHub
parent 291034b688
commit bec6cdac05
3 changed files with 43 additions and 5 deletions
@@ -85,7 +85,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
.sink { [weak self] recoveryKeyState in .sink { [weak self] recoveryKeyState in
guard let self, appSettings.chatBackupEnabled else { return } guard let self, appSettings.chatBackupEnabled else { return }
let requiresSecureBackupSetup = recoveryKeyState == .unknown || recoveryKeyState == .disabled || recoveryKeyState == .incomplete let requiresSecureBackupSetup = recoveryKeyState == .disabled || recoveryKeyState == .incomplete
state.showUserMenuBadge = requiresSecureBackupSetup state.showUserMenuBadge = requiresSecureBackupSetup
state.showSettingsMenuOptionBadge = requiresSecureBackupSetup state.showSettingsMenuOptionBadge = requiresSecureBackupSetup
@@ -23,7 +23,7 @@ struct SecureBackupScreen: View {
var body: some View { var body: some View {
Form { Form {
if context.viewState.keyBackupState != .enabled { if context.viewState.keyBackupState == .disabled {
keyBackupSection keyBackupSection
} else { } else {
keyBackupSection keyBackupSection
@@ -77,10 +77,12 @@ struct SecureBackupScreen: View {
ListRow(label: .plain(title: L10n.screenChatBackupKeyBackupActionDisable, role: .destructive), kind: .navigationLink { ListRow(label: .plain(title: L10n.screenChatBackupKeyBackupActionDisable, role: .destructive), kind: .navigationLink {
context.send(viewAction: .keyBackup) context.send(viewAction: .keyBackup)
}) })
case .disabled, .enabling, .unknown: case .disabled, .enabling:
ListRow(label: .plain(title: L10n.screenChatBackupKeyBackupActionEnable), kind: .navigationLink { ListRow(label: .plain(title: L10n.screenChatBackupKeyBackupActionEnable), kind: .navigationLink {
context.send(viewAction: .keyBackup) context.send(viewAction: .keyBackup)
}) })
case .unknown:
EmptyView()
} }
} }
@@ -27,6 +27,9 @@ class SecureBackupController: SecureBackupControllerProtocol {
private var backupStateListenerTaskHandle: TaskHandle? private var backupStateListenerTaskHandle: TaskHandle?
private var recoveryStateListenerTaskHandle: TaskHandle? private var recoveryStateListenerTaskHandle: TaskHandle?
/// Used to dedupe remote backup state requests
@CancellableTask private var remoteBackupStateTask: Task<Void, Error>?
var recoveryKeyState: CurrentValuePublisher<SecureBackupRecoveryKeyState, Never> { var recoveryKeyState: CurrentValuePublisher<SecureBackupRecoveryKeyState, Never> {
recoveryKeyStateSubject.asCurrentValuePublisher() recoveryKeyStateSubject.asCurrentValuePublisher()
} }
@@ -39,11 +42,18 @@ class SecureBackupController: SecureBackupControllerProtocol {
self.encryption = encryption self.encryption = encryption
backupStateListenerTaskHandle = encryption.backupStateListener(listener: SecureBackupControllerBackupStateListener { [weak self] state in backupStateListenerTaskHandle = encryption.backupStateListener(listener: SecureBackupControllerBackupStateListener { [weak self] state in
self?.keyBackupStateSubject.send(state.keyBackupState) guard let self else { return }
keyBackupStateSubject.send(state.keyBackupState)
if case .unknown = state {
updateBackupStateFromRemote()
}
}) })
recoveryStateListenerTaskHandle = encryption.recoveryStateListener(listener: SecureBackupRecoveryStateListener { [weak self] state in recoveryStateListenerTaskHandle = encryption.recoveryStateListener(listener: SecureBackupRecoveryStateListener { [weak self] state in
guard let self else { return } guard let self else { return }
switch state { switch state {
case .unknown: case .unknown:
recoveryKeyStateSubject.send(.unknown) recoveryKeyStateSubject.send(.unknown)
@@ -55,6 +65,8 @@ class SecureBackupController: SecureBackupControllerProtocol {
recoveryKeyStateSubject.send(.incomplete) recoveryKeyStateSubject.send(.incomplete)
} }
}) })
updateBackupStateFromRemote()
} }
func enable() async -> Result<Void, SecureBackupControllerError> { func enable() async -> Result<Void, SecureBackupControllerError> {
@@ -140,6 +152,30 @@ class SecureBackupController: SecureBackupControllerProtocol {
return .failure(.failedUploadingForBackup) return .failure(.failedUploadingForBackup)
} }
} }
// MARK: - Private
private func updateBackupStateFromRemote(retry: Bool = true) {
remoteBackupStateTask = Task {
do {
let backupExists = try await self.encryption.backupExistsOnServer()
if Task.isCancelled {
return
}
if !backupExists {
keyBackupStateSubject.send(.disabled)
}
} catch {
MXLog.error("Failed retrieving remote backup state with error: \(error)")
if retry {
updateBackupStateFromRemote(retry: false)
}
}
}
}
} }
private final class SecureBackupControllerBackupStateListener: BackupStateListener { private final class SecureBackupControllerBackupStateListener: BackupStateListener {
@@ -182,7 +218,7 @@ extension BackupState {
var keyBackupState: SecureBackupKeyBackupState { var keyBackupState: SecureBackupKeyBackupState {
switch self { switch self {
case .unknown: case .unknown:
return .disabled return .unknown
case .creating: case .creating:
return .enabling return .enabling
case .enabling: case .enabling: