From bcfb203f74e16fcc9df53e69720680a7cb328eb0 Mon Sep 17 00:00:00 2001 From: Gauthier Roebroeck Date: Wed, 6 Jan 2021 15:09:39 +0800 Subject: [PATCH] feat(api): find previous/next book in readlist this also works for users with limited access to libraries --- .../komga/infrastructure/jooq/BookDtoDao.kt | 50 ++++++++++- .../interfaces/rest/ReadListController.kt | 68 ++++++++++++-- .../rest/persistence/BookDtoRepository.kt | 20 +++++ .../interfaces/rest/ReadListControllerTest.kt | 88 +++++++++++++++++++ 4 files changed, 215 insertions(+), 11 deletions(-) diff --git a/komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/BookDtoDao.kt b/komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/BookDtoDao.kt index 7d0a73350..cb43f5024 100644 --- a/komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/BookDtoDao.kt +++ b/komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/BookDtoDao.kt @@ -121,9 +121,28 @@ class BookDtoDao( .fetchAndMap() .firstOrNull() - override fun findPreviousInSeries(bookId: String, userId: String): BookDto? = findSibling(bookId, userId, next = false) + override fun findPreviousInSeries(bookId: String, userId: String): BookDto? = + findSiblingSeries(bookId, userId, next = false) - override fun findNextInSeries(bookId: String, userId: String): BookDto? = findSibling(bookId, userId, next = true) + override fun findNextInSeries(bookId: String, userId: String): BookDto? = + findSiblingSeries(bookId, userId, next = true) + + + override fun findPreviousInReadList( + readListId: String, + bookId: String, + userId: String, + filterOnLibraryIds: Collection? + ): BookDto? = + findSiblingReadList(readListId, bookId, userId, filterOnLibraryIds, next = false) + + override fun findNextInReadList( + readListId: String, + bookId: String, + userId: String, + filterOnLibraryIds: Collection? + ): BookDto? = + findSiblingReadList(readListId, bookId, userId, filterOnLibraryIds, next = true) override fun findOnDeck(libraryIds: Collection, userId: String, pageable: Pageable): Page { @@ -163,7 +182,7 @@ class BookDtoDao( private fun readProgressCondition(userId: String): Condition = r.USER_ID.eq(userId).or(r.USER_ID.isNull) - private fun findSibling(bookId: String, userId: String, next: Boolean): BookDto? { + private fun findSiblingSeries(bookId: String, userId: String, next: Boolean): BookDto? { val record = dsl.select(b.SERIES_ID, d.NUMBER_SORT) .from(b) .leftJoin(d).on(b.ID.eq(d.BOOK_ID)) @@ -181,6 +200,31 @@ class BookDtoDao( .firstOrNull() } + private fun findSiblingReadList( + readListId: String, + bookId: String, + userId: String, + filterOnLibraryIds: Collection?, + next: Boolean + ): BookDto? { + val numberSort = dsl.select(rlb.NUMBER) + .from(b) + .leftJoin(rlb).on(b.ID.eq(rlb.BOOK_ID)) + .where(b.ID.eq(bookId)) + .and(rlb.READLIST_ID.eq(readListId)) + .apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } } + .fetchOne(0, Int::class.java) + + return selectBase(userId, JoinConditions(selectReadListNumber = true)) + .where(rlb.READLIST_ID.eq(readListId)) + .apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } } + .orderBy(rlb.NUMBER.let { if (next) it.asc() else it.desc() }) + .seek(numberSort) + .limit(1) + .fetchAndMap() + .firstOrNull() + } + private fun selectBase(userId: String, joinConditions: JoinConditions = JoinConditions()) = dsl.selectDistinct( *b.fields(), diff --git a/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/ReadListController.kt b/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/ReadListController.kt index ee6d0e10f..9a7d2b6a8 100644 --- a/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/ReadListController.kt +++ b/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/ReadListController.kt @@ -73,10 +73,28 @@ class ReadListController( ) return when { - principal.user.sharedAllLibraries && libraryIds == null -> readListRepository.findAll(searchTerm, pageable = pageRequest) - principal.user.sharedAllLibraries && libraryIds != null -> readListRepository.findAllByLibraries(libraryIds, null, searchTerm, pageable = pageRequest) - !principal.user.sharedAllLibraries && libraryIds != null -> readListRepository.findAllByLibraries(libraryIds, principal.user.sharedLibrariesIds, searchTerm, pageable = pageRequest) - else -> readListRepository.findAllByLibraries(principal.user.sharedLibrariesIds, principal.user.sharedLibrariesIds, searchTerm, pageable = pageRequest) + principal.user.sharedAllLibraries && libraryIds == null -> readListRepository.findAll( + searchTerm, + pageable = pageRequest + ) + principal.user.sharedAllLibraries && libraryIds != null -> readListRepository.findAllByLibraries( + libraryIds, + null, + searchTerm, + pageable = pageRequest + ) + !principal.user.sharedAllLibraries && libraryIds != null -> readListRepository.findAllByLibraries( + libraryIds, + principal.user.sharedLibrariesIds, + searchTerm, + pageable = pageRequest + ) + else -> readListRepository.findAllByLibraries( + principal.user.sharedLibrariesIds, + principal.user.sharedLibrariesIds, + searchTerm, + pageable = pageRequest + ) }.map { it.toDto() } } @@ -108,10 +126,12 @@ class ReadListController( @Valid @RequestBody readList: ReadListCreationDto ): ReadListDto = try { - readListLifecycle.addReadList(ReadList( - name = readList.name, - bookIds = readList.bookIds.toIndexedMap() - )).toDto() + readListLifecycle.addReadList( + ReadList( + name = readList.name, + bookIds = readList.bookIds.toIndexedMap() + ) + ).toDto() } catch (e: DuplicateNameException) { throw ResponseStatusException(HttpStatus.BAD_REQUEST, e.message) } @@ -174,5 +194,37 @@ class ReadListController( ) .map { it.restrictUrl(!principal.user.roleAdmin) } } ?: throw ResponseStatusException(HttpStatus.NOT_FOUND) + + @GetMapping("{id}/books/{bookId}/previous") + fun getBookSiblingPrevious( + @AuthenticationPrincipal principal: KomgaPrincipal, + @PathVariable id: String, + @PathVariable bookId: String + ): BookDto = + readListRepository.findByIdOrNull(id, principal.user.getAuthorizedLibraryIds(null))?.let { readList -> + bookDtoRepository.findPreviousInReadList( + id, + bookId, + principal.user.id, + principal.user.getAuthorizedLibraryIds(null) + ) + ?.restrictUrl(!principal.user.roleAdmin) + } ?: throw ResponseStatusException(HttpStatus.NOT_FOUND) + + @GetMapping("{id}/books/{bookId}/next") + fun getBookSiblingNext( + @AuthenticationPrincipal principal: KomgaPrincipal, + @PathVariable id: String, + @PathVariable bookId: String + ): BookDto = + readListRepository.findByIdOrNull(id, principal.user.getAuthorizedLibraryIds(null))?.let { readList -> + bookDtoRepository.findNextInReadList( + id, + bookId, + principal.user.id, + principal.user.getAuthorizedLibraryIds(null) + ) + ?.restrictUrl(!principal.user.roleAdmin) + } ?: throw ResponseStatusException(HttpStatus.NOT_FOUND) } diff --git a/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/persistence/BookDtoRepository.kt b/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/persistence/BookDtoRepository.kt index 1427a1192..d626375ab 100644 --- a/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/persistence/BookDtoRepository.kt +++ b/komga/src/main/kotlin/org/gotson/komga/interfaces/rest/persistence/BookDtoRepository.kt @@ -7,6 +7,10 @@ import org.springframework.data.domain.Pageable interface BookDtoRepository { fun findAll(search: BookSearchWithReadProgress, userId: String, pageable: Pageable): Page + + /** + * Find books that are part of a readlist, optionally filtered by library + */ fun findByReadListId( readListId: String, userId: String, @@ -15,7 +19,23 @@ interface BookDtoRepository { ): Page fun findByIdOrNull(bookId: String, userId: String): BookDto? + fun findPreviousInSeries(bookId: String, userId: String): BookDto? fun findNextInSeries(bookId: String, userId: String): BookDto? + + fun findPreviousInReadList( + readListId: String, + bookId: String, + userId: String, + filterOnLibraryIds: Collection? + ): BookDto? + + fun findNextInReadList( + readListId: String, + bookId: String, + userId: String, + filterOnLibraryIds: Collection? + ): BookDto? + fun findOnDeck(libraryIds: Collection, userId: String, pageable: Pageable): Page } diff --git a/komga/src/test/kotlin/org/gotson/komga/interfaces/rest/ReadListControllerTest.kt b/komga/src/test/kotlin/org/gotson/komga/interfaces/rest/ReadListControllerTest.kt index f8139ae6c..d7ac54d14 100644 --- a/komga/src/test/kotlin/org/gotson/komga/interfaces/rest/ReadListControllerTest.kt +++ b/komga/src/test/kotlin/org/gotson/komga/interfaces/rest/ReadListControllerTest.kt @@ -202,6 +202,94 @@ class ReadListControllerTest( } } + @Nested + inner class Siblings { + @Test + @WithMockCustomUser + fun `given user with access to all libraries when getting book siblings then it is returned or not found`() { + makeReadLists() + + val first = booksLibrary1.first().id // Book_1 + val second = booksLibrary1[1].id // Book_2 + val last = booksLibrary2.last().id // Book_10 + + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${first}/previous") + .andExpect { status { isNotFound() } } + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${first}/next") + .andExpect { + status { isOk() } + jsonPath("$.name") { value("Book_2") } + } + + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${second}/previous") + .andExpect { + status { isOk() } + jsonPath("$.name") { value("Book_1") } + } + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${second}/next") + .andExpect { + status { isOk() } + jsonPath("$.name") { value("Book_3") } + } + + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${last}/previous") + .andExpect { + status { isOk() } + jsonPath("$.name") { value("Book_9") } + } + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${last}/next") + .andExpect { status { isNotFound() } } + } + + @Test + @WithMockCustomUser(sharedAllLibraries = false, sharedLibraries = ["1"]) + fun `given user with access to a single library when getting book siblings then it takes into account the library filter`() { + makeReadLists() + + val first = booksLibrary1.first().id // Book_1 + val second = booksLibrary1[1].id // Book_2 + val last = booksLibrary1.last().id // Book_5 + + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${first}/previous") + .andExpect { status { isNotFound() } } + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${first}/next") + .andExpect { + status { isOk() } + jsonPath("$.name") { value("Book_2") } + } + + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${second}/previous") + .andExpect { + status { isOk() } + jsonPath("$.name") { value("Book_1") } + } + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${second}/next") + .andExpect { + status { isOk() } + jsonPath("$.name") { value("Book_3") } + } + + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${last}/previous") + .andExpect { + status { isOk() } + jsonPath("$.name") { value("Book_4") } + } + mockMvc.get("/api/v1/readlists/${rlLibBoth.id}/books/${last}/next") + .andExpect { status { isNotFound() } } + } + + @Test + @WithMockCustomUser(sharedAllLibraries = false, sharedLibraries = ["1"]) + fun `given user with access to a single library when getting books from single read list from another library then return not found`() { + makeReadLists() + + mockMvc.get("/api/v1/readlists/${rlLib2.id}/books") + .andExpect { + status { isNotFound() } + } + } + } + @Nested inner class Creation { @Test