From 83974b4121a2368d7155080db5509e822990952a Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 28 Oct 2024 15:07:02 -0300 Subject: [PATCH] Show app backup check error screen and error notification --- .../com/stevesoltys/seedvault/repo/Checker.kt | 29 ++++++++++-- .../seedvault/repo/CheckerResult.kt | 26 +++++++++- .../seedvault/restore/RestoreSetAdapter.kt | 20 +++++++- .../transport/restore/RestorableBackup.kt | 4 +- .../ui/check/AppCheckResultActivity.kt | 47 +++++++++++++++++-- .../notification/BackupNotificationManager.kt | 33 ++++++++++--- ..._success.xml => activity_check_result.xml} | 5 +- app/src/main/res/values/strings.xml | 7 +++ 8 files changed, 147 insertions(+), 24 deletions(-) rename app/src/main/res/layout/{activity_check_success.xml => activity_check_result.xml} (94%) diff --git a/app/src/main/java/com/stevesoltys/seedvault/repo/Checker.kt b/app/src/main/java/com/stevesoltys/seedvault/repo/Checker.kt index d793addf..e336f5b7 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/repo/Checker.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/repo/Checker.kt @@ -22,7 +22,9 @@ import org.calyxos.seedvault.core.toHexString import java.security.DigestInputStream import java.security.GeneralSecurityException import java.security.MessageDigest +import java.util.concurrent.ConcurrentSkipListSet import java.util.concurrent.atomic.AtomicLong +import kotlin.math.max import kotlin.math.min import kotlin.math.roundToInt import kotlin.math.roundToLong @@ -36,6 +38,7 @@ internal class Checker( ) { private val log = KotlinLogging.logger { } + private var handleSize: Int? = null private var snapshots: List? = null private val concurrencyLimit: Int get() { @@ -55,6 +58,7 @@ internal class Checker( } val snapshots = snapshotManager.onSnapshotsLoaded(handles) this.snapshots = snapshots // remember loaded snapshots + this.handleSize = handles.size // remember number of snapshot handles we had // get total disk space used by snapshots val sizeMap = mutableMapOf() @@ -71,6 +75,10 @@ internal class Checker( if (snapshots == null) getBackupSize() // just get size again to be sure we get snapshots val snapshots = snapshots ?: error("Snapshots still null") + val handleSize = handleSize ?: error("Handle size still null") + check(handleSize >= snapshots.size) { + "Got $handleSize handles, but ${snapshots.size} snapshots." + } val blobSample = getBlobSample(snapshots, percent) val sampleSize = blobSample.values.sumOf { it.length.toLong() } log.info { "Blob sample has ${blobSample.size} blobs worth $sampleSize bytes." } @@ -78,6 +86,7 @@ internal class Checker( // check blobs concurrently val semaphore = Semaphore(concurrencyLimit) val size = AtomicLong() + val badChunks = ConcurrentSkipListSet() val lastNotification = AtomicLong() val startTime = System.currentTimeMillis() coroutineScope { @@ -86,8 +95,13 @@ internal class Checker( launch { // suspend here until we get a permit from the semaphore (there's free workers) semaphore.withPermit { - // TODO record errors - checkBlob(chunkId, blob) + try { + checkBlob(chunkId, blob) + } catch (e: Exception) { + log.error(e) { "Error loading chunk $chunkId: " } + // TODO we could try differentiating transient backend issues + badChunks.add(chunkId) + } } // keep track of how much we checked and for how long val newSize = size.addAndGet(blob.length.toLong()) @@ -106,10 +120,15 @@ internal class Checker( if (sampleSize != size.get()) log.error { "Checked ${size.get()} bytes, but expected $sampleSize" } - val passedTime = System.currentTimeMillis() - startTime + val passedTime = max(System.currentTimeMillis() - startTime, 1000) // no div by zero val bandwidth = size.get() / (passedTime.toDouble() / 1000).roundToLong() - nm.onCheckComplete(size.get(), bandwidth) - checkerResult = CheckerResult.Success(snapshots, percent, size.get()) + checkerResult = if (badChunks.isEmpty() && handleSize == snapshots.size && handleSize > 0) { + nm.onCheckComplete(size.get(), bandwidth) + CheckerResult.Success(snapshots, percent, size.get()) + } else { + nm.onCheckFinishedWithError(size.get(), bandwidth) + CheckerResult.Error(handleSize, snapshots, badChunks) + } this.snapshots = null } diff --git a/app/src/main/java/com/stevesoltys/seedvault/repo/CheckerResult.kt b/app/src/main/java/com/stevesoltys/seedvault/repo/CheckerResult.kt index 56b9b0b3..a2d5ecdf 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/repo/CheckerResult.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/repo/CheckerResult.kt @@ -15,9 +15,31 @@ sealed class CheckerResult { ) : CheckerResult() data class Error( + /** + * This number is greater than the size of [snapshots], + * if we could not read/decrypt one or more snapshots. + */ + val existingSnapshots: Int, val snapshots: List, - val errors: Map, - ) : CheckerResult() + /** + * The list of chunkIDs that had errors. + */ + val errorChunkIds: Set, + ) : CheckerResult() { + val goodSnapshots: List + val badSnapshots: List + + init { + val good = mutableListOf() + val bad = mutableListOf() + snapshots.forEach { snapshot -> + val isGood = snapshot.blobsMap.keys.intersect(errorChunkIds).isEmpty() + if (isGood) good.add(snapshot) else bad.add(snapshot) + } + goodSnapshots = good + badSnapshots = bad + } + } data class GeneralError(val e: Exception) : CheckerResult() } diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreSetAdapter.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreSetAdapter.kt index 05e4476e..6f7461db 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreSetAdapter.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreSetAdapter.kt @@ -14,9 +14,11 @@ import android.view.View import android.view.View.GONE import android.view.View.VISIBLE import android.view.ViewGroup +import android.widget.ImageView import android.widget.TextView import androidx.recyclerview.widget.RecyclerView.Adapter import androidx.recyclerview.widget.RecyclerView.ViewHolder +import com.google.android.material.color.MaterialColors import com.stevesoltys.seedvault.R import com.stevesoltys.seedvault.restore.RestoreSetAdapter.RestoreSetViewHolder import com.stevesoltys.seedvault.transport.restore.RestorableBackup @@ -40,6 +42,7 @@ internal class RestoreSetAdapter( inner class RestoreSetViewHolder(private val v: View) : ViewHolder(v) { + private val imageView = v.requireViewById(R.id.imageView) private val titleView = v.requireViewById(R.id.titleView) private val appView = v.requireViewById(R.id.appView) private val apkView = v.requireViewById(R.id.apkView) @@ -49,6 +52,11 @@ internal class RestoreSetAdapter( if (listener != null) { v.setOnClickListener { listener.onRestorableBackupClicked(item) } } + if (item.canBeRestored) { + imageView.setImageResource(R.drawable.ic_phone_android) + } else { + imageView.setImageResource(R.drawable.ic_error_red) + } titleView.text = item.name appView.text = if (item.sizeAppData > 0) { @@ -61,7 +69,9 @@ internal class RestoreSetAdapter( v.context.getString(R.string.restore_restore_set_apps_no_size, item.numAppData) } appView.visibility = if (item.numAppData > 0) VISIBLE else GONE - apkView.text = if (item.sizeApks > 0) { + apkView.text = if (!item.canBeRestored) { + v.context.getString(R.string.restore_restore_set_can_not_get_restored) + } else if (item.sizeApks > 0) { v.context.getString( R.string.restore_restore_set_apks, item.numApks, @@ -70,7 +80,13 @@ internal class RestoreSetAdapter( } else { v.context.getString(R.string.restore_restore_set_apks_no_size, item.numApks) } - apkView.visibility = if (item.numApks > 0) VISIBLE else GONE + apkView.visibility = if (item.numApks > 0 || !item.canBeRestored) VISIBLE else GONE + val apkTextColor = if (item.canBeRestored) { + appView.currentTextColor + } else { + MaterialColors.getColor(apkView, R.attr.colorError) + } + apkView.setTextColor(apkTextColor) timeView.text = getRelativeTime(item.time) } diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorableBackup.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorableBackup.kt index 2dc7de45..054b75bd 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorableBackup.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestorableBackup.kt @@ -19,12 +19,14 @@ data class RestorableBackup( val backupMetadata: BackupMetadata, val repoId: String? = null, val snapshot: Snapshot? = null, + val canBeRestored: Boolean = true, ) { - constructor(repoId: String, snapshot: Snapshot) : this( + constructor(repoId: String, snapshot: Snapshot, canBeRestored: Boolean = true) : this( backupMetadata = BackupMetadata.fromSnapshot(snapshot), repoId = repoId, snapshot = snapshot, + canBeRestored = canBeRestored, ) val name: String diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/check/AppCheckResultActivity.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/check/AppCheckResultActivity.kt index 700d81f3..922ba469 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/check/AppCheckResultActivity.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/ui/check/AppCheckResultActivity.kt @@ -7,6 +7,8 @@ package com.stevesoltys.seedvault.ui.check import android.os.Bundle import android.text.format.Formatter.formatShortFileSize +import android.view.View.GONE +import android.widget.ImageView import android.widget.TextView import androidx.recyclerview.widget.RecyclerView import com.stevesoltys.seedvault.R @@ -52,10 +54,7 @@ class AppCheckResultActivity : BackupActivity() { private fun onActionReceived() { when (val result = checker.checkerResult) { is CheckerResult.Success -> onSuccess(result) - is CheckerResult.Error -> { - // TODO - log.info { "snapshots: ${result.snapshots.size}, errors: ${result.errors.size}" } - } + is CheckerResult.Error -> onError(result) is CheckerResult.GeneralError, null -> { // TODO if (result == null) log.error { "No more result" } @@ -66,7 +65,7 @@ class AppCheckResultActivity : BackupActivity() { } private fun onSuccess(result: CheckerResult.Success) { - setContentView(R.layout.activity_check_success) + setContentView(R.layout.activity_check_result) val intro = getString( R.string.backup_app_check_success_intro, result.snapshots.size, @@ -84,4 +83,42 @@ class AppCheckResultActivity : BackupActivity() { ) } + private fun onError(result: CheckerResult.Error) { + setContentView(R.layout.activity_check_result) + requireViewById(R.id.imageView).setImageResource(R.drawable.ic_cloud_error) + requireViewById(R.id.titleView).setText(R.string.backup_app_check_error_title) + val disclaimerView = requireViewById(R.id.disclaimerView) + + val intro = if (result.existingSnapshots == 0) { + disclaimerView.visibility = GONE + getString(R.string.backup_app_check_error_no_snapshots) + } else if (result.snapshots.isEmpty()) { + disclaimerView.visibility = GONE + getString( + R.string.backup_app_check_error_only_broken_snapshots, + result.existingSnapshots, + ) + } else if (result.existingSnapshots > result.snapshots.size) { + getString( + R.string.backup_app_check_error_some_snapshots, + result.existingSnapshots, + result.snapshots.size, + ) + } else { + getString(R.string.backup_app_check_error_read_all_snapshots, result.snapshots.size) + } + requireViewById(R.id.introView).text = intro + + val items = (result.goodSnapshots.map { snapshot -> + RestorableBackup("", snapshot) + } + result.badSnapshots.map { snapshot -> + RestorableBackup("", snapshot, false) + }).sortedByDescending { it.time } + val listView = requireViewById(R.id.listView) + listView.adapter = RestoreSetAdapter( + listener = null, + items = items, + ) + } + } diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt index 4a83f5b6..255a75b8 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt @@ -363,6 +363,31 @@ internal class BackupNotificationManager(private val context: Context) { formatShortFileSize(context, size), "${formatShortFileSize(context, speed)}/s", ) + val notification = getOnCheckFinishedBuilder() + .setContentTitle(context.getString(R.string.notification_checking_finished_title)) + .setContentText(text) + .setSmallIcon(R.drawable.ic_cloud_done) + .build() + nm.cancel(NOTIFICATION_ID_CHECKING) + nm.notify(NOTIFICATION_ID_CHECK_FINISHED, notification) + } + + fun onCheckFinishedWithError(size: Long, speed: Long) { + val text = context.getString( + R.string.notification_checking_error_text, + formatShortFileSize(context, size), + "${formatShortFileSize(context, speed)}/s", + ) + val notification = getOnCheckFinishedBuilder() + .setContentTitle(context.getString(R.string.notification_checking_error_title)) + .setContentText(text) + .setSmallIcon(R.drawable.ic_cloud_error) + .build() + nm.cancel(NOTIFICATION_ID_CHECKING) + nm.notify(NOTIFICATION_ID_CHECK_FINISHED, notification) + } + + private fun getOnCheckFinishedBuilder(): Builder { // the background activity launch (BAL) gets restricted for setDeleteIntent() // if we don't use these special ActivityOptions, may cause issues in future SDKs val options = ActivityOptions.makeBasic() @@ -381,17 +406,11 @@ internal class BackupNotificationManager(private val context: Context) { val deleteIntent = getActivity(context, 2, dIntent, FLAG_IMMUTABLE, options) val actionTitle = context.getString(R.string.notification_checking_action) val action = Action.Builder(null, actionTitle, contentIntent).build() - val notification = Builder(context, CHANNEL_ID_CHECKING) - .setContentTitle(context.getString(R.string.notification_checking_finished_title)) - .setContentText(text) - .setSmallIcon(R.drawable.ic_cloud_done) + return Builder(context, CHANNEL_ID_CHECKING) .setContentIntent(contentIntent) .addAction(action) .setDeleteIntent(deleteIntent) .setAutoCancel(true) - .build() - nm.cancel(NOTIFICATION_ID_CHECKING) - nm.notify(NOTIFICATION_ID_CHECK_FINISHED, notification) } fun onCheckCompleteNotificationSeen() { diff --git a/app/src/main/res/layout/activity_check_success.xml b/app/src/main/res/layout/activity_check_result.xml similarity index 94% rename from app/src/main/res/layout/activity_check_success.xml rename to app/src/main/res/layout/activity_check_result.xml index c72f2494..f3a0a3a3 100644 --- a/app/src/main/res/layout/activity_check_success.xml +++ b/app/src/main/res/layout/activity_check_result.xml @@ -51,10 +51,10 @@ android:layout_height="wrap_content" android:layout_margin="0dp" app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager" - app:layout_constraintBottom_toTopOf="@+id/disclaimerView" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@+id/introView" + app:layout_constraintVertical_bias="0.0" tools:itemCount="4" tools:listitem="@layout/list_item_restore_set" /> @@ -70,7 +70,8 @@ app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" - app:layout_constraintTop_toBottomOf="@+id/listView" /> + app:layout_constraintTop_toBottomOf="@+id/listView" + app:layout_constraintVertical_bias="0.0" /> diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f084a1ba..e11e0eb9 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -191,11 +191,17 @@ Checking app backups… App backup integrity verified Successfully checked %1$s at an average speed of %2$s. + Backup errors found + Tap for details. We checked %1$s at an average speed of %2$s. Details %1$d snapshots were found and %2$d%% of their data (%3$s) successfully verified: Note: We can not verify whether apps include all of their data in the backup. + @string/notification_checking_error_title We could not find any backup. Please run a successful backup first and then try checking again. + We found %1$d backup snapshots. However, all of them were corrupted. Please run a successful backup and then try checking again. + We found %1$d backup snapshots. However, we could only read the following %2$d snapshots. + We found %1$d backup snapshots. However, some had errors and can not be fully restored. @@ -229,6 +235,7 @@ Has user data for %1$d apps Contains %1$d apps (%2$s) Contains %1$d apps + Can not be (fully) restored. Don\'t restore Skip restoring apps No backups found