mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 08:21:39 -06:00
fix(metadata): refetch Book Dock metadata on demand
Allow ready Book Dock rows to refresh provider matches without depending on the failed-only bulk retry path. Preserve bounded queueing, ownership checks, and stale metadata cleanup. Fixes #1118
This commit is contained in:
@@ -316,6 +316,63 @@ describe('BookDockIngestService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('refetchMetadata', () => {
|
||||
it.each(['ready', 'error'])('queues a %s row and forces a provider fetch', async (status) => {
|
||||
const { service, repo } = makeService();
|
||||
const dockRow = {
|
||||
id: 4,
|
||||
status,
|
||||
format: 'epub',
|
||||
absolutePath: '/bucket/4.epub',
|
||||
errorMessage: status === 'error' ? 'failed' : null,
|
||||
};
|
||||
repo.findById.mockResolvedValueOnce(dockRow).mockResolvedValueOnce({ ...dockRow, status: 'pending' });
|
||||
repo.update.mockResolvedValue(dockRow);
|
||||
const autoFetchSpy = vi.spyOn(service as any, 'autoFetchMetadataAsync').mockResolvedValue(undefined);
|
||||
|
||||
await expect(service.refetchMetadata(4)).resolves.toBe(true);
|
||||
await (service as any).metadataQueue.waitForIdle();
|
||||
|
||||
expect(repo.update).toHaveBeenCalledWith(4, { status: 'pending', errorMessage: null });
|
||||
expect(autoFetchSpy).toHaveBeenCalledWith(4, true);
|
||||
expect((service as any).forcedAutoFetchFileIds.has(4)).toBe(false);
|
||||
});
|
||||
|
||||
it('ignores missing, active, and unsupported rows', async () => {
|
||||
const { service, repo } = makeService();
|
||||
repo.findById
|
||||
.mockResolvedValueOnce(null)
|
||||
.mockResolvedValueOnce({ id: 5, status: 'fetching', format: 'epub' })
|
||||
.mockResolvedValueOnce({ id: 6, status: 'ready', format: null, absolutePath: '/bucket/6' });
|
||||
|
||||
await expect(service.refetchMetadata(4)).resolves.toBe(false);
|
||||
await expect(service.refetchMetadata(5)).resolves.toBe(false);
|
||||
await expect(service.refetchMetadata(6)).resolves.toBe(false);
|
||||
|
||||
expect(repo.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('restores the previous state when the work queue rejects the file', async () => {
|
||||
const { service, repo } = makeService();
|
||||
const dockRow = {
|
||||
id: 7,
|
||||
status: 'ready',
|
||||
format: 'epub',
|
||||
absolutePath: '/bucket/7.epub',
|
||||
errorMessage: null,
|
||||
};
|
||||
repo.findById.mockResolvedValue(dockRow);
|
||||
repo.update.mockResolvedValue(dockRow);
|
||||
vi.spyOn(service as any, 'extractMetadataAsync').mockReturnValue(false);
|
||||
|
||||
await expect(service.refetchMetadata(7)).resolves.toBe(false);
|
||||
|
||||
expect(repo.update).toHaveBeenNthCalledWith(1, 7, { status: 'pending', errorMessage: null });
|
||||
expect(repo.update).toHaveBeenNthCalledWith(2, 7, { status: 'ready', errorMessage: null });
|
||||
expect((service as any).forcedAutoFetchFileIds.has(7)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('autoFetchMetadataAsync', () => {
|
||||
it('returns early when auto-fetch is disabled', async () => {
|
||||
const { service, appSettings, repo } = makeService();
|
||||
@@ -327,6 +384,34 @@ describe('BookDockIngestService', () => {
|
||||
expect(repo.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('forces an explicit refetch when automatic fetching is disabled', async () => {
|
||||
const { service, appSettings, repo, metadataFetchPipeline } = makeService();
|
||||
appSettings.isBookDockAutoFetchEnabled.mockResolvedValue(false);
|
||||
repo.findById.mockResolvedValue({
|
||||
id: 8,
|
||||
fileName: 'dune.epub',
|
||||
status: 'ready',
|
||||
embeddedMetadata: { title: 'Dune' },
|
||||
});
|
||||
(metadataFetchPipeline as any).runWithSources = vi.fn().mockResolvedValue({
|
||||
resolved: { title: 'Dune' },
|
||||
sources: { title: 'google' },
|
||||
});
|
||||
|
||||
await (service as any).autoFetchMetadataAsync(8, true);
|
||||
|
||||
expect(appSettings.isBookDockAutoFetchEnabled).not.toHaveBeenCalled();
|
||||
expect(metadataFetchPipeline.runWithSources).toHaveBeenCalledTimes(1);
|
||||
expect(repo.update).toHaveBeenLastCalledWith(
|
||||
8,
|
||||
expect.objectContaining({
|
||||
status: 'ready',
|
||||
fetchedMetadata: { title: 'Dune' },
|
||||
fetchedMetadataSources: { title: 'google' },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
fileName: 'Batman #007.cbz',
|
||||
@@ -572,6 +657,31 @@ describe('BookDockIngestService', () => {
|
||||
expect(repo.update).toHaveBeenNthCalledWith(2, 13, { status: 'ready' });
|
||||
});
|
||||
|
||||
it('clears stale provider metadata when an explicit refetch finds no result', async () => {
|
||||
const { service, appSettings, repo, metadataFetchPipeline } = makeService();
|
||||
appSettings.isBookDockAutoFetchEnabled.mockResolvedValue(false);
|
||||
repo.findById.mockResolvedValue({
|
||||
id: 15,
|
||||
fileName: 'known-title.epub',
|
||||
status: 'ready',
|
||||
embeddedMetadata: { title: 'Known Title' },
|
||||
fetchedMetadata: { title: 'Stale Title' },
|
||||
fetchedMetadataSources: { title: 'google' },
|
||||
confidence: 90,
|
||||
});
|
||||
(metadataFetchPipeline as any).runWithSources = vi.fn().mockResolvedValue({ resolved: {}, sources: {} });
|
||||
|
||||
await (service as any).autoFetchMetadataAsync(15, true);
|
||||
|
||||
expect(repo.update).toHaveBeenNthCalledWith(1, 15, { status: 'fetching' });
|
||||
expect(repo.update).toHaveBeenNthCalledWith(2, 15, {
|
||||
status: 'ready',
|
||||
fetchedMetadata: null,
|
||||
confidence: null,
|
||||
fetchedMetadataSources: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('emits a summary refresh before provider fetching starts', async () => {
|
||||
const { service, appSettings, repo, metadataFetchPipeline } = makeService();
|
||||
appSettings.isBookDockAutoFetchEnabled.mockResolvedValue(true);
|
||||
@@ -603,7 +713,7 @@ describe('BookDockIngestService', () => {
|
||||
await (service as any).metadataQueue.waitForIdle();
|
||||
|
||||
expect(metadataService.extractAndSave).toHaveBeenCalledWith(12, '/bucket/12.epub', 'epub', '/books/book-dock/covers');
|
||||
expect(autoFetchSpy).toHaveBeenCalledWith(12);
|
||||
expect(autoFetchSpy).toHaveBeenCalledWith(12, false);
|
||||
expect(repo.update).toHaveBeenCalledWith(12, { status: 'extracting' });
|
||||
expect(emitSummarySpy).toHaveBeenCalledTimes(2);
|
||||
expect(events.emit).toHaveBeenCalledWith('book-dock.file.ingested', 12);
|
||||
|
||||
@@ -31,6 +31,7 @@ export class BookDockIngestService implements OnApplicationBootstrap, OnModuleDe
|
||||
private readonly logger = new Logger(BookDockIngestService.name);
|
||||
private bookDockPath: string;
|
||||
private readonly metadataQueue: BookDockWorkQueue;
|
||||
private readonly forcedAutoFetchFileIds = new Set<number>();
|
||||
|
||||
constructor(
|
||||
private readonly config: ConfigService,
|
||||
@@ -111,6 +112,28 @@ export class BookDockIngestService implements OnApplicationBootstrap, OnModuleDe
|
||||
this.extractMetadataAsync(fileId, row.format, metadataQueuePriority(row));
|
||||
}
|
||||
|
||||
async refetchMetadata(fileId: number): Promise<boolean> {
|
||||
const row = await this.repo.findById(fileId);
|
||||
if (!row || (row.status !== 'ready' && row.status !== 'error')) return false;
|
||||
|
||||
const format = resolveSupportedFormat(row);
|
||||
if (!format) return false;
|
||||
|
||||
this.forcedAutoFetchFileIds.add(fileId);
|
||||
const updated = await this.repo.update(fileId, { status: 'pending', errorMessage: null });
|
||||
if (!updated) {
|
||||
this.forcedAutoFetchFileIds.delete(fileId);
|
||||
return false;
|
||||
}
|
||||
|
||||
const queued = this.extractMetadataAsync(fileId, format, metadataQueuePriority(row));
|
||||
if (!queued) {
|
||||
this.forcedAutoFetchFileIds.delete(fileId);
|
||||
await this.repo.update(fileId, { status: row.status, errorMessage: row.errorMessage });
|
||||
}
|
||||
return queued;
|
||||
}
|
||||
|
||||
async ingestFromWatchedFolder(absolutePath: string): Promise<number | null> {
|
||||
const existing = await this.repo.findByAbsolutePath(absolutePath);
|
||||
if (existing) {
|
||||
@@ -142,10 +165,10 @@ export class BookDockIngestService implements OnApplicationBootstrap, OnModuleDe
|
||||
return row.id;
|
||||
}
|
||||
|
||||
private extractMetadataAsync(fileId: number, format: string, priority?: BookDockWorkPriority): void {
|
||||
if (!isSupportedFormat(format)) return;
|
||||
private extractMetadataAsync(fileId: number, format: string, priority?: BookDockWorkPriority): boolean {
|
||||
if (!isSupportedFormat(format)) return false;
|
||||
if (this.processingState.getCachedPaused()) this.metadataQueue.pause();
|
||||
this.metadataQueue.enqueue(fileId, priority);
|
||||
return this.metadataQueue.enqueue(fileId, priority);
|
||||
}
|
||||
|
||||
pauseProcessing(): void {
|
||||
@@ -195,19 +218,24 @@ export class BookDockIngestService implements OnApplicationBootstrap, OnModuleDe
|
||||
return;
|
||||
}
|
||||
|
||||
const row = await this.repo.findById(fileId);
|
||||
if (!row || !PROCESSABLE_METADATA_STATUSES.has(row.status)) return;
|
||||
const forceAutoFetch = this.forcedAutoFetchFileIds.has(fileId);
|
||||
try {
|
||||
const row = await this.repo.findById(fileId);
|
||||
if (!row || !PROCESSABLE_METADATA_STATUSES.has(row.status)) return;
|
||||
|
||||
const format = resolveSupportedFormat(row);
|
||||
if (!format) return;
|
||||
const format = resolveSupportedFormat(row);
|
||||
if (!format) return;
|
||||
|
||||
const coversDir = join(this.bookDockPath, 'covers');
|
||||
await this.repo.update(fileId, { status: 'extracting' });
|
||||
this.emitChange();
|
||||
await this.metadataService.extractAndSave(fileId, row.absolutePath, format, coversDir);
|
||||
await this.autoFetchMetadataAsync(fileId);
|
||||
this.emitChange();
|
||||
this.events.emit(BOOK_DOCK_FILE_INGESTED, fileId);
|
||||
const coversDir = join(this.bookDockPath, 'covers');
|
||||
await this.repo.update(fileId, { status: 'extracting' });
|
||||
this.emitChange();
|
||||
await this.metadataService.extractAndSave(fileId, row.absolutePath, format, coversDir);
|
||||
await this.autoFetchMetadataAsync(fileId, forceAutoFetch);
|
||||
this.emitChange();
|
||||
this.events.emit(BOOK_DOCK_FILE_INGESTED, fileId);
|
||||
} finally {
|
||||
this.forcedAutoFetchFileIds.delete(fileId);
|
||||
}
|
||||
}
|
||||
|
||||
private logMetadataQueueFailure(fileId: number, err: unknown): void {
|
||||
@@ -218,9 +246,11 @@ export class BookDockIngestService implements OnApplicationBootstrap, OnModuleDe
|
||||
);
|
||||
}
|
||||
|
||||
private async autoFetchMetadataAsync(fileId: number): Promise<void> {
|
||||
const enabled = await this.appSettings.isBookDockAutoFetchEnabled();
|
||||
if (!enabled) return;
|
||||
private async autoFetchMetadataAsync(fileId: number, force = false): Promise<void> {
|
||||
if (!force) {
|
||||
const enabled = await this.appSettings.isBookDockAutoFetchEnabled();
|
||||
if (!enabled) return;
|
||||
}
|
||||
|
||||
const row = await this.repo.findById(fileId);
|
||||
if (!row || row.status === 'error') return;
|
||||
@@ -254,7 +284,14 @@ export class BookDockIngestService implements OnApplicationBootstrap, OnModuleDe
|
||||
confidence: computeConfidence(row.embeddedMetadata ?? {}, fetched),
|
||||
fetchedMetadataSources,
|
||||
}
|
||||
: { status: 'ready' as const };
|
||||
: force
|
||||
? {
|
||||
status: 'ready' as const,
|
||||
fetchedMetadata: null,
|
||||
confidence: null,
|
||||
fetchedMetadataSources: null,
|
||||
}
|
||||
: { status: 'ready' as const };
|
||||
await this.repo.update(fileId, updates);
|
||||
} catch (err) {
|
||||
this.logger.warn(`Auto-fetch metadata failed for Book Dock file ${fileId}: ${err instanceof Error ? err.message : String(err)}`);
|
||||
|
||||
@@ -33,6 +33,7 @@ function makeController() {
|
||||
bulkDiscard: vi.fn(),
|
||||
bulkApplyFetched: vi.fn(),
|
||||
bulkRetryFetch: vi.fn(),
|
||||
refetchMetadata: vi.fn(),
|
||||
bulkSetTarget: vi.fn(),
|
||||
selectionSummary: vi.fn(),
|
||||
bulkEdit: vi.fn(),
|
||||
@@ -145,6 +146,7 @@ describe('BookDockController', () => {
|
||||
await controller.bulkDiscard(MOCK_USER, { fileIds: [1], selectAll: false, excludedIds: [2], status: 'error', search: 'x' });
|
||||
await controller.applyFetched(MOCK_USER, { fileIds: [1], selectAll: true, excludedIds: [2], status: 'ready', search: 'x' });
|
||||
await controller.retryFetch(MOCK_USER, { fileIds: [3], selectAll: false, excludedIds: [4], status: 'error', search: 'y' });
|
||||
await controller.refetchMetadata(MOCK_USER, 11);
|
||||
await controller.setTarget(MOCK_USER, {
|
||||
fileIds: [5],
|
||||
selectAll: false,
|
||||
@@ -187,6 +189,7 @@ describe('BookDockController', () => {
|
||||
await controller.resume();
|
||||
|
||||
expect(service.bulkSetTarget).toHaveBeenCalledWith([5], false, [6], null, null, undefined, undefined, MOCK_USER.id, false, undefined);
|
||||
expect(service.refetchMetadata).toHaveBeenCalledWith(11, MOCK_USER.id, false);
|
||||
expect(finalizeService.previewNames).toHaveBeenCalledWith([10], false, [], 2, MOCK_USER.id, false, undefined, undefined, undefined);
|
||||
expect(finalizeService.previewFinalize).toHaveBeenCalledWith(1, false, false, [10], false, [], 2, 3, [], undefined, undefined, undefined);
|
||||
expect(finalizeService.discardDuplicateCandidates).toHaveBeenCalledWith(
|
||||
|
||||
@@ -203,6 +203,12 @@ export class BookDockController {
|
||||
);
|
||||
}
|
||||
|
||||
@Post('files/:id/refetch-metadata')
|
||||
@HttpCode(HttpStatus.ACCEPTED)
|
||||
refetchMetadata(@CurrentUser() user: RequestUser, @Param('id', ParseIntPipe) id: number) {
|
||||
return this.service.refetchMetadata(id, user.id, this.canManageAll(user));
|
||||
}
|
||||
|
||||
@Post('files/set-target')
|
||||
setTarget(@CurrentUser() user: RequestUser, @Body() dto: BulkSetTargetDto) {
|
||||
return this.service.bulkSetTarget(
|
||||
|
||||
@@ -49,6 +49,7 @@ function makeService() {
|
||||
};
|
||||
const ingestService = {
|
||||
retryFetch: vi.fn(),
|
||||
refetchMetadata: vi.fn(),
|
||||
pauseProcessing: vi.fn(),
|
||||
resumeProcessing: vi.fn().mockResolvedValue(undefined),
|
||||
requeueProcessableFiles: vi.fn().mockResolvedValue(0),
|
||||
@@ -260,6 +261,27 @@ describe('BookDockService', () => {
|
||||
expect(ingestService.retryFetch).toHaveBeenCalledWith(3);
|
||||
});
|
||||
|
||||
it('refetchMetadata enforces ownership and queues an eligible file', async () => {
|
||||
const { service, repo, ingestService } = makeService();
|
||||
repo.findById.mockResolvedValue(row({ id: 7, uploadedBy: 4 }));
|
||||
ingestService.refetchMetadata.mockResolvedValue(true);
|
||||
|
||||
await expect(service.refetchMetadata(7, 4, false)).resolves.toBeUndefined();
|
||||
|
||||
expect(ingestService.refetchMetadata).toHaveBeenCalledWith(7);
|
||||
|
||||
await expect(service.refetchMetadata(7, 5, false)).rejects.toBeInstanceOf(ForbiddenException);
|
||||
expect(ingestService.refetchMetadata).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('refetchMetadata rejects a file that cannot be queued', async () => {
|
||||
const { service, repo, ingestService } = makeService();
|
||||
repo.findById.mockResolvedValue(row({ id: 7, uploadedBy: 4 }));
|
||||
ingestService.refetchMetadata.mockResolvedValue(false);
|
||||
|
||||
await expect(service.refetchMetadata(7, 4, false)).rejects.toBeInstanceOf(BadRequestException);
|
||||
});
|
||||
|
||||
it('bulkSetTarget enforces complete destination tuple and returns update counts', async () => {
|
||||
const { service, repo } = makeService();
|
||||
await expect(service.bulkSetTarget([1], false, [], 10, undefined)).rejects.toBeInstanceOf(BadRequestException);
|
||||
|
||||
@@ -249,6 +249,12 @@ export class BookDockService {
|
||||
return { total, queued };
|
||||
}
|
||||
|
||||
async refetchMetadata(id: number, userId: number, canManageAll: boolean): Promise<void> {
|
||||
await this.findFileForUser(id, userId, canManageAll);
|
||||
const queued = await this.ingestService.refetchMetadata(id);
|
||||
if (!queued) throw new BadRequestException('Metadata can only be re-fetched for a ready or failed Book Dock file');
|
||||
}
|
||||
|
||||
async bulkSetTarget(
|
||||
fileIds: number[],
|
||||
selectAll?: boolean,
|
||||
|
||||
Reference in New Issue
Block a user