mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 08:21:39 -06:00
fix(books): re-rank the primary file when an import joins an existing book
A file filed into a book that already existed was added without re-ranking, so the book kept whichever file created it: an ebook that arrived first stayed primary in a library that ranks audiobooks first. Imports and uploads now re-rank with the library's priority, keep the current primary on a tie, and a rolled-back finalize restores the primary it replaced.
This commit is contained in:
@@ -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']);
|
||||
|
||||
@@ -41,6 +41,22 @@ export function selectPrimaryFile<T extends PrimaryFileCandidate>(
|
||||
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<T extends PrimaryFileCandidate>(
|
||||
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
|
||||
|
||||
@@ -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<string, unknown> = {}) => ({
|
||||
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<typeof joinedBook>, 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);
|
||||
|
||||
@@ -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<UnitBookRecords> {
|
||||
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<number, JoinedBook>();
|
||||
|
||||
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<void> {
|
||||
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<ReplacedPrimary | null> {
|
||||
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<void> {
|
||||
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 {
|
||||
|
||||
@@ -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<typeof schema>;
|
||||
type StoredUploadResult = UploadResult & { absolutePath: string; created: boolean; libraryId: number };
|
||||
|
||||
type PrimaryFileCandidate = Pick<typeof bookFiles.$inferSelect, 'id' | 'format' | 'sizeBytes' | 'mediaOverlayAvailable'>;
|
||||
|
||||
@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<void> {
|
||||
const event = 'book.rename_files';
|
||||
const startedAt = Date.now();
|
||||
|
||||
@@ -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<ReturnType<typeof createLibraryWithFolder>>, fileName: string): Promise<number> {
|
||||
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);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user