Consider individual manga as transactions rather than entire restore job (closes #2482)

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 kotlinx.coroutines.CoroutineExceptionHandler
-import java.util.concurrent.Executors
+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
 
-    /**
+    private var job: Job? = null
-     * Subscription where the update is done.
-     */
-    private var subscription: Subscription? = 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()
     }
 
-    /**
+    override fun stopService(name: Intent?): Boolean {
-     * Method called when the service is destroyed. It destroys the running subscription and
+        destroyJob()
-     * releases the wake lock.
+        return super.stopService(name)
-     */
+    }
+
     override fun onDestroy() {
-        subscription?.unsubscribe()
+        destroyJob()
-        executor.shutdown() // must be called after unsubscribe
+        super.onDestroy()
+    }
+
+    private fun destroyJob() {
+        job?.cancel()
         if (wakeLock.isHeld) {
             wakeLock.release()
         }
-        super.onDestroy()
     }
 
     /**
@@ -176,36 +174,40 @@ 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<Uri>(BackupConst.EXTRA_URI) ?: return START_NOT_STICKY
 
-        val uri = intent.getParcelableExtra<Uri>(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.
+            val errorIntent = Intent(BackupConst.INTENT_FILTER).apply {
-        subscription?.unsubscribe()
+                putExtra(BackupConst.ACTION, BackupConst.ACTION_RESTORE_ERROR)
+                putExtra(BackupConst.EXTRA_ERROR_MESSAGE, exception.message)
+            }
+            sendLocalBroadcast(errorIntent)
 
-        subscription = Observable.using(
+            stopSelf(startId)
-            { db.lowLevel().beginTransaction() },
+        }
-            { getRestoreObservable(uri).doOnNext { db.lowLevel().setTransactionSuccessful() } },
+        job = GlobalScope.launch(handler) {
-            { executor.execute { db.lowLevel().endTransaction() } }
+            restoreBackup(uri)
-        )
+        }
-            .doAfterTerminate { stopSelf(startId) }
+        job?.invokeOnCompletion {
-            .subscribeOn(Schedulers.from(executor))
+            stopSelf(startId)
-            .subscribe()
+        }
 
         return START_NOT_STICKY
     }
 
     /**
-     * Returns an [Observable] containing restore process.
+     * Restores data from backup file.
      *
-     * @param uri restore file
+     * @param uri backup file to restore
-     * @return [Observable<Manga>]
      */
-    private fun getRestoreObservable(uri: Uri): Observable<List<Manga>> {
+    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
 
@@ -224,16 +226,14 @@ class BackupRestoreService : Service() {
         // Restore categories
         restoreCategories(json.get(CATEGORIES))
 
-                mangasJson
+        // Restore individual manga
+        mangasJson.forEach {
+            restoreManga(it.asJsonObject)
         }
-            .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)
@@ -244,78 +244,49 @@ class BackupRestoreService : Service() {
         }
         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() }
-    }
 
     private fun restoreCategories(categoriesJson: JsonElement) {
+        db.executeTransaction {
             backupManager.restoreCategories(categoriesJson.asJsonArray)
+
             restoreProgress += 1
             showRestoreProgress(restoreProgress, restoreAmount, getString(R.string.categories))
         }
+    }
 
-    private fun restoreManga(mangaJson: JsonElement): Observable<out Manga>? {
+    private fun restoreManga(mangaJson: JsonObject) {
-        val obj = mangaJson.asJsonObject
+        db.executeTransaction {
-
+            val manga = backupManager.parser.fromJson<MangaImpl>(mangaJson.get(MANGA))
-        val manga = backupManager.parser.fromJson<MangaImpl>(obj.get(MANGA))
             val chapters = backupManager.parser.fromJson<List<ChapterImpl>>(
-            obj.get(CHAPTERS)
+                mangaJson.get(CHAPTERS)
                     ?: JsonArray()
             )
             val categories = backupManager.parser.fromJson<List<String>>(
-            obj.get(CATEGORIES)
+                mangaJson.get(CATEGORIES)
                     ?: JsonArray()
             )
             val history = backupManager.parser.fromJson<List<DHistory>>(
-            obj.get(HISTORY)
+                mangaJson.get(HISTORY)
                     ?: JsonArray()
             )
             val tracks = backupManager.parser.fromJson<List<TrackImpl>>(
-            obj.get(TRACK)
+                mangaJson.get(TRACK)
                     ?: JsonArray()
             )
 
-        val observable = getMangaRestoreObservable(manga, chapters, categories, history, tracks)
+            if (job?.isActive != true) {
-        return if (observable != null) {
+                throw Exception(getString(R.string.restoring_backup_canceled))
-            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)
-        }
             }
 
-    /**
-     * Write errors to error log
-     */
-    private fun writeErrorLog(): File {
             try {
-            if (errors.isNotEmpty()) {
+                restoreMangaData(manga, chapters, categories, history, tracks)
-                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
+                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<Chapter>,
         categories: List<String>,
         history: List<DHistory>,
         tracks: List<Track>
-    ): Observable<Manga>? {
+    ) {
         // 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<Chapter>,
         categories: List<String>,
         history: List<DHistory>,
         tracks: List<Track>
-    ): Observable<Manga> {
+    ) {
-        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<Chapter>,
         categories: List<String>,
         history: List<DHistory>,
         tracks: List<Track>
-    ): Observable<Manga> {
+    ) {
-        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<String>, history: List<DHistory>, tracks: List<Track>) {
@@ -482,9 +442,30 @@ class BackupRestoreService : Service() {
     private fun showRestoreProgress(
         progress: Int,
         amount: Int,
-        title: String,
+        title: String
-        content: String = title.chop(30)
     ) {
-        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("")
     }
 }

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()
+    }
 }

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
     }
 }

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()) {

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

@@ -316,7 +316,6 @@
     <string name="pref_backup_interval">Backup frequency</string>
     <string name="pref_backup_slots">Max automatic backups</string>
     <string name="source_not_found">Source not found</string>
-    <string name="dialog_restoring_source_not_found">Restoring backup\n%1$s source not found</string>
     <string name="backup_created">Backup created</string>
     <string name="restore_completed">Restore completed</string>
     <string name="restore_duration">%02d min, %02d sec</string>