Simplify transport init and token handling

The token used to be very important, because it was our restore set folder name. Now it is just a number in a snapshot, so things get a bit simpler.

app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt

@@ -95,7 +95,7 @@ class PluginTest : KoinComponent {
         assertEquals(0, backend.getAvailableBackupFileHandles().toList().size)
 
         // prepare returned tokens requested when initializing device
-        every { mockedSettingsManager.getToken() } returnsMany listOf(token, token + 1, token + 1)
+        every { mockedSettingsManager.token } returnsMany listOf(token, token + 1, token + 1)
 
         // write metadata (needed for backup to be recognized)
         backend.save(LegacyAppBackupFile.Metadata(token))
@@ -117,7 +117,7 @@ class PluginTest : KoinComponent {
 
     @Test
     fun testMetadataWriteRead() = runBlocking(Dispatchers.IO) {
-        every { mockedSettingsManager.getToken() } returns token
+        every { mockedSettingsManager.token } returns token
 
         // write metadata
         val metadata = getRandomByteArray()
@@ -201,7 +201,7 @@ class PluginTest : KoinComponent {
     }
 
     private fun initStorage(token: Long) = runBlocking {
-        every { mockedSettingsManager.getToken() } returns token
+        every { mockedSettingsManager.token } returns token
     }
 
 }

app/src/androidTest/java/com/stevesoltys/seedvault/e2e/LargeTestBase.kt

@@ -85,7 +85,7 @@ internal interface LargeTestBase : KoinComponent {
 
     fun resetApplicationState() {
         backupManager.setAutoRestore(false)
-        settingsManager.setNewToken(null)
+        settingsManager.token = null
 
         val sharedPreferences = permitDiskReads {
             PreferenceManager.getDefaultSharedPreferences(targetContext)

app/src/main/java/com/stevesoltys/seedvault/App.kt

@@ -125,9 +125,6 @@ open class App : Application() {
                     .build()
             )
         }
-        permitDiskReads {
-            migrateTokenFromMetadataToSettingsManager()
-        }
         if (!isTest) migrateToOwnScheduling()
     }
 
@@ -153,25 +150,10 @@ open class App : Application() {
     )
 
     private val settingsManager: SettingsManager by inject()
-    private val metadataManager: MetadataManager by inject()
     private val backupManager: IBackupManager by inject()
     private val backendManager: BackendManager by inject()
     private val backupStateManager: BackupStateManager by inject()
 
-    /**
-     * The responsibility for the current token was moved to the [SettingsManager]
-     * in the end of 2020.
-     * This method migrates the token for existing installs and can be removed
-     * after sufficient time has passed.
-     */
-    private fun migrateTokenFromMetadataToSettingsManager() {
-        @Suppress("DEPRECATION")
-        val token = metadataManager.getBackupToken()
-        if (token != 0L && settingsManager.getToken() == null) {
-            settingsManager.setNewToken(token)
-        }
-    }
-
     /**
      * Disables the framework scheduling in favor of our own.
      * Introduced in the first half of 2024 and can be removed after a suitable migration period.

app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataManager.kt

@@ -8,9 +8,6 @@ package com.stevesoltys.seedvault.metadata
 import android.content.Context
 import android.content.Context.MODE_PRIVATE
 import android.content.pm.PackageInfo
-import android.content.pm.PackageManager.PERMISSION_GRANTED
-import android.os.Build
-import android.os.UserManager
 import android.util.Log
 import androidx.annotation.VisibleForTesting
 import androidx.annotation.WorkerThread
@@ -18,8 +15,6 @@ import androidx.lifecycle.LiveData
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.distinctUntilChanged
 import com.stevesoltys.seedvault.Clock
-import com.stevesoltys.seedvault.crypto.Crypto
-import com.stevesoltys.seedvault.encodeBase64
 import com.stevesoltys.seedvault.header.VERSION
 import com.stevesoltys.seedvault.metadata.PackageState.APK_AND_DATA
 import com.stevesoltys.seedvault.settings.SettingsManager
@@ -39,14 +34,13 @@ internal const val METADATA_SALT_SIZE = 32
 internal class MetadataManager(
     private val context: Context,
     private val clock: Clock,
-    private val crypto: Crypto,
     private val metadataWriter: MetadataWriter,
     private val metadataReader: MetadataReader,
     private val packageService: PackageService,
     private val settingsManager: SettingsManager,
 ) {
 
-    private val uninitializedMetadata = BackupMetadata(token = 0L, salt = "")
+    private val uninitializedMetadata = BackupMetadata(token = -42L, salt = "foo bar")
     private var metadata: BackupMetadata = uninitializedMetadata
         get() {
             if (field == uninitializedMetadata) {
@@ -65,37 +59,10 @@ internal class MetadataManager(
             return field
         }
 
-    val backupSize: Long get() = metadata.size
-
     private val launchableSystemApps by lazy {
         packageService.launchableSystemApps.map { it.activityInfo.packageName }.toSet()
     }
 
-    /**
-     * Call this when initializing a new device.
-     *
-     * Existing [BackupMetadata] will be cleared
-     * and new metadata with the given [token] will be written to the internal cache
-     * with a fresh salt.
-     */
-    @Synchronized
-    @Throws(IOException::class)
-    fun onDeviceInitialization(token: Long) {
-        val salt = crypto.getRandomBytes(METADATA_SALT_SIZE).encodeBase64()
-        modifyCachedMetadata {
-            val userName = getUserName()
-            metadata = BackupMetadata(
-                token = token,
-                salt = salt,
-                deviceName = if (userName == null) {
-                    "${Build.MANUFACTURER} ${Build.MODEL}"
-                } else {
-                    "${Build.MANUFACTURER} ${Build.MODEL} - $userName"
-                },
-            )
-        }
-    }
-
     /**
      * Call this after a package's APK has been backed up successfully.
      *
@@ -253,18 +220,6 @@ internal class MetadataManager(
         mLastBackupTime.postValue(metadata.time) // TODO only do after snapshot was written
     }
 
-    /**
-     * Returns the current backup token.
-     *
-     * If the token is 0L, it is not yet initialized and must not be used for anything.
-     */
-    @Synchronized
-    @Deprecated(
-        "Responsibility for current token moved to SettingsManager",
-        ReplaceWith("settingsManager.getToken()")
-    )
-    fun getBackupToken(): Long = metadata.token
-
     /**
      * Returns the last backup time in unix epoch milli seconds.
      *
@@ -317,12 +272,4 @@ internal class MetadataManager(
         }
     }
 
-    private fun getUserName(): String? {
-        val perm = "android.permission.QUERY_USERS"
-        return if (context.checkSelfPermission(perm) == PERMISSION_GRANTED) {
-            val userManager = context.getSystemService(UserManager::class.java) ?: return null
-            userManager.userName
-        } else null
-    }
-
 }

app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataModule.kt

@@ -9,7 +9,7 @@ import org.koin.android.ext.koin.androidContext
 import org.koin.dsl.module
 
 val metadataModule = module {
-    single { MetadataManager(androidContext(), get(), get(), get(), get(), get(), get()) }
+    single { MetadataManager(androidContext(), get(), get(), get(), get(), get()) }
     single<MetadataWriter> { MetadataWriterImpl(get()) }
     single<MetadataReader> { MetadataReaderImpl(get()) }
 }

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

@@ -85,12 +85,6 @@ internal class AppDataRestoreManager(
 
         Log.d(TAG, "Starting new restore session to restore backup $token")
 
-        // if we had no token before (i.e. restore from setup wizard),
-        // use the token of the current restore set from now on
-        if (settingsManager.getToken() == null) {
-            settingsManager.setNewToken(token)
-        }
-
         // start a new restore session
         val session = try {
             getOrStartSession()

app/src/main/java/com/stevesoltys/seedvault/settings/SettingsManager.kt

@@ -13,7 +13,6 @@ import androidx.annotation.UiThread
 import androidx.preference.PreferenceManager
 import com.stevesoltys.seedvault.backend.webdav.WebDavHandler.Companion.createWebDavProperties
 import com.stevesoltys.seedvault.permitDiskReads
-import com.stevesoltys.seedvault.transport.backup.BackupCoordinator
 import org.calyxos.seedvault.core.backends.Backend
 import org.calyxos.seedvault.core.backends.saf.SafBackend
 import org.calyxos.seedvault.core.backends.saf.SafProperties
@@ -63,9 +62,6 @@ class SettingsManager(private val context: Context) {
         PreferenceManager.getDefaultSharedPreferences(context)
     }
 
-    @Volatile
-    private var token: Long? = null
-
     fun registerOnSharedPreferenceChangeListener(listener: OnSharedPreferenceChangeListener) {
         prefs.registerOnSharedPreferenceChangeListener(listener)
     }
@@ -83,29 +79,26 @@ class SettingsManager(private val context: Context) {
         ConcurrentSkipListSet(prefs.getStringSet(PREF_KEY_BACKUP_APP_BLACKLIST, emptySet()))
     }
 
-    fun getToken(): Long? = token ?: run {
+    @Volatile
-        val value = prefs.getLong(PREF_KEY_TOKEN, 0L)
+    var token: Long? = null
-        if (value == 0L) null else value
+        set(newToken) {
-    }
+            if (newToken == null) {
-
+                prefs.edit()
-    /**
+                    .remove(PREF_KEY_TOKEN)
-     * Sets a new RestoreSet token.
+                    .apply()
-     * Should only be called by the [BackupCoordinator]
+            } else {
-     * to ensure that related work is performed after moving to a new token.
+                prefs.edit()
-     */
+                    .putLong(PREF_KEY_TOKEN, newToken)
-    fun setNewToken(newToken: Long?) {
+                    .apply()
-        if (newToken == null) {
+            }
-            prefs.edit()
+            field = newToken
-                .remove(PREF_KEY_TOKEN)
+        }
-                .apply()
+        // we may be able to get this from latest snapshot,
-        } else {
+        // but that is not always readily available
-            prefs.edit()
+        get() = field ?: run {
-                .putLong(PREF_KEY_TOKEN, newToken)
+            val value = prefs.getLong(PREF_KEY_TOKEN, 0L)
-                .apply()
+            if (value == 0L) null else value
         }
-
-        token = newToken
-    }
 
     internal val storagePluginType: StoragePluginType?
         get() {

app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt

@@ -15,7 +15,6 @@ import android.app.backup.BackupTransport.TRANSPORT_NOT_INITIALIZED
 import android.app.backup.BackupTransport.TRANSPORT_OK
 import android.app.backup.BackupTransport.TRANSPORT_PACKAGE_REJECTED
 import android.app.backup.BackupTransport.TRANSPORT_QUOTA_EXCEEDED
-import android.app.backup.RestoreSet
 import android.content.Context
 import android.content.pm.PackageInfo
 import android.os.ParcelFileDescriptor
@@ -84,22 +83,6 @@ internal class BackupCoordinator(
     // Transport initialization and quota
     //
 
-    /**
-     * Starts a new [RestoreSet] with a new token (the current unix epoch in milliseconds).
-     * Call this at least once before calling [initializeDevice]
-     * which must be called after this method to properly initialize the backup transport.
-     *
-     * @return the token of the new [RestoreSet].
-     */
-    @Throws(IOException::class)
-    private suspend fun startNewRestoreSet() {
-        val token = clock.time()
-        Log.i(TAG, "Starting new RestoreSet with token $token...")
-        settingsManager.setNewToken(token)
-        Log.d(TAG, "Resetting backup metadata...")
-        metadataManager.onDeviceInitialization(token)
-    }
-
     /**
      * Initialize the storage for this device, erasing all stored data.
      * The transport may send the request immediately, or may buffer it.
@@ -118,20 +101,15 @@ internal class BackupCoordinator(
      * @return One of [TRANSPORT_OK] (OK so far) or
      * [TRANSPORT_ERROR] (to retry following network error or other failure).
      */
-    suspend fun initializeDevice(): Int = try {
+    fun initializeDevice(): Int {
         // we don't respect the intended system behavior here by always starting a new [RestoreSet]
         // instead of simply deleting the current one
-        startNewRestoreSet()
         Log.i(TAG, "Initialize Device!")
+
         // [finishBackup] will only be called when we return [TRANSPORT_OK] here
         // so we remember that we initialized successfully
         state.calledInitialize = true
-        TRANSPORT_OK
+        return TRANSPORT_OK
-    } catch (e: Exception) {
-        Log.e(TAG, "Error initializing device", e)
-        // Show error notification if we needed init or were ready for backups
-        if (metadataManager.requiresInit || backendManager.canDoBackupNow()) nm.onBackupError()
-        TRANSPORT_ERROR
     }
 
     fun isAppEligibleForBackup(

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

@@ -162,10 +162,9 @@ internal class RestoreCoordinator(
      * or 0 if there is no backup set available corresponding to the current device state.
      */
     fun getCurrentRestoreSet(): Long {
-        Log.d(TAG, "getCurrentRestoreSet() = ") // TODO where to store current token?
+        val token = settingsManager.token ?: 0L
-        return (settingsManager.getToken() ?: 0L).apply {
+        Log.d(TAG, "getCurrentRestoreSet() = $token")
-            Log.i(TAG, "Got current restore set token: $this")
+        return token
-        }
     }
 
     /**

app/src/test/java/com/stevesoltys/seedvault/metadata/MetadataManagerTest.kt

@@ -14,7 +14,6 @@ import android.content.pm.ApplicationInfo.FLAG_SYSTEM
 import android.content.pm.PackageInfo
 import android.content.pm.PackageManager
 import android.content.pm.ResolveInfo
-import android.os.UserManager
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.stevesoltys.seedvault.Clock
 import com.stevesoltys.seedvault.TestApp
@@ -72,7 +71,6 @@ class MetadataManagerTest {
     private val manager = MetadataManager(
         context = context,
         clock = clock,
-        crypto = crypto,
         metadataWriter = metadataWriter,
         metadataReader = metadataReader,
         packageService = packageService,
@@ -106,59 +104,6 @@ class MetadataManagerTest {
         stopKoin()
     }
 
-    @Test
-    fun `test onDeviceInitialization() without user permission`() {
-        every { clock.time() } returns time
-        every { crypto.getRandomBytes(METADATA_SALT_SIZE) } returns saltBytes
-        expectReadFromCache()
-        expectModifyMetadata(initialMetadata)
-
-        every {
-            context.checkSelfPermission("android.permission.QUERY_USERS")
-        } returns PackageManager.PERMISSION_DENIED
-
-        manager.onDeviceInitialization(token)
-
-        assertEquals(token, manager.getBackupToken())
-        assertEquals(0L, manager.getLastBackupTime())
-
-        verify {
-            cacheInputStream.close()
-            cacheOutputStream.close()
-        }
-    }
-
-    @Test
-    fun `test onDeviceInitialization() with user permission`() {
-        val userManager: UserManager = mockk()
-        val userName = getRandomString()
-        val newMetadata = initialMetadata.copy(
-            deviceName = initialMetadata.deviceName + " - $userName",
-        )
-
-        every { clock.time() } returns time
-        every { crypto.getRandomBytes(METADATA_SALT_SIZE) } returns saltBytes
-        expectReadFromCache()
-        expectModifyMetadata(newMetadata)
-
-        every {
-            context.checkSelfPermission("android.permission.QUERY_USERS")
-        } returns PackageManager.PERMISSION_GRANTED
-        every { context.getSystemService(UserManager::class.java) } returns userManager
-        every { userManager.userName } returns userName
-
-        manager.onDeviceInitialization(token)
-
-        assertEquals(token, manager.getBackupToken())
-        assertEquals(0L, manager.getLastBackupTime())
-
-        verify {
-            cacheInputStream.close()
-            cacheOutputStream.close()
-            userManager.userName
-        }
-    }
-
     @Test
     fun `test onApkBackedUp() with no prior package metadata`() {
         val packageMetadata = PackageMetadata(
@@ -503,7 +448,6 @@ class MetadataManagerTest {
         expectReadFromCache()
 
         assertEquals(initialMetadata.time, manager.getLastBackupTime())
-        assertEquals(initialMetadata.token, manager.getBackupToken())
 
         verify { cacheInputStream.close() }
     }

app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt

@@ -5,7 +5,6 @@
 
 package com.stevesoltys.seedvault.transport.backup
 
-import android.app.backup.BackupTransport.TRANSPORT_ERROR
 import android.app.backup.BackupTransport.TRANSPORT_NOT_INITIALIZED
 import android.app.backup.BackupTransport.TRANSPORT_OK
 import android.app.backup.BackupTransport.TRANSPORT_PACKAGE_REJECTED
@@ -15,7 +14,6 @@ import android.net.Uri
 import android.os.ParcelFileDescriptor
 import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.backend.BackendManager
-import com.stevesoltys.seedvault.coAssertThrows
 import com.stevesoltys.seedvault.getRandomString
 import com.stevesoltys.seedvault.metadata.BackupType
 import com.stevesoltys.seedvault.metadata.PackageMetadata
@@ -78,7 +76,6 @@ internal class BackupCoordinatorTest : BackupTest() {
 
     @Test
     fun `device initialization succeeds and delegates to plugin`() = runBlocking {
-        expectStartNewRestoreSet()
         every { kv.hasState } returns false
         every { full.hasState } returns false
 
@@ -86,52 +83,6 @@ internal class BackupCoordinatorTest : BackupTest() {
         assertEquals(TRANSPORT_OK, backup.finishBackup())
     }
 
-    private fun expectStartNewRestoreSet() {
-        every { clock.time() } returns token
-        every { settingsManager.setNewToken(token) } just Runs
-        every { metadataManager.onDeviceInitialization(token) } just Runs
-    }
-
-    @Test
-    fun `error notification when device initialization fails`() = runBlocking {
-        val maybeTrue = Random.nextBoolean()
-
-        every { clock.time() } returns token
-        every { settingsManager.setNewToken(token) } just Runs
-        every { metadataManager.onDeviceInitialization(token) } throws IOException()
-        every { metadataManager.requiresInit } returns maybeTrue
-        every { backendManager.canDoBackupNow() } returns !maybeTrue
-        every { notificationManager.onBackupError() } just Runs
-
-        assertEquals(TRANSPORT_ERROR, backup.initializeDevice())
-
-        // finish will only be called when TRANSPORT_OK is returned, so it should throw
-        every { kv.hasState } returns false
-        every { full.hasState } returns false
-        coAssertThrows(IllegalStateException::class.java) {
-            backup.finishBackup()
-        }
-    }
-
-    @Test
-    fun `no error notification when device initialization fails when no backup possible`() =
-        runBlocking {
-            every { clock.time() } returns token
-            every { settingsManager.setNewToken(token) } just Runs
-            every { metadataManager.onDeviceInitialization(token) } throws IOException()
-            every { metadataManager.requiresInit } returns false
-            every { backendManager.canDoBackupNow() } returns false
-
-            assertEquals(TRANSPORT_ERROR, backup.initializeDevice())
-
-            // finish will only be called when TRANSPORT_OK is returned, so it should throw
-            every { kv.hasState } returns false
-            every { full.hasState } returns false
-            coAssertThrows(IllegalStateException::class.java) {
-                backup.finishBackup()
-            }
-        }
-
     @Test
     fun `performIncrementalBackup of @pm@ causes re-init when legacy format`() = runBlocking {
         val packageInfo = PackageInfo().apply { packageName = MAGIC_PACKAGE_MANAGER }
@@ -139,11 +90,6 @@ internal class BackupCoordinatorTest : BackupTest() {
         every { backendManager.canDoBackupNow() } returns true
         every { metadataManager.requiresInit } returns true
 
-        // start new restore set
-        every { clock.time() } returns token + 1
-        every { settingsManager.setNewToken(token + 1) } just Runs
-        every { metadataManager.onDeviceInitialization(token + 1) } just Runs
-
         every { data.close() } just Runs
 
         // returns TRANSPORT_NOT_INITIALIZED to re-init next time
@@ -203,7 +149,6 @@ internal class BackupCoordinatorTest : BackupTest() {
         every { kv.currentPackageInfo } returns packageInfo
         coEvery { kv.finishBackup() } throws IOException()
 
-        every { settingsManager.getToken() } returns token
         every {
             metadataManager.onPackageBackupError(
                 packageInfo,
@@ -342,7 +287,6 @@ internal class BackupCoordinatorTest : BackupTest() {
 
     private fun expectApkBackupAndMetadataWrite() {
         coEvery { apkBackup.backupApkIfNecessary(packageInfo) } just Runs
-        every { settingsManager.getToken() } returns token
         every { metadataManager.onApkBackedUp(any(), packageMetadata) } just Runs
     }
 

app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt

@@ -135,7 +135,7 @@ internal class RestoreCoordinatorTest : TransportTest() {
 
     @Test
     fun `getCurrentRestoreSet() delegates to plugin`() {
-        every { settingsManager.getToken() } returns token
+        every { settingsManager.token } returns token
         assertEquals(token, restore.getCurrentRestoreSet())
     }