From dc7f9d45da840ece82aa1ba027066921bfa44d8e Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 16 Aug 2024 18:04:47 -0300 Subject: [PATCH] make file restore numbers add up by showing duplicates and errors --- .../restore/RestoreStats.kt | 6 +++-- .../backup/storage/api/RestoreObserver.kt | 1 + .../backup/storage/restore/FileSplitter.kt | 24 ++++++++++++++++--- .../restore/NotificationRestoreObserver.kt | 13 +++++++++- .../calyxos/backup/storage/restore/Restore.kt | 4 +++- .../backup/storage/ui/Notifications.kt | 12 ++++++++++ storage/lib/src/main/res/values/strings.xml | 2 ++ 7 files changed, 55 insertions(+), 7 deletions(-) diff --git a/storage/demo/src/main/java/de/grobox/storagebackuptester/restore/RestoreStats.kt b/storage/demo/src/main/java/de/grobox/storagebackuptester/restore/RestoreStats.kt index fcad4b128..4d8583e69 100644 --- a/storage/demo/src/main/java/de/grobox/storagebackuptester/restore/RestoreStats.kt +++ b/storage/demo/src/main/java/de/grobox/storagebackuptester/restore/RestoreStats.kt @@ -13,7 +13,6 @@ import de.grobox.storagebackuptester.backup.getSpeed import org.calyxos.backup.storage.api.BackupFile import org.calyxos.backup.storage.restore.NotificationRestoreObserver import kotlin.time.DurationUnit -import kotlin.time.ExperimentalTime import kotlin.time.toDuration data class RestoreProgress( @@ -41,6 +40,10 @@ class RestoreStats( liveData.postValue(RestoreProgress(filesProcessed, totalFiles, text)) } + override fun onFileDuplicatesRemoved(num: Int) { + // no-op + } + override fun onFileRestored( file: BackupFile, bytesWritten: Long, @@ -68,7 +71,6 @@ class RestoreStats( liveData.postValue(RestoreProgress(filesProcessed, totalFiles)) } - @OptIn(ExperimentalTime::class) override fun onRestoreComplete(restoreDuration: Long) { super.onRestoreComplete(restoreDuration) val sb = StringBuilder("\n") diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/api/RestoreObserver.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/api/RestoreObserver.kt index cc380b3ed..e11afb57f 100644 --- a/storage/lib/src/main/java/org/calyxos/backup/storage/api/RestoreObserver.kt +++ b/storage/lib/src/main/java/org/calyxos/backup/storage/api/RestoreObserver.kt @@ -7,6 +7,7 @@ package org.calyxos.backup.storage.api public interface RestoreObserver { public fun onRestoreStart(numFiles: Int, totalSize: Long) + public fun onFileDuplicatesRemoved(num: Int) public fun onFileRestored(file: BackupFile, bytesWritten: Long, tag: String) /** diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/restore/FileSplitter.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/restore/FileSplitter.kt index cec446c23..8720f0a3f 100644 --- a/storage/lib/src/main/java/org/calyxos/backup/storage/restore/FileSplitter.kt +++ b/storage/lib/src/main/java/org/calyxos/backup/storage/restore/FileSplitter.kt @@ -21,24 +21,32 @@ internal data class RestorableChunk( /** * Call this after the RestorableChunk is complete and **before** using it for restore. + * + * @return the number of duplicate files removed */ - fun finalize() { + fun finalize(): Int { // entries in the zip chunk need to be sorted by their index in the zip files.sortBy { it.zipIndex } // There might be duplicates in case the *exact* same set of files exists more than once // so they'll produce the same chunk ID. // But since the content is there and this is an unlikely scenario, we drop the duplicates. var lastIndex = 0 + var numRemoved = 0 val iterator = files.iterator() while (iterator.hasNext()) { val file = iterator.next() val i = file.zipIndex when { i < lastIndex -> error("unsorted list") - i == lastIndex -> iterator.remove() // remove duplicate + i == lastIndex -> { // remove duplicate + numRemoved++ + iterator.remove() + } + i > lastIndex -> lastIndex = i // gaps are possible when we don't restore all files } } + return numRemoved } } @@ -87,6 +95,14 @@ internal data class FileSplitterResult( * Files referenced in [multiChunkMap] sorted for restoring. */ val multiChunkFiles: Collection, + /** + * The number of duplicate files that was removed from [zipChunks]. + * Duplicate files in [zipChunks] with the same chunk ID will have the same index in the ZIP. + * So we remove them to make restore easier. + * With some extra work, we could restore those files, + * but by not doing so we are probably doing a favor to the user. + */ + val numRemovedDuplicates: Int, ) /** @@ -121,7 +137,7 @@ internal object FileSplitter { } } // entries in the zip chunk need to be sorted by their index in the zip, duplicated removed - zipChunkMap.values.forEach { zipChunk -> zipChunk.finalize() } + val numRemovedDuplicates = zipChunkMap.values.sumOf { zipChunk -> zipChunk.finalize() } val singleChunks = chunkMap.values.filter { it.isSingle } val multiChunks = chunkMap.filterValues { !it.isSingle } return FileSplitterResult( @@ -129,6 +145,7 @@ internal object FileSplitter { singleChunks = singleChunks, multiChunkMap = multiChunks, multiChunkFiles = getMultiFiles(multiChunks), + numRemovedDuplicates = numRemovedDuplicates, ) } @@ -145,6 +162,7 @@ internal object FileSplitter { f1.chunkIdsCount == f2.chunkIdsCount -> { f1.chunkIds.joinToString().compareTo(f2.chunkIds.joinToString()) } + else -> 1 } } diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/restore/NotificationRestoreObserver.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/restore/NotificationRestoreObserver.kt index 564353b73..f342ef6ba 100644 --- a/storage/lib/src/main/java/org/calyxos/backup/storage/restore/NotificationRestoreObserver.kt +++ b/storage/lib/src/main/java/org/calyxos/backup/storage/restore/NotificationRestoreObserver.kt @@ -18,6 +18,7 @@ public open class NotificationRestoreObserver internal constructor(private val n private var totalFiles = 0 private var filesRestored = 0 + private var filesRemovedAsDuplicates = 0 private var filesWithError = 0 override fun onRestoreStart(numFiles: Int, totalSize: Long) { @@ -25,6 +26,10 @@ public open class NotificationRestoreObserver internal constructor(private val n n.updateRestoreNotification(filesRestored + filesWithError, totalFiles) } + override fun onFileDuplicatesRemoved(num: Int) { + filesRemovedAsDuplicates = num + } + override fun onFileRestored(file: BackupFile, bytesWritten: Long, tag: String) { filesRestored++ n.updateRestoreNotification(filesRestored + filesWithError, totalFiles) @@ -36,7 +41,13 @@ public open class NotificationRestoreObserver internal constructor(private val n } override fun onRestoreComplete(restoreDuration: Long) { - n.showRestoreCompleteNotification(filesRestored, totalFiles, getRestoreCompleteIntent()) + n.showRestoreCompleteNotification( + restored = filesRestored, + duplicates = filesRemovedAsDuplicates, + errors = filesWithError, + total = totalFiles, + intent = getRestoreCompleteIntent(), + ) } protected open fun getRestoreCompleteIntent(): PendingIntent? { diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/restore/Restore.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/restore/Restore.kt index 4f0fd6c59..13e6609d3 100644 --- a/storage/lib/src/main/java/org/calyxos/backup/storage/restore/Restore.kt +++ b/storage/lib/src/main/java/org/calyxos/backup/storage/restore/Restore.kt @@ -110,8 +110,10 @@ internal class Restore( observer?.onRestoreStart(filesTotal, totalSize) val split = FileSplitter.splitSnapshot(snapshot) + observer?.onFileDuplicatesRemoved(split.numRemovedDuplicates) + var restoredFiles = split.numRemovedDuplicates // count removed dups, so numbers add up + val version = snapshot.version - var restoredFiles = 0 val smallFilesDuration = measure { restoredFiles += zipChunkRestore.restore( version, diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/ui/Notifications.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/ui/Notifications.kt index 4b6944700..f4b86b59a 100644 --- a/storage/lib/src/main/java/org/calyxos/backup/storage/ui/Notifications.kt +++ b/storage/lib/src/main/java/org/calyxos/backup/storage/ui/Notifications.kt @@ -116,13 +116,25 @@ internal class Notifications(private val context: Context) { internal fun showRestoreCompleteNotification( restored: Int, + duplicates: Int, + errors: Int, total: Int, intent: PendingIntent?, ) { val title = context.getString(R.string.notification_restore_complete_title, restored, total) + val msg = StringBuilder().apply { + if (duplicates > 0) { + append(context.getString(R.string.notification_restore_complete_dups, duplicates)) + } + if (errors > 0) { + if (duplicates > 0) append("\n") + append(context.getString(R.string.notification_restore_complete_errors, errors)) + } + }.toString().ifEmpty { null } val notification = NotificationCompat.Builder(context, CHANNEL_ID_BACKUP).apply { setSmallIcon(R.drawable.ic_cloud_done) setContentTitle(title) + setContentText(msg) setOngoing(false) setShowWhen(true) setAutoCancel(true) diff --git a/storage/lib/src/main/res/values/strings.xml b/storage/lib/src/main/res/values/strings.xml index 30e4044a2..b79f55bc6 100644 --- a/storage/lib/src/main/res/values/strings.xml +++ b/storage/lib/src/main/res/values/strings.xml @@ -18,6 +18,8 @@ Restoring files… %1$d/%2$d %1$d of %2$d files restored + %1$d files were duplicates. + %1$d files had errors. Available storage backups No storage backups found\n\nSorry, but there is nothing that can be restored.