Windy/komga
1
0
Fork 0
mirror of https://github.com/gotson/komga.git synced 2025-01-07 03:07:16 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat(api): readlist books are not always sorted by number

Browse Source 
Closes: #1803
This commit is contained in:
Gauthier Roebroeck 2025-01-02 13:27:19 +08:00
parent 1552b9b0c4
commit 0dd4b270ef
9 changed files with 110 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/BookSearchHelper.kt
View File

@ -58,18 +58,23 @@ class BookSearchHelper(
is SearchCondition.SeriesId -> searchCondition.operator.toCondition(Tables.BOOK.SERIES_ID) to emptySet()
is SearchCondition.ReadListId ->
Tables.BOOK.ID.let { field ->
val inner = { readListId: String ->
DSL
.select(Tables.READLIST_BOOK.BOOK_ID)
.from(Tables.READLIST_BOOK)
.where(Tables.READLIST_BOOK.READLIST_ID.eq(readListId))
when (searchCondition.operator) {
// for IS condition we have to do a join, so as to order the books by readList number
is SearchOperator.Is ->
Tables.READLIST_BOOK
.`as`("RLB_${searchCondition.operator.value}")
.READLIST_ID
.eq(searchCondition.operator.value) to setOf(RequiredJoin.ReadList(searchCondition.operator.value))
is SearchOperator.IsNot -> {
val inner = { readListId: String ->
DSL
.select(Tables.READLIST_BOOK.BOOK_ID)
.from(Tables.READLIST_BOOK)
.where(Tables.READLIST_BOOK.READLIST_ID.eq(readListId))
}
Tables.BOOK.ID.notIn(inner(searchCondition.operator.value)) to emptySet()
}
when (searchCondition.operator) {
is SearchOperator.Is -> field.`in`(inner(searchCondition.operator.value))
is SearchOperator.IsNot -> field.notIn(inner(searchCondition.operator.value))
}
} to emptySet()
}
is SearchCondition.Title ->
searchCondition.operator.toCondition(Tables.BOOK_METADATA.TITLE) to

4
komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/RequiredJoin.kt
View File

@ -12,6 +12,10 @@ sealed class RequiredJoin {
val userId: String,
) : RequiredJoin()
data class ReadList(
val readListId: String,
) : RequiredJoin()
data object BookMetadataAggregation : RequiredJoin()
data object SeriesMetadata : RequiredJoin()

2
komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/Utils.kt
View File

@ -147,3 +147,5 @@ fun ObjectMapper.deserializeMediaExtension(
null
}
}
fun rlbAlias(readListId: String) = Tables.READLIST_BOOK.`as`("RLB_$readListId")

9
komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/main/BookDao.kt
View File

@ -7,6 +7,7 @@ import org.gotson.komga.domain.persistence.BookRepository
import org.gotson.komga.infrastructure.jooq.BookSearchHelper
import org.gotson.komga.infrastructure.jooq.RequiredJoin
import org.gotson.komga.infrastructure.jooq.insertTempStrings
import org.gotson.komga.infrastructure.jooq.rlbAlias
import org.gotson.komga.infrastructure.jooq.selectTempStrings
import org.gotson.komga.infrastructure.jooq.toOrderBy
import org.gotson.komga.jooq.main.Tables
@ -140,6 +141,10 @@ class BookDao(
RequiredJoin.SeriesMetadata -> innerJoin(sd).on(b.SERIES_ID.eq(sd.SERIES_ID))
RequiredJoin.Media -> innerJoin(m).on(b.ID.eq(m.BOOK_ID))
is RequiredJoin.ReadProgress -> leftJoin(r).on(b.ID.eq(r.BOOK_ID)).and(r.USER_ID.eq(join.userId))
is RequiredJoin.ReadList -> {
val rlbAlias = rlbAlias(join.readListId)
leftJoin(rlbAlias).on(rlbAlias.BOOK_ID.eq(b.ID).and(rlbAlias.READLIST_ID.eq(join.readListId)))
}
// shouldn't be required for books
RequiredJoin.BookMetadataAggregation -> Unit
}
@ -160,6 +165,10 @@ class BookDao(
RequiredJoin.SeriesMetadata -> innerJoin(sd).on(b.SERIES_ID.eq(sd.SERIES_ID))
RequiredJoin.Media -> innerJoin(m).on(b.ID.eq(m.BOOK_ID))
is RequiredJoin.ReadProgress -> leftJoin(r).on(b.ID.eq(r.BOOK_ID)).and(r.USER_ID.eq(join.userId))
is RequiredJoin.ReadList -> {
val rlbAlias = rlbAlias(join.readListId)
leftJoin(rlbAlias).on(rlbAlias.BOOK_ID.eq(b.ID).and(rlbAlias.READLIST_ID.eq(join.readListId)))
}
// shouldn't be required for books
RequiredJoin.BookMetadataAggregation -> Unit
}

33
komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/main/BookDtoDao.kt
View File

@ -9,6 +9,7 @@ import org.gotson.komga.infrastructure.jooq.BookSearchHelper
import org.gotson.komga.infrastructure.jooq.RequiredJoin
import org.gotson.komga.infrastructure.jooq.insertTempStrings
import org.gotson.komga.infrastructure.jooq.noCase
import org.gotson.komga.infrastructure.jooq.rlbAlias
import org.gotson.komga.infrastructure.jooq.selectTempStrings
import org.gotson.komga.infrastructure.jooq.sortByValues
import org.gotson.komga.infrastructure.jooq.toCondition
@ -128,10 +129,17 @@ class BookDtoDao(
val orderBy =
pageable.sort.mapNotNull {
if (it.property == "relevance" && !bookIds.isNullOrEmpty())
if (it.property == "relevance" && !bookIds.isNullOrEmpty()) {
b.ID.sortByValues(bookIds, it.isAscending)
else
it.toSortField(sorts)
} else {
if (it.property == "readList.number") {
val readListId = joins.filterIsInstance<RequiredJoin.ReadList>().firstOrNull()?.readListId ?: return@mapNotNull null
val f = rlb.`as`("RLB_$readListId").NUMBER
if (it.isAscending) f.asc() else f.desc()
} else {
it.toSortField(sorts)
}
}
}
val (count, dtos) =
@ -156,6 +164,10 @@ class BookDtoDao(
.apply {
joins.forEach { join ->
when (join) {
is RequiredJoin.ReadList -> {
val rlbAlias = rlbAlias(join.readListId)
leftJoin(rlbAlias).on(rlbAlias.BOOK_ID.eq(b.ID).and(rlbAlias.READLIST_ID.eq(join.readListId)))
}
// always joined
RequiredJoin.BookMetadata -> Unit
RequiredJoin.Media -> Unit
@ -171,7 +183,7 @@ class BookDtoDao(
)
val dtos =
selectBase(userId, joins, pageable.sort.any { it.property == "readList.number" })
selectBase(userId, joins)
.where(conditions)
.and(searchCondition)
.orderBy(orderBy)
@ -334,11 +346,10 @@ class BookDtoDao(
.apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } }
.fetchOne(rlb.NUMBER)
return selectBase(userId, joinOnReadList = true)
.where(rlb.READLIST_ID.eq(readList.id))
return selectBase(userId, setOf(RequiredJoin.ReadList(readList.id)))
.apply { if (restrictions.isRestricted) and(restrictions.toCondition()) }
.apply { filterOnLibraryIds?.let { and(b.LIBRARY_ID.`in`(it)) } }
.orderBy(rlb.NUMBER.let { if (next) it.asc() else it.desc() })
.orderBy(rlbAlias(readList.id).NUMBER.let { if (next) it.asc() else it.desc() })
.seek(numberSort)
.limit(1)
.fetchAndMap()
@ -377,7 +388,6 @@ class BookDtoDao(
private fun selectBase(
userId: String,
joins: Set<RequiredJoin> = emptySet(),
joinOnReadList: Boolean = false,
): SelectOnConditionStep<Record> {
val selectFields =
listOf(
@ -389,7 +399,7 @@ class BookDtoDao(
)
return dsl
.let { if (joinOnReadList) it.selectDistinct(selectFields) else it.select(selectFields) }
.select(selectFields)
.from(b)
.leftJoin(m)
.on(b.ID.eq(m.BOOK_ID))
@ -401,9 +411,12 @@ class BookDtoDao(
.leftJoin(sd)
.on(b.SERIES_ID.eq(sd.SERIES_ID))
.apply {
if (joinOnReadList) leftJoin(rlb).on(b.ID.eq(rlb.BOOK_ID))
joins.forEach { join ->
when (join) {
is RequiredJoin.ReadList -> {
val rlbAlias = rlbAlias(join.readListId)
leftJoin(rlbAlias).on(rlbAlias.BOOK_ID.eq(b.ID).and(rlbAlias.READLIST_ID.eq(join.readListId)))
}
// always joined
RequiredJoin.BookMetadata -> Unit
RequiredJoin.Media -> Unit

1
komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/main/SeriesDao.kt
View File

@ -137,6 +137,7 @@ class SeriesDao(
// Book joins - not needed
RequiredJoin.BookMetadata -> Unit
RequiredJoin.Media -> Unit
is RequiredJoin.ReadList -> Unit
}
}
}.where(conditions)

3
komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/main/SeriesDtoDao.kt
View File

@ -163,6 +163,7 @@ class SeriesDtoDao(
RequiredJoin.Media -> Unit
RequiredJoin.BookMetadata -> Unit
RequiredJoin.BookMetadataAggregation -> Unit
is RequiredJoin.ReadList -> Unit
}
}
}.where(conditionsRefined)
@ -209,6 +210,7 @@ class SeriesDtoDao(
RequiredJoin.BookMetadata -> Unit
RequiredJoin.BookMetadataAggregation -> Unit
RequiredJoin.Media -> Unit
is RequiredJoin.ReadList -> Unit
}
}
}
@ -244,6 +246,7 @@ class SeriesDtoDao(
RequiredJoin.BookMetadata -> Unit
RequiredJoin.BookMetadataAggregation -> Unit
RequiredJoin.Media -> Unit
is RequiredJoin.ReadList -> Unit
}
}
}.where(conditions)

2
komga/src/main/kotlin/org/gotson/komga/infrastructure/jooq/main/SyncPointDao.kt
View File

@ -100,6 +100,8 @@ class SyncPointDao(
.apply {
joins.forEach {
when (it) {
// for future work
is RequiredJoin.ReadList -> Unit
// we don't have to handle those since we already join on those tables anyway, the 'when' is here for future proofing
RequiredJoin.BookMetadata -> Unit
RequiredJoin.SeriesMetadata -> Unit

50
komga/src/test/kotlin/org/gotson/komga/infrastructure/jooq/main/BookSearchTest.kt
View File

@ -189,6 +189,56 @@ class BookSearchTest(
}
}
@Test
fun `given some books in multiple read lists when searching by read list then results are accurate`() {
val book1 = makeBook("1", libraryId = library1.id, seriesId = series1.id)
val book2 = makeBook("2", libraryId = library2.id, seriesId = series2.id)
seriesLifecycle.addBooks(series1, listOf(book1))
seriesLifecycle.addBooks(series2, listOf(book2))
val readList1 = ReadList("rl1", bookIds = mapOf(1 to book1.id, 2 to book2.id).toSortedMap())
val readList2 = ReadList("rl2", bookIds = mapOf(1 to book2.id, 2 to book1.id).toSortedMap())
readListRepository.insert(readList1)
readListRepository.insert(readList2)
// search by readList 1
run {
val search = BookSearch(SearchCondition.ReadListId(SearchOperator.Is(readList1.id)))
val found = bookDao.findAll(search.condition, SearchContext(user1), UnpagedSorted(Sort.by(Sort.Order.asc("readList.number")))).content
val foundDto = bookDtoDao.findAll(search, SearchContext(user1), UnpagedSorted(Sort.by(Sort.Order.asc("readList.number")))).content
// order not guaranteed for bookDao
assertThat(found.map { it.name }).containsExactlyInAnyOrder("1", "2")
assertThat(foundDto.map { it.name }).containsExactly("1", "2")
}
// search by readList 2
run {
val search = BookSearch(SearchCondition.ReadListId(SearchOperator.Is(readList2.id)))
val found = bookDao.findAll(search.condition, SearchContext(user1), UnpagedSorted(Sort.by(Sort.Order.asc("readList.number")))).content
val foundDto = bookDtoDao.findAll(search, SearchContext(user1), UnpagedSorted(Sort.by(Sort.Order.asc("readList.number")))).content
// order not guaranteed for bookDao
assertThat(found.map { it.name }).containsExactlyInAnyOrder("2", "1")
assertThat(foundDto.map { it.name }).containsExactly("2", "1")
}
// search by readList 1 or 2 - order is not guaranteed in that case
run {
val search =
BookSearch(
SearchCondition.AnyOfBook(
SearchCondition.ReadListId(SearchOperator.Is(readList1.id)),
SearchCondition.ReadListId(SearchOperator.Is(readList2.id)),
),
)
val found = bookDao.findAll(search.condition, SearchContext(user1), UnpagedSorted(Sort.by(Sort.Order.asc("readList.number")))).content
val foundDto = bookDtoDao.findAll(search, SearchContext(user1), UnpagedSorted(Sort.by(Sort.Order.asc("readList.number")))).content
assertThat(found.map { it.name }).containsExactlyInAnyOrder("2", "1")
assertThat(foundDto.map { it.name }).containsExactlyInAnyOrder("2", "1")
}
}
@Test
fun `given some books when searching by deleted then results are accurate`() {
val book1 = makeBook("1", libraryId = library1.id, seriesId = series1.id).copy(deletedDate = LocalDateTime.now())