diff --git a/server/controllers/LibraryController.js b/server/controllers/LibraryController.js index 9b9a63760..33f80bc74 100644 --- a/server/controllers/LibraryController.js +++ b/server/controllers/LibraryController.js @@ -638,6 +638,7 @@ class LibraryController { limit: Number(payload.limit) || 0, offset: Number(payload.offset) || 0 } + payload.hideSingleBookSeries = !!req.library.settings.hideSingleBookSeries 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, req.library.settings.hideSingleBookSeries) diff --git a/server/utils/libraryHelpers.js b/server/utils/libraryHelpers.js index 1baee56e6..41ffa405c 100644 --- a/server/utils/libraryHelpers.js +++ b/server/utils/libraryHelpers.js @@ -91,8 +91,12 @@ function getSeriesSequenceList(collapsedSeries) { module.exports = { async toCollapsedSeriesPayload(libraryItems, seriesId, hideSingleBookSeries = false) { - const collapsedItems = collapseBookSeries(libraryItems, seriesId, hideSingleBookSeries) - const shapedItems = !(collapsedItems.length === 1 && collapsedItems[0].collapsedSeries) ? collapsedItems : libraryItems + const shapedItems = libraryItems.some((libraryItem) => libraryItem.collapsedSeries) + ? libraryItems + : (() => { + const collapsedItems = collapseBookSeries(libraryItems, seriesId, hideSingleBookSeries) + return !(collapsedItems.length === 1 && collapsedItems[0].collapsedSeries) ? collapsedItems : libraryItems + })() return Promise.all( shapedItems.map(async (libraryItem) => { diff --git a/server/utils/queries/libraryItemsBookFilters.js b/server/utils/queries/libraryItemsBookFilters.js index accd9f419..353b029e3 100644 --- a/server/utils/queries/libraryItemsBookFilters.js +++ b/server/utils/queries/libraryItemsBookFilters.js @@ -165,12 +165,88 @@ async function loadCollapsedSeriesWindow({ findOptions, countOptions, limit, off limit: Number(limit) || null, offset: Number(offset) || 0 }), - Database.bookModel.count(countOptions) + countOptions ? Database.bookModel.count(countOptions) : Promise.resolve(null) ]) return { books, count } } +function mapCollapsedSeriesBookToLibraryItem(bookExpanded) { + const libraryItem = bookExpanded.libraryItem + const book = bookExpanded + + delete book.libraryItem + book.series = book.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 +} + +function buildCollapsedSeriesView(libraryItems, seriesId, hideSingleBookSeries = false) { + const seriesEntriesById = {} + + libraryItems.forEach((libraryItem) => { + const filteredSeries = libraryItem.media.series.find((series) => series.id === seriesId) + const subseries = (libraryItem.media.series || []).filter((series) => series.id !== seriesId) + + subseries.forEach((series) => { + if (!seriesEntriesById[series.id]) { + seriesEntriesById[series.id] = { + id: series.id, + name: series.name, + nameIgnorePrefix: series.nameIgnorePrefix, + books: [] + } + } + + seriesEntriesById[series.id].books.push({ + id: libraryItem.id, + filterSeriesSequence: filteredSeries?.bookSeries?.sequence + }) + }) + }) + + let seriesEntries = Object.values(seriesEntriesById) + if (hideSingleBookSeries) { + seriesEntries = seriesEntries.filter((series) => series.books.length > 1) + } + seriesEntries.forEach((series) => { + seriesEntriesById[series.id] = series + }) + + const activeSeriesEntries = seriesEntries + const visibleRows = [] + + libraryItems.forEach((libraryItem) => { + const firstSeriesEntries = activeSeriesEntries.filter((series) => series.books[0]?.id === libraryItem.id) + firstSeriesEntries.forEach((series) => { + visibleRows.push({ type: 'collapsed', libraryItemId: libraryItem.id, seriesId: series.id }) + }) + + const isCollapsedBook = activeSeriesEntries.some((series) => series.books.some((book) => book.id === libraryItem.id)) + if (!isCollapsedBook) { + visibleRows.push({ type: 'plain', libraryItemId: libraryItem.id }) + } + }) + + if (visibleRows.length === 1 && visibleRows[0].type === 'collapsed') { + return { + visibleRows: libraryItems.map((libraryItem) => ({ type: 'plain', libraryItemId: libraryItem.id })), + seriesEntriesById: {} + } + } + + return { visibleRows, seriesEntriesById } +} + function getNextBrowseCursor(books, limit, sortBy, sortDesc, cursorKeys) { const rowLimit = Number(limit) || 0 if (!rowLimit || books.length <= rowLimit) return null @@ -973,62 +1049,52 @@ module.exports = { subQuery: false } - const countOptions = { - where: userPermissionBookWhere.bookWhere, - replacements: userPermissionBookWhere.replacements, - include: [ - { - model: Database.libraryItemModel, - required: true, - where: { - libraryId - }, - include: libraryItemIncludes - }, - { - model: Database.bookSeriesModel, - required: true, - where: { - seriesId - } - } - ], - distinct: true, - subQuery: false + const batchLimit = Math.max(Number(window.limit) || 0, 1) + const allLibraryItems = [] + let rawOffset = 0 + + while (true) { + const { books } = await loadCollapsedSeriesWindow({ + findOptions, + countOptions: null, + limit: batchLimit, + offset: rawOffset + }) + + if (!books.length) break + + allLibraryItems.push( + ...books + .map((bookExpanded) => mapCollapsedSeriesBookToLibraryItem(bookExpanded)) + .filter((libraryItem) => user?.checkCanAccessLibraryItem ? user.checkCanAccessLibraryItem(libraryItem) : true) + ) + + rawOffset += books.length + if (books.length < batchLimit) break } - const { books, count } = await loadCollapsedSeriesWindow({ - findOptions, - countOptions, - limit: window.limit, - offset: window.offset - }) + const { visibleRows, seriesEntriesById } = buildCollapsedSeriesView(allLibraryItems, seriesId, payload.hideSingleBookSeries) + const visibleOffset = Number(window.offset) || 0 + const visibleLimit = Number(window.limit) || 0 + const selectedRows = visibleLimit ? visibleRows.slice(visibleOffset, visibleOffset + visibleLimit) : visibleRows + const libraryItemsById = new Map(allLibraryItems.map((libraryItem) => [libraryItem.id, libraryItem])) - const libraryItems = books - .map((bookExpanded) => { - const libraryItem = bookExpanded.libraryItem - const book = bookExpanded - - delete book.libraryItem - - book.series = book.series || [] - delete book.bookSeries - - book.authors = book.bookAuthors?.map((ba) => ba.author) || [] - delete book.bookAuthors - - if (libraryItem.feeds?.length) { - libraryItem.rssFeed = libraryItem.feeds[0] + const libraryItems = selectedRows + .map((row) => { + const libraryItem = libraryItemsById.get(row.libraryItemId) + if (!libraryItem) return null + if (row.type === 'collapsed') { + return Object.assign(Object.create(Object.getPrototypeOf(libraryItem)), libraryItem, { + collapsedSeries: seriesEntriesById[row.seriesId] + }) } - - libraryItem.media = book return libraryItem }) - .filter((libraryItem) => user?.checkCanAccessLibraryItem ? user.checkCanAccessLibraryItem(libraryItem) : true) + .filter(Boolean) return { libraryItems, - count + count: visibleRows.length } }, diff --git a/test/server/controllers/LibraryController.largeLibraryBrowse.test.js b/test/server/controllers/LibraryController.largeLibraryBrowse.test.js index d04b78b44..b41ea8e3f 100644 --- a/test/server/controllers/LibraryController.largeLibraryBrowse.test.js +++ b/test/server/controllers/LibraryController.largeLibraryBrowse.test.js @@ -297,7 +297,7 @@ describe('LibraryController large-library browse contract', () => { expect(findAllStub.firstCall.args[0].offset || 0).to.equal(0) }) - it('applies the requested collapsed-series follow-up window in SQL query options', async () => { + it('uses bounded batch query options for collapsed-series follow-up windows', async () => { const findByPkStub = sinon.stub(Database.seriesModel, 'findByPk').resolves({ books: [] }) const findAllStub = sinon.stub(Database.bookModel, 'findAll').resolves([]) const req = { @@ -319,7 +319,7 @@ describe('LibraryController large-library browse contract', () => { 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) + expect(findAllStub.firstCall.args[0].offset).to.equal(0) }) it('restores collapsed-series payload fields and preserves rssfeed serialization', async () => { @@ -384,6 +384,112 @@ describe('LibraryController large-library browse contract', () => { expect(findAllStub.firstCall.args[0].order[0][0].val || findAllStub.firstCall.args[0].order[0][0]).to.include('sequence') }) + it('paginates collapsed-series browse by visible rows instead of raw books', async () => { + const makeBook = ({ libraryItemId, filterSequence, subseriesId = null, subseriesName = null }) => ({ + libraryItem: { + id: libraryItemId, + mediaType: 'book', + toOldJSONMinified() { + return { id: libraryItemId, media: { metadata: {}, duration: 1 } } + } + }, + series: [ + { id: 'series_1', name: 'Main Series', nameIgnorePrefix: 'Main Series', bookSeries: { sequence: filterSequence } }, + ...(subseriesId ? [{ id: subseriesId, name: subseriesName, nameIgnorePrefix: subseriesName, bookSeries: { sequence: filterSequence } }] : []) + ], + bookAuthors: [] + }) + + const findAllStub = sinon.stub(Database.bookModel, 'findAll') + sinon.stub(Database.bookModel, 'count').resolves(3) + findAllStub.onCall(0).resolves([ + makeBook({ libraryItemId: 'li_1', filterSequence: '1', subseriesId: 'sub_1', subseriesName: 'Subseries 1' }) + ]) + findAllStub.onCall(1).resolves([ + makeBook({ libraryItemId: 'li_2', filterSequence: '2', subseriesId: 'sub_1', subseriesName: 'Subseries 1' }) + ]) + findAllStub.onCall(2).resolves([ + makeBook({ libraryItemId: 'li_3', filterSequence: '3' }) + ]) + findAllStub.onCall(3).resolves([]) + + const req = { + query: { + filter: `series.${Buffer.from('series_1').toString('base64')}`, + collapseseries: '1', + limit: '1', + page: '1', + 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) + + const response = res.json.firstCall.args[0] + expect(response.total).to.equal(2) + expect(response.results).to.have.length(1) + expect(response.results[0].id).to.equal('li_3') + expect(response.results[0].collapsedSeries).to.equal(undefined) + }) + + it('keeps a sub-series collapsed on the first visible page instead of splitting it across raw-book windows', async () => { + const makeBook = ({ libraryItemId, filterSequence, subseriesId = null, subseriesName = null }) => ({ + libraryItem: { + id: libraryItemId, + mediaType: 'book', + toOldJSONMinified() { + return { id: libraryItemId, media: { metadata: {}, duration: 1 } } + } + }, + series: [ + { id: 'series_1', name: 'Main Series', nameIgnorePrefix: 'Main Series', bookSeries: { sequence: filterSequence } }, + ...(subseriesId ? [{ id: subseriesId, name: subseriesName, nameIgnorePrefix: subseriesName, bookSeries: { sequence: filterSequence } }] : []) + ], + bookAuthors: [] + }) + + const findAllStub = sinon.stub(Database.bookModel, 'findAll') + sinon.stub(Database.bookModel, 'count').resolves(3) + findAllStub.onCall(0).resolves([ + makeBook({ libraryItemId: 'li_1', filterSequence: '1', subseriesId: 'sub_1', subseriesName: 'Subseries 1' }) + ]) + findAllStub.onCall(1).resolves([ + makeBook({ libraryItemId: 'li_2', filterSequence: '2', subseriesId: 'sub_1', subseriesName: 'Subseries 1' }) + ]) + findAllStub.onCall(2).resolves([ + makeBook({ libraryItemId: 'li_3', filterSequence: '3' }) + ]) + findAllStub.onCall(3).resolves([]) + + const req = { + query: { + filter: `series.${Buffer.from('series_1').toString('base64')}`, + collapseseries: '1', + limit: '1', + 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) + + const response = res.json.firstCall.args[0] + expect(response.total).to.equal(2) + expect(response.results).to.have.length(1) + expect(response.results[0].collapsedSeries).to.include({ + id: 'sub_1', + name: 'Subseries 1', + numBooks: 2 + }) + expect(response.results[0].collapsedSeries.libraryItemIds).to.deep.equal(['li_1', 'li_2']) + expect(response.results[0].collapsedSeries.seriesSequenceList).to.equal('1-2') + }) + it('adds a deterministic libraryItem.id tie-breaker for keyset title browse with duplicate titles', async () => { global.ServerSettings = { sortingIgnorePrefix: true }