Merge pull request #166 from grote/app-groups

Group app status list into three sections

app/src/androidTest/java/com/stevesoltys/seedvault/transport/backup/PackageServiceTest.kt

@@ -14,7 +14,7 @@ class PackageServiceTest : KoinComponent {
 
     @Test
     fun testNotAllowedPackages() {
-        val packages = packageService.notAllowedPackages
+        val packages = packageService.notBackedUpPackages
         Log.e("TEST", "Packages: $packages")
     }
 

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

@@ -1,5 +1,6 @@
 package com.stevesoltys.seedvault.settings
 
+import android.annotation.StringRes
 import android.content.Context
 import android.content.pm.PackageManager
 import android.graphics.drawable.Drawable
@@ -13,6 +14,7 @@ import com.stevesoltys.seedvault.transport.backup.PackageService
 import com.stevesoltys.seedvault.ui.AppBackupState
 import com.stevesoltys.seedvault.ui.AppBackupState.FAILED
 import com.stevesoltys.seedvault.ui.AppBackupState.FAILED_NOT_ALLOWED
+import com.stevesoltys.seedvault.ui.AppBackupState.FAILED_NOT_INSTALLED
 import com.stevesoltys.seedvault.ui.AppBackupState.FAILED_NO_DATA
 import com.stevesoltys.seedvault.ui.AppBackupState.FAILED_QUOTA_EXCEEDED
 import com.stevesoltys.seedvault.ui.AppBackupState.FAILED_WAS_STOPPED
@@ -28,6 +30,20 @@ private const val PACKAGE_NAME_SETTINGS = "com.android.providers.settings"
 private const val PACKAGE_NAME_CALL_LOG = "com.android.calllogbackup"
 private const val PACKAGE_NAME_CONTACTS = "org.calyxos.backup.contacts"
 
+sealed class AppListItem
+
+data class AppStatus(
+    val packageName: String,
+    var enabled: Boolean,
+    val icon: Drawable,
+    val name: String,
+    val time: Long,
+    val status: AppBackupState,
+    val isSpecial: Boolean = false
+) : AppListItem()
+
+class AppSectionTitle(@StringRes val titleRes: Int) : AppListItem()
+
 internal class AppListRetriever(
     private val context: Context,
     private val packageService: PackageService,
@@ -38,11 +54,13 @@ internal class AppListRetriever(
     private val pm: PackageManager = context.packageManager
 
     @WorkerThread
-    fun getAppList(): List<AppStatus> {
+    fun getAppList(): List<AppListItem> {
-        return getSpecialApps() + getUserApps()
+        return listOf(AppSectionTitle(R.string.backup_section_system)) + getSpecialApps() +
+            listOf(AppSectionTitle(R.string.backup_section_user)) + getUserApps() +
+            listOf(AppSectionTitle(R.string.backup_section_not_allowed)) + getNotAllowedApps()
     }
 
-    private fun getSpecialApps(): List<AppStatus> {
+    private fun getSpecialApps(): List<AppListItem> {
         val specialPackages = listOf(
             Pair(PACKAGE_NAME_SMS, R.string.backup_sms),
             Pair(PACKAGE_NAME_SETTINGS, R.string.backup_settings),
@@ -51,13 +69,18 @@ internal class AppListRetriever(
         )
         return specialPackages.map { (packageName, stringId) ->
             val metadata = metadataManager.getPackageMetadata(packageName)
+            val status = if (packageName == PACKAGE_NAME_CONTACTS && metadata?.state == null) {
+                // handle local contacts backup specially as it might not be installed
+                if (packageService.getVersionName(packageName) == null) FAILED_NOT_INSTALLED
+                else NOT_YET_BACKED_UP
+            } else metadata?.state.toAppBackupState()
             AppStatus(
                 packageName = packageName,
                 enabled = settingsManager.isBackupEnabled(packageName),
                 icon = getIcon(packageName),
                 name = context.getString(stringId),
                 time = metadata?.time ?: 0,
-                status = metadata?.state.toAppBackupState(),
+                status = status,
                 isSpecial = true
             )
         }
@@ -86,6 +109,20 @@ internal class AppListRetriever(
         }.sortedBy { it.name.toLowerCase(locale) }
     }
 
+    private fun getNotAllowedApps(): List<AppStatus> {
+        val locale = Locale.getDefault()
+        return packageService.userNotAllowedApps.map {
+            AppStatus(
+                packageName = it.packageName,
+                enabled = settingsManager.isBackupEnabled(it.packageName),
+                icon = getIcon(it.packageName),
+                name = getAppName(context, it.packageName).toString(),
+                time = 0,
+                status = FAILED_NOT_ALLOWED
+            )
+        }.sortedBy { it.name.toLowerCase(locale) }
+    }
+
     private fun getIcon(packageName: String): Drawable = when (packageName) {
         MAGIC_PACKAGE_MANAGER -> context.getDrawable(R.drawable.ic_launcher_default)!!
         PACKAGE_NAME_SMS -> context.getDrawable(R.drawable.ic_message)!!

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

@@ -1,7 +1,6 @@
 package com.stevesoltys.seedvault.settings
 
 import android.content.Intent
-import android.graphics.drawable.Drawable
 import android.net.Uri
 import android.provider.Settings.ACTION_APPLICATION_DETAILS_SETTINGS
 import android.view.LayoutInflater
@@ -11,34 +10,53 @@ import android.view.View.INVISIBLE
 import android.view.View.VISIBLE
 import android.view.ViewGroup
 import android.widget.ImageView.ScaleType
+import android.widget.TextView
 import androidx.core.content.ContextCompat.startActivity
 import androidx.recyclerview.widget.DiffUtil
 import androidx.recyclerview.widget.DiffUtil.DiffResult
+import androidx.recyclerview.widget.RecyclerView
 import androidx.recyclerview.widget.RecyclerView.Adapter
 import androidx.recyclerview.widget.RecyclerView.NO_POSITION
 import com.stevesoltys.seedvault.R
-import com.stevesoltys.seedvault.settings.AppStatusAdapter.AppStatusViewHolder
+import com.stevesoltys.seedvault.ui.AppBackupState.FAILED_NOT_ALLOWED
-import com.stevesoltys.seedvault.ui.AppBackupState
 import com.stevesoltys.seedvault.ui.AppBackupState.SUCCEEDED
 import com.stevesoltys.seedvault.ui.AppViewHolder
 import com.stevesoltys.seedvault.ui.toRelativeTime
 
 internal class AppStatusAdapter(private val toggleListener: AppStatusToggleListener) :
-    Adapter<AppStatusViewHolder>() {
+    Adapter<RecyclerView.ViewHolder>() {
 
-    private val items = ArrayList<AppStatus>()
+    private val items = ArrayList<AppListItem>()
     private var editMode = false
 
-    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): AppStatusViewHolder {
+    override fun getItemViewType(position: Int): Int = when (items[position]) {
+        is AppStatus -> 0
+        is AppSectionTitle -> 1
+    }
+
+    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RecyclerView.ViewHolder {
+        return when (viewType) {
+            0 -> {
                 val v = LayoutInflater.from(parent.context)
                     .inflate(R.layout.list_item_app_status, parent, false)
-        return AppStatusViewHolder(v)
+                AppStatusViewHolder(v)
+            }
+            1 -> {
+                val v = LayoutInflater.from(parent.context)
+                    .inflate(R.layout.list_item_app_section_title, parent, false)
+                AppSectionTitleViewHolder(v)
+            }
+            else -> throw AssertionError("unknown view type")
+        }
     }
 
     override fun getItemCount() = items.size
 
-    override fun onBindViewHolder(holder: AppStatusViewHolder, position: Int) {
+    override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) {
-        holder.bind(items[position])
+        when (holder) {
+            is AppStatusViewHolder -> holder.bind(items[position] as AppStatus)
+            is AppSectionTitleViewHolder -> holder.bind(items[position] as AppSectionTitle)
+        }
     }
 
     fun setEditMode(enabled: Boolean) {
@@ -46,17 +64,24 @@ internal class AppStatusAdapter(private val toggleListener: AppStatusToggleListe
         notifyDataSetChanged()
     }
 
-    fun update(newItems: List<AppStatus>, diff: DiffResult) {
+    fun update(newItems: List<AppListItem>, diff: DiffResult) {
         items.clear()
         items.addAll(newItems)
         diff.dispatchUpdatesTo(this)
     }
 
     fun onItemChanged(item: AppStatus) {
-        val pos = items.indexOfFirst { it.packageName == item.packageName }
+        val pos = items.indexOfFirst { it is AppStatus && it.packageName == item.packageName }
         if (pos != NO_POSITION) notifyItemChanged(pos, item)
     }
 
+    class AppSectionTitleViewHolder(v: View) : RecyclerView.ViewHolder(v) {
+        private val titleView: TextView = v as TextView
+        fun bind(item: AppSectionTitle) {
+            titleView.setText(item.titleRes)
+        }
+    }
+
     inner class AppStatusViewHolder(v: View) : AppViewHolder(v) {
         fun bind(item: AppStatus) {
             appName.text = item.name
@@ -83,7 +108,13 @@ internal class AppStatusAdapter(private val toggleListener: AppStatusToggleListe
                     startActivity(context, intent, null)
                     true
                 }
+                if (item.status == FAILED_NOT_ALLOWED) {
+                    appStatus.visibility = INVISIBLE
+                    progressBar.visibility = INVISIBLE
+                    appInfo.visibility = GONE
+                } else {
                     setState(item.status, false)
+                }
                 if (item.status == SUCCEEDED) {
                     appInfo.text = item.time.toRelativeTime(context)
                     appInfo.visibility = VISIBLE
@@ -106,34 +137,31 @@ internal class AppStatusAdapter(private val toggleListener: AppStatusToggleListe
 
 }
 
-data class AppStatus(
-    val packageName: String,
-    var enabled: Boolean,
-    val icon: Drawable,
-    val name: String,
-    val time: Long,
-    val status: AppBackupState,
-    val isSpecial: Boolean = false
-)
-
 internal class AppStatusDiff(
-    private val oldItems: List<AppStatus>,
+    private val oldItems: List<AppListItem>,
-    private val newItems: List<AppStatus>
+    private val newItems: List<AppListItem>
 ) : DiffUtil.Callback() {
 
     override fun getOldListSize() = oldItems.size
     override fun getNewListSize() = newItems.size
 
     override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean {
-        return oldItems[oldItemPosition].packageName == newItems[newItemPosition].packageName
+        val old = oldItems[oldItemPosition]
+        val new = newItems[newItemPosition]
+        if (old is AppSectionTitle && new is AppSectionTitle) return old.titleRes == new.titleRes
+        if (old is AppStatus && new is AppStatus) return old.packageName == new.packageName
+        return false
     }
 
     override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean {
-        return oldItems[oldItemPosition] == newItems[newItemPosition]
+        val old = oldItems[oldItemPosition]
+        val new = newItems[newItemPosition]
+        if (old is AppSectionTitle && new is AppSectionTitle) return old.titleRes == new.titleRes
+        return old == new
     }
 }
 
 internal class AppStatusResult(
-    val appStatusList: List<AppStatus>,
+    val appStatusList: List<AppListItem>,
     val diff: DiffResult
 )

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

@@ -259,7 +259,7 @@ internal class BackupCoordinator(
         val result = kv.performBackup(packageInfo, data, flags)
         if (result == TRANSPORT_OK && packageName == MAGIC_PACKAGE_MANAGER) {
             // hook in here to back up APKs of apps that are otherwise not allowed for backup
-            backUpNotAllowedPackages()
+            backUpApksOfNotBackedUpPackages()
         }
         return result
     }
@@ -388,13 +388,13 @@ internal class BackupCoordinator(
     }
 
     @VisibleForTesting(otherwise = PRIVATE)
-    internal suspend fun backUpNotAllowedPackages() {
+    internal suspend fun backUpApksOfNotBackedUpPackages() {
         Log.d(TAG, "Checking if APKs of opt-out apps need backup...")
-        val notAllowedPackages = packageService.notAllowedPackages
+        val notBackedUpPackages = packageService.notBackedUpPackages
-        notAllowedPackages.forEachIndexed { i, packageInfo ->
+        notBackedUpPackages.forEachIndexed { i, packageInfo ->
             val packageName = packageInfo.packageName
             try {
-                nm.onOptOutAppBackup(packageName, i + 1, notAllowedPackages.size)
+                nm.onOptOutAppBackup(packageName, i + 1, notBackedUpPackages.size)
                 val packageState =
                     if (packageInfo.isStopped()) WAS_STOPPED else NOT_ALLOWED
                 val wasBackedUp = backUpApk(packageInfo, packageState)

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

@@ -66,7 +66,7 @@ internal class PackageService(
             return packageArray.toTypedArray()
         }
 
-    val notAllowedPackages: List<PackageInfo>
+    val notBackedUpPackages: List<PackageInfo>
         @WorkerThread
         get() {
             // We need the GET_SIGNING_CERTIFICATES flag here,
@@ -88,13 +88,22 @@ internal class PackageService(
         }
 
     /**
-     * A list of non-system apps (without instrumentation test apps).
+     * A list of non-system apps
+     * (without instrumentation test apps and without apps that don't allow backup).
      */
     val userApps: List<PackageInfo>
         @WorkerThread
-        get() {
+        get() = packageManager.getInstalledPackages(GET_INSTRUMENTATION).filter { packageInfo ->
-            return packageManager.getInstalledPackages(GET_INSTRUMENTATION)
+            packageInfo.isUserVisible(context) && packageInfo.allowsBackup()
-                .filter { it.isUserVisible(context) }
+        }
+
+    /**
+     * A list of apps that does not allow backup.
+     */
+    val userNotAllowedApps: List<PackageInfo>
+        @WorkerThread
+        get() = packageManager.getInstalledPackages(0).filter { packageInfo ->
+            !packageInfo.allowsBackup() && !packageInfo.isSystemApp()
         }
 
     val expectedAppTotals: ExpectedAppTotals
@@ -148,6 +157,11 @@ internal fun PackageInfo.isSystemApp(): Boolean {
     return applicationInfo.flags and FLAG_SYSTEM != 0
 }
 
+internal fun PackageInfo.allowsBackup(): Boolean {
+    if (packageName == MAGIC_PACKAGE_MANAGER || applicationInfo == null) return false
+    return applicationInfo.flags and FLAG_ALLOW_BACKUP != 0
+}
+
 /**
  * Returns true if this is a system app that hasn't been updated.
  * We don't back up those APKs.

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

@@ -23,8 +23,8 @@ enum class AppBackupState {
         FAILED -> notShownString
         FAILED_NO_DATA -> context.getString(R.string.backup_app_no_data)
         FAILED_WAS_STOPPED -> context.getString(R.string.backup_app_was_stopped)
-        FAILED_NOT_ALLOWED -> context.getString(R.string.backup_app_not_allowed)
+        FAILED_NOT_ALLOWED -> notShownString
-        FAILED_NOT_INSTALLED -> notShownString
+        FAILED_NOT_INSTALLED -> context.getString(R.string.restore_app_not_installed)
         FAILED_QUOTA_EXCEEDED -> context.getString(R.string.backup_app_quota_exceeded)
     }
 

app/src/main/res/layout/list_item_app_section_title.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<TextView xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
+    android:layout_width="match_parent"
+    android:layout_height="wrap_content"
+    android:padding="16dp"
+    android:textColor="?android:textColorPrimary"
+    tools:text="@string/backup_section_not_allowed" />

app/src/main/res/layout/list_item_app_status.xml

@@ -12,8 +12,8 @@
 
     <ImageView
         android:id="@+id/appIcon"
-        android:layout_width="48dp"
+        android:layout_width="42dp"
-        android:layout_height="48dp"
+        android:layout_height="42dp"
         android:scaleType="fitCenter"
         app:layout_constraintBottom_toBottomOf="parent"
         app:layout_constraintStart_toStartOf="parent"
@@ -28,6 +28,7 @@
         android:layout_height="wrap_content"
         android:layout_marginStart="16dp"
         android:layout_marginEnd="16dp"
+        android:textColor="?android:textColorPrimary"
         app:layout_constraintBottom_toTopOf="@+id/appInfo"
         app:layout_constraintEnd_toStartOf="@+id/switchView"
         app:layout_constraintStart_toEndOf="@+id/appIcon"
@@ -38,13 +39,13 @@
         android:id="@+id/appInfo"
         android:layout_width="0dp"
         android:layout_height="wrap_content"
-        android:layout_marginTop="8dp"
+        android:layout_marginTop="4dp"
         android:visibility="gone"
         app:layout_constraintBottom_toBottomOf="parent"
         app:layout_constraintEnd_toEndOf="@+id/appName"
         app:layout_constraintStart_toStartOf="@+id/appName"
         app:layout_constraintTop_toBottomOf="@+id/appName"
-        tools:text="Some additional information about why the app could not be installed or its data not restored."
+        tools:text="@string/backup_app_not_yet_backed_up"
         tools:visibility="visible" />
 
     <ImageView
@@ -60,8 +61,8 @@
     <ProgressBar
         android:id="@+id/progressBar"
         style="?android:attr/progressBarStyle"
-        android:layout_width="24dp"
+        android:layout_width="20dp"
-        android:layout_height="24dp"
+        android:layout_height="20dp"
         app:layout_constraintBottom_toBottomOf="parent"
         app:layout_constraintEnd_toEndOf="parent"
         app:layout_constraintTop_toTopOf="parent" />
@@ -70,6 +71,7 @@
         android:id="@+id/switchView"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
+        android:clickable="false"
         android:visibility="invisible"
         app:layout_constraintBottom_toBottomOf="parent"
         app:layout_constraintEnd_toEndOf="parent"

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

@@ -24,7 +24,7 @@
     <string name="settings_backup_apk_dialog_message">Disabled app backup will still back up app data. However, it will not get restored automatically.\n\nYou will need to install all your apps manually while having \"Automatic Restore\" switched on.</string>
     <string name="settings_backup_apk_dialog_cancel">Cancel</string>
     <string name="settings_backup_apk_dialog_disable">Disable app backup</string>
-    <string name="settings_backup_status_title">App backup status</string>
+    <string name="settings_backup_status_title">Backup status</string>
     <string name="settings_backup_status_summary">Last backup: %1$s</string>
     <string name="settings_backup_exclude_apps">Exclude apps</string>
     <string name="settings_backup_now">Backup now</string>
@@ -91,21 +91,23 @@
 
     <!-- App Backup and Restore State -->
 
+    <string name="backup_section_system">System Apps</string>
     <string name="backup_sms">SMS text messages</string>
     <string name="backup_settings">Device settings</string>
     <string name="backup_call_log">Call history</string>
     <string name="backup_contacts">Local contacts</string>
+    <string name="backup_section_user">Installed Apps</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="backup_app_not_yet_backed_up">Waiting to back up…</string>
     <string name="restore_app_not_yet_backed_up">Was not yet backed up</string>
     <string name="backup_app_was_stopped">Not backed up as it wasn\'t used recently</string>
     <string name="restore_app_was_stopped">Was not backed up as it hadn\'t been used recently</string>
     <string name="backup_app_no_data">App reported no data for backup</string>
-    <string name="backup_app_not_allowed">App doesn\'t allow backup</string>
     <string name="restore_app_not_allowed">App didn\'t allow backup</string>
     <string name="backup_app_quota_exceeded">Backup quota exceeded</string>
     <string name="restore_app_quota_exceeded">Backup quota was exceeded</string>
     <string name="restore_app_not_installed">App not installed</string>
+    <string name="backup_section_not_allowed">Apps that do not allow data backup</string>
 
     <!-- Restore -->
     <string name="restore_title">Restore from backup</string>

app/src/main/res/xml/settings.xml

@@ -7,6 +7,14 @@
         app:persistent="false"
         app:title="@string/settings_backup" />
 
+    <androidx.preference.Preference
+        app:allowDividerAbove="true"
+        app:fragment="com.stevesoltys.seedvault.settings.AppStatusFragment"
+        app:icon="@drawable/ic_apps"
+        app:key="backup_status"
+        app:title="@string/settings_backup_status_title"
+        tools:summary="Last backup: Never" />
+
     <androidx.preference.Preference
         app:dependency="backup"
         app:icon="@drawable/ic_storage"
@@ -21,17 +29,6 @@
         app:summary="@string/settings_auto_restore_summary"
         app:title="@string/settings_auto_restore_title" />
 
-    <androidx.preference.PreferenceCategory
-        app:key="category_app_data_backup"
-        app:title="@string/settings_category_app_data_backup">
-
-        <androidx.preference.Preference
-            app:fragment="com.stevesoltys.seedvault.settings.AppStatusFragment"
-            app:icon="@drawable/ic_apps"
-            app:key="backup_status"
-            app:title="@string/settings_backup_status_title"
-            tools:summary="Last backup: Never" />
-
     <androidx.preference.SwitchPreferenceCompat
         app:defaultValue="true"
         app:dependency="backup"
@@ -39,8 +36,6 @@
         app:summary="@string/settings_backup_apk_summary"
         app:title="@string/settings_backup_apk_title" />
 
-    </androidx.preference.PreferenceCategory>
-
     <androidx.preference.Preference
         app:allowDividerAbove="true"
         app:allowDividerBelow="false"

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

@@ -357,7 +357,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         // do actual @pm@ backup
         coEvery { kv.performBackup(packageInfo, fileDescriptor, 0) } returns TRANSPORT_OK
         // now check if we have opt-out apps that we need to back up APKs for
-        every { packageService.notAllowedPackages } returns notAllowedPackages
+        every { packageService.notBackedUpPackages } returns notAllowedPackages
         // update notification
         every {
             notificationManager.onOptOutAppBackup(
@@ -411,7 +411,7 @@ internal class BackupCoordinatorTest : BackupTest() {
     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 { packageService.notBackedUpPackages } returns listOf(packageInfo)
         every {
             notificationManager.onOptOutAppBackup(packageInfo.packageName, 1, 1)
         } just Runs
@@ -431,7 +431,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         } just Runs
         every { metadataOutputStream.close() } just Runs
 
-        backup.backUpNotAllowedPackages()
+        backup.backUpApksOfNotBackedUpPackages()
 
         verify {
             metadataManager.onPackageBackupError(packageInfo, NOT_ALLOWED, metadataOutputStream)
@@ -441,7 +441,7 @@ internal class BackupCoordinatorTest : BackupTest() {
 
     @Test
     fun `APK backup of not allowed apps updates state even without old state`() = runBlocking {
-        every { packageService.notAllowedPackages } returns listOf(packageInfo)
+        every { packageService.notBackedUpPackages } returns listOf(packageInfo)
         every {
             notificationManager.onOptOutAppBackup(packageInfo.packageName, 1, 1)
         } just Runs
@@ -459,7 +459,7 @@ internal class BackupCoordinatorTest : BackupTest() {
         } just Runs
         every { metadataOutputStream.close() } just Runs
 
-        backup.backUpNotAllowedPackages()
+        backup.backUpApksOfNotBackedUpPackages()
 
         verify {
             metadataManager.onPackageBackupError(packageInfo, NOT_ALLOWED, metadataOutputStream)