diff --git a/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/BookController.kt b/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/BookController.kt index 549e2be5..1142bc57 100644 --- a/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/BookController.kt +++ b/komga/src/main/kotlin/org/gotson/komga/interfaces/api/rest/BookController.kt @@ -79,6 +79,7 @@ import org.springframework.http.MediaType import org.springframework.http.ResponseEntity import org.springframework.security.access.prepost.PreAuthorize import org.springframework.security.core.annotation.AuthenticationPrincipal +import org.springframework.util.MimeTypeUtils import org.springframework.web.bind.annotation.DeleteMapping import org.springframework.web.bind.annotation.GetMapping import org.springframework.web.bind.annotation.PatchMapping @@ -86,6 +87,7 @@ import org.springframework.web.bind.annotation.PathVariable import org.springframework.web.bind.annotation.PostMapping import org.springframework.web.bind.annotation.PutMapping import org.springframework.web.bind.annotation.RequestBody +import org.springframework.web.bind.annotation.RequestHeader import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RequestParam import org.springframework.web.bind.annotation.ResponseStatus @@ -479,6 +481,9 @@ class BookController( @Parameter(description = "If set to true, pages will start at index 0. If set to false, pages will start at index 1.") @RequestParam(value = "zero_based", defaultValue = "false") zeroBasedIndex: Boolean, + @Parameter(description = "Some very limited server driven content negotiation is handled. If a book is a PDF book, and the Accept header contains 'application/pdf' as a more specific type than other 'image/' types, a raw PDF page will be returned.") + @RequestHeader(HttpHeaders.ACCEPT, required = false) + acceptHeaders: MutableList?, ): ResponseEntity = bookRepository.findByIdOrNull((bookId))?.let { book -> val media = mediaRepository.findById(bookId) @@ -491,6 +496,14 @@ class BookController( principal.user.checkContentRestriction(book) + if (media.mediaType == PDF.type && acceptHeaders != null && acceptHeaders.any { it.isCompatibleWith(MediaType.APPLICATION_PDF) }) { + // keep only pdf and image + acceptHeaders.removeIf { !it.isCompatibleWith(MediaType.APPLICATION_PDF) && !it.isCompatibleWith(MediaType("image")) } + MimeTypeUtils.sortBySpecificity(acceptHeaders) + if (acceptHeaders.first().isCompatibleWith(MediaType.APPLICATION_PDF)) + return getBookPageRaw(book, media, pageNumber) + } + try { val convertFormat = when (convertTo?.lowercase()) { "jpeg" -> ImageType.JPEG @@ -550,34 +563,36 @@ class BookController( principal.user.checkContentRestriction(book) - try { - val pageContent = bookAnalyzer.getPageContentRaw(BookWithMedia(book, media), pageNumber) - - ResponseEntity.ok() - .headers( - HttpHeaders().apply { - val extension = contentDetector.mediaTypeToExtension(pageContent.mediaType) ?: "" - val pageFileName = "${book.name}-$pageNumber$extension" - contentDisposition = ContentDisposition.builder("inline") - .filename(pageFileName, UTF_8) - .build() - }, - ) - .contentType(getMediaTypeOrDefault(pageContent.mediaType)) - .setNotModified(media) - .body(pageContent.content) - } catch (ex: IndexOutOfBoundsException) { - throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Page number does not exist") - } catch (ex: MediaUnsupportedException) { - throw ResponseStatusException(HttpStatus.BAD_REQUEST, ex.message) - } catch (ex: MediaNotReadyException) { - throw ResponseStatusException(HttpStatus.NOT_FOUND, "Book analysis failed") - } catch (ex: NoSuchFileException) { - logger.warn(ex) { "File not found: $book" } - throw ResponseStatusException(HttpStatus.NOT_FOUND, "File not found, it may have moved") - } + getBookPageRaw(book, media, pageNumber) } ?: throw ResponseStatusException(HttpStatus.NOT_FOUND) + private fun getBookPageRaw(book: Book, media: Media, pageNumber: Int): ResponseEntity = try { + val pageContent = bookAnalyzer.getPageContentRaw(BookWithMedia(book, media), pageNumber) + + ResponseEntity.ok() + .headers( + HttpHeaders().apply { + val extension = contentDetector.mediaTypeToExtension(pageContent.mediaType) ?: "" + val pageFileName = "${book.name}-$pageNumber$extension" + contentDisposition = ContentDisposition.builder("inline") + .filename(pageFileName, UTF_8) + .build() + }, + ) + .contentType(getMediaTypeOrDefault(pageContent.mediaType)) + .setNotModified(media) + .body(pageContent.content) + } catch (ex: IndexOutOfBoundsException) { + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Page number does not exist") + } catch (ex: MediaUnsupportedException) { + throw ResponseStatusException(HttpStatus.BAD_REQUEST, ex.message) + } catch (ex: MediaNotReadyException) { + throw ResponseStatusException(HttpStatus.NOT_FOUND, "Book analysis failed") + } catch (ex: NoSuchFileException) { + logger.warn(ex) { "File not found: $book" } + throw ResponseStatusException(HttpStatus.NOT_FOUND, "File not found, it may have moved") + } + @ApiResponse(content = [Content(schema = Schema(type = "string", format = "binary"))]) @GetMapping( value = ["api/v1/books/{bookId}/pages/{pageNumber}/thumbnail"], @@ -647,7 +662,7 @@ class BookController( @PathVariable bookId: String, ): ResponseEntity = bookDtoRepository.findByIdOrNull(bookId, principal.user.id)?.let { bookDto -> - if (bookDto.media.mediaType != KomgaMediaType.PDF.type) throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Book media type '${bookDto.media.mediaType}' not compatible with requested profile") + if (bookDto.media.mediaType != PDF.type) throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Book media type '${bookDto.media.mediaType}' not compatible with requested profile") principal.user.checkContentRestriction(bookDto) val manifest = bookDto.toManifestPdf( mediaRepository.findById(bookDto.id), diff --git a/komga/src/test/kotlin/org/gotson/komga/interfaces/api/rest/BookControllerPageTest.kt b/komga/src/test/kotlin/org/gotson/komga/interfaces/api/rest/BookControllerPageTest.kt new file mode 100644 index 00000000..67316add --- /dev/null +++ b/komga/src/test/kotlin/org/gotson/komga/interfaces/api/rest/BookControllerPageTest.kt @@ -0,0 +1,176 @@ +package org.gotson.komga.interfaces.api.rest + +import com.ninjasquad.springmockk.MockkBean +import com.ninjasquad.springmockk.SpykBean +import io.mockk.every +import org.gotson.komga.domain.model.BookPage +import org.gotson.komga.domain.model.BookPageContent +import org.gotson.komga.domain.model.Media +import org.gotson.komga.domain.model.makeBook +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.SeriesRepository +import org.gotson.komga.domain.service.BookAnalyzer +import org.gotson.komga.domain.service.BookLifecycle +import org.gotson.komga.domain.service.KomgaUserLifecycle +import org.gotson.komga.domain.service.LibraryLifecycle +import org.gotson.komga.domain.service.SeriesLifecycle +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.Arguments +import org.junit.jupiter.params.provider.MethodSource +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.http.HttpHeaders +import org.springframework.http.MediaType +import org.springframework.test.web.servlet.MockMvc +import org.springframework.test.web.servlet.get +import java.util.stream.Stream + +@SpringBootTest +@AutoConfigureMockMvc(printOnlyOnFailure = false) +class BookControllerPageTest( + @Autowired private val seriesRepository: SeriesRepository, + @Autowired private val seriesLifecycle: SeriesLifecycle, + @Autowired private val mediaRepository: MediaRepository, + @Autowired private val libraryRepository: LibraryRepository, + @Autowired private val libraryLifecycle: LibraryLifecycle, + @Autowired private val bookRepository: BookRepository, + @Autowired private val userRepository: KomgaUserRepository, + @Autowired private val userLifecycle: KomgaUserLifecycle, + @Autowired private val mockMvc: MockMvc, +) { + @MockkBean + private lateinit var mockAnalyzer: BookAnalyzer + + @SpykBean + private lateinit var bookLifecycle: BookLifecycle + + private val library = makeLibrary(id = "1") + + @BeforeAll + fun `setup library`() { + libraryRepository.insert(library) + } + + @AfterAll + fun teardown() { + libraryRepository.findAll().forEach { + libraryLifecycle.deleteLibrary(it) + } + } + + @AfterEach + fun `clear repository`() { + seriesLifecycle.deleteMany(seriesRepository.findAll()) + } + + @ParameterizedTest + @MethodSource("arguments") + @WithMockCustomUser + fun `given pdf book when getting page with Accept header then returns page in correct format`(bookType: String, acceptTypes: List, success: Boolean, resultType: String?) { + 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 { + mediaRepository.update( + it.copy( + status = Media.Status.READY, + mediaType = bookType, + pages = listOf(BookPage("file", "image/jpeg")), + ), + ) + } + + every { mockAnalyzer.getPageContentRaw(any(), 1) } returns BookPageContent(ByteArray(0), "application/pdf") + every { bookLifecycle.getBookPage(any(), 1, any(), any()) } returns BookPageContent(ByteArray(0), "image/jpeg") + + mockMvc.get("/api/v1/books/${book.id}/pages/1") { + if (acceptTypes.isNotEmpty()) accept(*acceptTypes.toTypedArray()) + } + .andExpect { + status { if (success) isOk() else isBadRequest() } + if (resultType != null) + header { string(HttpHeaders.CONTENT_TYPE, resultType) } + } + } + + fun arguments(): Stream = + Stream.of( + // PDF book: request nothing, get image + Arguments.of( + "application/pdf", + emptyList(), + true, + "image/jpeg", + ), + // PDF book: request PDF, get PDF + Arguments.of( + "application/pdf", + listOf(MediaType.APPLICATION_PDF), + true, + "application/pdf", + ), + // PDF book: request PDF + others, get PDF + Arguments.of( + "application/pdf", + listOf(MediaType.APPLICATION_PDF, MediaType("image")), + true, + "application/pdf", + ), + // PDF book: request image, get image + Arguments.of( + "application/pdf", + listOf(MediaType("image")), + true, + "image/jpeg", + ), + // PDF book: request unhandled image, still get image. We don't check image subtypes. + Arguments.of( + "application/pdf", + listOf(MediaType("image", "avif")), + true, + "image/jpeg", + ), + // PDF book: request random, get image. We ignore non-pdf types. + Arguments.of( + "application/pdf", + listOf(MediaType.APPLICATION_ATOM_XML), + true, + "image/jpeg", + ), + // PDF book: request PDF with lower priority than image, get image + Arguments.of( + "application/pdf", + listOf(MediaType.IMAGE_JPEG, MediaType.APPLICATION_PDF), + true, + "image/jpeg", + ), + // PDF book: request PDF with lower quality than image, get image + Arguments.of( + "application/pdf", + listOf(MediaType("application", "pdf", 0.5), MediaType.IMAGE_JPEG), + true, + "image/jpeg", + ), + // PDF book: request PDF with higher quality than image, get pdf + Arguments.of( + "application/pdf", + listOf(MediaType("image", "jpeg", 0.5), MediaType("application", "pdf", 0.8)), + true, + "application/pdf", + ), + ) +}