Cleanup LibraryUpdateService (#8237)

app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateNotifier.kt

@@ -157,10 +157,6 @@ class LibraryUpdateNotifier(private val context: Context) {
      * @param updates a list of manga with new updates.
      */
     fun showUpdateNotifications(updates: List<Pair<Manga, Array<Chapter>>>) {
-        if (updates.isEmpty()) {
-            return
-        }
-
         NotificationManagerCompat.from(context).apply {
             // Parent group notification
             notify(

app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateService.kt

@@ -12,6 +12,7 @@ import eu.kanade.domain.category.model.Category
 import eu.kanade.domain.chapter.interactor.GetChapterByMangaId
 import eu.kanade.domain.chapter.interactor.SyncChaptersWithSource
 import eu.kanade.domain.chapter.interactor.SyncChaptersWithTrackServiceTwoWay
+import eu.kanade.domain.chapter.model.Chapter
 import eu.kanade.domain.chapter.model.toDbChapter
 import eu.kanade.domain.download.service.DownloadPreferences
 import eu.kanade.domain.library.model.LibraryManga
@@ -27,8 +28,6 @@ import eu.kanade.domain.track.model.toDbTrack
 import eu.kanade.domain.track.model.toDomainTrack
 import eu.kanade.tachiyomi.R
 import eu.kanade.tachiyomi.data.cache.CoverCache
-import eu.kanade.tachiyomi.data.database.models.Chapter
-import eu.kanade.tachiyomi.data.database.models.toDomainChapter
 import eu.kanade.tachiyomi.data.download.DownloadManager
 import eu.kanade.tachiyomi.data.download.DownloadService
 import eu.kanade.tachiyomi.data.library.LibraryUpdateService.Companion.start
@@ -73,7 +72,6 @@ import java.util.Date
 import java.util.concurrent.CopyOnWriteArrayList
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.atomic.AtomicInteger
-import eu.kanade.domain.chapter.model.Chapter as DomainChapter
 
 /**
  * This class will take care of updating the chapters of the manga from the library. It can be
@@ -191,8 +189,6 @@ class LibraryUpdateService(
      */
     override fun onDestroy() {
         updateJob?.cancel()
-        // Despite what Android Studio
-        // states this can be null
         ioScope?.cancel()
         if (wakeLock.isHeld) {
             wakeLock.release()
@@ -254,6 +250,9 @@ class LibraryUpdateService(
         return START_REDELIVER_INTENT
     }
 
+    private val isUpdateJobActive: Boolean
+        get() = (updateJob?.isActive == true)
+
     /**
      * Adds list of manga to be updated.
      *
@@ -266,24 +265,25 @@ class LibraryUpdateService(
             libraryManga.filter { it.category == categoryId }
         } else {
             val categoriesToUpdate = libraryPreferences.libraryUpdateCategories().get().map { it.toLong() }
-            val listToInclude = if (categoriesToUpdate.isNotEmpty()) {
+            val includedManga = if (categoriesToUpdate.isNotEmpty()) {
                 libraryManga.filter { it.category in categoriesToUpdate }
             } else {
                 libraryManga
             }
 
             val categoriesToExclude = libraryPreferences.libraryUpdateCategoriesExclude().get().map { it.toLong() }
-            val listToExclude = if (categoriesToExclude.isNotEmpty()) {
-                libraryManga.filter { it.category in categoriesToExclude }
+            val excludedMangaIds = if (categoriesToExclude.isNotEmpty()) {
+                libraryManga.filter { it.category in categoriesToExclude }.map { it.manga.id }
             } else {
                 emptyList()
             }
 
-            listToInclude.minus(listToExclude)
+            includedManga
+                .filterNot { it.manga.id in excludedMangaIds }
+                .distinctBy { it.manga.id }
         }
 
         mangaToUpdate = listToUpdate
-            .distinctBy { it.manga.id }
             .sortedBy { it.manga.title }
 
         // Warn when excessively checking a single source
@@ -308,7 +308,7 @@ class LibraryUpdateService(
         val semaphore = Semaphore(5)
         val progressCount = AtomicInteger(0)
         val currentlyUpdatingManga = CopyOnWriteArrayList<Manga>()
-        val newUpdates = CopyOnWriteArrayList<Pair<Manga, Array<DomainChapter>>>()
+        val newUpdates = CopyOnWriteArrayList<Pair<Manga, Array<Chapter>>>()
         val skippedUpdates = CopyOnWriteArrayList<Pair<Manga, String?>>()
         val failedUpdates = CopyOnWriteArrayList<Pair<Manga, String?>>()
         val hasDownloads = AtomicBoolean(false)
@@ -317,69 +317,65 @@ class LibraryUpdateService(
         val restrictions = libraryPreferences.libraryUpdateMangaRestriction().get()
 
         withIOContext {
-            mangaToUpdate.groupBy { it.manga.source }
-                .values
+            mangaToUpdate.groupBy { it.manga.source }.values
                 .map { mangaInSource ->
                     async {
                         semaphore.withPermit {
                             mangaInSource.forEach { libraryManga ->
                                 val manga = libraryManga.manga
-                                if (updateJob?.isActive != true) {
+                                if (!isUpdateJobActive) {
+                                    notifier.cancelProgressNotification()
                                     return@async
                                 }
 
                                 // Don't continue to update if manga is not in library
-                                manga.id.let { getManga.await(it) } ?: return@forEach
+                                if (getManga.await(manga.id)?.favorite != true) {
+                                    return@forEach
+                                }
 
                                 withUpdateNotification(
                                     currentlyUpdatingManga,
                                     progressCount,
                                     manga,
                                 ) {
-                                    try {
-                                        when {
-                                            MANGA_NON_COMPLETED in restrictions && manga.status.toInt() == SManga.COMPLETED ->
-                                                skippedUpdates.add(manga to getString(R.string.skipped_reason_completed))
+                                    when {
+                                        MANGA_NON_COMPLETED in restrictions && manga.status.toInt() == SManga.COMPLETED ->
+                                            skippedUpdates.add(manga to getString(R.string.skipped_reason_completed))
 
-                                            MANGA_HAS_UNREAD in restrictions && libraryManga.unreadCount != 0L ->
-                                                skippedUpdates.add(manga to getString(R.string.skipped_reason_not_caught_up))
+                                        MANGA_HAS_UNREAD in restrictions && libraryManga.unreadCount != 0L ->
+                                            skippedUpdates.add(manga to getString(R.string.skipped_reason_not_caught_up))
 
-                                            MANGA_NON_READ in restrictions && libraryManga.totalChapters > 0L && !libraryManga.hasStarted ->
-                                                skippedUpdates.add(manga to getString(R.string.skipped_reason_not_started))
+                                        MANGA_NON_READ in restrictions && libraryManga.totalChapters > 0L && !libraryManga.hasStarted ->
+                                            skippedUpdates.add(manga to getString(R.string.skipped_reason_not_started))
 
-                                            manga.updateStrategy != UpdateStrategy.ALWAYS_UPDATE ->
-                                                skippedUpdates.add(manga to getString(R.string.skipped_reason_not_always_update))
+                                        manga.updateStrategy != UpdateStrategy.ALWAYS_UPDATE ->
+                                            skippedUpdates.add(manga to getString(R.string.skipped_reason_not_always_update))
 
-                                            else -> {
+                                        else -> {
+                                            try {
                                                 val newChapters = updateManga(manga)
-                                                val newDbChapters = newChapters.map { it.toDbChapter() }
+                                                    .sortedByDescending { it.sourceOrder }
 
                                                 if (newChapters.isNotEmpty()) {
                                                     val categoryIds = getCategories.await(manga.id).map { it.id }
                                                     if (manga.shouldDownloadNewChapters(categoryIds, downloadPreferences)) {
-                                                        downloadChapters(manga, newDbChapters)
+                                                        downloadChapters(manga, newChapters)
                                                         hasDownloads.set(true)
                                                     }
 
                                                     // Convert to the manga that contains new chapters
-                                                    newUpdates.add(
-                                                        manga to
-                                                            newDbChapters
-                                                                .map { it.toDomainChapter()!! }
-                                                                .sortedByDescending { it.sourceOrder }
-                                                                .toTypedArray(),
-                                                    )
+                                                    newUpdates.add(manga to newChapters.toTypedArray())
                                                 }
+                                            } catch (e: Throwable) {
+                                                val errorMessage = when (e) {
+                                                    is NoChaptersException -> getString(R.string.no_chapters_error)
+                                                    // failedUpdates will already have the source, don't need to copy it into the message
+                                                    is SourceManager.SourceNotInstalledException -> getString(R.string.loader_not_implemented_error)
+                                                    else -> e.message
+                                                }
+                                                failedUpdates.add(manga to errorMessage)
                                             }
                                         }
-                                    } catch (e: Throwable) {
-                                        val errorMessage = when (e) {
-                                            is NoChaptersException -> getString(R.string.no_chapters_error)
-                                            // failedUpdates will already have the source, don't need to copy it into the message
-                                            is SourceManager.SourceNotInstalledException -> getString(R.string.loader_not_implemented_error)
-                                            else -> e.message
-                                        }
-                                        failedUpdates.add(manga to errorMessage)
                                     }
 
                                     if (libraryPreferences.autoUpdateTrackers().get()) {
@@ -419,7 +415,8 @@ class LibraryUpdateService(
     private fun downloadChapters(manga: Manga, chapters: List<Chapter>) {
         // We don't want to start downloading while the library is updating, because websites
         // may don't like it and they could ban the user.
-        downloadManager.downloadChapters(manga, chapters, false)
+        val dbChapters = chapters.map { it.toDbChapter() }
+        downloadManager.downloadChapters(manga, dbChapters, false)
     }
 
     /**
@@ -428,7 +425,7 @@ class LibraryUpdateService(
      * @param manga the manga to update.
      * @return a pair of the inserted and removed chapters.
      */
-    private suspend fun updateManga(manga: Manga): List<DomainChapter> {
+    private suspend fun updateManga(manga: Manga): List<Chapter> {
         val source = sourceManager.getOrStub(manga.source)
 
         // Update manga metadata if needed
@@ -439,12 +436,10 @@ class LibraryUpdateService(
 
         val chapters = source.getChapterList(manga.toSManga())
 
-        // Get manga from database to account for if it was removed during the update
-        val dbManga = getManga.await(manga.id)
-            ?: return emptyList()
+        // Get manga from database to account for if it was removed during the update and
+        // to get latest data so it doesn't get overwritten later on
+        val dbManga = getManga.await(manga.id)?.takeIf { it.favorite } ?: return emptyList()
 
-        // [dbmanga] was used so that manga data doesn't get overwritten
-        // in case manga gets new chapter
         return syncChaptersWithSource.await(chapters, dbManga, source)
     }
 
@@ -461,7 +456,8 @@ class LibraryUpdateService(
                         semaphore.withPermit {
                             mangaInSource.forEach { libraryManga ->
                                 val manga = libraryManga.manga
-                                if (updateJob?.isActive != true) {
+                                if (!isUpdateJobActive) {
+                                    notifier.cancelProgressNotification()
                                     return@async
                                 }
 
@@ -505,7 +501,8 @@ class LibraryUpdateService(
 
         mangaToUpdate.forEach { libraryManga ->
             val manga = libraryManga.manga
-            if (updateJob?.isActive != true) {
+            if (!isUpdateJobActive) {
+                notifier.cancelProgressNotification()
                 return
             }
 
@@ -550,7 +547,8 @@ class LibraryUpdateService(
         manga: Manga,
         block: suspend () -> Unit,
     ) {
-        if (updateJob?.isActive != true) {
+        if (!isUpdateJobActive) {
+            notifier.cancelProgressNotification()
             return
         }
 
@@ -563,7 +561,8 @@ class LibraryUpdateService(
 
         block()
 
-        if (updateJob?.isActive != true) {
+        if (!isUpdateJobActive) {
+            notifier.cancelProgressNotification()
             return
         }
 
@@ -602,9 +601,7 @@ class LibraryUpdateService(
                 }
                 return file
             }
-        } catch (e: Exception) {
-            // Empty
-        }
+        } catch (_: Exception) {}
         return File("")
     }
 }