SecureBackup: improve error flow when backup state cannot be retrieved, and add tests.

features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/root/SecureBackupRootEvents.kt

@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.features.securebackup.impl.root
+
+sealed interface SecureBackupRootEvents {
+    data object RetryKeyBackupState : SecureBackupRootEvents
+}

features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/root/SecureBackupRootPresenter.kt

@@ -18,18 +18,23 @@ package io.element.android.features.securebackup.impl.root
 
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
+import androidx.compose.runtime.MutableState
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
-import androidx.compose.runtime.setValue
+import androidx.compose.runtime.rememberCoroutineScope
 import io.element.android.features.securebackup.impl.loggerTagRoot
+import io.element.android.libraries.architecture.Async
 import io.element.android.libraries.architecture.Presenter
+import io.element.android.libraries.architecture.runCatchingUpdatingState
 import io.element.android.libraries.core.meta.BuildMeta
 import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher
 import io.element.android.libraries.designsystem.utils.snackbar.collectSnackbarMessageAsState
 import io.element.android.libraries.matrix.api.encryption.BackupState
 import io.element.android.libraries.matrix.api.encryption.EncryptionService
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.launch
 import timber.log.Timber
 import javax.inject.Inject
 
@@ -41,6 +46,7 @@ class SecureBackupRootPresenter @Inject constructor(
 
     @Composable
     override fun present(): SecureBackupRootState {
+        val localCoroutineScope = rememberCoroutineScope()
         val snackbarMessage by snackbarDispatcher.collectSnackbarMessageAsState()
 
         val backupState by encryptionService.backupStateStateFlow.collectAsState()
@@ -49,23 +55,33 @@ class SecureBackupRootPresenter @Inject constructor(
         Timber.tag(loggerTagRoot.value).d("backupState: $backupState")
         Timber.tag(loggerTagRoot.value).d("recoveryState: $recoveryState")
 
-        var doesBackupExistOnServer: Boolean? by remember { mutableStateOf(null) }
+        val doesBackupExistOnServerAction: MutableState<Async<Boolean>> = remember { mutableStateOf(Async.Uninitialized) }
 
         LaunchedEffect(backupState) {
-            doesBackupExistOnServer = if (backupState == BackupState.UNKNOWN) {
+            if (backupState == BackupState.UNKNOWN) {
-                encryptionService.doesBackupExistOnServer().getOrNull() == true
+                getKeyBackupStatus(doesBackupExistOnServerAction)
-            } else {
+            }
-                // The value will not be used when the backupState is not UNKNOWN.
+        }
-                false
+
+        fun handleEvents(event: SecureBackupRootEvents) {
+            when (event) {
+                SecureBackupRootEvents.RetryKeyBackupState -> localCoroutineScope.getKeyBackupStatus(doesBackupExistOnServerAction)
             }
         }
 
         return SecureBackupRootState(
             backupState = backupState,
-            doesBackupExistOnServer = doesBackupExistOnServer,
+            doesBackupExistOnServer = doesBackupExistOnServerAction.value,
             recoveryState = recoveryState,
             appName = buildMeta.applicationName,
             snackbarMessage = snackbarMessage,
+            eventSink = ::handleEvents,
         )
     }
+
+    private fun CoroutineScope.getKeyBackupStatus(action: MutableState<Async<Boolean>>) = launch {
+        suspend {
+            encryptionService.doesBackupExistOnServer().getOrThrow()
+        }.runCatchingUpdatingState(action)
+    }
 }

features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/root/SecureBackupRootState.kt

@@ -16,14 +16,16 @@
 
 package io.element.android.features.securebackup.impl.root
 
+import io.element.android.libraries.architecture.Async
 import io.element.android.libraries.designsystem.utils.snackbar.SnackbarMessage
 import io.element.android.libraries.matrix.api.encryption.BackupState
 import io.element.android.libraries.matrix.api.encryption.RecoveryState
 
 data class SecureBackupRootState(
     val backupState: BackupState,
-    val doesBackupExistOnServer: Boolean?,
+    val doesBackupExistOnServer: Async<Boolean>,
     val recoveryState: RecoveryState,
     val appName: String,
     val snackbarMessage: SnackbarMessage?,
+    val eventSink: (SecureBackupRootEvents) -> Unit,
 )

features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/root/SecureBackupRootStateProvider.kt

@@ -17,6 +17,7 @@
 package io.element.android.features.securebackup.impl.root
 
 import androidx.compose.ui.tooling.preview.PreviewParameterProvider
+import io.element.android.libraries.architecture.Async
 import io.element.android.libraries.designsystem.utils.snackbar.SnackbarMessage
 import io.element.android.libraries.matrix.api.encryption.BackupState
 import io.element.android.libraries.matrix.api.encryption.RecoveryState
@@ -24,9 +25,10 @@ import io.element.android.libraries.matrix.api.encryption.RecoveryState
 open class SecureBackupRootStateProvider : PreviewParameterProvider<SecureBackupRootState> {
     override val values: Sequence<SecureBackupRootState>
         get() = sequenceOf(
-            aSecureBackupRootState(backupState = BackupState.UNKNOWN, doesBackupExistOnServer = null),
+            aSecureBackupRootState(backupState = BackupState.UNKNOWN, doesBackupExistOnServer = Async.Uninitialized),
-            aSecureBackupRootState(backupState = BackupState.UNKNOWN, doesBackupExistOnServer = true),
+            aSecureBackupRootState(backupState = BackupState.UNKNOWN, doesBackupExistOnServer = Async.Success(true)),
-            aSecureBackupRootState(backupState = BackupState.UNKNOWN, doesBackupExistOnServer = false),
+            aSecureBackupRootState(backupState = BackupState.UNKNOWN, doesBackupExistOnServer = Async.Success(false)),
+            aSecureBackupRootState(backupState = BackupState.UNKNOWN, doesBackupExistOnServer = Async.Failure(Exception("An error"))),
             aSecureBackupRootState(backupState = BackupState.ENABLED),
             aSecureBackupRootState(recoveryState = RecoveryState.UNKNOWN),
             aSecureBackupRootState(recoveryState = RecoveryState.ENABLED),
@@ -38,7 +40,7 @@ open class SecureBackupRootStateProvider : PreviewParameterProvider<SecureBackup
 
 fun aSecureBackupRootState(
     backupState: BackupState = BackupState.UNKNOWN,
-    doesBackupExistOnServer: Boolean? = true,
+    doesBackupExistOnServer: Async<Boolean> = Async.Uninitialized,
     recoveryState: RecoveryState = RecoveryState.UNKNOWN,
     snackbarMessage: SnackbarMessage? = null,
 ) = SecureBackupRootState(
@@ -47,4 +49,5 @@ fun aSecureBackupRootState(
     recoveryState = recoveryState,
     appName = "Element",
     snackbarMessage = snackbarMessage,
+    eventSink = {},
 )

features/securebackup/impl/src/main/kotlin/io/element/android/features/securebackup/impl/root/SecureBackupRootView.kt

@@ -16,18 +16,27 @@
 
 package io.element.android.features.securebackup.impl.root
 
+import androidx.compose.foundation.layout.Arrangement
+import androidx.compose.foundation.layout.Row
+import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.tooling.preview.PreviewParameter
 import io.element.android.features.securebackup.impl.R
+import io.element.android.libraries.architecture.Async
 import io.element.android.libraries.designsystem.components.async.AsyncLoading
+import io.element.android.libraries.designsystem.components.list.ListItemContent
 import io.element.android.libraries.designsystem.components.preferences.PreferenceDivider
 import io.element.android.libraries.designsystem.components.preferences.PreferencePage
 import io.element.android.libraries.designsystem.components.preferences.PreferenceText
 import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
 import io.element.android.libraries.designsystem.text.buildAnnotatedStringWithStyledPart
+import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator
+import io.element.android.libraries.designsystem.theme.components.ListItem
+import io.element.android.libraries.designsystem.theme.components.Text
+import io.element.android.libraries.designsystem.theme.components.TextButton
 import io.element.android.libraries.designsystem.utils.snackbar.SnackbarHost
 import io.element.android.libraries.designsystem.utils.snackbar.rememberSnackbarHostState
 import io.element.android.libraries.matrix.api.encryption.BackupState
@@ -73,23 +82,55 @@ fun SecureBackupRootView(
             BackupState.WAITING_FOR_SYNC -> Unit
             BackupState.UNKNOWN -> {
                 when (state.doesBackupExistOnServer) {
-                    true -> {
+                    is Async.Success -> when (state.doesBackupExistOnServer.data) {
-                        // Should not happen, we will have the state BackupState.ENABLED
+                        true -> {
-                        PreferenceText(
+                            // Should not happen, we will have the state BackupState.ENABLED
-                            title = stringResource(id = R.string.screen_chat_backup_key_backup_action_disable),
+                            PreferenceText(
-                            tintColor = ElementTheme.colors.textCriticalPrimary,
+                                title = stringResource(id = R.string.screen_chat_backup_key_backup_action_disable),
-                            onClick = onDisableClicked,
+                                tintColor = ElementTheme.colors.textCriticalPrimary,
-                        )
+                                onClick = onDisableClicked,
+                            )
+                        }
+                        false -> {
+                            PreferenceText(
+                                title = stringResource(id = R.string.screen_chat_backup_key_backup_action_enable),
+                                onClick = onEnableClicked,
+                            )
+                        }
+                    }
+                    is Async.Loading,
+                    Async.Uninitialized -> {
+                        ListItem(headlineContent = {
+                            Row(
+                                modifier = Modifier.fillMaxWidth(),
+                                horizontalArrangement = Arrangement.Center,
+                            ) {
+                                CircularProgressIndicator()
+                            }
+                        })
                     }
-                    false -> {
+                    is Async.Failure -> {
+                        ListItem(
+                            headlineContent = {
+                                Text(
+                                    text = stringResource(id = CommonStrings.error_unknown),
+                                )
+                            },
+                            trailingContent = ListItemContent.Custom {
+                                TextButton(
+                                    text = stringResource(
+                                        id = CommonStrings.action_retry
+                                    ),
+                                    onClick = { state.eventSink.invoke(SecureBackupRootEvents.RetryKeyBackupState) }
+                                )
+                            }
+                        )
+
                         PreferenceText(
                             title = stringResource(id = R.string.screen_chat_backup_key_backup_action_enable),
                             onClick = onEnableClicked,
                         )
                     }
-                    null -> {
-                        AsyncLoading()
-                    }
                 }
             }
             BackupState.CREATING,

features/securebackup/impl/src/test/kotlin/io/element/android/features/securebackup/impl/root/SecureBackupRootPresenterTest.kt

@@ -20,12 +20,16 @@ import app.cash.molecule.RecompositionMode
 import app.cash.molecule.moleculeFlow
 import app.cash.turbine.test
 import com.google.common.truth.Truth.assertThat
+import io.element.android.libraries.architecture.Async
 import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher
 import io.element.android.libraries.matrix.api.encryption.BackupState
 import io.element.android.libraries.matrix.api.encryption.EncryptionService
+import io.element.android.libraries.matrix.api.encryption.RecoveryState
+import io.element.android.libraries.matrix.test.AN_EXCEPTION
 import io.element.android.libraries.matrix.test.core.aBuildMeta
 import io.element.android.libraries.matrix.test.encryption.FakeEncryptionService
 import io.element.android.tests.testutils.WarmUpRule
+import io.element.android.tests.testutils.awaitLastSequentialItem
 import kotlinx.coroutines.test.runTest
 import org.junit.Rule
 import org.junit.Test
@@ -40,9 +44,38 @@ class SecureBackupRootPresenterTest {
         moleculeFlow(RecompositionMode.Immediate) {
             presenter.present()
         }.test {
-            val initialState = awaitItem()
+            val initialState = awaitLastSequentialItem()
             assertThat(initialState.backupState).isEqualTo(BackupState.UNKNOWN)
+            assertThat(initialState.doesBackupExistOnServer.dataOrNull()).isTrue()
+            assertThat(initialState.recoveryState).isEqualTo(RecoveryState.UNKNOWN)
             assertThat(initialState.appName).isEqualTo("Element")
+            assertThat(initialState.snackbarMessage).isNull()
+        }
+    }
+
+    @Test
+    fun `present - Unknown state`() = runTest {
+        val encryptionService = FakeEncryptionService()
+        val presenter = createSecureBackupRootPresenter(
+            encryptionService = encryptionService,
+        )
+        moleculeFlow(RecompositionMode.Immediate) {
+            presenter.present()
+        }.test {
+            val initialState = awaitItem()
+            encryptionService.givenDoesBackupExistOnServerResult(Result.failure(AN_EXCEPTION))
+            assertThat(initialState.backupState).isEqualTo(BackupState.UNKNOWN)
+            assertThat(initialState.doesBackupExistOnServer).isEqualTo(Async.Uninitialized)
+            val loadingState1 = awaitItem()
+            assertThat(loadingState1.doesBackupExistOnServer).isInstanceOf(Async.Loading::class.java)
+            val errorState = awaitItem()
+            assertThat(errorState.doesBackupExistOnServer).isEqualTo(Async.Failure<Boolean>(AN_EXCEPTION))
+            encryptionService.givenDoesBackupExistOnServerResult(Result.success(false))
+            errorState.eventSink.invoke(SecureBackupRootEvents.RetryKeyBackupState)
+            val loadingState2 = awaitItem()
+            assertThat(loadingState2.doesBackupExistOnServer).isInstanceOf(Async.Loading::class.java)
+            val finalState = awaitItem()
+            assertThat(finalState.doesBackupExistOnServer.dataOrNull()).isFalse()
         }
     }
 

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/encryption/FakeEncryptionService.kt

@@ -34,6 +34,7 @@ class FakeEncryptionService : EncryptionService {
     private var waitForBackupUploadSteadyStateFlow: Flow<BackupUploadState> = flowOf()
 
     private var fixRecoveryIssuesFailure: Exception? = null
+    private var doesBackupExistOnServerResult: Result<Boolean> = Result.success(true)
 
     override suspend fun enableBackups(): Result<Unit> = simulateLongTask {
         return Result.success(Unit)
@@ -52,8 +53,12 @@ class FakeEncryptionService : EncryptionService {
         return Result.success(Unit)
     }
 
+    fun givenDoesBackupExistOnServerResult(result: Result<Boolean>) {
+        doesBackupExistOnServerResult = result
+    }
+
     override suspend fun doesBackupExistOnServer(): Result<Boolean> = simulateLongTask {
-        return Result.success(true)
+        return doesBackupExistOnServerResult
     }
 
     override suspend fun fixRecoveryIssues(recoveryKey: String): Result<Unit> = simulateLongTask {