Add system app flag to metadata and check before installing system apps

that they are present as an older system app on the restore system.

Also ignore system apps without data to backup.

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

@@ -57,6 +57,7 @@ data class PackageMetadata(
          */
         internal var time: Long = 0L,
         internal var state: PackageState = UNKNOWN_ERROR,
+        internal val system: Boolean = false,
         internal val version: Long? = null,
         internal val installer: String? = null,
         internal val sha256: String? = null,
@@ -69,6 +70,7 @@ data class PackageMetadata(
 
 internal const val JSON_PACKAGE_TIME = "time"
 internal const val JSON_PACKAGE_STATE = "state"
+internal const val JSON_PACKAGE_SYSTEM = "system"
 internal const val JSON_PACKAGE_VERSION = "version"
 internal const val JSON_PACKAGE_INSTALLER = "installer"
 internal const val JSON_PACKAGE_SHA256 = "sha256"

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

@@ -2,10 +2,14 @@ package com.stevesoltys.seedvault.metadata
 
 import android.content.Context
 import android.content.Context.MODE_PRIVATE
+import android.content.pm.ApplicationInfo.FLAG_SYSTEM
+import android.content.pm.ApplicationInfo.FLAG_UPDATED_SYSTEM_APP
+import android.content.pm.PackageInfo
 import android.util.Log
 import androidx.annotation.VisibleForTesting
 import androidx.annotation.WorkerThread
 import com.stevesoltys.seedvault.Clock
+import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.metadata.PackageState.APK_AND_DATA
 import com.stevesoltys.seedvault.metadata.PackageState.NOT_ALLOWED
 import java.io.FileNotFoundException
@@ -59,7 +63,8 @@ class MetadataManager(
      */
     @Synchronized
     @Throws(IOException::class)
-    fun onApkBackedUp(packageName: String, packageMetadata: PackageMetadata, metadataOutputStream: OutputStream) {
+    fun onApkBackedUp(packageInfo: PackageInfo, packageMetadata: PackageMetadata, metadataOutputStream: OutputStream) {
+        val packageName = packageInfo.packageName
         metadata.packageMetadataMap[packageName]?.let {
             check(packageMetadata.version != null) {
                 "APK backup returned version null"
@@ -78,6 +83,7 @@ class MetadataManager(
         modifyMetadata(metadataOutputStream) {
             metadata.packageMetadataMap[packageName] = oldPackageMetadata.copy(
                     state = newState,
+                    system = packageInfo.isSystemApp(),
                     version = packageMetadata.version,
                     installer = packageMetadata.installer,
                     sha256 = packageMetadata.sha256,
@@ -94,7 +100,8 @@ class MetadataManager(
      */
     @Synchronized
     @Throws(IOException::class)
-    fun onPackageBackedUp(packageName: String, metadataOutputStream: OutputStream) {
+    fun onPackageBackedUp(packageInfo: PackageInfo, metadataOutputStream: OutputStream) {
+        val packageName = packageInfo.packageName
         modifyMetadata(metadataOutputStream) {
             val now = clock.time()
             metadata.time = now
@@ -104,7 +111,8 @@ class MetadataManager(
             } else {
                 metadata.packageMetadataMap[packageName] = PackageMetadata(
                         time = now,
-                        state = APK_AND_DATA
+                        state = APK_AND_DATA,
+                        system = packageInfo.isSystemApp()
                 )
             }
         }
@@ -118,15 +126,17 @@ class MetadataManager(
      */
     @Synchronized
     @Throws(IOException::class)
-    internal fun onPackageBackupError(packageName: String, packageState: PackageState, metadataOutputStream: OutputStream) {
+    internal fun onPackageBackupError(packageInfo: PackageInfo, packageState: PackageState, metadataOutputStream: OutputStream) {
         check(packageState != APK_AND_DATA) { "Backup Error with non-error package state." }
+        val packageName = packageInfo.packageName
         modifyMetadata(metadataOutputStream) {
             if (metadata.packageMetadataMap.containsKey(packageName)) {
                 metadata.packageMetadataMap[packageName]!!.state = packageState
             } else {
                 metadata.packageMetadataMap[packageName] = PackageMetadata(
                         time = 0L,
-                        state = packageState
+                        state = packageState,
+                        system = packageInfo.isSystemApp()
                 )
             }
         }
@@ -194,3 +204,13 @@ class MetadataManager(
     }
 
 }
+
+fun PackageInfo.isSystemApp(): Boolean {
+    if (packageName == MAGIC_PACKAGE_MANAGER || applicationInfo == null) return true
+    return applicationInfo.flags and FLAG_SYSTEM != 0
+}
+
+fun PackageInfo.isUpdatedSystemApp(): Boolean {
+    if (packageName == MAGIC_PACKAGE_MANAGER || applicationInfo == null) return false
+    return applicationInfo.flags and FLAG_UPDATED_SYSTEM_APP != 0
+}

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

@@ -67,6 +67,7 @@ internal class MetadataReaderImpl(private val crypto: Crypto) : MetadataReader {
                     NOT_ALLOWED.name -> NOT_ALLOWED
                     else -> UNKNOWN_ERROR
                 }
+                val pSystem = p.optBoolean(JSON_PACKAGE_SYSTEM, false)
                 val pVersion = p.optLong(JSON_PACKAGE_VERSION, 0L)
                 val pInstaller = p.optString(JSON_PACKAGE_INSTALLER)
                 val pSha256 = p.optString(JSON_PACKAGE_SHA256)
@@ -80,6 +81,7 @@ internal class MetadataReaderImpl(private val crypto: Crypto) : MetadataReader {
                 packageMetadataMap[packageName] = PackageMetadata(
                         time = p.getLong(JSON_PACKAGE_TIME),
                         state = pState,
+                        system = pSystem,
                         version = if (pVersion == 0L) null else pVersion,
                         installer = if (pInstaller == "") null else pInstaller,
                         sha256 = if (pSha256 == "") null else pSha256,

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

@@ -40,6 +40,9 @@ internal class MetadataWriterImpl(private val crypto: Crypto) : MetadataWriter {
                 if (packageMetadata.state != APK_AND_DATA) {
                     put(JSON_PACKAGE_STATE, packageMetadata.state.name)
                 }
+                if (packageMetadata.system) {
+                    put(JSON_PACKAGE_SYSTEM, packageMetadata.system)
+                }
                 packageMetadata.version?.let { put(JSON_PACKAGE_VERSION, it) }
                 packageMetadata.installer?.let { put(JSON_PACKAGE_INSTALLER, it) }
                 packageMetadata.sha256?.let { put(JSON_PACKAGE_SHA256, it) }

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

@@ -1,7 +1,5 @@
 package com.stevesoltys.seedvault.transport.backup
 
-import android.content.pm.ApplicationInfo.FLAG_SYSTEM
-import android.content.pm.ApplicationInfo.FLAG_UPDATED_SYSTEM_APP
 import android.content.pm.PackageInfo
 import android.content.pm.PackageManager
 import android.content.pm.Signature
@@ -10,9 +8,7 @@ import android.util.Log
 import android.util.PackageUtils.computeSha256DigestBytes
 import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.encodeBase64
-import com.stevesoltys.seedvault.metadata.MetadataManager
+import com.stevesoltys.seedvault.metadata.*
-import com.stevesoltys.seedvault.metadata.PackageMetadata
-import com.stevesoltys.seedvault.metadata.PackageState
 import com.stevesoltys.seedvault.settings.SettingsManager
 import java.io.File
 import java.io.FileNotFoundException
@@ -45,9 +41,7 @@ class ApkBackup(
         if (!settingsManager.backupApks()) return null
 
         // do not back up system apps that haven't been updated
-        val isSystemApp = packageInfo.applicationInfo.flags and FLAG_SYSTEM != 0
+        if (packageInfo.isSystemApp() && !packageInfo.isUpdatedSystemApp()) {
-        val isUpdatedSystemApp = packageInfo.applicationInfo.flags and FLAG_UPDATED_SYSTEM_APP != 0
-        if (isSystemApp && !isUpdatedSystemApp) {
             Log.d(TAG, "Package $packageName is vanilla system app. Not backing it up.")
             return null
         }

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

@@ -11,6 +11,7 @@ import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER
 import com.stevesoltys.seedvault.metadata.MetadataManager
 import com.stevesoltys.seedvault.metadata.PackageState
 import com.stevesoltys.seedvault.metadata.PackageState.*
+import com.stevesoltys.seedvault.metadata.isSystemApp
 import com.stevesoltys.seedvault.settings.SettingsManager
 import java.io.IOException
 import java.util.concurrent.TimeUnit.DAYS
@@ -262,7 +263,7 @@ internal class BackupCoordinator(
                 plugin.getApkOutputStream(packageInfo)
             }?.let { packageMetadata ->
                 val outputStream = plugin.getMetadataOutputStream()
-                metadataManager.onApkBackedUp(packageName, packageMetadata, outputStream)
+                metadataManager.onApkBackedUp(packageInfo, packageMetadata, outputStream)
             }
             result
         } catch (e: IOException) {
@@ -275,17 +276,19 @@ internal class BackupCoordinator(
         val packageName = packageInfo.packageName
         try {
             val outputStream = plugin.getMetadataOutputStream()
-            metadataManager.onPackageBackedUp(packageName, outputStream)
+            metadataManager.onPackageBackedUp(packageInfo, outputStream)
         } catch (e: IOException) {
             Log.e(TAG, "Error while writing metadata for $packageName", e)
         }
     }
 
     private fun onPackageBackupError(packageInfo: PackageInfo) {
+        // don't bother with system apps that have no data
+        if (cancelReason == NO_DATA && packageInfo.isSystemApp()) return
         val packageName = packageInfo.packageName
         try {
             val outputStream = plugin.getMetadataOutputStream()
-            metadataManager.onPackageBackupError(packageName, cancelReason, outputStream)
+            metadataManager.onPackageBackupError(packageInfo, cancelReason, outputStream)
         } catch (e: IOException) {
             Log.e(TAG, "Error while writing metadata for $packageName", e)
         }

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

@@ -1,13 +1,13 @@
 package com.stevesoltys.seedvault.transport.restore
 
 import android.content.Context
-import android.content.pm.PackageManager.GET_SIGNATURES
+import android.content.pm.PackageManager.*
-import android.content.pm.PackageManager.GET_SIGNING_CERTIFICATES
 import android.graphics.drawable.Drawable
 import android.util.Log
 import com.stevesoltys.seedvault.encodeBase64
 import com.stevesoltys.seedvault.metadata.PackageMetadata
 import com.stevesoltys.seedvault.metadata.PackageMetadataMap
+import com.stevesoltys.seedvault.metadata.isSystemApp
 import com.stevesoltys.seedvault.transport.backup.getSignatures
 import com.stevesoltys.seedvault.transport.restore.ApkRestoreStatus.*
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -124,6 +124,20 @@ internal class ApkRestore(
         installResult.update(packageName) { it.copy(status = IN_PROGRESS, name = name, icon = icon) }
         emit(installResult)
 
+        // ensure system apps are actually installed and newer system apps as well
+        if (metadata.system) {
+            try {
+                val installedPackageInfo = pm.getPackageInfo(packageName, 0)
+                // 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.")
+                fail(installResult, packageName)
+                return@flow
+            }
+        }
+
         // install APK and emit updates from it
         apkInstaller.install(cachedApk, packageName, metadata.installer, installResult).collect { result ->
             emit(result)

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

@@ -2,6 +2,10 @@ package com.stevesoltys.seedvault.metadata
 
 import android.content.Context
 import android.content.Context.MODE_PRIVATE
+import android.content.pm.ApplicationInfo
+import android.content.pm.ApplicationInfo.FLAG_ALLOW_BACKUP
+import android.content.pm.ApplicationInfo.FLAG_SYSTEM
+import android.content.pm.PackageInfo
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.stevesoltys.seedvault.Clock
 import com.stevesoltys.seedvault.getRandomByteArray
@@ -33,6 +37,10 @@ class MetadataManagerTest {
     private val time = 42L
     private val token = Random.nextLong()
     private val packageName = getRandomString()
+    private val packageInfo = PackageInfo().apply {
+        packageName = this@MetadataManagerTest.packageName
+        applicationInfo = ApplicationInfo().apply { flags = FLAG_ALLOW_BACKUP }
+    }
     private val initialMetadata = BackupMetadata(token = token)
     private val storageOutputStream = ByteArrayOutputStream()
     private val cacheOutputStream: FileOutputStream = mockk()
@@ -68,11 +76,29 @@ class MetadataManagerTest {
         expectReadFromCache()
         expectModifyMetadata(initialMetadata)
 
-        manager.onApkBackedUp(packageName, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
 
         assertEquals(packageMetadata, manager.getPackageMetadata(packageName))
     }
 
+    @Test
+    fun `test onApkBackedUp() sets system metadata`() {
+        packageInfo.applicationInfo = ApplicationInfo().apply { flags = FLAG_SYSTEM }
+        val packageMetadata = PackageMetadata(
+                time = 0L,
+                version = Random.nextLong(Long.MAX_VALUE),
+                installer = getRandomString(),
+                signatures = listOf("sig")
+        )
+
+        expectReadFromCache()
+        expectModifyMetadata(initialMetadata)
+
+        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
+
+        assertEquals(packageMetadata.copy(system = true), manager.getPackageMetadata(packageName))
+    }
+
     @Test
     fun `test onApkBackedUp() with existing package metadata`() {
         val packageMetadata = PackageMetadata(
@@ -92,7 +118,7 @@ class MetadataManagerTest {
         expectReadFromCache()
         expectModifyMetadata(initialMetadata)
 
-        manager.onApkBackedUp(packageName, updatedPackageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, updatedPackageMetadata, storageOutputStream)
 
         assertEquals(updatedPackageMetadata, manager.getPackageMetadata(packageName))
     }
@@ -112,53 +138,60 @@ class MetadataManagerTest {
 
         // state doesn't change for APK_AND_DATA
         packageMetadata = packageMetadata.copy(version = ++version, state = APK_AND_DATA)
-        manager.onApkBackedUp(packageName, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
         assertEquals(packageMetadata.copy(state = oldState), manager.getPackageMetadata(packageName))
 
         // state doesn't change for QUOTA_EXCEEDED
         packageMetadata = packageMetadata.copy(version = ++version, state = QUOTA_EXCEEDED)
-        manager.onApkBackedUp(packageName, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
         assertEquals(packageMetadata.copy(state = oldState), manager.getPackageMetadata(packageName))
 
         // state doesn't change for NO_DATA
         packageMetadata = packageMetadata.copy(version = ++version, state = NO_DATA)
-        manager.onApkBackedUp(packageName, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
         assertEquals(packageMetadata.copy(state = oldState), manager.getPackageMetadata(packageName))
 
         // state DOES change for NOT_ALLOWED
         packageMetadata = packageMetadata.copy(version = ++version, state = NOT_ALLOWED)
-        manager.onApkBackedUp(packageName, packageMetadata, storageOutputStream)
+        manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
         assertEquals(packageMetadata.copy(state = NOT_ALLOWED), manager.getPackageMetadata(packageName))
     }
 
     @Test
     fun `test onPackageBackedUp()`() {
-        val updatedMetadata = initialMetadata.copy()
+        packageInfo.applicationInfo.flags = FLAG_SYSTEM
-        updatedMetadata.time = time
+        val updatedMetadata = initialMetadata.copy(
-        updatedMetadata.packageMetadataMap[packageName] = PackageMetadata(time)
+                time = time,
+                packageMetadataMap = PackageMetadataMap() // otherwise this isn't copied, but referenced
+        )
+        val packageMetadata = PackageMetadata(time)
+        updatedMetadata.packageMetadataMap[packageName] = packageMetadata
 
         expectReadFromCache()
         every { clock.time() } returns time
-        expectModifyMetadata(updatedMetadata)
+        expectModifyMetadata(initialMetadata)
 
-        manager.onPackageBackedUp(packageName, storageOutputStream)
+        manager.onPackageBackedUp(packageInfo, storageOutputStream)
 
+        assertEquals(packageMetadata.copy(state = APK_AND_DATA, system = true), manager.getPackageMetadata(packageName))
         assertEquals(time, manager.getLastBackupTime())
     }
 
     @Test
     fun `test onPackageBackedUp() fails to write to storage`() {
         val updateTime = time + 1
-        val updatedMetadata = initialMetadata.copy()
+        val updatedMetadata = initialMetadata.copy(
-        updatedMetadata.time = updateTime
+                time = updateTime,
-        updatedMetadata.packageMetadataMap[packageName] = PackageMetadata(updateTime)
+                packageMetadataMap = PackageMetadataMap() // otherwise this isn't copied, but referenced
+        )
+        updatedMetadata.packageMetadataMap[packageName] = PackageMetadata(updateTime, APK_AND_DATA)
 
         expectReadFromCache()
         every { clock.time() } returns updateTime
         every { metadataWriter.write(updatedMetadata, storageOutputStream) } throws IOException()
 
         try {
-            manager.onPackageBackedUp(packageName, storageOutputStream)
+            manager.onPackageBackedUp(packageInfo, storageOutputStream)
             fail()
         } catch (e: IOException) {
             // expected
@@ -185,7 +218,7 @@ class MetadataManagerTest {
         every { clock.time() } returns time
         expectModifyMetadata(updatedMetadata)
 
-        manager.onPackageBackedUp(packageName, storageOutputStream)
+        manager.onPackageBackedUp(packageInfo, storageOutputStream)
 
         assertEquals(time, manager.getLastBackupTime())
         assertEquals(PackageMetadata(time), manager.getPackageMetadata(cachedPackageName))

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

@@ -127,6 +127,17 @@ class MetadataReaderTest {
         assertEquals(UNKNOWN_ERROR, metadata.packageMetadataMap["org.example"]!!.state)
     }
 
+    @Test
+    fun `package metadata missing system gets mapped to false`() {
+        val json = JSONObject(metadataByteArray.toString(Utf8))
+        json.put("org.example", JSONObject().apply {
+            put(JSON_PACKAGE_TIME, Random.nextLong())
+        })
+        val jsonBytes = json.toString().toByteArray(Utf8)
+        val metadata = decoder.decode(jsonBytes, metadata.version, metadata.token)
+        assertFalse(metadata.packageMetadataMap["org.example"]!!.system)
+    }
+
     @Test
     fun `package metadata can only include time`() {
         val json = JSONObject(metadataByteArray.toString(Utf8))

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

@@ -55,6 +55,7 @@ internal class MetadataWriterDecoderTest {
             put(getRandomString(), PackageMetadata(
                     time = Random.nextLong(),
                     state = QUOTA_EXCEEDED,
+                    system = Random.nextBoolean(),
                     version = Random.nextLong(),
                     installer = getRandomString(),
                     sha256 = getRandomString(),
@@ -63,6 +64,7 @@ internal class MetadataWriterDecoderTest {
             put(getRandomString(), PackageMetadata(
                     time = Random.nextLong(),
                     state = NO_DATA,
+                    system = Random.nextBoolean(),
                     version = Random.nextLong(),
                     installer = getRandomString(),
                     sha256 = getRandomString(),
@@ -71,6 +73,7 @@ internal class MetadataWriterDecoderTest {
             put(getRandomString(), PackageMetadata(
                     time = 0L,
                     state = NOT_ALLOWED,
+                    system = Random.nextBoolean(),
                     version = Random.nextLong(),
                     installer = getRandomString(),
                     sha256 = getRandomString(),

app/src/test/java/com/stevesoltys/seedvault/transport/CoordinatorIntegrationTest.kt

@@ -98,8 +98,8 @@ internal class CoordinatorIntegrationTest : TransportTest() {
         every { kvBackupPlugin.getOutputStreamForRecord(packageInfo, key264) } returns bOutputStream2
         every { apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, any()) } returns packageMetadata
         every { backupPlugin.getMetadataOutputStream() } returns metadataOutputStream
-        every { metadataManager.onApkBackedUp(packageInfo.packageName, packageMetadata, metadataOutputStream) } just Runs
+        every { metadataManager.onApkBackedUp(packageInfo, packageMetadata, metadataOutputStream) } just Runs
-        every { metadataManager.onPackageBackedUp(packageInfo.packageName, metadataOutputStream) } just Runs
+        every { metadataManager.onPackageBackedUp(packageInfo, metadataOutputStream) } just Runs
 
         // start and finish K/V backup
         assertEquals(TRANSPORT_OK, backup.performIncrementalBackup(packageInfo, fileDescriptor, 0))
@@ -152,7 +152,7 @@ internal class CoordinatorIntegrationTest : TransportTest() {
         every { kvBackupPlugin.getOutputStreamForRecord(packageInfo, key64) } returns bOutputStream
         every { apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, any()) } returns null
         every { backupPlugin.getMetadataOutputStream() } returns metadataOutputStream
-        every { metadataManager.onPackageBackedUp(packageInfo.packageName, metadataOutputStream) } just Runs
+        every { metadataManager.onPackageBackedUp(packageInfo, metadataOutputStream) } just Runs
 
         // start and finish K/V backup
         assertEquals(TRANSPORT_OK, backup.performIncrementalBackup(packageInfo, fileDescriptor, 0))
@@ -190,8 +190,8 @@ internal class CoordinatorIntegrationTest : TransportTest() {
         every { fullBackupPlugin.getQuota() } returns DEFAULT_QUOTA_FULL_BACKUP
         every { apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, any()) } returns packageMetadata
         every { backupPlugin.getMetadataOutputStream() } returns metadataOutputStream
-        every { metadataManager.onApkBackedUp(packageInfo.packageName, packageMetadata, metadataOutputStream) } just Runs
+        every { metadataManager.onApkBackedUp(packageInfo, packageMetadata, metadataOutputStream) } just Runs
-        every { metadataManager.onPackageBackedUp(packageInfo.packageName, metadataOutputStream) } just Runs
+        every { metadataManager.onPackageBackedUp(packageInfo, metadataOutputStream) } just Runs
 
         // perform backup to output stream
         assertEquals(TRANSPORT_OK, backup.performFullBackup(packageInfo, fileDescriptor, 0))

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

@@ -118,7 +118,7 @@ internal class ApkBackupTest : BackupTest() {
         expectChecks()
         every { streamGetter.invoke() } returns apkOutputStream
         every { pm.getInstallerPackageName(packageInfo.packageName) } returns updatedMetadata.installer
-        every { metadataManager.onApkBackedUp(packageInfo.packageName, updatedMetadata, outputStream) } just Runs
+        every { metadataManager.onApkBackedUp(packageInfo, updatedMetadata, outputStream) } just Runs
 
         assertEquals(updatedMetadata, apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, streamGetter))
         assertArrayEquals(apkBytes, apkOutputStream.toByteArray())

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

@@ -147,7 +147,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         every { full.hasState() } returns false
         every { kv.getCurrentPackage() } returns packageInfo
         every { plugin.getMetadataOutputStream() } returns metadataOutputStream
-        every { metadataManager.onPackageBackedUp(packageInfo.packageName, metadataOutputStream) } just Runs
+        every { metadataManager.onPackageBackedUp(packageInfo, metadataOutputStream) } just Runs
         every { kv.finishBackup() } returns result
 
         assertEquals(result, backup.finishBackup())
@@ -161,7 +161,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         every { full.hasState() } returns true
         every { full.getCurrentPackage() } returns packageInfo
         every { plugin.getMetadataOutputStream() } returns metadataOutputStream
-        every { metadataManager.onPackageBackedUp(packageInfo.packageName, metadataOutputStream) } just Runs
+        every { metadataManager.onPackageBackedUp(packageInfo, metadataOutputStream) } just Runs
         every { full.finishBackup() } returns result
 
         assertEquals(result, backup.finishBackup())
@@ -182,7 +182,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         every { full.getQuota() } returns DEFAULT_QUOTA_FULL_BACKUP
         every { full.checkFullBackupSize(DEFAULT_QUOTA_FULL_BACKUP + 1) } returns TRANSPORT_QUOTA_EXCEEDED
         every { full.getCurrentPackage() } returns packageInfo
-        every { metadataManager.onPackageBackupError(packageInfo.packageName, QUOTA_EXCEEDED, metadataOutputStream) } just Runs
+        every { metadataManager.onPackageBackupError(packageInfo, QUOTA_EXCEEDED, metadataOutputStream) } just Runs
         every { full.cancelFullBackup() } just Runs
         every { settingsManager.getStorage() } returns storage
 
@@ -196,7 +196,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         assertEquals(0L, backup.requestFullBackupTime())
 
         verify(exactly = 1) {
-            metadataManager.onPackageBackupError(packageInfo.packageName, QUOTA_EXCEEDED, metadataOutputStream)
+            metadataManager.onPackageBackupError(packageInfo, QUOTA_EXCEEDED, metadataOutputStream)
         }
     }
 
@@ -207,7 +207,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         every { full.getQuota() } returns DEFAULT_QUOTA_FULL_BACKUP
         every { full.checkFullBackupSize(0) } returns TRANSPORT_PACKAGE_REJECTED
         every { full.getCurrentPackage() } returns packageInfo
-        every { metadataManager.onPackageBackupError(packageInfo.packageName, NO_DATA, metadataOutputStream) } just Runs
+        every { metadataManager.onPackageBackupError(packageInfo, NO_DATA, metadataOutputStream) } just Runs
         every { full.cancelFullBackup() } just Runs
         every { settingsManager.getStorage() } returns storage
 
@@ -220,18 +220,16 @@ internal class BackupCoordinatorTest : BackupTest() {
         assertEquals(0L, backup.requestFullBackupTime())
 
         verify(exactly = 1) {
-            metadataManager.onPackageBackupError(packageInfo.packageName, NO_DATA, metadataOutputStream)
+            metadataManager.onPackageBackupError(packageInfo, NO_DATA, metadataOutputStream)
         }
     }
 
     @Test
     fun `not allowed apps get their APKs backed up during @pm@ backup`() {
         val packageInfo = PackageInfo().apply { packageName = MAGIC_PACKAGE_MANAGER }
-        val packageName1 = "org.example.1"
-        val packageName2 = "org.example.2"
         val notAllowedPackages = listOf(
-                PackageInfo().apply { packageName = packageName1 },
+                PackageInfo().apply { packageName = "org.example.1" },
-                PackageInfo().apply { packageName = packageName2 }
+                PackageInfo().apply { packageName = "org.example.2" }
         )
         val packageMetadata: PackageMetadata = mockk()
 
@@ -242,7 +240,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         // was backed up, get new packageMetadata
         every { apkBackup.backupApkIfNecessary(notAllowedPackages[1], NOT_ALLOWED, any()) } returns packageMetadata
         every { plugin.getMetadataOutputStream() } returns metadataOutputStream
-        every { metadataManager.onApkBackedUp(packageName2, packageMetadata, metadataOutputStream) } just Runs
+        every { metadataManager.onApkBackedUp(notAllowedPackages[1], packageMetadata, metadataOutputStream) } just Runs
         // do actual @pm@ backup
         every { kv.performBackup(packageInfo, fileDescriptor, 0) } returns TRANSPORT_OK
 
@@ -258,7 +256,7 @@ internal class BackupCoordinatorTest : BackupTest() {
     private fun expectApkBackupAndMetadataWrite() {
         every { apkBackup.backupApkIfNecessary(packageInfo, UNKNOWN_ERROR, any()) } returns packageMetadata
         every { plugin.getMetadataOutputStream() } returns metadataOutputStream
-        every { metadataManager.onApkBackedUp(packageInfo.packageName, packageMetadata, metadataOutputStream) } just Runs
+        every { metadataManager.onApkBackedUp(packageInfo, packageMetadata, metadataOutputStream) } just Runs
     }
 
 }

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

@@ -1,6 +1,9 @@
 package com.stevesoltys.seedvault.transport.restore
 
 import android.content.Context
+import android.content.pm.ApplicationInfo
+import android.content.pm.ApplicationInfo.*
+import android.content.pm.PackageInfo
 import android.content.pm.PackageManager
 import android.graphics.drawable.Drawable
 import com.stevesoltys.seedvault.getRandomString
@@ -21,6 +24,7 @@ import org.junit.jupiter.api.io.TempDir
 import java.io.ByteArrayInputStream
 import java.io.File
 import java.nio.file.Path
+import java.util.logging.Logger.getLogger
 import kotlin.random.Random
 
 @ExperimentalCoroutinesApi
@@ -178,4 +182,60 @@ internal class ApkRestoreTest : RestoreTest() {
         }
     }
 
+    @Test
+    fun `test system apps only get reinstalled when older system apps exist`(@TempDir tmpDir: Path) = runBlocking {
+        val packageMetadata = this@ApkRestoreTest.packageMetadata.copy(system = true)
+        packageMetadataMap[packageName] = packageMetadata
+        packageInfo.applicationInfo = mockk()
+        val installedPackageInfo: PackageInfo = mockk()
+        val willFail = Random.nextBoolean()
+        installedPackageInfo.applicationInfo = ApplicationInfo().apply {
+            // 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())
+        every { restorePlugin.getApkInputStream(token, packageName) } returns apkInputStream
+        every { pm.getPackageArchiveInfo(any(), any()) } returns packageInfo
+        every { pm.loadItemIcon(packageInfo.applicationInfo, packageInfo.applicationInfo) } returns icon
+        every { packageInfo.applicationInfo.loadIcon(pm) } returns icon
+        every { pm.getApplicationLabel(packageInfo.applicationInfo) } returns appName
+        every { pm.getPackageInfo(packageName, 0) } returns installedPackageInfo
+        every { installedPackageInfo.longVersionCode } returns packageMetadata.version!! - 1
+        if (!willFail) {
+            val installResult = MutableInstallResult(1).apply {
+                put(packageName, ApkRestoreResult(packageName, progress = 1, total = 1, status = SUCCEEDED))
+            }
+            every { apkInstaller.install(any(), packageName, installerName, any()) } returns flowOf(installResult)
+        }
+
+        var i = 0
+        apkRestore.restore(token, packageMetadataMap).collect { value ->
+            when (i) {
+                0 -> {
+                    val result = value[packageName] ?: fail()
+                    assertEquals(QUEUED, result.status)
+                    assertEquals(1, result.progress)
+                    assertEquals(1, result.total)
+                }
+                1 -> {
+                    val result = value[packageName] ?: fail()
+                    assertEquals(IN_PROGRESS, result.status)
+                    assertEquals(appName, result.name)
+                    assertEquals(icon, result.icon)
+                }
+                2 -> {
+                    val result = value[packageName] ?: fail()
+                    if (willFail) {
+                        assertEquals(FAILED, result.status)
+                    } else {
+                        assertEquals(SUCCEEDED, result.status)
+                    }
+                }
+                else -> fail()
+            }
+            i++
+        }
+    }
+
 }