From a4313d388d430949423aceab4dd17de9e811a985 Mon Sep 17 00:00:00 2001 From: len Date: Thu, 6 Apr 2017 17:26:24 +0200 Subject: [PATCH] Fix activity leaks in backup, restore dialogs and properly handle db transactions --- .../data/backup/BackupCreateService.kt | 5 ++ .../data/backup/BackupRestoreService.kt | 89 ++++++++++--------- .../ui/setting/SettingsBackupFragment.kt | 79 ++++++++-------- 3 files changed, 94 insertions(+), 79 deletions(-) diff --git a/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupCreateService.kt b/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupCreateService.kt index d892ac58cd..370995f0df 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupCreateService.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupCreateService.kt @@ -14,6 +14,7 @@ import eu.kanade.tachiyomi.data.backup.models.Backup.MANGAS import eu.kanade.tachiyomi.data.backup.models.Backup.VERSION import eu.kanade.tachiyomi.data.database.models.Manga import eu.kanade.tachiyomi.ui.setting.SettingsBackupFragment +import eu.kanade.tachiyomi.util.AndroidComponentUtil import eu.kanade.tachiyomi.util.sendLocalBroadcast import timber.log.Timber import eu.kanade.tachiyomi.BuildConfig.APPLICATION_ID as ID @@ -61,6 +62,10 @@ class BackupCreateService : IntentService(NAME) { } context.startService(intent) } + + fun isRunning(context: Context): Boolean { + return AndroidComponentUtil.isServiceRunning(context, BackupCreateService::class.java) + } } private val backupManager by lazy { BackupManager(this) } diff --git a/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupRestoreService.kt b/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupRestoreService.kt index 2c1eaac8dc..f4c0223161 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupRestoreService.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/data/backup/BackupRestoreService.kt @@ -35,6 +35,8 @@ import uy.kohesive.injekt.injectLazy import java.io.File import java.text.SimpleDateFormat import java.util.* +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors import eu.kanade.tachiyomi.BuildConfig.APPLICATION_ID as ID /** @@ -119,6 +121,8 @@ class BackupRestoreService : Service() { */ private val db: DatabaseHelper by injectLazy() + lateinit var executor: ExecutorService + /** * Method called when the service is created. It injects dependencies and acquire the wake lock. */ @@ -127,6 +131,7 @@ class BackupRestoreService : Service() { wakeLock = (getSystemService(Context.POWER_SERVICE) as PowerManager).newWakeLock( PowerManager.PARTIAL_WAKE_LOCK, "BackupRestoreService:WakeLock") wakeLock.acquire() + executor = Executors.newSingleThreadExecutor() } /** @@ -135,6 +140,7 @@ class BackupRestoreService : Service() { */ override fun onDestroy() { subscription?.unsubscribe() + executor.shutdown() // must be called after unsubscribe if (wakeLock.isHeld) { wakeLock.release() } @@ -159,53 +165,19 @@ class BackupRestoreService : Service() { override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { if (intent == null) return Service.START_NOT_STICKY + val file = UniFile.fromUri(this, Uri.parse(intent.getStringExtra(EXTRA_URI))) + // Unsubscribe from any previous subscription if needed. subscription?.unsubscribe() - val startTime = System.currentTimeMillis() - subscription = Observable.defer { - // Get URI - val uri = Uri.parse(intent.getStringExtra(EXTRA_URI)) - // Get file from Uri - val file = UniFile.fromUri(this, uri) + subscription = Observable.using( + { db.lowLevel().beginTransaction() }, + { getRestoreObservable(file).doOnNext{ db.lowLevel().setTransactionSuccessful() } }, + { executor.execute { db.lowLevel().endTransaction() } }) + .doAfterTerminate { stopSelf(startId) } + .subscribeOn(Schedulers.from(executor)) + .subscribe() - // Clear errors - errors.clear() - - // Reset progress - restoreProgress = 0 - - db.lowLevel().beginTransaction() - getRestoreObservable(file) - } - .subscribeOn(Schedulers.io()) - .subscribe({ - }, { error -> - db.lowLevel().endTransaction() - Timber.e(error) - writeErrorLog() - val errorIntent = Intent(SettingsBackupFragment.INTENT_FILTER).apply { - putExtra(SettingsBackupFragment.ACTION, SettingsBackupFragment.ACTION_ERROR_RESTORE_DIALOG) - putExtra(SettingsBackupFragment.EXTRA_ERROR_MESSAGE, error.message) - } - sendLocalBroadcast(errorIntent) - stopSelf(startId) - }, { - db.lowLevel().setTransactionSuccessful() - db.lowLevel().endTransaction() - val endTime = System.currentTimeMillis() - val time = endTime - startTime - val file = writeErrorLog() - val completeIntent = Intent(SettingsBackupFragment.INTENT_FILTER).apply { - putExtra(SettingsBackupFragment.EXTRA_TIME, time) - putExtra(SettingsBackupFragment.EXTRA_ERRORS, errors.size) - putExtra(SettingsBackupFragment.EXTRA_ERROR_FILE_PATH, file.parent) - putExtra(SettingsBackupFragment.EXTRA_ERROR_FILE, file.name) - putExtra(SettingsBackupFragment.ACTION, SettingsBackupFragment.ACTION_RESTORE_COMPLETED_DIALOG) - } - sendLocalBroadcast(completeIntent) - stopSelf(startId) - }) return Service.START_NOT_STICKY } @@ -215,7 +187,9 @@ class BackupRestoreService : Service() { * @param file restore file * @return [Observable] */ - private fun getRestoreObservable(file: UniFile): Observable { + private fun getRestoreObservable(file: UniFile): Observable> { + val startTime = System.currentTimeMillis() + val reader = JsonReader(file.openInputStream().bufferedReader()) val json = JsonParser().parse(reader).asJsonObject @@ -228,6 +202,8 @@ class BackupRestoreService : Service() { val mangasJson = json.get(MANGAS).asJsonArray restoreAmount = mangasJson.size() + 1 // +1 for categories + restoreProgress = 0 + errors.clear() // Restore categories json.get(CATEGORIES)?.let { @@ -256,6 +232,31 @@ class BackupRestoreService : Service() { Observable.just(manga) } } + .toList() + .doOnNext { + val endTime = System.currentTimeMillis() + val time = endTime - startTime + val logFile = writeErrorLog() + val completeIntent = Intent(SettingsBackupFragment.INTENT_FILTER).apply { + putExtra(SettingsBackupFragment.EXTRA_TIME, time) + putExtra(SettingsBackupFragment.EXTRA_ERRORS, errors.size) + putExtra(SettingsBackupFragment.EXTRA_ERROR_FILE_PATH, logFile.parent) + putExtra(SettingsBackupFragment.EXTRA_ERROR_FILE, logFile.name) + putExtra(SettingsBackupFragment.ACTION, SettingsBackupFragment.ACTION_RESTORE_COMPLETED_DIALOG) + } + sendLocalBroadcast(completeIntent) + + } + .doOnError { error -> + Timber.e(error) + writeErrorLog() + val errorIntent = Intent(SettingsBackupFragment.INTENT_FILTER).apply { + putExtra(SettingsBackupFragment.ACTION, SettingsBackupFragment.ACTION_ERROR_RESTORE_DIALOG) + putExtra(SettingsBackupFragment.EXTRA_ERROR_MESSAGE, error.message) + } + sendLocalBroadcast(errorIntent) + } + .onErrorReturn { emptyList() } } /** diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/setting/SettingsBackupFragment.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/setting/SettingsBackupFragment.kt index 864075c504..f05274c8ec 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/setting/SettingsBackupFragment.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/setting/SettingsBackupFragment.kt @@ -1,6 +1,7 @@ package eu.kanade.tachiyomi.ui.setting import android.app.Activity +import android.app.Dialog import android.content.BroadcastReceiver import android.content.Context import android.content.Intent @@ -25,6 +26,7 @@ import eu.kanade.tachiyomi.util.* import eu.kanade.tachiyomi.widget.CustomLayoutPickerActivity import eu.kanade.tachiyomi.widget.preference.IntListPreference import net.xpece.android.support.preference.Preference +import rx.subscriptions.Subscriptions import uy.kohesive.injekt.injectLazy import java.io.File import java.util.concurrent.TimeUnit @@ -149,7 +151,7 @@ class SettingsBackupFragment : SettingsFragment() { sendIntent.putExtra(Intent.EXTRA_STREAM, file.uri) startActivity(Intent.createChooser(sendIntent, "")) } - .show() + .safeShow() } ACTION_SET_PROGRESS_DIALOG -> { @@ -194,7 +196,7 @@ class SettingsBackupFragment : SettingsFragment() { } materialDialog.dismiss() } - .show() + .safeShow() } } ACTION_ERROR_BACKUP_DIALOG -> { @@ -210,19 +212,28 @@ class SettingsBackupFragment : SettingsFragment() { } - override fun onPause() { - context.unregisterLocalReceiver(receiver) - super.onPause() - } - override fun onStart() { super.onStart() context.registerLocalReceiver(receiver, IntentFilter(INTENT_FILTER)) } + override fun onPause() { + context.unregisterLocalReceiver(receiver) + super.onPause() + } + override fun onViewCreated(view: View, savedState: Bundle?) { super.onViewCreated(view, savedState) + if (savedState != null) { + if (BackupRestoreService.isRunning(context)) { + restoreDialog.safeShow() + } + else if (BackupCreateService.isRunning(context)) { + backupDialog.safeShow() + } + } + (activity as BaseActivity).requestPermissionsOnMarshmallow() // Set onClickListeners @@ -268,7 +279,7 @@ class SettingsBackupFragment : SettingsFragment() { .itemsDisabledIndices(0) .positiveText(getString(R.string.action_create)) .negativeText(android.R.string.cancel) - .show() + .safeShow() true } @@ -359,7 +370,7 @@ class SettingsBackupFragment : SettingsFragment() { val dir = data.data.path val file = File(dir, Backup.getDefaultFilename()) - backupDialog.show() + backupDialog.safeShow() BackupCreateService.makeBackup(context, file.toURI().toString(), backup_flags) } else { val uri = data.data @@ -369,45 +380,43 @@ class SettingsBackupFragment : SettingsFragment() { context.contentResolver.takePersistableUriPermission(uri, flags) val file = UniFile.fromUri(context, uri) - backupDialog.show() + backupDialog.safeShow() BackupCreateService.makeBackup(context, file.uri.toString(), backup_flags) } } BACKUP_RESTORE -> if (data != null && resultCode == Activity.RESULT_OK) { - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.KITKAT) { - val uri = Uri.fromFile(File(data.data.path)) - - MaterialDialog.Builder(context) - .title(getString(R.string.pref_restore_backup)) - .content(getString(R.string.backup_restore_content)) - .positiveText(getString(R.string.action_restore)) - .onPositive { materialDialog, _ -> - materialDialog.dismiss() - restoreDialog.show() - BackupRestoreService.start(context, uri.toString()) - } - .show() + val uri = if (Build.VERSION.SDK_INT < Build.VERSION_CODES.KITKAT) { + Uri.fromFile(File(data.data.path)) } else { val uri = data.data val flags = Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION context.contentResolver.takePersistableUriPermission(uri, flags) - val file = UniFile.fromUri(context, uri) - - MaterialDialog.Builder(context) - .title(getString(R.string.pref_restore_backup)) - .content(getString(R.string.backup_restore_content)) - .positiveText(getString(R.string.action_restore)) - .onPositive { materialDialog, _ -> - materialDialog.dismiss() - restoreDialog.show() - BackupRestoreService.start(context, file.uri.toString()) - } - .show() + UniFile.fromUri(context, uri).uri } + + MaterialDialog.Builder(context) + .title(getString(R.string.pref_restore_backup)) + .content(getString(R.string.backup_restore_content)) + .positiveText(getString(R.string.action_restore)) + .onPositive { _, _ -> + restoreDialog.safeShow() + BackupRestoreService.start(context, uri.toString()) + } + .safeShow() } } } + fun MaterialDialog.Builder.safeShow(): Dialog { + return build().safeShow() + } + + fun Dialog.safeShow(): Dialog { + subscriptions += Subscriptions.create { dismiss() } + show() + return this + } + } \ No newline at end of file