mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 08:21:39 -06:00
feat(book): implement file rename and delete interactions
- Adds rename and delete file capabilities to Book Details > Files tab - Automatically promotes a fallback file to primary upon deletion - Refactors path update logic to correctly apply db folder path updates - Ensures file type format badges in header search use global format colors - Replaces legacy file property overrides with rename-only DTO Closes #214
This commit is contained in:
@@ -46,6 +46,7 @@ import { UpsertAudioProgressDto } from './dto/upsert-audio-progress.dto';
|
||||
import { UpdateBookMetadataAndLocksDto } from './dto/update-book-metadata-and-locks.dto';
|
||||
import { UpdateBookMetadataDto } from './dto/update-book-metadata.dto';
|
||||
import { SearchBooksDto } from './dto/search-books.dto';
|
||||
import { UpdateBookFileDto } from './dto/update-book-file.dto';
|
||||
import { SetStatusDto } from '../user-book-status/dto/set-status.dto';
|
||||
import { Permission, AuditAction, AuditResource } from '@bookorbit/types';
|
||||
import type { BookQuery } from '@bookorbit/types';
|
||||
@@ -501,6 +502,18 @@ export class BookController {
|
||||
await this.bookService.clearFileProgress(user.id, fileId, user);
|
||||
}
|
||||
|
||||
@Patch('files/:fileId')
|
||||
@HttpCode(HttpStatus.NO_CONTENT)
|
||||
async renameFile(@Param('fileId', ParseIntPipe) fileId: number, @Body() dto: UpdateBookFileDto, @CurrentUser() user: RequestUser) {
|
||||
await this.bookService.renameFile(fileId, dto, user);
|
||||
}
|
||||
|
||||
@Delete('files/:fileId')
|
||||
@HttpCode(HttpStatus.NO_CONTENT)
|
||||
async deleteFile(@Param('fileId', ParseIntPipe) fileId: number, @CurrentUser() user: RequestUser) {
|
||||
await this.bookService.deleteFile(fileId, user);
|
||||
}
|
||||
|
||||
@Get(':id/audio-progress')
|
||||
async getAudioProgress(@Param('id', ParseIntPipe) id: number, @CurrentUser() user: RequestUser) {
|
||||
return (await this.bookService.getAudioProgress(user.id, id, user)) ?? null;
|
||||
|
||||
@@ -613,10 +613,12 @@ export class BookRepository {
|
||||
id: bookFiles.id,
|
||||
absolutePath: bookFiles.absolutePath,
|
||||
format: bookFiles.format,
|
||||
role: bookFiles.role,
|
||||
bookId: bookFiles.bookId,
|
||||
libraryId: books.libraryId,
|
||||
fileHash: bookFiles.fileHash,
|
||||
sizeBytes: bookFiles.sizeBytes,
|
||||
durationSeconds: bookFiles.durationSeconds,
|
||||
})
|
||||
.from(bookFiles)
|
||||
.innerJoin(books, eq(books.id, bookFiles.bookId))
|
||||
@@ -625,6 +627,36 @@ export class BookRepository {
|
||||
return file ?? null;
|
||||
}
|
||||
|
||||
async deleteBookFile(fileId: number): Promise<void> {
|
||||
await this.db.delete(bookFiles).where(eq(bookFiles.id, fileId));
|
||||
}
|
||||
|
||||
async updateBookFile(fileId: number, data: { format?: string | null; role?: string; absolutePath?: string; sizeBytes?: number }): Promise<void> {
|
||||
await this.db
|
||||
.update(bookFiles)
|
||||
.set({ ...data, updatedAt: new Date() })
|
||||
.where(eq(bookFiles.id, fileId));
|
||||
}
|
||||
|
||||
async updateBookPrimaryFile(bookId: number, primaryFileId: number | null): Promise<void> {
|
||||
await this.db.update(books).set({ primaryFileId, updatedAt: new Date() }).where(eq(books.id, bookId));
|
||||
}
|
||||
|
||||
async findFilesForBook(bookId: number) {
|
||||
return this.db
|
||||
.select({
|
||||
id: bookFiles.id,
|
||||
role: bookFiles.role,
|
||||
})
|
||||
.from(bookFiles)
|
||||
.where(eq(bookFiles.bookId, bookId));
|
||||
}
|
||||
|
||||
async findBookBase(bookId: number) {
|
||||
const [row] = await this.db.select().from(books).where(eq(books.id, bookId)).limit(1);
|
||||
return row ?? null;
|
||||
}
|
||||
|
||||
async findProgress(userId: number, fileId: number) {
|
||||
const [row] = await this.db
|
||||
.select()
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { BadRequestException, ConflictException, ForbiddenException, Logger, NotFoundException } from '@nestjs/common';
|
||||
import type { MockedFunction } from 'vitest';
|
||||
import { access, readdir, rm, stat } from 'fs/promises';
|
||||
import { access, readdir, rm, stat, rename } from 'fs/promises';
|
||||
|
||||
import type { RequestUser } from '../../common/types/request-user';
|
||||
import { AUDIO_BOOK_FILE_WRITE_FIELDS, MetadataProviderKey, Permission, type BookQuery, type MetadataFetchDiagnostics } from '@bookorbit/types';
|
||||
@@ -24,6 +24,7 @@ vi.mock('fs/promises', async () => {
|
||||
access: vi.fn(),
|
||||
readdir: vi.fn(),
|
||||
rm: vi.fn(),
|
||||
rename: vi.fn(),
|
||||
stat: vi.fn(),
|
||||
};
|
||||
});
|
||||
@@ -3947,4 +3948,108 @@ describe('BookService', () => {
|
||||
expect(result.rename.reason).toContain('unavailable');
|
||||
});
|
||||
});
|
||||
|
||||
describe('renameFile', () => {
|
||||
it('renames file on disk and updates db', async () => {
|
||||
const { service, bookRepo, libraryService } = makeService();
|
||||
const user = makeUser({ id: 1 });
|
||||
const fileId = 100;
|
||||
const file = { absolutePath: '/path/to/old.epub', filename: 'old.epub', format: 'epub', role: 'content', bookId: 10, libraryId: 1 };
|
||||
|
||||
bookRepo.findFileById = vi.fn().mockResolvedValue(file);
|
||||
libraryService.checkLibraryAccess = vi.fn().mockResolvedValue(true);
|
||||
|
||||
vi.mocked(rename).mockResolvedValue(undefined);
|
||||
bookRepo.updateBookFile = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await service.renameFile(fileId, { filename: 'new.epub' }, user);
|
||||
|
||||
expect(rename).toHaveBeenCalledWith('/path/to/old.epub', '/path/to/new.epub');
|
||||
expect(bookRepo.updateBookFile).toHaveBeenCalledWith(fileId, {
|
||||
absolutePath: '/path/to/new.epub',
|
||||
});
|
||||
});
|
||||
|
||||
it('throws BadRequestException for invalid filename', async () => {
|
||||
const { service, bookRepo, libraryService } = makeService();
|
||||
const user = makeUser({ id: 1 });
|
||||
const fileId = 100;
|
||||
const file = { absolutePath: '/path/to/old.epub', filename: 'old.epub', format: 'epub', role: 'content', bookId: 10, libraryId: 1 };
|
||||
|
||||
bookRepo.findFileById = vi.fn().mockResolvedValue(file);
|
||||
libraryService.checkLibraryAccess = vi.fn().mockResolvedValue(true);
|
||||
|
||||
await expect(service.renameFile(fileId, { filename: '../new.epub' }, user)).rejects.toThrow(BadRequestException);
|
||||
await expect(service.renameFile(fileId, { filename: 'dir/new.epub' }, user)).rejects.toThrow(BadRequestException);
|
||||
});
|
||||
|
||||
it('only updates db if filename is not provided or unchanged', async () => {
|
||||
const { service, bookRepo, libraryService } = makeService();
|
||||
const user = makeUser({ id: 1 });
|
||||
const fileId = 100;
|
||||
const file = { absolutePath: '/path/to/old.epub', filename: 'old.epub', format: 'epub', role: 'content', bookId: 10, libraryId: 1 };
|
||||
|
||||
bookRepo.findFileById = vi.fn().mockResolvedValue(file);
|
||||
libraryService.checkLibraryAccess = vi.fn().mockResolvedValue(true);
|
||||
|
||||
vi.mocked(rename).mockResolvedValue(undefined);
|
||||
bookRepo.updateBookFile = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await service.renameFile(fileId, { filename: 'old.epub' }, user);
|
||||
|
||||
expect(rename).not.toHaveBeenCalled();
|
||||
expect(bookRepo.updateBookFile).toHaveBeenCalledWith(fileId, {
|
||||
absolutePath: undefined,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('deleteFile', () => {
|
||||
it('deletes file on disk and updates db and marks missing if last file', async () => {
|
||||
const { service, bookRepo, libraryService } = makeService();
|
||||
const user = makeUser({ id: 1 });
|
||||
const fileId = 100;
|
||||
const file = { absolutePath: '/path/to/old.epub', bookId: 10, libraryId: 1 };
|
||||
|
||||
bookRepo.findFileById = vi.fn().mockResolvedValue(file);
|
||||
libraryService.checkLibraryAccess = vi.fn().mockResolvedValue(true);
|
||||
|
||||
vi.mocked(rm).mockResolvedValue(undefined);
|
||||
bookRepo.deleteBookFile = vi.fn().mockResolvedValue(undefined);
|
||||
bookRepo.findFilesForBook = vi.fn().mockResolvedValue([]);
|
||||
bookRepo.findBookBase = vi.fn().mockResolvedValue({ id: 10, primaryFileId: 100 });
|
||||
bookRepo.updateBookPrimaryFile = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await service.deleteFile(fileId, user);
|
||||
|
||||
expect(rm).toHaveBeenCalledWith('/path/to/old.epub', { force: true });
|
||||
expect(bookRepo.deleteBookFile).toHaveBeenCalledWith(fileId);
|
||||
expect(bookRepo.updateBookPrimaryFile).toHaveBeenCalledWith(10, null);
|
||||
});
|
||||
|
||||
it('elects new primary file if deleted file was primary and other files exist', async () => {
|
||||
const { service, bookRepo, libraryService } = makeService();
|
||||
const user = makeUser({ id: 1 });
|
||||
const fileId = 100;
|
||||
const file = { absolutePath: '/path/to/old.epub', bookId: 10, libraryId: 1 };
|
||||
|
||||
bookRepo.findFileById = vi.fn().mockResolvedValue(file);
|
||||
libraryService.checkLibraryAccess = vi.fn().mockResolvedValue(true);
|
||||
|
||||
vi.mocked(rm).mockResolvedValue(undefined);
|
||||
bookRepo.deleteBookFile = vi.fn().mockResolvedValue(undefined);
|
||||
bookRepo.findFilesForBook = vi.fn().mockResolvedValue([
|
||||
{ id: 100, role: 'content' },
|
||||
{ id: 101, role: 'content' },
|
||||
]);
|
||||
bookRepo.findBookBase = vi.fn().mockResolvedValue({ id: 10, primaryFileId: 100 });
|
||||
bookRepo.updateBookPrimaryFile = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await service.deleteFile(fileId, user);
|
||||
|
||||
expect(rm).toHaveBeenCalledWith('/path/to/old.epub', { force: true });
|
||||
expect(bookRepo.deleteBookFile).toHaveBeenCalledWith(fileId);
|
||||
expect(bookRepo.updateBookPrimaryFile).toHaveBeenCalledWith(10, 101);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
Optional,
|
||||
} from '@nestjs/common';
|
||||
import { ConfigService } from '@nestjs/config';
|
||||
import { access, readdir, rm, stat } from 'fs/promises';
|
||||
import { access, readdir, rm, stat, rename } from 'fs/promises';
|
||||
import { inArray, type SQL } from 'drizzle-orm';
|
||||
|
||||
import { bookCoverDirPath, bookThumbnailPath, findPreferredBookCoverFileName } from '../../common/book-cover-storage';
|
||||
@@ -52,6 +52,7 @@ import type {
|
||||
import type { ContentFilterRules } from '@bookorbit/types';
|
||||
import { assembleBookCards, assembleCollapsedBookCards } from './utils/assemble-book-cards';
|
||||
import type { RequestUser } from '../../common/types/request-user';
|
||||
import { UpdateBookFileDto } from './dto/update-book-file.dto';
|
||||
import { AppSettingsService } from '../app-settings/app-settings.service';
|
||||
import { BookEmbedderService } from '../embedding/book-embedder.service';
|
||||
import { MetadataService } from '../metadata/metadata.service';
|
||||
@@ -1147,6 +1148,88 @@ export class BookService {
|
||||
return this.resolveDownloadFilenameForFile(file);
|
||||
}
|
||||
|
||||
async renameFile(fileId: number, dto: UpdateBookFileDto, user: RequestUser): Promise<void> {
|
||||
const event = 'book.rename_file';
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(`[${event}] [start] fileId=${fileId} userId=${user.id} - rename file started`);
|
||||
try {
|
||||
const file = await this.verifyFileAccess(fileId, user);
|
||||
|
||||
let newAbsolutePath = file.absolutePath;
|
||||
if (dto.filename && dto.filename !== basename(file.absolutePath)) {
|
||||
if (dto.filename.includes('/') || dto.filename.includes('\\')) {
|
||||
throw new BadRequestException('Filename cannot contain path separators');
|
||||
}
|
||||
newAbsolutePath = join(dirname(file.absolutePath), dto.filename);
|
||||
if (newAbsolutePath !== file.absolutePath) {
|
||||
try {
|
||||
await rename(file.absolutePath, newAbsolutePath);
|
||||
} catch (err) {
|
||||
throw new BadRequestException(`Failed to rename file on disk: ${err instanceof Error ? err.message : String(err)}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
await this.bookRepo.updateBookFile(fileId, {
|
||||
absolutePath: newAbsolutePath !== file.absolutePath ? newAbsolutePath : undefined,
|
||||
});
|
||||
|
||||
this.logger.log(`[${event}] [end] fileId=${fileId} durationMs=${Date.now() - startedAt} - rename file completed`);
|
||||
} catch (err) {
|
||||
const errorClass = err instanceof Error ? err.name : 'Error';
|
||||
const errorMessage = sanitizeLogValue(err instanceof Error ? err.message : String(err));
|
||||
this.logger.warn(
|
||||
`[${event}] [fail] fileId=${fileId} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - rename file failed`,
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
async deleteFile(fileId: number, user: RequestUser): Promise<void> {
|
||||
const event = 'book.delete_file';
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(`[${event}] [start] fileId=${fileId} userId=${user.id} - delete file started`);
|
||||
try {
|
||||
const file = await this.verifyFileAccess(fileId, user);
|
||||
|
||||
try {
|
||||
await rm(file.absolutePath, { force: true });
|
||||
} catch {
|
||||
this.logger.warn(`Failed to physically delete file at ${file.absolutePath}`);
|
||||
}
|
||||
|
||||
// the file watcher will eventually catch the unlink and clean up the database.
|
||||
// however, to be responsive, we can manually clean up the database here too,
|
||||
// but if the file is the last file, the scanner logic is better suited to mark the book missing.
|
||||
// So we leave the DB cleanup to the file watcher, which is more robust.
|
||||
const book = await this.bookRepo.findBookBase(file.bookId);
|
||||
const wasPrimary = book?.primaryFileId === fileId;
|
||||
|
||||
await this.bookRepo.deleteBookFile(fileId);
|
||||
|
||||
const allFiles = await this.bookRepo.findFilesForBook(file.bookId);
|
||||
// deleteBookFile already removes it, but just in case
|
||||
const remaining = allFiles.filter((f) => f.id !== fileId);
|
||||
if (remaining.length === 0) {
|
||||
// mark book as missing if no files left
|
||||
await this.bookRepo.updateBookPrimaryFile(file.bookId, null);
|
||||
} else if (wasPrimary) {
|
||||
// pick the first remaining content file or just the first remaining
|
||||
const newPrimary = remaining.find((f) => f.role === 'content') || remaining[0];
|
||||
await this.bookRepo.updateBookPrimaryFile(file.bookId, newPrimary?.id ?? null);
|
||||
}
|
||||
|
||||
this.logger.log(`[${event}] [end] fileId=${fileId} durationMs=${Date.now() - startedAt} - delete file completed`);
|
||||
} catch (err) {
|
||||
const errorClass = err instanceof Error ? err.name : 'Error';
|
||||
const errorMessage = sanitizeLogValue(err instanceof Error ? err.message : String(err));
|
||||
this.logger.warn(
|
||||
`[${event}] [fail] fileId=${fileId} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - delete file failed`,
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
async searchAcrossLibraries(q: string, limit: number, user: RequestUser) {
|
||||
const libs = await this.libraryService.findAll(user);
|
||||
const libraryIds = libs.map((l) => l.id);
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
import { IsString, IsOptional, IsNotEmpty } from 'class-validator';
|
||||
|
||||
export class UpdateBookFileDto {
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
@IsOptional()
|
||||
filename?: string;
|
||||
}
|
||||
@@ -82,9 +82,9 @@ describe('FileRenameRepository', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('applyFileRename updates file and book paths in one transaction', async () => {
|
||||
it('updateBookFilePath updates absolutePath and relPath', async () => {
|
||||
const setCalls: unknown[] = [];
|
||||
const tx = {
|
||||
const db = {
|
||||
update: vi.fn().mockImplementation(() => ({
|
||||
set: vi.fn().mockImplementation((values: unknown) => {
|
||||
setCalls.push(values);
|
||||
@@ -92,19 +92,32 @@ describe('FileRenameRepository', () => {
|
||||
}),
|
||||
})),
|
||||
};
|
||||
|
||||
const repo = new FileRenameRepository(db as never);
|
||||
|
||||
await repo.updateBookFilePath(10, '/library/new-folder/Dune.epub', 'new-folder/Dune.epub');
|
||||
|
||||
expect(db.update).toHaveBeenCalledTimes(1);
|
||||
expect(setCalls).toEqual([{ absolutePath: '/library/new-folder/Dune.epub', relPath: 'new-folder/Dune.epub' }]);
|
||||
});
|
||||
|
||||
it('updateBookFolderPath updates folderPath', async () => {
|
||||
const setCalls: unknown[] = [];
|
||||
const db = {
|
||||
transaction: vi.fn().mockImplementation(async (callback: (value: unknown) => Promise<unknown>) => callback(tx)),
|
||||
update: vi.fn().mockImplementation(() => ({
|
||||
set: vi.fn().mockImplementation((values: unknown) => {
|
||||
setCalls.push(values);
|
||||
return { where: vi.fn().mockResolvedValue(undefined) };
|
||||
}),
|
||||
})),
|
||||
};
|
||||
|
||||
const repo = new FileRenameRepository(db as never);
|
||||
|
||||
await repo.applyFileRename(5, 10, '/library/new-folder/Dune.epub', 'new-folder/Dune.epub', '/library/new-folder/Dune.epub');
|
||||
await repo.updateBookFolderPath(5, '/library/new-folder');
|
||||
|
||||
expect(db.transaction).toHaveBeenCalledTimes(1);
|
||||
expect(setCalls).toEqual([
|
||||
{ absolutePath: '/library/new-folder/Dune.epub', relPath: 'new-folder/Dune.epub' },
|
||||
{ folderPath: '/library/new-folder/Dune.epub' },
|
||||
]);
|
||||
expect(db.update).toHaveBeenCalledTimes(1);
|
||||
expect(setCalls).toEqual([{ folderPath: '/library/new-folder' }]);
|
||||
});
|
||||
|
||||
it('applyFolderRename updates every file path and the stored book folder', async () => {
|
||||
|
||||
Reference in New Issue
Block a user