Apply chapter sync in one transaction

A chapter sync's removals, inserts and updates go through one ChapterRepository.updateFromRemote call, applied atomically. A failure no longer leaves some of a sync's changes written and others missing. Restore folds duplicate chapters in a backup into one before writing, and the chapter deep link looks up the synced chapter afterwards rather than relying on the sync's return value.

app/src/main/java/eu/kanade/domain/chapter/interactor/SyncChaptersWithSource.kt

@@ -15,9 +15,7 @@ import kotlinx.datetime.TimeZone
 import kotlinx.datetime.toInstant
 import kotlinx.datetime.toLocalDateTime
 import tachiyomi.data.chapter.ChapterSanitizer
-import tachiyomi.domain.chapter.interactor.GetChaptersByMangaId
 import tachiyomi.domain.chapter.interactor.ShouldUpdateDbChapter
-import tachiyomi.domain.chapter.interactor.UpdateChapter
 import tachiyomi.domain.chapter.model.Chapter
 import tachiyomi.domain.chapter.model.ChapterRemoteUpdate
 import tachiyomi.domain.chapter.model.NoChaptersException
@@ -37,8 +35,6 @@ class SyncChaptersWithSource(
     private val chapterRepository: ChapterRepository,
     private val shouldUpdateDbChapter: ShouldUpdateDbChapter,
     private val updateManga: UpdateManga,
-    private val updateChapter: UpdateChapter,
-    private val getChaptersByMangaId: GetChaptersByMangaId,
     private val getExcludedScanlators: GetExcludedScanlators,
     private val libraryPreferences: LibraryPreferences,
 ) {
@@ -75,15 +71,12 @@ class SyncChaptersWithSource(
                     .copy(mangaId = manga.id, sourceOrder = i.toLong())
             }
 
-        val dbChapters = getChaptersByMangaId.await(manga.id)
+        val dbChapters = chapterRepository.getChapterByMangaId(manga.id)
+        val dbChaptersByUrl = dbChapters.associateBy { it.url }
 
         val newChapters = mutableListOf<Chapter>()
         val updatedChapters = mutableListOf<ChapterRemoteUpdate>()
-        val removedChapters = dbChapters.filterNot { dbChapter ->
-            sourceChapters.any { sourceChapter ->
-                dbChapter.url == sourceChapter.url
-            }
-        }
+        val sourceUrls = mutableSetOf<String>()
 
         // Used to not set upload date of older chapters
         // to a higher value than newer chapters
@@ -100,11 +93,13 @@ class SyncChaptersWithSource(
                 chapter = chapter.copyFromSChapter(sChapter)
             }
 
+            if (!sourceUrls.add(chapter.url)) continue
+
             // Recognize chapter number for the chapter.
             val chapterNumber = ChapterRecognition.parseChapterNumber(manga.title, chapter.name, chapter.chapterNumber)
             chapter = chapter.copy(chapterNumber = chapterNumber)
 
-            val dbChapter = dbChapters.find { it.url == chapter.url }
+            val dbChapter = dbChaptersByUrl[chapter.url]
 
             if (dbChapter == null) {
                 val toAddChapter = if (chapter.dateUpload == 0L) {
@@ -145,6 +140,8 @@ class SyncChaptersWithSource(
             }
         }
 
+        val removedChapters = dbChapters.filterNot { it.url in sourceUrls }
+
         // Return if there's nothing to add, delete, or update to avoid unnecessary db transactions.
         if (newChapters.isEmpty() && removedChapters.isEmpty() && updatedChapters.isEmpty()) {
             if (manualFetch || manga.fetchInterval == 0 || manga.nextUpdate < fetchWindow.first) {
@@ -185,7 +182,7 @@ class SyncChaptersWithSource(
         // Date fetch is set in such a way that the upper ones will have bigger value than the lower ones
         // Sources MUST return the chapters from most to less recent, which is common.
         var itemCount = newChapters.size
-        var updatedToAdd = newChapters.map { toAddItem ->
+        val toAdd = newChapters.map { toAddItem ->
             var chapter = toAddItem.copy(dateFetch = nowMillis + itemCount--)
 
             if (chapter.chapterNumber in readChapterNumbers && markDuplicateAsRead) {
@@ -210,18 +207,11 @@ class SyncChaptersWithSource(
             chapter
         }
 
-        if (removedChapters.isNotEmpty()) {
-            val toDeleteIds = removedChapters.map { it.id }
-            chapterRepository.removeChaptersWithIds(toDeleteIds)
-        }
-
-        if (updatedToAdd.isNotEmpty()) {
-            updatedToAdd = chapterRepository.addAll(updatedToAdd)
-        }
-
-        if (updatedChapters.isNotEmpty()) {
-            updateChapter.awaitAllRemote(updatedChapters)
-        }
+        val added = chapterRepository.updateFromRemote(
+            removedIds = removedChapters.map { it.id },
+            added = toAdd,
+            updated = updatedChapters,
+        )
         updateManga.awaitUpdateFetchInterval(manga, timeZone, now, fetchWindow)
 
         // Set this manga as updated since chapters were changed
@@ -230,6 +220,6 @@ class SyncChaptersWithSource(
 
         val excludedScanlators = getExcludedScanlators.await(manga.id).toHashSet()
 
-        return updatedToAdd.filterNot { it.url in changedOrDuplicateReadUrls || it.scanlator in excludedScanlators }
+        return added.filterNot { it.url in changedOrDuplicateReadUrls || it.scanlator in excludedScanlators }
     }
 }

app/src/main/java/eu/kanade/tachiyomi/ui/deeplink/DeepLinkViewModel.kt

@@ -78,11 +78,9 @@ class DeepLinkViewModel(
     private suspend fun getChapterFromSChapter(sChapter: SChapter, manga: Manga, source: Source): Chapter? {
         val localChapter = getChapterByUrlAndMangaId.await(sChapter.url, manga.id)
 
-        return localChapter
-            ?: updateMangaFromRemote(manga, fetchChapters = true)
-                .getOrElse { return null }
-                .newChapters
-                .find { it.url == sChapter.url }
+        if (localChapter != null) return localChapter
+        updateMangaFromRemote(manga, fetchChapters = true).getOrElse { return null }
+        return getChapterByUrlAndMangaId.await(sChapter.url, manga.id)
     }
 
     sealed interface State {

data/src/main/java/tachiyomi/data/backup/RestoreRepositoryImpl.kt

@@ -145,6 +145,16 @@ class RestoreRepositoryImpl(
             .associateBy { it.url }
 
         val (existingChapters, newChapters) = restoredChapters
+            .groupBy { it.url }
+            .map { (_, copies) ->
+                copies.reduce { kept, other ->
+                    kept.copy(
+                        read = kept.read || other.read,
+                        bookmark = kept.bookmark || other.bookmark,
+                        lastPageRead = max(kept.lastPageRead, other.lastPageRead),
+                    )
+                }
+            }
             .mapNotNull {
                 val chapter = it.copy(mangaId = manga.id)
 

data/src/main/java/tachiyomi/data/chapter/ChapterRepositoryImpl.kt

@@ -9,9 +9,7 @@ import dev.zacsweers.metro.Inject
 import dev.zacsweers.metro.SingleIn
 import kotlinx.coroutines.flow.Flow
 import kotlinx.serialization.json.JsonObject
-import logcat.LogPriority
 import tachiyomi.core.common.util.lang.toLong
-import tachiyomi.core.common.util.system.logcat
 import tachiyomi.data.Database
 import tachiyomi.data.subscribeToList
 import tachiyomi.domain.chapter.model.Chapter
@@ -26,10 +24,33 @@ class ChapterRepositoryImpl(
     private val database: Database,
 ) : ChapterRepository {
 
-    override suspend fun addAll(chapters: List<Chapter>): List<Chapter> {
-        return try {
-            database.transactionWithResult {
-                chapters.map { chapter ->
+    override suspend fun update(chapterUpdate: ChapterUpdate) {
+        partialUpdate(chapterUpdate)
+    }
+
+    override suspend fun updateAll(chapterUpdates: List<ChapterUpdate>) {
+        partialUpdate(*chapterUpdates.toTypedArray())
+    }
+
+    override suspend fun updateFromRemote(
+        removedIds: List<Long>,
+        added: List<Chapter>,
+        updated: List<ChapterRemoteUpdate>,
+    ): List<Chapter> {
+        return database.transactionWithResult {
+            if (removedIds.isNotEmpty()) {
+                database.chaptersQueries.removeChaptersWithIds(removedIds)
+            }
+            val existing = added.map { it.mangaId }
+                .distinct()
+                .flatMap { mangaId ->
+                    database.chaptersQueries
+                        .getChaptersByMangaId(mangaId, applyScanlatorFilter = false.toLong(), ::mapChapter)
+                        .awaitAsList()
+                        .map { mangaId to it.url }
+                }
+                .toMutableSet()
+            val stored = added.filter { existing.add(it.mangaId to it.url) }.map { chapter ->
                 val chapterId = database.chaptersQueries.insertReturningId(
                     chapter.mangaId,
                     chapter.url,
@@ -47,24 +68,7 @@ class ChapterRepositoryImpl(
                     .awaitAsOne()
                 chapter.copy(id = chapterId)
             }
-            }
-        } catch (e: Exception) {
-            logcat(LogPriority.ERROR, e)
-            emptyList()
-        }
-    }
-
-    override suspend fun update(chapterUpdate: ChapterUpdate) {
-        partialUpdate(chapterUpdate)
-    }
-
-    override suspend fun updateAll(chapterUpdates: List<ChapterUpdate>) {
-        partialUpdate(*chapterUpdates.toTypedArray())
-    }
-
-    override suspend fun updateAllRemote(chapterUpdates: List<ChapterRemoteUpdate>) {
-        database.transaction {
-            chapterUpdates.forEach { chapterUpdate ->
+            updated.forEach { chapterUpdate ->
                 database.chaptersQueries.updateRemote(
                     name = chapterUpdate.name,
                     scanlator = chapterUpdate.scanlator,
@@ -75,6 +79,7 @@ class ChapterRepositoryImpl(
                     memo = chapterUpdate.memo,
                 )
             }
+            stored
         }
     }
 
@@ -92,14 +97,6 @@ class ChapterRepositoryImpl(
         }
     }
 
-    override suspend fun removeChaptersWithIds(chapterIds: List<Long>) {
-        try {
-            database.chaptersQueries.removeChaptersWithIds(chapterIds)
-        } catch (e: Exception) {
-            logcat(LogPriority.ERROR, e)
-        }
-    }
-
     override suspend fun getChapterByMangaId(mangaId: Long, applyScanlatorFilter: Boolean): List<Chapter> {
         return database.chaptersQueries
             .getChaptersByMangaId(mangaId, applyScanlatorFilter.toLong(), ::mapChapter)

domain/src/main/java/tachiyomi/domain/chapter/interactor/UpdateChapter.kt

@@ -3,7 +3,6 @@ package tachiyomi.domain.chapter.interactor
 import dev.zacsweers.metro.Inject
 import logcat.LogPriority
 import tachiyomi.core.common.util.system.logcat
-import tachiyomi.domain.chapter.model.ChapterRemoteUpdate
 import tachiyomi.domain.chapter.model.ChapterUpdate
 import tachiyomi.domain.chapter.repository.ChapterRepository
 
@@ -27,12 +26,4 @@ class UpdateChapter(
             logcat(LogPriority.ERROR, e)
         }
     }
-
-    suspend fun awaitAllRemote(chapterUpdates: List<ChapterRemoteUpdate>) {
-        try {
-            chapterRepository.updateAllRemote(chapterUpdates)
-        } catch (e: Exception) {
-            logcat(LogPriority.ERROR, e)
-        }
-    }
 }

domain/src/main/java/tachiyomi/domain/chapter/repository/ChapterRepository.kt

@@ -7,15 +7,15 @@ import tachiyomi.domain.chapter.model.ChapterUpdate
 
 interface ChapterRepository {
 
-    suspend fun addAll(chapters: List<Chapter>): List<Chapter>
-
     suspend fun update(chapterUpdate: ChapterUpdate)
 
     suspend fun updateAll(chapterUpdates: List<ChapterUpdate>)
 
-    suspend fun updateAllRemote(chapterUpdates: List<ChapterRemoteUpdate>)
-
-    suspend fun removeChaptersWithIds(chapterIds: List<Long>)
+    suspend fun updateFromRemote(
+        removedIds: List<Long>,
+        added: List<Chapter>,
+        updated: List<ChapterRemoteUpdate>,
+    ): List<Chapter>
 
     suspend fun getChapterByMangaId(mangaId: Long, applyScanlatorFilter: Boolean = false): List<Chapter>