fix(library): refuse renames into a folder another book holds

In folder-as-book libraries a pattern that files several books under one folder either failed on
the folder_path unique index or silently merged a different book into the folder's owner, deleting
its reading history. Check the folder owner on every rename path and merge only when both books
share a title and primary author; otherwise skip with a clear reason. The bulk preview now reports
these as collisions.

Fixes #1682
This commit is contained in:
neon
2026-10-06 00:02:21 +00:00
parent a24dc2e2cc
commit 4ca7128b76
11 changed files with 543 additions and 73 deletions
@@ -1,5 +1,6 @@
import {
findSiblingOccupiedTarget,
isSameWork,
resolveBookFileTargets,
resolvePrimaryFileTarget,
type BookFileTargetsInput,
@@ -465,4 +466,25 @@ describe('resolveBookFileTargets', () => {
expect(findSiblingOccupiedTarget(files, targets)).toBe(`${folder}/book-part02.mp3`);
});
});
describe('isSameWork', () => {
it('matches the same title and primary author regardless of case, spacing, and Unicode form', () => {
expect(isSameWork({ title: 'Dune', primaryAuthor: 'Frank Herbert' }, { title: ' dune ', primaryAuthor: 'FRANK HERBERT' })).toBe(true);
expect(isSameWork({ title: 'Caf\u00e9', primaryAuthor: null }, { title: 'Cafe\u0301', primaryAuthor: null })).toBe(true);
});
it('tells apart different volumes of one series', () => {
expect(isSameWork({ title: 'Dune Messiah', primaryAuthor: 'Frank Herbert' }, { title: 'Dune', primaryAuthor: 'Frank Herbert' })).toBe(false);
});
it('tells apart one title by two authors', () => {
expect(isSameWork({ title: 'Dune', primaryAuthor: 'Frank Herbert' }, { title: 'Dune', primaryAuthor: 'Brian Herbert' })).toBe(false);
expect(isSameWork({ title: 'Dune', primaryAuthor: 'Frank Herbert' }, { title: 'Dune', primaryAuthor: null })).toBe(false);
});
it('never treats untitled books as the same work', () => {
expect(isSameWork({ title: null, primaryAuthor: 'Frank Herbert' }, { title: null, primaryAuthor: 'Frank Herbert' })).toBe(false);
expect(isSameWork({ title: ' ', primaryAuthor: null }, { title: ' ', primaryAuthor: null })).toBe(false);
});
});
});
@@ -132,6 +132,30 @@ export function findSiblingOccupiedTarget(files: TargetBookFile[], targets: Map<
return null;
}
export interface WorkIdentity {
title: string | null;
primaryAuthor: string | null;
}
/**
* Whether two books are the same work, so one may be folded into the other's folder.
*
* In folder-as-book mode a folder is a book, and a rename that lands in another book's folder
* either merges the two or has to be refused. Merging is right for an ebook landing beside its own
* audiobook. It is wrong for a pattern that files a whole series under one folder: that would fold
* every later volume into the first and delete each one's reading history with its book row. The
* folder path cannot tell those apart, so the books' own title and primary author decide.
*/
export function isSameWork(a: WorkIdentity, b: WorkIdentity): boolean {
const title = normalizeIdentity(a.title);
if (!title || title !== normalizeIdentity(b.title)) return false;
return normalizeIdentity(a.primaryAuthor) === normalizeIdentity(b.primaryAuthor);
}
function normalizeIdentity(value: string | null): string {
return (value ?? '').normalize('NFKC').toLowerCase().replace(/\s+/g, ' ').trim();
}
function resolveRelPathFor(file: TargetBookFile, input: BookFileTargetsInput): string | null {
const extension = extname(file.absolutePath);
const format = (file.format ?? extension.slice(1)).toLowerCase();
@@ -52,7 +52,7 @@ describe('FileRenameService narrator-specific editions', () => {
}),
checkPathTakenByOtherBook: vi.fn().mockResolvedValue(false),
applyFolderRename: vi.fn().mockResolvedValue(undefined),
findBookByExactFolderPath: vi.fn().mockResolvedValue(null),
findFolderOwners: vi.fn().mockResolvedValue(new Map()),
applyExistingFolderMerge: vi.fn().mockResolvedValue(undefined),
};
const lockService = {
@@ -162,19 +162,53 @@ describe('FileRenameRepository', () => {
]);
});
it('findBookByExactFolderPath returns the exact folder owner in a library', async () => {
const row = { id: 99, folderPath: '/library/Author/Book', primaryFileId: 42, status: 'present' };
it('findFolderOwners maps each held folder to its book, title, and first-listed author', async () => {
const db = {
select: vi.fn().mockReturnValue({
select: vi
.fn()
.mockReturnValueOnce({
from: vi.fn().mockReturnThis(),
leftJoin: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue([
{ bookId: 99, folderPath: '/library/Frank Herbert/Dune', title: 'Dune' },
{ bookId: 100, folderPath: '/library/Anonymous/Beowulf', title: null },
]),
})
.mockReturnValueOnce({
from: vi.fn().mockReturnThis(),
innerJoin: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
orderBy: vi.fn().mockResolvedValue([
{ bookId: 99, name: 'Frank Herbert' },
{ bookId: 99, name: 'Brian Herbert' },
]),
}),
};
const repo = new FileRenameRepository(db as never);
const owners = await repo.findFolderOwners(3, ['/library/Frank Herbert/Dune', '/library/Anonymous/Beowulf', '/library/Frank Herbert/Dune']);
expect(owners).toEqual(
new Map([
['/library/Frank Herbert/Dune', { bookId: 99, title: 'Dune', primaryAuthor: 'Frank Herbert' }],
['/library/Anonymous/Beowulf', { bookId: 100, title: null, primaryAuthor: null }],
]),
);
});
it('findFolderOwners skips the author query when no book holds any of the folders', async () => {
const db = {
select: vi.fn().mockReturnValueOnce({
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([row]),
leftJoin: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue([]),
}),
};
const repo = new FileRenameRepository(db as never);
await expect(repo.findBookByExactFolderPath(3, '/library/Author/Book')).resolves.toEqual(row);
await expect(repo.findFolderOwners(3, ['/library/Frank Herbert/Dune'])).resolves.toEqual(new Map());
expect(db.select).toHaveBeenCalledTimes(1);
});
it('applyExistingFolderMerge reassigns files, deletes the source book, and fills a missing target primary', async () => {
@@ -45,6 +45,14 @@ export interface BookFilePathUpdate {
relPath: string | null;
}
export interface FolderOwner {
bookId: number;
title: string | null;
primaryAuthor: string | null;
}
const FOLDER_OWNER_BATCH_SIZE = 500;
@Injectable()
export class FileRenameRepository {
constructor(@Inject(DB) private readonly db: Db) {}
@@ -164,22 +172,42 @@ export class FileRenameRepository {
});
}
async findBookByExactFolderPath(
libraryId: number,
folderPath: string,
): Promise<Pick<typeof books.$inferSelect, 'id' | 'folderPath' | 'primaryFileId' | 'status'> | null> {
const [row] = await this.db
.select({
id: books.id,
folderPath: books.folderPath,
primaryFileId: books.primaryFileId,
status: books.status,
})
.from(books)
.where(and(eq(books.libraryId, libraryId), eq(books.folderPath, folderPath)))
.limit(1);
/** The book that holds each folder, keyed by folder path. Folders no book holds are absent. */
async findFolderOwners(libraryId: number, folderPaths: string[]): Promise<Map<string, FolderOwner>> {
const owners = new Map<string, FolderOwner>();
const uniquePaths = [...new Set(folderPaths)];
return row ?? null;
for (let i = 0; i < uniquePaths.length; i += FOLDER_OWNER_BATCH_SIZE) {
const batch = uniquePaths.slice(i, i + FOLDER_OWNER_BATCH_SIZE);
const rows = await this.db
.select({ bookId: books.id, folderPath: books.folderPath, title: bookMetadata.title })
.from(books)
.leftJoin(bookMetadata, eq(bookMetadata.bookId, books.id))
.where(and(eq(books.libraryId, libraryId), inArray(books.folderPath, batch)));
if (rows.length === 0) continue;
const ownerIds = rows.map((row) => row.bookId);
const authorRows = await this.db
.select({ bookId: bookAuthors.bookId, name: authors.name })
.from(bookAuthors)
.innerJoin(authors, eq(authors.id, bookAuthors.authorId))
.where(inArray(bookAuthors.bookId, ownerIds))
.orderBy(asc(bookAuthors.bookId), asc(bookAuthors.displayOrder));
const primaryAuthorByBookId = new Map<number, string>();
for (const author of authorRows) {
if (!primaryAuthorByBookId.has(author.bookId)) primaryAuthorByBookId.set(author.bookId, author.name);
}
for (const row of rows) {
owners.set(row.folderPath, {
bookId: row.bookId,
title: row.title ?? null,
primaryAuthor: primaryAuthorByBookId.get(row.bookId) ?? null,
});
}
}
return owners;
}
async applyExistingFolderMerge(input: {
@@ -81,7 +81,7 @@ describe('FileRenameService', () => {
findBookRenameData: vi.fn(),
checkPathTakenByOtherBook: vi.fn().mockResolvedValue(false),
applyFolderRename: vi.fn().mockResolvedValue(undefined),
findBookByExactFolderPath: vi.fn().mockResolvedValue(null),
findFolderOwners: vi.fn().mockResolvedValue(new Map()),
applyExistingFolderMerge: vi.fn().mockResolvedValue(undefined),
findAllBookFiles: vi.fn().mockImplementation(async () => {
const data = await renameRepo.findBookRenameData();
@@ -868,7 +868,7 @@ describe('FileRenameService', () => {
const result = await service.performRename(5, 12);
expect(result).toEqual(expect.objectContaining({ status: 'success' }));
expect(renameRepo.findBookByExactFolderPath).not.toHaveBeenCalled();
expect(renameRepo.applyExistingFolderMerge).not.toHaveBeenCalled();
expect(mockRename).toHaveBeenCalledTimes(1);
expect(mockRename).toHaveBeenCalledWith('/library/Frank herbert/Dune/Dune.epub', '/library/Frank Herbert/Dune/Dune.epub');
});
@@ -1242,12 +1242,9 @@ describe('FileRenameService', () => {
if (path.toString() === '/library/Frank Herbert/Dune (1965)') return Promise.resolve(undefined);
return Promise.reject(Object.assign(new Error('missing'), { code: 'ENOENT' }));
});
renameRepo.findBookByExactFolderPath.mockResolvedValue({
id: 99,
folderPath: '/library/Frank Herbert/Dune (1965)',
primaryFileId: 42,
status: 'present',
});
renameRepo.findFolderOwners.mockResolvedValue(
new Map([['/library/Frank Herbert/Dune (1965)', { bookId: 99, title: 'Dune', primaryAuthor: 'Frank Herbert' }]]),
);
const result = await service.performRename(5, 12);
@@ -1258,7 +1255,7 @@ describe('FileRenameService', () => {
newPath: '/library/Frank Herbert/Dune (1965)/Dune (1965).epub',
}),
);
expect(renameRepo.findBookByExactFolderPath).toHaveBeenCalledWith(1, '/library/Frank Herbert/Dune (1965)');
expect(renameRepo.findFolderOwners).toHaveBeenCalledWith(1, ['/library/Frank Herbert/Dune (1965)']);
expect(renameRepo.applyExistingFolderMerge).toHaveBeenCalledWith({
sourceBookId: 5,
targetBookId: 99,
@@ -1319,12 +1316,9 @@ describe('FileRenameService', () => {
if (path.toString() === '/library/Frank Herbert/Dune (1965)') return Promise.resolve(undefined);
return Promise.reject(Object.assign(new Error('missing'), { code: 'ENOENT' }));
});
renameRepo.findBookByExactFolderPath.mockResolvedValue({
id: 99,
folderPath: '/library/Frank Herbert/Dune (1965)',
primaryFileId: 42,
status: 'present',
});
renameRepo.findFolderOwners.mockResolvedValue(
new Map([['/library/Frank Herbert/Dune (1965)', { bookId: 99, title: 'Dune', primaryAuthor: 'Frank Herbert' }]]),
);
await expect(service.performRename(5, 12)).resolves.toEqual(expect.objectContaining({ status: 'success' }));
@@ -1354,12 +1348,9 @@ describe('FileRenameService', () => {
}
return Promise.reject(Object.assign(new Error('missing'), { code: 'ENOENT' }));
});
renameRepo.findBookByExactFolderPath.mockResolvedValue({
id: 99,
folderPath: '/library/Frank Herbert/Dune (1965)',
primaryFileId: 42,
status: 'present',
});
renameRepo.findFolderOwners.mockResolvedValue(
new Map([['/library/Frank Herbert/Dune (1965)', { bookId: 99, title: 'Dune', primaryAuthor: 'Frank Herbert' }]]),
);
const result = await service.performRename(5, 12);
@@ -1389,7 +1380,7 @@ describe('FileRenameService', () => {
if (path.toString() === '/library/Frank Herbert/Dune (1965)') return Promise.resolve(undefined);
return Promise.reject(Object.assign(new Error('missing'), { code: 'ENOENT' }));
});
renameRepo.findBookByExactFolderPath.mockResolvedValue(null);
renameRepo.findFolderOwners.mockResolvedValue(new Map());
const result = await service.performRename(5, 12);
@@ -1431,12 +1422,9 @@ describe('FileRenameService', () => {
if (path.toString() === '/library/Frank Herbert/Dune (1965)') return Promise.resolve(undefined);
return Promise.reject(Object.assign(new Error('missing'), { code: 'ENOENT' }));
});
renameRepo.findBookByExactFolderPath.mockResolvedValue({
id: 99,
folderPath: '/library/Frank Herbert/Dune (1965)',
primaryFileId: 42,
status: 'present',
});
renameRepo.findFolderOwners.mockResolvedValue(
new Map([['/library/Frank Herbert/Dune (1965)', { bookId: 99, title: 'Dune', primaryAuthor: 'Frank Herbert' }]]),
);
renameRepo.applyExistingFolderMerge.mockRejectedValue(new Error('db unavailable'));
const result = await service.performRename(5, 12);
@@ -1449,6 +1437,137 @@ describe('FileRenameService', () => {
expect(mockRename).toHaveBeenNthCalledWith(4, '/library/Frank Herbert/Dune (1965)/Dune (1965).epub', '/library/Incoming/old.epub');
});
describe('a target folder held by another book', () => {
const seriesPattern = '{authors}/<{series}/><{seriesIndex}> - <{title}|{originalFilename}>';
const seriesFolder = '/library/Frank Herbert/Dune';
const duneOwner = { bookId: 99, title: 'Dune', primaryAuthor: 'Frank Herbert' };
const messiahMetadata = { title: 'Dune Messiah', seriesName: 'Dune', seriesIndex: '2' };
function expectNothingTouched(renameRepo: ReturnType<typeof makeService>['renameRepo']) {
expect(renameRepo.applyFolderRename).not.toHaveBeenCalled();
expect(renameRepo.applyExistingFolderMerge).not.toHaveBeenCalled();
expect(mockRename).not.toHaveBeenCalled();
expect(mockMkdir).not.toHaveBeenCalled();
}
it('refuses a loose file whose series folder belongs to a different book instead of failing on the folder index', async () => {
const { service, renameRepo, notificationService } = makeService();
renameRepo.findBookRenameData.mockResolvedValue(
makeRenameData({
organizationMode: 'book_per_folder',
fileNamingPattern: seriesPattern,
metadata: messiahMetadata,
file: { absolutePath: '/library/dune-messiah.epub', relPath: 'dune-messiah.epub' },
bookFolderPath: '/library/dune-messiah.epub',
}),
);
renameRepo.findFolderOwners.mockResolvedValue(new Map([[seriesFolder, duneOwner]]));
mockAccess.mockResolvedValue(undefined as never);
const result = await service.performRename(5, 12);
expect(result).toEqual(
expect.objectContaining({
status: 'skipped',
reason: 'target folder belongs to another book',
oldPath: '/library/dune-messiah.epub',
newPath: '/library/Frank Herbert/Dune/02 - Dune Messiah.epub',
}),
);
expect(renameRepo.findFolderOwners).toHaveBeenCalledWith(1, [seriesFolder]);
expectNothingTouched(renameRepo);
expect(notificationService.notify).toHaveBeenCalledWith(
expect.objectContaining({ message: 'File rename skipped: target folder belongs to another book.' }),
);
});
it('refuses to fold a different book with its own folder into the series folder', async () => {
const { service, renameRepo } = makeService();
renameRepo.findBookRenameData.mockResolvedValue(
makeRenameData({
organizationMode: 'book_per_folder',
fileNamingPattern: seriesPattern,
metadata: messiahMetadata,
file: { absolutePath: '/library/Frank Herbert/Dune Messiah/Dune Messiah.epub', relPath: 'Frank Herbert/Dune Messiah/Dune Messiah.epub' },
bookFolderPath: '/library/Frank Herbert/Dune Messiah',
}),
);
renameRepo.findFolderOwners.mockResolvedValue(new Map([[seriesFolder, duneOwner]]));
mockAccess.mockResolvedValue(undefined as never);
const result = await service.performRename(5, 12);
expect(result).toEqual(expect.objectContaining({ status: 'skipped', reason: 'target folder belongs to another book' }));
expectNothingTouched(renameRepo);
});
it('refuses a nested move into a parent folder that belongs to a different book', async () => {
const { service, renameRepo } = makeService();
renameRepo.findBookRenameData.mockResolvedValue(
makeRenameData({
organizationMode: 'book_per_folder',
fileNamingPattern: seriesPattern,
metadata: messiahMetadata,
file: { absolutePath: '/library/Frank Herbert/Dune/Messiah/Dune Messiah.epub', relPath: 'Frank Herbert/Dune/Messiah/Dune Messiah.epub' },
bookFolderPath: '/library/Frank Herbert/Dune/Messiah',
}),
);
renameRepo.findFolderOwners.mockResolvedValue(new Map([[seriesFolder, duneOwner]]));
mockAccess.mockResolvedValue(undefined as never);
const result = await service.performRename(5, 12);
expect(result).toEqual(expect.objectContaining({ status: 'skipped', reason: 'target folder belongs to another book' }));
expectNothingTouched(renameRepo);
});
it('refuses the same work when its folder is gone from disk rather than reviving a missing book', async () => {
const { service, renameRepo } = makeService();
renameRepo.findBookRenameData.mockResolvedValue(
makeRenameData({
organizationMode: 'book_per_folder',
fileNamingPattern: '{authors}/{title} ({year})/{title} ({year})',
file: { absolutePath: '/library/Incoming/old.epub', relPath: 'Incoming/old.epub' },
bookFolderPath: '/library/Incoming',
}),
);
renameRepo.findFolderOwners.mockResolvedValue(new Map([['/library/Frank Herbert/Dune (1965)', duneOwner]]));
mockAccess.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' }));
const result = await service.performRename(5, 12);
expect(result).toEqual(expect.objectContaining({ status: 'skipped', reason: 'target folder belongs to another book' }));
expectNothingTouched(renameRepo);
});
it('merges a loose audiobook into the folder its own ebook already holds', async () => {
const { service, renameRepo } = makeService();
renameRepo.findBookRenameData.mockResolvedValue(
makeRenameData({
organizationMode: 'book_per_folder',
fileNamingPattern: '{authors}/{title} ({year})/{title} ({year})',
file: { absolutePath: '/library/dune.m4b', relPath: 'dune.m4b', format: 'm4b' },
bookFolderPath: '/library/dune.m4b',
}),
);
renameRepo.findAllBookFiles.mockResolvedValue([
{ id: 10, absolutePath: '/library/dune.m4b', relPath: 'dune.m4b', role: 'content', format: 'm4b' },
]);
renameRepo.findFolderOwners.mockResolvedValue(new Map([['/library/Frank Herbert/Dune (1965)', { ...duneOwner, title: 'DUNE' }]]));
mockAccess.mockImplementation((path: any) => {
if (path.toString() === '/library/Frank Herbert/Dune (1965)') return Promise.resolve(undefined);
return Promise.reject(Object.assign(new Error('missing'), { code: 'ENOENT' }));
});
const result = await service.performRename(5, 12);
expect(result).toEqual(expect.objectContaining({ status: 'success', newPath: '/library/Frank Herbert/Dune (1965)/Dune (1965).m4b' }));
expect(renameRepo.applyExistingFolderMerge).toHaveBeenCalledWith(expect.objectContaining({ sourceBookId: 5, targetBookId: 99 }));
expect(renameRepo.applyFolderRename).not.toHaveBeenCalled();
expect(mockRename).toHaveBeenCalledWith('/library/dune.m4b', '/library/Frank Herbert/Dune (1965)/Dune (1965).m4b');
});
});
it('detects case-only nested folder moves and uses per-file individual moves (avoids EINVAL on case-insensitive filesystems)', async () => {
const { service, renameRepo } = makeService();
renameRepo.findBookRenameData.mockResolvedValue(
@@ -17,7 +17,7 @@ import type { BookFilePathUpdate, BookRenameData } from './file-rename.repositor
import { FileRenameRepository } from './file-rename.repository';
import { FileLockService, bookOperationLockKey } from './file-lock.service';
import { buildPatternTokens } from '../../common/utils/pattern-tokens.utils';
import { resolveBookFileTargets } from './book-file-targets';
import { isSameWork, resolveBookFileTargets } from './book-file-targets';
const FILE_RENAME_EVENT = 'file.rename';
const FILE_RENAME_ROLLBACK_EVENT = 'file.rename_rollback';
@@ -173,18 +173,32 @@ export class FileRenameService implements OnModuleDestroy {
return this.logAndReturn(bookId, startedAt, { status: 'skipped', reason: 'path unchanged' });
}
let mergeTargetBookId: number | null = null;
if (isBookPerFolder && newFolderPath !== currentFolderPath) {
const owner = (await this.renameRepo.findFolderOwners(data.libraryId, [newFolderPath])).get(newFolderPath);
if (owner && owner.bookId !== bookId) {
const sameWork = isSameWork({ title: data.metadata.title, primaryAuthor: data.authors[0] ?? null }, owner);
if (!sameWork || !(await this.pathExists(newFolderPath))) {
const reason = 'target folder belongs to another book';
this.logger.warn(
`[${FILE_RENAME_EVENT}] [end] bookId=${bookId} userId=${userId} durationMs=${Date.now() - startedAt} status=skipped reason="${sanitizeLogValue(reason)}" ownerBookId=${owner.bookId} newFolder="${sanitizeLogValue(newFolderPath)}" - rename skipped: target folder belongs to another book`,
);
await this.notifyFailure(userId, bookId, `File rename skipped: ${reason}.`, suppressNotification);
return { status: 'skipped', reason, oldPath: currentAbsolutePath, newPath: newAbsolutePath, durationMs: Date.now() - startedAt };
}
mergeTargetBookId = owner.bookId;
}
}
const nestedFolderMove = bookHasOwnFolder && newFolderPath !== currentFolderPath && this.foldersAreNested(currentFolderPath, newFolderPath);
const renamingFolder = bookHasOwnFolder && newFolderPath !== currentFolderPath && !nestedFolderMove;
let moveIntoExistingFolder = false;
let mergeTargetBookId: number | null = null;
if (renamingFolder) {
if (await this.pathExists(newFolderPath)) {
const sameFolder = await this.pathsReferToSameSource(currentFolderPath, newFolderPath, data.libraryFolderPath, sanitizeForCrossPlatform);
if (!sameFolder) {
moveIntoExistingFolder = true;
const targetBook = await this.renameRepo.findBookByExactFolderPath(data.libraryId, newFolderPath);
if (targetBook && targetBook.id !== bookId) mergeTargetBookId = targetBook.id;
const existingTargetPath = await this.findExistingTargetFilePath(allFiles, fileTargets, data.libraryFolderPath, sanitizeForCrossPlatform);
if (existingTargetPath) {
@@ -232,18 +246,18 @@ export class FileRenameService implements OnModuleDestroy {
const suppressPaths = this.buildSuppressedRenamePaths(allFiles, fileTargets, currentFolderPath, newFolderPath, data.libraryFolderPath);
this.selfWriteRegistry.begin(suppressPaths);
try {
if (bookHasOwnFolder && newFolderPath !== currentFolderPath) {
if (mergeTargetBookId !== null) {
await this.mergeBookIntoExistingFolder(
bookId,
data,
currentFolderPath,
newFolderPath,
fileTargets,
mergeTargetBookId,
sanitizeForCrossPlatform,
);
} else if (moveIntoExistingFolder) {
if (mergeTargetBookId !== null) {
await this.mergeBookIntoExistingFolder(
bookId,
data,
currentFolderPath,
newFolderPath,
fileTargets,
mergeTargetBookId,
sanitizeForCrossPlatform,
);
} else if (bookHasOwnFolder && newFolderPath !== currentFolderPath) {
if (moveIntoExistingFolder) {
await this.renameBookIntoExistingFolder(bookId, data, currentFolderPath, newFolderPath, fileTargets, sanitizeForCrossPlatform);
} else {
await this.renameBookWithFolder(bookId, data, currentFolderPath, newFolderPath, fileTargets, sanitizeForCrossPlatform);
@@ -88,6 +88,7 @@ describe('BulkRenameService', () => {
const fileRenameRepo = {
findExistingPaths: vi.fn(),
findFolderOwners: vi.fn(),
};
const fileRenameService = {
@@ -119,6 +120,7 @@ describe('BulkRenameService', () => {
appSettings.getUploadPatternBookPerFolder.mockResolvedValue('{authors}/{title}/');
notificationService.notify.mockResolvedValue(undefined);
fileRenameRepo.findExistingPaths.mockResolvedValue(new Map());
fileRenameRepo.findFolderOwners.mockResolvedValue(new Map());
fileWatcherService.pauseWatcher.mockReturnValue(true);
service = new BulkRenameService(
@@ -401,6 +403,88 @@ describe('BulkRenameService', () => {
expect(result.items[0].newPath).toContain('FolderBook');
});
describe('folders held by another book', () => {
const seriesPattern = '{authors}/<{series}/><{seriesIndex}> - {title}';
const seriesFolder = '/library/Frank Herbert/Dune';
function looseSeriesBook(bookId: number, title: string, seriesIndex: string, format = 'epub'): BulkRenameBookData {
const book = makeBookData({
bookId,
title,
absolutePath: `/library/loose-${bookId}.${format}`,
format,
organizationMode: 'book_per_folder',
fileNamingPattern: seriesPattern,
authors: ['Frank Herbert'],
});
book.metadata.seriesName = 'Dune';
book.metadata.seriesIndex = seriesIndex;
return book;
}
beforeEach(() => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue({
fileRenameEnabled: true,
fileNamingPattern: seriesPattern,
organizationMode: 'book_per_folder',
watch: false,
});
});
it('holds back a book whose target folder belongs to a different work', async () => {
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([looseSeriesBook(2, 'Dune Messiah', '2')]);
fileRenameRepo.findFolderOwners.mockResolvedValue(new Map([[seriesFolder, { bookId: 1, title: 'Dune', primaryAuthor: 'Frank Herbert' }]]));
const result = await service.getPreview(1, 1, 50);
expect(fileRenameRepo.findFolderOwners).toHaveBeenCalledWith(1, [seriesFolder]);
expect(result.items[0]).toEqual(expect.objectContaining({ status: 'collision', reason: 'Target folder belongs to another book' }));
expect(result.totalByStatus.collision).toBe(1);
expect(result.totalByStatus.will_rename).toBe(0);
});
it('keeps a rename into the folder held by the same work in another format', async () => {
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([looseSeriesBook(2, 'Dune', '1', 'm4b')]);
fileRenameRepo.findFolderOwners.mockResolvedValue(new Map([[seriesFolder, { bookId: 1, title: 'Dune', primaryAuthor: 'Frank Herbert' }]]));
const result = await service.getPreview(1, 1, 50);
expect(result.items[0].status).toBe('will_rename');
});
it('holds back every book of a series that would share one new folder', async () => {
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([looseSeriesBook(1, 'Dune', '1'), looseSeriesBook(2, 'Dune Messiah', '2')]);
const result = await service.getPreview(1, 1, 50);
expect(result.items.map((item) => item.status)).toEqual(['collision', 'collision']);
expect(result.items[0].reason).toBe('Multiple books would resolve to the same folder');
});
it('keeps two formats of one work that would share a new folder', async () => {
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([looseSeriesBook(1, 'Dune', '1'), looseSeriesBook(2, 'Dune', '1', 'm4b')]);
const result = await service.getPreview(1, 1, 50);
expect(result.items.map((item) => item.status)).toEqual(['will_rename', 'will_rename']);
});
it('does not look up folder owners for a file-as-book library', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue({
fileRenameEnabled: true,
fileNamingPattern: '{title}',
organizationMode: 'book_per_file',
watch: false,
});
bulkRenameRepo.findAllBooksForLibrary.mockResolvedValue([willRenameBook(1)]);
const result = await service.getPreview(1, 1, 50);
expect(result.items[0].status).toBe('will_rename');
expect(fileRenameRepo.findFolderOwners).not.toHaveBeenCalled();
});
});
it('returns empty items for library with no books', async () => {
bulkRenameRepo.findLibrarySettings.mockResolvedValue({
fileRenameEnabled: true,
@@ -1,4 +1,5 @@
import { BadRequestException, Injectable, Logger, NotFoundException } from '@nestjs/common';
import { dirname } from 'path';
import type { BulkRenamePreviewItem, BulkRenamePreviewPage, BulkRenameProgressEvent, BulkRenameStatus } from '@bookorbit/types';
import { NotificationType } from '@bookorbit/types';
@@ -10,7 +11,7 @@ import { FileRenameRepository } from '../file-write/file-rename.repository';
import { FileWatcherService } from '../scanner/file-watcher.service';
import type { BulkRenameBookData } from '../file-write/bulk-rename.repository';
import { BulkRenameRepository } from '../file-write/bulk-rename.repository';
import { findSiblingOccupiedTarget, resolveBookFileTargets } from '../file-write/book-file-targets';
import { findSiblingOccupiedTarget, isSameWork, resolveBookFileTargets, type WorkIdentity } from '../file-write/book-file-targets';
const CACHE_TTL_MS = 60_000;
@@ -274,6 +275,10 @@ export class BulkRenameService {
}
}
if (settings.organizationMode === 'book_per_folder') {
await this.markFolderCollisions(libraryId, previewItems, books);
}
const totalByStatus: Record<BulkRenameStatus, number> = {
will_rename: 0,
unchanged: 0,
@@ -288,6 +293,51 @@ export class BulkRenameService {
return { items: previewItems, totalByStatus, pattern: pattern ?? '', createdAt: Date.now() };
}
/**
* A folder is a book in this mode, so the rename refuses to land in a folder held by a different
* work. A folder that several renamed books would share is held back as a whole unless they are
* all one work, because which of them gets there first depends on the reviewer's selection.
*/
private async markFolderCollisions(libraryId: number, items: BulkRenamePreviewItem[], books: BulkRenameBookData[]): Promise<void> {
const bookById = new Map(books.map((book) => [book.bookId, book]));
const itemsByFolder = new Map<string, BulkRenamePreviewItem[]>();
for (const item of items) {
const book = bookById.get(item.bookId);
if (item.status !== 'will_rename' || !item.newPath || !book) continue;
const folder = dirname(item.newPath);
if (folder === book.bookFolderPath) continue;
const group = itemsByFolder.get(folder);
if (group) group.push(item);
else itemsByFolder.set(folder, [item]);
}
if (itemsByFolder.size === 0) return;
const owners = await this.fileRenameRepo.findFolderOwners(libraryId, [...itemsByFolder.keys()]);
const identityOf = (bookId: number): WorkIdentity => {
const book = bookById.get(bookId)!;
return { title: book.metadata.title, primaryAuthor: book.authors[0] ?? null };
};
for (const [folder, group] of itemsByFolder) {
const owner = owners.get(folder);
if (owner) {
for (const item of group) {
if (isSameWork(identityOf(item.bookId), owner)) continue;
item.status = 'collision';
item.reason = 'Target folder belongs to another book';
}
continue;
}
const first = identityOf(group[0].bookId);
if (group.length < 2 || group.every((item, index) => index === 0 || isSameWork(identityOf(item.bookId), first))) continue;
for (const item of group) {
item.status = 'collision';
item.reason = 'Multiple books would resolve to the same folder';
}
}
}
private computePreviewItem(book: BulkRenameBookData, pattern: string | null, sanitizeForCrossPlatform: boolean): BulkRenamePreviewItem {
const baseItem: BulkRenamePreviewItem = {
bookId: book.bookId,
@@ -8,7 +8,7 @@ import { mkdir } from 'fs/promises';
import { asc, eq } from 'drizzle-orm';
import type { NodePgDatabase } from 'drizzle-orm/node-postgres';
import { DEFAULT_FORMAT_PRIORITY } from '@bookorbit/types';
import type { BulkRenamePreviewPage, BulkRenameProgressEvent, BulkRenameStatus } from '@bookorbit/types';
import type { BookWriteAndRenameResult, BulkRenamePreviewPage, BulkRenameProgressEvent, BulkRenameStatus } from '@bookorbit/types';
import { AppModule } from '../../../src/app.module';
import { DB } from '../../../src/db';
@@ -246,7 +246,7 @@ export async function setBookMetadata(
publisher?: string | null;
publishedYear?: number | null;
seriesName?: string | null;
seriesIndex?: number | null;
seriesIndex?: string | null;
language?: string | null;
isbn13?: string | null;
authors?: string[];
@@ -311,6 +311,20 @@ export async function getBulkRenameStatus(ctx: FileRenameE2EContext, libraryId:
return response.json() as { running: boolean };
}
export async function writeAndRename(ctx: FileRenameE2EContext, bookId: number): Promise<BookWriteAndRenameResult> {
const response = await ctx.app.inject({
method: 'POST',
url: `/api/v1/books/${bookId}/write-and-rename`,
headers: authHeader(ctx.adminToken),
});
if (response.statusCode !== 201) {
throw new Error(`Write and rename returned ${response.statusCode}: ${response.body}`);
}
return response.json() as BookWriteAndRenameResult;
}
export interface BulkRenameExecuteResult {
statusCode: number;
events: BulkRenameProgressEvent[];
+85 -4
View File
@@ -18,6 +18,7 @@
* - Empty-folder cleanup after book_per_folder rename
* - Cross-directory move within book_per_file
* - Flat-to-folder transition (book_per_folder, book was NOT in own folder)
* - Target folder held by another book (book_per_folder): refused unless the same work
*/
import { mkdtemp, rm, access, readdir } from 'fs/promises';
@@ -39,6 +40,7 @@ import {
getBulkRenameStatus,
setBookMetadata,
triggerAndWaitForScan,
writeAndRename,
type FileRenameE2EContext,
} from './e2e/file-rename/file-rename-harness';
import {
@@ -50,6 +52,7 @@ import {
buildCollisionPair,
buildSpecialCharBook,
createEpubFile,
createPdfFile,
} from './e2e/file-rename/file-rename-fixture-builder';
const SUITE_TIMEOUT_MS = 180_000;
@@ -268,9 +271,9 @@ describe('Bulk file rename (e2e)', { timeout: SUITE_TIMEOUT_MS }, () => {
expect(books).toHaveLength(3);
const seriesData = [
{ title: 'Book One', seriesName: 'My Series', seriesIndex: 1 },
{ title: 'Book Two', seriesName: 'My Series', seriesIndex: 2 },
{ title: 'Book Three', seriesName: 'My Series', seriesIndex: 3 },
{ title: 'Book One', seriesName: 'My Series', seriesIndex: '1' },
{ title: 'Book Two', seriesName: 'My Series', seriesIndex: '2' },
{ title: 'Book Three', seriesName: 'My Series', seriesIndex: '3' },
];
for (let i = 0; i < books.length; i++) {
@@ -983,7 +986,85 @@ describe('Bulk file rename (e2e)', { timeout: SUITE_TIMEOUT_MS }, () => {
});
});
// ── 20. Non-existent library ──────────────────────────────────────────────
// ── 20. book_per_folder — target folder held by another book ──────────────
describe('book_per_folder — target folder held by another book', { timeout: TEST_TIMEOUT_MS }, () => {
const seriesPattern = '{authors}/<{series}/><{seriesIndex}> - <{title}|{originalFilename}>';
async function scanDuneVolumes(lib: { libraryId: number; folderPath: string }, duneRelPath: string, messiahRelPath: string) {
await createEpubFile(join(lib.folderPath, duneRelPath), 'Dune');
await createEpubFile(join(lib.folderPath, messiahRelPath), 'Dune Messiah');
await triggerAndWaitForScan(ctx, lib.libraryId);
const books = await findAllBooksInLibrary(ctx, lib.libraryId);
const dune = books.find((b) => b.absolutePath === join(lib.folderPath, duneRelPath))!;
const messiah = books.find((b) => b.absolutePath === join(lib.folderPath, messiahRelPath))!;
await setBookMetadata(ctx, dune.bookId, { title: 'Dune', authors: ['Frank Herbert'], seriesName: 'Dune', seriesIndex: '1' });
await setBookMetadata(ctx, messiah.bookId, { title: 'Dune Messiah', authors: ['Frank Herbert'], seriesName: 'Dune', seriesIndex: '2' });
return { dune, messiah };
}
it('refuses to move a loose series volume into the folder another book holds', async () => {
const lib = await createLibrary(ctx, { mode: 'book_per_folder', fileRenameEnabled: false, fileNamingPattern: seriesPattern });
const { dune, messiah } = await scanDuneVolumes(lib, 'dune.epub', 'dune-messiah.epub');
expect((await writeAndRename(ctx, dune.bookId)).rename.status).toBe('success');
const second = await writeAndRename(ctx, messiah.bookId);
expect(second.rename).toEqual(expect.objectContaining({ status: 'skipped', reason: 'target folder belongs to another book' }));
const after = await findAllBooksInLibrary(ctx, lib.libraryId);
expect(after.find((b) => b.bookId === dune.bookId)?.folderPath).toBe(join(lib.folderPath, 'Frank Herbert/Dune'));
expect(after.find((b) => b.bookId === messiah.bookId)?.absolutePath).toBe(join(lib.folderPath, 'dune-messiah.epub'));
await expect(pathExists(join(lib.folderPath, 'dune-messiah.epub'))).resolves.toBe(true);
});
it('refuses to fold a series volume that has its own folder into another book', async () => {
const lib = await createLibrary(ctx, { mode: 'book_per_folder', fileRenameEnabled: false, fileNamingPattern: seriesPattern });
const { dune, messiah } = await scanDuneVolumes(lib, 'Frank Herbert/Dune/Dune.epub', 'Frank Herbert/Dune Messiah/Dune Messiah.epub');
expect((await writeAndRename(ctx, dune.bookId)).rename.status).toBe('success');
const second = await writeAndRename(ctx, messiah.bookId);
expect(second.rename).toEqual(expect.objectContaining({ status: 'skipped', reason: 'target folder belongs to another book' }));
const after = await findAllBooksInLibrary(ctx, lib.libraryId);
expect(new Set(after.map((b) => b.bookId))).toEqual(new Set([dune.bookId, messiah.bookId]));
await expect(pathExists(join(lib.folderPath, 'Frank Herbert/Dune Messiah/Dune Messiah.epub'))).resolves.toBe(true);
});
it('still merges another format of the same work into the folder that work holds', async () => {
const lib = await createLibrary(ctx, { mode: 'book_per_folder', fileRenameEnabled: false, fileNamingPattern: '{authors}/{title}/{title}' });
await createEpubFile(join(lib.folderPath, 'Frank Herbert/Dune/Dune.epub'), 'Dune');
await createPdfFile(join(lib.folderPath, 'Incoming/scan.pdf'), 'Dune');
await triggerAndWaitForScan(ctx, lib.libraryId);
const books = await findAllBooksInLibrary(ctx, lib.libraryId);
const ebook = books.find((b) => b.absolutePath.endsWith('Dune.epub'))!;
const pdf = books.find((b) => b.absolutePath.endsWith('scan.pdf'))!;
for (const book of [ebook, pdf]) await setBookMetadata(ctx, book.bookId, { title: 'Dune', authors: ['Frank Herbert'] });
const result = await writeAndRename(ctx, pdf.bookId);
expect(result.rename.status).toBe('success');
const after = await findAllBooksInLibrary(ctx, lib.libraryId);
expect(after.map((b) => b.bookId)).toEqual([ebook.bookId, ebook.bookId]);
expect(after.map((b) => b.relPath).sort()).toEqual(['Frank Herbert/Dune/Dune.epub', 'Frank Herbert/Dune/Dune.pdf']);
});
it('bulk preview holds back series volumes that would share one folder, and the run renames none of them', async () => {
const lib = await createLibrary(ctx, { mode: 'book_per_folder', fileRenameEnabled: true, fileNamingPattern: seriesPattern });
await scanDuneVolumes(lib, 'dune.epub', 'dune-messiah.epub');
const preview = await getBulkRenamePreview(ctx, lib.libraryId);
expect(preview.items.map((item) => item.status)).toEqual(['collision', 'collision']);
const done = getDoneEvent((await executeBulkRename(ctx, lib.libraryId)).events);
expect(done!.processed).toBe(0);
await expect(pathExists(join(lib.folderPath, 'dune.epub'))).resolves.toBe(true);
await expect(pathExists(join(lib.folderPath, 'dune-messiah.epub'))).resolves.toBe(true);
});
});
// ── 21. Non-existent library ──────────────────────────────────────────────
describe('non-existent library', { timeout: TEST_TIMEOUT_MS }, () => {
it('execute returns 404 for non-existent libraryId', async () => {