diff --git a/server/src/common/utils/primary-file-selection.utils.test.ts b/server/src/common/utils/primary-file-selection.utils.test.ts index 6446a40f5..fe7b439b7 100644 --- a/server/src/common/utils/primary-file-selection.utils.test.ts +++ b/server/src/common/utils/primary-file-selection.utils.test.ts @@ -1,6 +1,6 @@ import { READ_ALONG_FORMAT_PRIORITY, withoutImplicitReadAlongFormatPriority, withReadAlongFormatPriority } from '@bookorbit/types'; -import { rankFileRowsByBook, rankFilesByFormatPriority, selectPrimaryFile } from './primary-file-selection.utils'; +import { rankFileRowsByBook, rankFilesByFormatPriority, selectPrimaryFile, selectPrimaryFileKeepingCurrent } from './primary-file-selection.utils'; describe('selectPrimaryFile', () => { const plainEpub = { id: 1, format: 'epub', sizeBytes: 100, mediaOverlayAvailable: false }; @@ -55,6 +55,36 @@ describe('selectPrimaryFile', () => { }); }); +describe('selectPrimaryFileKeepingCurrent', () => { + const firstEpub = { id: 1, format: 'epub', sizeBytes: 100, mediaOverlayAvailable: false }; + const secondEpub = { id: 2, format: 'epub', sizeBytes: 100, mediaOverlayAvailable: false }; + const audiobook = { id: 3, format: 'm4b', sizeBytes: 300, mediaOverlayAvailable: false }; + + it('keeps the current primary over a file that only ties with it', () => { + expect(selectPrimaryFile([firstEpub, secondEpub], ['epub'])).toBe(firstEpub); + expect(selectPrimaryFileKeepingCurrent([firstEpub, secondEpub], secondEpub.id, ['epub'])).toBe(secondEpub); + }); + + it('replaces the current primary with a better-ranked format', () => { + expect(selectPrimaryFileKeepingCurrent([firstEpub, audiobook], firstEpub.id, ['m4b', 'epub'])).toBe(audiobook); + }); + + it('still lets a read-along EPUB outrank a plain current one of the same format', () => { + const readAlong = { ...secondEpub, mediaOverlayAvailable: true }; + expect(selectPrimaryFileKeepingCurrent([firstEpub, readAlong], firstEpub.id, ['epub'])).toBe(readAlong); + }); + + it('ranks normally when there is no current primary, or it is not among the files', () => { + expect(selectPrimaryFileKeepingCurrent([firstEpub, secondEpub], null, ['epub'])).toBe(firstEpub); + expect(selectPrimaryFileKeepingCurrent([firstEpub, secondEpub], 99, ['epub'])).toBe(firstEpub); + }); + + it('does not keep an empty current primary', () => { + const emptyCurrent = { ...secondEpub, sizeBytes: 0 }; + expect(selectPrimaryFileKeepingCurrent([firstEpub, emptyCurrent], emptyCurrent.id, ['epub'])).toBe(firstEpub); + }); +}); + describe('read-along format priority helpers', () => { it('shows the read-along entry directly before epub when a stored list lacks it', () => { expect(withReadAlongFormatPriority(['m4b', 'epub', 'pdf'])).toEqual(['m4b', READ_ALONG_FORMAT_PRIORITY, 'epub', 'pdf']); diff --git a/server/src/common/utils/primary-file-selection.utils.ts b/server/src/common/utils/primary-file-selection.utils.ts index fa2f2df5a..a04290088 100644 --- a/server/src/common/utils/primary-file-selection.utils.ts +++ b/server/src/common/utils/primary-file-selection.utils.ts @@ -41,6 +41,22 @@ export function selectPrimaryFile( return entryPool.find((file) => file.mediaOverlayAvailable === true) ?? first; } +/** + * {@link selectPrimaryFile} for a book that already has a primary: a file that only ties with it, + * such as a second EPUB beside the first, does not take its place. Only a better-ranked file does. + */ +export function selectPrimaryFileKeepingCurrent( + files: readonly T[], + currentPrimaryFileId: number | null, + formatPriority: readonly string[], +): T | null { + const ordered = + currentPrimaryFileId == null + ? files + : [...files.filter((file) => file.id === currentPrimaryFileId), ...files.filter((file) => file.id !== currentPrimaryFileId)]; + return selectPrimaryFile(ordered, formatPriority); +} + /** * A book's files in edition order: the primary first, then content files by the library's format * priority (a read-along EPUB ranked apart from a plain one), then covers and sidecars. The sort is diff --git a/server/src/modules/upload/upload-processor.service.test.ts b/server/src/modules/upload/upload-processor.service.test.ts index 4f35ecdd9..efa6194b1 100644 --- a/server/src/modules/upload/upload-processor.service.test.ts +++ b/server/src/modules/upload/upload-processor.service.test.ts @@ -2,6 +2,8 @@ vi.mock('fs/promises', () => ({ stat: vi.fn() })); vi.mock('../scanner/lib/hash', () => ({ computeFileHash: vi.fn() })); import { InternalServerErrorException } from '@nestjs/common'; +import { PgDialect } from 'drizzle-orm/pg-core'; +import type { SQL } from 'drizzle-orm'; import { stat } from 'fs/promises'; import { computeFileHash } from '../scanner/lib/hash'; import { books, bookFiles, bookMetadata, uploadSessions } from '../../db/schema'; @@ -34,8 +36,11 @@ describe('UploadProcessorService', () => { const updateSessionWhere = vi.fn(); const selectFrom = vi.fn(); + const selectInnerJoin = vi.fn(); const selectWhere = vi.fn(); + const selectFor = vi.fn(); const selectLimit = vi.fn(); + const selectOrderBy = vi.fn(); const deleteWhere = vi.fn(); const tx = { @@ -72,9 +77,12 @@ describe('UploadProcessorService', () => { beforeEach(() => { vi.resetAllMocks(); - selectFrom.mockReturnValue({ where: selectWhere }); - selectWhere.mockReturnValue({ limit: selectLimit }); + selectFrom.mockReturnValue({ where: selectWhere, innerJoin: selectInnerJoin }); + selectInnerJoin.mockReturnValue({ where: selectWhere }); + selectWhere.mockReturnValue({ limit: selectLimit, for: selectFor, orderBy: selectOrderBy }); + selectFor.mockReturnValue({ limit: selectLimit }); selectLimit.mockResolvedValue([]); // no existing book by default + selectOrderBy.mockResolvedValue([]); // no content files to rank by default insertBooksValues.mockReturnValue({ returning: insertBooksReturning }); insertBooksReturning.mockResolvedValue([{ id: 42 }]); @@ -371,7 +379,7 @@ describe('UploadProcessorService', () => { const result = await service.createUnitBookRecords(1, 2, unitFiles); expect(db.transaction).toHaveBeenCalledTimes(1); - expect(result).toEqual({ bookIds: [42], createdBookIds: [42], attachedFileIds: [] }); + expect(result).toEqual({ bookIds: [42], createdBookIds: [42], attachedFileIds: [], replacedPrimaries: [] }); expect(insertBooksValues).toHaveBeenCalledTimes(1); expect(insertBookFilesValues).toHaveBeenCalledTimes(2); }); @@ -383,7 +391,7 @@ describe('UploadProcessorService', () => { const result = await service.createUnitBookRecords(1, 2, [unitFiles[0]!]); - expect(result).toEqual({ bookIds: [99], createdBookIds: [], attachedFileIds: [777] }); + expect(result).toEqual({ bookIds: [99], createdBookIds: [], attachedFileIds: [777], replacedPrimaries: [] }); }); it('leaves a row that already pointed at that path out of the rollback set', async () => { @@ -391,7 +399,7 @@ describe('UploadProcessorService', () => { const result = await service.createUnitBookRecords(1, 2, [unitFiles[0]!]); - expect(result).toEqual({ bookIds: [99], createdBookIds: [], attachedFileIds: [] }); + expect(result).toEqual({ bookIds: [99], createdBookIds: [], attachedFileIds: [], replacedPrimaries: [] }); }); it('hashes every file before opening the transaction', async () => { @@ -406,17 +414,231 @@ describe('UploadProcessorService', () => { }); }); + /** + * A file that joins a book already in the library has to be ranked against the files there. The + * book used to keep whichever file created it, so an ebook that arrived first stayed primary in + * a library that ranks audiobooks first. + */ + describe('ranking the primary of a book a file joins', () => { + const audioFirst = ['m4b', 'mp3', 'epub', 'pdf']; + const epubFirst = ['epub', 'pdf', 'm4b', 'mp3']; + const joinedBook = (overrides: Record = {}) => ({ + id: 99, + primaryFileId: 500, + status: 'present', + formatPriority: audioFirst, + ...overrides, + }); + const ebook = { id: 500, format: 'epub', sizeBytes: 1000, mediaOverlayAvailable: false }; + const audiobook = { id: 777, format: 'm4b', sizeBytes: 5000, mediaOverlayAvailable: false }; + + function joinExistingBook(book: ReturnType, contentFiles: unknown[], newFileId = 777) { + // The locked book lookup, then no `book_files` row yet at the new path. + selectLimit.mockResolvedValueOnce([book]).mockResolvedValueOnce([]); + insertBookFilesReturning.mockResolvedValueOnce([{ id: newFileId }]); + selectOrderBy.mockResolvedValueOnce(contentFiles); + } + + it('makes the audiobook primary when it joins an ebook in a library that ranks audio first', async () => { + joinExistingBook(joinedBook(), [ebook, audiobook]); + + const result = await service.createBookRecord(1, 2, '/folder', '/folder/book.m4b', 'book/book.m4b', 'm4b', 5000); + + expect(result).toEqual({ bookId: 99, created: false }); + expect(updateBooksSet).toHaveBeenCalledTimes(1); + expect(updateBooksSet).toHaveBeenCalledWith({ primaryFileId: 777, updatedAt: expect.any(Date) }); + }); + + it('keeps the ebook primary when the library ranks ebooks first', async () => { + joinExistingBook(joinedBook({ formatPriority: epubFirst }), [ebook, audiobook]); + + await service.createBookRecord(1, 2, '/folder', '/folder/book.m4b', 'book/book.m4b', 'm4b', 5000); + + expect(updateBooksSet).not.toHaveBeenCalled(); + }); + + it('keeps the current primary when the joining file only ties with it', async () => { + // Plain ranking would take the lowest id among equal EPUBs; the book's own primary has to win the tie. + const older = { id: 450, format: 'epub', sizeBytes: 900, mediaOverlayAvailable: false }; + const primary = { ...ebook, id: 800 }; + const joining = { ...ebook, id: 900 }; + joinExistingBook(joinedBook({ primaryFileId: 800, formatPriority: epubFirst }), [older, primary, joining], 900); + + await service.createBookRecord(1, 2, '/folder', '/folder/other.epub', 'book/other.epub', 'epub', 1000); + + expect(updateBooksSet).not.toHaveBeenCalled(); + }); + + it('prefers a joining read-along EPUB over a plain one, as the scanner does', async () => { + const readAlong = { id: 900, format: 'epub', sizeBytes: 3000, mediaOverlayAvailable: true }; + joinExistingBook(joinedBook({ formatPriority: epubFirst }), [ebook, readAlong], 900); + + await service.createBookRecord(1, 2, '/folder', '/folder/readalong.epub', 'book/readalong.epub', 'epub', 3000); + + expect(updateBooksSet).toHaveBeenCalledWith({ primaryFileId: 900, updatedAt: expect.any(Date) }); + }); + + it('gives a book with no primary one', async () => { + joinExistingBook(joinedBook({ primaryFileId: null }), [audiobook]); + + await service.createBookRecord(1, 2, '/folder', '/folder/book.m4b', 'book/book.m4b', 'm4b', 5000); + + expect(updateBooksSet).toHaveBeenCalledWith({ primaryFileId: 777, updatedAt: expect.any(Date) }); + }); + + it('keeps the primary rather than clearing it when every file is empty', async () => { + joinExistingBook(joinedBook(), [ + { ...ebook, sizeBytes: 0 }, + { ...audiobook, sizeBytes: 0 }, + ]); + + await service.createBookRecord(1, 2, '/folder', '/folder/book.m4b', 'book/book.m4b', 'm4b', 0); + + expect(updateBooksSet).not.toHaveBeenCalled(); + }); + + it('leaves a missing book alone, since its other rows point at files that are gone', async () => { + joinExistingBook(joinedBook({ status: 'missing' }), [ebook, audiobook]); + + await service.createBookRecord(1, 2, '/folder', '/folder/book.m4b', 'book/book.m4b', 'm4b', 5000); + + expect(selectOrderBy).not.toHaveBeenCalled(); + expect(updateBooksSet).not.toHaveBeenCalled(); + }); + + it('does not re-rank for a file that is not content', async () => { + joinExistingBook(joinedBook(), [ebook, audiobook]); + + await service.createBookRecord(1, 2, '/folder', '/folder/cover.jpg', 'book/cover.jpg', 'jpg', 50, { role: 'cover' }); + + expect(selectOrderBy).not.toHaveBeenCalled(); + expect(updateBooksSet).not.toHaveBeenCalled(); + }); + + it('does not re-rank a book this call created', async () => { + await service.createBookRecord(1, 2, '/folder', '/folder/book.epub', 'book/book.epub', 'epub', 12345); + + expect(selectOrderBy).not.toHaveBeenCalled(); + expect(updateBooksSet).toHaveBeenCalledWith({ primaryFileId: 420 }); + }); + + it('locks the book it joins until the transaction ends', async () => { + joinExistingBook(joinedBook(), [ebook, audiobook]); + + await service.createBookRecord(1, 2, '/folder', '/folder/book.m4b', 'book/book.m4b', 'm4b', 5000); + + expect(selectInnerJoin).toHaveBeenCalledTimes(1); + expect(selectFor).toHaveBeenCalledWith('update', { of: books }); + }); + + it('reports the primary a unit replaced so a rollback can put it back', async () => { + joinExistingBook(joinedBook(), [ebook, audiobook]); + + const result = await service.createUnitBookRecords(1, 2, [ + { folderPath: '/folder', absolutePath: '/folder/book.m4b', relPath: 'book/book.m4b', format: 'm4b', sizeBytes: 5000 }, + ]); + + expect(result).toEqual({ + bookIds: [99], + createdBookIds: [], + attachedFileIds: [777], + replacedPrimaries: [{ bookId: 99, previousPrimaryFileId: 500, primaryFileId: 777 }], + }); + }); + + it('ranks a multi-file unit once, after every file is in', async () => { + const trackOne = { ...audiobook, id: 777, format: 'mp3' }; + const trackTwo = { ...audiobook, id: 778, format: 'mp3' }; + selectLimit.mockResolvedValueOnce([joinedBook()]).mockResolvedValueOnce([]).mockResolvedValueOnce([joinedBook()]).mockResolvedValueOnce([]); + insertBookFilesReturning.mockResolvedValueOnce([{ id: 777 }]).mockResolvedValueOnce([{ id: 778 }]); + selectOrderBy.mockResolvedValueOnce([ebook, trackOne, trackTwo]); + + const result = await service.createUnitBookRecords(1, 2, [ + { folderPath: '/folder', absolutePath: '/folder/01.mp3', relPath: 'book/01.mp3', format: 'mp3', sizeBytes: 5000, sortOrder: 1 }, + { folderPath: '/folder', absolutePath: '/folder/02.mp3', relPath: 'book/02.mp3', format: 'mp3', sizeBytes: 5000, sortOrder: 2 }, + ]); + + expect(selectOrderBy).toHaveBeenCalledTimes(1); + expect(updateBooksSet).toHaveBeenCalledTimes(1); + expect(updateBooksSet).toHaveBeenCalledWith({ primaryFileId: 777, updatedAt: expect.any(Date) }); + expect(result.replacedPrimaries).toEqual([{ bookId: 99, previousPrimaryFileId: 500, primaryFileId: 777 }]); + }); + + it('does not re-rank a book the unit itself created', async () => { + insertBooksReturning.mockResolvedValueOnce([{ id: 42 }]); + selectLimit + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([joinedBook({ id: 42, primaryFileId: 420 })]) + .mockResolvedValueOnce([]); + + const result = await service.createUnitBookRecords(1, 2, [ + { folderPath: '/folder', absolutePath: '/folder/book.epub', relPath: 'book/book.epub', format: 'epub', sizeBytes: 1000 }, + { folderPath: '/folder', absolutePath: '/folder/book.m4b', relPath: 'book/book.m4b', format: 'm4b', sizeBytes: 5000 }, + ]); + + expect(selectOrderBy).not.toHaveBeenCalled(); + expect(result.replacedPrimaries).toEqual([]); + }); + }); + describe('deleteUnitBookRecords', () => { it('clears the file rows before the books that own them', async () => { - await service.deleteUnitBookRecords({ bookIds: [42], createdBookIds: [42], attachedFileIds: [777] }); + await service.deleteUnitBookRecords({ bookIds: [42], createdBookIds: [42], attachedFileIds: [777], replacedPrimaries: [] }); expect(tx.delete).toHaveBeenNthCalledWith(1, bookFiles); expect(tx.delete).toHaveBeenNthCalledWith(2, bookFiles); expect(tx.delete).toHaveBeenNthCalledWith(3, books); }); + const dialect = new PgDialect(); + const restoreWhere = () => dialect.sqlToQuery(updateBooksWhere.mock.calls[0]![0] as SQL); + + it('puts back a primary the unit replaced, after its rows are gone', async () => { + await service.deleteUnitBookRecords({ + bookIds: [99], + createdBookIds: [], + attachedFileIds: [777], + replacedPrimaries: [{ bookId: 99, previousPrimaryFileId: 500, primaryFileId: 777 }], + }); + + expect(tx.delete).toHaveBeenCalledWith(bookFiles); + expect(updateBooksSet).toHaveBeenCalledWith({ primaryFileId: 500, updatedAt: expect.any(Date) }); + expect(deleteWhere.mock.invocationCallOrder[0]).toBeLessThan(updateBooksSet.mock.invocationCallOrder[0]!); + + // Only a primary that was cleared, or is still the one the unit chose, is undone, and only + // onto a file that is still this book's. + const { sql, params } = restoreWhere(); + expect(sql).toMatch(/"primary_file_id" is null or "books"\."primary_file_id" = \$\d/); + expect(sql).toMatch(/EXISTS \(SELECT 1 FROM "book_files"/); + expect(params).toEqual(expect.arrayContaining([99, 777, 500])); + }); + + it('puts back an absent primary without requiring a file for it', async () => { + await service.deleteUnitBookRecords({ + bookIds: [99], + createdBookIds: [], + attachedFileIds: [777], + replacedPrimaries: [{ bookId: 99, previousPrimaryFileId: null, primaryFileId: 777 }], + }); + + expect(updateBooksSet).toHaveBeenCalledWith({ primaryFileId: null, updatedAt: expect.any(Date) }); + expect(restoreWhere().sql).not.toMatch(/EXISTS/); + }); + + it('puts back a replaced primary even when the unit added no rows of its own', async () => { + await service.deleteUnitBookRecords({ + bookIds: [99], + createdBookIds: [], + attachedFileIds: [], + replacedPrimaries: [{ bookId: 99, previousPrimaryFileId: 500, primaryFileId: 777 }], + }); + + expect(db.transaction).toHaveBeenCalledTimes(1); + expect(updateBooksSet).toHaveBeenCalledWith({ primaryFileId: 500, updatedAt: expect.any(Date) }); + }); + it('does nothing when the unit created nothing of its own', async () => { - await service.deleteUnitBookRecords({ bookIds: [99], createdBookIds: [], attachedFileIds: [] }); + await service.deleteUnitBookRecords({ bookIds: [99], createdBookIds: [], attachedFileIds: [], replacedPrimaries: [] }); expect(db.transaction).not.toHaveBeenCalled(); }); @@ -431,7 +653,7 @@ describe('UploadProcessorService', () => { coverReconciler as any, ); - await withCovers.deleteUnitBookRecords({ bookIds: [42, 7], createdBookIds: [42], attachedFileIds: [777] }); + await withCovers.deleteUnitBookRecords({ bookIds: [42, 7], createdBookIds: [42], attachedFileIds: [777], replacedPrimaries: [] }); expect(coverStore.removeCoverDirectory).toHaveBeenCalledWith(42); expect(coverStore.removeCoverDirectory).not.toHaveBeenCalledWith(7); diff --git a/server/src/modules/upload/upload-processor.service.ts b/server/src/modules/upload/upload-processor.service.ts index f1504040e..ffbd6a257 100644 --- a/server/src/modules/upload/upload-processor.service.ts +++ b/server/src/modules/upload/upload-processor.service.ts @@ -1,15 +1,16 @@ import { Inject, Injectable, InternalServerErrorException, Logger, Optional } from '@nestjs/common'; import { stat } from 'fs/promises'; -import { and, eq, inArray } from 'drizzle-orm'; +import { and, asc, eq, inArray, isNull, or, sql } from 'drizzle-orm'; import { NodePgDatabase } from 'drizzle-orm/node-postgres'; import { isAudioFormat } from '@bookorbit/types'; import type { FileRole as BookFileRole } from '../scanner/lib/classify'; import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils'; +import { selectPrimaryFileKeepingCurrent } from '../../common/utils/primary-file-selection.utils'; import { FORMATS_WITH_UNBOUNDED_METADATA_READS, MAX_BUFFERED_METADATA_BYTES } from '../../common/constants/upload.constants'; import { DB } from '../../db'; import * as schema from '../../db/schema'; -import { bookFiles, bookMetadata, books, uploadSessions } from '../../db/schema'; +import { bookFiles, bookMetadata, books, libraries, uploadSessions } from '../../db/schema'; import { BookMetadataFetchOrchestratorService } from '../book-metadata-fetch/book-metadata-fetch-orchestrator.service'; import { BookCoverStore } from '../book-cover-store/book-cover-store.service'; import { CoverSlotReconciler } from '../metadata/cover-slot-reconciler.service'; @@ -34,13 +35,29 @@ export interface UnitBookFileInput { /** * What a unit actually wrote, so a later failure can take it back. `attachedFileIds` are rows * added to books that were already there: undoing those must remove the file rows and leave the - * book alone. + * book alone. `replacedPrimaries` are those books whose primary the new files outranked, because + * deleting the rows alone would leave such a book with no primary at all. */ export interface UnitBookRecords { /** Every book the unit touched, primary first. */ bookIds: number[]; createdBookIds: number[]; attachedFileIds: number[]; + replacedPrimaries: ReplacedPrimary[]; +} + +export interface ReplacedPrimary { + bookId: number; + previousPrimaryFileId: number | null; + primaryFileId: number; +} + +/** A book that a file joined, as it stood before any file of this transaction was added. */ +interface JoinedBook { + id: number; + primaryFileId: number | null; + status: string; + formatPriority: string[]; } interface MeasuredFile { @@ -101,8 +118,8 @@ export class UploadProcessorService { /** * One `book_files` row, attached to the book that owns `folderPath` or to a new one. Calling it * repeatedly with the same `folderPath` is how a multi-file unit becomes one book with many - * files, and it is why the caller must pass the **primary file first**: `primaryFileId` is set - * only on the call that creates the book. + * files, and it is why the caller must pass the **primary file first**: a new book takes its + * first file as primary. A book that already existed re-ranks its primary instead. */ async createBookRecord( libraryId: number, @@ -124,6 +141,9 @@ export class UploadProcessorService { { folderPath, absolutePath, relPath, format, sizeBytes, ...fileOptions }, measured, ); + if (upserted.joinedBook && (fileOptions.role ?? 'content') === 'content') { + await this.reselectPrimaryFile(tx, upserted.joinedBook); + } if (uploadSessionId) { await tx.update(uploadSessions).set({ resultBookId: upserted.bookId, updatedAt: new Date() }).where(eq(uploadSessions.id, uploadSessionId)); } @@ -138,6 +158,7 @@ export class UploadProcessorService { * three left the first two behind as a ghost book. Here nothing is visible until all of it is. * * Files must arrive **primary first**: `primaryFileId` is set on the row that creates the book. + * A book that already existed re-ranks its primary once every file of the unit is in. */ async createUnitBookRecords(libraryId: number, libraryFolderId: number, files: UnitBookFileInput[]): Promise { if (files.length === 0) throw new InternalServerErrorException('Cannot create a book from an empty unit'); @@ -151,6 +172,7 @@ export class UploadProcessorService { const bookIds: number[] = []; const createdBookIds: number[] = []; const attachedFileIds: number[] = []; + const joinedBooks = new Map(); for (const [index, file] of files.entries()) { const result = await this.upsertBookFile(tx, libraryId, libraryFolderId, file, measured[index]!); @@ -160,9 +182,21 @@ export class UploadProcessorService { // book that was already there need remembering - and only the ones actually inserted, // never one that was already pointing at that path. else if (result.createdFile && !createdBookIds.includes(result.bookId)) attachedFileIds.push(result.fileId); + + // The first sighting is the book as it stood before this unit, which is what a rollback restores. + const joined = result.joinedBook; + if (joined && (file.role ?? 'content') === 'content' && !createdBookIds.includes(joined.id) && !joinedBooks.has(joined.id)) { + joinedBooks.set(joined.id, joined); + } } - return { bookIds, createdBookIds, attachedFileIds }; + const replacedPrimaries: ReplacedPrimary[] = []; + for (const joined of joinedBooks.values()) { + const replaced = await this.reselectPrimaryFile(tx, joined); + if (replaced) replacedPrimaries.push(replaced); + } + + return { bookIds, createdBookIds, attachedFileIds, replacedPrimaries }; }); } @@ -172,12 +206,13 @@ export class UploadProcessorService { * everything except the rows this unit added. */ async deleteUnitBookRecords(records: UnitBookRecords): Promise { - if (records.createdBookIds.length === 0 && records.attachedFileIds.length === 0) return; + if (records.createdBookIds.length === 0 && records.attachedFileIds.length === 0 && records.replacedPrimaries.length === 0) return; await this.db.transaction(async (tx) => { if (records.attachedFileIds.length > 0) { await tx.delete(bookFiles).where(inArray(bookFiles.id, records.attachedFileIds)); } + for (const replaced of records.replacedPrimaries) await this.restorePrimaryFile(tx, replaced); if (records.createdBookIds.length > 0) { // `books.primary_file_id` is `on delete set null`, so clearing the files first leaves // nothing pointing at a row that is about to disappear. @@ -205,7 +240,7 @@ export class UploadProcessorService { libraryFolderId: number, file: UnitBookFileInput, measured: MeasuredFile, - ): Promise<{ bookId: number; createdBook: boolean; fileId: number; createdFile: boolean }> { + ): Promise<{ bookId: number; createdBook: boolean; fileId: number; createdFile: boolean; joinedBook: JoinedBook | null }> { const { folderPath, absolutePath, relPath, format, sizeBytes } = file; const role = file.role ?? 'content'; const sortOrder = file.sortOrder ?? null; @@ -226,9 +261,13 @@ export class UploadProcessorService { }; const [existingBook] = await tx - .select({ id: books.id }) + .select({ id: books.id, primaryFileId: books.primaryFileId, status: books.status, formatPriority: libraries.formatPriority }) .from(books) + .innerJoin(libraries, eq(books.libraryId, libraries.id)) .where(and(eq(books.libraryId, libraryId), eq(books.folderPath, folderPath))) + // Held until commit, so a second import filing into the same book waits and then ranks both + // files, instead of each transaction ranking only the file it can see. + .for('update', { of: books }) .limit(1); if (existingBook) { @@ -239,7 +278,13 @@ export class UploadProcessorService { .onConflictDoUpdate({ target: bookFiles.absolutePath, set: { ...values, bookId: existingBook.id } }) .returning({ id: bookFiles.id }); if (!fileRow) throw new InternalServerErrorException('Failed to create book file'); - return { bookId: existingBook.id, createdBook: false, fileId: fileRow.id, createdFile: alreadyThere === undefined }; + return { + bookId: existingBook.id, + createdBook: false, + fileId: fileRow.id, + createdFile: alreadyThere === undefined, + joinedBook: existingBook, + }; } const [book] = await tx.insert(books).values({ libraryId, libraryFolderId, folderPath, status: 'present' }).returning({ id: books.id }); @@ -256,7 +301,56 @@ export class UploadProcessorService { await tx.update(books).set({ primaryFileId: fileRow.id }).where(eq(books.id, book.id)); - return { bookId: book.id, createdBook: true, fileId: fileRow.id, createdFile: true }; + return { bookId: book.id, createdBook: true, fileId: fileRow.id, createdFile: true, joinedBook: null }; + } + + /** + * Re-ranks the primary of a book that files were just added to, as the scanner would. Without it + * the book kept whichever file created it, so an EPUB that arrived first stayed primary in a + * library that ranks audiobooks first. A missing book is left alone: its other rows describe files + * that are gone, and the scanner re-ranks it when it restores the book. + */ + private async reselectPrimaryFile(tx: Tx, book: JoinedBook): Promise { + if (book.status === 'missing') return null; + + const contentFiles = await tx + .select({ + id: bookFiles.id, + format: bookFiles.format, + sizeBytes: bookFiles.sizeBytes, + mediaOverlayAvailable: bookFiles.mediaOverlayAvailable, + }) + .from(bookFiles) + .where(and(eq(bookFiles.bookId, book.id), eq(bookFiles.role, 'content'))) + .orderBy(asc(bookFiles.id)); + + const winner = selectPrimaryFileKeepingCurrent(contentFiles, book.primaryFileId, book.formatPriority); + // No winner means every file is empty, and clearing a primary over that is worse than keeping it. + if (!winner || winner.id === book.primaryFileId) return null; + + await tx.update(books).set({ primaryFileId: winner.id, updatedAt: new Date() }).where(eq(books.id, book.id)); + return { bookId: book.id, previousPrimaryFileId: book.primaryFileId, primaryFileId: winner.id }; + } + + /** + * Puts back the primary a unit replaced. Deleting the unit's rows has already cleared a primary + * that pointed at one of them, so a cleared primary or the one this unit chose is ours to undo; + * anything else was chosen since, and stays. + */ + private async restorePrimaryFile(tx: Tx, replaced: ReplacedPrimary): Promise { + const previous = replaced.previousPrimaryFileId; + await tx + .update(books) + .set({ primaryFileId: previous, updatedAt: new Date() }) + .where( + and( + eq(books.id, replaced.bookId), + or(isNull(books.primaryFileId), eq(books.primaryFileId, replaced.primaryFileId)), + previous === null + ? undefined + : sql`EXISTS (SELECT 1 FROM ${bookFiles} WHERE ${bookFiles.id} = ${previous} AND ${bookFiles.bookId} = ${replaced.bookId})`, + ), + ); } processNewBookImportAsync(bookId: number, libraryId: number, absolutePath: string, format: string): void { diff --git a/server/src/modules/upload/upload.service.ts b/server/src/modules/upload/upload.service.ts index 54d7f6bbe..717b89e48 100644 --- a/server/src/modules/upload/upload.service.ts +++ b/server/src/modules/upload/upload.service.ts @@ -7,7 +7,7 @@ import { and, asc, eq } from 'drizzle-orm'; import { NodePgDatabase } from 'drizzle-orm/node-postgres'; import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils'; import { buildPatternTokens } from '../../common/utils/pattern-tokens.utils'; -import { selectPrimaryFile } from '../../common/utils/primary-file-selection.utils'; +import { selectPrimaryFileKeepingCurrent } from '../../common/utils/primary-file-selection.utils'; import { DB } from '../../db'; import * as schema from '../../db/schema'; @@ -37,8 +37,6 @@ import { uploadError } from './upload-errors'; type Db = NodePgDatabase; type StoredUploadResult = UploadResult & { absolutePath: string; created: boolean; libraryId: number }; -type PrimaryFileCandidate = Pick; - @Injectable() export class UploadService { private readonly logger = new Logger(UploadService.name); @@ -315,7 +313,7 @@ export class UploadService { .where(and(eq(bookFiles.bookId, bookId), eq(bookFiles.role, 'content'))) .orderBy(asc(bookFiles.id)); - const winner = this.pickPrimaryFile(contentFiles, lockedBook.primaryFileId, lockedBook.formatPriority); + const winner = selectPrimaryFileKeepingCurrent(contentFiles, lockedBook.primaryFileId, lockedBook.formatPriority); const nextPrimaryFileId = winner?.id ?? null; const needsPrimaryUpdate = nextPrimaryFileId !== lockedBook.primaryFileId; const needsStatusUpdate = lockedBook.status === 'missing'; @@ -375,14 +373,6 @@ export class UploadService { } } - private pickPrimaryFile(files: PrimaryFileCandidate[], currentPrimaryFileId: number | null, formatPriority: string[]): PrimaryFileCandidate | null { - const ordered = - currentPrimaryFileId == null - ? files - : [...files.filter((file) => file.id === currentPrimaryFileId), ...files.filter((file) => file.id !== currentPrimaryFileId)]; - return selectPrimaryFile(ordered, formatPriority); - } - async renameBookFiles(bookId: number, user: RequestUser): Promise { const event = 'book.rename_files'; const startedAt = Date.now(); diff --git a/server/test/book-dock-ingest-finalize.e2e-spec.ts b/server/test/book-dock-ingest-finalize.e2e-spec.ts index c5d9ca9fe..209251d60 100644 --- a/server/test/book-dock-ingest-finalize.e2e-spec.ts +++ b/server/test/book-dock-ingest-finalize.e2e-spec.ts @@ -448,6 +448,83 @@ describe('Book Dock ingest + finalize (e2e)', () => { expect(await getBookDockRow(context, bookDockRow.id)).toBeUndefined(); }); + /** + * An ebook and its audiobook finalized one after the other land in one folder, so the second joins + * the book the first created. The book has to open with whichever the library ranks first, not + * with whichever arrived first. + */ + describe('when a second format joins a finalized book', () => { + const metadata = { title: 'Two Media Title', authors: ['Two Media Author'] }; + + async function libraryRanking(formatPriority: string[] | null) { + const destination = await createLibraryWithFolder(context, { allowedFormats: ['epub', 'm4b'] }); + if (formatPriority) { + await context.db.update(schema.libraries).set({ formatPriority }).where(eq(schema.libraries.id, destination.libraryId)); + } + return destination; + } + + async function finalizeInto(destination: Awaited>, fileName: string): Promise { + const row = await createBookDockRow(context, { + fileName, + selectedMetadata: metadata, + targetLibraryId: destination.libraryId, + targetFolderId: destination.libraryFolderId, + }); + const response = await context.app.inject({ + method: 'POST', + url: '/api/v1/book-dock/finalize', + headers: authHeader(context.adminToken), + payload: { fileIds: [row.id] }, + }); + expect(response.statusCode).toBe(201); + const result = (response.json() as BookDockFinalizeResult).results[0]; + expect(result).toMatchObject({ success: true, bookId: expect.any(Number) }); + return result!.bookId!; + } + + async function primaryFormatOf(bookId: number) { + const files = await context.db + .select({ id: schema.bookFiles.id, format: schema.bookFiles.format }) + .from(schema.bookFiles) + .where(eq(schema.bookFiles.bookId, bookId)); + const [book] = await context.db + .select({ primaryFileId: schema.books.primaryFileId }) + .from(schema.books) + .where(eq(schema.books.id, bookId)) + .limit(1); + return { formats: files.map((file) => file.format).sort(), primary: files.find((file) => file.id === book?.primaryFileId)?.format }; + } + + it('makes the audiobook primary when it joins an ebook in a library that ranks audio first', async () => { + const destination = await libraryRanking(['m4b', 'epub']); + + const bookId = await finalizeInto(destination, 'two-media-audio-first.epub'); + expect(await primaryFormatOf(bookId)).toEqual({ formats: ['epub'], primary: 'epub' }); + + expect(await finalizeInto(destination, 'two-media-audio-first.m4b')).toBe(bookId); + expect(await primaryFormatOf(bookId)).toEqual({ formats: ['epub', 'm4b'], primary: 'm4b' }); + }); + + it('keeps the audiobook primary when an ebook joins it in a library that ranks audio first', async () => { + const destination = await libraryRanking(['m4b', 'epub']); + + const bookId = await finalizeInto(destination, 'two-media-audio-kept.m4b'); + expect(await finalizeInto(destination, 'two-media-audio-kept.epub')).toBe(bookId); + + expect(await primaryFormatOf(bookId)).toEqual({ formats: ['epub', 'm4b'], primary: 'm4b' }); + }); + + it('keeps the ebook primary when an audiobook joins it under the default ranking', async () => { + const destination = await libraryRanking(null); + + const bookId = await finalizeInto(destination, 'two-media-default.epub'); + expect(await finalizeInto(destination, 'two-media-default.m4b')).toBe(bookId); + + expect(await primaryFormatOf(bookId)).toEqual({ formats: ['epub', 'm4b'], primary: 'epub' }); + }); + }); + it('finalize returns partial success with duplicate and destination conflicts', async () => { const destination = await createLibraryWithFolder(context);