Treat stopped apps different from opt-out apps

Apps that have FLAG_STOPPED will not get backed up, just like apps
without flag ALLOW_BACKUP will not get backed up.
In the UI both cases are shown the same way: app does not allow backup
This can be confusing for the user as it is not true for stopped apps.
Therefore, this commit introduces a new stopped state for apps,
so we can differentiate between both cases.

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

@@ -1,5 +1,6 @@
 package com.stevesoltys.seedvault.metadata
 
+import android.content.pm.ApplicationInfo.FLAG_STOPPED
 import android.os.Build
 import com.stevesoltys.seedvault.header.VERSION
 import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
@@ -39,6 +40,10 @@ enum class PackageState {
      * Package data could not get backed up, because the app reported no data to back up.
      */
     NO_DATA,
+    /**
+     * Package data could not get backed up, because the app has [FLAG_STOPPED].
+     */
+    WAS_STOPPED,
     /**
      * Package data could not get backed up, because it was not allowed.
      * Most often, this is a manifest opt-out, but it could also be a disabled or system-user app.

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

@@ -13,6 +13,7 @@ import com.stevesoltys.seedvault.Clock
 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
+import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
 import com.stevesoltys.seedvault.transport.backup.isSystemApp
 import java.io.FileNotFoundException
 import java.io.IOException
@@ -84,12 +85,15 @@ class MetadataManager(
         }
         val oldPackageMetadata = metadata.packageMetadataMap[packageName]
             ?: PackageMetadata()
-        // only allow state change if backup of this package is not allowed
+        // only allow state change if backup of this package is not allowed,
-        val newState = if (packageMetadata.state == NOT_ALLOWED) {
+        // because we need to change from the default of UNKNOWN_ERROR here,
-            packageMetadata.state
+        // but otherwise don't want to modify the state since set elsewhere.
-        } else {
+        val newState =
-            oldPackageMetadata.state
+            if (packageMetadata.state == NOT_ALLOWED || packageMetadata.state == WAS_STOPPED) {
-        }
+                packageMetadata.state
+            } else {
+                oldPackageMetadata.state
+            }
         modifyMetadata(metadataOutputStream) {
             metadata.packageMetadataMap[packageName] = oldPackageMetadata.copy(
                 state = newState,

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

@@ -9,6 +9,7 @@ import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.metadata.PackageState.NO_DATA
 import com.stevesoltys.seedvault.metadata.PackageState.QUOTA_EXCEEDED
 import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
+import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
 import org.json.JSONException
 import org.json.JSONObject
 import java.io.IOException
@@ -64,11 +65,12 @@ internal class MetadataReaderImpl(private val crypto: Crypto) : MetadataReader {
             for (packageName in json.keys()) {
                 if (packageName == JSON_METADATA) continue
                 val p = json.getJSONObject(packageName)
-                val pState = when(p.optString(JSON_PACKAGE_STATE)) {
+                val pState = when (p.optString(JSON_PACKAGE_STATE)) {
                     "" -> APK_AND_DATA
                     QUOTA_EXCEEDED.name -> QUOTA_EXCEEDED
                     NO_DATA.name -> NO_DATA
                     NOT_ALLOWED.name -> NOT_ALLOWED
+                    WAS_STOPPED.name -> WAS_STOPPED
                     else -> UNKNOWN_ERROR
                 }
                 val pSystem = p.optBoolean(JSON_PACKAGE_SYSTEM, false)

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

@@ -71,7 +71,7 @@ internal class RestoreProgressAdapter : Adapter<PackageViewHolder>() {
 enum class AppRestoreStatus {
     IN_PROGRESS,
     SUCCEEDED,
-    NOT_ELIGIBLE,
+    NOT_YET_BACKED_UP,
     FAILED,
     FAILED_NO_DATA,
     FAILED_NOT_ALLOWED,

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

@@ -25,12 +25,14 @@ import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.metadata.PackageState.NO_DATA
 import com.stevesoltys.seedvault.metadata.PackageState.QUOTA_EXCEEDED
 import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
+import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED_NOT_ALLOWED
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED_NOT_INSTALLED
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED_NO_DATA
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED_QUOTA_EXCEEDED
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.IN_PROGRESS
+import com.stevesoltys.seedvault.restore.AppRestoreStatus.NOT_YET_BACKED_UP
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.SUCCEEDED
 import com.stevesoltys.seedvault.restore.DisplayFragment.RESTORE_APPS
 import com.stevesoltys.seedvault.restore.DisplayFragment.RESTORE_BACKUP
@@ -218,6 +220,7 @@ internal class RestoreViewModel(
         val metadata = restorableBackup.packageMetadataMap[packageName] ?: return FAILED
         return when (metadata.state) {
             NO_DATA -> FAILED_NO_DATA
+            WAS_STOPPED -> NOT_YET_BACKED_UP
             NOT_ALLOWED -> FAILED_NOT_ALLOWED
             QUOTA_EXCEEDED -> FAILED_QUOTA_EXCEEDED
             UNKNOWN_ERROR -> FAILED

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

@@ -23,11 +23,12 @@ import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.metadata.PackageState.NO_DATA
 import com.stevesoltys.seedvault.metadata.PackageState.QUOTA_EXCEEDED
 import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
+import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED_NOT_ALLOWED
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED_NO_DATA
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED_QUOTA_EXCEEDED
-import com.stevesoltys.seedvault.restore.AppRestoreStatus.NOT_ELIGIBLE
+import com.stevesoltys.seedvault.restore.AppRestoreStatus.NOT_YET_BACKED_UP
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.SUCCEEDED
 import com.stevesoltys.seedvault.transport.backup.PackageService
 import com.stevesoltys.seedvault.transport.requestBackup
@@ -97,9 +98,10 @@ internal class SettingsViewModel(
             val status = when (metadata?.state) {
                 null -> {
                     Log.w(TAG, "No metadata available for: ${it.packageName}")
-                    NOT_ELIGIBLE
+                    NOT_YET_BACKED_UP
                 }
                 NO_DATA -> FAILED_NO_DATA
+                WAS_STOPPED -> NOT_YET_BACKED_UP
                 NOT_ALLOWED -> FAILED_NOT_ALLOWED
                 QUOTA_EXCEEDED -> FAILED_QUOTA_EXCEEDED
                 UNKNOWN_ERROR -> FAILED

app/src/main/java/com/stevesoltys/seedvault/transport/ConfigurableBackupTransportService.kt

@@ -63,6 +63,7 @@ fun requestBackup(context: Context) {
     val observer = NotificationBackupObserver(context, packages.size, appTotals)
     val result = try {
         val backupManager: IBackupManager = get().koin.get()
+        // TODO check why this is not doing incremental K/V backups like `bmgr backupnow`
         backupManager.requestBackup(packages, observer, BackupMonitor(), FLAG_USER_INITIATED)
     } catch (e: RemoteException) {
         Log.e(TAG, "Error during backup: ", e)

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

@@ -10,6 +10,8 @@ import android.content.pm.PackageInfo
 import android.content.pm.PackageManager.GET_SIGNING_CERTIFICATES
 import android.os.ParcelFileDescriptor
 import android.util.Log
+import androidx.annotation.VisibleForTesting
+import androidx.annotation.VisibleForTesting.PRIVATE
 import androidx.annotation.WorkerThread
 import com.stevesoltys.seedvault.Clock
 import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
@@ -19,6 +21,7 @@ import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.metadata.PackageState.NO_DATA
 import com.stevesoltys.seedvault.metadata.PackageState.QUOTA_EXCEEDED
 import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
+import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
 import com.stevesoltys.seedvault.settings.SettingsManager
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
 import java.io.IOException
@@ -299,34 +302,54 @@ internal class BackupCoordinator(
         else -> throw IllegalStateException("Unexpected state in finishBackup()")
     }
 
-    private suspend fun backUpNotAllowedPackages() {
+    @VisibleForTesting(otherwise = PRIVATE)
+    internal suspend fun backUpNotAllowedPackages() {
         Log.d(TAG, "Checking if APKs of opt-out apps need backup...")
         val notAllowedPackages = packageService.notAllowedPackages
-        notAllowedPackages.forEachIndexed { i, optOutPackageInfo ->
+        notAllowedPackages.forEachIndexed { i, packageInfo ->
+            val packageName = packageInfo.packageName
             try {
-                nm.onOptOutAppBackup(optOutPackageInfo.packageName, i + 1, notAllowedPackages.size)
+                nm.onOptOutAppBackup(packageName, i + 1, notAllowedPackages.size)
-                backUpApk(optOutPackageInfo, NOT_ALLOWED)
+                val packageState = if (packageInfo.isStopped()) WAS_STOPPED else NOT_ALLOWED
+                val wasBackedUp = backUpApk(packageInfo, packageState)
+                if (!wasBackedUp) {
+                    val packageMetadata = metadataManager.getPackageMetadata(packageName)
+                    val oldPackageState = packageMetadata?.state
+                    if (oldPackageState != null && oldPackageState != packageState) {
+                        Log.e(TAG, "Package $packageName was in $oldPackageState, update to $packageState")
+                        plugin.getMetadataOutputStream().use {
+                            metadataManager.onPackageBackupError(packageInfo, packageState, it)
+                        }
+                    }
+                }
             } catch (e: IOException) {
-                Log.e(TAG, "Error backing up opt-out APK of ${optOutPackageInfo.packageName}", e)
+                Log.e(TAG, "Error backing up opt-out APK of $packageName", e)
             }
         }
     }
 
+    /**
+     * Backs up an APK for the given [PackageInfo].
+     *
+     * @return true if a backup was performed and false if no backup was needed or it failed.
+     */
     private suspend fun backUpApk(
         packageInfo: PackageInfo,
         packageState: PackageState = UNKNOWN_ERROR
-    ) {
+    ): Boolean {
         val packageName = packageInfo.packageName
-        try {
+        return try {
             apkBackup.backupApkIfNecessary(packageInfo, packageState) {
                 plugin.getApkOutputStream(packageInfo)
             }?.let { packageMetadata ->
                 plugin.getMetadataOutputStream().use {
                     metadataManager.onApkBackedUp(packageInfo, packageMetadata, it)
                 }
-            }
+                true
+            } ?: false
         } catch (e: IOException) {
             Log.e(TAG, "Error while writing APK or metadata for $packageName", e)
+            false
         }
     }
 

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

@@ -157,3 +157,8 @@ internal fun PackageInfo.doesNotGetBackedUp(): Boolean {
     return applicationInfo.flags and FLAG_ALLOW_BACKUP == 0 || // does not allow backup
             applicationInfo.flags and FLAG_STOPPED != 0 // is stopped
 }
+
+internal fun PackageInfo.isStopped(): Boolean {
+    if (packageName == MAGIC_PACKAGE_MANAGER || applicationInfo == null) return false
+    return applicationInfo.flags and FLAG_STOPPED != 0
+}

app/src/main/java/com/stevesoltys/seedvault/ui/AppViewHolder.kt

@@ -19,7 +19,7 @@ import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED_NOT_INSTALLED
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED_NO_DATA
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.FAILED_QUOTA_EXCEEDED
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.IN_PROGRESS
-import com.stevesoltys.seedvault.restore.AppRestoreStatus.NOT_ELIGIBLE
+import com.stevesoltys.seedvault.restore.AppRestoreStatus.NOT_YET_BACKED_UP
 import com.stevesoltys.seedvault.restore.AppRestoreStatus.SUCCEEDED
 
 
@@ -64,7 +64,7 @@ internal open class AppViewHolder(protected val v: View) : RecyclerView.ViewHold
     }
 
     private fun AppRestoreStatus.getInfo(): String = when (this) {
-        NOT_ELIGIBLE -> context.getString(R.string.restore_app_not_eligible)
+        NOT_YET_BACKED_UP -> context.getString(R.string.restore_app_not_yet_backed_up)
         FAILED_NO_DATA -> context.getString(R.string.restore_app_no_data)
         FAILED_NOT_ALLOWED -> context.getString(R.string.restore_app_not_allowed)
         FAILED_NOT_INSTALLED -> context.getString(R.string.restore_app_not_installed)

app/src/main/res/values/strings.xml

@@ -106,7 +106,7 @@
     <string name="restore_restoring">Restoring backup</string>
     <string name="restore_magic_package">System package manager</string>
     <!-- This text gets shown for apps that the OS did not try to backup for whatever reason e.g. no backup was run yet -->
-    <string name="restore_app_not_eligible">Not yet backed up</string>
+    <string name="restore_app_not_yet_backed_up">Not yet backed up</string>
     <string name="restore_app_no_data">App reported no data for backup</string>
     <string name="restore_app_not_allowed">App doesn\'t allow backup</string>
     <string name="restore_app_not_installed">App not installed</string>

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

@@ -15,6 +15,7 @@ import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.metadata.PackageState.NO_DATA
 import com.stevesoltys.seedvault.metadata.PackageState.QUOTA_EXCEEDED
 import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
+import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
 import io.mockk.Runs
 import io.mockk.every
 import io.mockk.just
@@ -163,6 +164,11 @@ class MetadataManagerTest {
         packageMetadata = packageMetadata.copy(version = ++version, state = NOT_ALLOWED)
         manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
         assertEquals(packageMetadata.copy(state = NOT_ALLOWED), manager.getPackageMetadata(packageName))
+
+        // state DOES change for WAS_STOPPED
+        packageMetadata = packageMetadata.copy(version = ++version, state = WAS_STOPPED)
+        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
+        assertEquals(packageMetadata.copy(state = WAS_STOPPED), manager.getPackageMetadata(packageName))
     }
 
     @Test

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

@@ -6,6 +6,7 @@ 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
 import com.stevesoltys.seedvault.metadata.PackageState.QUOTA_EXCEEDED
+import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
 import io.mockk.mockk
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Test
@@ -32,6 +33,7 @@ internal class MetadataWriterDecoderTest {
         val time = Random.nextLong()
         val packages = HashMap<String, PackageMetadata>().apply {
             put(getRandomString(), PackageMetadata(time, APK_AND_DATA))
+            put(getRandomString(), PackageMetadata(time, WAS_STOPPED))
         }
         val metadata = getMetadata(packages)
         assertEquals(metadata, decoder.decode(encoder.encode(metadata), metadata.version, metadata.token))

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

@@ -4,6 +4,8 @@ import android.app.backup.BackupTransport.TRANSPORT_ERROR
 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.content.pm.ApplicationInfo
+import android.content.pm.ApplicationInfo.FLAG_STOPPED
 import android.content.pm.PackageInfo
 import android.net.Uri
 import android.os.ParcelFileDescriptor
@@ -16,6 +18,7 @@ import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import com.stevesoltys.seedvault.metadata.PackageState.NO_DATA
 import com.stevesoltys.seedvault.metadata.PackageState.QUOTA_EXCEEDED
 import com.stevesoltys.seedvault.metadata.PackageState.UNKNOWN_ERROR
+import com.stevesoltys.seedvault.metadata.PackageState.WAS_STOPPED
 import com.stevesoltys.seedvault.settings.Storage
 import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager
 import io.mockk.Runs
@@ -318,7 +321,11 @@ internal class BackupCoordinatorTest : BackupTest() {
         val packageInfo = PackageInfo().apply { packageName = MAGIC_PACKAGE_MANAGER }
         val notAllowedPackages = listOf(
             PackageInfo().apply { packageName = "org.example.1" },
-            PackageInfo().apply { packageName = "org.example.2" }
+            PackageInfo().apply {
+                packageName = "org.example.2"
+                // the second package does not get backed up, because it is stopped
+                applicationInfo = ApplicationInfo().apply { flags = FLAG_STOPPED }
+            }
         )
         val packageMetadata: PackageMetadata = mockk()
 
@@ -337,13 +344,13 @@ internal class BackupCoordinatorTest : BackupTest() {
         } just Runs
         // no backup needed
         coEvery {
-            apkBackup.backupApkIfNecessary(
+            apkBackup.backupApkIfNecessary(notAllowedPackages[0], NOT_ALLOWED, any())
-                notAllowedPackages[0],
-                NOT_ALLOWED,
-                any()
-            )
         } returns null
-        // update notification
+        // check old metadata for state changes, because we won't update it otherwise
+        every { metadataManager.getPackageMetadata(notAllowedPackages[0].packageName) } returns packageMetadata
+        every { packageMetadata.state } returns NOT_ALLOWED // no change
+
+        // update notification for second package
         every {
             notificationManager.onOptOutAppBackup(
                 notAllowedPackages[1].packageName,
@@ -353,11 +360,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         } just Runs
         // was backed up, get new packageMetadata
         coEvery {
-            apkBackup.backupApkIfNecessary(
+            apkBackup.backupApkIfNecessary(notAllowedPackages[1], WAS_STOPPED, any())
-                notAllowedPackages[1],
-                NOT_ALLOWED,
-                any()
-            )
         } returns packageMetadata
         coEvery { plugin.getMetadataOutputStream() } returns metadataOutputStream
         every {
@@ -369,14 +372,38 @@ internal class BackupCoordinatorTest : BackupTest() {
         } just Runs
         every { metadataOutputStream.close() } just Runs
 
-        assertEquals(
+        assertEquals(TRANSPORT_OK, backup.performIncrementalBackup(packageInfo, fileDescriptor, 0))
-            TRANSPORT_OK,
-            backup.performIncrementalBackup(packageInfo, fileDescriptor, 0)
-        )
 
         coVerify {
             apkBackup.backupApkIfNecessary(notAllowedPackages[0], NOT_ALLOWED, any())
-            apkBackup.backupApkIfNecessary(notAllowedPackages[1], NOT_ALLOWED, any())
+            apkBackup.backupApkIfNecessary(notAllowedPackages[1], WAS_STOPPED, any())
+            metadataOutputStream.close()
+        }
+    }
+
+    @Test
+    fun `APK backup of not allowed apps updates state even without new APK`() = runBlocking {
+        val oldPackageMetadata: PackageMetadata = mockk()
+
+        every { packageService.notAllowedPackages } returns listOf(packageInfo)
+        every { notificationManager.onOptOutAppBackup(packageInfo.packageName, 1, 1) } just Runs
+        coEvery { apkBackup.backupApkIfNecessary(packageInfo, NOT_ALLOWED, any()) } returns null
+        every { metadataManager.getPackageMetadata(packageInfo.packageName) } returns oldPackageMetadata
+        every { oldPackageMetadata.state } returns WAS_STOPPED // state differs now, was stopped before
+        coEvery { plugin.getMetadataOutputStream() } returns metadataOutputStream
+        every {
+            metadataManager.onPackageBackupError(
+                packageInfo,
+                NOT_ALLOWED,
+                metadataOutputStream
+            )
+        } just Runs
+        every { metadataOutputStream.close() } just Runs
+
+        backup.backUpNotAllowedPackages()
+
+        verify {
+            metadataManager.onPackageBackupError(packageInfo, NOT_ALLOWED, metadataOutputStream)
             metadataOutputStream.close()
         }
     }