fix: don't fail epub analysis when optional features are missing

Refs: #1909

ERRORCODES.md

@@ -38,3 +38,8 @@
 | ERR_1032     | EPUB file has wrong media type                          |
 | ERR_1033     | Some entries are missing                                |
 | ERR_1034     | An API key with that comment already exists             |
+| ERR_1035     | Error while getting EPUB TOC                            |
+| ERR_1036     | Error while getting EPUB Landmarks                      |
+| ERR_1037     | Error while getting EPUB page list                      |
+| ERR_1038     | Error while getting EPUB divina pages                   |
+| ERR_1039     | Error while getting EPUB positions                      |

komga-webui/src/locales/en.json

@@ -827,7 +827,12 @@
     "ERR_1031": "ComicRack CBL Book is missing series or number",
     "ERR_1032": "EPUB file has wrong media type",
     "ERR_1033": "Some entries are missing",
-    "ERR_1034": "An API key with that comment already exists"
+    "ERR_1034": "An API key with that comment already exists",
+    "ERR_1035": "Error while getting EPUB TOC",
+    "ERR_1036": "Error while getting EPUB Landmarks",
+    "ERR_1037": "Error while getting EPUB page list",
+    "ERR_1038": "Error while getting EPUB divina pages",
+    "ERR_1039": "Error while getting EPUB positions"
   },
   "filter": {
     "age_rating": "age rating",

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

@@ -23,6 +23,7 @@ import org.gotson.komga.infrastructure.image.ImageType
 import org.gotson.komga.infrastructure.mediacontainer.ContentDetector
 import org.gotson.komga.infrastructure.mediacontainer.divina.DivinaExtractor
 import org.gotson.komga.infrastructure.mediacontainer.epub.EpubExtractor
+import org.gotson.komga.infrastructure.mediacontainer.epub.epub
 import org.gotson.komga.infrastructure.mediacontainer.pdf.PdfExtractor
 import org.springframework.beans.factory.annotation.Qualifier
 import org.springframework.beans.factory.annotation.Value
@@ -143,30 +144,85 @@ class BookAnalyzer(
     book: Book,
     analyzeDimensions: Boolean,
   ): Media {
-    val manifest = epubExtractor.getManifest(book.path, analyzeDimensions)
+    book.path.epub { epub ->
+      val (resources, missingResources) = epubExtractor.getResources(epub).partition { it.fileSize != null }
+      val isFixedLayout = epubExtractor.isFixedLayout(epub)
+      val pageCount = epubExtractor.computePageCount(epub)
+      val isKepub = epubExtractor.isKepub(epub, resources)
+
+      val errors = mutableListOf<String>()
+
+      val toc =
+        try {
+          epubExtractor.getToc(epub)
+        } catch (e: Exception) {
+          logger.error(e) { "Error while getting EPUB TOC" }
+          errors.add("ERR_1035")
+          emptyList()
+        }
+
+      val landmarks =
+        try {
+          epubExtractor.getLandmarks(epub)
+        } catch (e: Exception) {
+          logger.error(e) { "Error while getting EPUB Landmarks" }
+          errors.add("ERR_1036")
+          emptyList()
+        }
+
+      val pageList =
+        try {
+          epubExtractor.getPageList(epub)
+        } catch (e: Exception) {
+          logger.error(e) { "Error while getting EPUB page list" }
+          errors.add("ERR_1037")
+          emptyList()
+        }
+
+      val divinaPages =
+        try {
+          epubExtractor.getDivinaPages(epub, isFixedLayout, pageCount, analyzeDimensions)
+        } catch (e: Exception) {
+          logger.error(e) { "Error while getting EPUB Divina pages" }
+          errors.add("ERR_1038")
+          emptyList()
+        }
+
+      val positions =
+        try {
+          epubExtractor.computePositions(epub, book.path, resources, isFixedLayout, isKepub)
+        } catch (e: Exception) {
+          logger.error(e) { "Error while getting EPUB positions" }
+          errors.add("ERR_1039")
+          emptyList()
+        }
+
       val entriesErrorSummary =
-      manifest.missingResources
+        missingResources
           .map { it.fileName }
           .ifEmpty { null }
           ?.joinToString(prefix = "ERR_1033 [", postfix = "]") { it }
+      val allErrors = (errors + entriesErrorSummary).joinToString(" ")
+
       return Media(
         status = Media.Status.READY,
-      pages = manifest.divinaPages,
+        pages = divinaPages,
-      files = manifest.resources,
+        files = resources,
-      pageCount = manifest.pageCount,
+        pageCount = pageCount,
-      epubDivinaCompatible = manifest.divinaPages.isNotEmpty(),
+        epubDivinaCompatible = divinaPages.isNotEmpty(),
-      epubIsKepub = manifest.isKepub,
+        epubIsKepub = isKepub,
         extension =
           MediaExtensionEpub(
-          toc = manifest.toc,
+            toc = toc,
-          landmarks = manifest.landmarks,
+            landmarks = landmarks,
-          pageList = manifest.pageList,
+            pageList = pageList,
-          isFixedLayout = manifest.isFixedLayout,
+            isFixedLayout = isFixedLayout,
-          positions = manifest.positions,
+            positions = positions,
           ),
-      comment = entriesErrorSummary,
+        comment = allErrors,
       )
     }
+  }
 
   private fun analyzePdf(
     book: Book,

komga/src/main/kotlin/org/gotson/komga/infrastructure/mediacontainer/epub/EpubExtractor.kt

@@ -81,30 +81,7 @@ class EpubExtractor(
       }
     }
 
-  fun getManifest(
+  fun getResources(epub: EpubPackage): List<MediaFile> {
-    path: Path,
-    analyzeDimensions: Boolean,
-  ): EpubManifest =
-    path.epub { epub ->
-      val (resources, missingResources) = getResources(epub).partition { it.fileSize != null }
-      val isFixedLayout = isFixedLayout(epub)
-      val pageCount = computePageCount(epub)
-      val isKepub = isKepub(epub, resources)
-      EpubManifest(
-        resources = resources,
-        missingResources = missingResources,
-        toc = getToc(epub),
-        landmarks = getLandmarks(epub),
-        pageList = getPageList(epub),
-        pageCount = pageCount,
-        isFixedLayout = isFixedLayout,
-        positions = computePositions(epub, path, resources, isFixedLayout, isKepub),
-        divinaPages = getDivinaPages(epub, isFixedLayout, pageCount, analyzeDimensions),
-        isKepub = isKepub,
-      )
-    }
-
-  private fun getResources(epub: EpubPackage): List<MediaFile> {
     val spine =
       epub.opfDoc
         .select("spine > itemref")
@@ -135,7 +112,7 @@ class EpubExtractor(
     }
   }
 
-  private fun getDivinaPages(
+  fun getDivinaPages(
     epub: EpubPackage,
     isFixedLayout: Boolean,
     pageCount: Int,
@@ -146,7 +123,6 @@ class EpubExtractor(
       return emptyList()
     }
 
-    try {
     val pagesWithImages =
       epub.opfDoc
         .select("spine > itemref")
@@ -205,13 +181,9 @@ class EpubExtractor(
       return emptyList()
     }
     return divinaPages
-    } catch (e: Exception) {
-      logger.warn(e) { "Error while getting divina pages" }
-      return emptyList()
-    }
   }
 
-  private fun isKepub(
+  fun isKepub(
     epub: EpubPackage,
     resources: List<MediaFile>,
   ): Boolean {
@@ -228,7 +200,7 @@ class EpubExtractor(
     return false
   }
 
-  private fun computePageCount(epub: EpubPackage): Int {
+  fun computePageCount(epub: EpubPackage): Int {
     val spine =
       epub.opfDoc
         .select("spine > itemref")
@@ -241,11 +213,11 @@ class EpubExtractor(
       .sumOf { ceil(it.compressedSize / 1024.0).toInt() }
   }
 
-  private fun isFixedLayout(epub: EpubPackage) =
+  fun isFixedLayout(epub: EpubPackage) =
     epub.opfDoc.selectFirst("metadata > *|meta[property=rendition:layout]")?.text() == "pre-paginated" ||
       epub.opfDoc.selectFirst("metadata > *|meta[name=fixed-layout]")?.attr("content") == "true"
 
-  private fun computePositions(
+  fun computePositions(
     epub: EpubPackage,
     path: Path,
     resources: List<MediaFile>,
@@ -346,7 +318,7 @@ class EpubExtractor(
         }
     }
 
-  private fun getToc(epub: EpubPackage): List<EpubTocEntry> {
+  fun getToc(epub: EpubPackage): List<EpubTocEntry> {
     // Epub 3
     epub.getNavResource()?.let { return processNav(it, Epub3Nav.TOC) }
     // Epub 2
@@ -354,7 +326,7 @@ class EpubExtractor(
     return emptyList()
   }
 
-  private fun getPageList(epub: EpubPackage): List<EpubTocEntry> {
+  fun getPageList(epub: EpubPackage): List<EpubTocEntry> {
     // Epub 3
     epub.getNavResource()?.let { return processNav(it, Epub3Nav.PAGELIST) }
     // Epub 2
@@ -362,7 +334,7 @@ class EpubExtractor(
     return emptyList()
   }
 
-  private fun getLandmarks(epub: EpubPackage): List<EpubTocEntry> {
+  fun getLandmarks(epub: EpubPackage): List<EpubTocEntry> {
     // Epub 3
     epub.getNavResource()?.let { return processNav(it, Epub3Nav.LANDMARKS) }
 

komga/src/main/kotlin/org/gotson/komga/infrastructure/mediacontainer/epub/EpubManifest.kt

@@ -1,19 +0,0 @@
-package org.gotson.komga.infrastructure.mediacontainer.epub
-
-import org.gotson.komga.domain.model.BookPage
-import org.gotson.komga.domain.model.EpubTocEntry
-import org.gotson.komga.domain.model.MediaFile
-import org.gotson.komga.domain.model.R2Locator
-
-data class EpubManifest(
-  val resources: List<MediaFile>,
-  val missingResources: List<MediaFile>,
-  val toc: List<EpubTocEntry>,
-  val landmarks: List<EpubTocEntry>,
-  val pageList: List<EpubTocEntry>,
-  val pageCount: Int,
-  val isFixedLayout: Boolean,
-  val positions: List<R2Locator>,
-  val divinaPages: List<BookPage>,
-  val isKepub: Boolean,
-)

komga/src/test/kotlin/org/gotson/komga/domain/service/BookAnalyzerTest.kt

@@ -1,6 +1,7 @@
 package org.gotson.komga.domain.service
 
 import com.ninjasquad.springmockk.SpykBean
+import io.mockk.clearAllMocks
 import io.mockk.every
 import io.mockk.verify
 import org.assertj.core.api.Assertions.assertThat
@@ -8,8 +9,12 @@ import org.gotson.komga.domain.model.Book
 import org.gotson.komga.domain.model.BookPage
 import org.gotson.komga.domain.model.BookWithMedia
 import org.gotson.komga.domain.model.Media
+import org.gotson.komga.domain.model.MediaExtensionEpub
 import org.gotson.komga.domain.model.makeBook
 import org.gotson.komga.infrastructure.configuration.KomgaProperties
+import org.gotson.komga.infrastructure.mediacontainer.epub.EpubExtractor
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.MethodSource
@@ -32,6 +37,16 @@ class BookAnalyzerTest(
   @SpykBean
   private lateinit var bookAnalyzer: BookAnalyzer
 
+  @SpykBean
+  private lateinit var epubExtractor: EpubExtractor
+
+  @AfterEach
+  fun afterEach() {
+    clearAllMocks()
+  }
+
+  @Nested
+  inner class ArchiveFormats {
     @Test
     fun `given rar4 archive when analyzing then media status is READY`() {
       val file = ClassPathResource("archives/rar4.rar")
@@ -131,7 +146,10 @@ class BookAnalyzerTest(
       assertThat(media.status).isEqualTo(Media.Status.READY)
       assertThat(media.pages).hasSize(0)
     }
+  }
 
+  @Nested
+  inner class Epub {
     @Test
     fun `given broken epub archive when analyzing then media status is ERROR`() {
       val file = ClassPathResource("archives/zip-as-epub.epub")
@@ -144,6 +162,92 @@ class BookAnalyzerTest(
       assertThat(media.pages).hasSize(0)
     }
 
+    @Test
+    fun `given epub archive when toc cannot be extracted then media status is READY with comments`() {
+      val file = ClassPathResource("epub/The Incomplete Theft - Ralph Burke.epub")
+      val book = Book("book", file.url, LocalDateTime.now())
+
+      every { epubExtractor.getToc(any()) } throws Exception("mock exception")
+
+      val media = bookAnalyzer.analyze(book, false)
+      val extension = media.extension as? MediaExtensionEpub
+
+      assertThat(media.mediaType).isEqualTo("application/epub+zip")
+      assertThat(media.status).isEqualTo(Media.Status.READY)
+      assertThat(media.comment).contains("ERR_1035")
+      assertThat(extension).isNotNull
+      assertThat(extension!!.toc).isEmpty()
+    }
+
+    @Test
+    fun `given epub archive when landmarks cannot be extracted then media status is READY with comments`() {
+      val file = ClassPathResource("epub/The Incomplete Theft - Ralph Burke.epub")
+      val book = Book("book", file.url, LocalDateTime.now())
+
+      every { epubExtractor.getLandmarks(any()) } throws Exception("mock exception")
+
+      val media = bookAnalyzer.analyze(book, false)
+      val extension = media.extension as? MediaExtensionEpub
+
+      assertThat(media.mediaType).isEqualTo("application/epub+zip")
+      assertThat(media.status).isEqualTo(Media.Status.READY)
+      assertThat(media.comment).contains("ERR_1036")
+      assertThat(extension).isNotNull
+      assertThat(extension!!.landmarks).isEmpty()
+    }
+
+    @Test
+    fun `given epub archive when page list cannot be extracted then media status is READY with comments`() {
+      val file = ClassPathResource("epub/The Incomplete Theft - Ralph Burke.epub")
+      val book = Book("book", file.url, LocalDateTime.now())
+
+      every { epubExtractor.getPageList(any()) } throws Exception("mock exception")
+
+      val media = bookAnalyzer.analyze(book, false)
+      val extension = media.extension as? MediaExtensionEpub
+
+      assertThat(media.mediaType).isEqualTo("application/epub+zip")
+      assertThat(media.status).isEqualTo(Media.Status.READY)
+      assertThat(media.comment).contains("ERR_1037")
+      assertThat(extension).isNotNull
+      assertThat(extension!!.pageList).isEmpty()
+    }
+
+    @Test
+    fun `given epub archive when divina pages cannot be extracted then media status is READY with comments`() {
+      val file = ClassPathResource("epub/The Incomplete Theft - Ralph Burke.epub")
+      val book = Book("book", file.url, LocalDateTime.now())
+
+      every { epubExtractor.getDivinaPages(any(), any(), any(), any()) } throws Exception("mock exception")
+
+      val media = bookAnalyzer.analyze(book, false)
+
+      assertThat(media.mediaType).isEqualTo("application/epub+zip")
+      assertThat(media.status).isEqualTo(Media.Status.READY)
+      assertThat(media.comment).contains("ERR_1038")
+      assertThat(media.pages).isEmpty()
+    }
+
+    @Test
+    fun `given epub archive when positions cannot be extracted then media status is READY with comments`() {
+      val file = ClassPathResource("epub/The Incomplete Theft - Ralph Burke.epub")
+      val book = Book("book", file.url, LocalDateTime.now())
+
+      every { epubExtractor.computePositions(any(), any(), any(), any(), any()) } throws Exception("mock exception")
+
+      val media = bookAnalyzer.analyze(book, false)
+      val extension = media.extension as? MediaExtensionEpub
+
+      assertThat(media.mediaType).isEqualTo("application/epub+zip")
+      assertThat(media.status).isEqualTo(Media.Status.READY)
+      assertThat(media.comment).contains("ERR_1039")
+      assertThat(extension).isNotNull
+      assertThat(extension!!.positions).isEmpty()
+    }
+  }
+
+  @Nested
+  inner class PageHashing {
     @Test
     fun `given book with a single page when hashing then all pages are hashed`() {
       val book = makeBook("book1")
@@ -218,8 +322,6 @@ class BookAnalyzerTest(
       assertThat(hashes.first()).isEqualTo(hashes.last())
     }
 
-  companion object {
+    private fun provideDirectoriesForPageHashing() = ClassPathResource("hashpage").uri.toPath().listDirectoryEntries()
-    @JvmStatic
-    fun provideDirectoriesForPageHashing() = ClassPathResource("hashpage").uri.toPath().listDirectoryEntries()
   }
 }