diff --git a/server/src/modules/book/book.module.test.ts b/server/src/modules/book/book.module.test.ts index 61d461452..462fcf612 100644 --- a/server/src/modules/book/book.module.test.ts +++ b/server/src/modules/book/book.module.test.ts @@ -9,6 +9,7 @@ vi.mock('../metadata-fetch/metadata-fetch.module', () => ({ MetadataFetchModule: vi.mock('../user-book-note/user-book-note.module', () => ({ UserBookNoteModule: class UserBookNoteModule {} })); import { MODULE_METADATA } from '@nestjs/common/constants'; +import { SelfWriteRegistryModule } from '../../common/self-write-registry.module'; import { LibraryModule } from '../library/library.module'; import { BookQueryBuilder } from './book-query-builder.service'; @@ -24,6 +25,10 @@ import { AudiobookEbookProgressSyncService } from './audiobook-ebook-progress-sy import { AudiolessEpubService } from './audioless-epub.service'; describe('BookModule', () => { + it('imports the shared registry used by the file watcher', () => { + expect(Reflect.getMetadata(MODULE_METADATA.IMPORTS, BookModule)).toContain(SelfWriteRegistryModule); + }); + it('registers expected controller/providers/exports', () => { expect(Reflect.getMetadata('controllers', BookModule)).toEqual([BookController, ReadingAttemptController]); expect(Reflect.getMetadata('providers', BookModule)).toEqual([ diff --git a/server/src/modules/book/book.module.ts b/server/src/modules/book/book.module.ts index 84687f2e6..80bbcb989 100644 --- a/server/src/modules/book/book.module.ts +++ b/server/src/modules/book/book.module.ts @@ -1,4 +1,5 @@ import { Module, forwardRef } from '@nestjs/common'; +import { SelfWriteRegistryModule } from '../../common/self-write-registry.module'; import { AchievementModule } from '../achievement/achievement.module'; import { AppSettingsModule } from '../app-settings/app-settings.module'; @@ -36,6 +37,7 @@ import { ReadingAttemptController } from './reading-attempt.controller'; EmbeddingModule, MetadataFetchModule, FileWriteModule, + SelfWriteRegistryModule, AppSettingsModule, MetadataScoreModule, NarratorModule, diff --git a/server/src/modules/book/book.service.test.ts b/server/src/modules/book/book.service.test.ts index db70755a1..4b0d211c8 100644 --- a/server/src/modules/book/book.service.test.ts +++ b/server/src/modules/book/book.service.test.ts @@ -1,8 +1,12 @@ import { BadRequestException, ConflictException, ForbiddenException, InternalServerErrorException, Logger, NotFoundException } from '@nestjs/common'; import type { MockedFunction } from 'vitest'; import { rm, stat, rename } from 'fs/promises'; +import { tmpdir } from 'os'; +import { join } from 'path'; import type { RequestUser } from '../../common/types/request-user'; +import { SelfWriteRegistry } from '../../common/services/self-write-registry.service'; +import { FileWatcherService, WATCHER_DEBOUNCE_MS } from '../scanner/file-watcher.service'; import { AUDIO_BOOK_FILE_WRITE_FIELDS, MetadataProviderKey, Permission, type BookQuery, type MetadataFetchDiagnostics } from '@bookorbit/types'; import { extractEpubMetadata } from '../metadata/lib/epub'; import { extractAudioMetadata } from '../metadata/extractors/audio.extractor'; @@ -107,7 +111,7 @@ function makeMetadataFetchDiagnostics(overrides: Partial (key === 'storage.appDataPath' ? '/tmp/books' : undefined)), }; + if (overrides.appDataPath !== undefined) config.get.mockReturnValue(overrides.appDataPath); const appSettings = { getDownloadPattern: vi.fn().mockResolvedValue('{originalFilename}'), }; @@ -271,6 +276,7 @@ function makeService(overrides: { bookMetadataLockService?: unknown } = {}) { }; bookRepo.withTransaction.mockImplementation(async (callback: (tx: unknown) => Promise) => callback({})); + const selfWriteRegistry = new SelfWriteRegistry(); const service = new BookService( bookRepo as never, @@ -288,6 +294,7 @@ function makeService(overrides: { bookMetadataLockService?: unknown } = {}) { customMetadataService as never, (overrides.bookMetadataLockService ?? bookMetadataLockService) as never, coverStore as never, + selfWriteRegistry, embedder as never, fileWriteService as never, fileRenameService as never, @@ -315,6 +322,7 @@ function makeService(overrides: { bookMetadataLockService?: unknown } = {}) { customMetadataService, bookMetadataLockService, coverStore, + selfWriteRegistry, }; } @@ -2482,10 +2490,9 @@ describe('BookService', () => { expect(onProgress).toHaveBeenCalledTimes(2); }); - it('deleteBooks verifies access, removes book files, and removes cover directories without failing on rm errors', async () => { + it('removes every content file before deleting records, then cleans up covers', async () => { const { service, bookRepo, libraryService } = makeService(); const user = makeUser(); - const warnSpy = vi.spyOn((service as unknown as { logger: { warn: (message: string) => void } }).logger, 'warn').mockImplementation(); bookRepo.findLibraryIdsByBookIds.mockResolvedValue([ { id: 3, libraryId: 7 }, @@ -2500,7 +2507,18 @@ describe('BookService', () => { { id: 4, title: null }, ]); bookRepo.deleteByIdsAndInvalidateScanState.mockResolvedValue(undefined); - mockRm.mockRejectedValue(new Error('cannot delete')); + mockRm.mockImplementation((_path, options) => { + if (options?.recursive) { + expect(bookRepo.deleteByIdsAndInvalidateScanState).toHaveBeenCalledTimes(1); + } else { + expect(bookRepo.deleteByIdsAndInvalidateScanState).not.toHaveBeenCalled(); + } + return Promise.resolve(); + }); + bookRepo.deleteByIdsAndInvalidateScanState.mockImplementation(() => { + expect(mockRm).toHaveBeenCalledTimes(2); + return Promise.resolve(); + }); const result = await service.deleteBooks([3, 4], user); @@ -2510,7 +2528,6 @@ describe('BookService', () => { expect(mockRm).toHaveBeenCalledWith('/tmp/books/covers/4', { recursive: true, force: true }); expect(mockRm).toHaveBeenCalledWith('/tmp/library/book3.epub', { force: true }); expect(mockRm).toHaveBeenCalledWith('/tmp/library/book4.pdf', { force: true }); - expect(warnSpy).toHaveBeenCalled(); expect(result).toEqual({ total: 2, books: [ @@ -2521,6 +2538,285 @@ describe('BookService', () => { }); }); + it.each(['EACCES', 'EPERM', 'EROFS'])('keeps records and covers and returns an actionable error when removal fails with %s', async (code) => { + const { service, bookRepo, selfWriteRegistry } = makeService(); + bookRepo.findLibraryIdsByBookIds.mockResolvedValue([{ id: 3, libraryId: 7 }]); + bookRepo.findDeletionAuditBooksByIds.mockResolvedValue([{ id: 3, title: 'Dune' }]); + bookRepo.findAllFilesByBookIds.mockResolvedValue([{ bookId: 3, absolutePath: '/path/to/library/book.epub', format: 'epub' }]); + let suppressedDuringRemoval = false; + mockRm.mockImplementation(() => { + suppressedDuringRemoval = selfWriteRegistry.isSuppressed('/path/to/library/book.epub'); + return Promise.reject(Object.assign(new Error(`denied /path/to/library/book.epub: ${code}`), { code })); + }); + + const error = await service.deleteBooks([3], makeUser()).catch((err: unknown) => err); + + expect(error).toBeInstanceOf(InternalServerErrorException); + const response = (error as InternalServerErrorException).getResponse(); + expect(response).toMatchObject({ statusCode: 500, message: expect.stringContaining('library permissions or read-only mounts') }); + expect(JSON.stringify(response)).not.toContain('/path/to/library'); + expect(bookRepo.deleteByIdsAndInvalidateScanState).not.toHaveBeenCalled(); + expect(mockRm).toHaveBeenCalledTimes(1); + expect(mockRm).toHaveBeenCalledWith('/path/to/library/book.epub', { force: true }); + expect(suppressedDuringRemoval).toBe(true); + expect(selfWriteRegistry.isSuppressed('/path/to/library/book.epub')).toBe(false); + }); + + it.each([ + { delayedStep: 'filesystem', fails: false }, + { delayedStep: 'database', fails: false }, + { delayedStep: 'filesystem', fails: true }, + { delayedStep: 'database', fails: true }, + ])('defers watcher reconciliation during a delayed $delayedStep step (fails=$fails)', async ({ delayedStep, fails }) => { + vi.useFakeTimers(); + const { service, bookRepo, selfWriteRegistry } = makeService(); + const paths = ['/path/to/library/first.epub', '/path/to/library/second.epub']; + bookRepo.findLibraryIdsByBookIds.mockResolvedValue([ + { id: 3, libraryId: 7 }, + { id: 4, libraryId: 7 }, + ]); + bookRepo.findDeletionAuditBooksByIds.mockResolvedValue([]); + bookRepo.findAllFilesByBookIds.mockResolvedValue(paths.map((absolutePath, i) => ({ bookId: i + 3, absolutePath }))); + let recordsExist = true; + const processor = { + handleUnlink: vi.fn(() => Promise.resolve(recordsExist ? { type: 'book-missing', libraryId: 7, bookIds: [3] } : { type: 'noop' })), + handleUnlinkDir: vi.fn().mockResolvedValue({ type: 'noop' }), + }; + const scanner = { + bufferBookMissingEvent: vi.fn(), + bufferBooksUnavailableNotification: vi.fn(), + }; + const watcher = new FileWatcherService({} as never, processor as never, {} as never, scanner as never, selfWriteRegistry); + (watcher as any).subscriptions.set(7, []); + let release!: () => void; + const pending = new Promise((resolve) => { + release = resolve; + }); + let markStarted!: () => void; + const started = new Promise((resolve) => { + markStarted = resolve; + }); + const delay = async () => { + markStarted(); + await pending; + if (fails) throw new Error('delayed failure'); + }; + mockRm.mockImplementation(async (path, options) => { + if (options?.recursive) return; + if (path === paths[0]) (watcher as any).schedule('delete', path, 7); + if (path === paths[1] && delayedStep === 'filesystem') await delay(); + }); + bookRepo.deleteByIdsAndInvalidateScanState.mockImplementation(async () => { + if (delayedStep === 'database') await delay(); + recordsExist = false; + }); + const deletion = service.deleteBooks([3, 4], makeUser()).catch((error: unknown) => error); + try { + await started; + await vi.advanceTimersByTimeAsync(WATCHER_DEBOUNCE_MS * 2); + expect(paths.every((path) => selfWriteRegistry.isSuppressed(path))).toBe(true); + expect(selfWriteRegistry.isSuppressed('/path/to/library/unrelated.epub')).toBe(false); + expect(processor.handleUnlink).not.toHaveBeenCalled(); + expect(scanner.bufferBooksUnavailableNotification).not.toHaveBeenCalled(); + + release(); + const result = await deletion; + if (fails) { + expect(result).toBeInstanceOf(Error); + expect(mockRm.mock.calls.every(([, options]) => !options?.recursive)).toBe(true); + if (delayedStep === 'filesystem') expect(bookRepo.deleteByIdsAndInvalidateScanState).not.toHaveBeenCalled(); + } else { + expect(result).toMatchObject({ total: 2 }); + } + expect(paths.every((path) => !selfWriteRegistry.isSuppressed(path))).toBe(true); + + await vi.advanceTimersByTimeAsync(WATCHER_DEBOUNCE_MS); + expect(processor.handleUnlink).toHaveBeenCalledExactlyOnceWith(paths[0], 7); + if (fails) { + expect(scanner.bufferBookMissingEvent).toHaveBeenCalledExactlyOnceWith(7, [3]); + expect(scanner.bufferBooksUnavailableNotification).toHaveBeenCalledExactlyOnceWith(7, [3]); + } else { + expect(scanner.bufferBookMissingEvent).not.toHaveBeenCalled(); + expect(scanner.bufferBooksUnavailableNotification).not.toHaveBeenCalled(); + } + } finally { + release(); + await deletion; + await watcher.onModuleDestroy(); + vi.useRealTimers(); + } + }); + + it('waits for all in-flight file removals before reporting a failure', async () => { + const { service, bookRepo, selfWriteRegistry } = makeService(); + bookRepo.findLibraryIdsByBookIds.mockResolvedValue([{ id: 3, libraryId: 7 }]); + bookRepo.findDeletionAuditBooksByIds.mockResolvedValue([]); + bookRepo.findAllFilesByBookIds.mockResolvedValue([ + { bookId: 3, absolutePath: '/path/to/library/blocked.epub' }, + { bookId: 3, absolutePath: '/path/to/library/slow.pdf' }, + ]); + let finishRemoval!: () => void; + const slowRemoval = new Promise((resolve) => { + finishRemoval = resolve; + }); + let startedRemoval!: () => void; + const started = new Promise((resolve) => { + startedRemoval = resolve; + }); + mockRm.mockImplementation(async (path) => { + if (path === '/path/to/library/blocked.epub') throw new Error('denied'); + startedRemoval(); + await slowRemoval; + }); + let settled = false; + const deletion = service.deleteBooks([3], makeUser()).catch((err: unknown) => { + settled = true; + return err; + }); + await started; + await new Promise((resolve) => setImmediate(resolve)); + expect(settled).toBe(false); + expect(selfWriteRegistry.isSuppressed('/path/to/library/blocked.epub')).toBe(true); + expect(selfWriteRegistry.isSuppressed('/path/to/library/slow.pdf')).toBe(true); + expect(bookRepo.deleteByIdsAndInvalidateScanState).not.toHaveBeenCalled(); + finishRemoval(); + expect(await deletion).toBeInstanceOf(InternalServerErrorException); + expect(mockRm).toHaveBeenCalledTimes(2); + expect(bookRepo.deleteByIdsAndInvalidateScanState).not.toHaveBeenCalled(); + expect(selfWriteRegistry.isSuppressed('/path/to/library/blocked.epub')).toBe(false); + expect(selfWriteRegistry.isSuppressed('/path/to/library/slow.pdf')).toBe(false); + }); + + it('bounds concurrent file and cover removals for large batches', async () => { + const { service, bookRepo } = makeService(); + const ids = Array.from({ length: 40 }, (_, i) => i + 1); + bookRepo.findLibraryIdsByBookIds.mockResolvedValue(ids.map((id) => ({ id, libraryId: 7 }))); + bookRepo.findDeletionAuditBooksByIds.mockResolvedValue([]); + bookRepo.findAllFilesByBookIds.mockResolvedValue(ids.map((bookId) => ({ bookId, absolutePath: `/path/to/library/${bookId}.epub` }))); + let active = 0; + let peak = 0; + mockRm.mockImplementation(async () => { + active += 1; + peak = Math.max(peak, active); + await new Promise((resolve) => setImmediate(resolve)); + active -= 1; + }); + await service.deleteBooks(ids, makeUser()); + expect(peak).toBeGreaterThan(1); + expect(peak).toBeLessThanOrEqual(8); + expect(active).toBe(0); + expect(mockRm).toHaveBeenCalledTimes(80); + }); + + it.each(['filesystem', 'database'])('can retry after partial removal and a %s failure, including files already absent', async (failure) => { + const fs = await vi.importActual('fs/promises'); + const root = await fs.mkdtemp(join(tmpdir(), 'book-deletion-')); + try { + const { service, bookRepo } = makeService({ appDataPath: root }); + const first = join(root, 'first.epub'); + const second = join(root, 'second.pdf'); + const cover = join(root, 'covers', '3', 'cover.jpg'); + await fs.writeFile(first, 'epub'); + await fs.writeFile(second, 'pdf'); + await fs.mkdir(join(root, 'covers', '3'), { recursive: true }); + await fs.writeFile(cover, 'cover'); + bookRepo.findLibraryIdsByBookIds.mockResolvedValue([ + { id: 3, libraryId: 7 }, + { id: 4, libraryId: 7 }, + ]); + bookRepo.findDeletionAuditBooksByIds.mockResolvedValue([ + { id: 3, title: 'First' }, + { id: 4, title: 'Second' }, + ]); + bookRepo.findAllFilesByBookIds.mockResolvedValue([ + { bookId: 3, absolutePath: first }, + { bookId: 4, absolutePath: second }, + ]); + let fail = true; + mockRm.mockImplementation(async (path, options) => { + if (fail && failure === 'filesystem' && path === second) throw Object.assign(new Error('denied'), { code: 'EACCES' }); + await fs.rm(path, options); + }); + if (failure === 'database') bookRepo.deleteByIdsAndInvalidateScanState.mockRejectedValueOnce(new Error('db down')); + + await expect(service.deleteBooks([3, 4], makeUser())).rejects.toThrow(); + await expect(fs.stat(first)).rejects.toMatchObject({ code: 'ENOENT' }); + expect(await fs.readFile(cover, 'utf8')).toBe('cover'); + if (failure === 'filesystem') { + expect(await fs.readFile(second, 'utf8')).toBe('pdf'); + expect(bookRepo.deleteByIdsAndInvalidateScanState).not.toHaveBeenCalled(); + } + + fail = false; + await expect(service.deleteBooks([3, 4], makeUser())).resolves.toMatchObject({ total: 2 }); + await expect(fs.stat(second)).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(fs.stat(cover)).rejects.toMatchObject({ code: 'ENOENT' }); + expect(bookRepo.deleteByIdsAndInvalidateScanState).toHaveBeenCalledWith([3, 4]); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }); + + it('treats cover cleanup as best effort only after a successful database commit', async () => { + const { service, bookRepo } = makeService({ appDataPath: '/path/to/data' }); + bookRepo.findLibraryIdsByBookIds.mockResolvedValue([{ id: 3, libraryId: 7 }]); + bookRepo.findDeletionAuditBooksByIds.mockResolvedValue([{ id: 3, title: 'Dune' }]); + bookRepo.findAllFilesByBookIds.mockResolvedValue([{ bookId: 3, absolutePath: '/path/to/library/book.epub' }]); + mockRm.mockImplementation((_path, options) => { + if (options?.recursive) { + expect(bookRepo.deleteByIdsAndInvalidateScanState).toHaveBeenCalledWith([3]); + throw new Error('cover cleanup denied'); + } + return Promise.resolve(); + }); + await expect(service.deleteBooks([3], makeUser())).resolves.toEqual({ total: 1, books: [{ id: 3, title: 'Dune' }], omitted: 0 }); + expect(mockRm).toHaveBeenCalledWith('/path/to/data/covers/3', { recursive: true, force: true }); + }); + + it('does not mutate any files or records when library access is denied', async () => { + const { service, bookRepo, libraryService } = makeService(); + bookRepo.findLibraryIdsByBookIds.mockResolvedValue([ + { id: 3, libraryId: 7 }, + { id: 4, libraryId: 9 }, + ]); + libraryService.verifyUserAccess.mockRejectedValue(new ForbiddenException()); + await expect(service.deleteBooks([3, 4], makeUser())).rejects.toBeInstanceOf(ForbiddenException); + expect(bookRepo.findAllFilesByBookIds).not.toHaveBeenCalled(); + expect(bookRepo.deleteByIdsAndInvalidateScanState).not.toHaveBeenCalled(); + expect(mockRm).not.toHaveBeenCalled(); + }); + + it('does not mutate an existing book when another selected book is missing', async () => { + const { service, bookRepo } = makeService(); + bookRepo.findLibraryIdsByBookIds.mockResolvedValue([{ id: 3, libraryId: 7 }]); + await expect(service.deleteBooks([3, 4], makeUser())).rejects.toBeInstanceOf(NotFoundException); + expect(bookRepo.findAllFilesByBookIds).not.toHaveBeenCalled(); + expect(bookRepo.deleteByIdsAndInvalidateScanState).not.toHaveBeenCalled(); + expect(mockRm).not.toHaveBeenCalled(); + }); + + it('deduplicates book ids and shared file paths', async () => { + const { service, bookRepo } = makeService(); + bookRepo.findLibraryIdsByBookIds.mockResolvedValue([{ id: 3, libraryId: 7 }]); + bookRepo.findDeletionAuditBooksByIds.mockResolvedValue([{ id: 3, title: 'Dune' }]); + bookRepo.findAllFilesByBookIds.mockResolvedValue([ + { bookId: 3, absolutePath: '/path/to/library/book.epub' }, + { bookId: 3, absolutePath: '/path/to/library/book.epub' }, + ]); + await expect(service.deleteBooks([3, 3], makeUser())).resolves.toMatchObject({ total: 1 }); + expect(bookRepo.findAllFilesByBookIds).toHaveBeenCalledWith([3]); + expect(bookRepo.deleteByIdsAndInvalidateScanState).toHaveBeenCalledWith([3]); + expect(mockRm).toHaveBeenCalledTimes(2); + }); + + it('returns an empty deletion without filesystem or database work', async () => { + const { service, bookRepo } = makeService(); + await expect(service.deleteBooks([], makeUser())).resolves.toEqual({ total: 0, books: [], omitted: 0 }); + expect(bookRepo.findLibraryIdsByBookIds).not.toHaveBeenCalled(); + expect(bookRepo.deleteByIdsAndInvalidateScanState).not.toHaveBeenCalled(); + expect(mockRm).not.toHaveBeenCalled(); + }); + it('caps deletion audit book details while preserving the deletion count', async () => { const { service, bookRepo } = makeService(); const user = makeUser(); @@ -5513,6 +5809,7 @@ describe('BookService', () => { customMetadataService, bookMetadataLockService, coverStore, + selfWriteRegistry, embedder, fileRenameService, achievementEvents, @@ -5536,6 +5833,7 @@ describe('BookService', () => { customMetadataService as never, bookMetadataLockService as never, coverStore as never, + selfWriteRegistry, embedder as never, null as never, fileRenameService as never, @@ -5565,6 +5863,7 @@ describe('BookService', () => { customMetadataService, bookMetadataLockService, coverStore, + selfWriteRegistry, embedder, fileWriteService, achievementEvents, @@ -5588,6 +5887,7 @@ describe('BookService', () => { customMetadataService as never, bookMetadataLockService as never, coverStore as never, + selfWriteRegistry, embedder as never, fileWriteService as never, null as never, diff --git a/server/src/modules/book/book.service.ts b/server/src/modules/book/book.service.ts index 61fdd5d60..668dec197 100644 --- a/server/src/modules/book/book.service.ts +++ b/server/src/modules/book/book.service.ts @@ -16,6 +16,8 @@ import { inArray, type SQL } from 'drizzle-orm'; import { MAX_BOOK_QUERY_OFFSET_ROWS, isBookQueryOffsetWithinLimit } from '../../common/constants/pagination.constants'; import { compareAudioTracks, coverFetchInputs, resolveIsAudiobook } from '../../common/utils/book-media.utils'; import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils'; +import { mapWithConcurrency } from '../../common/utils/batch.utils'; +import { SelfWriteRegistry } from '../../common/services/self-write-registry.service'; import { selectPrimaryFile } from '../../common/utils/primary-file-selection.utils'; import { normalizeMetadataText, normalizeMetadataTextKey } from '../../common/utils/metadata-text-normalize.utils'; import { naturalCompare } from '../../common/utils/natural-sort.utils'; @@ -175,6 +177,7 @@ const EXPORT_LIMITS = { } as const; const MAX_DELETION_AUDIT_BOOKS = 25; +const BOOK_DELETION_CONCURRENCY = 8; type ExportCandidateFile = { bookId: number; @@ -293,6 +296,7 @@ export class BookService { private readonly customMetadataService: CustomMetadataService, private readonly bookMetadataLockService: BookMetadataLockService, private readonly coverStore: BookCoverStore, + private readonly selfWriteRegistry: SelfWriteRegistry, @Optional() private readonly embedder: BookEmbedderService, @Optional() private readonly fileWriteService: FileWriteService, @Optional() private readonly fileRenameService: FileRenameService, @@ -1635,31 +1639,41 @@ export class BookService { const row = auditRowsById.get(bookId); return row ? [row] : []; }); - const files = await this.bookRepo.findAllFilesByBookIds(bookIds); - await this.bookRepo.deleteByIdsAndInvalidateScanState(deletedBookIds); - const deleteTargets = [ - ...rows.map((row) => ({ - path: join(this.appDataPath, 'covers', String(row.id)), - options: { recursive: true, force: true }, - kind: 'coverDir' as const, - })), - ...files.map((file) => ({ path: file.absolutePath, options: { force: true }, kind: 'bookFile' as const })), - ]; - const deleteResults = await Promise.allSettled(deleteTargets.map((target) => rm(target.path, target.options))); - let failedDeletes = 0; - for (let i = 0; i < deleteResults.length; i += 1) { - const result = deleteResults[i]; - if (result?.status !== 'rejected') continue; - failedDeletes += 1; - const target = deleteTargets[i]!; - const reason = result.reason; - const errorClass = reason instanceof Error ? reason.name : 'Error'; - const errorMessage = sanitizeLogValue(reason instanceof Error ? reason.message : String(reason)); - const pathValue = sanitizeLogValue(target.path); - this.logger.warn( - `[${event}] [fail] userId=${user.id} path="${pathValue}" kind=${target.kind} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - delete books cleanup target failed`, - ); - } + const files = await this.bookRepo.findAllFilesByBookIds(deletedBookIds); + const removeTarget = async (path: string, kind: 'bookFile' | 'coverDir'): Promise => { + try { + await rm(path, kind === 'coverDir' ? { recursive: true, force: true } : { force: true }); + return true; + } catch (err) { + const errorClass = err instanceof Error ? err.name : 'Error'; + const errorMessage = sanitizeLogValue(err instanceof Error ? err.message : String(err)); + this.logger.warn( + `[${event}] [fail] userId=${user.id} path="${sanitizeLogValue(path)}" kind=${kind} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - delete books cleanup target failed`, + ); + return false; + } + }; + + // A surviving content file would be imported again by the scanner. Keep all records and + // user state on failure; force:true lets a retry finish after a partially removed batch. + const paths = [...new Set(files.map((file) => file.absolutePath))]; + // Defer watcher reconciliation until the records reflect the intentional removal. + await this.selfWriteRegistry.track(paths, async () => { + const fileResults = await mapWithConcurrency(paths, BOOK_DELETION_CONCURRENCY, (path) => removeTarget(path, 'bookFile')); + if (fileResults.some((removed) => !removed)) { + throw new InternalServerErrorException( + "Could not delete all book files. Book records were kept, but some files may already be removed. Check the server's library permissions or read-only mounts, then retry.", + ); + } + + await this.bookRepo.deleteByIdsAndInvalidateScanState(deletedBookIds); + }); + // Cover cleanup cannot resurrect a book and must not turn a committed deletion into a + // failure. Leave covers alone until the database transaction has succeeded. + const coverResults = await mapWithConcurrency(deletedBookIds, BOOK_DELETION_CONCURRENCY, (bookId) => + removeTarget(join(this.appDataPath, 'covers', String(bookId)), 'coverDir'), + ); + const failedDeletes = coverResults.filter((removed) => !removed).length; this.logger.log( `[${event}] [end] count=${bookIds.length} durationMs=${Date.now() - startedAt} deletedBooks=${rows.length} deletedFiles=${files.length} failedDeletes=${failedDeletes} - delete books completed`, );