fix(library): only process books marked for rename in bulk rename

The bulk rename job was incorrectly iterating over every book in the library instead of restricting the operation to books that actually require a rename. This caused unnecessary processing and logged excessive noise. We now compute the preview first and filter the list of candidate book IDs down to those with the 'will_rename' status before executing the rename loop.

Closes #361
This commit is contained in:
neon
2026-06-17 22:14:49 +00:00
parent ceb2bb72d6
commit 65f4fa7fca
16 changed files with 351 additions and 89 deletions
@@ -1,7 +1,7 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
vi.mock('vue-sonner', () => ({ toast: { success: vi.fn() } }))
vi.mock('canvas-confetti', () => ({ default: vi.fn() }))
vi.mock('vue-sonner', () => ({ toast: { success: vi.fn<(...args: unknown[]) => unknown>() } }))
vi.mock('canvas-confetti', () => ({ default: vi.fn<(...args: unknown[]) => unknown>() }))
import { toast } from 'vue-sonner'
import confetti from 'canvas-confetti'
@@ -15,7 +15,7 @@ import {
MAX_AUTHOR_IMAGE_BYTES,
} from './author'
vi.mock('@/lib/api', () => ({ api: vi.fn() }))
vi.mock('@/lib/api', () => ({ api: vi.fn<(...args: unknown[]) => unknown>() }))
const mockedApi = vi.mocked(api)
@@ -30,7 +30,7 @@ function mockErrorResponse(status = 500) {
function mockStreamResponse(lines: string[]) {
let index = 0
const reader = {
read: vi.fn().mockImplementation(() => {
read: vi.fn<(...args: unknown[]) => unknown>().mockImplementation(() => {
if (index < lines.length) {
const value = new TextEncoder().encode(lines[index++] + '\n')
return Promise.resolve({ done: false, value })
@@ -38,7 +38,7 @@ function mockStreamResponse(lines: string[]) {
return Promise.resolve({ done: true, value: undefined })
}),
}
return { ok: true, status: 200, body: { getReader: () => reader }, json: vi.fn() } as unknown as Response
return { ok: true, status: 200, body: { getReader: () => reader }, json: vi.fn<(...args: unknown[]) => unknown>() } as unknown as Response
}
beforeEach(() => {
@@ -273,12 +273,12 @@ describe('fetchAuthorMetadataCandidates', () => {
it('includes optional params in the URL', async () => {
mockedApi.mockResolvedValue(mockOkResponse([]))
await fetchAuthorMetadataCandidates({ q: 'tolkien', region: 'en', limit: 5, providers: ['openlibrary', 'musicbrainz'] })
await fetchAuthorMetadataCandidates({ q: 'tolkien', region: 'en', limit: 5, providers: ['audnexus'] })
const url = mockedApi.mock.calls[0][0] as string
expect(url).toContain('region=en')
expect(url).toContain('limit=5')
expect(url).toContain('providers=openlibrary%2Cmusicbrainz')
expect(url).toContain('providers=audnexus')
})
it('throws on non-ok response', async () => {
@@ -314,7 +314,7 @@ describe('bulkRefreshAuthorsMetadata', () => {
]
mockedApi.mockResolvedValue(mockStreamResponse(lines))
const onProgress = vi.fn()
const onProgress = vi.fn<(...args: unknown[]) => unknown>()
await bulkRefreshAuthorsMetadata([10, 11], onProgress)
expect(onProgress).toHaveBeenCalledTimes(2)
@@ -166,8 +166,8 @@ describe('mergeBookCardWithDetail', () => {
const detail = makeDetail({
audioMetadata: {
narrators: [
{ id: 1, name: 'Narrator One' },
{ id: 2, name: 'Narrator Two' },
{ id: 1, name: 'Narrator One', sortName: null, displayOrder: 0 },
{ id: 2, name: 'Narrator Two', sortName: null, displayOrder: 1 },
],
durationSeconds: 3600,
abridged: false,
@@ -187,7 +187,7 @@ describe('mergeBookCardWithDetail', () => {
it('returns empty narrators array when audioMetadata is undefined', () => {
const detail = makeDetail()
;(detail as BookDetail & { audioMetadata: undefined }).audioMetadata = undefined as unknown as null
;(detail as { audioMetadata: unknown }).audioMetadata = undefined
const result = mergeBookCardWithDetail(makeBook(), detail)
expect(result.narrators).toEqual([])
})
@@ -2,19 +2,19 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'
import type { NotificationItem } from '@bookorbit/types'
import { NotificationType } from '@bookorbit/types'
const mockApi = vi.fn()
const mockGetAccessToken = vi.fn().mockReturnValue('token')
const mockApi = vi.fn<(...args: unknown[]) => unknown>()
const mockGetAccessToken = vi.fn<(...args: unknown[]) => unknown>().mockReturnValue('token')
vi.mock('@/lib/api', () => ({
api: (...args: unknown[]) => mockApi(...args),
getAccessToken: () => mockGetAccessToken(),
}))
const mockIo = vi.fn()
const mockIo = vi.fn<(...args: unknown[]) => unknown>()
vi.mock('socket.io-client', () => ({
io: (...args: unknown[]) => mockIo(...args),
}))
const mockShowAchievementToast = vi.fn()
const mockShowAchievementToast = vi.fn<(...args: unknown[]) => unknown>()
vi.mock('@/features/achievements/utils/achievementToast', () => ({
showAchievementToast: (...args: unknown[]) => mockShowAchievementToast(...args),
}))
@@ -43,10 +43,10 @@ function makeNotification(overrides: Partial<NotificationItem> = {}): Notificati
function makeMockSocket() {
return {
on: vi.fn(),
off: vi.fn(),
disconnect: vi.fn(),
emit: vi.fn(),
on: vi.fn<(...args: unknown[]) => unknown>(),
off: vi.fn<(...args: unknown[]) => unknown>(),
disconnect: vi.fn<(...args: unknown[]) => unknown>(),
emit: vi.fn<(...args: unknown[]) => unknown>(),
connected: false,
}
}
@@ -91,7 +91,7 @@ describe('useNotifications', () => {
const { subscribe } = useNotifications()
subscribe()
const notificationNewHandler = mockSocket.on.mock.calls.find(([event]: [string]) => event === 'notification:new')?.[1] as
const notificationNewHandler = mockSocket.on.mock.calls.find(([event]: unknown[]) => event === 'notification:new')?.[1] as
| ((item: NotificationItem) => void)
| undefined
@@ -116,7 +116,7 @@ describe('useNotifications', () => {
const { subscribe } = useNotifications()
subscribe()
const notificationNewHandler = mockSocket.on.mock.calls.find(([event]: [string]) => event === 'notification:new')?.[1] as (
const notificationNewHandler = mockSocket.on.mock.calls.find(([event]: unknown[]) => event === 'notification:new')?.[1] as (
item: NotificationItem,
) => void
@@ -137,7 +137,7 @@ describe('useNotifications', () => {
const { subscribe } = useNotifications()
subscribe()
const notificationNewHandler = mockSocket.on.mock.calls.find(([event]: [string]) => event === 'notification:new')?.[1] as (
const notificationNewHandler = mockSocket.on.mock.calls.find(([event]: unknown[]) => event === 'notification:new')?.[1] as (
item: NotificationItem,
) => void
@@ -161,7 +161,7 @@ describe('useNotifications', () => {
const useNotifications = await loadModule()
const { subscribe } = useNotifications()
subscribe()
return mockSocket.on.mock.calls.find(([event]: [string]) => event === 'notification:new')?.[1] as (item: NotificationItem) => void
return mockSocket.on.mock.calls.find(([event]: unknown[]) => event === 'notification:new')?.[1] as (item: NotificationItem) => void
}
it('uses message as name when present', async () => {
@@ -1,7 +1,7 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
import { ref } from 'vue'
vi.mock('@/lib/api', () => ({ api: vi.fn() }))
vi.mock('@/lib/api', () => ({ api: vi.fn<(...args: unknown[]) => unknown>() }))
import { api } from '@/lib/api'
import { formatTimeRemaining, useReaderProgress } from './useReaderProgress'
@@ -2,13 +2,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'
import { ref } from 'vue'
import { CBX_READER_DEFAULTS, READER_GROUP_DEFAULTS } from '@bookorbit/types'
const apiMock = vi.hoisted(() => vi.fn())
const apiMock = vi.hoisted(() => vi.fn<(...args: unknown[]) => Promise<unknown>>())
vi.mock('@/lib/api', () => ({ api: apiMock }))
const toastMock = vi.hoisted(() => ({ success: vi.fn() }))
const toastMock = vi.hoisted(() => ({ success: vi.fn<(...args: unknown[]) => unknown>() }))
vi.mock('vue-sonner', () => ({ toast: toastMock }))
const useAuthMock = vi.hoisted(() => vi.fn(() => ({ user: ref(null) })))
const useAuthMock = vi.hoisted(() => vi.fn<(...args: unknown[]) => unknown>(() => ({ user: ref(null) })))
vi.mock('@/features/auth/composables/useAuth', () => ({ useAuth: useAuthMock }))
import { useReaderSettings, useReaderDefaultSettings } from './useReaderSettings'
@@ -3,12 +3,12 @@ import { ref } from 'vue'
import type { BookCard, ScanBooksAddedEvent } from '@bookorbit/types'
const mockSocket = {
on: vi.fn(),
off: vi.fn(),
on: vi.fn<(...args: unknown[]) => unknown>(),
off: vi.fn<(...args: unknown[]) => unknown>(),
}
vi.mock('./useScanProgress', () => ({
getSocket: vi.fn(() => mockSocket),
getSocket: vi.fn<(...args: unknown[]) => unknown>(() => mockSocket),
}))
import { useLiveScanBooks } from './useLiveScanBooks'
@@ -36,7 +36,7 @@ function makeBook(overrides: Partial<BookCard> = {}): BookCard {
}
function captureHandler(): (event: ScanBooksAddedEvent) => void {
const call = mockSocket.on.mock.calls.find(([event]: [string]) => event === 'scan:books:added')
const call = mockSocket.on.mock.calls.find(([event]: unknown[]) => event === 'scan:books:added')
const handler = call?.[1] as ((event: ScanBooksAddedEvent) => void) | undefined
if (!handler) throw new Error('scan:books:added handler not registered')
return handler
@@ -46,7 +46,7 @@ describe('useLiveScanBooks', () => {
beforeEach(() => {
vi.useFakeTimers()
vi.clearAllMocks()
mockGetSocket.mockReturnValue(mockSocket as ReturnType<typeof getSocket>)
mockGetSocket.mockReturnValue(mockSocket as unknown as ReturnType<typeof getSocket>)
})
afterEach(() => {
@@ -178,7 +178,7 @@ describe('useLiveScanBooks', () => {
expect(existingBooks.value).toHaveLength(1)
vi.clearAllMocks()
mockGetSocket.mockReturnValue(mockSocket as ReturnType<typeof getSocket>)
mockGetSocket.mockReturnValue(mockSocket as unknown as ReturnType<typeof getSocket>)
const { start: start2, newBookIds: newBookIds2 } = useLiveScanBooks(libraryId, existingBooks, total)
start2()
@@ -213,7 +213,7 @@ describe('useLiveScanBooks', () => {
start()
start()
const calls = mockSocket.on.mock.calls.filter(([event]: [string]) => event === 'scan:books:added')
const calls = mockSocket.on.mock.calls.filter(([event]: unknown[]) => event === 'scan:books:added')
expect(calls).toHaveLength(1)
})
@@ -37,7 +37,7 @@ import {
} from './statistics.api'
import type { StatisticsFilterConfig } from '@bookorbit/types'
vi.mock('@/lib/api', () => ({ api: vi.fn() }))
vi.mock('@/lib/api', () => ({ api: vi.fn<(...args: unknown[]) => unknown>() }))
const mockedApi = vi.mocked(api)
@@ -51,8 +51,8 @@ function mockErrorResponse(status = 500) {
const filters: StatisticsFilterConfig = {
libraryIds: [1, 2],
booksOverTimeGranularity: 'month',
booksOverTimeRange: '1y',
booksOverTimeGranularity: 'monthly',
booksOverTimeRange: 'last-year',
}
beforeEach(() => {
@@ -111,8 +111,8 @@ describe('fetchBooksAddedOverTime', () => {
const url = mockedApi.mock.calls[0][0] as string
expect(url).toContain('/api/v1/statistics/books-added-over-time')
expect(url).toContain('granularity=month')
expect(url).toContain('range=1y')
expect(url).toContain('granularity=monthly')
expect(url).toContain('range=last-year')
expect(result).toEqual(data)
})
@@ -16,7 +16,7 @@ import {
getEntityInfo,
} from './entity-manager'
vi.mock('@/lib/api', () => ({ api: vi.fn() }))
vi.mock('@/lib/api', () => ({ api: vi.fn<(...args: unknown[]) => unknown>() }))
const mockedApi = vi.mocked(api)
@@ -2,19 +2,19 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
import type { BrowseEntityItem, DuplicateCluster, DuplicateScanStatus, DismissedPairInfo } from '@bookorbit/types'
vi.mock('../api/entity-manager', () => ({
getDuplicateScanStatus: vi.fn(),
refreshDuplicates: vi.fn(),
browseEntities: vi.fn(),
mergeEntities: vi.fn(),
renameEntity: vi.fn(),
deleteEntity: vi.fn(),
bulkDeleteEntities: vi.fn(),
splitEntity: vi.fn(),
scanDuplicates: vi.fn(),
dismissPair: vi.fn(),
undismissPair: vi.fn(),
getDismissedPairs: vi.fn(),
getEntityInfo: vi.fn(),
getDuplicateScanStatus: vi.fn<(...args: unknown[]) => unknown>(),
refreshDuplicates: vi.fn<(...args: unknown[]) => unknown>(),
browseEntities: vi.fn<(...args: unknown[]) => unknown>(),
mergeEntities: vi.fn<(...args: unknown[]) => unknown>(),
renameEntity: vi.fn<(...args: unknown[]) => unknown>(),
deleteEntity: vi.fn<(...args: unknown[]) => unknown>(),
bulkDeleteEntities: vi.fn<(...args: unknown[]) => unknown>(),
splitEntity: vi.fn<(...args: unknown[]) => unknown>(),
scanDuplicates: vi.fn<(...args: unknown[]) => unknown>(),
dismissPair: vi.fn<(...args: unknown[]) => unknown>(),
undismissPair: vi.fn<(...args: unknown[]) => unknown>(),
getDismissedPairs: vi.fn<(...args: unknown[]) => unknown>(),
getEntityInfo: vi.fn<(...args: unknown[]) => unknown>(),
}))
import * as entityManagerApi from '../api/entity-manager'
@@ -113,7 +113,6 @@ describe('useEntityManager', () => {
})
it('triggers scan when status transitions from computing to done', async () => {
const { scanDuplicates: mockScan } = entityManagerApi
mockGetDuplicateScanStatus.mockResolvedValueOnce(makeStatus('computing'))
vi.mocked(entityManagerApi.scanDuplicates).mockResolvedValue({
clusters: [],
+1 -1
View File
@@ -1,7 +1,7 @@
{
"extends": "@vue/tsconfig/tsconfig.dom.json",
"include": ["env.d.ts", "src/**/*", "src/**/*.vue"],
"exclude": ["src/**/__tests__/*"],
"exclude": ["src/**/__tests__/*", "src/**/*.test.ts", "src/**/*.spec.ts"],
"compilerOptions": {
// Extra safety for array and object lookups, but may have false positives.
"noUncheckedIndexedAccess": true,
+4 -3
View File
@@ -3,13 +3,14 @@
// Override to include only test files and clear exclusions.
// Application code imported in tests is automatically included via module resolution.
"include": ["src/**/__tests__/*", "env.d.ts"],
"include": ["src/**/__tests__/*", "src/**/*.test.ts", "src/**/*.spec.ts", "env.d.ts"],
"exclude": [],
"compilerOptions": {
// Vitest runs in a different environment than the application code.
// Adjust lib and types accordingly.
"lib": [],
// Adjust lib and types accordingly. ES2022 matches the app's target so
// newer built-ins (e.g. Array.prototype.at) are available in tests.
"lib": ["ES2022"],
"types": ["node", "jsdom"],
// noUncheckedIndexedAccess produces false positives in tests where the
@@ -0,0 +1,184 @@
import { drizzle } from 'drizzle-orm/node-postgres';
import * as schema from '../../db/schema';
import { BulkRenameRepository } from './bulk-rename.repository';
describe('BulkRenameRepository', () => {
function queryChain(rows: unknown) {
return {
from: vi.fn().mockReturnThis(),
innerJoin: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
orderBy: vi.fn().mockResolvedValue(rows),
};
}
function subqueryChain() {
return {
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
};
}
describe('findAllBooksForLibrary', () => {
it('maps book rows and groups authors by book', async () => {
const bookRows = [
{
bookId: 1,
absolutePath: '/lib/old/a.epub',
relPath: 'old/a.epub',
format: 'epub',
libraryFolderPath: '/lib',
organizationMode: 'book_per_file',
fileNamingPattern: '{title}',
bookFolderPath: '/lib/old/a.epub',
title: 'A',
subtitle: null,
publisher: null,
language: 'en',
isbn13: null,
publishedYear: 2001,
seriesName: null,
seriesIndex: null,
},
{
bookId: 2,
absolutePath: '/lib/old/b.epub',
relPath: 'old/b.epub',
format: 'epub',
libraryFolderPath: '/lib',
organizationMode: 'book_per_file',
fileNamingPattern: '{title}',
bookFolderPath: '/lib/old/b.epub',
title: 'B',
subtitle: null,
publisher: null,
language: null,
isbn13: null,
publishedYear: null,
seriesName: null,
seriesIndex: null,
},
];
const authorRows = [
{ bookId: 1, name: 'Author One' },
{ bookId: 1, name: 'Author Two' },
{ bookId: 2, name: 'Author Three' },
];
const db = {
select: vi.fn().mockReturnValueOnce(queryChain(bookRows)).mockReturnValueOnce(queryChain(authorRows)).mockReturnValue(subqueryChain()),
};
const repo = new BulkRenameRepository(db as never);
const result = await repo.findAllBooksForLibrary(7);
expect(result).toHaveLength(2);
expect(result[0]).toMatchObject({
bookId: 1,
title: 'A',
absolutePath: '/lib/old/a.epub',
authors: ['Author One', 'Author Two'],
metadata: { language: 'en', publishedYear: 2001 },
});
expect(result[1]).toMatchObject({ bookId: 2, authors: ['Author Three'] });
});
it('returns books with empty author lists when none are linked', async () => {
const bookRows = [
{
bookId: 1,
absolutePath: '/lib/old/a.epub',
relPath: 'old/a.epub',
format: 'epub',
libraryFolderPath: '/lib',
organizationMode: 'book_per_file',
fileNamingPattern: '{title}',
bookFolderPath: '/lib/old/a.epub',
title: 'A',
subtitle: null,
publisher: null,
language: null,
isbn13: null,
publishedYear: null,
seriesName: null,
seriesIndex: null,
},
];
const db = {
select: vi.fn().mockReturnValueOnce(queryChain(bookRows)).mockReturnValueOnce(queryChain([])).mockReturnValue(subqueryChain()),
};
const repo = new BulkRenameRepository(db as never);
const result = await repo.findAllBooksForLibrary(7);
expect(result).toHaveLength(1);
expect(result[0].authors).toEqual([]);
});
it('returns an empty array and skips the author query when the library has no books', async () => {
const db = { select: vi.fn().mockReturnValueOnce(queryChain([])) };
const repo = new BulkRenameRepository(db as never);
const result = await repo.findAllBooksForLibrary(7);
expect(result).toEqual([]);
expect(db.select).toHaveBeenCalledTimes(1);
});
it('fetches authors via a library subquery, never a per-book parameter list (issue #361)', async () => {
const calls: Array<{ text: string; params: unknown[] }> = [];
const fakeClient = {
query: vi.fn().mockImplementation((cfg: { text: string }, params: unknown[]) => {
calls.push({ text: cfg.text, params });
// The first (books) query must return at least one row so the author query runs.
return Promise.resolve({ rows: calls.length === 1 ? [new Array(16).fill(null)] : [] });
}),
};
const db = drizzle({ client: fakeClient as never, schema });
const repo = new BulkRenameRepository(db as never);
await repo.findAllBooksForLibrary(42);
expect(calls).toHaveLength(2);
const authorQuery = calls[1];
// The ONLY bind parameter is the libraryId - not one entry per book - so the statement can
// never exceed PostgreSQL's 65535-parameter wire-protocol limit, regardless of library size.
expect(authorQuery.params).toEqual([42]);
const sqlText = authorQuery.text.toLowerCase();
expect(sqlText).toContain('in (select');
expect(sqlText).toContain('"library_id"');
});
});
describe('findLibrarySettings', () => {
function settingsChain(rows: unknown) {
return {
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue(rows),
};
}
it('returns the settings row when the library exists', async () => {
const row = { fileRenameEnabled: true, fileNamingPattern: '{title}', organizationMode: 'book_per_file', watch: false };
const db = { select: vi.fn().mockReturnValue(settingsChain([row])) };
const repo = new BulkRenameRepository(db as never);
await expect(repo.findLibrarySettings(1)).resolves.toEqual(row);
});
it('returns null when the library is missing', async () => {
const db = { select: vi.fn().mockReturnValue(settingsChain([])) };
const repo = new BulkRenameRepository(db as never);
await expect(repo.findLibrarySettings(999)).resolves.toBeNull();
});
});
});
@@ -63,8 +63,7 @@ export class BulkRenameRepository {
.where(eq(books.libraryId, libraryId))
.orderBy(asc(books.id));
const bookIds = rows.map((r) => r.bookId);
if (bookIds.length === 0) return [];
if (rows.length === 0) return [];
const authorRows = await this.db
.select({
@@ -73,7 +72,7 @@ export class BulkRenameRepository {
})
.from(bookAuthors)
.innerJoin(authors, eq(authors.id, bookAuthors.authorId))
.where(inArray(bookAuthors.bookId, bookIds))
.where(inArray(bookAuthors.bookId, this.db.select({ id: books.id }).from(books).where(eq(books.libraryId, libraryId))))
.orderBy(asc(bookAuthors.bookId), asc(bookAuthors.displayOrder));
const authorsByBook = new Map<number, string[]>();
@@ -129,9 +128,4 @@ export class BulkRenameRepository {
return row ?? null;
}
async findLibraryBookIds(libraryId: number): Promise<number[]> {
const rows = await this.db.select({ id: books.id }).from(books).where(eq(books.libraryId, libraryId)).orderBy(asc(books.id));
return rows.map((r) => r.id);
}
}
@@ -30,13 +30,19 @@ function makeBookData(overrides: Partial<BulkRenameBookData> & { bookId?: number
};
}
// Under the default '{title}' pattern, a book whose file sits in /library/folder/* resolves to
// /library/<title>.* and therefore always lands in the will_rename bucket. Distinct titles keep
// each new path unique so they are never reclassified as collisions.
function willRenameBook(id: number): BulkRenameBookData {
return makeBookData({ bookId: id, title: `Book ${id}`, absolutePath: `/library/folder/book-${id}.epub` });
}
describe('BulkRenameService', () => {
let service: BulkRenameService;
const bulkRenameRepo = {
findAllBooksForLibrary: vi.fn(),
findLibrarySettings: vi.fn(),
findLibraryBookIds: vi.fn(),
};
const fileRenameRepo = {
@@ -411,9 +417,9 @@ describe('BulkRenameService', () => {
watch: false,
};
it('renames all books sequentially and reports summary', async () => {
it('renames only the will_rename candidates and reports summary', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1, 2, 3]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1), willRenameBook(2), willRenameBook(3)]);
const results: FileRenameResult[] = [
{ status: 'success', durationMs: 10 },
@@ -439,9 +445,63 @@ describe('BulkRenameService', () => {
expect(fileRenameService.performRename).toHaveBeenCalledTimes(3);
});
it('only renames will_rename books, never the unchanged or collision ones', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
const willRename = willRenameBook(1);
// Already at its target path under the '{title}' pattern -> unchanged.
const unchanged = makeBookData({ bookId: 2, title: 'Settled', absolutePath: '/library/Settled.epub' });
// Two books resolving to /library/Same.epub -> both collision.
const collideA = makeBookData({ bookId: 3, title: 'Same', absolutePath: '/library/folder/a.epub' });
const collideB = makeBookData({ bookId: 4, title: 'Same', absolutePath: '/library/folder/b.epub' });
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRename, unchanged, collideA, collideB]);
fileRenameService.performRename.mockResolvedValue({ status: 'success', durationMs: 10 });
const events: any[] = [];
const summary = await service.execute(1, 42, {
onProgress: (e) => events.push(e),
isCancelled: () => false,
});
expect(fileRenameService.performRename).toHaveBeenCalledTimes(1);
expect(fileRenameService.performRename).toHaveBeenCalledWith(1, 42, false, true);
expect(events).toEqual([{ bookId: 1, status: 'success', reason: undefined }]);
expect(summary.processed).toBe(1);
expect(summary.succeeded).toBe(1);
});
it('suppresses per-book notifications during bulk rename', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
fileRenameService.performRename.mockResolvedValue({ status: 'success', durationMs: 10 });
await service.execute(1, 42, {
onProgress: () => {},
isCancelled: () => false,
});
// performRename(bookId, userId, force=false, suppressNotification=true)
expect(fileRenameService.performRename).toHaveBeenCalledWith(1, 42, false, true);
});
it('does nothing per-book when there are no will_rename candidates but still notifies completion', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
const unchanged = makeBookData({ bookId: 1, title: 'Settled', absolutePath: '/library/Settled.epub' });
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([unchanged]);
const summary = await service.execute(1, 42, {
onProgress: () => {},
isCancelled: () => false,
});
expect(fileRenameService.performRename).not.toHaveBeenCalled();
expect(summary).toMatchObject({ processed: 0, succeeded: 0, failed: 0, skipped: 0, cancelled: false });
expect(notificationService.notify).toHaveBeenCalledWith(expect.objectContaining({ type: 'bulk_rename_completed' }));
});
it('stops when cancelled and reports partial progress', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1, 2, 3]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1), willRenameBook(2), willRenameBook(3)]);
let callCount = 0;
fileRenameService.performRename.mockImplementation(() => {
@@ -486,7 +546,7 @@ describe('BulkRenameService', () => {
it('prevents concurrent execution on same library', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
let resolveRename: ((value: FileRenameResult) => void) | undefined;
fileRenameService.performRename.mockImplementation(
@@ -517,7 +577,7 @@ describe('BulkRenameService', () => {
it('stops and restarts file watcher when library has watch enabled', async () => {
const watchSettings = { ...defaultSettings, watch: true };
bulkRenameRepo.findLibrarySettings.mockResolvedValue(watchSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
fileRenameService.performRename.mockResolvedValue({ status: 'success', durationMs: 10 });
await service.execute(1, 42, {
@@ -531,7 +591,7 @@ describe('BulkRenameService', () => {
it('does not touch file watcher when library has watch disabled', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
fileRenameService.performRename.mockResolvedValue({ status: 'success', durationMs: 10 });
await service.execute(1, 42, {
@@ -546,7 +606,7 @@ describe('BulkRenameService', () => {
it('restarts file watcher even after failure', async () => {
const watchSettings = { ...defaultSettings, watch: true };
bulkRenameRepo.findLibrarySettings.mockResolvedValue(watchSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
fileRenameService.performRename.mockRejectedValue(new Error('disk error'));
await service.execute(1, 42, {
@@ -560,7 +620,7 @@ describe('BulkRenameService', () => {
it('counts failed renames from performRename', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1, 2]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1), willRenameBook(2)]);
fileRenameService.performRename.mockResolvedValueOnce({ status: 'failed', reason: 'collision', durationMs: 5 });
fileRenameService.performRename.mockResolvedValueOnce({ status: 'success', durationMs: 10 });
@@ -579,7 +639,7 @@ describe('BulkRenameService', () => {
it('handles thrown errors from performRename gracefully', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
fileRenameService.performRename.mockRejectedValue(new Error('disk error'));
const events: any[] = [];
@@ -593,9 +653,33 @@ describe('BulkRenameService', () => {
expect(events[0].reason).toBe('disk error');
});
it('notifies failure and rethrows when the run errors unexpectedly', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
fileRenameService.performRename.mockResolvedValue({ status: 'success', durationMs: 10 });
await expect(
service.execute(1, 42, {
onProgress: () => {},
isCancelled: () => {
throw new Error('stream broke');
},
}),
).rejects.toThrow('stream broke');
expect(notificationService.notify).toHaveBeenCalledWith(
expect.objectContaining({
type: 'bulk_rename_failed',
title: 'Bulk rename failed',
}),
);
// The running lock must be released even on an unexpected failure.
expect(service.isRunning(1)).toBe(false);
});
it('sends success notification when all renames succeed', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
fileRenameService.performRename.mockResolvedValue({ status: 'success', durationMs: 10 });
await service.execute(1, 42, {
@@ -613,7 +697,7 @@ describe('BulkRenameService', () => {
it('sends failure notification when some renames fail', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
fileRenameService.performRename.mockResolvedValue({ status: 'failed', reason: 'collision', durationMs: 5 });
await service.execute(1, 42, {
@@ -631,7 +715,7 @@ describe('BulkRenameService', () => {
it('clears running lock after execution completes', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
fileRenameService.performRename.mockResolvedValue({ status: 'success', durationMs: 10 });
expect(service.isRunning(1)).toBe(false);
@@ -647,23 +731,22 @@ describe('BulkRenameService', () => {
it('invalidates preview cache on execute', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue(defaultSettings);
const book = makeBookData({ bookId: 1, title: 'CacheTest' });
book.metadata.title = 'CacheTest';
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([book]);
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
fileRenameService.performRename.mockResolvedValue({ status: 'success', durationMs: 10 });
await service.getPreview(1, 1, 50);
expect(bulkRenameRepo.findAllBooksForLibrary).toHaveBeenCalledTimes(1);
bulkRenameRepo.findLibraryBookIds.mockResolvedValue([1]);
fileRenameService.performRename.mockResolvedValue({ status: 'success', durationMs: 10 });
// execute() recomputes the preview internally to derive the will_rename set (call #2),
// and clears the cache so the following getPreview recomputes again (call #3).
await service.execute(1, 42, {
onProgress: () => {},
isCancelled: () => false,
});
expect(bulkRenameRepo.findAllBooksForLibrary).toHaveBeenCalledTimes(2);
await service.getPreview(1, 1, 50);
expect(bulkRenameRepo.findAllBooksForLibrary).toHaveBeenCalledTimes(2);
expect(bulkRenameRepo.findAllBooksForLibrary).toHaveBeenCalledTimes(3);
});
});
@@ -92,8 +92,9 @@ export class BulkRenameService {
this.runningLibraries.add(libraryId);
this.previewCache.delete(libraryId);
const bookIds = await this.bulkRenameRepo.findLibraryBookIds(libraryId);
this.logger.log(`[${event}] [start] libraryId=${libraryId} userId=${userId} bookCount=${bookIds.length} - bulk rename started`);
const preview = await this.computeFullPreview(libraryId);
const bookIds = preview.items.filter((item) => item.status === 'will_rename').map((item) => item.bookId);
this.logger.log(`[${event}] [start] libraryId=${libraryId} userId=${userId} candidateCount=${bookIds.length} - bulk rename started`);
let watcherWasStopped = false;
if (settings.watch) {
@@ -110,7 +111,7 @@ export class BulkRenameService {
if (options.isCancelled()) break;
try {
const result = await this.fileRenameService.performRename(bookId, userId);
const result = await this.fileRenameService.performRename(bookId, userId, false, true);
if (result.status === 'success') succeeded++;
else if (result.status === 'failed') failed++;