fix(scanner): re-evaluate primary file format priority on upload

Re-evaluate the library format priority when uploading additional files to an existing book so higher-priority file formats properly take over as the primary file.

Closes #755

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<typeof vi.fn>) {
+  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'));

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<typeof schema>;
 
+type PrimaryFileCandidate = Pick<typeof bookFiles.$inferSelect, 'id' | 'format' | 'sizeBytes'>;
+
 @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<void> {
     const event = 'book.rename_files';
     const startedAt = Date.now();