fix: tighten browse keyset fallback rules

This commit is contained in:
Jonathan Finley 2026-03-13 21:49:30 -04:00
parent b701b6efb6
commit 2d13ec3f8c
4 changed files with 132 additions and 30 deletions

View file

@ -2,7 +2,11 @@ function getTitleCursorKey() {
return global.ServerSettings && global.ServerSettings.sortingIgnorePrefix ? 'titleIgnorePrefix' : 'title'
}
function getKeysetCursorKeys(sortBy) {
function getKeysetCursorKeys(sortBy, collapseseries) {
if (sortBy === 'media.metadata.title' && collapseseries) {
return []
}
if (sortBy === 'media.metadata.title') {
return [getTitleCursorKey(), 'id']
}
@ -23,27 +27,15 @@ function getKeysetCursorKeys(sortBy) {
return ['updatedAt', 'id']
}
if (sortBy === 'progress') {
return ['mediaProgresses.updatedAt', 'id']
}
if (sortBy === 'progress.createdAt') {
return ['mediaProgresses.createdAt', 'id']
}
if (sortBy === 'progress.finishedAt') {
return ['mediaProgresses.finishedAt', 'id']
}
return []
}
function getPaginationMode(pageMode, sortBy) {
function getPaginationMode(pageMode, sortBy, collapseseries) {
if (pageMode !== 'endless') {
return 'offset'
}
return getKeysetCursorKeys(sortBy).length ? 'keyset' : 'offset'
return getKeysetCursorKeys(sortBy, collapseseries).length ? 'keyset' : 'offset'
}
function getFamily({ mediaType, sortBy, filterGroup, collapseseries }) {
@ -60,7 +52,7 @@ function getFamily({ mediaType, sortBy, filterGroup, collapseseries }) {
function getLibraryBrowseStrategy({ mediaType, sortBy, filterGroup, pageMode, collapseseries } = {}) {
const normalizedSort = sortBy || 'media.metadata.title'
const paginationMode = getPaginationMode(pageMode, normalizedSort)
const paginationMode = getPaginationMode(pageMode, normalizedSort, collapseseries)
if (normalizedSort === 'random') {
return {
@ -79,7 +71,7 @@ function getLibraryBrowseStrategy({ mediaType, sortBy, filterGroup, pageMode, co
countMode: paginationMode === 'keyset' ? 'deferred-exact' : 'exact-on-initial-page',
deepScrollAllowed: paginationMode === 'keyset',
tieBreaker: 'id',
cursorKeys: getKeysetCursorKeys(normalizedSort)
cursorKeys: getKeysetCursorKeys(normalizedSort, collapseseries)
}
}

View file

@ -353,6 +353,8 @@ module.exports = {
getOrder(sortBy, sortDesc, collapseseries) {
const dir = sortDesc ? 'DESC' : 'ASC'
const libraryItemIdOrder = [Sequelize.literal('libraryItem.id'), dir]
const getTitleOrder = () => {
if (global.ServerSettings.sortingIgnorePrefix) {
return [Sequelize.literal('`libraryItem`.`titleIgnorePrefix` COLLATE NOCASE'), dir]
@ -362,7 +364,7 @@ module.exports = {
}
if (sortBy === 'addedAt') {
return [[Sequelize.literal('libraryItem.createdAt'), dir]]
return [[Sequelize.literal('libraryItem.createdAt'), dir], libraryItemIdOrder]
} else if (sortBy === 'size') {
return [[Sequelize.literal('libraryItem.size'), dir]]
} else if (sortBy === 'birthtimeMs') {
@ -375,15 +377,15 @@ module.exports = {
return [[Sequelize.literal(`CAST(\`book\`.\`publishedYear\` AS INTEGER)`), dir]]
} else if (sortBy === 'media.metadata.authorNameLF') {
// Sort by author name last first, secondary sort by title
return [[Sequelize.literal('`libraryItem`.`authorNamesLastFirst` COLLATE NOCASE'), dir], getTitleOrder()]
return [[Sequelize.literal('`libraryItem`.`authorNamesLastFirst` COLLATE NOCASE'), dir], getTitleOrder(), libraryItemIdOrder]
} else if (sortBy === 'media.metadata.authorName') {
// Sort by author name first last, secondary sort by title
return [[Sequelize.literal('`libraryItem`.`authorNamesFirstLast` COLLATE NOCASE'), dir], getTitleOrder()]
return [[Sequelize.literal('`libraryItem`.`authorNamesFirstLast` COLLATE NOCASE'), dir], getTitleOrder(), libraryItemIdOrder]
} else if (sortBy === 'media.metadata.title') {
if (collapseseries) {
return [[Sequelize.literal('display_title COLLATE NOCASE'), dir]]
}
return [getTitleOrder()]
return [getTitleOrder(), libraryItemIdOrder]
} else if (sortBy === 'sequence') {
const nullDir = sortDesc ? 'DESC NULLS FIRST' : 'ASC NULLS LAST'
return [[Sequelize.literal(`CAST(\`series.bookSeries.sequence\` AS FLOAT) ${nullDir}`)]]
@ -395,6 +397,8 @@ module.exports = {
return [[Sequelize.literal(`mediaProgresses.finishedAt ${dir} NULLS LAST`)]]
} else if (sortBy === 'random') {
return [Database.sequelize.random()]
} else if (sortBy === 'updatedAt') {
return [[Sequelize.literal('libraryItem.updatedAt'), dir], libraryItemIdOrder]
}
return []
},

View file

@ -5,6 +5,7 @@ const sinon = require('sinon')
const Database = require('../../../server/Database')
const LibraryController = require('../../../server/controllers/LibraryController')
const libraryFilters = require('../../../server/utils/queries/libraryFilters')
const { getLibraryBrowseStrategy } = require('../../../server/utils/queries/libraryBrowseStrategy')
const libraryItemsBookFilters = require('../../../server/utils/queries/libraryItemsBookFilters')
const libraryItemsPodcastFilters = require('../../../server/utils/queries/libraryItemsPodcastFilters')
@ -226,4 +227,93 @@ describe('LibraryController large-library browse contract', () => {
expect(countStub.called).to.equal(false)
expect(findAndCountAllStub.called).to.equal(false)
})
it('falls back to offset mode for endless progress browse because null-aware keyset is unsupported', () => {
const strategy = getLibraryBrowseStrategy({
mediaType: 'book',
sortBy: 'progress',
filterGroup: 'progress',
pageMode: 'endless'
})
expect(strategy.paginationMode).to.equal('offset')
expect(strategy.countMode).to.equal('exact-on-initial-page')
expect(strategy.deepScrollAllowed).to.equal(false)
})
it('falls back to offset mode for collapsed-series endless title browse because the SQL sort key differs', () => {
global.ServerSettings = { sortingIgnorePrefix: true }
const strategy = getLibraryBrowseStrategy({
mediaType: 'book',
sortBy: 'media.metadata.title',
filterGroup: 'series',
pageMode: 'endless',
collapseseries: true
})
expect(strategy.paginationMode).to.equal('offset')
expect(strategy.countMode).to.equal('exact-on-initial-page')
})
it('adds a deterministic libraryItem.id tie-breaker for keyset title browse with duplicate titles', async () => {
global.ServerSettings = { sortingIgnorePrefix: true }
const countStub = sinon.stub(Database.bookModel, 'count').resolves(123)
const findAndCountAllStub = sinon.stub(Database.bookModel, 'findAndCountAll').resolves({ rows: [], count: 123 })
const findAllStub = sinon.stub(Database.bookModel, 'findAll').resolves([
{ id: 'book-1', title: 'Alpha', libraryItem: { id: 'item-1', titleIgnorePrefix: 'Alpha', dataValues: { titleIgnorePrefix: 'Alpha' } } },
{ id: 'book-2', title: 'Alpha', libraryItem: { id: 'item-2', titleIgnorePrefix: 'Alpha', dataValues: { titleIgnorePrefix: 'Alpha' } } },
{ id: 'book-3', title: 'Beta', libraryItem: { id: 'item-3', titleIgnorePrefix: 'Beta', dataValues: { titleIgnorePrefix: 'Beta' } } }
])
const result = await libraryItemsBookFilters.getFilteredLibraryItems(
'lib_1',
{ id: 'user_1', canAccessExplicitContent: true, accessAllTags: true },
null,
null,
'media.metadata.title',
false,
false,
[],
2,
0,
false,
{ pageMode: 'endless' }
)
expect(result.paginationMode).to.equal('keyset')
expect(result.nextCursor).to.be.a('string')
expect(findAllStub.firstCall.args[0].order[1][0].val || findAllStub.firstCall.args[0].order[1][0]).to.include('libraryItem')
expect(findAllStub.firstCall.args[0].order[1][0].val || findAllStub.firstCall.args[0].order[1][0]).to.include('id')
expect(countStub.calledOnce).to.equal(true)
expect(findAndCountAllStub.called).to.equal(false)
})
it('uses offset fallback for progress browse requests with null-valued sort fields', async () => {
const countStub = sinon.stub(Database.bookModel, 'count').resolves(123)
const findAllStub = sinon.stub(Database.bookModel, 'findAll').resolves([])
const findAndCountAllStub = sinon.stub(Database.bookModel, 'findAndCountAll').resolves({ rows: [], count: 7 })
const result = await libraryItemsBookFilters.getFilteredLibraryItems(
'lib_1',
{ id: 'user_1', canAccessExplicitContent: true, accessAllTags: true },
'progress',
'in-progress',
'progress',
true,
false,
[],
20,
0,
false,
{ cursor: 'cursor-2', pageMode: 'endless' }
)
expect(result.paginationMode).to.equal('offset')
expect(result.countMode).to.equal('exact-on-initial-page')
expect(findAndCountAllStub.calledOnce).to.equal(true)
expect(findAllStub.called).to.equal(false)
expect(countStub.called).to.equal(false)
})
})

View file

@ -64,7 +64,7 @@ describe('libraryBrowseStrategy', () => {
expect(strategy.cursorKeys).to.deep.equal(['createdAt', 'id'])
})
it('uses keyset pagination for progress browse', () => {
it('falls back to offset pagination for progress browse because null-aware keyset is unsupported', () => {
const strategy = getLibraryBrowseStrategy({
mediaType: 'book',
sortBy: 'progress',
@ -73,8 +73,8 @@ describe('libraryBrowseStrategy', () => {
})
expect(strategy.family).to.equal('progress-browse')
expect(strategy.paginationMode).to.equal('keyset')
expect(strategy.cursorKeys).to.deep.equal(['mediaProgresses.updatedAt', 'id'])
expect(strategy.paginationMode).to.equal('offset')
expect(strategy.cursorKeys).to.deep.equal([])
})
it('uses keyset pagination for updated browse', () => {
@ -120,7 +120,7 @@ describe('libraryBrowseStrategy', () => {
expect(strategy.cursorKeys).to.deep.equal(['authorNamesLastFirst', 'title', 'id'])
})
it('uses keyset pagination for progress created-at browse', () => {
it('falls back to offset pagination for progress created-at browse because null-aware keyset is unsupported', () => {
const strategy = getLibraryBrowseStrategy({
mediaType: 'book',
sortBy: 'progress.createdAt',
@ -129,11 +129,11 @@ describe('libraryBrowseStrategy', () => {
})
expect(strategy.family).to.equal('progress-browse')
expect(strategy.paginationMode).to.equal('keyset')
expect(strategy.cursorKeys).to.deep.equal(['mediaProgresses.createdAt', 'id'])
expect(strategy.paginationMode).to.equal('offset')
expect(strategy.cursorKeys).to.deep.equal([])
})
it('uses keyset pagination for progress finished-at browse', () => {
it('falls back to offset pagination for progress finished-at browse because null-aware keyset is unsupported', () => {
const strategy = getLibraryBrowseStrategy({
mediaType: 'book',
sortBy: 'progress.finishedAt',
@ -142,8 +142,8 @@ describe('libraryBrowseStrategy', () => {
})
expect(strategy.family).to.equal('progress-browse')
expect(strategy.paginationMode).to.equal('keyset')
expect(strategy.cursorKeys).to.deep.equal(['mediaProgresses.finishedAt', 'id'])
expect(strategy.paginationMode).to.equal('offset')
expect(strategy.cursorKeys).to.deep.equal([])
})
it('defines a filtered browse family for generic library filters', () => {
@ -180,6 +180,22 @@ describe('libraryBrowseStrategy', () => {
expect(strategy.family).to.equal('collapsed-series-browse')
})
it('falls back to offset for collapsed-series endless title browse', () => {
global.ServerSettings = { sortingIgnorePrefix: true }
const strategy = getLibraryBrowseStrategy({
mediaType: 'book',
sortBy: 'media.metadata.title',
filterGroup: 'series',
pageMode: 'endless',
collapseseries: true
})
expect(strategy.family).to.equal('collapsed-series-browse')
expect(strategy.paginationMode).to.equal('offset')
expect(strategy.cursorKeys).to.deep.equal([])
})
it('defines a podcast browse family', () => {
const strategy = getLibraryBrowseStrategy({
mediaType: 'podcast',