mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(metadata): write finalized metadata back to the imported file (#1499)
* fix(metadata): write finalized metadata back to the imported file Finalizing a Book Dock file only moved the bytes, so a library with file writes enabled kept the metadata the file was uploaded with rather than the metadata the dock settled on. Schedule a write-back for every book a finalize produces once all of them have their metadata, attributed to the uploader. Fixes #1385 * fix(metadata): schedule dock file writes after cleanup Prevent a failed Book Dock cleanup from leaving a pending file write. Keep the integration assertion synchronized with the debounced writer and restore the missing directory stability mock for end-to-end coverage. --------- Co-authored-by: neon <18559658+neonsolstice@users.noreply.github.com>
This commit is contained in:
@@ -96,6 +96,9 @@ function makeService() {
|
||||
const notificationService = {
|
||||
notify: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const fileWriteService = {
|
||||
scheduleWrite: vi.fn(),
|
||||
};
|
||||
|
||||
const service = new BookDockFinalizeService(
|
||||
db as never,
|
||||
@@ -112,6 +115,7 @@ function makeService() {
|
||||
gateway as never,
|
||||
notificationService as never,
|
||||
processingState as never,
|
||||
fileWriteService as never,
|
||||
undefined as never,
|
||||
seriesMemberships as never,
|
||||
);
|
||||
@@ -133,6 +137,7 @@ function makeService() {
|
||||
processingState,
|
||||
seriesMemberships,
|
||||
notificationService,
|
||||
fileWriteService,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -144,6 +149,7 @@ function makeRow(overrides?: Partial<Record<string, unknown>>) {
|
||||
fileSize: 100,
|
||||
format: 'epub',
|
||||
status: 'ready',
|
||||
uploadedBy: 7,
|
||||
embeddedMetadata: { title: 'Embedded Title', genres: ['Embedded Genre'] } as BookDockMetadata,
|
||||
selectedMetadata: null as BookDockMetadata | null,
|
||||
fetchedMetadata: null as BookDockMetadata | null,
|
||||
@@ -1699,7 +1705,13 @@ describe('BookDockFinalizeService', () => {
|
||||
|
||||
function arrange(
|
||||
harness: ReturnType<typeof makeService>,
|
||||
options: { organizationMode?: string; formatPriority?: string[]; destPath?: string; importFormats?: string } = {},
|
||||
options: {
|
||||
organizationMode?: string;
|
||||
formatPriority?: string[];
|
||||
destPath?: string;
|
||||
importFormats?: string;
|
||||
fileWriteEnabled?: boolean;
|
||||
} = {},
|
||||
) {
|
||||
const { service } = harness;
|
||||
harness.appSettings.getBookRequestImportFormats.mockResolvedValue(options.importFormats ?? 'all');
|
||||
@@ -1710,6 +1722,7 @@ describe('BookDockFinalizeService', () => {
|
||||
fileNamingPattern: null,
|
||||
formatPriority: options.formatPriority ?? [],
|
||||
organizationMode: options.organizationMode ?? 'book_per_folder',
|
||||
fileWriteEnabled: options.fileWriteEnabled ?? false,
|
||||
} as never);
|
||||
vi.spyOn(service as never, 'findFolderOrFail').mockResolvedValue({ id: 9, libraryId: 5, path: '/library' } as never);
|
||||
vi.spyOn(service as never, 'resolveDestination').mockResolvedValue((options.destPath ?? '/library/Neuromancer/track-01.mp3') as never);
|
||||
@@ -1953,6 +1966,64 @@ describe('BookDockFinalizeService', () => {
|
||||
expect(harness.processor.reconcileCoversAsync).toHaveBeenCalledWith([81, 82]);
|
||||
});
|
||||
|
||||
describe('metadata write-back', () => {
|
||||
it('schedules a write for every book the unit produced, attributed to the uploader', async () => {
|
||||
const harness = makeService();
|
||||
harness.repo.findUnitFiles.mockResolvedValue(MULTI_FORMAT_FILES);
|
||||
arrange(harness, { organizationMode: 'book_per_file', destPath: '/library/Dune.epub', fileWriteEnabled: true });
|
||||
harness.processor.createUnitBookRecords.mockResolvedValue({ bookIds: [101, 102], createdBookIds: [101, 102], attachedFileIds: [] });
|
||||
|
||||
const result = await finalize(harness, multiFormatRow());
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(harness.fileWriteService.scheduleWrite.mock.calls).toEqual([
|
||||
[101, 'auto', 7],
|
||||
[102, 'auto', 7],
|
||||
]);
|
||||
});
|
||||
|
||||
it('does not schedule a write when the library does not write metadata to files', async () => {
|
||||
const harness = makeService();
|
||||
harness.repo.findUnitFiles.mockResolvedValue(AUDIO_UNIT_FILES);
|
||||
arrange(harness, { fileWriteEnabled: false });
|
||||
harness.processor.createUnitBookRecords.mockResolvedValue({ bookIds: [101], createdBookIds: [101], attachedFileIds: [] });
|
||||
|
||||
const result = await finalize(harness, unitRow());
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(harness.fileWriteService.scheduleWrite).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not schedule a write for any book when a later book of the unit fails', async () => {
|
||||
const harness = makeService();
|
||||
harness.repo.findUnitFiles.mockResolvedValue(MULTI_FORMAT_FILES);
|
||||
arrange(harness, { organizationMode: 'book_per_file', destPath: '/library/Dune.epub', fileWriteEnabled: true });
|
||||
harness.processor.createUnitBookRecords.mockResolvedValue({ bookIds: [101, 102], createdBookIds: [102], attachedFileIds: [9] });
|
||||
vi.spyOn(harness.service as never, 'applyMetadata')
|
||||
.mockResolvedValueOnce(undefined as never)
|
||||
.mockRejectedValueOnce(new Error('metadata exploded') as never);
|
||||
|
||||
const result = await finalize(harness, multiFormatRow());
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(harness.processor.deleteUnitBookRecords).toHaveBeenCalled();
|
||||
expect(harness.fileWriteService.scheduleWrite).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not schedule a write when dock record cleanup fails', async () => {
|
||||
const harness = makeService();
|
||||
harness.repo.findUnitFiles.mockResolvedValue(AUDIO_UNIT_FILES);
|
||||
arrange(harness, { fileWriteEnabled: true });
|
||||
harness.processor.createUnitBookRecords.mockResolvedValue({ bookIds: [101], createdBookIds: [101], attachedFileIds: [] });
|
||||
vi.spyOn(harness.service as never, 'cleanupBookDockRecord').mockRejectedValue(new Error('cleanup failed') as never);
|
||||
|
||||
const result = await finalize(harness, unitRow());
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(harness.fileWriteService.scheduleWrite).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* A pattern with no path separator resolves to a bare filename, and `dirname()` of that is the
|
||||
* library folder root. Without a floor every unit would land there and merge into one book.
|
||||
|
||||
@@ -53,6 +53,7 @@ import { DB } from '../../db';
|
||||
import * as schema from '../../db/schema';
|
||||
import { bookMetadata, libraries, libraryFolders } from '../../db/schema';
|
||||
import { AppSettingsService } from '../app-settings/app-settings.service';
|
||||
import { FileWriteService } from '../file-write/file-write.service';
|
||||
import { LibraryService } from '../library/library.service';
|
||||
import { MetadataService } from '../metadata/metadata.service';
|
||||
import { MetadataScoreService } from '../metadata-score/metadata-score.service';
|
||||
@@ -234,6 +235,7 @@ export class BookDockFinalizeService implements OnModuleInit, OnApplicationBoots
|
||||
private readonly gateway: BookDockGateway,
|
||||
private readonly notificationService: NotificationService,
|
||||
private readonly processingState: BookDockProcessingStateService,
|
||||
private readonly fileWriteService: FileWriteService,
|
||||
@Optional() private readonly seriesIdentity?: SeriesIdentityService,
|
||||
@Optional() private readonly seriesMemberships?: SeriesMembershipService,
|
||||
) {
|
||||
@@ -419,6 +421,11 @@ export class BookDockFinalizeService implements OnModuleInit, OnApplicationBoots
|
||||
|
||||
this.processor.reconcileCoversAsync(written.bookIds);
|
||||
await this.cleanupBookDockRecord(row);
|
||||
if (library.fileWriteEnabled) {
|
||||
for (const created of written.bookIds) {
|
||||
this.fileWriteService.scheduleWrite(created, 'auto', row.uploadedBy ?? undefined);
|
||||
}
|
||||
}
|
||||
existingDestinations.set(this.destinationKey(library.id, destPath), bookId);
|
||||
existingDestinations.set(this.destinationKey(library.id, persistedDestPath), bookId);
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import 'reflect-metadata';
|
||||
|
||||
vi.mock('../app-settings/app-settings.module', () => ({ AppSettingsModule: class AppSettingsModule {} }));
|
||||
vi.mock('../auth/auth.module', () => ({ AuthModule: class AuthModule {} }));
|
||||
vi.mock('../file-write/file-write.module', () => ({ FileWriteModule: class FileWriteModule {} }));
|
||||
vi.mock('../library/library.module', () => ({ LibraryModule: class LibraryModule {} }));
|
||||
vi.mock('../metadata-fetch/metadata-fetch.module', () => ({ MetadataFetchModule: class MetadataFetchModule {} }));
|
||||
vi.mock('../metadata/metadata.module', () => ({ MetadataModule: class MetadataModule {} }));
|
||||
|
||||
@@ -6,6 +6,7 @@ import type { StringValue } from 'ms';
|
||||
import { AppSettingsModule } from '../app-settings/app-settings.module';
|
||||
import { AuthModule } from '../auth/auth.module';
|
||||
import { BookModule } from '../book/book.module';
|
||||
import { FileWriteModule } from '../file-write/file-write.module';
|
||||
import { LibraryModule } from '../library/library.module';
|
||||
import { MetadataFetchModule } from '../metadata-fetch/metadata-fetch.module';
|
||||
import { MetadataModule } from '../metadata/metadata.module';
|
||||
@@ -28,6 +29,7 @@ import { BookDockRepository } from './book-dock.repository';
|
||||
UploadModule,
|
||||
AuthModule,
|
||||
BookModule,
|
||||
FileWriteModule,
|
||||
LibraryModule,
|
||||
MetadataFetchModule,
|
||||
MetadataModule,
|
||||
|
||||
@@ -12,6 +12,8 @@ import {
|
||||
} from '@bookorbit/types';
|
||||
|
||||
import * as schema from '../src/db/schema';
|
||||
import { parseFb2File } from '../src/modules/metadata/lib/fb2-parser';
|
||||
import { waitForCondition } from './e2e/app-harness';
|
||||
import { buildFb2Fixture } from './e2e/book-dock/book-dock-fixture-builder';
|
||||
import { createPdfFixture } from './e2e/metadata-write/metadata-write-fixture-builder';
|
||||
import {
|
||||
@@ -525,6 +527,57 @@ describe('Book Dock ingest + finalize (e2e)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('finalize writes the dock metadata into the file when the library writes metadata to files', async () => {
|
||||
const destination = await createLibraryWithFolder(context, {
|
||||
fileWriteEnabled: true,
|
||||
fileWriteFb2Enabled: true,
|
||||
});
|
||||
const uploader = await createUserAndLogin(context, { permissions: [Permission.ManageBookDock] });
|
||||
const bookDockRow = await createBookDockRow(context, {
|
||||
fileName: 'write-back.fb2',
|
||||
content: buildFb2Fixture({ title: 'Uploaded File Title', authors: ['Uploaded File Author'] }),
|
||||
embeddedMetadata: { title: 'Uploaded File Title', authors: ['Uploaded File Author'] },
|
||||
selectedMetadata: { title: 'Dock Edited Title', authors: ['Dock Edited Author'] },
|
||||
targetLibraryId: destination.libraryId,
|
||||
targetFolderId: destination.libraryFolderId,
|
||||
uploadedBy: uploader.userId,
|
||||
});
|
||||
|
||||
const response = await context.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/book-dock/finalize',
|
||||
headers: authHeader(context.adminToken),
|
||||
payload: { fileIds: [bookDockRow.id] },
|
||||
});
|
||||
|
||||
expect(response.statusCode).toBe(201);
|
||||
const body = response.json() as BookDockFinalizeResult;
|
||||
expect(body).toMatchObject({ total: 1, succeeded: 1, failed: 0 });
|
||||
const finalizedBookId = body.results[0]!.bookId!;
|
||||
|
||||
const [bookFile] = await context.db
|
||||
.select({ absolutePath: schema.bookFiles.absolutePath })
|
||||
.from(schema.bookFiles)
|
||||
.where(eq(schema.bookFiles.bookId, finalizedBookId))
|
||||
.limit(1);
|
||||
expect(bookFile).toBeDefined();
|
||||
|
||||
// The write is debounced, so the file is polled rather than drained: draining cancels a timer
|
||||
// that has not fired yet.
|
||||
await waitForCondition(async () => {
|
||||
const parsed = await parseFb2File(bookFile!.absolutePath);
|
||||
expect(parsed?.title).toBe('Dock Edited Title');
|
||||
expect(parsed?.authors.map((author) => author.name)).toEqual(['Dock Edited Author']);
|
||||
|
||||
const [writeLog] = await context.db
|
||||
.select({ status: schema.fileWriteLog.status, triggeredBy: schema.fileWriteLog.triggeredBy, userId: schema.fileWriteLog.userId })
|
||||
.from(schema.fileWriteLog)
|
||||
.where(eq(schema.fileWriteLog.bookId, finalizedBookId))
|
||||
.limit(1);
|
||||
expect(writeLog).toMatchObject({ status: 'success', triggeredBy: 'auto', userId: uploader.userId });
|
||||
});
|
||||
});
|
||||
|
||||
it('finalize returns partial success with duplicate and destination conflicts', async () => {
|
||||
const destination = await createLibraryWithFolder(context);
|
||||
|
||||
|
||||
@@ -2,4 +2,5 @@ import { vi } from 'vitest';
|
||||
|
||||
vi.mock('../src/common/utils/fs-stability.utils', () => ({
|
||||
waitForStability: vi.fn().mockResolvedValue(undefined),
|
||||
waitForDirectoryStability: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
@@ -27,6 +27,7 @@ const ADMIN_SETUP_DTO = {
|
||||
|
||||
interface EnvSnapshot {
|
||||
appDataPath: string | undefined;
|
||||
fileWriteDebounceMs: string | undefined;
|
||||
}
|
||||
|
||||
export interface BookDockE2EContext {
|
||||
@@ -81,9 +82,13 @@ export async function createBookDockE2EContext(): Promise<BookDockE2EContext> {
|
||||
const fixture = await createBookDockFixtureRoot();
|
||||
const envSnapshot: EnvSnapshot = {
|
||||
appDataPath: process.env.APP_DATA_PATH,
|
||||
fileWriteDebounceMs: process.env.FILE_WRITE_DEBOUNCE_MS,
|
||||
};
|
||||
|
||||
process.env.APP_DATA_PATH = fixture.booksPath;
|
||||
// Read at config-factory time and cached in the FileWriteService constructor, so it has to be set
|
||||
// before the module compiles.
|
||||
process.env.FILE_WRITE_DEBOUNCE_MS = '25';
|
||||
|
||||
const moduleFixture = await Test.createTestingModule({
|
||||
imports: [AppModule],
|
||||
@@ -137,6 +142,8 @@ export async function createLibraryWithFolder(
|
||||
mode?: 'book_per_file' | 'book_per_folder';
|
||||
allowedFormats?: string[];
|
||||
name?: string;
|
||||
fileWriteEnabled?: boolean;
|
||||
fileWriteFb2Enabled?: boolean;
|
||||
} = {},
|
||||
): Promise<CreatedLibrary> {
|
||||
const folderPath = join(ctx.fixture.booksPath, `library-${randomUUID()}`);
|
||||
@@ -150,6 +157,17 @@ export async function createLibraryWithFolder(
|
||||
name: options.name,
|
||||
});
|
||||
|
||||
// FB2 writes have their own toggle which is off by default, so `fileWriteEnabled` alone still skips them.
|
||||
if (options.fileWriteEnabled !== undefined || options.fileWriteFb2Enabled !== undefined) {
|
||||
await ctx.db
|
||||
.update(schema.libraries)
|
||||
.set({
|
||||
...(options.fileWriteEnabled !== undefined ? { fileWriteEnabled: options.fileWriteEnabled } : {}),
|
||||
...(options.fileWriteFb2Enabled !== undefined ? { fileWriteFb2Enabled: options.fileWriteFb2Enabled } : {}),
|
||||
})
|
||||
.where(eq(schema.libraries.id, libraryId));
|
||||
}
|
||||
|
||||
return {
|
||||
libraryId,
|
||||
libraryFolderId,
|
||||
@@ -391,4 +409,6 @@ async function setSetting(db: Db, key: string, value: string): Promise<void> {
|
||||
function restoreEnv(snapshot: EnvSnapshot): void {
|
||||
if (snapshot.appDataPath === undefined) delete process.env.APP_DATA_PATH;
|
||||
else process.env.APP_DATA_PATH = snapshot.appDataPath;
|
||||
if (snapshot.fileWriteDebounceMs === undefined) delete process.env.FILE_WRITE_DEBOUNCE_MS;
|
||||
else process.env.FILE_WRITE_DEBOUNCE_MS = snapshot.fileWriteDebounceMs;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user