fix(komga): better handling of collection/readlist creation/update when using multiple threads

Closes: #1317

komga/src/main/kotlin/org/gotson/komga/domain/service/BookMetadataLifecycle.kt

@@ -7,11 +7,9 @@ import org.gotson.komga.domain.model.BookMetadataPatchCapability
 import org.gotson.komga.domain.model.BookWithMedia
 import org.gotson.komga.domain.model.DomainEvent
 import org.gotson.komga.domain.model.MetadataPatchTarget
-import org.gotson.komga.domain.model.ReadList
 import org.gotson.komga.domain.persistence.BookMetadataRepository
 import org.gotson.komga.domain.persistence.LibraryRepository
 import org.gotson.komga.domain.persistence.MediaRepository
-import org.gotson.komga.domain.persistence.ReadListRepository
 import org.gotson.komga.infrastructure.metadata.BookMetadataProvider
 import org.springframework.context.ApplicationEventPublisher
 import org.springframework.stereotype.Service
@@ -25,7 +23,6 @@ class BookMetadataLifecycle(
   private val mediaRepository: MediaRepository,
   private val bookMetadataRepository: BookMetadataRepository,
   private val libraryRepository: LibraryRepository,
-  private val readListRepository: ReadListRepository,
   private val readListLifecycle: ReadListLifecycle,
   private val eventPublisher: ApplicationEventPublisher,
 ) {
@@ -60,7 +57,9 @@ class BookMetadataLifecycle(
           }
 
           if (provider.shouldLibraryHandlePatch(library, MetadataPatchTarget.READLIST)) {
-            handlePatchForReadLists(patch, book)
+            patch?.readLists?.forEach { readList ->
+              readListLifecycle.addBookToReadList(readList.name, book, readList.number)
+            }
           }
         }
       }
@@ -69,43 +68,6 @@ class BookMetadataLifecycle(
     if (changed) eventPublisher.publishEvent(DomainEvent.BookUpdated(book))
   }
 
-  private fun handlePatchForReadLists(
-    patch: BookMetadataPatch?,
-    book: Book,
-  ) {
-    patch?.readLists?.forEach { readList ->
-
-      readListRepository.findByNameOrNull(readList.name).let { existing ->
-        if (existing != null) {
-          if (existing.bookIds.containsValue(book.id))
-            logger.debug { "Book is already in existing read list '${existing.name}'" }
-          else {
-            val map = existing.bookIds.toSortedMap()
-            val key = if (readList.number != null && existing.bookIds.containsKey(readList.number)) {
-              logger.debug { "Existing read list '${existing.name}' already contains a book at position ${readList.number}, adding book '${book.name}' at the end" }
-              existing.bookIds.lastKey() + 1
-            } else {
-              logger.debug { "Adding book '${book.name}' to existing read list '${existing.name}'" }
-              readList.number ?: (existing.bookIds.lastKey() + 1)
-            }
-            map[key] = book.id
-            readListLifecycle.updateReadList(
-              existing.copy(bookIds = map),
-            )
-          }
-        } else {
-          logger.debug { "Adding book '${book.name}' to new read list '$readList'" }
-          readListLifecycle.addReadList(
-            ReadList(
-              name = readList.name,
-              bookIds = mapOf((readList.number ?: 0) to book.id).toSortedMap(),
-            ),
-          )
-        }
-      }
-    }
-  }
-
   private fun handlePatchForBookMetadata(
     patch: BookMetadataPatch?,
     book: Book,

komga/src/main/kotlin/org/gotson/komga/domain/service/ReadListLifecycle.kt

@@ -1,6 +1,7 @@
 package org.gotson.komga.domain.service
 
 import mu.KotlinLogging
+import org.gotson.komga.domain.model.Book
 import org.gotson.komga.domain.model.DomainEvent
 import org.gotson.komga.domain.model.DuplicateNameException
 import org.gotson.komga.domain.model.ReadList
@@ -12,6 +13,7 @@ import org.gotson.komga.infrastructure.image.MosaicGenerator
 import org.gotson.komga.infrastructure.metadata.comicrack.ReadListProvider
 import org.springframework.context.ApplicationEventPublisher
 import org.springframework.stereotype.Service
+import org.springframework.transaction.annotation.Transactional
 import org.springframework.transaction.support.TransactionTemplate
 
 private val logger = KotlinLogging.logger {}
@@ -31,6 +33,7 @@ class ReadListLifecycle(
   @Throws(
     DuplicateNameException::class,
   )
+  @Transactional
   fun addReadList(readList: ReadList): ReadList {
     logger.info { "Adding new read list: $readList" }
 
@@ -44,6 +47,7 @@ class ReadListLifecycle(
     return readListRepository.findByIdOrNull(readList.id)!!
   }
 
+  @Transactional
   fun updateReadList(toUpdate: ReadList) {
     logger.info { "Update read list: $toUpdate" }
     val existing = readListRepository.findByIdOrNull(toUpdate.id)
@@ -66,6 +70,42 @@ class ReadListLifecycle(
     eventPublisher.publishEvent(DomainEvent.ReadListDeleted(readList))
   }
 
+  /**
+   * Add book to read list by name.
+   * Read list will be created if it doesn't exist.
+   */
+  @Transactional
+  fun addBookToReadList(readListName: String, book: Book, numberInList: Int?) {
+    readListRepository.findByNameOrNull(readListName).let { existing ->
+      if (existing != null) {
+        if (existing.bookIds.containsValue(book.id))
+          logger.debug { "Book is already in existing read list '${existing.name}'" }
+        else {
+          val map = existing.bookIds.toSortedMap()
+          val key = if (numberInList != null && existing.bookIds.containsKey(numberInList)) {
+            logger.debug { "Existing read list '${existing.name}' already contains a book at position $numberInList, adding book '${book.name}' at the end" }
+            existing.bookIds.lastKey() + 1
+          } else {
+            logger.debug { "Adding book '${book.name}' to existing read list '${existing.name}'" }
+            numberInList ?: (existing.bookIds.lastKey() + 1)
+          }
+          map[key] = book.id
+          updateReadList(
+            existing.copy(bookIds = map),
+          )
+        }
+      } else {
+        logger.debug { "Adding book '${book.name}' to new read list '$readListName'" }
+        addReadList(
+          ReadList(
+            name = readListName,
+            bookIds = mapOf((numberInList ?: 0) to book.id).toSortedMap(),
+          ),
+        )
+      }
+    }
+  }
+
   fun deleteEmptyReadLists() {
     logger.info { "Deleting empty read lists" }
     transactionTemplate.executeWithoutResult {

komga/src/main/kotlin/org/gotson/komga/domain/service/SeriesCollectionLifecycle.kt

@@ -3,6 +3,7 @@ package org.gotson.komga.domain.service
 import mu.KotlinLogging
 import org.gotson.komga.domain.model.DomainEvent
 import org.gotson.komga.domain.model.DuplicateNameException
+import org.gotson.komga.domain.model.Series
 import org.gotson.komga.domain.model.SeriesCollection
 import org.gotson.komga.domain.model.ThumbnailSeriesCollection
 import org.gotson.komga.domain.persistence.SeriesCollectionRepository
@@ -10,6 +11,7 @@ import org.gotson.komga.domain.persistence.ThumbnailSeriesCollectionRepository
 import org.gotson.komga.infrastructure.image.MosaicGenerator
 import org.springframework.context.ApplicationEventPublisher
 import org.springframework.stereotype.Service
+import org.springframework.transaction.annotation.Transactional
 import org.springframework.transaction.support.TransactionTemplate
 
 private val logger = KotlinLogging.logger {}
@@ -27,6 +29,7 @@ class SeriesCollectionLifecycle(
   @Throws(
     DuplicateNameException::class,
   )
+  @Transactional
   fun addCollection(collection: SeriesCollection): SeriesCollection {
     logger.info { "Adding new collection: $collection" }
 
@@ -40,6 +43,7 @@ class SeriesCollectionLifecycle(
     return collectionRepository.findByIdOrNull(collection.id)!!
   }
 
+  @Transactional
   fun updateCollection(toUpdate: SeriesCollection) {
     logger.info { "Update collection: $toUpdate" }
 
@@ -62,6 +66,34 @@ class SeriesCollectionLifecycle(
     eventPublisher.publishEvent(DomainEvent.CollectionDeleted(collection))
   }
 
+  /**
+   * Add series to collection by name.
+   * Collection will be created if it doesn't exist.
+   */
+  @Transactional
+  fun addSeriesToCollection(collectionName: String, series: Series) {
+    collectionRepository.findByNameOrNull(collectionName).let { existing ->
+      if (existing != null) {
+        if (existing.seriesIds.contains(series.id))
+          logger.debug { "Series is already in existing collection '${existing.name}'" }
+        else {
+          logger.debug { "Adding series '${series.name}' to existing collection '${existing.name}'" }
+          updateCollection(
+            existing.copy(seriesIds = existing.seriesIds + series.id),
+          )
+        }
+      } else {
+        logger.debug { "Adding series '${series.name}' to new collection '$collectionName'" }
+        addCollection(
+          SeriesCollection(
+            name = collectionName,
+            seriesIds = listOf(series.id),
+          ),
+        )
+      }
+    }
+  }
+
   fun deleteEmptyCollections() {
     logger.info { "Deleting empty collections" }
     transactionTemplate.executeWithoutResult {

komga/src/main/kotlin/org/gotson/komga/domain/service/SeriesMetadataLifecycle.kt

@@ -5,14 +5,12 @@ import org.gotson.komga.domain.model.BookWithMedia
 import org.gotson.komga.domain.model.DomainEvent
 import org.gotson.komga.domain.model.MetadataPatchTarget
 import org.gotson.komga.domain.model.Series
-import org.gotson.komga.domain.model.SeriesCollection
 import org.gotson.komga.domain.model.SeriesMetadataPatch
 import org.gotson.komga.domain.persistence.BookMetadataAggregationRepository
 import org.gotson.komga.domain.persistence.BookMetadataRepository
 import org.gotson.komga.domain.persistence.BookRepository
 import org.gotson.komga.domain.persistence.LibraryRepository
 import org.gotson.komga.domain.persistence.MediaRepository
-import org.gotson.komga.domain.persistence.SeriesCollectionRepository
 import org.gotson.komga.domain.persistence.SeriesMetadataRepository
 import org.gotson.komga.infrastructure.metadata.SeriesMetadataFromBookProvider
 import org.gotson.komga.infrastructure.metadata.SeriesMetadataProvider
@@ -34,7 +32,6 @@ class SeriesMetadataLifecycle(
   private val bookMetadataAggregationRepository: BookMetadataAggregationRepository,
   private val libraryRepository: LibraryRepository,
   private val bookRepository: BookRepository,
-  private val collectionRepository: SeriesCollectionRepository,
   private val collectionLifecycle: SeriesCollectionLifecycle,
   private val eventPublisher: ApplicationEventPublisher,
 ) {
@@ -68,7 +65,9 @@ class SeriesMetadataLifecycle(
           }
 
           if (provider.shouldLibraryHandlePatch(library, MetadataPatchTarget.COLLECTION)) {
-            handlePatchForCollections(patches, series)
+            patches.flatMap { it.collections }.distinct().forEach { collection ->
+              collectionLifecycle.addSeriesToCollection(collection, series)
+            }
           }
         }
       }
@@ -98,34 +97,6 @@ class SeriesMetadataLifecycle(
     if (changed) eventPublisher.publishEvent(DomainEvent.SeriesUpdated(series))
   }
 
-  private fun handlePatchForCollections(
-    patches: List<SeriesMetadataPatch>,
-    series: Series,
-  ) {
-    patches.flatMap { it.collections }.distinct().forEach { collection ->
-      collectionRepository.findByNameOrNull(collection).let { existing ->
-        if (existing != null) {
-          if (existing.seriesIds.contains(series.id))
-            logger.debug { "Series is already in existing collection '${existing.name}'" }
-          else {
-            logger.debug { "Adding series '${series.name}' to existing collection '${existing.name}'" }
-            collectionLifecycle.updateCollection(
-              existing.copy(seriesIds = existing.seriesIds + series.id),
-            )
-          }
-        } else {
-          logger.debug { "Adding series '${series.name}' to new collection '$collection'" }
-          collectionLifecycle.addCollection(
-            SeriesCollection(
-              name = collection,
-              seriesIds = listOf(series.id),
-            ),
-          )
-        }
-      }
-    }
-  }
-
   private fun handlePatchForSeriesMetadata(
     patches: List<SeriesMetadataPatch>,
     series: Series,