diff --git a/server/utils/queries/libraryBrowseStrategy.js b/server/utils/queries/libraryBrowseStrategy.js index 141e54a48..91e26eb23 100644 --- a/server/utils/queries/libraryBrowseStrategy.js +++ b/server/utils/queries/libraryBrowseStrategy.js @@ -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) } } diff --git a/server/utils/queries/libraryItemsBookFilters.js b/server/utils/queries/libraryItemsBookFilters.js index 3fa2ff26e..22e99c228 100644 --- a/server/utils/queries/libraryItemsBookFilters.js +++ b/server/utils/queries/libraryItemsBookFilters.js @@ -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 [] }, diff --git a/test/server/controllers/LibraryController.largeLibraryBrowse.test.js b/test/server/controllers/LibraryController.largeLibraryBrowse.test.js index c4338ca95..337db5e2d 100644 --- a/test/server/controllers/LibraryController.largeLibraryBrowse.test.js +++ b/test/server/controllers/LibraryController.largeLibraryBrowse.test.js @@ -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) + }) }) diff --git a/test/server/utils/libraryBrowseStrategy.test.js b/test/server/utils/libraryBrowseStrategy.test.js index c8a218a8f..bf47355ed 100644 --- a/test/server/utils/libraryBrowseStrategy.test.js +++ b/test/server/utils/libraryBrowseStrategy.test.js @@ -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',