From c714a4e7e11c2187369af5deefef63b80708e2ce Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 2 Sep 2019 17:01:12 -0300 Subject: [PATCH] Show error notification when backup fails The implementation is rudimentary for now. E.g. The notification is only shown when a device init fails which seems to be triggered after the first failure. --- .../java/com/stevesoltys/backup/Backup.kt | 9 ++ .../backup/BackupNotificationManager.kt | 93 +++++++++++++++++++ .../backup/NotificationBackupObserver.kt | 51 ++-------- .../backup/settings/SettingsActivity.kt | 9 +- .../backup/settings/SettingsViewModel.kt | 14 ++- .../backup/transport/PluginManager.kt | 3 +- .../transport/backup/BackupCoordinator.kt | 5 +- app/src/main/res/drawable/ic_cloud_error.xml | 10 ++ app/src/main/res/values/strings.xml | 5 + .../transport/CoordinatorIntegrationTest.kt | 4 +- .../transport/backup/BackupCoordinatorTest.kt | 5 +- build.gradle | 2 +- 12 files changed, 156 insertions(+), 54 deletions(-) create mode 100644 app/src/main/java/com/stevesoltys/backup/BackupNotificationManager.kt create mode 100644 app/src/main/res/drawable/ic_cloud_error.xml diff --git a/app/src/main/java/com/stevesoltys/backup/Backup.kt b/app/src/main/java/com/stevesoltys/backup/Backup.kt index a71caef0..1efe3c72 100644 --- a/app/src/main/java/com/stevesoltys/backup/Backup.kt +++ b/app/src/main/java/com/stevesoltys/backup/Backup.kt @@ -5,6 +5,7 @@ import android.app.Application import android.app.backup.IBackupManager import android.content.Context.BACKUP_SERVICE import android.content.pm.PackageManager.PERMISSION_GRANTED +import android.net.Uri import android.os.Build import android.os.ServiceManager.getService import android.util.Log @@ -14,6 +15,8 @@ import com.stevesoltys.backup.settings.getDeviceName import com.stevesoltys.backup.settings.setDeviceName import io.github.novacrypto.hashing.Sha256.sha256Twice +private const val URI_AUTHORITY_EXTERNAL_STORAGE = "com.android.externalstorage.documents" + private val TAG = Backup::class.java.simpleName /** @@ -31,6 +34,10 @@ class Backup : Application() { } } + val notificationManager by lazy { + BackupNotificationManager(this) + } + override fun onCreate() { super.onCreate() storeDeviceName() @@ -53,3 +60,5 @@ class Backup : Application() { } } + +fun Uri.isOnExternalStorage() = authority == URI_AUTHORITY_EXTERNAL_STORAGE diff --git a/app/src/main/java/com/stevesoltys/backup/BackupNotificationManager.kt b/app/src/main/java/com/stevesoltys/backup/BackupNotificationManager.kt new file mode 100644 index 00000000..51b9317e --- /dev/null +++ b/app/src/main/java/com/stevesoltys/backup/BackupNotificationManager.kt @@ -0,0 +1,93 @@ +package com.stevesoltys.backup + +import android.app.NotificationChannel +import android.app.NotificationManager +import android.app.NotificationManager.IMPORTANCE_DEFAULT +import android.app.NotificationManager.IMPORTANCE_LOW +import android.app.PendingIntent +import android.app.backup.BackupTransport.TRANSPORT_PACKAGE_REJECTED +import android.content.Context +import android.content.Intent +import androidx.core.app.NotificationCompat.* +import com.stevesoltys.backup.settings.SettingsActivity + +private const val CHANNEL_ID_OBSERVER = "NotificationBackupObserver" +private const val CHANNEL_ID_ERROR = "NotificationError" +private const val NOTIFICATION_ID_OBSERVER = 1 +private const val NOTIFICATION_ID_ERROR = 2 + +class BackupNotificationManager(private val context: Context) { + + private val nm = context.getSystemService(NotificationManager::class.java)!!.apply { + createNotificationChannel(getObserverChannel()) + createNotificationChannel(getErrorChannel()) + } + + private fun getObserverChannel(): NotificationChannel { + val title = context.getString(R.string.notification_channel_title) + return NotificationChannel(CHANNEL_ID_OBSERVER, title, IMPORTANCE_LOW).apply { + enableVibration(false) + } + } + + private fun getErrorChannel(): NotificationChannel { + val title = context.getString(R.string.notification_error_channel_title) + return NotificationChannel(CHANNEL_ID_ERROR, title, IMPORTANCE_DEFAULT) + } + + private val observerBuilder = Builder(context, CHANNEL_ID_OBSERVER).apply { + setSmallIcon(R.drawable.ic_cloud_upload) + } + + private val errorBuilder = Builder(context, CHANNEL_ID_ERROR).apply { + setSmallIcon(R.drawable.ic_cloud_error) + } + + fun onBackupUpdate(app: CharSequence, transferred: Int, expected: Int, userInitiated: Boolean) { + val notification = observerBuilder.apply { + setContentTitle(context.getString(R.string.notification_title)) + setContentText(app) + setProgress(expected, transferred, false) + priority = if (userInitiated) PRIORITY_DEFAULT else PRIORITY_LOW + }.build() + nm.notify(NOTIFICATION_ID_OBSERVER, notification) + } + + fun onBackupResult(app: CharSequence, status: Int, userInitiated: Boolean) { + val title = context.getString(when (status) { + 0 -> R.string.notification_backup_result_complete + TRANSPORT_PACKAGE_REJECTED -> R.string.notification_backup_result_rejected + else -> R.string.notification_backup_result_error + }) + val notification = observerBuilder.apply { + setContentTitle(title) + setContentText(app) + priority = if (userInitiated) PRIORITY_DEFAULT else PRIORITY_LOW + }.build() + nm.notify(NOTIFICATION_ID_OBSERVER, notification) + } + + fun onBackupFinished() { + nm.cancel(NOTIFICATION_ID_OBSERVER) + } + + fun onBackupError() { + val intent = Intent(context, SettingsActivity::class.java) + val pendingIntent = PendingIntent.getActivity(context, 0, intent, 0) + val actionText = context.getString(R.string.notification_error_action) + val action = Action(R.drawable.ic_storage, actionText, pendingIntent) + val notification = errorBuilder.apply { + setContentTitle(context.getString(R.string.notification_error_title)) + setContentText(context.getString(R.string.notification_error_text)) + addAction(action) + setOnlyAlertOnce(true) + setAutoCancel(true) + }.build() + nm.notify(NOTIFICATION_ID_ERROR, notification) + } + + fun onBackupErrorSeen() { + nm.cancel(NOTIFICATION_ID_ERROR) + } + +} diff --git a/app/src/main/java/com/stevesoltys/backup/NotificationBackupObserver.kt b/app/src/main/java/com/stevesoltys/backup/NotificationBackupObserver.kt index 8729e623..de510f80 100644 --- a/app/src/main/java/com/stevesoltys/backup/NotificationBackupObserver.kt +++ b/app/src/main/java/com/stevesoltys/backup/NotificationBackupObserver.kt @@ -1,40 +1,18 @@ package com.stevesoltys.backup -import android.app.NotificationChannel -import android.app.NotificationManager -import android.app.NotificationManager.IMPORTANCE_LOW import android.app.backup.BackupProgress -import android.app.backup.BackupTransport.TRANSPORT_PACKAGE_REJECTED import android.app.backup.IBackupObserver import android.content.Context import android.util.Log import android.util.Log.INFO import android.util.Log.isLoggable -import androidx.core.app.NotificationCompat -import androidx.core.app.NotificationCompat.PRIORITY_DEFAULT -import androidx.core.app.NotificationCompat.PRIORITY_LOW - -private const val CHANNEL_ID = "NotificationBackupObserver" -private const val NOTIFICATION_ID = 1042 private val TAG = NotificationBackupObserver::class.java.simpleName -class NotificationBackupObserver( - private val context: Context, - private val userInitiated: Boolean) : IBackupObserver.Stub() { +class NotificationBackupObserver(context: Context, private val userInitiated: Boolean) : IBackupObserver.Stub() { private val pm = context.packageManager - private val nm = context.getSystemService(NotificationManager::class.java).apply { - val title = context.getString(R.string.notification_channel_title) - val channel = NotificationChannel(CHANNEL_ID, title, IMPORTANCE_LOW).apply { - enableVibration(false) - } - createNotificationChannel(channel) - } - private val notificationBuilder = NotificationCompat.Builder(context, CHANNEL_ID).apply { - setSmallIcon(R.drawable.ic_cloud_upload) - priority = if (userInitiated) PRIORITY_DEFAULT else PRIORITY_LOW - } + private val nm = (context.applicationContext as Backup).notificationManager /** * This method could be called several times for packages with full data backup. @@ -44,17 +22,13 @@ class NotificationBackupObserver( * @param backupProgress Current progress of backup for the package. */ override fun onUpdate(currentBackupPackage: String, backupProgress: BackupProgress) { - val transferred = backupProgress.bytesTransferred - val expected = backupProgress.bytesExpected + val transferred = backupProgress.bytesTransferred.toInt() + val expected = backupProgress.bytesExpected.toInt() if (isLoggable(TAG, INFO)) { Log.i(TAG, "Update. Target: $currentBackupPackage, $transferred/$expected") } - val notification = notificationBuilder.apply { - setContentTitle(context.getString(R.string.notification_title)) - setContentText(getAppName(currentBackupPackage)) - setProgress(expected.toInt(), transferred.toInt(), false) - }.build() - nm.notify(NOTIFICATION_ID, notification) + val app = getAppName(currentBackupPackage) + nm.onBackupUpdate(app, transferred, expected, userInitiated) } /** @@ -71,16 +45,7 @@ class NotificationBackupObserver( if (isLoggable(TAG, INFO)) { Log.i(TAG, "Completed. Target: $target, status: $status") } - val title = context.getString(when (status) { - 0 -> R.string.notification_backup_result_complete - TRANSPORT_PACKAGE_REJECTED -> R.string.notification_backup_result_rejected - else -> R.string.notification_backup_result_error - }) - val notification = notificationBuilder.apply { - setContentTitle(title) - setContentText(getAppName(target)) - }.build() - nm.notify(NOTIFICATION_ID, notification) + nm.onBackupResult(getAppName(target), status, userInitiated) } /** @@ -94,7 +59,7 @@ class NotificationBackupObserver( if (isLoggable(TAG, INFO)) { Log.i(TAG, "Backup finished. Status: $status") } - nm.cancel(NOTIFICATION_ID) + nm.onBackupFinished() } private fun getAppName(packageId: String): CharSequence { diff --git a/app/src/main/java/com/stevesoltys/backup/settings/SettingsActivity.kt b/app/src/main/java/com/stevesoltys/backup/settings/SettingsActivity.kt index 18a46926..36f52c99 100644 --- a/app/src/main/java/com/stevesoltys/backup/settings/SettingsActivity.kt +++ b/app/src/main/java/com/stevesoltys/backup/settings/SettingsActivity.kt @@ -7,6 +7,7 @@ import android.view.MenuItem import androidx.appcompat.app.AppCompatActivity import androidx.fragment.app.Fragment import androidx.lifecycle.ViewModelProviders +import com.stevesoltys.backup.Backup import com.stevesoltys.backup.LiveEventHandler import com.stevesoltys.backup.R @@ -25,8 +26,8 @@ class SettingsActivity : AppCompatActivity() { setContentView(R.layout.activity_settings) viewModel = ViewModelProviders.of(this).get(SettingsViewModel::class.java) - viewModel.onLocationSet.observeEvent(this, LiveEventHandler { wasEmptyBefore -> - if (wasEmptyBefore) showFragment(SettingsFragment()) + viewModel.onLocationSet.observeEvent(this, LiveEventHandler { initialSetUp -> + if (initialSetUp) showFragment(SettingsFragment()) else supportFragmentManager.popBackStack() }) viewModel.chooseBackupLocation.observeEvent(this, LiveEventHandler { show -> @@ -54,8 +55,10 @@ class SettingsActivity : AppCompatActivity() { // check that backup is provisioned if (!viewModel.recoveryCodeIsSet()) { showRecoveryCodeActivity() - } else if (!viewModel.locationIsSet()) { + } else if (!viewModel.validLocationIsSet()) { showFragment(BackupLocationFragment()) + // remove potential error notifications + (application as Backup).notificationManager.onBackupErrorSeen() } } diff --git a/app/src/main/java/com/stevesoltys/backup/settings/SettingsViewModel.kt b/app/src/main/java/com/stevesoltys/backup/settings/SettingsViewModel.kt index 7ba92a1c..6840dabd 100644 --- a/app/src/main/java/com/stevesoltys/backup/settings/SettingsViewModel.kt +++ b/app/src/main/java/com/stevesoltys/backup/settings/SettingsViewModel.kt @@ -5,10 +5,12 @@ import android.content.Intent import android.content.Intent.FLAG_GRANT_READ_URI_PERMISSION import android.content.Intent.FLAG_GRANT_WRITE_URI_PERMISSION import android.util.Log +import androidx.documentfile.provider.DocumentFile import androidx.lifecycle.AndroidViewModel import com.stevesoltys.backup.Backup import com.stevesoltys.backup.LiveEvent import com.stevesoltys.backup.MutableLiveEvent +import com.stevesoltys.backup.isOnExternalStorage import com.stevesoltys.backup.transport.ConfigurableBackupTransportService import com.stevesoltys.backup.transport.requestBackup @@ -30,7 +32,13 @@ class SettingsViewModel(application: Application) : AndroidViewModel(application internal fun chooseBackupLocation() = mChooseBackupLocation.setEvent(true) fun recoveryCodeIsSet() = Backup.keyManager.hasBackupKey() - fun locationIsSet() = getBackupFolderUri(getApplication()) != null + + fun validLocationIsSet(): Boolean { + val uri = getBackupFolderUri(app) ?: return false + if (uri.isOnExternalStorage()) return true // might be a temporary failure + val file = DocumentFile.fromTreeUri(app, uri) ?: return false + return file.isDirectory + } fun handleChooseFolderResult(result: Intent?) { val folderUri = result?.data ?: return @@ -40,13 +48,13 @@ class SettingsViewModel(application: Application) : AndroidViewModel(application app.contentResolver.takePersistableUriPermission(folderUri, takeFlags) // check if this is initial set-up or a later change - val wasEmptyBefore = getBackupFolderUri(app) == null + val initialSetUp = !validLocationIsSet() // store backup folder location in settings setBackupFolderUri(app, folderUri) // notify the UI that the location has been set - locationWasSet.setEvent(wasEmptyBefore) + locationWasSet.setEvent(initialSetUp) // stop backup service to be sure the old location will get updated app.stopService(Intent(app, ConfigurableBackupTransportService::class.java)) diff --git a/app/src/main/java/com/stevesoltys/backup/transport/PluginManager.kt b/app/src/main/java/com/stevesoltys/backup/transport/PluginManager.kt index 1aeb85d4..0346049c 100644 --- a/app/src/main/java/com/stevesoltys/backup/transport/PluginManager.kt +++ b/app/src/main/java/com/stevesoltys/backup/transport/PluginManager.kt @@ -36,8 +36,9 @@ class PluginManager(context: Context) { private val inputFactory = InputFactory() private val kvBackup = KVBackup(backupPlugin.kvBackupPlugin, inputFactory, headerWriter, crypto) private val fullBackup = FullBackup(backupPlugin.fullBackupPlugin, inputFactory, headerWriter, crypto) + private val notificationManager = (context.applicationContext as Backup).notificationManager - internal val backupCoordinator = BackupCoordinator(backupPlugin, kvBackup, fullBackup) + internal val backupCoordinator = BackupCoordinator(backupPlugin, kvBackup, fullBackup, notificationManager) private val restorePlugin = DocumentsProviderRestorePlugin(storage) diff --git a/app/src/main/java/com/stevesoltys/backup/transport/backup/BackupCoordinator.kt b/app/src/main/java/com/stevesoltys/backup/transport/backup/BackupCoordinator.kt index 65ad91b0..a4d357f3 100644 --- a/app/src/main/java/com/stevesoltys/backup/transport/backup/BackupCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/backup/transport/backup/BackupCoordinator.kt @@ -5,6 +5,7 @@ import android.app.backup.BackupTransport.TRANSPORT_OK import android.content.pm.PackageInfo import android.os.ParcelFileDescriptor import android.util.Log +import com.stevesoltys.backup.BackupNotificationManager import java.io.IOException private val TAG = BackupCoordinator::class.java.simpleName @@ -16,7 +17,8 @@ private val TAG = BackupCoordinator::class.java.simpleName class BackupCoordinator( private val plugin: BackupPlugin, private val kv: KVBackup, - private val full: FullBackup) { + private val full: FullBackup, + private val nm: BackupNotificationManager) { private var calledInitialize = false private var calledClearBackupData = false @@ -53,6 +55,7 @@ class BackupCoordinator( TRANSPORT_OK } catch (e: IOException) { Log.e(TAG, "Error initializing device", e) + nm.onBackupError() TRANSPORT_ERROR } } diff --git a/app/src/main/res/drawable/ic_cloud_error.xml b/app/src/main/res/drawable/ic_cloud_error.xml new file mode 100644 index 00000000..ecb6ff7d --- /dev/null +++ b/app/src/main/res/drawable/ic_cloud_error.xml @@ -0,0 +1,10 @@ + + + \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 1ac6ec6d..dce1cdd0 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -65,4 +65,9 @@ Not backed up Backup failed + Error Notification + Backup Error + A device backup failed to run. + Fix + diff --git a/app/src/test/java/com/stevesoltys/backup/transport/CoordinatorIntegrationTest.kt b/app/src/test/java/com/stevesoltys/backup/transport/CoordinatorIntegrationTest.kt index 7875db5e..2f9ceb3c 100644 --- a/app/src/test/java/com/stevesoltys/backup/transport/CoordinatorIntegrationTest.kt +++ b/app/src/test/java/com/stevesoltys/backup/transport/CoordinatorIntegrationTest.kt @@ -7,6 +7,7 @@ import android.app.backup.BackupTransport.TRANSPORT_OK import android.app.backup.RestoreDescription import android.app.backup.RestoreDescription.TYPE_FULL_STREAM import android.os.ParcelFileDescriptor +import com.stevesoltys.backup.BackupNotificationManager import com.stevesoltys.backup.crypto.CipherFactoryImpl import com.stevesoltys.backup.crypto.CryptoImpl import com.stevesoltys.backup.crypto.KeyManagerTestImpl @@ -37,7 +38,8 @@ internal class CoordinatorIntegrationTest : TransportTest() { private val kvBackup = KVBackup(kvBackupPlugin, inputFactory, headerWriter, cryptoImpl) private val fullBackupPlugin = mockk() private val fullBackup = FullBackup(fullBackupPlugin, inputFactory, headerWriter, cryptoImpl) - private val backup = BackupCoordinator(backupPlugin, kvBackup, fullBackup) + private val notificationManager = mockk() + private val backup = BackupCoordinator(backupPlugin, kvBackup, fullBackup, notificationManager) private val restorePlugin = mockk() private val kvRestorePlugin = mockk() diff --git a/app/src/test/java/com/stevesoltys/backup/transport/backup/BackupCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/backup/transport/backup/BackupCoordinatorTest.kt index e5833277..8e8b38fb 100644 --- a/app/src/test/java/com/stevesoltys/backup/transport/backup/BackupCoordinatorTest.kt +++ b/app/src/test/java/com/stevesoltys/backup/transport/backup/BackupCoordinatorTest.kt @@ -2,6 +2,7 @@ package com.stevesoltys.backup.transport.backup import android.app.backup.BackupTransport.TRANSPORT_ERROR import android.app.backup.BackupTransport.TRANSPORT_OK +import com.stevesoltys.backup.BackupNotificationManager import io.mockk.Runs import io.mockk.every import io.mockk.just @@ -17,8 +18,9 @@ internal class BackupCoordinatorTest: BackupTest() { private val plugin = mockk() private val kv = mockk() private val full = mockk() + private val notificationManager = mockk() - private val backup = BackupCoordinator(plugin, kv, full) + private val backup = BackupCoordinator(plugin, kv, full, notificationManager) @Test fun `device initialization succeeds and delegates to plugin`() { @@ -33,6 +35,7 @@ internal class BackupCoordinatorTest: BackupTest() { @Test fun `device initialization fails`() { every { plugin.initializeDevice() } throws IOException() + every { notificationManager.onBackupError() } just Runs assertEquals(TRANSPORT_ERROR, backup.initializeDevice()) diff --git a/build.gradle b/build.gradle index 26dd7758..84707d92 100644 --- a/build.gradle +++ b/build.gradle @@ -2,7 +2,7 @@ buildscript { - ext.kotlin_version = '1.3.41' + ext.kotlin_version = '1.3.50' repositories { jcenter()