Refactor fetching of restorable backups

so that we don't go through the BackupManager API, but use RestoreCoordinator directly

app/src/main/java/com/stevesoltys/seedvault/restore/RestorableBackup.kt

@@ -1,19 +1,15 @@
 package com.stevesoltys.seedvault.restore
 
-import android.app.backup.RestoreSet
 import com.stevesoltys.seedvault.metadata.BackupMetadata
 import com.stevesoltys.seedvault.metadata.PackageMetadataMap
 
-data class RestorableBackup(
+data class RestorableBackup(private val backupMetadata: BackupMetadata) {
-    private val restoreSet: RestoreSet,
-    private val backupMetadata: BackupMetadata
-) {
 
     val name: String
-        get() = restoreSet.name
+        get() = backupMetadata.deviceName
 
     val token: Long
-        get() = restoreSet.token
+        get() = backupMetadata.token
 
     val time: Long
         get() = backupMetadata.time

app/src/main/java/com/stevesoltys/seedvault/restore/RestoreViewModel.kt

@@ -20,7 +20,6 @@ import com.stevesoltys.seedvault.BackupMonitor
 import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.R
 import com.stevesoltys.seedvault.crypto.KeyManager
-import com.stevesoltys.seedvault.metadata.BackupMetadata
 import com.stevesoltys.seedvault.metadata.PackageState.APK_AND_DATA
 import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.metadata.PackageState.NO_DATA
@@ -65,9 +64,6 @@ import org.calyxos.backup.storage.restore.RestoreService.Companion.EXTRA_TIMESTA
 import org.calyxos.backup.storage.restore.RestoreService.Companion.EXTRA_USER_ID
 import org.calyxos.backup.storage.ui.restore.SnapshotViewModel
 import java.util.LinkedList
-import kotlin.coroutines.Continuation
-import kotlin.coroutines.resume
-import kotlin.coroutines.suspendCoroutine
 
 private val TAG = RestoreViewModel::class.java.simpleName
 
@@ -126,8 +122,6 @@ internal class RestoreViewModel(
 
     @Throws(RemoteException::class)
     private fun getOrStartSession(): IRestoreSession {
-        // TODO consider not using the BackupManager for this, but our own API directly
-        //  this is less error-prone (hanging sessions) and can provide more data
         val session = this.session
             ?: backupManager.beginRestoreSessionForUser(UserHandle.myUserId(), null, TRANSPORT_ID)
             ?: throw RemoteException("beginRestoreSessionForUser returned null")
@@ -135,36 +129,27 @@ internal class RestoreViewModel(
         return session
     }
 
-    internal fun loadRestoreSets() = viewModelScope.launch {
+    internal fun loadRestoreSets() = viewModelScope.launch(ioDispatcher) {
-        mRestoreSetResults.value = getAvailableRestoreSets()
+        val backups = restoreCoordinator.getAvailableMetadata()?.mapNotNull { (token, metadata) ->
-    }
+            when (metadata.time) {
-
+                0L -> {
-    private suspend fun getAvailableRestoreSets() =
+                    Log.d(TAG, "Ignoring RestoreSet with no last backup time: $token.")
-        suspendCoroutine<RestoreSetResult> { continuation ->
+                    null
-            val session = try {
+                }
-                getOrStartSession()
+                else -> RestorableBackup(metadata)
-            } catch (e: RemoteException) {
-                Log.e(TAG, "Error starting new session", e)
-                continuation.resume(RestoreSetResult(app.getString(R.string.restore_set_error)))
-                return@suspendCoroutine
-            }
-
-            val observer = RestoreObserver(continuation)
-            val setResult = session.getAvailableRestoreSets(observer, monitor)
-            if (setResult != 0) {
-                Log.e(TAG, "getAvailableRestoreSets() returned non-zero value")
-                continuation.resume(RestoreSetResult(app.getString(R.string.restore_set_error)))
-                return@suspendCoroutine
             }
         }
+        val result = when {
+            backups == null -> RestoreSetResult(app.getString(R.string.restore_set_error))
+            backups.isEmpty() -> RestoreSetResult(app.getString(R.string.restore_set_empty_result))
+            else -> RestoreSetResult(backups)
+        }
+        mRestoreSetResults.postValue(result)
+    }
 
     override fun onRestorableBackupClicked(restorableBackup: RestorableBackup) {
         mChosenRestorableBackup.value = restorableBackup
         mDisplayFragment.setEvent(RESTORE_APPS)
-
-        // re-installing apps will take some time and the session probably times out
-        // so better close it cleanly and re-open it later
-        closeSession()
     }
 
     private fun getInstallResult(backup: RestorableBackup): LiveData<InstallResult> {
@@ -193,7 +178,7 @@ internal class RestoreViewModel(
     }
 
     @WorkerThread
-    private suspend fun startRestore(token: Long) {
+    private fun startRestore(token: Long) {
         Log.d(TAG, "Starting new restore session to restore backup $token")
 
         // if we had no token before (i.e. restore from setup wizard),
@@ -202,25 +187,35 @@ internal class RestoreViewModel(
             settingsManager.setNewToken(token)
         }
 
-        // we need to start a new session and retrieve the restore sets before starting the restore
+        // start a new restore session
-        val restoreSetResult = getAvailableRestoreSets()
+        val session = try {
-        if (restoreSetResult.hasError()) {
+            getOrStartSession()
+        } catch (e: RemoteException) {
+            Log.e(TAG, "Error starting new session", e)
             mRestoreBackupResult.postValue(
-                RestoreBackupResult(app.getString(R.string.restore_finished_error))
+                RestoreBackupResult(app.getString(R.string.restore_set_error))
             )
             return
         }
 
-        // now we can start the restore of all available packages
+        // we need to retrieve the restore sets before starting the restore
-        val observer = RestoreObserver()
+        // otherwise restoreAll() won't work as they need the restore sets cached internally
-        val restoreAllResult = session?.restoreAll(token, observer, monitor) ?: 1
+        val observer = RestoreObserver { observer ->
-        if (restoreAllResult != 0) {
+            // this lambda gets executed after we got the restore sets
-            if (session == null) Log.e(TAG, "session was null")
+            // now we can start the restore of all available packages
-            else Log.e(TAG, "restoreAll() returned non-zero value")
+            val restoreAllResult = session.restoreAll(token, observer, monitor)
+            if (restoreAllResult != 0) {
+                Log.e(TAG, "restoreAll() returned non-zero value")
+                mRestoreBackupResult.postValue(
+                    RestoreBackupResult(app.getString(R.string.restore_finished_error))
+                )
+            }
+        }
+        if (session.getAvailableRestoreSets(observer, monitor) != 0) {
+            Log.e(TAG, "getAvailableRestoreSets() returned non-zero value")
             mRestoreBackupResult.postValue(
-                RestoreBackupResult(app.getString(R.string.restore_finished_error))
+                RestoreBackupResult(app.getString(R.string.restore_set_error))
             )
-            return
         }
     }
 
@@ -297,9 +292,8 @@ internal class RestoreViewModel(
     }
 
     @WorkerThread
-    private inner class RestoreObserver(
+    private inner class RestoreObserver(private val startRestore: (RestoreObserver) -> Unit) :
-        private val continuation: Continuation<RestoreSetResult>? = null
+        IRestoreObserver.Stub() {
-    ) : IRestoreObserver.Stub() {
 
         /**
          * Supply a list of the restore datasets available from the current transport.
@@ -311,39 +305,7 @@ internal class RestoreViewModel(
          *   the current device. If no applicable datasets exist, restoreSets will be null.
          */
         override fun restoreSetsAvailable(restoreSets: Array<out RestoreSet>?) {
-            check(continuation != null) { "Getting restore sets without continuation" }
+            startRestore(this)
-
-            val result = if (restoreSets == null || restoreSets.isEmpty()) {
-                RestoreSetResult(app.getString(R.string.restore_set_empty_result))
-            } else {
-                val backupMetadata = restoreCoordinator.getAndClearBackupMetadata()
-                if (backupMetadata == null) {
-                    Log.e(TAG, "RestoreCoordinator#getAndClearBackupMetadata() returned null")
-                    RestoreSetResult(app.getString(R.string.restore_set_error))
-                } else {
-                    val restorableBackups = restoreSets.mapNotNull { set ->
-                        getRestorableBackup(set, backupMetadata[set.token])
-                    }
-                    if (restorableBackups.isEmpty()) {
-                        RestoreSetResult(app.getString(R.string.restore_set_empty_result))
-                    } else RestoreSetResult(restorableBackups)
-                }
-            }
-            continuation.resume(result)
-        }
-
-        private fun getRestorableBackup(set: RestoreSet, metadata: BackupMetadata?) = when {
-            metadata == null -> {
-                Log.e(TAG, "No metadata for token ${set.token}.")
-                null
-            }
-            metadata.time == 0L -> {
-                Log.d(TAG, "Ignoring RestoreSet with no last backup time: ${set.token}.")
-                null
-            }
-            else -> {
-                RestorableBackup(set, metadata)
-            }
         }
 
         /**

app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt

@@ -54,16 +54,9 @@ internal class RestoreCoordinator(
     private var backupMetadata: LongSparseArray<BackupMetadata>? = null
     private val failedPackages = ArrayList<String>()
 
-    /**
+    suspend fun getAvailableMetadata(): Map<Long, BackupMetadata>? {
-     * Get the set of all backups currently available over this transport.
-     *
-     * @return Descriptions of the set of restore images available for this device,
-     *   or null if an error occurred (the attempt should be rescheduled).
-     **/
-    suspend fun getAvailableRestoreSets(): Array<RestoreSet>? {
         val availableBackups = plugin.getAvailableBackups() ?: return null
-        val restoreSets = ArrayList<RestoreSet>()
+        val metadataMap = HashMap<Long, BackupMetadata>()
-        val metadataMap = LongSparseArray<BackupMetadata>()
         for (encryptedMetadata in availableBackups) {
             if (encryptedMetadata.error) continue
             check(encryptedMetadata.inputStream != null) {
@@ -74,9 +67,7 @@ internal class RestoreCoordinator(
                     encryptedMetadata.inputStream,
                     encryptedMetadata.token
                 )
-                metadataMap.put(encryptedMetadata.token, metadata)
+                metadataMap[encryptedMetadata.token] = metadata
-                val set = RestoreSet(metadata.deviceName, metadata.deviceName, metadata.token)
-                restoreSets.add(set)
             } catch (e: IOException) {
                 Log.e(TAG, "Error while getting restore set ${encryptedMetadata.token}", e)
                 continue
@@ -93,9 +84,20 @@ internal class RestoreCoordinator(
                 closeQuietly(encryptedMetadata.inputStream)
             }
         }
-        Log.i(TAG, "Got available restore sets: $restoreSets")
+        Log.i(TAG, "Got available metadata for tokens: ${metadataMap.keys}")
-        this.backupMetadata = metadataMap
+        return metadataMap
-        return restoreSets.toTypedArray()
+    }
+
+    /**
+     * Get the set of all backups currently available over this transport.
+     *
+     * @return Descriptions of the set of restore images available for this device,
+     *   or null if an error occurred (the attempt should be rescheduled).
+     **/
+    suspend fun getAvailableRestoreSets(): Array<RestoreSet>? {
+        return getAvailableMetadata()?.map { (_, metadata) ->
+            RestoreSet(metadata.deviceName, metadata.deviceName, metadata.token)
+        }?.toTypedArray()
     }
 
     /**
@@ -211,7 +213,8 @@ internal class RestoreCoordinator(
         } catch (e: IOException) {
             Log.e(TAG, "Error finding restore data for $packageName.", e)
             failedPackages.add(packageName)
-            return null
+            // don't return null and cause abort here, but try next package
+            return nextRestorePackage()
         }
         return RestoreDescription(packageName, type)
     }