Change UI for re-installation of system apps

We are re-installing system apps if they are present on the restore device as a system app and have a newer version code.
Before, when one of those conditions is not true, we were showing a failure and gave the user the option to re-install the app from an app store.
Now, we don't offer the manual re-install option anymore and only show a success when a newer or same version of the system app is already installed.

app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt

@@ -1,15 +1,16 @@
 package com.stevesoltys.seedvault.restore.install
 
 import android.content.Context
+import android.content.pm.PackageManager
 import android.content.pm.PackageManager.GET_SIGNATURES
 import android.content.pm.PackageManager.GET_SIGNING_CERTIFICATES
-import android.content.pm.PackageManager.NameNotFoundException
 import android.util.Log
 import com.stevesoltys.seedvault.metadata.PackageMetadata
 import com.stevesoltys.seedvault.metadata.PackageMetadataMap
-import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED
+import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED_SYSTEM_APP
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.IN_PROGRESS
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.QUEUED
+import com.stevesoltys.seedvault.restore.install.ApkInstallState.SUCCEEDED
 import com.stevesoltys.seedvault.transport.backup.copyStreamsAndGetHash
 import com.stevesoltys.seedvault.transport.backup.getSignatures
 import com.stevesoltys.seedvault.transport.backup.isSystemApp
@@ -55,13 +56,13 @@ internal class ApkRestore(
                 restore(this, token, packageName, metadata, installResult)
             } catch (e: IOException) {
                 Log.e(TAG, "Error re-installing APK for $packageName.", e)
-                emit(fail(installResult, packageName))
+                emit(installResult.fail(packageName))
             } catch (e: SecurityException) {
                 Log.e(TAG, "Security error re-installing APK for $packageName.", e)
-                emit(fail(installResult, packageName))
+                emit(installResult.fail(packageName))
             } catch (e: TimeoutCancellationException) {
                 Log.e(TAG, "Timeout while re-installing APK for $packageName.", e)
-                emit(fail(installResult, packageName))
+                emit(installResult.fail(packageName))
             }
         }
         installResult.isFinished = true
@@ -128,17 +129,10 @@ internal class ApkRestore(
         }
         collector.emit(installResult)
 
-        // ensure system apps are actually installed and newer system apps as well
+        // ensure system apps are actually already installed and newer system apps as well
         if (metadata.system) {
-            try {
+            shouldInstallSystemApp(packageName, metadata, installResult)?.let {
-                val installedPackageInfo = pm.getPackageInfo(packageName, 0)
+                collector.emit(it)
-                // metadata.version is not null, because here hasApk() must be true
-                val isOlder = metadata.version!! <= installedPackageInfo.longVersionCode
-                if (isOlder || !installedPackageInfo.isSystemApp()) throw NameNotFoundException()
-            } catch (e: NameNotFoundException) {
-                Log.w(TAG, "Not installing $packageName because older or not a system app here.")
-                // TODO consider reporting different status here to prevent manual installs
-                collector.emit(fail(installResult, packageName))
                 return
             }
         }
@@ -148,8 +142,33 @@ internal class ApkRestore(
         collector.emit(result)
     }
 
-    private fun fail(installResult: MutableInstallResult, packageName: String): InstallResult {
+    /**
-        return installResult.update(packageName) { it.copy(state = FAILED) }
+     * Returns null if this system app should get re-installed,
+     * or a new [InstallResult] to be emitted otherwise.
+     */
+    private fun shouldInstallSystemApp(
+        packageName: String,
+        metadata: PackageMetadata,
+        installResult: MutableInstallResult
+    ): InstallResult? {
+        val installedPackageInfo = try {
+            pm.getPackageInfo(packageName, 0)
+        } catch (e: PackageManager.NameNotFoundException) {
+            Log.w(TAG, "Not installing system app $packageName because not installed here.")
+            // we report a different FAILED status here to prevent manual installs
+            return installResult.fail(packageName, FAILED_SYSTEM_APP)
+        }
+        // metadata.version is not null, because here hasApk() must be true
+        val isOlder = metadata.version!! <= installedPackageInfo.longVersionCode
+        return if (isOlder) {
+            Log.w(TAG, "Not installing $packageName because ours is older.")
+            installResult.update(packageName) { it.copy(state = SUCCEEDED) }
+        } else if (!installedPackageInfo.isSystemApp()) {
+            Log.w(TAG, "Not installing $packageName because not a system app here.")
+            installResult.update(packageName) { it.copy(state = SUCCEEDED) }
+        } else {
+            null // everything is good, we can re-install this
+        }
     }
 
 }

app/src/main/java/com/stevesoltys/seedvault/restore/install/InstallProgressAdapter.kt

@@ -11,6 +11,7 @@ import androidx.recyclerview.widget.SortedList
 import androidx.recyclerview.widget.SortedListAdapterCallback
 import com.stevesoltys.seedvault.R
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED
+import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED_SYSTEM_APP
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.IN_PROGRESS
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.QUEUED
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.SUCCEEDED
@@ -97,6 +98,11 @@ internal class InstallProgressAdapter(
                         appInfo.setText(R.string.restore_installing_tap_to_install)
                     }
                 }
+                FAILED_SYSTEM_APP -> {
+                    appStatus.setImageResource(R.drawable.ic_error_red)
+                    appStatus.visibility = VISIBLE
+                    progressBar.visibility = INVISIBLE
+                }
                 QUEUED -> throw AssertionError()
             }
         }

app/src/main/java/com/stevesoltys/seedvault/restore/install/InstallResult.kt

@@ -97,6 +97,10 @@ internal class MutableInstallResult(override val total: Int) : InstallResult {
         return this
     }
 
+    fun fail(packageName: String, state: ApkInstallState = FAILED): InstallResult {
+        return update(packageName) { it.copy(state = state) }
+    }
+
     override fun reCheckFailedPackage(pm: PackageManager, packageName: String): Boolean {
         check(isFinished) { "re-checking failed packages only allowed when finished" }
         if (pm.isInstalled(packageName)) {
@@ -133,5 +137,11 @@ enum class ApkInstallState {
     QUEUED,
     IN_PROGRESS,
     SUCCEEDED,
-    FAILED
+    FAILED,
+
+    /**
+     * The app was a system app and can't be installed on the restore device,
+     * because it is not preset there.
+     */
+    FAILED_SYSTEM_APP
 }

app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt

@@ -12,6 +12,7 @@ import com.stevesoltys.seedvault.getRandomString
 import com.stevesoltys.seedvault.metadata.PackageMetadata
 import com.stevesoltys.seedvault.metadata.PackageMetadataMap
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED
+import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED_SYSTEM_APP
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.IN_PROGRESS
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.QUEUED
 import com.stevesoltys.seedvault.restore.install.ApkInstallState.SUCCEEDED
@@ -24,6 +25,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.collectIndexed
 import kotlinx.coroutines.runBlocking
+import org.junit.jupiter.api.Assertions
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Assertions.assertFalse
 import org.junit.jupiter.api.Assertions.assertTrue
@@ -242,10 +244,7 @@ internal class ApkRestoreTest : TransportTest() {
             packageInfo.applicationInfo = mockk()
             val installedPackageInfo: PackageInfo = mockk()
             val willFail = Random.nextBoolean()
-            installedPackageInfo.applicationInfo = ApplicationInfo().apply {
+            val isSystemApp = Random.nextBoolean()
-                // will not fail when app really is a system app
-                flags = if (willFail) FLAG_INSTALLED else FLAG_SYSTEM or FLAG_UPDATED_SYSTEM_APP
-            }
 
             every { strictContext.cacheDir } returns File(tmpDir.toString())
             coEvery { restorePlugin.getApkInputStream(token, packageName) } returns apkInputStream
@@ -258,18 +257,28 @@ internal class ApkRestoreTest : TransportTest() {
             } returns icon
             every { packageInfo.applicationInfo.loadIcon(pm) } returns icon
             every { pm.getApplicationLabel(packageInfo.applicationInfo) } returns appName
-            every { pm.getPackageInfo(packageName, 0) } returns installedPackageInfo
+            if (willFail) {
-            every { installedPackageInfo.longVersionCode } returns packageMetadata.version!! - 1
+                every {
-            if (!willFail) {
+                    pm.getPackageInfo(packageName, 0)
-                val installResult = MutableInstallResult(1).apply {
+                } throws PackageManager.NameNotFoundException()
-                    set(
+            } else {
-                        packageName,
+                installedPackageInfo.applicationInfo = ApplicationInfo().apply {
-                        ApkInstallResult(packageName, progress = 1, state = SUCCEEDED)
+                    flags =
-                    )
+                        if (!isSystemApp) FLAG_INSTALLED else FLAG_SYSTEM or FLAG_UPDATED_SYSTEM_APP
+                }
+                every { pm.getPackageInfo(packageName, 0) } returns installedPackageInfo
+                every { installedPackageInfo.longVersionCode } returns packageMetadata.version!! - 1
+                if (isSystemApp) { // if the installed app is not a system app, we don't install
+                    val installResult = MutableInstallResult(1).apply {
+                        set(
+                            packageName,
+                            ApkInstallResult(packageName, progress = 1, state = SUCCEEDED)
+                        )
+                    }
+                    coEvery {
+                        apkInstaller.install(any(), packageName, installerName, any())
+                    } returns installResult
                 }
-                coEvery {
-                    apkInstaller.install(any(), packageName, installerName, any())
-                } returns installResult
             }
 
             apkRestore.restore(token, packageMetadataMap).collectIndexed { i, value ->
@@ -289,14 +298,12 @@ internal class ApkRestoreTest : TransportTest() {
                     2 -> {
                         val result = value[packageName]
                         if (willFail) {
-                            assertEquals(FAILED, result.state)
+                            assertEquals(FAILED_SYSTEM_APP, result.state)
                         } else {
                             assertEquals(SUCCEEDED, result.state)
                         }
-                        assertEquals(willFail, value.hasFailed)
                     }
                     3 -> {
-                        assertEquals(willFail, value.hasFailed)
                         assertTrue(value.isFinished)
                     }
                     else -> fail("more values emitted")
@@ -307,5 +314,5 @@ internal class ApkRestoreTest : TransportTest() {
 }
 
 private operator fun InstallResult.get(packageName: String): ApkInstallResult {
-    return (this as MutableInstallResult)[packageName] ?: fail("$packageName not found")
+    return (this as MutableInstallResult)[packageName] ?: Assertions.fail("$packageName not found")
 }