diff --git a/server/src/modules/authors/authors.module.ts b/server/src/modules/authors/authors.module.ts index a8439896a..6cc0f1e00 100644 --- a/server/src/modules/authors/authors.module.ts +++ b/server/src/modules/authors/authors.module.ts @@ -8,6 +8,7 @@ import { AuthModule } from '../auth/auth.module'; import { BookModule } from '../book/book.module'; import { LibraryModule } from '../library/library.module'; import { MetadataModule } from '../metadata/metadata.module'; +import { MetadataScoreModule } from '../metadata-score/metadata-score.module'; import { NotificationModule } from '../notification/notification.module'; import { AuthorImageStorageService } from './author-image-storage.service'; import { AuthorEnrichmentConfigService } from './author-enrichment-config.service'; @@ -33,6 +34,7 @@ const AUTHOR_PROVIDER_CLASSES = [AudnexusAuthorMetadataProvider]; LibraryModule, AppSettingsModule, MetadataModule, + MetadataScoreModule, forwardRef(() => NotificationModule), AuthModule, JwtModule.registerAsync({ diff --git a/server/src/modules/authors/authors.repository.test.ts b/server/src/modules/authors/authors.repository.test.ts index 2d46c632e..9b6dbb614 100644 --- a/server/src/modules/authors/authors.repository.test.ts +++ b/server/src/modules/authors/authors.repository.test.ts @@ -133,6 +133,7 @@ describe('AuthorsRepository', () => { await expect(repo.findVisibleAuthorIds([], [1])).resolves.toEqual([]); await expect(repo.findVisibleAuthorIds([1], [])).resolves.toEqual([]); await expect(repo.countDistinctBooks([])).resolves.toBe(0); + await expect(repo.findBookIdsByAuthorIds([])).resolves.toEqual([]); await expect(repo.findRelatedLibraryIds([])).resolves.toEqual([]); await expect(repo.mergeAuthors(1, [])).resolves.toBeUndefined(); await expect(repo.deleteAuthors([])).resolves.toBeUndefined(); @@ -144,10 +145,14 @@ describe('AuthorsRepository', () => { it('maps distinct ids for visibility and related-library lookup queries', async () => { const { db, selectDistinctBuilder } = makeDb(); - selectDistinctBuilder.where.mockResolvedValueOnce([{ id: 5 }, { id: 8 }]).mockResolvedValueOnce([{ libraryId: 11 }, { libraryId: 12 }]); + selectDistinctBuilder.where + .mockResolvedValueOnce([{ id: 5 }, { id: 8 }]) + .mockResolvedValueOnce([{ bookId: 21 }, { bookId: 34 }]) + .mockResolvedValueOnce([{ libraryId: 11 }, { libraryId: 12 }]); const repo = new AuthorsRepository(db as never); await expect(repo.findVisibleAuthorIds([5, 8], [100])).resolves.toEqual([5, 8]); + await expect(repo.findBookIdsByAuthorIds([1, 2])).resolves.toEqual([21, 34]); await expect(repo.findRelatedLibraryIds([1, 2])).resolves.toEqual([11, 12]); }); diff --git a/server/src/modules/authors/authors.repository.ts b/server/src/modules/authors/authors.repository.ts index a83834dc5..18dee4405 100644 --- a/server/src/modules/authors/authors.repository.ts +++ b/server/src/modules/authors/authors.repository.ts @@ -249,6 +249,12 @@ export class AuthorsRepository { return Number(total); } + async findBookIdsByAuthorIds(authorIds: number[]): Promise { + if (authorIds.length === 0) return []; + const rows = await this.db.selectDistinct({ bookId: bookAuthors.bookId }).from(bookAuthors).where(inArray(bookAuthors.authorId, authorIds)); + return rows.map((row) => row.bookId); + } + async findRelatedLibraryIds(authorIds: number[]): Promise { if (authorIds.length === 0) return []; const rows = await this.db diff --git a/server/src/modules/authors/authors.service.test.ts b/server/src/modules/authors/authors.service.test.ts index e81fcd6de..90bb3fa61 100644 --- a/server/src/modules/authors/authors.service.test.ts +++ b/server/src/modules/authors/authors.service.test.ts @@ -22,6 +22,7 @@ describe('AuthorsService', () => { updateAuthorById: vi.fn(), findVisibleAuthorIds: vi.fn(), countDistinctBooks: vi.fn(), + findBookIdsByAuthorIds: vi.fn(), mergeAuthors: vi.fn(), deleteAuthors: vi.fn(), findRelatedLibraryIds: vi.fn(), @@ -67,6 +68,9 @@ describe('AuthorsService', () => { const appSettings = { getAuthorsAutoEnrichmentWriteMode: vi.fn(), }; + const metadataScoreService = { + calculateAndSaveMany: vi.fn(), + }; let service: AuthorsService; @@ -81,6 +85,7 @@ describe('AuthorsService', () => { authorImageStorage as any, enrichmentExecutor as any, enrichmentOrchestrator as any, + metadataScoreService as any, ); libraryService.findAll.mockResolvedValue([{ id: 1 }, { id: 2 }]); authorImageStorage.getThumbnailUrlIfExists.mockResolvedValue(null); @@ -88,6 +93,7 @@ describe('AuthorsService', () => { enrichmentOrchestrator.schedule.mockResolvedValue(1); enrichmentOrchestrator.backfillLinkedAuthors.mockResolvedValue(8); appSettings.getAuthorsAutoEnrichmentWriteMode.mockResolvedValue('missing_only'); + metadataScoreService.calculateAndSaveMany.mockResolvedValue(undefined); vi.mocked(assembleBookCards).mockReturnValue([]); }); @@ -114,12 +120,13 @@ describe('AuthorsService', () => { it('delete removes authors and returns impacted count', async () => { authorsRepo.findVisibleAuthorIds.mockResolvedValue([10, 11]); authorsRepo.findRelatedLibraryIds.mockResolvedValue([1, 2]); - authorsRepo.countDistinctBooks.mockResolvedValue(6); + authorsRepo.findBookIdsByAuthorIds.mockResolvedValue([1, 2, 3, 4, 5, 6]); authorsRepo.deleteAuthors.mockResolvedValue(undefined); const result = await service.delete(reqUser(7, true), { authorIds: [10, 11, 11] }); expect(authorsRepo.deleteAuthors).toHaveBeenCalledWith([10, 11]); + expect(metadataScoreService.calculateAndSaveMany).toHaveBeenCalledWith([1, 2, 3, 4, 5, 6]); expect(result.deletedAuthorIds).toEqual([10, 11]); expect(result.affectedBookCount).toBe(6); }); @@ -127,7 +134,7 @@ describe('AuthorsService', () => { it('merge deduplicates sources, merges, and returns impacted count', async () => { authorsRepo.findVisibleAuthorIds.mockResolvedValue([10, 11, 12]); authorsRepo.findRelatedLibraryIds.mockResolvedValue([1, 2]); - authorsRepo.countDistinctBooks.mockResolvedValue(8); + authorsRepo.findBookIdsByAuthorIds.mockResolvedValue([1, 2, 3, 4, 5, 6, 7, 8]); authorsRepo.mergeAuthors.mockResolvedValue(undefined); authorsRepo.findById.mockResolvedValue({ id: 10, @@ -141,6 +148,7 @@ describe('AuthorsService', () => { const result = await service.merge(reqUser(7, true), { targetAuthorId: 10, sourceAuthorIds: [10, 11, 11, 12] }); expect(authorsRepo.mergeAuthors).toHaveBeenCalledWith(10, [11, 12]); + expect(metadataScoreService.calculateAndSaveMany).toHaveBeenCalledWith([1, 2, 3, 4, 5, 6, 7, 8]); expect(enrichmentOrchestrator.schedule).toHaveBeenCalledWith(10, AUTHOR_ENRICHMENT_REASONS.AUTHOR_MERGE_TARGET); expect(result.mergedAuthorIds).toEqual([11, 12]); expect(result.affectedBookCount).toBe(8); diff --git a/server/src/modules/authors/authors.service.ts b/server/src/modules/authors/authors.service.ts index 12ad3cd6f..a950f0c42 100644 --- a/server/src/modules/authors/authors.service.ts +++ b/server/src/modules/authors/authors.service.ts @@ -20,6 +20,7 @@ import { books } from '../../db/schema'; import { BookReadService } from '../book/book-read.service'; import { LibraryService } from '../library/library.service'; import { AppSettingsService } from '../app-settings/app-settings.service'; +import { MetadataScoreService } from '../metadata-score/metadata-score.service'; import { AuthorImageStorageError, AuthorImageStorageService } from './author-image-storage.service'; import { AUTHOR_ENRICHMENT_REASONS } from './author-enrichment-reasons'; import { AuthorEnrichmentExecutorService } from './author-enrichment-executor.service'; @@ -50,6 +51,7 @@ export class AuthorsService { private readonly authorImageStorage: AuthorImageStorageService, private readonly enrichmentExecutor: AuthorEnrichmentExecutorService, private readonly enrichmentOrchestrator: AuthorEnrichmentOrchestratorService, + private readonly metadataScoreService: MetadataScoreService, ) {} private assertPaginationWindow(page: number, size: number): void { @@ -268,18 +270,19 @@ export class AuthorsService { const allAuthorIds = [dto.targetAuthorId, ...uniqueSourceIds]; await this.assertMutationAccess(user, allAuthorIds); - const affectedBookCount = await this.authorsRepo.countDistinctBooks(uniqueSourceIds); + const affectedBookIds = await this.authorsRepo.findBookIdsByAuthorIds(uniqueSourceIds); await this.authorsRepo.mergeAuthors(dto.targetAuthorId, uniqueSourceIds); + await this.metadataScoreService.calculateAndSaveMany(affectedBookIds); await this.enrichmentOrchestrator.schedule(dto.targetAuthorId, AUTHOR_ENRICHMENT_REASONS.AUTHOR_MERGE_TARGET); const target = await this.findOne(user, dto.targetAuthorId); this.logger.log( - `[${event}] [end] userId=${user.id} targetAuthorId=${dto.targetAuthorId} durationMs=${Date.now() - startedAt} mergedCount=${uniqueSourceIds.length} affectedBookCount=${affectedBookCount} - author merge completed`, + `[${event}] [end] userId=${user.id} targetAuthorId=${dto.targetAuthorId} durationMs=${Date.now() - startedAt} mergedCount=${uniqueSourceIds.length} affectedBookCount=${affectedBookIds.length} - author merge completed`, ); return { target, mergedAuthorIds: uniqueSourceIds, - affectedBookCount, + affectedBookCount: affectedBookIds.length, }; } catch (err) { const errorClass = err instanceof Error ? err.name : 'Error'; @@ -303,15 +306,16 @@ export class AuthorsService { const authorIds = [...new Set(dto.authorIds)]; await this.assertMutationAccess(user, authorIds); - const affectedBookCount = await this.authorsRepo.countDistinctBooks(authorIds); + const affectedBookIds = await this.authorsRepo.findBookIdsByAuthorIds(authorIds); await this.authorsRepo.deleteAuthors(authorIds); + await this.metadataScoreService.calculateAndSaveMany(affectedBookIds); this.logger.log( - `[${event}] [end] userId=${user.id} durationMs=${Date.now() - startedAt} deletedCount=${authorIds.length} affectedBookCount=${affectedBookCount} - author delete completed`, + `[${event}] [end] userId=${user.id} durationMs=${Date.now() - startedAt} deletedCount=${authorIds.length} affectedBookCount=${affectedBookIds.length} - author delete completed`, ); return { deletedAuthorIds: authorIds, - affectedBookCount, + affectedBookCount: affectedBookIds.length, }; } catch (err) { const errorClass = err instanceof Error ? err.name : 'Error'; diff --git a/server/src/modules/book-dock/book-dock-finalize.service.test.ts b/server/src/modules/book-dock/book-dock-finalize.service.test.ts index 1b39a078c..c2a50f8c6 100644 --- a/server/src/modules/book-dock/book-dock-finalize.service.test.ts +++ b/server/src/modules/book-dock/book-dock-finalize.service.test.ts @@ -48,6 +48,9 @@ function makeService() { replaceNarrators: vi.fn().mockResolvedValue(undefined), upsertComicMetadata: vi.fn().mockResolvedValue(undefined), }; + const metadataScoreService = { + calculateAndSave: vi.fn().mockResolvedValue(undefined), + }; const bookReadService = { replaceCommunityRatings: vi.fn().mockResolvedValue(undefined), }; @@ -82,6 +85,7 @@ function makeService() { libraryService as never, appSettings as never, metadataService as never, + metadataScoreService as never, bookReadService as never, validator as never, storage as never, @@ -101,6 +105,7 @@ function makeService() { libraryService, appSettings, metadataService, + metadataScoreService, bookReadService, validator, storage, @@ -1066,7 +1071,7 @@ describe('BookDockFinalizeService', () => { }); it('applyMetadata updates scalar metadata fields and related author/genre rows', async () => { - const { service, db, metadataService } = makeService(); + const { service, db, metadataService, metadataScoreService } = makeService(); const updateChain = { set: vi.fn(), where: vi.fn().mockResolvedValue(undefined), @@ -1109,6 +1114,20 @@ describe('BookDockFinalizeService', () => { ); expect(metadataService.replaceAuthors).toHaveBeenCalledWith(15, [{ name: 'Author A', sortName: null }]); expect(metadataService.replaceGenres).toHaveBeenCalledWith(15, ['Fantasy']); + expect(metadataScoreService.calculateAndSave).toHaveBeenCalledWith(15); + }); + + it('applyMetadata propagates score persistence failures', async () => { + const { service, db, metadataScoreService } = makeService(); + const updateChain = { + set: vi.fn(), + where: vi.fn().mockResolvedValue(undefined), + }; + updateChain.set.mockReturnValue(updateChain); + db.update.mockReturnValue(updateChain); + metadataScoreService.calculateAndSave.mockRejectedValue(new Error('score failed')); + + await expect((service as any).applyMetadata(15, makeRow())).rejects.toThrow('score failed'); }); it('applyMetadata nulls publishedYear when it is outside database bounds', async () => { diff --git a/server/src/modules/book-dock/book-dock-finalize.service.ts b/server/src/modules/book-dock/book-dock-finalize.service.ts index fefb61ba2..731caf96c 100644 --- a/server/src/modules/book-dock/book-dock-finalize.service.ts +++ b/server/src/modules/book-dock/book-dock-finalize.service.ts @@ -42,6 +42,7 @@ import { bookMetadata, libraries, libraryFolders } from '../../db/schema'; import { AppSettingsService } from '../app-settings/app-settings.service'; import { LibraryService } from '../library/library.service'; import { MetadataService } from '../metadata/metadata.service'; +import { MetadataScoreService } from '../metadata-score/metadata-score.service'; import { UploadProcessorService } from '../upload/upload-processor.service'; import { UploadStorageService } from '../upload/upload-storage.service'; import { UploadValidatorService } from '../upload/upload-validator.service'; @@ -133,6 +134,7 @@ export class BookDockFinalizeService implements OnModuleInit, OnApplicationBoots private readonly libraryService: LibraryService, private readonly appSettings: AppSettingsService, private readonly metadataService: MetadataService, + private readonly metadataScoreService: MetadataScoreService, private readonly bookReadService: BookReadService, private readonly validator: UploadValidatorService, private readonly storage: UploadStorageService, @@ -855,6 +857,8 @@ export class BookDockFinalizeService implements OnModuleInit, OnApplicationBoots audio.narrators.map((name) => ({ name, sortName: null })), ); } + + await this.metadataScoreService.calculateAndSave(bookId); } private async cleanupBookDockRecord(row: BookDockFileRow): Promise { diff --git a/server/src/modules/book-dock/book-dock.module.ts b/server/src/modules/book-dock/book-dock.module.ts index 586153666..a8650782c 100644 --- a/server/src/modules/book-dock/book-dock.module.ts +++ b/server/src/modules/book-dock/book-dock.module.ts @@ -9,6 +9,7 @@ import { BookModule } from '../book/book.module'; import { LibraryModule } from '../library/library.module'; import { MetadataFetchModule } from '../metadata-fetch/metadata-fetch.module'; import { MetadataModule } from '../metadata/metadata.module'; +import { MetadataScoreModule } from '../metadata-score/metadata-score.module'; import { NotificationModule } from '../notification/notification.module'; import { UploadModule } from '../upload/upload.module'; import { BookDockController } from './book-dock.controller'; @@ -30,6 +31,7 @@ import { BookDockRepository } from './book-dock.repository'; LibraryModule, MetadataFetchModule, MetadataModule, + MetadataScoreModule, forwardRef(() => NotificationModule), AppSettingsModule, JwtModule.registerAsync({ diff --git a/server/src/modules/book-metadata-fetch/book-metadata-fetch-orchestrator.service.test.ts b/server/src/modules/book-metadata-fetch/book-metadata-fetch-orchestrator.service.test.ts index da4715fbe..59db0d5fb 100644 --- a/server/src/modules/book-metadata-fetch/book-metadata-fetch-orchestrator.service.test.ts +++ b/server/src/modules/book-metadata-fetch/book-metadata-fetch-orchestrator.service.test.ts @@ -469,7 +469,7 @@ describe('BookMetadataFetchOrchestratorService', () => { ); }); - it('processOne marks done and clears session when metadata pipeline succeeds', async () => { + it('processOne marks the queue item failed when score persistence fails', async () => { const { service, queueRepo, bookReadService, pipeline, scoreService, session } = makeService(); session.addToTotal(1); bookReadService.findById.mockResolvedValue({ @@ -488,9 +488,9 @@ describe('BookMetadataFetchOrchestratorService', () => { await (service as any).processOne(90, 'Book'); - expect(queueRepo.markDone).toHaveBeenCalledWith(90); - expect(queueRepo.markFailed).not.toHaveBeenCalled(); - expect(session.getSnapshot().sessionDone).toBe(1); + expect(queueRepo.markDone).not.toHaveBeenCalled(); + expect(queueRepo.markFailed).toHaveBeenCalledWith(90, 'score delayed', undefined); + expect(session.getSnapshot().sessionDone).toBe(0); expect(session.getSnapshot().currentItemName).toBeNull(); }); diff --git a/server/src/modules/book-metadata-fetch/book-metadata-fetch-orchestrator.service.ts b/server/src/modules/book-metadata-fetch/book-metadata-fetch-orchestrator.service.ts index 0b1d687de..5e6794cf1 100644 --- a/server/src/modules/book-metadata-fetch/book-metadata-fetch-orchestrator.service.ts +++ b/server/src/modules/book-metadata-fetch/book-metadata-fetch-orchestrator.service.ts @@ -257,13 +257,7 @@ export class BookMetadataFetchOrchestratorService implements OnApplicationBootst await this.persistResolved(bookId, resolved, providerIds, authorRows, genreRows, narratorRows); - this.scoreService - .calculateAndSave(bookId) - .catch((err: Error) => - this.logger.warn( - `[book.metadata_fetch.score_recalc] [fail] bookId=${bookId} errorClass=${err.name} error="${sanitizeLogValue(err.message)}" - metadata score recalculation failed`, - ), - ); + await this.scoreService.calculateAndSave(bookId); this.logger.debug(`[book.metadata_fetch] [end] bookId=${bookId} durationMs=${Date.now() - startedAt} - metadata fetch completed`); await this.queueRepo.markDone(bookId); diff --git a/server/src/modules/book/book.service.test.ts b/server/src/modules/book/book.service.test.ts index 586d57f82..94bbdb3e1 100644 --- a/server/src/modules/book/book.service.test.ts +++ b/server/src/modules/book/book.service.test.ts @@ -178,7 +178,11 @@ function makeService(overrides: { bookMetadataLockService?: unknown } = {}) { }; const scoreService = { calculateAndSave: vi.fn().mockResolvedValue(undefined), + calculateAndSaveMany: vi.fn(), }; + scoreService.calculateAndSaveMany.mockImplementation((bookIds: number[]) => + Promise.all(bookIds.map((bookId) => scoreService.calculateAndSave(bookId))).then(() => undefined), + ); const embedder = { embedBook: vi.fn().mockResolvedValue(undefined), }; @@ -3344,18 +3348,15 @@ describe('BookService', () => { expect(scoreService.calculateAndSave).toHaveBeenCalledWith(3); }); - it('bulkSetRating logs score recalculation failures without interrupting the update', async () => { + it('bulkSetRating propagates score recalculation failures', async () => { const { service, bookRepo, scoreService } = makeService(); const user = makeUser({ id: 42 }); - const warnSpy = vi.spyOn((service as unknown as { logger: { warn: (message: string) => void } }).logger, 'warn').mockImplementation(); vi.spyOn(service, 'verifyLibraryAccessForBookIds').mockResolvedValue(undefined); scoreService.calculateAndSave.mockRejectedValue(new Error('score exploded')); - await service.bulkSetRating([3], 4, user); - await Promise.resolve(); + await expect(service.bulkSetRating([3], 4, user)).rejects.toThrow('score exploded'); expect(bookRepo.bulkSetRating).toHaveBeenCalledWith([3], 4, 42); - expect(warnSpy).toHaveBeenCalledWith('Score calculation failed for book 3: score exploded'); }); it('bulkSetMetadata updates a single metadata field and queues follow-up work', async () => { diff --git a/server/src/modules/book/book.service.ts b/server/src/modules/book/book.service.ts index d40d3f205..4e1df51d8 100644 --- a/server/src/modules/book/book.service.ts +++ b/server/src/modules/book/book.service.ts @@ -1772,7 +1772,7 @@ export class BookService { this.fileRenameService?.scheduleRename(id, user.id); } } - this.scoreService.calculateAndSave(id).catch((err: Error) => this.logger.warn(`Score calculation failed for book ${id}: ${err.message}`)); + await this.scoreService.calculateAndSave(id); } const detail = await this.getDetail(id, user); @@ -2206,7 +2206,7 @@ export class BookService { const updatableIds = bookIds.filter((bookId) => !lockedIds.has(bookId)); if (updatableIds.length > 0) { await this.bookRepo.bulkSetRating(updatableIds, rating, user.id); - this.triggerPostMetadataUpdateEffects(updatableIds, user.id); + await this.triggerPostMetadataUpdateEffects(updatableIds, user.id); this.achievementEvents?.emit(ACHIEVEMENT_EVENT_BOOK_RATING_CHANGED, { userId: user.id, bookIds: updatableIds, @@ -2273,7 +2273,7 @@ export class BookService { } }); } - this.triggerPostMetadataUpdateEffects(updatableIds, user.id); + await this.triggerPostMetadataUpdateEffects(updatableIds, user.id); } this.logger.log( `[${event}] [end] userId=${user.id} count=${bookIds.length} field=${field} updated=${updatableIds.length} skippedLocked=${lockedIds.size} durationMs=${Date.now() - startedAt} - bulk set metadata completed`, @@ -2300,7 +2300,7 @@ export class BookService { await this.metadataService.replaceTags(bookId, next, { executor: tx }); } }); - this.triggerPostMetadataUpdateEffects(bookIds, user.id); + await this.triggerPostMetadataUpdateEffects(bookIds, user.id); this.logger.log( `[${event}] [end] userId=${user.id} count=${bookIds.length} mode=${mode} tagCount=${tags.length} durationMs=${Date.now() - startedAt} - bulk update tags completed`, @@ -2495,7 +2495,7 @@ export class BookService { } if (allUpdatedBookIds.size > 0) { - this.triggerPostMetadataUpdateEffects([...allUpdatedBookIds], user.id); + await this.triggerPostMetadataUpdateEffects([...allUpdatedBookIds], user.id); } const result: BulkEditMetadataResult = { @@ -3285,14 +3285,13 @@ export class BookService { ); } - private triggerPostMetadataUpdateEffects(bookIds: number[], userId: number): void { + private async triggerPostMetadataUpdateEffects(bookIds: number[], userId: number): Promise { for (const bookId of bookIds) { this.fileWriteService?.scheduleWrite(bookId, 'auto', userId); this.fileRenameService?.scheduleRename(bookId, userId); - void this.scoreService - .calculateAndSave(bookId) - .catch((err: Error) => this.logger.warn(`Score calculation failed for book ${bookId}: ${err.message}`)); } + + await this.scoreService.calculateAndSaveMany(bookIds); } private resolveChapters( diff --git a/server/src/modules/cover/cover.module.ts b/server/src/modules/cover/cover.module.ts index 34347c041..436970506 100644 --- a/server/src/modules/cover/cover.module.ts +++ b/server/src/modules/cover/cover.module.ts @@ -4,6 +4,7 @@ import { BookMetadataLockModule } from '../book-metadata-lock/book-metadata-lock import { FileWriteModule } from '../file-write/file-write.module'; import { LibraryModule } from '../library/library.module'; import { MetadataPreferencesModule } from '../metadata-preferences/metadata-preferences.module'; +import { MetadataScoreModule } from '../metadata-score/metadata-score.module'; import { COVER_PROVIDERS } from './constants'; import { CoverController } from './cover.controller'; import { CoverService } from './cover.service'; @@ -16,7 +17,7 @@ import { CoverProviderRegistry } from './provider-registry'; const PROVIDER_CLASSES = [DuckDuckGoCoverProvider, ITunesCoverProvider, AudiobookCoversCoverProvider]; @Module({ - imports: [BookModule, BookMetadataLockModule, FileWriteModule, LibraryModule, MetadataPreferencesModule], + imports: [BookModule, BookMetadataLockModule, FileWriteModule, LibraryModule, MetadataPreferencesModule, MetadataScoreModule], controllers: [CoverController], providers: [ ...PROVIDER_CLASSES, diff --git a/server/src/modules/cover/cover.service.test.ts b/server/src/modules/cover/cover.service.test.ts index c93dfd1d7..da7024476 100644 --- a/server/src/modules/cover/cover.service.test.ts +++ b/server/src/modules/cover/cover.service.test.ts @@ -105,6 +105,7 @@ function createService(providerRegistry: CoverProviderRegistry, options?: { asse { verifyUserAccess: vi.fn().mockResolvedValue(undefined) } as never, { get: vi.fn().mockReturnValue('/tmp/books') } as never, providerRegistry, + { calculateAndSave: vi.fn().mockResolvedValue(undefined) } as never, ); } @@ -116,6 +117,9 @@ function createMutationService(options?: { assertFieldsUnlocked?: ReturnType { @@ -474,7 +479,7 @@ describe('CoverService', () => { }); it('saves custom cover file and thumbnail when unlocked', async () => { - const { service, mockDb, updateSet } = createMutationService(); + const { service, mockDb, updateSet, scoreService } = createMutationService(); const source = Buffer.from('image-data'); const normalized = Buffer.from('normalized-image'); @@ -510,6 +515,21 @@ describe('CoverService', () => { ); expect(mockDb.update).toHaveBeenCalledWith(books); expect(updateSet).toHaveBeenCalledWith({ updatedAt: expect.any(Date) }); + expect(scoreService.calculateAndSave).toHaveBeenCalledWith(12); + }); + + it('propagates score persistence failures after updating the cover source', async () => { + const { service, scoreService } = createMutationService(); + scoreService.calculateAndSave.mockRejectedValue(new Error('score failed')); + vi.mocked(coverDirPath).mockReturnValue('/tmp/books/covers/12'); + vi.mocked(imageExt).mockReturnValue('jpg'); + vi.mocked(generateThumbnail).mockResolvedValue(Buffer.from('thumb')); + vi.mocked(readdir).mockResolvedValue([] as never); + vi.mocked(mkdir).mockResolvedValue(undefined as never); + vi.mocked(writeFile).mockResolvedValue(undefined); + vi.mocked(rename).mockResolvedValue(undefined); + + await expect(service.uploadCover(12, Buffer.from('image-data'), 'image/jpeg', makeUser())).rejects.toThrow('score failed'); }); it('preserves the existing cover when image normalization fails', async () => { diff --git a/server/src/modules/cover/cover.service.ts b/server/src/modules/cover/cover.service.ts index 73f6932c8..a604c410e 100644 --- a/server/src/modules/cover/cover.service.ts +++ b/server/src/modules/cover/cover.service.ts @@ -19,6 +19,7 @@ import { BookMetadataLockService } from '../book-metadata-lock/book-metadata-loc import { FileWriteService } from '../file-write/file-write.service'; import { LibraryService } from '../library/library.service'; import { generateThumbnail, imageExt, normalizeProgressiveJpeg } from '../metadata/lib/cover'; +import { MetadataScoreService } from '../metadata-score/metadata-score.service'; import { COVER_CUSTOM_FILE_PREFIX, COVER_PROXY_MAX_IMAGE_BYTES, @@ -54,6 +55,7 @@ export class CoverService { private readonly libraryService: LibraryService, private readonly config: ConfigService, private readonly providerRegistry: CoverProviderRegistry, + private readonly metadataScoreService: MetadataScoreService, ) { this.appDataPath = this.config.get('storage.appDataPath')!; } @@ -299,6 +301,7 @@ export class CoverService { .values({ bookId, coverSource: source, updatedAt: now }) .onConflictDoUpdate({ target: bookMetadata.bookId, set: { coverSource: source, updatedAt: now } }); await this.db.update(books).set({ updatedAt: now }).where(eq(books.id, bookId)); + await this.metadataScoreService.calculateAndSave(bookId); } private async fetchRemoteImage(rawUrl: string): Promise<{ buffer: Buffer; contentType: string }> { diff --git a/server/src/modules/entity-manager/entity-manager.module.ts b/server/src/modules/entity-manager/entity-manager.module.ts index 764700c7b..582b0d4fb 100644 --- a/server/src/modules/entity-manager/entity-manager.module.ts +++ b/server/src/modules/entity-manager/entity-manager.module.ts @@ -3,6 +3,7 @@ import { Module } from '@nestjs/common'; import { AuthorsModule } from '../authors/authors.module'; import { FileWriteModule } from '../file-write/file-write.module'; import { LibraryModule } from '../library/library.module'; +import { MetadataScoreModule } from '../metadata-score/metadata-score.module'; import { EntityManagerController } from './entity-manager.controller'; import { EntityManagerRepository } from './entity-manager.repository'; import { EntityManagerService } from './entity-manager.service'; @@ -16,7 +17,7 @@ import { SeriesStrategy } from './strategies/series.strategy'; import { TagStrategy } from './strategies/tag.strategy'; @Module({ - imports: [AuthorsModule, FileWriteModule, LibraryModule], + imports: [AuthorsModule, FileWriteModule, LibraryModule, MetadataScoreModule], controllers: [EntityManagerController], providers: [ EntityManagerService, diff --git a/server/src/modules/entity-manager/entity-manager.service.test.ts b/server/src/modules/entity-manager/entity-manager.service.test.ts index 46064b609..325d82b1c 100644 --- a/server/src/modules/entity-manager/entity-manager.service.test.ts +++ b/server/src/modules/entity-manager/entity-manager.service.test.ts @@ -43,6 +43,9 @@ function makeService() { const fileWriteService = { scheduleWrite: vi.fn(), }; + const metadataScoreService = { + calculateAndSaveMany: vi.fn().mockResolvedValue(undefined), + }; const duplicateCompute = { readCandidatePairs: vi.fn().mockResolvedValue([]), @@ -63,6 +66,7 @@ function makeService() { repo as any, libraryService as any, fileWriteService as any, + metadataScoreService as any, duplicateCompute as any, authorStrategy as any, genreStrategy as any, @@ -78,6 +82,7 @@ function makeService() { repo, libraryService, fileWriteService, + metadataScoreService, duplicateCompute, strategies: { author: authorStrategy, @@ -600,7 +605,7 @@ describe('EntityManagerService', () => { describe('merge', () => { it('calls strategy merge and cleans up dismissed pairs for first-class', async () => { - const { service, strategies, repo, duplicateCompute } = makeService(); + const { service, strategies, repo, duplicateCompute, metadataScoreService } = makeService(); (strategies.author.merge as any).mockResolvedValue({ affectedBookIds: [10, 20], imagePromoted: true, @@ -615,6 +620,7 @@ describe('EntityManagerService', () => { expect(repo.deleteDismissedPairsForEntity).toHaveBeenCalledWith('author', 3); expect(duplicateCompute.invalidateCandidatesForEntities).toHaveBeenCalledTimes(1); expect(duplicateCompute.invalidateCandidatesForEntities).toHaveBeenCalledWith('author', [2, 3]); + expect(metadataScoreService.calculateAndSaveMany).toHaveBeenCalledWith([10, 20]); expect(result).toEqual({ targetId: 1, mergedIds: [2, 3], @@ -624,6 +630,14 @@ describe('EntityManagerService', () => { }); }); + it('propagates score persistence failures', async () => { + const { service, strategies, metadataScoreService } = makeService(); + (strategies.author.merge as any).mockResolvedValue({ affectedBookIds: [10] }); + metadataScoreService.calculateAndSaveMany.mockRejectedValue(new Error('score failed')); + + await expect(service.merge('author', mockUser, 1, [2], false)).rejects.toThrow('score failed'); + }); + it('invalidates candidate rows once for all merged source IDs', async () => { const { service, strategies, duplicateCompute } = makeService(); (strategies.author.merge as any).mockResolvedValue({ affectedBookIds: [] }); diff --git a/server/src/modules/entity-manager/entity-manager.service.ts b/server/src/modules/entity-manager/entity-manager.service.ts index 3af5c74ed..bfa4e71bd 100644 --- a/server/src/modules/entity-manager/entity-manager.service.ts +++ b/server/src/modules/entity-manager/entity-manager.service.ts @@ -18,6 +18,7 @@ import type { RequestUser } from '../../common/types/request-user'; import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils'; import { FileWriteService } from '../file-write/file-write.service'; import { LibraryService } from '../library/library.service'; +import { MetadataScoreService } from '../metadata-score/metadata-score.service'; import { EntityManagerRepository } from './entity-manager.repository'; import { DuplicateComputeService } from './duplicate-compute.service'; import type { EntityBookScope, EntityStrategy, RawCandidatePair } from './strategies/entity-strategy.interface'; @@ -48,6 +49,7 @@ export class EntityManagerService { private readonly repo: EntityManagerRepository, private readonly libraryService: LibraryService, private readonly fileWriteService: FileWriteService, + private readonly metadataScoreService: MetadataScoreService, private readonly duplicateCompute: DuplicateComputeService, authorStrategy: AuthorStrategy, genreStrategy: GenreStrategy, @@ -260,6 +262,7 @@ export class EntityManagerService { if (writeFiles) { this.scheduleWrites(result.affectedBookIds, user.id); } + await this.metadataScoreService.calculateAndSaveMany(result.affectedBookIds); this.logger.log( `[${event}] [end] userId=${user.id} entityType=${entityType} durationMs=${Date.now() - startedAt} affectedBooks=${result.affectedBookIds.length} - merge completed`, @@ -294,6 +297,7 @@ export class EntityManagerService { if (writeFiles) { this.scheduleWrites(result.affectedBookIds, user.id); } + await this.metadataScoreService.calculateAndSaveMany(result.affectedBookIds); this.logger.log( `[${event}] [end] userId=${user.id} entityType=${entityType} durationMs=${Date.now() - startedAt} implicitMerge=${result.wasImplicitMerge} - rename completed`, @@ -343,6 +347,7 @@ export class EntityManagerService { if (writeFiles) { this.scheduleWrites(result.affectedBookIds, user.id); } + await this.metadataScoreService.calculateAndSaveMany(result.affectedBookIds); this.logger.log( `[${event}] [end] userId=${user.id} entityType=${entityType} durationMs=${Date.now() - startedAt} affectedBooks=${result.affectedBookIds.length} - delete completed`, @@ -403,6 +408,7 @@ export class EntityManagerService { if (writeFiles) { this.scheduleWrites(result.affectedBookIds, user.id); } + await this.metadataScoreService.calculateAndSaveMany(result.affectedBookIds); this.logger.log( `[${event}] [end] userId=${user.id} entityType=${entityType} durationMs=${Date.now() - startedAt} newEntities=${result.newEntities.length} - split completed`, diff --git a/server/src/modules/metadata-score/metadata-score.service.test.ts b/server/src/modules/metadata-score/metadata-score.service.test.ts index 10b8760e7..71d4bd920 100644 --- a/server/src/modules/metadata-score/metadata-score.service.test.ts +++ b/server/src/modules/metadata-score/metadata-score.service.test.ts @@ -38,6 +38,7 @@ function makeWeights(): MetadataScoreWeights { openLibraryId: 1, itunesId: 1, koboId: 1, + aladinId: 1, }; } @@ -130,6 +131,44 @@ describe('MetadataScoreService', () => { expect(repo.updateMetadataScore).not.toHaveBeenCalled(); }); + it('serializes concurrent score calculations for the same book', async () => { + const { service, repo, scorer, appSettings } = makeService(); + const firstLoad = deferred>(); + const firstData = { title: 'Before' }; + const secondData = { title: 'After' }; + repo.loadScoreData.mockReturnValueOnce(firstLoad.promise).mockResolvedValueOnce(secondData); + appSettings.getMetadataScoreWeights.mockResolvedValue(makeWeights()); + scorer.compute.mockReturnValueOnce(20).mockReturnValueOnce(90); + + const firstCalculation = service.calculateAndSave(12); + const secondCalculation = service.calculateAndSave(12); + for (let attempt = 0; attempt < 5 && repo.loadScoreData.mock.calls.length === 0; attempt++) { + await Promise.resolve(); + } + + expect(repo.loadScoreData).toHaveBeenCalledTimes(1); + firstLoad.resolve(firstData); + await Promise.all([firstCalculation, secondCalculation]); + + expect(repo.loadScoreData).toHaveBeenCalledTimes(2); + expect(repo.updateMetadataScore).toHaveBeenNthCalledWith(1, 12, 20); + expect(repo.updateMetadataScore).toHaveBeenNthCalledWith(2, 12, 90); + }); + + it('recalculates unique books in bulk with one weights lookup', async () => { + const { service, repo, scorer, appSettings } = makeService(); + repo.loadScoreData.mockResolvedValue({ title: 'Book' }); + appSettings.getMetadataScoreWeights.mockResolvedValue(makeWeights()); + scorer.compute.mockReturnValue(75); + + await service.calculateAndSaveMany([3, 5, 3, 0]); + + expect(appSettings.getMetadataScoreWeights).toHaveBeenCalledTimes(1); + expect(repo.loadScoreData).toHaveBeenCalledTimes(2); + expect(repo.updateMetadataScore).toHaveBeenCalledWith(3, 75); + expect(repo.updateMetadataScore).toHaveBeenCalledWith(5, 75); + }); + it('starts manual recalculation and reports completed status with success/failure counters', async () => { const { service, repo, scorer, appSettings } = makeService(); const weights = makeWeights(); diff --git a/server/src/modules/metadata-score/metadata-score.service.ts b/server/src/modules/metadata-score/metadata-score.service.ts index 31489e3d0..c525a8595 100644 --- a/server/src/modules/metadata-score/metadata-score.service.ts +++ b/server/src/modules/metadata-score/metadata-score.service.ts @@ -7,6 +7,7 @@ import { MetadataScoreScorer, type ScoreData } from './metadata-score.scorer'; const RECALCULATION_PAGE_SIZE = 200; const RECALCULATION_WRITE_CONCURRENCY = 25; +const ON_DEMAND_WRITE_CONCURRENCY = 25; const RECALCULATION_EVENT = 'metadata_score.recalculate_all'; const RECALCULATION_BOOK_EVENT = 'metadata_score.recalculate_book'; @@ -31,6 +32,7 @@ export type MetadataRecalculationStatus = { @Injectable() export class MetadataScoreService { private readonly logger = new Logger(MetadataScoreService.name); + private readonly pendingBookCalculations = new Map>(); private runningPromise: Promise | null = null; private status: MetadataRecalculationStatus = this.makeInitialStatus(); @@ -66,13 +68,43 @@ export class MetadataScoreService { return { started: true, status: this.getRecalculationStatus() }; } - async calculateAndSave(bookId: number): Promise { - const [data, weights] = await Promise.all([this.repo.loadScoreData(bookId), this.appSettings.getMetadataScoreWeights()]); + calculateAndSave(bookId: number): Promise { + return this.enqueueBookCalculation(bookId, async () => { + const weights = await this.appSettings.getMetadataScoreWeights(); + await this.calculateAndSaveWithWeights(bookId, weights); + }); + } + + async calculateAndSaveMany(bookIds: number[]): Promise { + const uniqueBookIds = [...new Set(bookIds)].filter((bookId) => Number.isInteger(bookId) && bookId > 0); + if (uniqueBookIds.length === 0) return; + + const weights = await this.appSettings.getMetadataScoreWeights(); + for (let index = 0; index < uniqueBookIds.length; index += ON_DEMAND_WRITE_CONCURRENCY) { + const chunk = uniqueBookIds.slice(index, index + ON_DEMAND_WRITE_CONCURRENCY); + await Promise.all(chunk.map((bookId) => this.enqueueBookCalculation(bookId, () => this.calculateAndSaveWithWeights(bookId, weights)))); + } + } + + private async calculateAndSaveWithWeights(bookId: number, weights: MetadataScoreWeights): Promise { + const data = await this.repo.loadScoreData(bookId); if (!data) return; const score = this.scorer.compute(data, weights); await this.repo.updateMetadataScore(bookId, score); } + private enqueueBookCalculation(bookId: number, calculate: () => Promise): Promise { + const previous = this.pendingBookCalculations.get(bookId) ?? Promise.resolve(); + const calculation = previous.catch(() => undefined).then(calculate); + this.pendingBookCalculations.set(bookId, calculation); + + return calculation.finally(() => { + if (this.pendingBookCalculations.get(bookId) === calculation) { + this.pendingBookCalculations.delete(bookId); + } + }); + } + private async runRecalculation(trigger: MetadataRecalculationTrigger): Promise { const startedAt = Date.now(); this.status = { diff --git a/server/src/modules/metadata/metadata.service.test.ts b/server/src/modules/metadata/metadata.service.test.ts index dc6acd876..f17d95568 100644 --- a/server/src/modules/metadata/metadata.service.test.ts +++ b/server/src/modules/metadata/metadata.service.test.ts @@ -452,7 +452,7 @@ describe('MetadataService', () => { await expect(service.extractAndSave(15, '/books/a.pdf', 'pdf')).rejects.toThrow('bad metadata'); }); - it('extractAndSave logs warning when score calculation or embedding fails', async () => { + it('extractAndSave propagates score calculation failures after persisting metadata', async () => { const { db } = makeDb(); const warnSpy = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined); const scoreService = { @@ -492,12 +492,12 @@ describe('MetadataService', () => { coverBuffer: null, }); - await service.extractAndSave(44, '/tmp/warn-book.pdf', 'pdf'); + await expect(service.extractAndSave(44, '/tmp/warn-book.pdf', 'pdf')).rejects.toThrow('score failed'); await Promise.resolve(); expect(scoreService.calculateAndSave).toHaveBeenCalledWith(44); expect(failingEmbedder.embedBook).toHaveBeenCalledWith(44); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('[metadata.score_calculation] [fail]')); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('[metadata.extract_and_save] [fail]')); expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('[metadata.embedding] [fail]')); }); diff --git a/server/src/modules/metadata/metadata.service.ts b/server/src/modules/metadata/metadata.service.ts index f6c3c5267..945f41630 100644 --- a/server/src/modules/metadata/metadata.service.ts +++ b/server/src/modules/metadata/metadata.service.ts @@ -110,11 +110,7 @@ export class MetadataService { await Promise.all([this.persistMetadata(bookId, data, format), data.cover ? this.persistCover(bookId, data.cover, true) : Promise.resolve()]); - this.scoreService.calculateAndSave(bookId).catch((error: Error) => { - this.logger.warn( - `[metadata.score_calculation] [fail] bookId=${bookId} errorClass=${error.name} error="${sanitizeLogValue(error.message)}" - metadata score calculation failed`, - ); - }); + await this.scoreService.calculateAndSave(bookId); this.logger.debug( `[${event}] [end] bookId=${bookId} format=${format} durationMs=${Date.now() - startedAt} coverExtracted=${data.cover != null} - metadata extraction completed`, @@ -194,8 +190,6 @@ export class MetadataService { } await this.persistCover(bookId, buffer, true); - this.logger.debug(`[${event}] [end] bookId=${bookId} durationMs=${Date.now() - startedAt} saved=true - cover download completed`); - return true; } catch (error) { const errorClass = error instanceof Error ? error.name : 'Error'; const errorMessage = sanitizeLogValue(error instanceof Error ? error.message : String(error)); @@ -204,10 +198,15 @@ export class MetadataService { ); return false; } + + await this.scoreService.calculateAndSave(bookId); + this.logger.debug(`[${event}] [end] bookId=${bookId} durationMs=${Date.now() - startedAt} saved=true - cover download completed`); + return true; } async saveExtractedCoverBytes(bookId: number, bytes: Buffer): Promise { await this.persistCover(bookId, bytes, true); + await this.scoreService.calculateAndSave(bookId); } async refreshCoverForBook(bookId: number, absolutePath: string, format: string): Promise { @@ -235,10 +234,6 @@ export class MetadataService { return false; } await this.persistCover(bookId, data.cover, false); - this.logger.debug( - `[${event}] [end] bookId=${bookId} format=${format} durationMs=${Date.now() - startedAt} refreshed=true - cover refresh completed`, - ); - return true; } catch (error) { const errorClass = error instanceof Error ? error.name : 'Error'; const errorMessage = sanitizeLogValue(error instanceof Error ? error.message : String(error)); @@ -247,6 +242,12 @@ export class MetadataService { ); return false; } + + await this.scoreService.calculateAndSave(bookId); + this.logger.debug( + `[${event}] [end] bookId=${bookId} format=${format} durationMs=${Date.now() - startedAt} refreshed=true - cover refresh completed`, + ); + return true; } // ── Audio helpers ────────────────────────────────────────────────────────────