diff --git a/server/src/modules/upload/upload.service.test.ts b/server/src/modules/upload/upload.service.test.ts index ec1d8c438..7978fc020 100644 --- a/server/src/modules/upload/upload.service.test.ts +++ b/server/src/modules/upload/upload.service.test.ts @@ -17,6 +17,7 @@ import { extractCbzMetadata } from '../metadata/lib/cbz-metadata'; import { parseMobiFile } from '../metadata/lib/mobi-parser'; import { parsePdfFile } from '../metadata/lib/pdf-parser'; import { computeFileHash } from '../scanner/lib/hash'; +import { books } from '../../db/schema'; import { UploadService } from './upload.service'; @@ -41,16 +42,44 @@ function selectChain(rows: unknown[]) { }; } +function lockedBookSelectChain(rows: unknown[], forUpdateMock: ReturnType) { + const limitMock = vi.fn().mockResolvedValue(rows); + forUpdateMock.mockReturnValue({ limit: limitMock }); + const whereMock = vi.fn().mockReturnValue({ for: forUpdateMock }); + const joinMock = vi.fn().mockReturnValue({ where: whereMock }); + + return { + from: vi.fn().mockReturnValue({ innerJoin: joinMock }), + }; +} + +function contentFilesSelectChain(rows: unknown[]) { + const orderByMock = vi.fn().mockResolvedValue(rows); + const whereMock = vi.fn().mockReturnValue({ orderBy: orderByMock }); + + return { + from: vi.fn().mockReturnValue({ where: whereMock }), + }; +} + describe('UploadService', () => { const insertReturning = vi.fn(); const insertValues = vi.fn(); const updateSet = vi.fn(); const updateWhere = vi.fn(); + const forUpdate = vi.fn(); + + const tx = { + select: vi.fn(), + insert: vi.fn(), + update: vi.fn(), + }; const db = { select: vi.fn(), insert: vi.fn(() => ({ values: insertValues })), update: vi.fn(() => ({ set: updateSet })), + transaction: vi.fn(), }; const appSettings = { getUploadPattern: vi.fn(), getUploadPatternBookPerFolder: vi.fn(), isCrossPlatformPathSanitizationEnabled: vi.fn() }; @@ -107,6 +136,9 @@ describe('UploadService', () => { mockExtractEpubMetadata.mockResolvedValue(null); // addFileToBook defaults + tx.insert.mockReturnValue({ values: insertValues }); + tx.update.mockReturnValue({ set: updateSet }); + db.transaction.mockImplementation((callback) => callback(tx)); insertValues.mockReturnValue({ returning: insertReturning }); insertReturning.mockResolvedValue([ { @@ -121,6 +153,14 @@ describe('UploadService', () => { ]); updateSet.mockReturnValue({ where: updateWhere }); updateWhere.mockResolvedValue(undefined); + tx.select + .mockReturnValueOnce(lockedBookSelectChain([{ primaryFileId: 99, status: 'present', formatPriority: ['epub', 'm4b', 'mp3'] }], forUpdate)) + .mockReturnValueOnce( + contentFilesSelectChain([ + { id: 55, format: 'epub', sizeBytes: 456 }, + { id: 99, format: 'epub', sizeBytes: 1000 }, + ]), + ); mockComputeFileHash.mockResolvedValue('hash-abc'); mockStat.mockResolvedValue({ ino: 12345n, mtime: new Date('2025-01-01') } as any); }); @@ -735,6 +775,32 @@ describe('UploadService', () => { return selectChain([]); } + function mockElection( + lockedBook: { primaryFileId: number | null; status: string; formatPriority: string[] }, + files: Array<{ id: number; format: string | null; sizeBytes: number | null }>, + ) { + tx.select + .mockReset() + .mockReturnValueOnce(lockedBookSelectChain([lockedBook], forUpdate)) + .mockReturnValueOnce(contentFilesSelectChain(files)); + } + + function mockUploadedFormat(format: string, filename = `book.${format}`) { + validator.sanitizeFilename.mockReturnValue(filename); + validator.validateFormat.mockReturnValue(format); + insertReturning.mockResolvedValue([ + { + id: 55, + format, + role: 'content', + sizeBytes: 456, + absolutePath: `/library/Book Title/${filename}`, + createdAt: new Date('2025-01-01'), + durationSeconds: null, + }, + ]); + } + it('adds a file successfully to an existing book with a primary file', async () => { db.select.mockReturnValueOnce(selectJoinChain([makeBookRow()])).mockReturnValueOnce(noHashConflict()); @@ -742,13 +808,142 @@ describe('UploadService', () => { expect(storage.streamToTemp).toHaveBeenCalled(); expect(storage.moveToPath).toHaveBeenCalledWith('/tmp/upload.bin', '/library/Book Title/book.epub'); - expect(db.insert).toHaveBeenCalled(); + expect(db.transaction).toHaveBeenCalledTimes(1); + expect(forUpdate).toHaveBeenCalledWith('update', { of: books }); + expect(tx.insert).toHaveBeenCalled(); expect(result.filename).toBe('book.epub'); expect(result.format).toBe('epub'); expect(result.bookStatus).toBe('present'); expect(result.role).toBe('content'); }); + it.each([ + { + name: 'promotes a higher-priority EPUB over the existing M4B', + currentFormat: 'm4b', + uploadedFormat: 'epub', + formatPriority: ['epub', 'm4b'], + expectedPrimaryFileId: 55, + }, + { + name: 'retains a higher-priority EPUB when an M4B is added', + currentFormat: 'epub', + uploadedFormat: 'm4b', + formatPriority: ['epub', 'm4b'], + expectedPrimaryFileId: 99, + }, + { + name: 'honors a custom priority that places M4B above EPUB', + currentFormat: 'epub', + uploadedFormat: 'm4b', + formatPriority: ['m4b', 'epub'], + expectedPrimaryFileId: 55, + }, + { + name: 'promotes a listed format over an unlisted current format', + currentFormat: 'm4b', + uploadedFormat: 'epub', + formatPriority: ['epub'], + expectedPrimaryFileId: 55, + }, + { + name: 'retains a listed current format over an unlisted upload', + currentFormat: 'epub', + uploadedFormat: 'm4b', + formatPriority: ['epub'], + expectedPrimaryFileId: 99, + }, + { + name: 'retains the current primary when both formats are unlisted', + currentFormat: 'm4b', + uploadedFormat: 'epub', + formatPriority: [], + expectedPrimaryFileId: 99, + }, + { + name: 'retains the current primary when another file of the same audio format is added', + currentFormat: 'mp3', + uploadedFormat: 'mp3', + formatPriority: ['epub', 'mp3'], + expectedPrimaryFileId: 99, + }, + ])('$name', async ({ currentFormat, uploadedFormat, formatPriority, expectedPrimaryFileId }) => { + mockUploadedFormat(uploadedFormat); + db.select.mockReturnValueOnce(selectJoinChain([makeBookRow()])).mockReturnValueOnce(noHashConflict()); + mockElection({ primaryFileId: 99, status: 'present', formatPriority }, [ + { id: 55, format: uploadedFormat, sizeBytes: 456 }, + { id: 99, format: currentFormat, sizeBytes: 1000 }, + ]); + + const result = await service.addFileToBook(10, `book.${uploadedFormat}`, {} as any, user); + + expect(insertValues).toHaveBeenCalledWith(expect.objectContaining({ format: uploadedFormat, role: 'content' })); + if (expectedPrimaryFileId === 55) { + expect(updateSet).toHaveBeenCalledWith(expect.objectContaining({ primaryFileId: 55 })); + expect(result.role).toBe('primary'); + } else { + expect(tx.update).not.toHaveBeenCalled(); + expect(result.role).toBe('content'); + } + }); + + it('elects an existing higher-priority file when the book has no primary', async () => { + mockUploadedFormat('m4b'); + db.select.mockReturnValueOnce(selectJoinChain([makeBookRow({ primaryFileId: null })])).mockReturnValueOnce(noHashConflict()); + mockElection({ primaryFileId: null, status: 'present', formatPriority: ['epub', 'm4b'] }, [ + { id: 10, format: 'epub', sizeBytes: 1000 }, + { id: 55, format: 'm4b', sizeBytes: 456 }, + ]); + + const result = await service.addFileToBook(10, 'book.m4b', {} as any, user); + + expect(updateSet).toHaveBeenCalledWith(expect.objectContaining({ primaryFileId: 10 })); + expect(result.role).toBe('content'); + }); + + it('repairs a stale primary reference using the highest-priority eligible file', async () => { + mockUploadedFormat('m4b'); + db.select.mockReturnValueOnce(selectJoinChain([makeBookRow({ primaryFileId: 777 })])).mockReturnValueOnce(noHashConflict()); + mockElection({ primaryFileId: 777, status: 'present', formatPriority: ['pdf', 'm4b'] }, [ + { id: 10, format: 'pdf', sizeBytes: 1000 }, + { id: 55, format: 'm4b', sizeBytes: 456 }, + ]); + + const result = await service.addFileToBook(10, 'book.m4b', {} as any, user); + + expect(updateSet).toHaveBeenCalledWith(expect.objectContaining({ primaryFileId: 10 })); + expect(result.role).toBe('content'); + }); + + it('ignores a zero-byte higher-priority legacy file during election', async () => { + mockUploadedFormat('pdf'); + db.select.mockReturnValueOnce(selectJoinChain([makeBookRow()])).mockReturnValueOnce(noHashConflict()); + mockElection({ primaryFileId: 99, status: 'present', formatPriority: ['epub', 'pdf', 'm4b'] }, [ + { id: 10, format: 'epub', sizeBytes: 0 }, + { id: 55, format: 'pdf', sizeBytes: 456 }, + { id: 99, format: 'm4b', sizeBytes: 1000 }, + ]); + + const result = await service.addFileToBook(10, 'book.pdf', {} as any, user); + + expect(updateSet).toHaveBeenCalledWith(expect.objectContaining({ primaryFileId: 55 })); + expect(result.role).toBe('primary'); + }); + + it('updates status and primary file together when a higher-priority format restores a missing book', async () => { + db.select.mockReturnValueOnce(selectJoinChain([makeBookRow({ status: 'missing' })])).mockReturnValueOnce(noHashConflict()); + mockElection({ primaryFileId: 99, status: 'missing', formatPriority: ['epub', 'm4b'] }, [ + { id: 55, format: 'epub', sizeBytes: 456 }, + { id: 99, format: 'm4b', sizeBytes: 1000 }, + ]); + + const result = await service.addFileToBook(10, 'book.epub', {} as any, user); + + expect(updateSet).toHaveBeenCalledWith(expect.objectContaining({ primaryFileId: 55, status: 'present' })); + expect(result.bookStatus).toBe('present'); + expect(result.role).toBe('primary'); + }); + it('kicks off per-file audio duration extraction for an added audio file', async () => { validator.sanitizeFilename.mockReturnValue('chapter-02.mp3'); validator.validateFormat.mockReturnValue('mp3'); @@ -769,30 +964,40 @@ describe('UploadService', () => { it('promotes new file to primary when book has no primary file', async () => { db.select.mockReturnValueOnce(selectJoinChain([makeBookRow({ primaryFileId: null })])).mockReturnValueOnce(noHashConflict()); + mockElection({ primaryFileId: null, status: 'present', formatPriority: ['epub', 'm4b'] }, [{ id: 55, format: 'epub', sizeBytes: 456 }]); const result = await service.addFileToBook(10, 'book.epub', {} as any, user); - expect(db.update).toHaveBeenCalled(); + expect(tx.update).toHaveBeenCalled(); + expect(updateSet).toHaveBeenCalledWith(expect.objectContaining({ primaryFileId: 55 })); expect(updateWhere).toHaveBeenCalled(); expect(result.role).toBe('primary'); }); it('updates book status from missing to present and promotes primary when null', async () => { db.select.mockReturnValueOnce(selectJoinChain([makeBookRow({ primaryFileId: null, status: 'missing' })])).mockReturnValueOnce(noHashConflict()); + mockElection({ primaryFileId: null, status: 'missing', formatPriority: ['epub', 'm4b'] }, [{ id: 55, format: 'epub', sizeBytes: 456 }]); const result = await service.addFileToBook(10, 'book.epub', {} as any, user); - expect(db.update).toHaveBeenCalled(); + expect(tx.update).toHaveBeenCalled(); + expect(updateSet).toHaveBeenCalledWith(expect.objectContaining({ primaryFileId: 55, status: 'present' })); expect(result.bookStatus).toBe('present'); expect(result.role).toBe('primary'); }); it('updates only status when book is missing but already has a primary file', async () => { db.select.mockReturnValueOnce(selectJoinChain([makeBookRow({ primaryFileId: 42, status: 'missing' })])).mockReturnValueOnce(noHashConflict()); + mockElection({ primaryFileId: 42, status: 'missing', formatPriority: ['epub', 'm4b'] }, [ + { id: 42, format: 'epub', sizeBytes: 1000 }, + { id: 55, format: 'epub', sizeBytes: 456 }, + ]); const result = await service.addFileToBook(10, 'book.epub', {} as any, user); - expect(db.update).toHaveBeenCalled(); + expect(tx.update).toHaveBeenCalled(); + expect(updateSet).toHaveBeenCalledWith(expect.objectContaining({ status: 'present' })); + expect(updateSet.mock.calls[0]?.[0]).not.toHaveProperty('primaryFileId'); expect(result.bookStatus).toBe('present'); expect(result.role).toBe('content'); }); @@ -802,7 +1007,7 @@ describe('UploadService', () => { await service.addFileToBook(10, 'book.epub', {} as any, user); - expect(db.update).not.toHaveBeenCalled(); + expect(tx.update).not.toHaveBeenCalled(); }); it('throws NotFoundException when book does not exist', async () => { @@ -812,6 +1017,18 @@ describe('UploadService', () => { expect(storage.streamToTemp).not.toHaveBeenCalled(); }); + it('does not insert when the book is deleted while the file is being uploaded', async () => { + db.select.mockReturnValueOnce(selectJoinChain([makeBookRow()])).mockReturnValueOnce(noHashConflict()); + tx.select.mockReset().mockReturnValueOnce(lockedBookSelectChain([], forUpdate)); + + await expect(service.addFileToBook(10, 'book.epub', {} as any, user)).rejects.toBeInstanceOf(NotFoundException); + + expect(forUpdate).toHaveBeenCalledWith('update', { of: books }); + expect(tx.insert).not.toHaveBeenCalled(); + expect(storage.cleanup).toHaveBeenCalledWith('/tmp/upload.bin'); + expect(storage.cleanup).not.toHaveBeenCalledWith('/library/Book Title/book.epub'); + }); + it('throws ForbiddenException when user has no library access', async () => { db.select.mockReturnValueOnce(selectJoinChain([makeBookRow()])); libraryService.verifyUserAccess.mockRejectedValue(new ForbiddenException('No access')); @@ -871,6 +1088,21 @@ describe('UploadService', () => { expect(storage.cleanup).not.toHaveBeenCalledWith('/library/Book Title/book.epub'); }); + it('does not dispatch processing when the primary election update fails', async () => { + db.select.mockReturnValueOnce(selectJoinChain([makeBookRow()])).mockReturnValueOnce(noHashConflict()); + mockElection({ primaryFileId: 99, status: 'present', formatPriority: ['epub', 'm4b'] }, [ + { id: 55, format: 'epub', sizeBytes: 456 }, + { id: 99, format: 'm4b', sizeBytes: 1000 }, + ]); + updateWhere.mockRejectedValue(new Error('primary update failed')); + + await expect(service.addFileToBook(10, 'book.epub', {} as any, user)).rejects.toThrow('primary update failed'); + + expect(processor.extractAudioDurationAsync).not.toHaveBeenCalled(); + expect(storage.cleanup).toHaveBeenCalledWith('/tmp/upload.bin'); + expect(storage.cleanup).not.toHaveBeenCalledWith('/library/Book Title/book.epub'); + }); + it('cleans up only temp when move fails (destination not yet written)', async () => { db.select.mockReturnValueOnce(selectJoinChain([makeBookRow()])).mockReturnValueOnce(noHashConflict()); storage.moveToPath.mockRejectedValue(new Error('disk full')); diff --git a/server/src/modules/upload/upload.service.ts b/server/src/modules/upload/upload.service.ts index e8c5fe65b..2298d996d 100644 --- a/server/src/modules/upload/upload.service.ts +++ b/server/src/modules/upload/upload.service.ts @@ -3,7 +3,7 @@ import { ModuleRef } from '@nestjs/core'; import { access as fsAccess, stat } from 'fs/promises'; import { basename, dirname, extname, join, relative } from 'path'; import { Readable } from 'stream'; -import { and, eq } from 'drizzle-orm'; +import { and, asc, eq } from 'drizzle-orm'; import { NodePgDatabase } from 'drizzle-orm/node-postgres'; import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils'; import { formatSeriesIndex } from '../../common/utils/series-index-format.utils'; @@ -28,6 +28,8 @@ import { computeFileHash } from '../scanner/lib/hash'; type Db = NodePgDatabase; +type PrimaryFileCandidate = Pick; + @Injectable() export class UploadService { private readonly logger = new Logger(UploadService.name); @@ -127,8 +129,6 @@ export class UploadService { folderPath: books.folderPath, libraryId: books.libraryId, libraryFolderId: books.libraryFolderId, - primaryFileId: books.primaryFileId, - status: books.status, allowedFormats: libraries.allowedFormats, organizationMode: libraries.organizationMode, libraryFolderPath: libraryFolders.path, @@ -190,48 +190,75 @@ export class UploadService { const ino = fileStat.ino; const relPath = relative(bookRow.libraryFolderPath, destination); - const [inserted] = await this.db - .insert(bookFiles) - .values({ - bookId, - libraryFolderId: bookRow.libraryFolderId, - absolutePath: destination, - relPath, - ino, - sizeBytes, - mtime: fileStat.mtime, - fileHash, - format, - role: 'content', - }) - .returning({ - id: bookFiles.id, - format: bookFiles.format, - role: bookFiles.role, - sizeBytes: bookFiles.sizeBytes, - absolutePath: bookFiles.absolutePath, - createdAt: bookFiles.createdAt, - durationSeconds: bookFiles.durationSeconds, - }); - - if (!inserted) throw new Error('Failed to insert book file record'); - - let finalStatus = bookRow.status; - const needsPrimaryPromotion = bookRow.primaryFileId === null; - const needsStatusUpdate = bookRow.status === 'missing'; - - if (needsPrimaryPromotion || needsStatusUpdate) { - await this.db - .update(books) - .set({ - ...(needsPrimaryPromotion ? { primaryFileId: inserted.id } : {}), - ...(needsStatusUpdate ? { status: 'present' } : {}), - updatedAt: new Date(), + const { inserted, isPrimary, finalStatus } = await this.db.transaction(async (tx) => { + const [lockedBook] = await tx + .select({ + primaryFileId: books.primaryFileId, + status: books.status, + formatPriority: libraries.formatPriority, }) - .where(eq(books.id, bookId)); + .from(books) + .innerJoin(libraries, eq(books.libraryId, libraries.id)) + .where(eq(books.id, bookId)) + .for('update', { of: books }) + .limit(1); - if (needsStatusUpdate) finalStatus = 'present'; - } + if (!lockedBook) throw new NotFoundException(`Book ${bookId} not found`); + + const [inserted] = await tx + .insert(bookFiles) + .values({ + bookId, + libraryFolderId: bookRow.libraryFolderId, + absolutePath: destination, + relPath, + ino, + sizeBytes, + mtime: fileStat.mtime, + fileHash, + format, + role: 'content', + }) + .returning({ + id: bookFiles.id, + format: bookFiles.format, + role: bookFiles.role, + sizeBytes: bookFiles.sizeBytes, + absolutePath: bookFiles.absolutePath, + createdAt: bookFiles.createdAt, + durationSeconds: bookFiles.durationSeconds, + }); + + if (!inserted) throw new Error('Failed to insert book file record'); + + const contentFiles = await tx + .select({ id: bookFiles.id, format: bookFiles.format, sizeBytes: bookFiles.sizeBytes }) + .from(bookFiles) + .where(and(eq(bookFiles.bookId, bookId), eq(bookFiles.role, 'content'))) + .orderBy(asc(bookFiles.id)); + + const winner = this.pickPrimaryFile(contentFiles, lockedBook.primaryFileId, lockedBook.formatPriority); + const nextPrimaryFileId = winner?.id ?? null; + const needsPrimaryUpdate = nextPrimaryFileId !== lockedBook.primaryFileId; + const needsStatusUpdate = lockedBook.status === 'missing'; + + if (needsPrimaryUpdate || needsStatusUpdate) { + await tx + .update(books) + .set({ + ...(needsPrimaryUpdate ? { primaryFileId: nextPrimaryFileId } : {}), + ...(needsStatusUpdate ? { status: 'present' } : {}), + updatedAt: new Date(), + }) + .where(eq(books.id, bookId)); + } + + return { + inserted, + isPrimary: inserted.id === nextPrimaryFileId, + finalStatus: needsStatusUpdate ? 'present' : lockedBook.status, + }; + }); this.processor.extractAudioDurationAsync(bookId, destination, format); @@ -242,7 +269,7 @@ export class UploadService { return { id: inserted.id, format: inserted.format, - role: needsPrimaryPromotion ? 'primary' : inserted.role, + role: isPrimary ? 'primary' : inserted.role, sizeBytes: inserted.sizeBytes, absolutePath: inserted.absolutePath, createdAt: inserted.createdAt.toISOString(), @@ -264,6 +291,18 @@ export class UploadService { } } + private pickPrimaryFile(files: PrimaryFileCandidate[], currentPrimaryFileId: number | null, formatPriority: string[]): PrimaryFileCandidate | null { + const candidates = files.filter((file) => (file.sizeBytes ?? 0) > 0); + if (candidates.length === 0) return null; + + const currentPrimary = candidates.find((file) => file.id === currentPrimaryFileId) ?? null; + const preferredFormat = formatPriority.find((candidateFormat) => candidates.some((file) => file.format === candidateFormat)); + + if (preferredFormat === undefined) return currentPrimary ?? candidates[0] ?? null; + if (currentPrimary?.format === preferredFormat) return currentPrimary; + return candidates.find((file) => file.format === preferredFormat) ?? null; + } + async renameBookFiles(bookId: number, user: RequestUser): Promise { const event = 'book.rename_files'; const startedAt = Date.now();