From 96c55db7cad399a00005de4394866d6bc9949d51 Mon Sep 17 00:00:00 2001 From: arkon Date: Sat, 25 Apr 2020 16:39:00 -0400 Subject: [PATCH] Consider individual manga as transactions rather than entire restore job (closes #2482) --- .../data/backup/BackupRestoreService.kt | 297 ++++++++---------- .../tachiyomi/data/database/DatabaseHelper.kt | 9 +- .../ui/setting/SettingsBackupController.kt | 8 +- .../ui/setting/backup/BackupNotifier.kt | 6 + app/src/main/res/values/strings.xml | 1 - 5 files changed, 154 insertions(+), 167 deletions(-) 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 3fb80a108..dc6906a52 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 @@ -10,6 +10,7 @@ import android.os.PowerManager import com.github.salomonbrys.kotson.fromJson import com.google.gson.JsonArray import com.google.gson.JsonElement +import com.google.gson.JsonObject import com.google.gson.JsonParser import com.google.gson.stream.JsonReader import eu.kanade.tachiyomi.R @@ -32,18 +33,17 @@ import eu.kanade.tachiyomi.data.notification.Notifications import eu.kanade.tachiyomi.data.track.TrackManager import eu.kanade.tachiyomi.source.Source import eu.kanade.tachiyomi.ui.setting.backup.BackupNotifier -import eu.kanade.tachiyomi.util.lang.chop import eu.kanade.tachiyomi.util.system.isServiceRunning import eu.kanade.tachiyomi.util.system.sendLocalBroadcast import java.io.File import java.text.SimpleDateFormat import java.util.Date import java.util.Locale -import java.util.concurrent.ExecutorService -import java.util.concurrent.Executors +import kotlinx.coroutines.CoroutineExceptionHandler +import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch import rx.Observable -import rx.Subscription -import rx.schedulers.Schedulers import timber.log.Timber import uy.kohesive.injekt.injectLazy @@ -60,7 +60,7 @@ class BackupRestoreService : Service() { * @param context the application context. * @return true if the service is running, false otherwise. */ - private fun isRunning(context: Context): Boolean = + fun isRunning(context: Context): Boolean = context.isServiceRunning(BackupRestoreService::class.java) /** @@ -103,10 +103,7 @@ class BackupRestoreService : Service() { */ private lateinit var wakeLock: PowerManager.WakeLock - /** - * Subscription where the update is done. - */ - private var subscription: Subscription? = null + private var job: Job? = null /** * The progress of a backup restore @@ -131,15 +128,12 @@ class BackupRestoreService : Service() { private lateinit var notifier: BackupNotifier - private lateinit var executor: ExecutorService - /** * Method called when the service is created. It injects dependencies and acquire the wake lock. */ override fun onCreate() { super.onCreate() notifier = BackupNotifier(this) - executor = Executors.newSingleThreadExecutor() startForeground(Notifications.ID_RESTORE, notifier.showRestoreProgress().build()) @@ -149,17 +143,21 @@ class BackupRestoreService : Service() { wakeLock.acquire() } - /** - * Method called when the service is destroyed. It destroys the running subscription and - * releases the wake lock. - */ + override fun stopService(name: Intent?): Boolean { + destroyJob() + return super.stopService(name) + } + override fun onDestroy() { - subscription?.unsubscribe() - executor.shutdown() // must be called after unsubscribe + destroyJob() + super.onDestroy() + } + + private fun destroyJob() { + job?.cancel() if (wakeLock.isHeld) { wakeLock.release() } - super.onDestroy() } /** @@ -176,146 +174,119 @@ class BackupRestoreService : Service() { * @return the start value of the command. */ override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { - if (intent == null) return START_NOT_STICKY + val uri = intent?.getParcelableExtra(BackupConst.EXTRA_URI) ?: return START_NOT_STICKY - val uri = intent.getParcelableExtra(BackupConst.EXTRA_URI) + // Cancel any previous job if needed. + job?.cancel() + val handler = CoroutineExceptionHandler { _, exception -> + Timber.e(exception) + writeErrorLog() - // Unsubscribe from any previous subscription if needed. - subscription?.unsubscribe() + val errorIntent = Intent(BackupConst.INTENT_FILTER).apply { + putExtra(BackupConst.ACTION, BackupConst.ACTION_RESTORE_ERROR) + putExtra(BackupConst.EXTRA_ERROR_MESSAGE, exception.message) + } + sendLocalBroadcast(errorIntent) - subscription = Observable.using( - { db.lowLevel().beginTransaction() }, - { getRestoreObservable(uri).doOnNext { db.lowLevel().setTransactionSuccessful() } }, - { executor.execute { db.lowLevel().endTransaction() } } - ) - .doAfterTerminate { stopSelf(startId) } - .subscribeOn(Schedulers.from(executor)) - .subscribe() + stopSelf(startId) + } + job = GlobalScope.launch(handler) { + restoreBackup(uri) + } + job?.invokeOnCompletion { + stopSelf(startId) + } return START_NOT_STICKY } /** - * Returns an [Observable] containing restore process. + * Restores data from backup file. * - * @param uri restore file - * @return [Observable] + * @param uri backup file to restore */ - private fun getRestoreObservable(uri: Uri): Observable> { + private fun restoreBackup(uri: Uri) { val startTime = System.currentTimeMillis() - return Observable.just(Unit) - .map { - val reader = JsonReader(contentResolver.openInputStream(uri)!!.bufferedReader()) - val json = JsonParser.parseReader(reader).asJsonObject + val reader = JsonReader(contentResolver.openInputStream(uri)!!.bufferedReader()) + val json = JsonParser.parseReader(reader).asJsonObject - // Get parser version - val version = json.get(VERSION)?.asInt ?: 1 + // Get parser version + val version = json.get(VERSION)?.asInt ?: 1 - // Initialize manager - backupManager = BackupManager(this, version) + // Initialize manager + backupManager = BackupManager(this, version) - val mangasJson = json.get(MANGAS).asJsonArray + val mangasJson = json.get(MANGAS).asJsonArray - restoreAmount = mangasJson.size() + 1 // +1 for categories - restoreProgress = 0 - errors.clear() + restoreAmount = mangasJson.size() + 1 // +1 for categories + restoreProgress = 0 + errors.clear() - // Restore categories - restoreCategories(json.get(CATEGORIES)) + // Restore categories + restoreCategories(json.get(CATEGORIES)) - mangasJson - } - .flatMap { Observable.from(it) } - .concatMap { - restoreManga(it) - } - .toList() - .doOnNext { - val endTime = System.currentTimeMillis() - val time = endTime - startTime - val logFile = writeErrorLog() - val completeIntent = Intent(BackupConst.INTENT_FILTER).apply { - putExtra(BackupConst.EXTRA_TIME, time) - putExtra(BackupConst.EXTRA_ERRORS, errors.size) - putExtra(BackupConst.EXTRA_ERROR_FILE_PATH, logFile.parent) - putExtra(BackupConst.EXTRA_ERROR_FILE, logFile.name) - putExtra(BackupConst.ACTION, BackupConst.ACTION_RESTORE_COMPLETED) - } - sendLocalBroadcast(completeIntent) - } - .doOnError { error -> - Timber.e(error) - writeErrorLog() - val errorIntent = Intent(BackupConst.INTENT_FILTER).apply { - putExtra(BackupConst.ACTION, BackupConst.ACTION_RESTORE_ERROR) - putExtra(BackupConst.EXTRA_ERROR_MESSAGE, error.message) - } - sendLocalBroadcast(errorIntent) - } - .onErrorReturn { emptyList() } + // Restore individual manga + mangasJson.forEach { + restoreManga(it.asJsonObject) + } + + val endTime = System.currentTimeMillis() + val time = endTime - startTime + + val logFile = writeErrorLog() + val completeIntent = Intent(BackupConst.INTENT_FILTER).apply { + putExtra(BackupConst.EXTRA_TIME, time) + putExtra(BackupConst.EXTRA_ERRORS, errors.size) + putExtra(BackupConst.EXTRA_ERROR_FILE_PATH, logFile.parent) + putExtra(BackupConst.EXTRA_ERROR_FILE, logFile.name) + putExtra(BackupConst.ACTION, BackupConst.ACTION_RESTORE_COMPLETED) + } + sendLocalBroadcast(completeIntent) } private fun restoreCategories(categoriesJson: JsonElement) { - backupManager.restoreCategories(categoriesJson.asJsonArray) - restoreProgress += 1 - showRestoreProgress(restoreProgress, restoreAmount, getString(R.string.categories)) - } + db.executeTransaction { + backupManager.restoreCategories(categoriesJson.asJsonArray) - private fun restoreManga(mangaJson: JsonElement): Observable? { - val obj = mangaJson.asJsonObject - - val manga = backupManager.parser.fromJson(obj.get(MANGA)) - val chapters = backupManager.parser.fromJson>( - obj.get(CHAPTERS) - ?: JsonArray() - ) - val categories = backupManager.parser.fromJson>( - obj.get(CATEGORIES) - ?: JsonArray() - ) - val history = backupManager.parser.fromJson>( - obj.get(HISTORY) - ?: JsonArray() - ) - val tracks = backupManager.parser.fromJson>( - obj.get(TRACK) - ?: JsonArray() - ) - - val observable = getMangaRestoreObservable(manga, chapters, categories, history, tracks) - return if (observable != null) { - observable - } else { - errors.add(Date() to "${manga.title} - ${getString(R.string.source_not_found)}") restoreProgress += 1 - val content = - getString(R.string.dialog_restoring_source_not_found, manga.title.chop(15)) - showRestoreProgress(restoreProgress, restoreAmount, manga.title, content) - Observable.just(manga) + showRestoreProgress(restoreProgress, restoreAmount, getString(R.string.categories)) } } - /** - * Write errors to error log - */ - private fun writeErrorLog(): File { - try { - if (errors.isNotEmpty()) { - val destFile = File(externalCacheDir, "tachiyomi_restore.txt") - val sdf = SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS", Locale.getDefault()) + private fun restoreManga(mangaJson: JsonObject) { + db.executeTransaction { + val manga = backupManager.parser.fromJson(mangaJson.get(MANGA)) + val chapters = backupManager.parser.fromJson>( + mangaJson.get(CHAPTERS) + ?: JsonArray() + ) + val categories = backupManager.parser.fromJson>( + mangaJson.get(CATEGORIES) + ?: JsonArray() + ) + val history = backupManager.parser.fromJson>( + mangaJson.get(HISTORY) + ?: JsonArray() + ) + val tracks = backupManager.parser.fromJson>( + mangaJson.get(TRACK) + ?: JsonArray() + ) - destFile.bufferedWriter().use { out -> - errors.forEach { (date, message) -> - out.write("[${sdf.format(date)}] $message\n") - } - } - return destFile + if (job?.isActive != true) { + throw Exception(getString(R.string.restoring_backup_canceled)) } - } catch (e: Exception) { - // Empty + + try { + restoreMangaData(manga, chapters, categories, history, tracks) + } catch (e: Exception) { + errors.add(Date() to "${manga.title} - ${getString(R.string.source_not_found)}") + } + + restoreProgress += 1 + showRestoreProgress(restoreProgress, restoreAmount, manga.title) } - return File("") } /** @@ -326,27 +297,26 @@ class BackupRestoreService : Service() { * @param categories categories data from json * @param history history data from json * @param tracks tracking data from json - * @return [Observable] containing manga restore information */ - private fun getMangaRestoreObservable( + private fun restoreMangaData( manga: Manga, chapters: List, categories: List, history: List, tracks: List - ): Observable? { + ) { // Get source val source = backupManager.sourceManager.getOrStub(manga.source) val dbManga = backupManager.getMangaFromDatabase(manga) - return if (dbManga == null) { + if (dbManga == null) { // Manga not in database - mangaFetchObservable(source, manga, chapters, categories, history, tracks) + restoreMangaFetch(source, manga, chapters, categories, history, tracks) } else { // Manga in database // Copy information from manga already in database backupManager.restoreMangaNoFetch(manga, dbManga) // Fetch rest of manga information - mangaNoFetchObservable(source, manga, chapters, categories, history, tracks) + restoreMangaNoFetch(source, manga, chapters, categories, history, tracks) } } @@ -357,15 +327,15 @@ class BackupRestoreService : Service() { * @param chapters chapters of manga that needs updating * @param categories categories that need updating */ - private fun mangaFetchObservable( + private fun restoreMangaFetch( source: Source, manga: Manga, chapters: List, categories: List, history: List, tracks: List - ): Observable { - return backupManager.restoreMangaFetchObservable(source, manga) + ) { + backupManager.restoreMangaFetchObservable(source, manga) .onErrorReturn { errors.add(Date() to "${manga.title} - ${it.message}") manga @@ -381,24 +351,19 @@ class BackupRestoreService : Service() { } .flatMap { trackingFetchObservable(it, tracks) - // Convert to the manga that contains new chapters. - .map { manga } - } - .doOnCompleted { - restoreProgress += 1 - showRestoreProgress(restoreProgress, restoreAmount, manga.title) } + .subscribe() } - private fun mangaNoFetchObservable( + private fun restoreMangaNoFetch( source: Source, backupManga: Manga, chapters: List, categories: List, history: List, tracks: List - ): Observable { - return Observable.just(backupManga) + ) { + Observable.just(backupManga) .flatMap { manga -> if (!backupManager.restoreChaptersForManga(manga, chapters)) { chapterFetchObservable(source, manga, chapters) @@ -412,13 +377,8 @@ class BackupRestoreService : Service() { } .flatMap { manga -> trackingFetchObservable(manga, tracks) - // Convert to the manga that contains new chapters. - .map { manga } - } - .doOnCompleted { - restoreProgress += 1 - showRestoreProgress(restoreProgress, restoreAmount, backupManga.title) } + .subscribe() } private fun restoreExtraForManga(manga: Manga, categories: List, history: List, tracks: List) { @@ -482,9 +442,30 @@ class BackupRestoreService : Service() { private fun showRestoreProgress( progress: Int, amount: Int, - title: String, - content: String = title.chop(30) + title: String ) { - notifier.showRestoreProgress(content, progress, amount) + notifier.showRestoreProgress(title, progress, amount) + } + + /** + * Write errors to error log + */ + private fun writeErrorLog(): File { + try { + if (errors.isNotEmpty()) { + val destFile = File(externalCacheDir, "tachiyomi_restore.txt") + val sdf = SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS", Locale.getDefault()) + + destFile.bufferedWriter().use { out -> + errors.forEach { (date, message) -> + out.write("[${sdf.format(date)}] $message\n") + } + } + return destFile + } + } catch (e: Exception) { + // Empty + } + return File("") } } diff --git a/app/src/main/java/eu/kanade/tachiyomi/data/database/DatabaseHelper.kt b/app/src/main/java/eu/kanade/tachiyomi/data/database/DatabaseHelper.kt index a2e4c2626..912530c57 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/data/database/DatabaseHelper.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/data/database/DatabaseHelper.kt @@ -46,5 +46,12 @@ open class DatabaseHelper(context: Context) : inline fun inTransaction(block: () -> Unit) = db.inTransaction(block) - fun lowLevel() = db.lowLevel() + fun executeTransaction(block: () -> Unit) { + db.lowLevel().beginTransaction() + + block() + + db.lowLevel().setTransactionSuccessful() + db.lowLevel().endTransaction() + } } diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/setting/SettingsBackupController.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/setting/SettingsBackupController.kt index f5fa0eb44..401fc1100 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/setting/SettingsBackupController.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/setting/SettingsBackupController.kt @@ -88,7 +88,7 @@ class SettingsBackupController : SettingsController() { summaryRes = R.string.pref_restore_backup_summ onClick { - if (!isRestoreStarted) { + if (!BackupRestoreService.isRunning(context)) { val intent = Intent(Intent.ACTION_GET_CONTENT) intent.addCategory(Intent.CATEGORY_OPENABLE) intent.type = "application/*" @@ -277,7 +277,6 @@ class SettingsBackupController : SettingsController() { val context = applicationContext if (context != null) { BackupRestoreService.start(context, args.getParcelable(KEY_URI)!!) - isRestoreStarted = true } } } @@ -303,8 +302,6 @@ class SettingsBackupController : SettingsController() { notifier.showBackupError(intent.getStringExtra(BackupConst.EXTRA_ERROR_MESSAGE)) } BackupConst.ACTION_RESTORE_COMPLETED -> { - isRestoreStarted = false - val time = intent.getLongExtra(BackupConst.EXTRA_TIME, 0) val errorCount = intent.getIntExtra(BackupConst.EXTRA_ERRORS, 0) val path = intent.getStringExtra(BackupConst.EXTRA_ERROR_FILE_PATH) @@ -312,8 +309,6 @@ class SettingsBackupController : SettingsController() { notifier.showRestoreComplete(time, errorCount, path, file) } BackupConst.ACTION_RESTORE_ERROR -> { - isRestoreStarted = false - notifier.showRestoreError(intent.getStringExtra(BackupConst.EXTRA_ERROR_MESSAGE)) } } @@ -326,6 +321,5 @@ class SettingsBackupController : SettingsController() { const val CODE_BACKUP_DIR = 503 var isBackupStarted = false - var isRestoreStarted = false } } diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/setting/backup/BackupNotifier.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/setting/backup/BackupNotifier.kt index 4c8ee4f44..0709c4fef 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/setting/backup/BackupNotifier.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/setting/backup/BackupNotifier.kt @@ -31,6 +31,7 @@ internal class BackupNotifier(private val context: Context) { setContentText(context.getString(R.string.creating_backup)) setProgress(0, 0, true) + setOngoing(true) } notificationBuilder.show(Notifications.ID_BACKUP) @@ -43,6 +44,7 @@ internal class BackupNotifier(private val context: Context) { // Remove progress bar setProgress(0, 0, false) + setOngoing(false) } notificationBuilder.show(Notifications.ID_BACKUP) @@ -58,6 +60,7 @@ internal class BackupNotifier(private val context: Context) { // Remove progress bar setProgress(0, 0, false) + setOngoing(false) // Clear old actions if they exist if (mActions.isNotEmpty()) { @@ -80,6 +83,7 @@ internal class BackupNotifier(private val context: Context) { setContentText(content) setProgress(maxAmount, progress, false) + setOngoing(true) // Clear old actions if they exist if (mActions.isNotEmpty()) { @@ -105,6 +109,7 @@ internal class BackupNotifier(private val context: Context) { // Remove progress bar setProgress(0, 0, false) + setOngoing(false) } notificationBuilder.show(Notifications.ID_RESTORE) @@ -128,6 +133,7 @@ internal class BackupNotifier(private val context: Context) { // Remove progress bar setProgress(0, 0, false) + setOngoing(false) // Clear old actions if they exist if (mActions.isNotEmpty()) { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 2b90e9360..bfbecccd8 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -316,7 +316,6 @@ Backup frequency Max automatic backups Source not found - Restoring backup\n%1$s source not found Backup created Restore completed %02d min, %02d sec