Don't restore files that still exist unchanged

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")

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)
 
     /**

storage/lib/src/main/java/org/calyxos/backup/storage/backup/BackupService.kt

@@ -7,6 +7,7 @@ package org.calyxos.backup.storage.backup
 
 import android.app.Service
 import android.content.Intent
+import android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_MANIFEST
 import android.os.IBinder
 import android.util.Log
 import kotlinx.coroutines.GlobalScope
@@ -30,15 +31,17 @@ public abstract class BackupService : Service() {
         Log.d(TAG, "onStartCommand $intent $flags $startId")
         startForeground(
             NOTIFICATION_ID_BACKUP,
-            n.getBackupNotification(R.string.notification_backup_scanning)
+            n.getBackupNotification(R.string.notification_backup_scanning),
+            FOREGROUND_SERVICE_TYPE_MANIFEST,
         )
         GlobalScope.launch {
             val success = storageBackup.runBackup(backupObserver)
             if (success) {
                 // only prune old backups when backup run was successful
                 startForeground(
                     NOTIFICATION_ID_PRUNE,
-                    n.getPruneNotification(R.string.notification_prune)
+                    n.getPruneNotification(R.string.notification_prune),
+                    FOREGROUND_SERVICE_TYPE_MANIFEST,
                 )
                 storageBackup.pruneOldBackups(backupObserver)
             }

storage/lib/src/main/java/org/calyxos/backup/storage/content/ContentFile.kt

@@ -100,13 +100,17 @@ internal data class MediaFile(
             .setName(fileName)
             .setSize(size)
             .addAllChunkIds(chunkIds)
+            .setIsFavorite(isFavorite)
             .setVolume(if (volume == MediaStore.VOLUME_EXTERNAL_PRIMARY) "" else volume)
         if (lastModified != null) {
             builder.lastModified = lastModified
         }
         if (zipIndex != null) {
             builder.zipIndex = zipIndex
         }
+        if (ownerPackageName != null) {
+            builder.setOwnerPackageName(ownerPackageName)
+        }
         return builder.build()
     }
 }

storage/lib/src/main/java/org/calyxos/backup/storage/restore/AbstractChunkRestore.kt

@@ -46,8 +46,6 @@ internal abstract class AbstractChunkRestore(
         tag: String,
         streamWriter: suspend (outputStream: OutputStream) -> Long,
     ) {
-        // TODO check if the file exists already (same name, size, chunk IDs)
-        //  and skip it in this case
         fileRestore.restoreFile(file, observer, tag, streamWriter)
     }
 

storage/lib/src/main/java/org/calyxos/backup/storage/restore/FileRestore.kt

@@ -24,7 +24,6 @@ import kotlin.random.Random
 
 private const val TAG = "FileRestore"
 
-@Suppress("BlockingMethodInNonBlockingContext")
 internal class FileRestore(
     private val context: Context,
     private val mediaScanner: MediaScanner,
@@ -46,10 +45,12 @@ internal class FileRestore(
                 bytes = restoreFile(file.mediaFile, streamWriter)
                 finalTag = "M$tag"
             }
+
             file.docFile != null -> {
                 bytes = restoreFile(file, streamWriter)
                 finalTag = "D$tag"
             }
+
             else -> {
                 error("unexpected file: $file")
             }
@@ -63,52 +64,64 @@ internal class FileRestore(
         streamWriter: suspend (outputStream: OutputStream) -> Long,
     ): Long {
         // ensure directory exists
-        @Suppress("DEPRECATION")
         val dir = File("${getExternalStorageDirectory()}/${docFile.dir}")
         if (!dir.mkdirs() && !dir.isDirectory) {
             throw IOException("Could not create ${dir.absolutePath}")
         }
-        // find non-existing file-name
         var file = File(dir, docFile.name)
-        var i = 0
-        // we don't support existing files, but at least don't overwrite them when they do exist
-        while (file.exists()) {
-            i++
-            val lastDot = docFile.name.lastIndexOf('.')
-            val newName = if (lastDot == -1) "${docFile.name} ($i)"
-            else docFile.name.replaceRange(lastDot..lastDot, " ($i).")
-            file = File(dir, newName)
-        }
-        val bytesWritten = try {
-            // copy chunk(s) into file
-            file.outputStream().use { outputStream ->
-                streamWriter(outputStream)
+        // TODO should we also calculate and check the chunk IDs?
+        if (file.isFile && file.length() == docFile.size &&
+            file.lastModified() == docFile.lastModified
+        ) {
+            Log.i(TAG, "Not restoring $file, already there unchanged.")
+            return file.length() // not restoring existing file with same length and date
+        } else {
+            var i = 0
+            // don't overwrite existing files, if they exist
+            while (file.exists()) { // find non-existing file-name
+                i++
+                val lastDot = docFile.name.lastIndexOf('.')
+                val newName = if (lastDot == -1) "${docFile.name} ($i)"
+                else docFile.name.replaceRange(lastDot..lastDot, " ($i).")
+                file = File(dir, newName)
             }
-        } catch (e: IOException) {
-            file.delete()
-            throw e
-        }
-        // re-set lastModified timestamp
-        file.setLastModified(docFile.lastModified ?: 0)
+            val bytesWritten = try {
+                // copy chunk(s) into file
+                file.outputStream().use { outputStream ->
+                    streamWriter(outputStream)
+                }
+            } catch (e: IOException) {
+                file.delete()
+                throw e
+            }
+            // re-set lastModified timestamp
+            file.setLastModified(docFile.lastModified ?: 0)
 
-        // This might be a media file, so do we need to index it.
-        // Otherwise things like a wrong size of 0 bytes in MediaStore can happen.
-        indexFile(file)
+            // This might be a media file, so do we need to index it.
+            // Otherwise things like a wrong size of 0 bytes in MediaStore can happen.
+            indexFile(file)
 
-        return bytesWritten
+            return bytesWritten
+        }
     }
 
     @Throws(IOException::class)
     private suspend fun restoreFile(
         mediaFile: BackupMediaFile,
         streamWriter: suspend (outputStream: OutputStream) -> Long,
     ): Long {
+        // TODO should we also calculate and check the chunk IDs?
+        if (mediaScanner.existsMediaFileUnchanged(mediaFile)) {
+            Log.i(
+                TAG,
+                "Not restoring ${mediaFile.path}/${mediaFile.name}, already there unchanged."
+            )
+            return mediaFile.size
+        }
         // Insert pending media item into MediaStore
         val contentValues = ContentValues().apply {
             put(MediaColumns.DISPLAY_NAME, mediaFile.name)
             put(MediaColumns.RELATIVE_PATH, mediaFile.path)
-            // changing owner requires backup permission
-            put(MediaColumns.OWNER_PACKAGE_NAME, mediaFile.ownerPackageName)
             put(MediaColumns.IS_PENDING, 1)
             put(MediaColumns.IS_FAVORITE, if (mediaFile.isFavorite) 1 else 0)
         }
@@ -124,6 +137,9 @@ internal class FileRestore(
         contentValues.clear()
         contentValues.apply {
             put(MediaColumns.IS_PENDING, 0)
+            // changing owner requires backup permission
+            // done here because we are not allowed to access pending media we don't own
+            put(MediaColumns.OWNER_PACKAGE_NAME, mediaFile.ownerPackageName)
         }
         try {
             contentResolver.update(uri, contentValues, null, null)
@@ -143,7 +159,6 @@ internal class FileRestore(
     }
 
     private fun setLastModifiedOnMediaFile(mediaFile: BackupMediaFile, uri: Uri) {
-        @Suppress("DEPRECATION")
         val extDir = getExternalStorageDirectory()
 
         // re-set lastModified as we can't use the MediaStore for this (read-only property)

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<RestorableFile>,
+    /**
+     * 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,14 +137,15 @@ 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(
             zipChunks = zipChunkMap.values,
             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
             }
         }

storage/lib/src/main/java/org/calyxos/backup/storage/restore/NotificationRestoreObserver.kt

@@ -18,13 +18,18 @@ 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) {
         totalFiles = numFiles
         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? {

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,

storage/lib/src/main/java/org/calyxos/backup/storage/restore/RestoreService.kt

@@ -7,6 +7,7 @@ package org.calyxos.backup.storage.restore
 
 import android.app.Service
 import android.content.Intent
+import android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_MANIFEST
 import android.os.IBinder
 import android.util.Log
 import kotlinx.coroutines.Dispatchers
@@ -49,7 +50,11 @@ public abstract class RestoreService : Service() {
         if (timestamp < 0) error("No timestamp in intent: $intent")
         val storedSnapshot = StoredSnapshot(userId, timestamp)
 
-        startForeground(NOTIFICATION_ID_RESTORE, n.getRestoreNotification())
+        startForeground(
+            NOTIFICATION_ID_RESTORE,
+            n.getRestoreNotification(),
+            FOREGROUND_SERVICE_TYPE_MANIFEST,
+        )
         GlobalScope.launch {
             val snapshot = withContext(Dispatchers.Main) {
                 fileSelectionManager.getBackupSnapshotAndReset()

storage/lib/src/main/java/org/calyxos/backup/storage/scanner/DocumentScanner.kt

@@ -56,7 +56,7 @@ public class DocumentScanner(context: Context) {
             queryUri, PROJECTION, null, null, null
         )
         val documentFiles = ArrayList<DocFile>(cursor?.count ?: 0)
-        cursor?.use { it ->
+        cursor?.use {
             while (it.moveToNext()) {
                 val id = it.getString(PROJECTION_ID)
                 val documentUri = DocumentsContract.buildDocumentUriUsingTree(uri, id)

storage/lib/src/main/java/org/calyxos/backup/storage/scanner/FileScanner.kt

@@ -16,7 +16,6 @@ import org.calyxos.backup.storage.content.DocFile
 import org.calyxos.backup.storage.content.MediaFile
 import org.calyxos.backup.storage.db.UriStore
 import org.calyxos.backup.storage.measure
-import kotlin.time.ExperimentalTime
 
 internal class FileScannerResult(
     val smallFiles: List<ContentFile>,
@@ -36,7 +35,6 @@ internal class FileScanner(
         private const val FILES_LARGE = "large"
     }
 
-    @OptIn(ExperimentalTime::class)
     fun getFiles(): FileScannerResult {
         // scan both APIs
         val mediaFiles = ArrayList<ContentFile>()