From 394f2e2254ccd8594d9ec2d6f6fb8f6b6ee45635 Mon Sep 17 00:00:00 2001 From: Jonathan Finley Date: Fri, 13 Mar 2026 22:35:04 -0400 Subject: [PATCH] perf: bound collapsed-series browse queries --- server/controllers/LibraryController.js | 8 +- server/utils/libraryHelpers.js | 254 +----------------- .../utils/queries/libraryItemsBookFilters.js | 117 ++++++++ ...braryController.largeLibraryBrowse.test.js | 49 ++++ 4 files changed, 182 insertions(+), 246 deletions(-) diff --git a/server/controllers/LibraryController.js b/server/controllers/LibraryController.js index 8627672b0..97cf6fbd5 100644 --- a/server/controllers/LibraryController.js +++ b/server/controllers/LibraryController.js @@ -634,7 +634,13 @@ class LibraryController { const filterByValue = filterByGroup ? libraryFilters.decode(payload.filterBy.replace(`${filterByGroup}.`, '')) : null if (filterByGroup === 'series' && filterByValue !== 'no-series' && payload.collapseseries) { const seriesId = libraryFilters.decode(payload.filterBy.split('.')[1]) - payload.results = await libraryHelpers.handleCollapseSubseries(payload, seriesId, req.user, req.library) + const collapsedSeriesWindow = { + limit: Number(payload.limit) || 0, + offset: Number(payload.offset) || 0 + } + const { libraryItems, count } = await libraryItemsBookFilters.getCollapsedSeriesWindow(req.library.id, seriesId, req.user, include, payload, collapsedSeriesWindow) + payload.total = count + payload.results = await libraryHelpers.toCollapsedSeriesPayload(libraryItems, seriesId) } else { const { libraryItems, count, nextCursor, paginationMode, countMode, isCountDeferred } = await Database.libraryItemModel.getByFilterAndSort(req.library, req.user, payload) payload.results = libraryItems diff --git a/server/utils/libraryHelpers.js b/server/utils/libraryHelpers.js index 5702071e5..602aba7da 100644 --- a/server/utils/libraryHelpers.js +++ b/server/utils/libraryHelpers.js @@ -1,252 +1,16 @@ -const { createNewSortInstance } = require('../libs/fastSort') -const Database = require('../Database') -const { getTitlePrefixAtEnd, isNullOrNaN, getTitleIgnorePrefix } = require('../utils/index') -const naturalSort = createNewSortInstance({ - comparer: new Intl.Collator(undefined, { numeric: true, sensitivity: 'base' }).compare -}) - module.exports = { - /** - * - * @param {import('../models/LibraryItem')[]} libraryItems - * @param {*} filterSeries - * @param {*} hideSingleBookSeries - * @returns - */ - getSeriesFromBooks(libraryItems, filterSeries, hideSingleBookSeries) { - const _series = {} - const seriesToFilterOut = {} - libraryItems.forEach((libraryItem) => { - // get all book series for item that is not already filtered out - const allBookSeries = (libraryItem.media.series || []).filter((se) => !seriesToFilterOut[se.id]) - if (!allBookSeries.length) return - - allBookSeries.forEach((bookSeries) => { - const abJson = libraryItem.toOldJSONMinified() - abJson.sequence = bookSeries.bookSeries.sequence - if (filterSeries) { - const series = libraryItem.media.series.find((se) => se.id === filterSeries) - abJson.filterSeriesSequence = series.bookSeries.sequence - } - if (!_series[bookSeries.id]) { - _series[bookSeries.id] = { - id: bookSeries.id, - name: bookSeries.name, - nameIgnorePrefix: getTitlePrefixAtEnd(bookSeries.name), - nameIgnorePrefixSort: getTitleIgnorePrefix(bookSeries.name), - type: 'series', - books: [abJson], - totalDuration: isNullOrNaN(abJson.media.duration) ? 0 : Number(abJson.media.duration) - } - } else { - _series[bookSeries.id].books.push(abJson) - _series[bookSeries.id].totalDuration += isNullOrNaN(abJson.media.duration) ? 0 : Number(abJson.media.duration) - } - }) - }) - - let seriesItems = Object.values(_series) - - // Library setting to hide series with only 1 book - if (hideSingleBookSeries) { - seriesItems = seriesItems.filter((se) => se.books.length > 1) - } - - return seriesItems.map((series) => { - series.books = naturalSort(series.books).asc((li) => li.sequence) - return series - }) - }, - - /** - * - * @param {import('../models/LibraryItem')[]} libraryItems - * @param {string} filterSeries - series id - * @param {boolean} hideSingleBookSeries - * @returns - */ - collapseBookSeries(libraryItems, filterSeries, hideSingleBookSeries) { - // Get series from the library items. If this list is being collapsed after filtering for a series, - // don't collapse that series, only books that are in other series. - const seriesObjects = this.getSeriesFromBooks(libraryItems, filterSeries, hideSingleBookSeries).filter((s) => s.id != filterSeries) - - const filteredLibraryItems = [] - - libraryItems.forEach((li) => { - if (li.mediaType != 'book') return - - // Handle when this is the first book in a series - seriesObjects - .filter((s) => s.books[0].id == li.id) - .forEach((series) => { - // Clone the library item as we need to attach data to it, but don't - // want to change the global copy of the library item - filteredLibraryItems.push(Object.assign(Object.create(Object.getPrototypeOf(li)), li, { collapsedSeries: series })) - }) - - // Only included books not contained in series - if (!seriesObjects.some((s) => s.books.some((b) => b.id == li.id))) filteredLibraryItems.push(li) - }) - - return filteredLibraryItems - }, - - /** - * - * @param {*} payload - * @param {string} seriesId - * @param {import('../models/User')} user - * @param {import('../models/Library')} library - * @returns {Object[]} - */ - async handleCollapseSubseries(payload, seriesId, user, library) { - const seriesWithBooks = await Database.seriesModel.findByPk(seriesId, { - include: { - model: Database.bookModel, - through: { - attributes: ['sequence'] - }, - include: [ - { - model: Database.libraryItemModel - }, - { - model: Database.authorModel, - through: { - attributes: [] - } - }, - { - model: Database.seriesModel, - through: { - attributes: ['sequence'] - } - } - ] - } - }) - if (!seriesWithBooks) { - payload.total = 0 - return [] - } - - const books = seriesWithBooks.books - payload.total = books.length - - let libraryItems = books - .map((book) => { - const libraryItem = book.libraryItem - delete book.libraryItem - libraryItem.media = book - return libraryItem - }) - .filter((li) => { - return user.checkCanAccessLibraryItem(li) - }) - - const collapsedItems = this.collapseBookSeries(libraryItems, seriesId, library.settings.hideSingleBookSeries) - if (!(collapsedItems.length == 1 && collapsedItems[0].collapsedSeries)) { - libraryItems = collapsedItems - payload.total = libraryItems.length - } - - const sortingIgnorePrefix = Database.serverSettings.sortingIgnorePrefix - - let sortArray = [] - const direction = payload.sortDesc ? 'desc' : 'asc' - if (!payload.sortBy || payload.sortBy === 'sequence') { - sortArray = [ - { - [direction]: (li) => { - const series = li.media.series.find((se) => se.id === seriesId) - return series.bookSeries.sequence - } - }, - { - // If no series sequence then fallback to sorting by title (or collapsed series name for sub-series) - [direction]: (li) => { - if (sortingIgnorePrefix) { - return li.collapsedSeries?.nameIgnorePrefix || li.media.titleIgnorePrefix - } else { - return li.collapsedSeries?.name || li.media.title - } - } - } - ] - } else { - // If series are collapsed and not sorting by title or sequence, - // sort all collapsed series to the end in alphabetical order - if (payload.sortBy !== 'media.metadata.title') { - sortArray.push({ - asc: (li) => { - if (li.collapsedSeries) { - return sortingIgnorePrefix ? li.collapsedSeries.nameIgnorePrefix : li.collapsedSeries.name - } else { - return '' - } - } - }) - } - sortArray.push({ - [direction]: (li) => { - if (payload.sortBy === 'media.metadata.title') { - if (sortingIgnorePrefix) { - return li.collapsedSeries?.nameIgnorePrefix || li.media.titleIgnorePrefix - } else { - return li.collapsedSeries?.name || li.media.title - } - } else { - return payload.sortBy.split('.').reduce((a, b) => a[b], li) - } - } - }) - } - - libraryItems = naturalSort(libraryItems).by(sortArray) - - if (payload.limit) { - const startIndex = payload.page * payload.limit - libraryItems = libraryItems.slice(startIndex, startIndex + payload.limit) - } - + async toCollapsedSeriesPayload(libraryItems, seriesId) { return Promise.all( - libraryItems.map(async (li) => { - const filteredSeries = li.media.series.find((se) => se.id === seriesId) - const json = li.toOldJSONMinified() - json.media.metadata.series = { - id: filteredSeries.id, - name: filteredSeries.name, - sequence: filteredSeries.bookSeries.sequence - } + libraryItems.map(async (libraryItem) => { + const filteredSeries = libraryItem.media.series.find((series) => series.id === seriesId) + const json = libraryItem.toOldJSONMinified() - if (li.collapsedSeries) { - json.collapsedSeries = { - id: li.collapsedSeries.id, - name: li.collapsedSeries.name, - nameIgnorePrefix: li.collapsedSeries.nameIgnorePrefix, - libraryItemIds: li.collapsedSeries.books.map((b) => b.id), - numBooks: li.collapsedSeries.books.length + if (filteredSeries) { + json.media.metadata.series = { + id: filteredSeries.id, + name: filteredSeries.name, + sequence: filteredSeries.bookSeries.sequence } - - // If collapsing by series and filtering by a series, generate the list of sequences the collapsed - // series represents in the filtered series - json.collapsedSeries.seriesSequenceList = naturalSort(li.collapsedSeries.books.filter((b) => b.filterSeriesSequence).map((b) => b.filterSeriesSequence)) - .asc() - .reduce((ranges, currentSequence) => { - let lastRange = ranges.at(-1) - let isNumber = /^(\d+|\d+\.\d*|\d*\.\d+)$/.test(currentSequence) - if (isNumber) currentSequence = parseFloat(currentSequence) - - if (lastRange && isNumber && lastRange.isNumber && lastRange.end + 1 == currentSequence) { - lastRange.end = currentSequence - } else { - ranges.push({ start: currentSequence, end: currentSequence, isNumber: isNumber }) - } - - return ranges - }, []) - .map((r) => (r.start == r.end ? r.start : `${r.start}-${r.end}`)) - .join(', ') } return json diff --git a/server/utils/queries/libraryItemsBookFilters.js b/server/utils/queries/libraryItemsBookFilters.js index 66dffbec5..e76ed3438 100644 --- a/server/utils/queries/libraryItemsBookFilters.js +++ b/server/utils/queries/libraryItemsBookFilters.js @@ -158,6 +158,19 @@ async function loadBookBrowseCount({ model, countOptions }) { return model.count(countOptions) } +async function loadCollapsedSeriesWindow({ findOptions, countOptions, limit, offset }) { + const [books, count] = await Promise.all([ + Database.bookModel.findAll({ + ...findOptions, + limit: Number(limit) || null, + offset: Number(offset) || 0 + }), + Database.bookModel.count(countOptions) + ]) + + return { books, count } +} + function getNextBrowseCursor(books, limit, sortBy, sortDesc, cursorKeys) { const rowLimit = Number(limit) || 0 if (!rowLimit || books.length <= rowLimit) return null @@ -895,6 +908,110 @@ module.exports = { } }, + async getCollapsedSeriesWindow(libraryId, seriesId, user, include, payload, window = {}) { + const libraryItemIncludes = [] + if (include.includes('rssfeed')) { + libraryItemIncludes.push({ + model: Database.feedModel + }) + } + + const userPermissionBookWhere = this.getUserPermissionBookWhereQuery(user) + const direction = payload.sortDesc ? 'DESC' : 'ASC' + const sortOrder = payload.sortBy === 'sequence' + ? [ + [Sequelize.literal(`${dialectHelpers.getSafeSequenceCast(Database.getDialect(), '"bookSeries"."sequence"')} ${direction} NULLS LAST`)], + [Sequelize.literal('"libraryItem"."id"'), 'ASC'] + ] + : this.getOrder(payload.sortBy, payload.sortDesc, false) + + const findOptions = { + where: userPermissionBookWhere.bookWhere, + replacements: userPermissionBookWhere.replacements, + include: [ + { + model: Database.libraryItemModel, + required: true, + where: { + libraryId + }, + include: libraryItemIncludes + }, + { + model: Database.bookSeriesModel, + required: true, + attributes: ['id', 'seriesId', 'sequence', 'createdAt'], + where: { + seriesId + }, + include: { + model: Database.seriesModel, + attributes: ['id', 'name', 'nameIgnorePrefix'] + } + }, + { + model: Database.bookAuthorModel, + attributes: ['authorId', 'createdAt'], + include: { + model: Database.authorModel, + attributes: ['id', 'name'] + }, + order: [['createdAt', 'ASC']], + separate: true + } + ], + distinct: true, + order: sortOrder, + subQuery: false + } + + const countOptions = { + ...findOptions, + distinct: true + } + delete countOptions.order + + const { books, count } = await loadCollapsedSeriesWindow({ + findOptions, + countOptions, + limit: window.limit, + offset: window.offset + }) + + const libraryItems = books + .map((bookExpanded) => { + const libraryItem = bookExpanded.libraryItem + const book = bookExpanded + + delete book.libraryItem + + book.series = + book.bookSeries?.map((bs) => { + const series = bs.series + delete bs.series + series.bookSeries = bs + return series + }) || [] + delete book.bookSeries + + book.authors = book.bookAuthors?.map((ba) => ba.author) || [] + delete book.bookAuthors + + if (libraryItem.feeds?.length) { + libraryItem.rssFeed = libraryItem.feeds[0] + } + + libraryItem.media = book + return libraryItem + }) + .filter((libraryItem) => user?.checkCanAccessLibraryItem ? user.checkCanAccessLibraryItem(libraryItem) : true) + + return { + libraryItems, + count + } + }, + /** * Get library items for continue series shelf * A series is included on the shelf if it meets the following: diff --git a/test/server/controllers/LibraryController.largeLibraryBrowse.test.js b/test/server/controllers/LibraryController.largeLibraryBrowse.test.js index 1e4915cdc..a4e2e88aa 100644 --- a/test/server/controllers/LibraryController.largeLibraryBrowse.test.js +++ b/test/server/controllers/LibraryController.largeLibraryBrowse.test.js @@ -29,6 +29,7 @@ describe('LibraryController large-library browse contract', () => { beforeEach(async () => { global.ServerSettings = {} Database.sequelize = new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false }) + Database.serverSettings = { sortingIgnorePrefix: false } Database.sequelize.uppercaseFirst = (str) => (str ? `${str[0].toUpperCase()}${str.substr(1)}` : '') await Database.buildModels() }) @@ -272,6 +273,54 @@ describe('LibraryController large-library browse contract', () => { expect(strategy.countMode).to.equal('exact-on-initial-page') }) + it('returns only the requested collapsed-series window without loading the full series graph', async () => { + const findByPkStub = sinon.stub(Database.seriesModel, 'findByPk').resolves({ books: [] }) + const findAllStub = sinon.stub(Database.bookModel, 'findAll').resolves([]) + const req = { + query: { + filter: `series.${Buffer.from('series_1').toString('base64')}`, + collapseseries: '1', + limit: '20', + sort: 'sequence' + }, + library: { id: 'lib_1', mediaType: 'book', isVirtual: false, settings: {} }, + user: { id: 'user_1', checkCanAccessLibraryItem: () => true } + } + const res = { json: sinon.spy() } + + await LibraryController.getLibraryItems(req, res) + + expect(findByPkStub.called).to.equal(false) + expect(findAllStub.calledOnce).to.equal(true) + expect(findAllStub.firstCall.args[0].limit).to.equal(20) + expect(findAllStub.firstCall.args[0].offset || 0).to.equal(0) + }) + + it('applies the requested collapsed-series follow-up window in SQL query options', async () => { + const findByPkStub = sinon.stub(Database.seriesModel, 'findByPk').resolves({ books: [] }) + const findAllStub = sinon.stub(Database.bookModel, 'findAll').resolves([]) + const req = { + query: { + filter: `series.${Buffer.from('series_1').toString('base64')}`, + collapseseries: '1', + limit: '20', + page: '1', + sort: 'sequence', + pageMode: 'paged' + }, + library: { id: 'lib_1', mediaType: 'book', isVirtual: false, settings: {} }, + user: { id: 'user_1', checkCanAccessLibraryItem: () => true } + } + const res = { json: sinon.spy() } + + await LibraryController.getLibraryItems(req, res) + + expect(findByPkStub.called).to.equal(false) + expect(findAllStub.calledOnce).to.equal(true) + expect(findAllStub.firstCall.args[0].limit).to.equal(20) + expect(findAllStub.firstCall.args[0].offset).to.equal(20) + }) + it('adds a deterministic libraryItem.id tie-breaker for keyset title browse with duplicate titles', async () => { global.ServerSettings = { sortingIgnorePrefix: true }