fix(books): correct book detail routes found while porting to Android

Fix the server gaps found while porting book detail actions to Android,
checked against the iOS app:

- audit: record a metadata refresh only when it writes; a preview,
  which every editor lookup takes, was logged as "Refreshed metadata"
- personal note: leave the note alone when the body has no note key,
  as the other partial routes do; null still clears it, and the web
  and every iOS build send the key
- covers: answer re-extract-cover with 409 when every slot it would
  write is locked, as the other cover writes do, instead of an empty
  count a client cannot tell from a file with no art
- types: describe the cover search response as the bare array the
  route returns

packages/types/src/book.ts

@@ -478,7 +478,5 @@ export type CoverSearchResult = {
   source: string;
 };
 
-export type CoverSearchResponse = {
-  results: CoverSearchResult[];
-  total: number;
-};
+/** `GET /books/cover/search` answers with a bare array of results, not an envelope. */
+export type CoverSearchResponse = CoverSearchResult[];

server/src/common/decorators/auditable.decorator.ts

@@ -16,6 +16,8 @@ export interface AuditableOptions {
   getResourceId?: (req: AuditRequest, responseBody: unknown) => number | undefined;
   getMeta?: (req: AuditRequest, responseBody: unknown) => Record<string, unknown> | undefined;
   description: string | ((req: AuditRequest, responseBody: unknown) => string);
+  /** Records the call only when this returns true. Without it every successful call is recorded. */
+  when?: (req: AuditRequest, responseBody: unknown) => boolean;
 }
 
 export const Auditable = (options: AuditableOptions) => {

server/src/common/interceptors/audit.interceptor.test.ts

@@ -75,6 +75,52 @@ describe('AuditInterceptor', () => {
     });
   });
 
+  it('records nothing when the endpoint says this call is not worth recording', async () => {
+    const when = vi.fn().mockReturnValue(false);
+    const reflector = {
+      get: vi.fn().mockReturnValue({
+        action: 'book.metadata_update',
+        description: 'Refreshed metadata for book #4',
+        when,
+      }),
+    };
+    const auditEvents = {
+      emit: vi.fn(),
+    };
+    const interceptor = new AuditInterceptor(reflector as never, auditEvents as never);
+    const next: CallHandler = {
+      handle: vi.fn().mockReturnValue(of({ id: 4 })),
+    };
+    const request = { ip: '::1', query: { preview: 'true' } };
+
+    const response = await firstValueFrom(interceptor.intercept(makeContext(request), next));
+
+    expect(response).toEqual({ id: 4 });
+    expect(when).toHaveBeenCalledWith(request, { id: 4 });
+    expect(auditEvents.emit).not.toHaveBeenCalled();
+  });
+
+  it('records the call when the endpoint says it is worth recording', async () => {
+    const reflector = {
+      get: vi.fn().mockReturnValue({
+        action: 'book.metadata_update',
+        description: 'Refreshed metadata for book #4',
+        when: () => true,
+      }),
+    };
+    const auditEvents = {
+      emit: vi.fn(),
+    };
+    const interceptor = new AuditInterceptor(reflector as never, auditEvents as never);
+    const next: CallHandler = {
+      handle: vi.fn().mockReturnValue(of({ id: 4 })),
+    };
+
+    await firstValueFrom(interceptor.intercept(makeContext({ ip: '::1', query: {} }), next));
+
+    expect(auditEvents.emit).toHaveBeenCalledTimes(1);
+  });
+
   it('falls back to system actor when request user is absent', async () => {
     const reflector = {
       get: vi.fn().mockReturnValue({

server/src/common/interceptors/audit.interceptor.ts

@@ -23,6 +23,7 @@ export class AuditInterceptor implements NestInterceptor {
 
     return next.handle().pipe(
       tap((responseBody) => {
+        if (options.when && !options.when(req, responseBody)) return;
         const user = req.user;
         const action = typeof options.action === 'function' ? options.action(req, responseBody) : options.action;
         const description = typeof options.description === 'function' ? options.description(req, responseBody) : options.description;

server/src/modules/book/book.controller.test.ts

@@ -135,6 +135,7 @@ function makeController() {
     globalQuery: vi.fn(),
     bulkRefreshMetadata: vi.fn(),
     bulkReExtractCover: vi.fn(),
+    reExtractCover: vi.fn(),
     bulkSetMetadata: vi.fn(),
     bulkEditMetadata: vi.fn(),
     bulkUpdateTags: vi.fn(),
@@ -860,24 +861,24 @@ describe('BookController', () => {
     await expect(controller.getThumbnail(7, makeUser(), reply, undefined, undefined)).rejects.toThrow(NotFoundException);
   });
 
-  it('delegates re-extract-cover endpoint to bulk cover flow', async () => {
+  it('delegates re-extract-cover endpoint to the single-book cover flow', async () => {
     const { controller, bookService } = makeController();
-    bookService.bulkReExtractCover.mockResolvedValue({ processed: 1, updated: 1 });
+    bookService.reExtractCover.mockResolvedValue({ processed: 1, updated: 1 });
     const user = makeUser();
 
-    await controller.reExtractCover(42, user);
+    await expect(controller.reExtractCover(42, user)).resolves.toEqual({ processed: 1, updated: 1 });
 
-    expect(bookService.bulkReExtractCover).toHaveBeenCalledWith([42], user);
+    expect(bookService.reExtractCover).toHaveBeenCalledWith(42, user, undefined);
   });
 
   it('passes an explicit medium through re-extract-cover', async () => {
     const { controller, bookService } = makeController();
     const user = makeUser();
-    bookService.bulkReExtractCover.mockResolvedValue({ processed: 1, updated: 1 });
+    bookService.reExtractCover.mockResolvedValue({ processed: 1, updated: 1 });
 
     await controller.reExtractCover(42, user, { medium: 'audio' });
 
-    expect(bookService.bulkReExtractCover).toHaveBeenCalledWith([42], user, undefined, { medium: 'audio' });
+    expect(bookService.reExtractCover).toHaveBeenCalledWith(42, user, 'audio');
   });
 
   it('logs and rethrows download failures', async () => {
@@ -1057,6 +1058,7 @@ describe('BookController', () => {
     const refreshAudit = Reflect.getMetadata(AUDITABLE_KEY, BookController.prototype.refreshMetadata) as {
       getResourceId: (req: { params: Record<string, string> }) => number;
       description: (req: { params: Record<string, string> }) => string;
+      when: (req: { query?: Record<string, string> }) => boolean;
     };
 
     const singleDeletion = { total: 1, books: [{ id: 1, title: 'Dune' }], omitted: 0 };
@@ -1080,6 +1082,9 @@ describe('BookController', () => {
     expect(updateLocksAudit.description({ params: { id: '44' } })).toBe('Updated metadata locks for book #44');
     expect(refreshAudit.getResourceId({ params: { id: '44' } })).toBe(44);
     expect(refreshAudit.description({ params: { id: '44' } })).toBe('Refreshed metadata for book #44');
+    expect(refreshAudit.when({ query: {} })).toBe(true);
+    expect(refreshAudit.when({ query: { preview: 'false' } })).toBe(true);
+    expect(refreshAudit.when({ query: { preview: 'true' } })).toBe(false);
   });
 
   it('marks bulk-edit endpoints as demo-restricted', () => {

server/src/modules/book/book.controller.ts

@@ -219,9 +219,7 @@ export class BookController {
   @Post(':id/re-extract-cover')
   @RequirePermission(Permission.LibraryEditMetadata)
   reExtractCover(@Param('id', ParseIntPipe) id: number, @CurrentUser() user: RequestUser, @Query() query?: CoverMediumQueryDto) {
-    return query?.medium
-      ? this.bookService.bulkReExtractCover([id], user, undefined, { medium: query.medium })
-      : this.bookService.bulkReExtractCover([id], user);
+    return this.bookService.reExtractCover(id, user, query?.medium);
   }
 
   @Post('export')
@@ -658,6 +656,8 @@ export class BookController {
     resource: AuditResource.Book,
     getResourceId: (req) => parseInt(req.params['id'], 10),
     description: (req) => `Refreshed metadata for book #${req.params['id']}`,
+    // A preview writes nothing, so recording it would fill the log with refreshes that never happened.
+    when: (req) => (req.query as { preview?: string } | undefined)?.preview !== 'true',
   })
   refreshMetadata(@Param('id', ParseIntPipe) id: number, @Query('preview') preview: string | undefined, @CurrentUser() user: RequestUser) {
     return this.bookService.refreshMetadata(id, preview === 'true', user);

server/src/modules/book/book.service.test.ts

@@ -2546,6 +2546,54 @@ describe('BookService', () => {
       expect(onProgress).not.toHaveBeenCalled();
     });
 
+    it('reExtractCover answers 409 for a locked slot rather than an empty count', async () => {
+      const { service, bookRepo, bookMetadataLockService, metadataService } = makeService();
+      const user = makeUser();
+      bookRepo.findLibraryIdsByBookIds.mockResolvedValue([{ id: 1, libraryId: 7 }]);
+      bookRepo.findCoverSourceFilesByBookIds.mockResolvedValue([
+        {
+          id: 10,
+          bookId: 1,
+          absolutePath: '/books/1.epub',
+          format: 'epub',
+          role: 'content',
+          sizeBytes: 10,
+          mediaOverlayAvailable: false,
+          formatPriority: ['epub', 'm4b'],
+        },
+      ]);
+      bookMetadataLockService.getCoverLockedBookIds.mockResolvedValue(new Set([1]));
+      bookMetadataLockService.assertFieldsUnlocked.mockRejectedValue(new ConflictException('Metadata fields are locked: cover'));
+
+      await expect(service.reExtractCover(1, user, 'ebook')).rejects.toThrow(ConflictException);
+
+      expect(bookMetadataLockService.assertFieldsUnlocked).toHaveBeenCalledWith(1, ['cover']);
+      expect(metadataService.refreshCoverForBook).not.toHaveBeenCalled();
+    });
+
+    it('reExtractCover returns the count when nothing it would write is locked', async () => {
+      const { service, bookRepo, bookMetadataLockService, metadataService } = makeService();
+      const user = makeUser();
+      bookRepo.findLibraryIdsByBookIds.mockResolvedValue([{ id: 1, libraryId: 7 }]);
+      bookRepo.findCoverSourceFilesByBookIds.mockResolvedValue([
+        {
+          id: 10,
+          bookId: 1,
+          absolutePath: '/books/1.epub',
+          format: 'epub',
+          role: 'content',
+          sizeBytes: 10,
+          mediaOverlayAvailable: false,
+          formatPriority: ['epub', 'm4b'],
+        },
+      ]);
+      metadataService.refreshCoverForBook.mockResolvedValue(false);
+
+      await expect(service.reExtractCover(1, user)).resolves.toEqual({ processed: 1, updated: 0 });
+
+      expect(bookMetadataLockService.assertFieldsUnlocked).not.toHaveBeenCalled();
+    });
+
     it('stops bulk metadata refresh when cancelled', async () => {
       const { service, bookRepo } = makeService();
       const user = makeUser();
@@ -3870,16 +3918,30 @@ describe('BookService', () => {
         expect(result).toEqual({ id: 10, personalNote: 'Loved it.' });
       });
 
-      it('normalizes an undefined note to null when clearing', async () => {
+      it('clears the note with an explicit null', async () => {
         const { service, userBookNoteService } = makeService();
         const user = makeUser({ id: 77 });
         vi.spyOn(service, 'verifyBookAccess').mockResolvedValue(undefined);
         vi.spyOn(service, 'getDetail').mockResolvedValue({ id: 10, personalNote: null } as never);
 
-        await service.updatePersonalNote(10, {}, user);
+        await service.updatePersonalNote(10, { note: null }, user);
 
         expect(userBookNoteService.setNote).toHaveBeenCalledWith(77, 10, null);
       });
+
+      it('leaves the note alone when the body has no note key', async () => {
+        const { service, userBookNoteService } = makeService();
+        const user = makeUser({ id: 77 });
+        const accessSpy = vi.spyOn(service, 'verifyBookAccess').mockResolvedValue(undefined);
+        const detailSpy = vi.spyOn(service, 'getDetail').mockResolvedValue({ id: 10, personalNote: 'Loved it.' } as never);
+
+        const result = await service.updatePersonalNote(10, {}, user);
+
+        expect(accessSpy).toHaveBeenCalledWith(10, user);
+        expect(userBookNoteService.setNote).not.toHaveBeenCalled();
+        expect(detailSpy).toHaveBeenCalledWith(10, user);
+        expect(result).toEqual({ id: 10, personalNote: 'Loved it.' });
+      });
     });
 
     describe('setReadStatus', () => {

server/src/modules/book/book.service.ts

@@ -3126,7 +3126,7 @@ export class BookService {
     bookIds: number[],
     user: RequestUser,
     onProgress?: (bookId: number) => void,
-    options?: { isCancelled?: () => boolean; medium?: CoverMedium },
+    options?: { isCancelled?: () => boolean; medium?: CoverMedium; onSkippedLocked?: (bookId: number) => void },
   ): Promise<{ processed: number; updated: number }> {
     const event = 'book.bulk_reextract_cover';
     const startedAt = Date.now();
@@ -3171,6 +3171,7 @@ export class BookService {
         const unlocked = targets.filter(({ medium }) => !(medium === 'audio' ? audioCoverLockedBookIds : coverLockedBookIds).has(id));
         if (unlocked.length === 0) {
           skipped++;
+          options?.onSkippedLocked?.(id);
           continue;
         }
         processed++;
@@ -3203,6 +3204,25 @@ export class BookService {
     }
   }
 
+  /**
+   * Re-extracts one book's cover. When every slot it would write is locked it answers 409, as the
+   * other cover writes do, rather than `{ processed: 0, updated: 0 }`, which a client cannot tell
+   * from a file with no art. Nothing is written in that case, so the check can follow the run.
+   */
+  async reExtractCover(id: number, user: RequestUser, medium?: CoverMedium): Promise<{ processed: number; updated: number }> {
+    let skippedLocked = false;
+    const result = await this.bulkReExtractCover([id], user, undefined, {
+      medium,
+      onSkippedLocked: () => {
+        skippedLocked = true;
+      },
+    });
+    if (skippedLocked) {
+      await this.bookMetadataLockService.assertFieldsUnlocked(id, medium ? [medium === 'audio' ? 'audioCover' : 'cover'] : ['cover', 'audioCover']);
+    }
+    return result;
+  }
+
   private async resolveExportFileSize(file: ExportCandidateFile): Promise<number> {
     if (typeof file.sizeBytes === 'number' && Number.isFinite(file.sizeBytes) && file.sizeBytes >= 0) {
       return file.sizeBytes;
@@ -3569,7 +3589,9 @@ export class BookService {
 
   async updatePersonalNote(id: number, dto: UpdatePersonalNoteDto, user: RequestUser): Promise<BookDetailDto> {
     await this.verifyBookAccess(id, user);
-    await this.userBookNoteService.setNote(user.id, id, dto.note ?? null);
+    // A body without `note` leaves the note alone, as an omitted key does on the other partial
+    // routes; `null` clears it. The web and every iOS build send the key, null included.
+    if (dto.note !== undefined) await this.userBookNoteService.setNote(user.id, id, dto.note);
     return this.getDetail(id, user);
   }