feat(library): allow renaming books into existing folders

Previously, attempting to rename a book into a folder that already existed on disk was blocked. This change allows users to move books into existing folders, enabling scenarios where different formats (like ebooks and audiobooks) can coexist in the same directory. If a book is already tracked at the target path, the files are merged.

Closes #523
This commit is contained in:
neon
2026-07-03 15:29:51 +00:00
parent 8a36f62b59
commit 1b46c7f3a0
5 changed files with 451 additions and 20 deletions
@@ -125,9 +125,9 @@ const fileWriteManualTooltip = computed(() => {
if (saving.value) return 'Save in progress'
if (fileWriteManualDisabledReasonLabel.value) return fileWriteManualDisabledReasonLabel.value
if (fileWriteStatus.value?.reason === 'library_disabled') {
return 'Write metadata to file and rename now; automatic write-back is disabled for this library'
return 'Write metadata to file and rename on disk; automatic write-back is disabled for this library'
}
return `Write ${fileWriteFieldCountLabel.value} to ${fileWriteTargetSummary.value} and rename if pattern is set`
return `Write ${fileWriteFieldCountLabel.value} to ${fileWriteTargetSummary.value} and rename on disk`
})
const comicSectionOpen = ref(true)
@@ -716,11 +716,13 @@ function handleCoverChanged(source: 'extracted' | 'custom' | null) {
<button
class="flex-none flex items-center gap-1.5 h-8 px-2.5 sm:px-3 rounded-lg border border-input bg-background text-sm hover:bg-muted transition-colors disabled:opacity-40"
:disabled="writingAndRenaming || saving || fileWriteManualDisabledReasonLabel !== null"
aria-label="Write to File & Rename"
@click="handleWriteAndRename"
>
<Loader2 v-if="writingAndRenaming" class="size-3.5 animate-spin" />
<HardDriveDownload v-else class="size-3.5" />
<span class="hidden sm:inline">Write to file</span>
<span class="sm:hidden">Write & Rename</span>
<span class="hidden sm:inline">Write to File & Rename</span>
</button>
</TooltipTrigger>
<TooltipContent>{{ fileWriteManualTooltip }}</TooltipContent>
@@ -153,6 +153,99 @@ 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' };
const db = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([row]),
}),
};
const repo = new FileRenameRepository(db as never);
await expect(repo.findBookByExactFolderPath(3, '/library/Author/Book')).resolves.toEqual(row);
});
it('applyExistingFolderMerge reassigns files, deletes the source book, and fills a missing target primary', async () => {
const setCalls: unknown[] = [];
const deleteWhere = vi.fn().mockResolvedValue(undefined);
const tx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
for: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([{ libraryFolderId: 7, primaryFileId: null }]),
}),
update: vi.fn().mockImplementation(() => ({
set: vi.fn().mockImplementation((values: unknown) => {
setCalls.push(values);
return { where: vi.fn().mockResolvedValue(undefined) };
}),
})),
delete: vi.fn().mockReturnValue({ where: deleteWhere }),
};
const db = {
transaction: vi.fn().mockImplementation(async (callback: (value: unknown) => Promise<unknown>) => callback(tx)),
};
const repo = new FileRenameRepository(db as never);
await repo.applyExistingFolderMerge({
sourceBookId: 5,
targetBookId: 99,
fallbackPrimaryFileId: 10,
updates: [
{ id: 10, absolutePath: '/library/Author/Book/Book.epub', relPath: 'Author/Book/Book.epub' },
{ id: 11, absolutePath: '/library/Author/Book/old.opf', relPath: 'Author/Book/old.opf' },
],
});
expect(db.transaction).toHaveBeenCalledTimes(1);
expect(setCalls).toEqual([
expect.objectContaining({ bookId: 99, libraryFolderId: 7, absolutePath: '/library/Author/Book/Book.epub', relPath: 'Author/Book/Book.epub' }),
expect.objectContaining({ bookId: 99, libraryFolderId: 7, absolutePath: '/library/Author/Book/old.opf', relPath: 'Author/Book/old.opf' }),
expect.objectContaining({ primaryFileId: 10, status: 'present' }),
]);
expect(tx.delete).toHaveBeenCalledTimes(1);
expect(deleteWhere).toHaveBeenCalledTimes(1);
});
it('applyExistingFolderMerge does not replace an existing target primary file', async () => {
const setCalls: unknown[] = [];
const tx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
for: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([{ libraryFolderId: 7, primaryFileId: 42 }]),
}),
update: vi.fn().mockImplementation(() => ({
set: vi.fn().mockImplementation((values: unknown) => {
setCalls.push(values);
return { where: vi.fn().mockResolvedValue(undefined) };
}),
})),
delete: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) }),
};
const db = {
transaction: vi.fn().mockImplementation(async (callback: (value: unknown) => Promise<unknown>) => callback(tx)),
};
const repo = new FileRenameRepository(db as never);
await repo.applyExistingFolderMerge({
sourceBookId: 5,
targetBookId: 99,
fallbackPrimaryFileId: 10,
updates: [{ id: 10, absolutePath: '/library/Author/Book/Book.epub', relPath: 'Author/Book/Book.epub' }],
});
expect(setCalls.at(-1)).toEqual(expect.not.objectContaining({ primaryFileId: 10 }));
expect(setCalls.at(-1)).toEqual(expect.objectContaining({ status: 'present' }));
});
it('checkPathTakenByOtherBook only flags collisions from other books', async () => {
const db = {
select: vi
@@ -1,5 +1,5 @@
import { Inject, Injectable } from '@nestjs/common';
import { asc, eq, inArray } from 'drizzle-orm';
import { and, asc, eq, inArray } from 'drizzle-orm';
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
import { DB } from '../../db';
@@ -147,6 +147,68 @@ 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);
return row ?? null;
}
async applyExistingFolderMerge(input: {
sourceBookId: number;
targetBookId: number;
updates: BookFilePathUpdate[];
fallbackPrimaryFileId: number | null;
}): Promise<void> {
if (input.sourceBookId === input.targetBookId) return;
await this.db.transaction(async (tx) => {
const now = new Date();
const [target] = await tx
.select({ libraryFolderId: books.libraryFolderId, primaryFileId: books.primaryFileId })
.from(books)
.where(eq(books.id, input.targetBookId))
.for('update')
.limit(1);
if (!target) return;
for (const update of input.updates) {
await tx
.update(bookFiles)
.set({
bookId: input.targetBookId,
libraryFolderId: target.libraryFolderId,
absolutePath: update.absolutePath,
relPath: update.relPath,
updatedAt: now,
})
.where(eq(bookFiles.id, update.id));
}
await tx.delete(books).where(eq(books.id, input.sourceBookId));
await tx
.update(books)
.set({
...(target?.primaryFileId == null && input.fallbackPrimaryFileId != null ? { primaryFileId: input.fallbackPrimaryFileId } : {}),
status: 'present',
updatedAt: now,
})
.where(eq(books.id, input.targetBookId));
});
}
async checkPathTakenByOtherBook(absolutePath: string, currentBookId: number): Promise<boolean> {
const rows = await this.db
.select({ bookId: books.id })
@@ -74,6 +74,8 @@ describe('FileRenameService', () => {
findBookRenameData: vi.fn(),
checkPathTakenByOtherBook: vi.fn().mockResolvedValue(false),
applyFolderRename: vi.fn().mockResolvedValue(undefined),
findBookByExactFolderPath: vi.fn().mockResolvedValue(null),
applyExistingFolderMerge: vi.fn().mockResolvedValue(undefined),
findAllBookFiles: vi.fn().mockImplementation(async () => {
const data = await renameRepo.findBookRenameData();
if (!data) return [];
@@ -944,6 +946,181 @@ describe('FileRenameService', () => {
expect(mockRename).toHaveBeenNthCalledWith(3, '/library/2666.jpg', '/library/Frank Herbert/Dune/2666.jpg');
});
it('merges renamed files into an existing folder book when target filenames are free', 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.findAllBookFiles.mockResolvedValue([
{ id: 10, absolutePath: '/library/Incoming/old.epub', relPath: 'Incoming/old.epub', role: 'primary', format: 'epub' },
{ id: 11, absolutePath: '/library/Incoming/old.opf', relPath: 'Incoming/old.opf', role: 'metadata', format: 'opf' },
]);
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' }));
});
renameRepo.findBookByExactFolderPath.mockResolvedValue({
id: 99,
folderPath: '/library/Frank Herbert/Dune (1965)',
primaryFileId: 42,
status: 'present',
});
const result = await service.performRename(5, 12);
expect(result).toEqual(
expect.objectContaining({
status: 'success',
oldPath: '/library/Incoming/old.epub',
newPath: '/library/Frank Herbert/Dune (1965)/Dune (1965).epub',
}),
);
expect(renameRepo.findBookByExactFolderPath).toHaveBeenCalledWith(1, '/library/Frank Herbert/Dune (1965)');
expect(renameRepo.applyExistingFolderMerge).toHaveBeenCalledWith({
sourceBookId: 5,
targetBookId: 99,
fallbackPrimaryFileId: 10,
updates: [
{
id: 10,
absolutePath: '/library/Frank Herbert/Dune (1965)/Dune (1965).epub',
relPath: 'Frank Herbert/Dune (1965)/Dune (1965).epub',
},
{
id: 11,
absolutePath: '/library/Frank Herbert/Dune (1965)/old.opf',
relPath: 'Frank Herbert/Dune (1965)/old.opf',
},
],
});
expect(renameRepo.applyFolderRename).not.toHaveBeenCalled();
expect(mockRename).toHaveBeenNthCalledWith(1, '/library/Incoming/old.epub', '/library/Frank Herbert/Dune (1965)/Dune (1965).epub');
expect(mockRename).toHaveBeenNthCalledWith(2, '/library/Incoming/old.opf', '/library/Frank Herbert/Dune (1965)/old.opf');
});
it('skips existing-folder merge when the concrete target file already exists on disk', 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',
}),
);
mockAccess.mockImplementation((path: any) => {
const value = path.toString();
if (value === '/library/Frank Herbert/Dune (1965)' || value === '/library/Frank Herbert/Dune (1965)/Dune (1965).epub') {
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',
});
const result = await service.performRename(5, 12);
expect(result).toEqual(expect.objectContaining({ status: 'skipped', reason: 'target path already exists on disk' }));
expect(renameRepo.applyExistingFolderMerge).not.toHaveBeenCalled();
expect(renameRepo.applyFolderRename).not.toHaveBeenCalled();
expect(mockRename).not.toHaveBeenCalled();
});
it('moves into an existing unowned folder without treating the folder as a collision', 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.findAllBookFiles.mockResolvedValue([
{ id: 10, absolutePath: '/library/Incoming/old.epub', relPath: 'Incoming/old.epub', role: 'primary', format: 'epub' },
]);
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' }));
});
renameRepo.findBookByExactFolderPath.mockResolvedValue(null);
const result = await service.performRename(5, 12);
expect(result).toEqual(expect.objectContaining({ status: 'success' }));
expect(renameRepo.applyFolderRename).toHaveBeenCalledWith(
5,
[
{
id: 10,
absolutePath: '/library/Frank Herbert/Dune (1965)/Dune (1965).epub',
relPath: 'Frank Herbert/Dune (1965)/Dune (1965).epub',
},
],
'/library/Frank Herbert/Dune (1965)',
);
expect(renameRepo.applyExistingFolderMerge).not.toHaveBeenCalled();
expect(mockRename).toHaveBeenCalledTimes(1);
expect(mockRename).toHaveBeenCalledWith('/library/Incoming/old.epub', '/library/Frank Herbert/Dune (1965)/Dune (1965).epub');
});
it('rolls back disk moves when an existing-folder merge fails in the database', 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.findAllBookFiles.mockResolvedValue([
{ id: 10, absolutePath: '/library/Incoming/old.epub', relPath: 'Incoming/old.epub', role: 'primary', format: 'epub' },
{ id: 11, absolutePath: '/library/Incoming/old.jpg', relPath: 'Incoming/old.jpg', role: 'cover', format: 'jpg' },
]);
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' }));
});
renameRepo.findBookByExactFolderPath.mockResolvedValue({
id: 99,
folderPath: '/library/Frank Herbert/Dune (1965)',
primaryFileId: 42,
status: 'present',
});
renameRepo.applyExistingFolderMerge.mockRejectedValue(new Error('db unavailable'));
const result = await service.performRename(5, 12);
expect(result).toEqual(expect.objectContaining({ status: 'failed', reason: 'db unavailable' }));
expect(mockRename).toHaveBeenCalledTimes(4);
expect(mockRename).toHaveBeenNthCalledWith(1, '/library/Incoming/old.epub', '/library/Frank Herbert/Dune (1965)/Dune (1965).epub');
expect(mockRename).toHaveBeenNthCalledWith(2, '/library/Incoming/old.jpg', '/library/Frank Herbert/Dune (1965)/old.jpg');
expect(mockRename).toHaveBeenNthCalledWith(3, '/library/Frank Herbert/Dune (1965)/old.jpg', '/library/Incoming/old.jpg');
expect(mockRename).toHaveBeenNthCalledWith(4, '/library/Frank Herbert/Dune (1965)/Dune (1965).epub', '/library/Incoming/old.epub');
});
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(
@@ -194,30 +194,35 @@ export class FileRenameService implements OnModuleDestroy {
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 reason = 'target folder already exists on disk';
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);
if (existingTargetPath) {
const reason = 'target path already exists on disk';
this.logger.warn(
`[${FILE_RENAME_EVENT}] [end] bookId=${bookId} userId=${userId} durationMs=${Date.now() - startedAt} status=skipped reason="${sanitizeLogValue(reason)}" newPath="${sanitizeLogValue(existingTargetPath)}" - rename skipped: target already exists on disk`,
);
await this.notifyFailure(userId, bookId, `File rename skipped: ${reason}.`, suppressNotification);
return { status: 'skipped', reason, oldPath: currentAbsolutePath, newPath: newAbsolutePath, durationMs: Date.now() - startedAt };
}
}
} else {
const existingTargetPath = await this.findExistingTargetFilePath(allFiles, fileTargets);
if (existingTargetPath) {
const reason = 'target path already exists on disk';
this.logger.warn(
`[${FILE_RENAME_EVENT}] [end] bookId=${bookId} userId=${userId} durationMs=${Date.now() - startedAt} status=skipped reason="${sanitizeLogValue(reason)}" newPath="${sanitizeLogValue(newAbsolutePath)}" - rename skipped: target already exists on disk`,
);
await this.notifyFailure(userId, bookId, `File rename skipped: ${reason}.`, suppressNotification);
return { status: 'skipped', reason, oldPath: currentAbsolutePath, newPath: newAbsolutePath, durationMs: Date.now() - startedAt };
}
} else {
for (const file of allFiles) {
const targetPath = fileTargets.get(file.id)!;
if (targetPath !== file.absolutePath) {
if (await this.pathExists(targetPath)) {
const reason = 'target path already exists on disk';
this.logger.warn(
`[${FILE_RENAME_EVENT}] [end] bookId=${bookId} userId=${userId} durationMs=${Date.now() - startedAt} status=skipped reason="${sanitizeLogValue(reason)}" newPath="${sanitizeLogValue(newAbsolutePath)}" - rename skipped: target already exists on disk`,
);
await this.notifyFailure(userId, bookId, `File rename skipped: ${reason}.`, suppressNotification);
return { status: 'skipped', reason, oldPath: currentAbsolutePath, newPath: newAbsolutePath, durationMs: Date.now() - startedAt };
}
}
}
}
for (const file of allFiles) {
@@ -242,7 +247,13 @@ export class FileRenameService implements OnModuleDestroy {
try {
if (bookHasOwnFolder && newFolderPath !== currentFolderPath) {
await this.renameBookWithFolder(bookId, data, currentFolderPath, newFolderPath, fileTargets);
if (mergeTargetBookId !== null) {
await this.mergeBookIntoExistingFolder(bookId, data, currentFolderPath, newFolderPath, fileTargets, mergeTargetBookId);
} else if (moveIntoExistingFolder) {
await this.renameBookIntoExistingFolder(bookId, data, currentFolderPath, newFolderPath, fileTargets);
} else {
await this.renameBookWithFolder(bookId, data, currentFolderPath, newFolderPath, fileTargets);
}
} else {
const nextBookFolderPath = isBookPerFolder ? newFolderPath : newAbsolutePath;
await this.renameBookFilesOnly(bookId, data, fileTargets, nextBookFolderPath);
@@ -379,6 +390,82 @@ export class FileRenameService implements OnModuleDestroy {
await this.tryRemoveEmptyDir(dirname(oldFolderPath));
}
private async renameBookIntoExistingFolder(
bookId: number,
data: BookRenameData,
oldFolderPath: string,
newFolderPath: string,
fileTargets: Map<number, string>,
): Promise<void> {
const allFiles = await this.renameRepo.findAllBookFiles(bookId);
const newUpdates = allFiles.map((file) => ({
id: file.id,
absolutePath: fileTargets.get(file.id)!,
relPath: relative(data.libraryFolderPath, fileTargets.get(file.id)!),
}));
const oldUpdates = allFiles.map((file) => ({ id: file.id, absolutePath: file.absolutePath, relPath: file.relPath }) satisfies BookFilePathUpdate);
await this.renameRepo.applyFolderRename(bookId, newUpdates, newFolderPath);
await this.moveBookFilesIndividually(bookId, allFiles, oldFolderPath, newFolderPath, fileTargets, oldUpdates);
await this.tryRemoveEmptyDir(oldFolderPath);
await this.tryRemoveEmptyDir(dirname(oldFolderPath));
}
private async mergeBookIntoExistingFolder(
bookId: number,
data: BookRenameData,
oldFolderPath: string,
newFolderPath: string,
fileTargets: Map<number, string>,
targetBookId: number,
): Promise<void> {
const allFiles = await this.renameRepo.findAllBookFiles(bookId);
const updates = allFiles.map((file) => ({
id: file.id,
absolutePath: fileTargets.get(file.id)!,
relPath: relative(data.libraryFolderPath, fileTargets.get(file.id)!),
}));
const movedFiles: Array<{ from: string; to: string }> = [];
try {
await mkdir(newFolderPath, { recursive: true });
for (const file of allFiles) {
const newFilePath = fileTargets.get(file.id)!;
const newFileDir = dirname(newFilePath);
if (newFileDir !== newFolderPath) {
await mkdir(newFileDir, { recursive: true });
}
if (file.absolutePath !== newFilePath) {
await this.lockService.withLock(file.absolutePath, () => fsRename(file.absolutePath, newFilePath));
movedFiles.push({ from: file.absolutePath, to: newFilePath });
}
}
await this.renameRepo.applyExistingFolderMerge({
sourceBookId: bookId,
targetBookId,
updates,
fallbackPrimaryFileId: data.file.id,
});
} catch (error) {
for (const { from, to } of [...movedFiles].reverse()) {
try {
await mkdir(dirname(from), { recursive: true });
await fsRename(to, from);
} catch (rollbackError) {
this.logRollbackFailure(bookId, error, rollbackError);
}
}
throw error;
}
await this.tryRemoveEmptyDir(oldFolderPath);
await this.tryRemoveEmptyDir(dirname(oldFolderPath));
}
private async moveBookFilesIndividually(
bookId: number,
allFiles: Awaited<ReturnType<FileRenameRepository['findAllBookFiles']>>,
@@ -512,6 +599,16 @@ export class FileRenameService implements OnModuleDestroy {
}
}
private async findExistingTargetFilePath(allFiles: RenameBookFile[], fileTargets: Map<number, string>): Promise<string | null> {
for (const file of allFiles) {
const targetPath = fileTargets.get(file.id)!;
if (targetPath !== file.absolutePath && (await this.pathExists(targetPath))) {
return targetPath;
}
}
return null;
}
private async tryRemoveEmptyDir(dirPath: string): Promise<void> {
try {
const entries = await readdir(dirPath);