mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 08:21:39 -06:00
fix(scanner): populate per-file audio duration on upload
The upload paths saved only the aggregate book_metadata duration, leaving book_files.durationSeconds null so the player summed to zero. Extract and aggregate per-file duration on new uploads and added audio files, matching the scanner. Ref #330
This commit is contained in:
@@ -950,6 +950,27 @@ describe('MetadataService', () => {
|
||||
expect(updateSet).toHaveBeenCalledWith({ durationSeconds: 4321 });
|
||||
});
|
||||
|
||||
it('extractAndAggregateAudioDuration writes the per-file duration before re-aggregating the total', async () => {
|
||||
const { db } = makeDb();
|
||||
const service = makeService(db);
|
||||
|
||||
const callOrder: string[] = [];
|
||||
const extractSpy = vi.spyOn(service, 'extractAudioFileDuration').mockImplementation(() => {
|
||||
callOrder.push('extract');
|
||||
return Promise.resolve();
|
||||
});
|
||||
const aggregateSpy = vi.spyOn(service, 'aggregateAudioDuration').mockImplementation(() => {
|
||||
callOrder.push('aggregate');
|
||||
return Promise.resolve();
|
||||
});
|
||||
|
||||
await service.extractAndAggregateAudioDuration(55, '/tmp/book.m4b');
|
||||
|
||||
expect(extractSpy).toHaveBeenCalledWith(55, '/tmp/book.m4b');
|
||||
expect(aggregateSpy).toHaveBeenCalledWith(55);
|
||||
expect(callOrder).toEqual(['extract', 'aggregate']);
|
||||
});
|
||||
|
||||
it('replaceNarrators and upsertComicMetadata delegate to collaborators', async () => {
|
||||
const { db } = makeDb();
|
||||
const narratorService = {
|
||||
|
||||
@@ -296,6 +296,11 @@ export class MetadataService {
|
||||
}
|
||||
}
|
||||
|
||||
async extractAndAggregateAudioDuration(bookId: number, absolutePath: string): Promise<void> {
|
||||
await this.extractAudioFileDuration(bookId, absolutePath);
|
||||
await this.aggregateAudioDuration(bookId);
|
||||
}
|
||||
|
||||
// ── Authors ──────────────────────────────────────────────────────────────────
|
||||
|
||||
async replaceAuthors(
|
||||
|
||||
@@ -13,6 +13,7 @@ const mockComputeFileHash = computeFileHash as MockedFunction<typeof computeFile
|
||||
describe('UploadProcessorService', () => {
|
||||
const metadataService = {
|
||||
extractAndSave: vi.fn(),
|
||||
extractAndAggregateAudioDuration: vi.fn(),
|
||||
};
|
||||
const orchestrator = {
|
||||
scheduleIfEligible: vi.fn(),
|
||||
@@ -233,6 +234,61 @@ describe('UploadProcessorService', () => {
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('upstream failed'));
|
||||
});
|
||||
|
||||
it('extractMetadataAsync extracts per-file duration after saving metadata for audio formats', async () => {
|
||||
metadataService.extractAndSave.mockResolvedValue(undefined);
|
||||
metadataService.extractAndAggregateAudioDuration.mockResolvedValue(undefined);
|
||||
|
||||
service.extractMetadataAsync(7, '/tmp/book.m4b', 'm4b');
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
expect(metadataService.extractAndSave).toHaveBeenCalledWith(7, '/tmp/book.m4b', 'm4b');
|
||||
expect(metadataService.extractAndAggregateAudioDuration).toHaveBeenCalledWith(7, '/tmp/book.m4b');
|
||||
});
|
||||
|
||||
it('extractMetadataAsync skips duration extraction for non-audio formats', async () => {
|
||||
metadataService.extractAndSave.mockResolvedValue(undefined);
|
||||
|
||||
service.extractMetadataAsync(7, '/tmp/book.epub', 'epub');
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
expect(metadataService.extractAndSave).toHaveBeenCalledWith(7, '/tmp/book.epub', 'epub');
|
||||
expect(metadataService.extractAndAggregateAudioDuration).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('extractMetadataAsync does not extract duration when metadata save fails for an audio file', async () => {
|
||||
vi.spyOn((service as unknown as { logger: { warn: (m: string) => void } }).logger, 'warn').mockImplementation();
|
||||
metadataService.extractAndSave.mockRejectedValue(new Error('probe failed'));
|
||||
|
||||
service.extractMetadataAsync(7, '/tmp/book.m4b', 'm4b');
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
expect(metadataService.extractAndAggregateAudioDuration).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('extractAudioDurationAsync ignores non-audio formats', () => {
|
||||
service.extractAudioDurationAsync(7, '/tmp/book.epub', 'epub');
|
||||
expect(metadataService.extractAndAggregateAudioDuration).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('extractAudioDurationAsync extracts and aggregates duration for audio formats', async () => {
|
||||
metadataService.extractAndAggregateAudioDuration.mockResolvedValue(undefined);
|
||||
|
||||
service.extractAudioDurationAsync(7, '/tmp/chapter-01.mp3', 'mp3');
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
expect(metadataService.extractAndAggregateAudioDuration).toHaveBeenCalledWith(7, '/tmp/chapter-01.mp3');
|
||||
});
|
||||
|
||||
it('extractAudioDurationAsync logs and suppresses extraction errors', async () => {
|
||||
const warn = vi.spyOn((service as unknown as { logger: { warn: (m: string) => void } }).logger, 'warn').mockImplementation();
|
||||
metadataService.extractAndAggregateAudioDuration.mockRejectedValue(new Error('aggregate failed'));
|
||||
|
||||
service.extractAudioDurationAsync(7, '/tmp/chapter-01.mp3', 'mp3');
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('aggregate failed'));
|
||||
});
|
||||
|
||||
describe('with undefined orchestrator', () => {
|
||||
let serviceNoOrch: UploadProcessorService;
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import { Inject, Injectable, InternalServerErrorException, Logger, Optional } fr
|
||||
import { stat } from 'fs/promises';
|
||||
import { and, eq } from 'drizzle-orm';
|
||||
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
|
||||
import { isAudioFormat } from '@bookorbit/types';
|
||||
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
|
||||
|
||||
import { DB } from '../../db';
|
||||
@@ -138,16 +139,51 @@ export class UploadProcessorService {
|
||||
const event = 'upload.extract_metadata';
|
||||
const startedAt = Date.now();
|
||||
this.logger.debug(`[${event}] [start] bookId=${bookId} format=${format} - metadata extraction started`);
|
||||
void this.runMetadataExtraction(bookId, absolutePath, format, event, startedAt);
|
||||
}
|
||||
|
||||
private async runMetadataExtraction(bookId: number, absolutePath: string, format: string, event: string, startedAt: number): Promise<void> {
|
||||
try {
|
||||
await this.metadataService.extractAndSave(bookId, absolutePath, format);
|
||||
// The embedded extractor writes the aggregate duration to book_metadata, but the
|
||||
// per-file book_files.durationSeconds the player sums is only populated here.
|
||||
if (isAudioFormat(format)) {
|
||||
await this.metadataService.extractAndAggregateAudioDuration(bookId, absolutePath);
|
||||
}
|
||||
this.logger.debug(`[${event}] [end] bookId=${bookId} format=${format} durationMs=${Date.now() - startedAt} - metadata extraction completed`);
|
||||
} catch (err) {
|
||||
const error = err as Error;
|
||||
const errorClass = error.name ?? 'Error';
|
||||
const errorMessage = sanitizeLogValue(error.message);
|
||||
this.logger.warn(
|
||||
`[${event}] [fail] bookId=${bookId} format=${format} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - metadata extraction failed`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Fires-and-forgets per-file audio duration extraction for a file added to an existing book.
|
||||
* Unlike {@link extractMetadataAsync} this never overwrites shared book metadata (title, cover, etc.)
|
||||
* Non-audio formats are ignored. Errors are logged but never surfaced to the caller.
|
||||
*/
|
||||
extractAudioDurationAsync(bookId: number, absolutePath: string, format: string): void {
|
||||
if (!isAudioFormat(format)) return;
|
||||
|
||||
const event = 'book.extract_audio_duration';
|
||||
const startedAt = Date.now();
|
||||
this.logger.debug(`[${event}] [start] bookId=${bookId} format=${format} - audio duration extraction started`);
|
||||
this.metadataService
|
||||
.extractAndSave(bookId, absolutePath, format)
|
||||
.extractAndAggregateAudioDuration(bookId, absolutePath)
|
||||
.then(() => {
|
||||
this.logger.debug(`[${event}] [end] bookId=${bookId} format=${format} durationMs=${Date.now() - startedAt} - metadata extraction completed`);
|
||||
this.logger.debug(
|
||||
`[${event}] [end] bookId=${bookId} format=${format} durationMs=${Date.now() - startedAt} - audio duration extraction completed`,
|
||||
);
|
||||
})
|
||||
.catch((err: Error) => {
|
||||
const errorClass = err.name ?? 'Error';
|
||||
const errorMessage = sanitizeLogValue(err.message);
|
||||
this.logger.warn(
|
||||
`[${event}] [fail] bookId=${bookId} format=${format} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - metadata extraction failed`,
|
||||
`[${event}] [fail] bookId=${bookId} format=${format} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - audio duration extraction failed`,
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -70,6 +70,7 @@ describe('UploadService', () => {
|
||||
const processor = {
|
||||
createBookRecord: vi.fn(),
|
||||
extractMetadataAsync: vi.fn(),
|
||||
extractAudioDurationAsync: vi.fn(),
|
||||
};
|
||||
|
||||
const user = { id: 7, isSuperuser: false, permissions: [] } as any;
|
||||
@@ -739,6 +740,24 @@ describe('UploadService', () => {
|
||||
expect(result.role).toBe('content');
|
||||
});
|
||||
|
||||
it('kicks off per-file audio duration extraction for an added audio file', async () => {
|
||||
validator.sanitizeFilename.mockReturnValue('chapter-02.mp3');
|
||||
validator.validateFormat.mockReturnValue('mp3');
|
||||
db.select.mockReturnValueOnce(selectJoinChain([makeBookRow()])).mockReturnValueOnce(noHashConflict());
|
||||
|
||||
await service.addFileToBook(10, 'chapter-02.mp3', {} as any, user);
|
||||
|
||||
expect(processor.extractAudioDurationAsync).toHaveBeenCalledWith(10, '/library/Book Title/chapter-02.mp3', 'mp3');
|
||||
});
|
||||
|
||||
it('delegates duration extraction to the processor regardless of format (processor gates on audio)', async () => {
|
||||
db.select.mockReturnValueOnce(selectJoinChain([makeBookRow()])).mockReturnValueOnce(noHashConflict());
|
||||
|
||||
await service.addFileToBook(10, 'book.epub', {} as any, user);
|
||||
|
||||
expect(processor.extractAudioDurationAsync).toHaveBeenCalledWith(10, '/library/Book Title/book.epub', 'epub');
|
||||
});
|
||||
|
||||
it('promotes new file to primary when book has no primary file', async () => {
|
||||
db.select.mockReturnValueOnce(selectJoinChain([makeBookRow({ primaryFileId: null })])).mockReturnValueOnce(noHashConflict());
|
||||
|
||||
|
||||
@@ -221,6 +221,8 @@ export class UploadService {
|
||||
if (needsStatusUpdate) finalStatus = 'present';
|
||||
}
|
||||
|
||||
this.processor.extractAudioDurationAsync(bookId, destination, format);
|
||||
|
||||
this.logger.log(
|
||||
`[${event}] [end] bookId=${bookId} userId=${user.id} fileId=${inserted.id} format=${format} sizeBytes=${sizeBytes} durationMs=${Date.now() - startedAt} - add file to book completed`,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user