Always start a new RestoreSet when initializing the device

This avoids deleting the current backup when the user disables backups (or the system decides to do a random re-init).
This commit is contained in:
Torsten Grote 2023-01-03 16:29:17 -03:00 committed by Chirayu Desai
parent bedba071df
commit 87a800438b
4 changed files with 19 additions and 37 deletions

View file

@ -88,13 +88,16 @@ internal class BackupCoordinator(
* Starts a new [RestoreSet] with a new token (the current unix epoch in milliseconds). * Starts a new [RestoreSet] with a new token (the current unix epoch in milliseconds).
* Call this at least once before calling [initializeDevice] * Call this at least once before calling [initializeDevice]
* which must be called after this method to properly initialize the backup transport. * which must be called after this method to properly initialize the backup transport.
*
* @return the token of the new [RestoreSet].
*/ */
@Throws(IOException::class) @Throws(IOException::class)
suspend fun startNewRestoreSet() { private suspend fun startNewRestoreSet(): Long {
val token = clock.time() val token = clock.time()
Log.i(TAG, "Starting new RestoreSet with token $token...") Log.i(TAG, "Starting new RestoreSet with token $token...")
settingsManager.setNewToken(token) settingsManager.setNewToken(token)
plugin.startNewRestoreSet(token) plugin.startNewRestoreSet(token)
return token
} }
/** /**
@ -116,16 +119,14 @@ internal class BackupCoordinator(
* [TRANSPORT_ERROR] (to retry following network error or other failure). * [TRANSPORT_ERROR] (to retry following network error or other failure).
*/ */
suspend fun initializeDevice(): Int = try { suspend fun initializeDevice(): Int = try {
val token = settingsManager.getToken() // we don't respect the intended system behavior here by always starting a new [RestoreSet]
if (token == null) { // instead of simply deleting the current one
Log.i(TAG, "No RestoreSet started, initialization is no-op.") val token = startNewRestoreSet()
} else { Log.i(TAG, "Initialize Device!")
Log.i(TAG, "Initialize Device!") plugin.initializeDevice()
plugin.initializeDevice() Log.d(TAG, "Resetting backup metadata for token $token...")
Log.d(TAG, "Resetting backup metadata for token $token...") plugin.getMetadataOutputStream(token).use {
plugin.getMetadataOutputStream().use { metadataManager.onDeviceInitialization(token, it)
metadataManager.onDeviceInitialization(token, it)
}
} }
// [finishBackup] will only be called when we return [TRANSPORT_OK] here // [finishBackup] will only be called when we return [TRANSPORT_OK] here
// so we remember that we initialized successfully // so we remember that we initialized successfully

View file

@ -107,9 +107,6 @@ internal class RecoveryCodeViewModel(
storageBackup.deleteAllSnapshots() storageBackup.deleteAllSnapshots()
storageBackup.clearCache() storageBackup.clearCache()
try { try {
// will also generate a new backup token for the new restore set
backupCoordinator.startNewRestoreSet()
// initialize the new location // initialize the new location
if (backupManager.isBackupEnabled) backupManager.initializeTransportsForUser( if (backupManager.isBackupEnabled) backupManager.initializeTransportsForUser(
UserHandle.myUserId(), UserHandle.myUserId(),

View file

@ -38,9 +38,6 @@ internal class BackupStorageViewModel(
storageBackup.deleteAllSnapshots() storageBackup.deleteAllSnapshots()
storageBackup.clearCache() storageBackup.clearCache()
try { try {
// will also generate a new backup token for the new restore set
backupCoordinator.startNewRestoreSet()
// initialize the new location (if backups are enabled) // initialize the new location (if backups are enabled)
if (backupManager.isBackupEnabled) backupManager.initializeTransportsForUser( if (backupManager.isBackupEnabled) backupManager.initializeTransportsForUser(
UserHandle.myUserId(), UserHandle.myUserId(),

View file

@ -72,18 +72,9 @@ internal class BackupCoordinatorTest : BackupTest() {
requiresNetwork = false requiresNetwork = false
) )
@Test
fun `starting a new restore set works as expected`() = runBlocking {
every { clock.time() } returns token
every { settingsManager.setNewToken(token) } just Runs
coEvery { plugin.startNewRestoreSet(token) } just Runs
backup.startNewRestoreSet()
}
@Test @Test
fun `device initialization succeeds and delegates to plugin`() = runBlocking { fun `device initialization succeeds and delegates to plugin`() = runBlocking {
every { settingsManager.getToken() } returns token expectStartNewRestoreSet()
coEvery { plugin.initializeDevice() } just Runs coEvery { plugin.initializeDevice() } just Runs
coEvery { plugin.getOutputStream(token, FILE_BACKUP_METADATA) } returns metadataOutputStream coEvery { plugin.getOutputStream(token, FILE_BACKUP_METADATA) } returns metadataOutputStream
every { metadataManager.onDeviceInitialization(token, metadataOutputStream) } just Runs every { metadataManager.onDeviceInitialization(token, metadataOutputStream) } just Runs
@ -97,21 +88,17 @@ internal class BackupCoordinatorTest : BackupTest() {
verify { metadataOutputStream.close() } verify { metadataOutputStream.close() }
} }
@Test private suspend fun expectStartNewRestoreSet() {
fun `device initialization does no-op when no token available`() = runBlocking { every { clock.time() } returns token
every { settingsManager.getToken() } returns null every { settingsManager.setNewToken(token) } just Runs
every { kv.hasState() } returns false coEvery { plugin.startNewRestoreSet(token) } just Runs
every { full.hasState() } returns false
assertEquals(TRANSPORT_OK, backup.initializeDevice())
assertEquals(TRANSPORT_OK, backup.finishBackup())
} }
@Test @Test
fun `error notification when device initialization fails`() = runBlocking { fun `error notification when device initialization fails`() = runBlocking {
val maybeTrue = Random.nextBoolean() val maybeTrue = Random.nextBoolean()
every { settingsManager.getToken() } returns token expectStartNewRestoreSet()
coEvery { plugin.initializeDevice() } throws IOException() coEvery { plugin.initializeDevice() } throws IOException()
every { metadataManager.requiresInit } returns maybeTrue every { metadataManager.requiresInit } returns maybeTrue
every { settingsManager.canDoBackupNow() } returns !maybeTrue every { settingsManager.canDoBackupNow() } returns !maybeTrue
@ -130,7 +117,7 @@ internal class BackupCoordinatorTest : BackupTest() {
@Test @Test
fun `no error notification when device initialization fails when no backup possible`() = fun `no error notification when device initialization fails when no backup possible`() =
runBlocking { runBlocking {
every { settingsManager.getToken() } returns token expectStartNewRestoreSet()
coEvery { plugin.initializeDevice() } throws IOException() coEvery { plugin.initializeDevice() } throws IOException()
every { metadataManager.requiresInit } returns false every { metadataManager.requiresInit } returns false
every { settingsManager.canDoBackupNow() } returns false every { settingsManager.canDoBackupNow() } returns false