From 1fc893ecb31138c3529ffb80a1c8fc05ea62bb07 Mon Sep 17 00:00:00 2001 From: Gauthier Roebroeck Date: Thu, 4 Jun 2020 16:12:29 +0800 Subject: [PATCH] feat(analysis): handle read progress during book analysis when a book is changed on disk, it is marked as outdated. If an outdated book has a different page count during analysis, then all existing read progress for that book will be removed. --- komga-webui/src/types/enum-books.ts | 3 +- .../komga/application/tasks/TaskHandler.kt | 2 +- .../komga/application/tasks/TaskReceiver.kt | 4 +- .../org/gotson/komga/domain/model/Media.kt | 4 +- .../komga/domain/service/BookLifecycle.kt | 7 + .../komga/domain/service/LibraryScanner.kt | 3 +- .../komga/interfaces/rest/BookController.kt | 11 +- .../komga/domain/service/BookLifecycleTest.kt | 132 ++++++++++++++++++ .../domain/service/LibraryScannerTest.kt | 36 +++++ 9 files changed, 190 insertions(+), 12 deletions(-) create mode 100644 komga/src/test/kotlin/org/gotson/komga/domain/service/BookLifecycleTest.kt diff --git a/komga-webui/src/types/enum-books.ts b/komga-webui/src/types/enum-books.ts index deddbf625..11b490521 100644 --- a/komga-webui/src/types/enum-books.ts +++ b/komga-webui/src/types/enum-books.ts @@ -9,7 +9,8 @@ export enum MediaStatus { READY = 'READY', UNKNOWN = 'UNKNOWN', ERROR = 'ERROR', - UNSUPPORTED = 'UNSUPPORTED' + UNSUPPORTED = 'UNSUPPORTED', + OUTDATED = 'OUTDATED' } export enum ReadProgress { diff --git a/komga/src/main/kotlin/org/gotson/komga/application/tasks/TaskHandler.kt b/komga/src/main/kotlin/org/gotson/komga/application/tasks/TaskHandler.kt index 0d5ff3140..83ff80cc0 100644 --- a/komga/src/main/kotlin/org/gotson/komga/application/tasks/TaskHandler.kt +++ b/komga/src/main/kotlin/org/gotson/komga/application/tasks/TaskHandler.kt @@ -34,7 +34,7 @@ class TaskHandler( is Task.ScanLibrary -> libraryRepository.findByIdOrNull(task.libraryId)?.let { libraryScanner.scanRootFolder(it) - taskReceiver.analyzeUnknownBooks(it) + taskReceiver.analyzeUnknownAndOutdatedBooks(it) } ?: logger.warn { "Cannot execute task $task: Library does not exist" } is Task.AnalyzeBook -> diff --git a/komga/src/main/kotlin/org/gotson/komga/application/tasks/TaskReceiver.kt b/komga/src/main/kotlin/org/gotson/komga/application/tasks/TaskReceiver.kt index 8f593ea1d..f8ee650e5 100644 --- a/komga/src/main/kotlin/org/gotson/komga/application/tasks/TaskReceiver.kt +++ b/komga/src/main/kotlin/org/gotson/komga/application/tasks/TaskReceiver.kt @@ -31,10 +31,10 @@ class TaskReceiver( submitTask(Task.ScanLibrary(libraryId)) } - fun analyzeUnknownBooks(library: Library) { + fun analyzeUnknownAndOutdatedBooks(library: Library) { bookRepository.findAllId(BookSearch( libraryIds = listOf(library.id), - mediaStatus = listOf(Media.Status.UNKNOWN) + mediaStatus = listOf(Media.Status.UNKNOWN, Media.Status.OUTDATED) )).forEach { submitTask(Task.AnalyzeBook(it)) } diff --git a/komga/src/main/kotlin/org/gotson/komga/domain/model/Media.kt b/komga/src/main/kotlin/org/gotson/komga/domain/model/Media.kt index 1dd0f527f..b4fae1a5f 100644 --- a/komga/src/main/kotlin/org/gotson/komga/domain/model/Media.kt +++ b/komga/src/main/kotlin/org/gotson/komga/domain/model/Media.kt @@ -14,8 +14,6 @@ class Media( override val lastModifiedDate: LocalDateTime = LocalDateTime.now() ) : Auditable() { - fun reset() = Media(bookId = this.bookId) - fun copy( status: Status = this.status, mediaType: String? = this.mediaType, @@ -40,7 +38,7 @@ class Media( ) enum class Status { - UNKNOWN, ERROR, READY, UNSUPPORTED + UNKNOWN, ERROR, READY, UNSUPPORTED, OUTDATED } override fun toString(): String = diff --git a/komga/src/main/kotlin/org/gotson/komga/domain/service/BookLifecycle.kt b/komga/src/main/kotlin/org/gotson/komga/domain/service/BookLifecycle.kt index bd1dec488..7722fa92c 100644 --- a/komga/src/main/kotlin/org/gotson/komga/domain/service/BookLifecycle.kt +++ b/komga/src/main/kotlin/org/gotson/komga/domain/service/BookLifecycle.kt @@ -36,6 +36,13 @@ class BookLifecycle( logger.error(ex) { "Error while analyzing book: $book" } Media(status = Media.Status.ERROR, comment = ex.message) }.copy(bookId = book.id) + + // if the number of pages has changed, delete all read progress for that book + val previous = mediaRepository.findById(book.id) + if (previous.status == Media.Status.OUTDATED && previous.pages.size != media.pages.size) { + readProgressRepository.deleteByBookId(book.id) + } + mediaRepository.update(media) } diff --git a/komga/src/main/kotlin/org/gotson/komga/domain/service/LibraryScanner.kt b/komga/src/main/kotlin/org/gotson/komga/domain/service/LibraryScanner.kt index de9f6534b..4fd69c8a9 100644 --- a/komga/src/main/kotlin/org/gotson/komga/domain/service/LibraryScanner.kt +++ b/komga/src/main/kotlin/org/gotson/komga/domain/service/LibraryScanner.kt @@ -2,6 +2,7 @@ package org.gotson.komga.domain.service import mu.KotlinLogging import org.gotson.komga.domain.model.Library +import org.gotson.komga.domain.model.Media import org.gotson.komga.domain.persistence.BookRepository import org.gotson.komga.domain.persistence.MediaRepository import org.gotson.komga.domain.persistence.SeriesRepository @@ -74,7 +75,7 @@ class LibraryScanner( fileSize = newBook.fileSize ) mediaRepository.findById(existingBook.id).let { - mediaRepository.update(it.reset()) + mediaRepository.update(it.copy(status = Media.Status.OUTDATED)) } bookRepository.update(updatedBook) } diff --git a/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/BookController.kt b/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/BookController.kt index a8f6ef52f..54a48ba53 100644 --- a/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/BookController.kt +++ b/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/BookController.kt @@ -222,10 +222,13 @@ class BookController( if (!principal.user.canAccessBook(book)) throw ResponseStatusException(HttpStatus.UNAUTHORIZED) val media = mediaRepository.findById(book.id) - if (media.status == Media.Status.UNKNOWN) throw ResponseStatusException(HttpStatus.NOT_FOUND, "Book has not been analyzed yet") - if (media.status in listOf(Media.Status.ERROR, Media.Status.UNSUPPORTED)) throw ResponseStatusException(HttpStatus.NOT_FOUND, "Book analysis failed") - - media.pages.mapIndexed { index, s -> PageDto(index + 1, s.fileName, s.mediaType) } + when (media.status) { + Media.Status.UNKNOWN -> throw ResponseStatusException(HttpStatus.NOT_FOUND, "Book has not been analyzed yet") + Media.Status.OUTDATED -> throw ResponseStatusException(HttpStatus.NOT_FOUND, "Book is outdated and must be re-analyzed") + Media.Status.ERROR -> throw ResponseStatusException(HttpStatus.NOT_FOUND, "Book analysis failed") + Media.Status.UNSUPPORTED -> throw ResponseStatusException(HttpStatus.NOT_FOUND, "Book format is not supported") + Media.Status.READY -> media.pages.mapIndexed { index, s -> PageDto(index + 1, s.fileName, s.mediaType) } + } } ?: throw ResponseStatusException(HttpStatus.NOT_FOUND) @ApiResponse(content = [Content( diff --git a/komga/src/test/kotlin/org/gotson/komga/domain/service/BookLifecycleTest.kt b/komga/src/test/kotlin/org/gotson/komga/domain/service/BookLifecycleTest.kt new file mode 100644 index 000000000..395c0c934 --- /dev/null +++ b/komga/src/test/kotlin/org/gotson/komga/domain/service/BookLifecycleTest.kt @@ -0,0 +1,132 @@ +package org.gotson.komga.domain.service + +import com.ninjasquad.springmockk.MockkBean +import io.mockk.every +import org.assertj.core.api.Assertions.assertThat +import org.gotson.komga.domain.model.BookPage +import org.gotson.komga.domain.model.KomgaUser +import org.gotson.komga.domain.model.Media +import org.gotson.komga.domain.model.makeBook +import org.gotson.komga.domain.model.makeBookPage +import org.gotson.komga.domain.model.makeLibrary +import org.gotson.komga.domain.model.makeSeries +import org.gotson.komga.domain.persistence.BookRepository +import org.gotson.komga.domain.persistence.KomgaUserRepository +import org.gotson.komga.domain.persistence.LibraryRepository +import org.gotson.komga.domain.persistence.MediaRepository +import org.gotson.komga.domain.persistence.ReadProgressRepository +import org.gotson.komga.domain.persistence.SeriesRepository +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.test.context.junit.jupiter.SpringExtension + +@ExtendWith(SpringExtension::class) +@SpringBootTest +@AutoConfigureTestDatabase +class BookLifecycleTest( + @Autowired private val bookLifecycle: BookLifecycle, + @Autowired private val bookRepository: BookRepository, + @Autowired private val libraryRepository: LibraryRepository, + @Autowired private val seriesRepository: SeriesRepository, + @Autowired private val seriesLifecycle: SeriesLifecycle, + @Autowired private val readProgressRepository: ReadProgressRepository, + @Autowired private val mediaRepository: MediaRepository, + @Autowired private val userRepository: KomgaUserRepository +) { + + @MockkBean + private lateinit var mockAnalyzer: BookAnalyzer + + private var library = makeLibrary() + private var user1 = KomgaUser("user1@example.org", "", false) + private var user2 = KomgaUser("user2@example.org", "", false) + + @BeforeAll + fun `setup library`() { + library = libraryRepository.insert(library) + + user1 = userRepository.save(user1) + user2 = userRepository.save(user2) + } + + @AfterAll + fun teardown() { + libraryRepository.deleteAll() + userRepository.deleteAll() + } + + @AfterEach + fun `clear repository`() { + seriesRepository.findAll().forEach { + seriesLifecycle.deleteSeries(it.id) + } + } + + @Test + fun `given outdated book with different number of pages than before when analyzing then existing read progress is deleted`() { + // given + makeSeries(name = "series", libraryId = library.id).let { series -> + seriesLifecycle.createSeries(series).let { created -> + val books = listOf(makeBook("1", libraryId = library.id)) + seriesLifecycle.addBooks(created, books) + } + } + + val book = bookRepository.findAll().first() + mediaRepository.findById(book.id).let { media -> + mediaRepository.update(media.copy( + status = Media.Status.OUTDATED, + pages = (1..10).map { BookPage("$it", "image/jpeg") } + )) + } + + bookLifecycle.markReadProgressCompleted(book.id, user1) + bookLifecycle.markReadProgress(book, user2, 4) + + assertThat(readProgressRepository.findAll()).hasSize(2) + + // when + every { mockAnalyzer.analyze(any()) } returns Media(status = Media.Status.READY, mediaType = "application/zip", pages = mutableListOf(makeBookPage("1.jpg"), makeBookPage("2.jpg"))) + bookLifecycle.analyzeAndPersist(book) + + // then + assertThat(readProgressRepository.findAll()).isEmpty() + } + + @Test + fun `given outdated book with same number of pages than before when analyzing then existing read progress is kept`() { + // given + makeSeries(name = "series", libraryId = library.id).let { series -> + seriesLifecycle.createSeries(series).let { created -> + val books = listOf(makeBook("1", libraryId = library.id)) + seriesLifecycle.addBooks(created, books) + } + } + + val book = bookRepository.findAll().first() + mediaRepository.findById(book.id).let { media -> + mediaRepository.update(media.copy( + status = Media.Status.OUTDATED, + pages = (1..10).map { BookPage("$it", "image/jpeg") } + )) + } + + bookLifecycle.markReadProgressCompleted(book.id, user1) + bookLifecycle.markReadProgress(book, user2, 4) + + assertThat(readProgressRepository.findAll()).hasSize(2) + + // when + every { mockAnalyzer.analyze(any()) } returns Media(status = Media.Status.READY, mediaType = "application/zip", pages = (1..10).map { BookPage("$it", "image/jpeg") }) + bookLifecycle.analyzeAndPersist(book) + + // then + assertThat(readProgressRepository.findAll()).hasSize(2) + } +} diff --git a/komga/src/test/kotlin/org/gotson/komga/domain/service/LibraryScannerTest.kt b/komga/src/test/kotlin/org/gotson/komga/domain/service/LibraryScannerTest.kt index 80ac50b2b..3f9623bbb 100644 --- a/komga/src/test/kotlin/org/gotson/komga/domain/service/LibraryScannerTest.kt +++ b/komga/src/test/kotlin/org/gotson/komga/domain/service/LibraryScannerTest.kt @@ -219,6 +219,42 @@ class LibraryScannerTest( } } + @Test + fun `given existing Book with different last modified date when rescanning then media is marked as outdated`() { + // given + val library = libraryRepository.insert(makeLibrary()) + + val book1 = makeBook("book1") + every { mockScanner.scanRootFolder(any()) } + .returnsMany( + mapOf(makeSeries(name = "series") to listOf(book1)), + mapOf(makeSeries(name = "series") to listOf(makeBook(name = "book1"))) + ) + libraryScanner.scanRootFolder(library) + + every { mockAnalyzer.analyze(any()) } returns Media(status = Media.Status.READY, mediaType = "application/zip", pages = mutableListOf(makeBookPage("1.jpg"), makeBookPage("2.jpg"))) + bookRepository.findAll().map { bookLifecycle.analyzeAndPersist(it) } + + // when + libraryScanner.scanRootFolder(library) + + // then + verify(exactly = 2) { mockScanner.scanRootFolder(any()) } + verify(exactly = 1) { mockAnalyzer.analyze(any()) } + + bookRepository.findAll().first().let { book -> + assertThat(book.lastModifiedDate).isNotEqualTo(book.createdDate) + + mediaRepository.findById(book.id).let { media -> + assertThat(media.status).isEqualTo(Media.Status.OUTDATED) + assertThat(media.mediaType).isEqualTo("application/zip") + assertThat(media.pages).hasSize(2) + assertThat(media.pages.map { it.fileName }).containsExactly("1.jpg", "2.jpg") + } + + } + } + @Test fun `given 2 libraries when deleting all books of one and scanning then the other library is kept intact`() { // given