mirror of
https://github.com/gotson/komga.git
synced 2026-01-20 07:03:00 +01:00
fix: sibling next/prev ordering for duplicate numberSort
Backgound
`/api/v1/books/{bookId}/next` used only `number_sort` for ordering and keyset seek.
When multiple books shared the same `number_sort`, the next/previous book could be unstable and diverge from `On Deck` selection, which uses number_sort + book_id as a tie-breaker.
Reason
`findSiblingSeries` ordered and seeked solely by number_sort, leaving ties without deterministic ordering.
Fix
Align sibling ordering with `On Deck` by adding `book_id` as a secondary sort key and seek parameter. This keeps endpoint semantics while making ties stable.
Tests
Add a sibling test with duplicate numberSort to assert stable next/previous results.
This commit is contained in:
parent
e6560e0c81
commit
afef41d4f9
2 changed files with 53 additions and 2 deletions
|
|
@ -323,11 +323,18 @@ class BookDtoDao(
|
|||
val seriesId = record.get(0, String::class.java)
|
||||
val numberSort = record.get(1, Float::class.java)
|
||||
|
||||
val orderBy =
|
||||
if (next) {
|
||||
listOf(d.NUMBER_SORT.asc(), b.ID.asc())
|
||||
} else {
|
||||
listOf(d.NUMBER_SORT.desc(), b.ID.desc())
|
||||
}
|
||||
|
||||
return dslRO
|
||||
.selectBase(userId)
|
||||
.where(b.SERIES_ID.eq(seriesId))
|
||||
.orderBy(d.NUMBER_SORT.let { if (next) it.asc() else it.desc() })
|
||||
.seek(numberSort)
|
||||
.orderBy(orderBy)
|
||||
.seek(numberSort, bookId)
|
||||
.limit(1)
|
||||
.fetchAndMap(dslRO)
|
||||
.firstOrNull()
|
||||
|
|
|
|||
|
|
@ -727,6 +727,50 @@ class BookControllerTest(
|
|||
.get("/api/v1/books/${book3.id}/next")
|
||||
.andExpect { status { isNotFound() } }
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithMockCustomUser
|
||||
fun `given series with duplicate numberSort when getting siblings then order is stable`() {
|
||||
val book1 = makeBook("1", libraryId = library.id, id = "1")
|
||||
val book2 = makeBook("2", libraryId = library.id, id = "2")
|
||||
val book3 = makeBook("3", libraryId = library.id, id = "3")
|
||||
makeSeries(name = "series", libraryId = library.id).let { series ->
|
||||
seriesLifecycle.createSeries(series).also { created ->
|
||||
val books = listOf(book1, book2, book3)
|
||||
seriesLifecycle.addBooks(created, books)
|
||||
seriesLifecycle.sortBooks(created)
|
||||
}
|
||||
}
|
||||
|
||||
bookMetadataRepository.findById(book1.id).let { metadata ->
|
||||
bookMetadataRepository.update(metadata.copy(numberSort = 1F))
|
||||
}
|
||||
bookMetadataRepository.findById(book2.id).let { metadata ->
|
||||
bookMetadataRepository.update(metadata.copy(numberSort = 1F))
|
||||
}
|
||||
bookMetadataRepository.findById(book3.id).let { metadata ->
|
||||
bookMetadataRepository.update(metadata.copy(numberSort = 2F))
|
||||
}
|
||||
|
||||
mockMvc
|
||||
.get("/api/v1/books/${book1.id}/next")
|
||||
.andExpect {
|
||||
status { isOk() }
|
||||
jsonPath("$.name") { value("2") }
|
||||
}
|
||||
mockMvc
|
||||
.get("/api/v1/books/${book2.id}/previous")
|
||||
.andExpect {
|
||||
status { isOk() }
|
||||
jsonPath("$.name") { value("1") }
|
||||
}
|
||||
mockMvc
|
||||
.get("/api/v1/books/${book2.id}/next")
|
||||
.andExpect {
|
||||
status { isOk() }
|
||||
jsonPath("$.name") { value("3") }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Nested
|
||||
|
|
|
|||
Loading…
Reference in a new issue