From 1e3263ec549d841530c112e9d27aee81c416fb6a Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 17 Sep 2020 14:00:46 -0300 Subject: [PATCH] Fix bug where we could not do two subsequent restores This probably never showed in practice, but it can be triggered easily when testing with `adb shell bmgr restore`. --- .../seedvault/transport/restore/RestoreCoordinator.kt | 7 +++++-- .../transport/restore/RestoreCoordinatorTest.kt | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt index 2df92e93..94ce9e98 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt @@ -25,7 +25,7 @@ import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager import libcore.io.IoUtils.closeQuietly import java.io.IOException -private class RestoreCoordinatorState( +private data class RestoreCoordinatorState( val token: Long, val packages: Iterator, /** @@ -124,7 +124,7 @@ internal class RestoreCoordinator( * or [TRANSPORT_ERROR] (an error occurred, the restore should be aborted and rescheduled). */ fun startRestore(token: Long, packages: Array): Int { - check(state == null) { "Started new restore with existing state" } + check(state == null) { "Started new restore with existing state: $state" } Log.i(TAG, "Start restore with ${packages.map { info -> info.packageName }}") // If there's only one package to restore (Auto Restore feature), add it to the state @@ -251,6 +251,7 @@ internal class RestoreCoordinator( * or will call [finishRestore] to shut down the restore operation. */ fun abortFullRestore(): Int { + Log.d(TAG, "abortFullRestore") state?.currentPackage?.let { failedPackages.add(it) } return full.abortFullRestore() } @@ -260,7 +261,9 @@ internal class RestoreCoordinator( * freeing any resources and connections used during the restore process. */ fun finishRestore() { + Log.d(TAG, "finishRestore") if (full.hasState()) full.finishRestore() + state = null } /** diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt index ac93ad51..7124af37 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt @@ -108,6 +108,16 @@ internal class RestoreCoordinatorTest : TransportTest() { } } + @Test + fun `startRestore() can be be called again after restore finished`() { + assertEquals(TRANSPORT_OK, restore.startRestore(token, packageInfoArray)) + + every { full.hasState() } returns false + restore.finishRestore() + + assertEquals(TRANSPORT_OK, restore.startRestore(token, packageInfoArray)) + } + @Test fun `startRestore() optimized auto-restore with removed storage shows notification`() { every { settingsManager.getStorage() } returns storage