mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
feat(authors): allow correcting metadata matches
Let users edit author facts, choose and pin a provider record, and keep later refreshes tied to that record. Preserve compatibility for older clients through additive API fields and a discoverable server feature. Keep server typechecking deterministic by breaking circular Drizzle inference and correcting the nullability mismatches it previously hid. Closes #1464
This commit is contained in:
@@ -0,0 +1 @@
|
||||
ALTER TABLE "authors" ADD COLUMN "metadata_provider_pinned" boolean DEFAULT false NOT NULL;
|
||||
File diff suppressed because it is too large
Load Diff
@@ -869,6 +869,13 @@
|
||||
"when": 1792512356419,
|
||||
"tag": "0123_add_koreader_hide_audio_only_books",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 124,
|
||||
"version": "7",
|
||||
"when": 1792516862510,
|
||||
"tag": "0124_add_author_metadata_provider_pinned",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { sql } from 'drizzle-orm';
|
||||
import {
|
||||
type AnyPgColumn,
|
||||
bigint,
|
||||
boolean,
|
||||
check,
|
||||
@@ -30,7 +31,9 @@ export const books = pgTable(
|
||||
libraryFolderId: integer('library_folder_id')
|
||||
.notNull()
|
||||
.references(() => libraryFolders.id, { onDelete: 'cascade' }),
|
||||
primaryFileId: integer('primary_file_id').references(() => bookFiles.id, { onDelete: 'set null' }),
|
||||
// Annotated because book_files references books back: an inferred return makes the two
|
||||
// table types circular, and depending on check order one of them silently becomes any.
|
||||
primaryFileId: integer('primary_file_id').references((): AnyPgColumn => bookFiles.id, { onDelete: 'set null' }),
|
||||
primaryAuthorSortName: varchar('primary_author_sort_name', { length: 500 }),
|
||||
folderPath: varchar('folder_path', { length: 4096 }).notNull(),
|
||||
status: varchar('status', { length: 20 }).notNull().default('present'),
|
||||
|
||||
@@ -181,6 +181,8 @@ export const authors = pgTable(
|
||||
influences: text('influences').array(),
|
||||
metadataProvider: varchar('metadata_provider', { length: 50 }),
|
||||
metadataProviderId: varchar('metadata_provider_id', { length: 128 }),
|
||||
/** Set when a user chose this provider match; enrichment then looks the id up instead of searching by name. */
|
||||
metadataProviderPinned: boolean('metadata_provider_pinned').notNull().default(false),
|
||||
lastEnrichedAt: timestamp('last_enriched_at', { withTimezone: true }),
|
||||
},
|
||||
(t) => [
|
||||
|
||||
@@ -100,7 +100,14 @@ describe('AppInfoService', () => {
|
||||
const info = await service.getAppInfo();
|
||||
expect(info.features).toEqual([...SERVER_FEATURES]);
|
||||
expect(info.features).toEqual(
|
||||
expect.arrayContaining(['annotation-stars', 'annotation-color-names', 'annotation-trash', 'book-journal', 'bookmark-edit']),
|
||||
expect.arrayContaining([
|
||||
'annotation-stars',
|
||||
'annotation-color-names',
|
||||
'annotation-trash',
|
||||
'book-journal',
|
||||
'bookmark-edit',
|
||||
'author-metadata-match',
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -40,8 +40,8 @@ describe('AuthorEnrichmentExecutorService', () => {
|
||||
findByIdForEnrichment: vi.fn(),
|
||||
updateAuthorById: vi.fn(),
|
||||
};
|
||||
const metadataFetch = { collectByProvider: vi.fn() };
|
||||
const imageStorage = { saveFromUrl: vi.fn(), getThumbnailPath: vi.fn() };
|
||||
const metadataFetch = { collectByProvider: vi.fn(), lookupByIdDetailed: vi.fn() };
|
||||
const imageStorage = { saveFromUrl: vi.fn(), getThumbnailPath: vi.fn(), deleteAuthorDir: vi.fn() };
|
||||
|
||||
let service: AuthorEnrichmentExecutorService;
|
||||
|
||||
@@ -245,6 +245,189 @@ describe('AuthorEnrichmentExecutorService', () => {
|
||||
expect(authorsRepo.updateAuthorById).toHaveBeenCalledWith(5, expect.objectContaining({ hasPhoto: true }));
|
||||
});
|
||||
|
||||
describe('pinned match', () => {
|
||||
const PINNED_ROW = {
|
||||
id: 5,
|
||||
name: 'Jane Doe',
|
||||
description: null,
|
||||
hasPhoto: false,
|
||||
bookCount: 3,
|
||||
metadataProvider: AuthorMetadataProviderKey.GOODREADS,
|
||||
metadataProviderId: '4242',
|
||||
metadataProviderPinned: true,
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
authorsRepo.findByIdForEnrichment.mockResolvedValue(PINNED_ROW);
|
||||
metadataFetch.lookupByIdDetailed.mockResolvedValue({ candidate: GOODREADS, failure: null, cacheServed: [], outboundRequests: 1 });
|
||||
});
|
||||
|
||||
it('looks the pinned id up instead of searching by name', async () => {
|
||||
const result = await service.execute({ authorId: 5, preferences: prefs() }, { policy: 'default', allowFallback: true });
|
||||
|
||||
expect(metadataFetch.collectByProvider).not.toHaveBeenCalled();
|
||||
expect(metadataFetch.lookupByIdDetailed).toHaveBeenCalledWith(AuthorMetadataProviderKey.GOODREADS, '4242', 'us', {
|
||||
directive: { policy: 'default', allowFallback: true },
|
||||
});
|
||||
expect(result).toMatchObject({ kind: 'done', provider: AuthorMetadataProviderKey.GOODREADS, descriptionUpdated: true });
|
||||
expect(authorsRepo.updateAuthorById).toHaveBeenCalledWith(5, expect.objectContaining({ description: 'Goodreads bio' }));
|
||||
});
|
||||
|
||||
it('never rewrites the pinned provider id', async () => {
|
||||
metadataFetch.lookupByIdDetailed.mockResolvedValue({
|
||||
candidate: { ...GOODREADS, providerId: '4242.Jane_Doe' },
|
||||
failure: null,
|
||||
cacheServed: [],
|
||||
outboundRequests: 1,
|
||||
});
|
||||
|
||||
await service.execute({ authorId: 5, preferences: prefs() });
|
||||
|
||||
const update = authorsRepo.updateAuthorById.mock.calls[0][1];
|
||||
expect(update).not.toHaveProperty('metadataProvider');
|
||||
expect(update).not.toHaveProperty('metadataProviderId');
|
||||
});
|
||||
|
||||
it('keeps the pin and reports no_match when the id now matches nobody', async () => {
|
||||
metadataFetch.lookupByIdDetailed.mockResolvedValue({ candidate: null, failure: null, cacheServed: [], outboundRequests: 1 });
|
||||
|
||||
const result = await service.execute({ authorId: 5, preferences: prefs() });
|
||||
|
||||
expect(result).toMatchObject({ kind: 'skipped', reason: 'no_match', outboundRequests: 1 });
|
||||
expect(authorsRepo.updateAuthorById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('reports a failed lookup as a failure so the queue retries', async () => {
|
||||
metadataFetch.lookupByIdDetailed.mockResolvedValue({
|
||||
candidate: null,
|
||||
failure: { provider: AuthorMetadataProviderKey.GOODREADS, message: 'HTTP 429', httpStatus: 429, retryAfterMs: 5000, transient: true },
|
||||
cacheServed: [],
|
||||
outboundRequests: 1,
|
||||
});
|
||||
|
||||
const result = await service.execute({ authorId: 5, preferences: prefs() });
|
||||
|
||||
expect(result).toMatchObject({ kind: 'failed', httpStatus: 429, retryAfterMs: 5000, transient: true });
|
||||
});
|
||||
|
||||
it('skips when the pinned provider is no longer used by any field', async () => {
|
||||
const preferences = prefs();
|
||||
for (const field of Object.keys(preferences.fields) as AuthorMetadataField[]) {
|
||||
preferences.fields[field].providers = [AuthorMetadataProviderKey.AUDNEXUS];
|
||||
}
|
||||
|
||||
const result = await service.execute({ authorId: 5, preferences });
|
||||
|
||||
expect(result).toMatchObject({ kind: 'skipped', reason: 'provider_disabled' });
|
||||
expect(metadataFetch.lookupByIdDetailed).not.toHaveBeenCalled();
|
||||
expect(metadataFetch.collectByProvider).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('searches by name again once unpinned', async () => {
|
||||
authorsRepo.findByIdForEnrichment.mockResolvedValue({ ...PINNED_ROW, metadataProviderPinned: false });
|
||||
|
||||
await service.execute({ authorId: 5, preferences: prefs() });
|
||||
|
||||
expect(metadataFetch.lookupByIdDetailed).not.toHaveBeenCalled();
|
||||
expect(metadataFetch.collectByProvider).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('applyMatch', () => {
|
||||
const WRONG_PERSON_ROW = {
|
||||
id: 5,
|
||||
name: 'John Reid',
|
||||
description: 'American journalist.',
|
||||
hasPhoto: true,
|
||||
birthDate: '1950-01-01',
|
||||
birthYear: 1950,
|
||||
deathDate: null,
|
||||
deathYear: null,
|
||||
website: 'https://wrong.example',
|
||||
genres: ['Journalism'],
|
||||
influences: null,
|
||||
bookCount: 2,
|
||||
metadataProvider: AuthorMetadataProviderKey.GOODREADS,
|
||||
metadataProviderId: '1111',
|
||||
metadataProviderPinned: false,
|
||||
};
|
||||
|
||||
it('replaces every enabled field, clears what the match lacks and pins it', async () => {
|
||||
const match = {
|
||||
provider: AuthorMetadataProviderKey.GOODREADS,
|
||||
providerId: '2222',
|
||||
name: 'John Reid',
|
||||
description: 'Scottish novelist.',
|
||||
genres: ['Thriller'],
|
||||
};
|
||||
|
||||
const result = await service.applyMatch(WRONG_PERSON_ROW as never, prefs(), match);
|
||||
|
||||
expect(imageStorage.deleteAuthorDir).toHaveBeenCalledWith(5);
|
||||
expect(authorsRepo.updateAuthorById).toHaveBeenCalledWith(5, {
|
||||
description: 'Scottish novelist.',
|
||||
birthDate: null,
|
||||
birthYear: null,
|
||||
deathDate: null,
|
||||
deathYear: null,
|
||||
website: null,
|
||||
genres: ['Thriller'],
|
||||
influences: null,
|
||||
metadataProvider: AuthorMetadataProviderKey.GOODREADS,
|
||||
metadataProviderId: '2222',
|
||||
metadataProviderPinned: true,
|
||||
hasPhoto: true,
|
||||
lastEnrichedAt: expect.any(Date),
|
||||
});
|
||||
expect(result).toMatchObject({ kind: 'applied', imageUpdated: true, downloads: 0 });
|
||||
});
|
||||
|
||||
it('stores the match portrait and leaves disabled fields alone', async () => {
|
||||
const result = await service.applyMatch(WRONG_PERSON_ROW as never, prefs({ website: { enabled: false } }), GOODREADS);
|
||||
|
||||
expect(imageStorage.saveFromUrl).toHaveBeenCalledWith(5, 'https://img.example/goodreads.jpg');
|
||||
expect(imageStorage.deleteAuthorDir).not.toHaveBeenCalled();
|
||||
expect(authorsRepo.updateAuthorById.mock.calls[0][1]).not.toHaveProperty('website');
|
||||
expect(result).toMatchObject({ kind: 'applied', imageUpdated: true, downloads: 1 });
|
||||
expect((result as { fieldsUpdated: string[] }).fieldsUpdated).toContain('photo');
|
||||
});
|
||||
|
||||
it('clears fields an Audnexus match cannot supply', async () => {
|
||||
await service.applyMatch(WRONG_PERSON_ROW as never, prefs(), AUDNEXUS);
|
||||
|
||||
expect(authorsRepo.updateAuthorById).toHaveBeenCalledWith(
|
||||
5,
|
||||
expect.objectContaining({
|
||||
description: 'Audnexus bio',
|
||||
birthYear: null,
|
||||
website: null,
|
||||
genres: null,
|
||||
metadataProvider: AuthorMetadataProviderKey.AUDNEXUS,
|
||||
metadataProviderId: 'A1',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('writes nothing when the portrait download fails', async () => {
|
||||
imageStorage.saveFromUrl.mockRejectedValue(new AuthorImageStorageError('rate limited', { httpStatus: 429, transient: true }));
|
||||
|
||||
const result = await service.applyMatch(WRONG_PERSON_ROW as never, prefs(), GOODREADS);
|
||||
|
||||
expect(result).toMatchObject({ kind: 'failed', message: 'rate limited', httpStatus: 429, transient: true });
|
||||
expect(authorsRepo.updateAuthorById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('leaves the existing match untouched when the portrait cannot be stored', async () => {
|
||||
imageStorage.saveFromUrl.mockResolvedValue(false);
|
||||
|
||||
const result = await service.applyMatch(WRONG_PERSON_ROW as never, prefs(), GOODREADS);
|
||||
|
||||
expect(result).toMatchObject({ kind: 'failed', message: 'Failed to save author image', transient: false });
|
||||
expect(imageStorage.deleteAuthorDir).not.toHaveBeenCalled();
|
||||
expect(authorsRepo.updateAuthorById).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('metadata cache', () => {
|
||||
const CACHED = { provider: AuthorMetadataProviderKey.GOODREADS, source: 'cache', stale: false, cachedAt: '2026-09-01T00:00:00.000Z' };
|
||||
|
||||
|
||||
@@ -52,6 +52,14 @@ type AuthorEnrichmentOutcome =
|
||||
|
||||
export type AuthorEnrichmentExecutionResult = AuthorEnrichmentOutcome & AuthorEnrichmentProvenance;
|
||||
|
||||
export type AuthorMatchApplyResult =
|
||||
| { kind: 'applied'; imageUpdated: boolean; fieldsUpdated: AuthorMetadataField[]; downloads: number }
|
||||
| Omit<Extract<AuthorEnrichmentOutcome, { kind: 'failed' }>, 'descriptionUpdated' | 'imageUpdated' | 'fieldsUpdated'>;
|
||||
|
||||
type PinnedMatch = { provider: AuthorMetadataProviderKey; providerId: string };
|
||||
|
||||
const AUTHOR_PROVIDER_KEYS = new Set<string>(Object.values(AuthorMetadataProviderKey));
|
||||
|
||||
type AuthorUpdate = Parameters<AuthorsRepository['updateAuthorById']>[1];
|
||||
|
||||
type SkipReason = 'author_not_found' | 'orphaned' | 'provider_disabled' | 'no_match';
|
||||
@@ -80,11 +88,12 @@ export class AuthorEnrichmentExecutorService {
|
||||
const providerOrder = this.providersInUse(params.preferences);
|
||||
if (providerOrder.length === 0) return this.skipped('provider_disabled');
|
||||
|
||||
const { candidates, failures, outboundRequests, cacheServed } = await this.authorMetadataFetchService.collectByProvider(
|
||||
{ name: author.name, region: 'us', limit: 1 },
|
||||
providerOrder,
|
||||
directive,
|
||||
);
|
||||
const pinned = this.pinnedMatch(author);
|
||||
if (pinned && !providerOrder.includes(pinned.provider)) return this.skipped('provider_disabled');
|
||||
|
||||
const { candidates, failures, outboundRequests, cacheServed } = pinned
|
||||
? await this.collectPinned(pinned, directive)
|
||||
: await this.authorMetadataFetchService.collectByProvider({ name: author.name, region: 'us', limit: 1 }, providerOrder, directive);
|
||||
const provenance: AuthorEnrichmentProvenance = { outboundRequests, cacheServed };
|
||||
|
||||
if (candidates.size === 0) {
|
||||
@@ -107,6 +116,90 @@ export class AuthorEnrichmentExecutorService {
|
||||
return this.applyCandidates(author, params.preferences, candidates, provenance);
|
||||
}
|
||||
|
||||
/**
|
||||
* Makes the author exactly the chosen person. Every enabled field takes the candidate's value and
|
||||
* is cleared where the candidate has none, so nothing from an earlier, wrong match survives. The
|
||||
* portrait is fetched before anything is written, so a failed download leaves the author as it was.
|
||||
*/
|
||||
async applyMatch(
|
||||
author: AuthorEnrichmentRow,
|
||||
preferences: AuthorMetadataPreferences,
|
||||
candidate: AuthorMetadataCandidate,
|
||||
): Promise<AuthorMatchApplyResult> {
|
||||
const update: AuthorUpdate = {};
|
||||
const fieldsUpdated: AuthorMetadataField[] = [];
|
||||
let imageUpdated = false;
|
||||
let downloads = 0;
|
||||
|
||||
if (preferences.fields.photo?.enabled) {
|
||||
if (candidate.imageUrl) {
|
||||
downloads += 1;
|
||||
try {
|
||||
imageUpdated = await this.authorImageStorage.saveFromUrl(author.id, candidate.imageUrl);
|
||||
if (!imageUpdated) {
|
||||
return {
|
||||
kind: 'failed',
|
||||
message: 'Failed to save author image',
|
||||
provider: candidate.provider,
|
||||
httpStatus: null,
|
||||
retryAfterMs: null,
|
||||
transient: false,
|
||||
};
|
||||
}
|
||||
} catch (error) {
|
||||
const storageError = error instanceof AuthorImageStorageError ? error : null;
|
||||
return {
|
||||
kind: 'failed',
|
||||
message: storageError?.message ?? (error instanceof Error ? error.message : 'Failed to save author image'),
|
||||
provider: candidate.provider,
|
||||
httpStatus: storageError?.httpStatus ?? null,
|
||||
retryAfterMs: storageError?.retryAfterMs ?? null,
|
||||
transient: storageError?.transient ?? true,
|
||||
};
|
||||
}
|
||||
} else if (author.hasPhoto) {
|
||||
await this.authorImageStorage.deleteAuthorDir(author.id);
|
||||
imageUpdated = true;
|
||||
}
|
||||
if (imageUpdated) fieldsUpdated.push('photo');
|
||||
}
|
||||
|
||||
for (const field of ALL_AUTHOR_METADATA_FIELDS) {
|
||||
if (field === 'photo' || !preferences.fields[field]?.enabled) continue;
|
||||
const values = this.extractField(field, candidate, 'overwrite');
|
||||
if (!values) continue;
|
||||
Object.assign(update, values);
|
||||
fieldsUpdated.push(field);
|
||||
}
|
||||
|
||||
update.metadataProvider = candidate.provider;
|
||||
update.metadataProviderId = candidate.providerId;
|
||||
update.metadataProviderPinned = true;
|
||||
update.hasPhoto = !!(await this.authorImageStorage.getThumbnailPath(author.id));
|
||||
update.lastEnrichedAt = new Date();
|
||||
await this.authorsRepo.updateAuthorById(author.id, update);
|
||||
|
||||
return { kind: 'applied', imageUpdated, fieldsUpdated, downloads };
|
||||
}
|
||||
|
||||
private pinnedMatch(author: AuthorEnrichmentRow): PinnedMatch | null {
|
||||
if (!author.metadataProviderPinned || !author.metadataProviderId) return null;
|
||||
if (!author.metadataProvider || !AUTHOR_PROVIDER_KEYS.has(author.metadataProvider)) return null;
|
||||
return { provider: author.metadataProvider as AuthorMetadataProviderKey, providerId: author.metadataProviderId };
|
||||
}
|
||||
|
||||
/** A pinned author is looked up by id alone: a name search is what found the wrong person. */
|
||||
private async collectPinned(pinned: PinnedMatch, directive: CacheDirective) {
|
||||
const { candidate, failure, cacheServed, outboundRequests } = await this.authorMetadataFetchService.lookupByIdDetailed(
|
||||
pinned.provider,
|
||||
pinned.providerId,
|
||||
'us',
|
||||
{ directive },
|
||||
);
|
||||
const candidates = new Map<AuthorMetadataProviderKey, AuthorMetadataCandidate>(candidate ? [[pinned.provider, candidate]] : []);
|
||||
return { candidates, failures: failure ? [failure] : [], cacheServed, outboundRequests };
|
||||
}
|
||||
|
||||
private async applyCandidates(
|
||||
author: AuthorEnrichmentRow,
|
||||
preferences: AuthorMetadataPreferences,
|
||||
@@ -143,7 +236,7 @@ export class AuthorEnrichmentExecutorService {
|
||||
}
|
||||
|
||||
const attribution = winners.get('description') ?? winners.get('photo') ?? [...winners.values()][0] ?? null;
|
||||
if (attribution) {
|
||||
if (attribution && !author.metadataProviderPinned) {
|
||||
const candidate = candidates.get(attribution);
|
||||
update.metadataProvider = attribution;
|
||||
update.metadataProviderId = candidate?.providerId ?? null;
|
||||
|
||||
@@ -4,10 +4,11 @@ import { BadRequestException } from '@nestjs/common';
|
||||
import { createReadStream } from 'fs';
|
||||
import { stat } from 'fs/promises';
|
||||
import { of } from 'rxjs';
|
||||
import { Permission } from '@bookorbit/types';
|
||||
import { AuditAction, Permission } from '@bookorbit/types';
|
||||
|
||||
import { AUDITABLE_KEY } from '../../common/decorators/auditable.decorator';
|
||||
import { FORBIDDEN_PERMISSION_KEY } from '../../common/decorators/forbid-permission.decorator';
|
||||
import { PERMISSION_KEY } from '../../common/decorators/require-permission.decorator';
|
||||
import type { RequestUser } from '../../common/types/request-user';
|
||||
import { AuthorsController } from './authors.controller';
|
||||
import { BulkRefreshAuthorsMetadataDto } from './dto/bulk-refresh-authors-metadata.dto';
|
||||
@@ -95,6 +96,8 @@ function makeController() {
|
||||
uploadImage: vi.fn(),
|
||||
deleteImage: vi.fn(),
|
||||
refreshEnrichment: vi.fn(),
|
||||
pinMetadataMatch: vi.fn(),
|
||||
unpinMetadataMatch: vi.fn(),
|
||||
update: vi.fn(),
|
||||
merge: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
@@ -378,6 +381,26 @@ describe('AuthorsController', () => {
|
||||
await expect(controller.delete(user, { authorIds: [2] })).resolves.toEqual({ deletedAuthorIds: [2], affectedBookCount: 3 });
|
||||
});
|
||||
|
||||
it('delegates metadata match pin and unpin, gated and audited like an author edit', async () => {
|
||||
const { controller, authorsService } = makeController();
|
||||
const user = makeUser();
|
||||
const dto = { provider: 'goodreads', providerId: '4567' } as never;
|
||||
authorsService.pinMetadataMatch.mockResolvedValue({ id: 9, metadataProviderPinned: true });
|
||||
authorsService.unpinMetadataMatch.mockResolvedValue({ id: 9, metadataProviderPinned: false });
|
||||
|
||||
await expect(controller.pinMetadataMatch(user, 9, dto)).resolves.toEqual({ id: 9, metadataProviderPinned: true });
|
||||
expect(authorsService.pinMetadataMatch).toHaveBeenCalledWith(user, 9, dto);
|
||||
await expect(controller.unpinMetadataMatch(user, 9)).resolves.toEqual({ id: 9, metadataProviderPinned: false });
|
||||
expect(authorsService.unpinMetadataMatch).toHaveBeenCalledWith(user, 9);
|
||||
|
||||
for (const handler of [AuthorsController.prototype.pinMetadataMatch, AuthorsController.prototype.unpinMetadataMatch]) {
|
||||
expect(Reflect.getMetadata(PERMISSION_KEY, handler)).toBe(Permission.LibraryEditMetadata);
|
||||
const audit = Reflect.getMetadata(AUDITABLE_KEY, handler) as { action: string; getResourceId: (req: unknown) => number };
|
||||
expect(audit.action).toBe(AuditAction.AuthorUpdate);
|
||||
expect(audit.getResourceId({ params: { id: '9' } })).toBe(9);
|
||||
}
|
||||
});
|
||||
|
||||
it('delegates delete image endpoint to AuthorsService', async () => {
|
||||
const { controller, authorsService } = makeController();
|
||||
const user = makeUser();
|
||||
|
||||
@@ -44,6 +44,7 @@ import { ListAuthorMetadataDto } from './dto/list-author-metadata.dto';
|
||||
import { ListAuthorLettersDto, ListAuthorsDto } from './dto/list-authors.dto';
|
||||
import { LookupAuthorMetadataDto } from './dto/lookup-author-metadata.dto';
|
||||
import { MergeAuthorsDto } from './dto/merge-authors.dto';
|
||||
import { PinAuthorMetadataMatchDto } from './dto/pin-author-metadata-match.dto';
|
||||
import { PreviewAuthorEnrichmentCountDto } from './dto/preview-author-enrichment-count.dto';
|
||||
import { UpdateAuthorDto } from './dto/update-author.dto';
|
||||
import { AuthorsService } from './authors.service';
|
||||
@@ -306,6 +307,30 @@ export class AuthorsController {
|
||||
return this.authorsService.refreshEnrichment(user, id);
|
||||
}
|
||||
|
||||
@Put(':id/metadata-match')
|
||||
@RequirePermission(Permission.LibraryEditMetadata)
|
||||
@Auditable({
|
||||
action: AuditAction.AuthorUpdate,
|
||||
resource: AuditResource.Author,
|
||||
getResourceId: (req) => parseInt(req.params['id'], 10),
|
||||
description: (req) => `Pinned the metadata match for author #${req.params['id']}`,
|
||||
})
|
||||
pinMetadataMatch(@CurrentUser() user: RequestUser, @Param('id', ParseIntPipe) id: number, @Body() dto: PinAuthorMetadataMatchDto) {
|
||||
return this.authorsService.pinMetadataMatch(user, id, dto);
|
||||
}
|
||||
|
||||
@Delete(':id/metadata-match')
|
||||
@RequirePermission(Permission.LibraryEditMetadata)
|
||||
@Auditable({
|
||||
action: AuditAction.AuthorUpdate,
|
||||
resource: AuditResource.Author,
|
||||
getResourceId: (req) => parseInt(req.params['id'], 10),
|
||||
description: (req) => `Unpinned the metadata match for author #${req.params['id']}`,
|
||||
})
|
||||
unpinMetadataMatch(@CurrentUser() user: RequestUser, @Param('id', ParseIntPipe) id: number) {
|
||||
return this.authorsService.unpinMetadataMatch(user, id);
|
||||
}
|
||||
|
||||
@Patch(':id')
|
||||
@RequirePermission(Permission.LibraryEditMetadata)
|
||||
@Auditable({
|
||||
|
||||
@@ -47,6 +47,7 @@ export type AuthorDetailRow = AuthorSummaryRow & {
|
||||
influences: string[] | null;
|
||||
metadataProvider: string | null;
|
||||
metadataProviderId: string | null;
|
||||
metadataProviderPinned: boolean;
|
||||
};
|
||||
|
||||
export type AuthorEnrichmentRow = AuthorSummaryRow & {
|
||||
@@ -58,6 +59,9 @@ export type AuthorEnrichmentRow = AuthorSummaryRow & {
|
||||
website: string | null;
|
||||
genres: string[] | null;
|
||||
influences: string[] | null;
|
||||
metadataProvider: string | null;
|
||||
metadataProviderId: string | null;
|
||||
metadataProviderPinned: boolean;
|
||||
};
|
||||
|
||||
@Injectable()
|
||||
@@ -188,6 +192,7 @@ export class AuthorsRepository {
|
||||
influences: authors.influences,
|
||||
metadataProvider: authors.metadataProvider,
|
||||
metadataProviderId: authors.metadataProviderId,
|
||||
metadataProviderPinned: authors.metadataProviderPinned,
|
||||
bookCount: sql<number>`count(distinct ${books.id})::int`,
|
||||
lastAddedAt: max(books.addedAt),
|
||||
})
|
||||
@@ -209,6 +214,7 @@ export class AuthorsRepository {
|
||||
authors.influences,
|
||||
authors.metadataProvider,
|
||||
authors.metadataProviderId,
|
||||
authors.metadataProviderPinned,
|
||||
)
|
||||
.limit(1);
|
||||
|
||||
@@ -230,6 +236,9 @@ export class AuthorsRepository {
|
||||
website: authors.website,
|
||||
genres: authors.genres,
|
||||
influences: authors.influences,
|
||||
metadataProvider: authors.metadataProvider,
|
||||
metadataProviderId: authors.metadataProviderId,
|
||||
metadataProviderPinned: authors.metadataProviderPinned,
|
||||
bookCount: sql<number>`count(distinct ${books.id})::int`,
|
||||
lastAddedAt: max(books.addedAt),
|
||||
})
|
||||
@@ -250,6 +259,9 @@ export class AuthorsRepository {
|
||||
authors.website,
|
||||
authors.genres,
|
||||
authors.influences,
|
||||
authors.metadataProvider,
|
||||
authors.metadataProviderId,
|
||||
authors.metadataProviderPinned,
|
||||
)
|
||||
.limit(1);
|
||||
return row ?? null;
|
||||
@@ -333,6 +345,7 @@ export class AuthorsRepository {
|
||||
influences: string[] | null;
|
||||
metadataProvider: string | null;
|
||||
metadataProviderId: string | null;
|
||||
metadataProviderPinned: boolean;
|
||||
lastEnrichedAt: Date | null;
|
||||
}>,
|
||||
) {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { BadRequestException, ForbiddenException, NotFoundException } from '@nestjs/common';
|
||||
import { BadRequestException, ForbiddenException, NotFoundException, ServiceUnavailableException } from '@nestjs/common';
|
||||
import { EMPTY_CONTENT_FILTER_RULES } from '@bookorbit/types';
|
||||
|
||||
import { AUTHOR_ENRICHMENT_REASONS } from './author-enrichment-reasons';
|
||||
@@ -72,6 +72,7 @@ describe('AuthorsService', () => {
|
||||
listProviders: vi.fn(),
|
||||
search: vi.fn(),
|
||||
lookupById: vi.fn(),
|
||||
lookupByIdDetailed: vi.fn(),
|
||||
stream: vi.fn(),
|
||||
quickSearch: vi.fn(),
|
||||
};
|
||||
@@ -88,6 +89,7 @@ describe('AuthorsService', () => {
|
||||
|
||||
const enrichmentExecutor = {
|
||||
execute: vi.fn(),
|
||||
applyMatch: vi.fn(),
|
||||
};
|
||||
|
||||
const enrichmentOrchestrator = {
|
||||
@@ -796,6 +798,181 @@ describe('AuthorsService', () => {
|
||||
await expect(service.update(reqUser(), 31, { name: ' ' })).rejects.toBeInstanceOf(BadRequestException);
|
||||
});
|
||||
|
||||
describe('author facts', () => {
|
||||
beforeEach(() => {
|
||||
authorsRepo.findVisibleAuthorIds.mockResolvedValue([32]);
|
||||
authorsRepo.findRelatedLibraryIds.mockResolvedValue([1]);
|
||||
authorsRepo.updateAuthorById.mockResolvedValue({ id: 32 });
|
||||
authorsRepo.findById.mockResolvedValue({ id: 32, name: 'John Reid', sortName: null, description: null, bookCount: 3, lastAddedAt: null });
|
||||
});
|
||||
|
||||
it('update writes facts, takes the year from a full date and tidies lists', async () => {
|
||||
await service.update(reqUser(), 32, {
|
||||
website: ' https://johnreid.example ',
|
||||
birthDate: '1965-03-14',
|
||||
birthYear: 1900,
|
||||
deathYear: 2020,
|
||||
genres: [' Thriller ', 'thriller', '', 'Military fiction'],
|
||||
influences: [' '],
|
||||
});
|
||||
|
||||
expect(authorsRepo.updateAuthorById).toHaveBeenCalledWith(32, {
|
||||
website: 'https://johnreid.example',
|
||||
birthDate: '1965-03-14',
|
||||
birthYear: 1965,
|
||||
deathDate: null,
|
||||
deathYear: 2020,
|
||||
genres: ['Thriller', 'Military fiction'],
|
||||
influences: null,
|
||||
});
|
||||
expect(enrichmentOrchestrator.schedule).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('update clears facts sent as null and keeps a year sent beside a cleared date', async () => {
|
||||
await service.update(reqUser(), 32, { website: null, birthDate: null, birthYear: 1965, deathDate: null, genres: null });
|
||||
|
||||
expect(authorsRepo.updateAuthorById).toHaveBeenCalledWith(32, {
|
||||
website: null,
|
||||
birthDate: null,
|
||||
birthYear: 1965,
|
||||
deathDate: null,
|
||||
genres: null,
|
||||
});
|
||||
});
|
||||
|
||||
it.each(['1965-02-30', '1965-13-01', '0000-01-01'])('update rejects the impossible date %s', async (birthDate) => {
|
||||
await expect(service.update(reqUser(), 32, { birthDate })).rejects.toBeInstanceOf(BadRequestException);
|
||||
expect(authorsRepo.updateAuthorById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('update accepts a leap day and a first-century date', async () => {
|
||||
await service.update(reqUser(), 32, { birthDate: '2024-02-29', deathDate: '0065-04-12' });
|
||||
|
||||
expect(authorsRepo.updateAuthorById).toHaveBeenCalledWith(32, {
|
||||
birthDate: '2024-02-29',
|
||||
birthYear: 2024,
|
||||
deathDate: '0065-04-12',
|
||||
deathYear: 65,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('metadata match', () => {
|
||||
const enrichmentRow = { id: 33, name: 'John Reid', hasPhoto: true, bookCount: 2, metadataProviderPinned: false };
|
||||
const candidate = { provider: 'goodreads', providerId: '4567', name: 'John Reid', description: 'Scottish novelist.' };
|
||||
|
||||
beforeEach(() => {
|
||||
authorsRepo.findVisibleAuthorIds.mockResolvedValue([33]);
|
||||
authorsRepo.findRelatedLibraryIds.mockResolvedValue([1]);
|
||||
authorsRepo.findByIdForEnrichment.mockResolvedValue(enrichmentRow);
|
||||
authorsRepo.findById.mockResolvedValue({
|
||||
id: 33,
|
||||
name: 'John Reid',
|
||||
sortName: null,
|
||||
description: 'Scottish novelist.',
|
||||
bookCount: 2,
|
||||
lastAddedAt: null,
|
||||
metadataProvider: 'goodreads',
|
||||
metadataProviderId: '4567',
|
||||
metadataProviderPinned: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('pins the looked-up person and returns the replaced author', async () => {
|
||||
authorMetadataFetchService.lookupByIdDetailed.mockResolvedValue({ candidate, failure: null, cacheServed: [], outboundRequests: 1 });
|
||||
enrichmentExecutor.applyMatch.mockResolvedValue({ kind: 'applied', imageUpdated: true, fieldsUpdated: ['description', 'photo'], downloads: 1 });
|
||||
|
||||
const result = await service.pinMetadataMatch(reqUser(), 33, { provider: 'goodreads', providerId: '4567' } as any);
|
||||
|
||||
expect(authorMetadataFetchService.lookupByIdDetailed).toHaveBeenCalledWith('goodreads', '4567', 'us', {
|
||||
directive: { policy: 'refresh', allowFallback: true },
|
||||
});
|
||||
expect(enrichmentExecutor.applyMatch).toHaveBeenCalledWith(enrichmentRow, expect.objectContaining({ fields: expect.any(Object) }), candidate);
|
||||
expect(result).toEqual(expect.objectContaining({ id: 33, metadataProviderPinned: true }));
|
||||
expect(result).not.toHaveProperty('metadataCache');
|
||||
});
|
||||
|
||||
it('reports a cache fallback the same way a refresh does', async () => {
|
||||
const served = { provider: 'goodreads', source: 'fallback', fetchedAt: '2026-10-01T00:00:00.000Z' };
|
||||
authorMetadataFetchService.lookupByIdDetailed.mockResolvedValue({ candidate, failure: null, cacheServed: [served], outboundRequests: 1 });
|
||||
enrichmentExecutor.applyMatch.mockResolvedValue({ kind: 'applied', imageUpdated: false, fieldsUpdated: [], downloads: 0 });
|
||||
|
||||
const result = await service.pinMetadataMatch(reqUser(), 33, { provider: 'goodreads', providerId: '4567' } as any);
|
||||
|
||||
expect(result.metadataCache).toEqual({ fallbackProviders: [served] });
|
||||
});
|
||||
|
||||
it('answers 404 when the id matches nobody and leaves the author alone', async () => {
|
||||
authorMetadataFetchService.lookupByIdDetailed.mockResolvedValue({ candidate: null, failure: null, cacheServed: [], outboundRequests: 1 });
|
||||
|
||||
await expect(service.pinMetadataMatch(reqUser(), 33, { provider: 'goodreads', providerId: '999' } as any)).rejects.toBeInstanceOf(
|
||||
NotFoundException,
|
||||
);
|
||||
expect(enrichmentExecutor.applyMatch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('answers 503 when the provider fails rather than calling it a missing author', async () => {
|
||||
authorMetadataFetchService.lookupByIdDetailed.mockResolvedValue({
|
||||
candidate: null,
|
||||
failure: { provider: 'goodreads', message: 'HTTP 503', httpStatus: 503, retryAfterMs: null, transient: true },
|
||||
cacheServed: [],
|
||||
outboundRequests: 1,
|
||||
});
|
||||
|
||||
await expect(service.pinMetadataMatch(reqUser(), 33, { provider: 'goodreads', providerId: '4567' } as any)).rejects.toBeInstanceOf(
|
||||
ServiceUnavailableException,
|
||||
);
|
||||
expect(enrichmentExecutor.applyMatch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('answers 404 when the provider reports that the id does not exist', async () => {
|
||||
authorMetadataFetchService.lookupByIdDetailed.mockResolvedValue({
|
||||
candidate: null,
|
||||
failure: { provider: 'goodreads', message: 'HTTP 404', httpStatus: 404, retryAfterMs: null, transient: false },
|
||||
cacheServed: [],
|
||||
outboundRequests: 1,
|
||||
});
|
||||
|
||||
await expect(service.pinMetadataMatch(reqUser(), 33, { provider: 'goodreads', providerId: '999' } as any)).rejects.toBeInstanceOf(
|
||||
NotFoundException,
|
||||
);
|
||||
expect(enrichmentExecutor.applyMatch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('answers 503 when the portrait download fails', async () => {
|
||||
authorMetadataFetchService.lookupByIdDetailed.mockResolvedValue({ candidate, failure: null, cacheServed: [], outboundRequests: 1 });
|
||||
enrichmentExecutor.applyMatch.mockResolvedValue({
|
||||
kind: 'failed',
|
||||
message: 'HTTP 429',
|
||||
provider: 'goodreads',
|
||||
httpStatus: 429,
|
||||
retryAfterMs: 1000,
|
||||
transient: true,
|
||||
});
|
||||
|
||||
await expect(service.pinMetadataMatch(reqUser(), 33, { provider: 'goodreads', providerId: '4567' } as any)).rejects.toBeInstanceOf(
|
||||
ServiceUnavailableException,
|
||||
);
|
||||
});
|
||||
|
||||
it('checks library access before looking anything up', async () => {
|
||||
libraryService.findEditableLibraryIds.mockResolvedValue([]);
|
||||
|
||||
await expect(service.pinMetadataMatch(reqUser(), 33, { provider: 'goodreads', providerId: '4567' } as any)).rejects.toBeInstanceOf(
|
||||
ForbiddenException,
|
||||
);
|
||||
expect(authorMetadataFetchService.lookupByIdDetailed).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('unpins without touching the metadata', async () => {
|
||||
authorsRepo.updateAuthorById.mockResolvedValue({ id: 33 });
|
||||
|
||||
await service.unpinMetadataMatch(reqUser(), 33);
|
||||
|
||||
expect(authorsRepo.updateAuthorById).toHaveBeenCalledWith(33, { metadataProviderPinned: false });
|
||||
});
|
||||
});
|
||||
|
||||
it('get image paths verify readability before touching storage', async () => {
|
||||
authorsRepo.findVisibleAuthorIds.mockResolvedValue([40]);
|
||||
authorImageStorage.getThumbnailPath.mockResolvedValue('/tmp/thumb.jpg');
|
||||
|
||||
@@ -41,6 +41,7 @@ import { ListAuthorMetadataDto } from './dto/list-author-metadata.dto';
|
||||
import { ListAuthorLettersDto, ListAuthorsDto } from './dto/list-authors.dto';
|
||||
import { LookupAuthorMetadataDto } from './dto/lookup-author-metadata.dto';
|
||||
import { MergeAuthorsDto } from './dto/merge-authors.dto';
|
||||
import { PinAuthorMetadataMatchDto } from './dto/pin-author-metadata-match.dto';
|
||||
import { UpdateAuthorDto } from './dto/update-author.dto';
|
||||
import { AuthorMetadataFetchService } from './metadata/author-metadata-fetch.service';
|
||||
|
||||
@@ -55,6 +56,47 @@ type EnrichmentRefreshOutcome = {
|
||||
cacheServed: MetadataCacheServedProvider[];
|
||||
};
|
||||
|
||||
type LifeDateUpdate = { date?: string | null; year?: number | null };
|
||||
|
||||
/** A full date carries its own year, and a year sent on its own clears a date that may name another. */
|
||||
function resolveLifeDate(
|
||||
field: string,
|
||||
hasDate: boolean,
|
||||
date: string | null | undefined,
|
||||
hasYear: boolean,
|
||||
year: number | null | undefined,
|
||||
): LifeDateUpdate {
|
||||
if (hasDate && date) {
|
||||
if (!isCalendarDate(date)) throw new BadRequestException(`${field} is not a calendar date`);
|
||||
return { date, year: Number(date.slice(0, 4)) };
|
||||
}
|
||||
if (hasDate) return hasYear ? { date: null, year: year ?? null } : { date: null };
|
||||
if (hasYear) return { date: null, year: year ?? null };
|
||||
return {};
|
||||
}
|
||||
|
||||
function isCalendarDate(value: string): boolean {
|
||||
const [year, month, day] = value.split('-').map(Number);
|
||||
if (year < 1) return false;
|
||||
// setUTCFullYear, because Date.UTC reads years below 100 as 19xx.
|
||||
const parsed = new Date(0);
|
||||
parsed.setUTCFullYear(year, month - 1, day);
|
||||
return parsed.getUTCFullYear() === year && parsed.getUTCMonth() === month - 1 && parsed.getUTCDate() === day;
|
||||
}
|
||||
|
||||
function normalizeAuthorList(values: string[] | null | undefined): string[] | null {
|
||||
const seen = new Set<string>();
|
||||
const result: string[] = [];
|
||||
for (const value of values ?? []) {
|
||||
const trimmed = value.trim();
|
||||
const key = trimmed.toLocaleLowerCase();
|
||||
if (!trimmed || seen.has(key)) continue;
|
||||
seen.add(key);
|
||||
result.push(trimmed);
|
||||
}
|
||||
return result.length > 0 ? result : null;
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
export class AuthorsService {
|
||||
static readonly MAX_AUTHOR_IMAGE_BYTES = 20 * 1024 * 1024;
|
||||
@@ -337,6 +379,26 @@ export class AuthorsService {
|
||||
values.description = dto.description?.trim() || null;
|
||||
}
|
||||
|
||||
if ('website' in dto) {
|
||||
values.website = dto.website?.trim() || null;
|
||||
}
|
||||
|
||||
const birth = resolveLifeDate('birthDate', 'birthDate' in dto, dto.birthDate, 'birthYear' in dto, dto.birthYear);
|
||||
if (birth.date !== undefined) values.birthDate = birth.date;
|
||||
if (birth.year !== undefined) values.birthYear = birth.year;
|
||||
|
||||
const death = resolveLifeDate('deathDate', 'deathDate' in dto, dto.deathDate, 'deathYear' in dto, dto.deathYear);
|
||||
if (death.date !== undefined) values.deathDate = death.date;
|
||||
if (death.year !== undefined) values.deathYear = death.year;
|
||||
|
||||
if ('genres' in dto) {
|
||||
values.genres = normalizeAuthorList(dto.genres);
|
||||
}
|
||||
|
||||
if ('influences' in dto) {
|
||||
values.influences = normalizeAuthorList(dto.influences);
|
||||
}
|
||||
|
||||
const fieldNames = Object.keys(values);
|
||||
if (fieldNames.length === 0) {
|
||||
const detail = await this.findOne(user, authorId);
|
||||
@@ -466,6 +528,57 @@ export class AuthorsService {
|
||||
}
|
||||
}
|
||||
|
||||
/** Points the author at one provider's person, replacing its metadata, and keeps refreshes on that id. */
|
||||
async pinMetadataMatch(user: RequestUser, authorId: number, dto: PinAuthorMetadataMatchDto): Promise<AuthorRefreshResult> {
|
||||
const event = 'author.pin_metadata_match';
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(
|
||||
`[${event}] [start] userId=${user.id} authorId=${authorId} provider=${dto.provider} providerId=${JSON.stringify(dto.providerId)} - author metadata match pin started`,
|
||||
);
|
||||
try {
|
||||
await this.assertMutationAccess(user, [authorId]);
|
||||
const author = await this.authorsRepo.findByIdForEnrichment(authorId);
|
||||
if (!author) throw new NotFoundException('Author not found');
|
||||
|
||||
const lookup = await this.authorMetadataFetchService.lookupByIdDetailed(dto.provider, dto.providerId, 'us', {
|
||||
directive: SINGLE_REFRESH_CACHE_DIRECTIVE,
|
||||
});
|
||||
if (lookup.failure) {
|
||||
if (lookup.failure.httpStatus === 404) throw new NotFoundException(`No ${dto.provider} author has that id`);
|
||||
throw new ServiceUnavailableException(`Author lookup failed for provider ${dto.provider}: ${lookup.failure.message}`);
|
||||
}
|
||||
if (!lookup.candidate) throw new NotFoundException(`No ${dto.provider} author has that id`);
|
||||
|
||||
const preferences = await this.metadataPreferences.getPreferences();
|
||||
const applied = await this.enrichmentExecutor.applyMatch(author, preferences, lookup.candidate);
|
||||
if (applied.kind === 'failed') {
|
||||
throw new ServiceUnavailableException(`Author image download failed for provider ${dto.provider}: ${applied.message}`);
|
||||
}
|
||||
|
||||
const detail = await this.findOne(user, authorId);
|
||||
const fallbackProviders = lookup.cacheServed.filter((served) => served.source === 'fallback');
|
||||
this.logger.log(
|
||||
`[${event}] [end] userId=${user.id} authorId=${authorId} provider=${dto.provider} durationMs=${Date.now() - startedAt} fields=${applied.fieldsUpdated.join(',') || 'none'} imageUpdated=${applied.imageUpdated} cacheFallbacks=${fallbackProviders.length} - author metadata match pin completed`,
|
||||
);
|
||||
return fallbackProviders.length > 0 ? { ...detail, metadataCache: { fallbackProviders } } : detail;
|
||||
} 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] userId=${user.id} authorId=${authorId} provider=${dto.provider} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${errorMessage}" - author metadata match pin failed`,
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
/** Lets refreshes search by name again; the metadata and the provider id stay as they are. */
|
||||
async unpinMetadataMatch(user: RequestUser, authorId: number): Promise<AuthorDetail> {
|
||||
await this.assertMutationAccess(user, [authorId]);
|
||||
const updated = await this.authorsRepo.updateAuthorById(authorId, { metadataProviderPinned: false });
|
||||
if (!updated) throw new NotFoundException('Author not found');
|
||||
return this.findOne(user, authorId);
|
||||
}
|
||||
|
||||
async getThumbnailPath(user: RequestUser, authorId: number): Promise<string | null> {
|
||||
await this.assertAuthorReadable(user, [authorId]);
|
||||
return this.authorImageStorage.getThumbnailPath(authorId);
|
||||
@@ -680,6 +793,7 @@ export class AuthorsService {
|
||||
influences: row.influences ?? [],
|
||||
metadataProvider: (row.metadataProvider as AuthorDetail['metadataProvider']) ?? null,
|
||||
metadataProviderId: row.metadataProviderId,
|
||||
metadataProviderPinned: row.metadataProviderPinned,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@ import { ListAuthorMetadataDto } from './list-author-metadata.dto';
|
||||
import { ListAuthorsDto } from './list-authors.dto';
|
||||
import { LookupAuthorMetadataDto } from './lookup-author-metadata.dto';
|
||||
import { MergeAuthorsDto } from './merge-authors.dto';
|
||||
import { PinAuthorMetadataMatchDto } from './pin-author-metadata-match.dto';
|
||||
import { PreviewAuthorEnrichmentCountDto } from './preview-author-enrichment-count.dto';
|
||||
import { UpdateAuthorDto } from './update-author.dto';
|
||||
|
||||
@@ -120,6 +121,45 @@ describe('Authors DTO validation', () => {
|
||||
expect((await errorsFor(UpdateAuthorDto, { name: 'x'.repeat(501) })).length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('accepts author facts and clearing them with null', async () => {
|
||||
expect(
|
||||
(
|
||||
await errorsFor(UpdateAuthorDto, {
|
||||
website: 'https://johnreid.example/about',
|
||||
birthDate: '1965-03-14',
|
||||
birthYear: 1965,
|
||||
deathDate: null,
|
||||
deathYear: null,
|
||||
genres: ['Thriller', 'Military fiction'],
|
||||
influences: ['Alistair MacLean'],
|
||||
})
|
||||
).length,
|
||||
).toBe(0);
|
||||
expect((await errorsFor(UpdateAuthorDto, { website: null, birthDate: null, genres: null, influences: null })).length).toBe(0);
|
||||
expect((await errorsFor(UpdateAuthorDto, { birthYear: -800 })).length).toBe(0);
|
||||
});
|
||||
|
||||
it('rejects unsafe or malformed author facts', async () => {
|
||||
expect((await errorsFor(UpdateAuthorDto, { website: 'javascript:alert(1)' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateAuthorDto, { website: 'johnreid.example' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateAuthorDto, { website: 'ftp://johnreid.example' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateAuthorDto, { birthDate: '14/03/1965' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateAuthorDto, { birthYear: 1965.5 })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateAuthorDto, { deathYear: 10_000 })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateAuthorDto, { genres: 'Thriller' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateAuthorDto, { genres: Array.from({ length: 51 }, (_, i) => `g${i}`) })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(UpdateAuthorDto, { influences: ['x'.repeat(501)] })).length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('validates a metadata match pin', async () => {
|
||||
expect((await errorsFor(PinAuthorMetadataMatchDto, { provider: AuthorMetadataProviderKey.GOODREADS, providerId: '12345' })).length).toBe(0);
|
||||
expect((await errorsFor(PinAuthorMetadataMatchDto, { provider: 'openlibrary', providerId: '12345' })).length).toBeGreaterThan(0);
|
||||
expect((await errorsFor(PinAuthorMetadataMatchDto, { provider: AuthorMetadataProviderKey.GOODREADS, providerId: '' })).length).toBeGreaterThan(0);
|
||||
expect(
|
||||
(await errorsFor(PinAuthorMetadataMatchDto, { provider: AuthorMetadataProviderKey.AUDNEXUS, providerId: 'B'.repeat(129) })).length,
|
||||
).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('requires enrichment preview conditions and boolean flags', async () => {
|
||||
expect(
|
||||
(
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
import { IsEnum, IsNotEmpty, IsString, MaxLength } from 'class-validator';
|
||||
import { AuthorMetadataProviderKey } from '@bookorbit/types';
|
||||
|
||||
/** Arrived with the `author-metadata-match` server feature. */
|
||||
export class PinAuthorMetadataMatchDto {
|
||||
@IsEnum(AuthorMetadataProviderKey)
|
||||
provider!: AuthorMetadataProviderKey;
|
||||
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
@MaxLength(128)
|
||||
providerId!: string;
|
||||
}
|
||||
@@ -1,5 +1,13 @@
|
||||
import { IsOptional, IsString, MaxLength } from 'class-validator';
|
||||
import { ArrayMaxSize, IsArray, IsInt, IsOptional, IsString, IsUrl, Matches, Max, MaxLength, Min } from 'class-validator';
|
||||
|
||||
const MAX_AUTHOR_LIST_ITEMS = 50;
|
||||
const MIN_LIFE_YEAR = -9999;
|
||||
const MAX_LIFE_YEAR = 9999;
|
||||
|
||||
/**
|
||||
* Every field beyond `name`, `sortName` and `description` arrived with the `author-metadata-match`
|
||||
* server feature; clients must only send them to servers advertising it.
|
||||
*/
|
||||
export class UpdateAuthorDto {
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@@ -15,4 +23,48 @@ export class UpdateAuthorDto {
|
||||
@IsString()
|
||||
@MaxLength(10_000)
|
||||
description?: string | null;
|
||||
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@MaxLength(2048)
|
||||
@IsUrl({ protocols: ['http', 'https'], require_protocol: true })
|
||||
website?: string | null;
|
||||
|
||||
/** A full date; its year replaces `birthYear`. */
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@Matches(/^\d{4}-\d{2}-\d{2}$/)
|
||||
birthDate?: string | null;
|
||||
|
||||
/** Sent without `birthDate`, it clears the full date, which may name another year. */
|
||||
@IsOptional()
|
||||
@IsInt()
|
||||
@Min(MIN_LIFE_YEAR)
|
||||
@Max(MAX_LIFE_YEAR)
|
||||
birthYear?: number | null;
|
||||
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@Matches(/^\d{4}-\d{2}-\d{2}$/)
|
||||
deathDate?: string | null;
|
||||
|
||||
@IsOptional()
|
||||
@IsInt()
|
||||
@Min(MIN_LIFE_YEAR)
|
||||
@Max(MAX_LIFE_YEAR)
|
||||
deathYear?: number | null;
|
||||
|
||||
@IsOptional()
|
||||
@IsArray()
|
||||
@ArrayMaxSize(MAX_AUTHOR_LIST_ITEMS)
|
||||
@IsString({ each: true })
|
||||
@MaxLength(200, { each: true })
|
||||
genres?: string[] | null;
|
||||
|
||||
@IsOptional()
|
||||
@IsArray()
|
||||
@ArrayMaxSize(MAX_AUTHOR_LIST_ITEMS)
|
||||
@IsString({ each: true })
|
||||
@MaxLength(500, { each: true })
|
||||
influences?: string[] | null;
|
||||
}
|
||||
|
||||
@@ -306,6 +306,20 @@ describe('AuthorMetadataFetchService', () => {
|
||||
await expect(service.lookupById('audnexus', '42')).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it('lookupByIdDetailed reports a provider failure apart from an id that matched nobody', async () => {
|
||||
const lookupById = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new AuthorMetadataProviderError('HTTP 503', { httpStatus: 503, transient: true }))
|
||||
.mockResolvedValueOnce(null);
|
||||
registry.find.mockReturnValue({ key: 'audnexus', label: 'Audnexus', identifiable: true, search: vi.fn(), lookupById });
|
||||
|
||||
await expect(service.lookupByIdDetailed('audnexus', '42')).resolves.toMatchObject({
|
||||
candidate: null,
|
||||
failure: { provider: 'audnexus', message: 'HTTP 503', httpStatus: 503, transient: true },
|
||||
});
|
||||
await expect(service.lookupByIdDetailed('audnexus', '43')).resolves.toMatchObject({ candidate: null, failure: null });
|
||||
});
|
||||
|
||||
describe('cache integration', () => {
|
||||
function cachedProvider(spec = cacheSpec()) {
|
||||
return {
|
||||
|
||||
@@ -57,6 +57,8 @@ export type AuthorMetadataSearchResult = AuthorMetadataFetchProvenance & {
|
||||
|
||||
export type AuthorMetadataLookupResult = AuthorMetadataFetchProvenance & {
|
||||
candidate: AuthorMetadataSearchCandidateEvent | null;
|
||||
/** Set when the provider failed, so a caller can tell an outage from an id that matched nobody. */
|
||||
failure: AuthorMetadataFetchFailure | null;
|
||||
};
|
||||
|
||||
export type AuthorMetadataCollectResult = AuthorMetadataFetchProvenance & {
|
||||
@@ -206,12 +208,13 @@ export class AuthorMetadataFetchService {
|
||||
options?: LookupOptions,
|
||||
): Promise<AuthorMetadataLookupResult> {
|
||||
const provider = this.registry.find(key);
|
||||
if (!provider || !isIdentifiableAuthorProvider(provider)) return { candidate: null, ...summarize([]) };
|
||||
if (!provider || !isIdentifiableAuthorProvider(provider)) return { candidate: null, failure: null, ...summarize([]) };
|
||||
|
||||
const run = await this.startRun(provider, options?.directive ?? DEFAULT_CACHE_DIRECTIVE);
|
||||
const outcome = await run.lookup(canonicalId(providerId) ?? '', { name: '', region: canonicalAuthorRegion(region) });
|
||||
const candidate = outcome.ok && outcome.value ? withCache(outcome.value, outcome.cache) : null;
|
||||
return { candidate, ...summarize([run]) };
|
||||
if (!outcome.ok) return { candidate: null, failure: this.toFailure(key, outcome.error), ...summarize([run]) };
|
||||
const candidate = outcome.value ? withCache(outcome.value, outcome.cache) : null;
|
||||
return { candidate, failure: null, ...summarize([run]) };
|
||||
}
|
||||
|
||||
/** The per-operation deadline; today's provider timeout, now enforced by the executor. */
|
||||
|
||||
@@ -35,7 +35,7 @@ const AUDIO_FORMAT_SQL = sql.join(
|
||||
sql`, `,
|
||||
);
|
||||
|
||||
type OuterBookId = number | typeof books.id | typeof bookMetadata.bookId;
|
||||
type OuterBookId = number | typeof books.id | typeof bookMetadata.bookId | typeof bookCovers.bookId;
|
||||
|
||||
/**
|
||||
* Drizzle leaves a column unqualified when the outer query reads a single table, and inside these
|
||||
|
||||
@@ -1620,7 +1620,7 @@ export class BookRepository {
|
||||
absolutePath?: string;
|
||||
relPath?: string | null;
|
||||
sizeBytes?: number;
|
||||
mediaOverlayAvailable?: boolean | null;
|
||||
mediaOverlayAvailable?: boolean;
|
||||
mediaOverlayDurationSeconds?: number | null;
|
||||
mediaOverlayCheckedAt?: Date | null;
|
||||
},
|
||||
@@ -2158,7 +2158,7 @@ export class BookRepository {
|
||||
absolutePath: string;
|
||||
format: string | null;
|
||||
sizeBytes: number | null;
|
||||
sortOrder: number;
|
||||
sortOrder: number | null;
|
||||
mediaOverlayAvailable: boolean;
|
||||
}[]
|
||||
> {
|
||||
|
||||
@@ -262,6 +262,8 @@ export class UploadService {
|
||||
const ino = fileStat.ino;
|
||||
const relPath = relative(bookRow.libraryFolderPath, destination);
|
||||
const mediaOverlayFields = await this.inspectMediaOverlayFields(destination, format);
|
||||
// The transaction callback cannot see the narrowing of the outer `let`.
|
||||
const absolutePath = destination;
|
||||
|
||||
const { inserted, isPrimary, finalStatus } = await this.db.transaction(async (tx) => {
|
||||
const [lockedBook] = await tx
|
||||
@@ -283,7 +285,7 @@ export class UploadService {
|
||||
.values({
|
||||
bookId,
|
||||
libraryFolderId: bookRow.libraryFolderId,
|
||||
absolutePath: destination,
|
||||
absolutePath,
|
||||
relPath,
|
||||
ino,
|
||||
sizeBytes,
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
{
|
||||
"totalRoutes": 665,
|
||||
"totalRoutes": 667,
|
||||
"byPermission": {
|
||||
"(none)": 287,
|
||||
"Permission.ManageMetadataConfig": 41,
|
||||
"Permission.EmailSend": 34,
|
||||
"Permission.LibraryEditMetadata": 35,
|
||||
"Permission.LibraryEditMetadata": 37,
|
||||
"Permission.BookDockAccess": 17,
|
||||
"Permission.ManageAppSettings": 62,
|
||||
"Permission.ManageLibraries": 24,
|
||||
|
||||
@@ -47,6 +47,30 @@
|
||||
"allowDefault": false,
|
||||
"useGuards": []
|
||||
},
|
||||
{
|
||||
"file": "server/src/modules/authors/authors.controller.ts",
|
||||
"className": "AuthorsController",
|
||||
"methodName": "pinMetadataMatch",
|
||||
"httpMethod": "PUT",
|
||||
"path": "/authors/:id/metadata-match",
|
||||
"permissions": ["Permission.LibraryEditMetadata"],
|
||||
"libraryAccess": [],
|
||||
"isPublic": false,
|
||||
"allowDefault": false,
|
||||
"useGuards": []
|
||||
},
|
||||
{
|
||||
"file": "server/src/modules/authors/authors.controller.ts",
|
||||
"className": "AuthorsController",
|
||||
"methodName": "unpinMetadataMatch",
|
||||
"httpMethod": "DELETE",
|
||||
"path": "/authors/:id/metadata-match",
|
||||
"permissions": ["Permission.LibraryEditMetadata"],
|
||||
"libraryAccess": [],
|
||||
"isPublic": false,
|
||||
"allowDefault": false,
|
||||
"useGuards": []
|
||||
},
|
||||
{
|
||||
"file": "server/src/modules/authors/authors.controller.ts",
|
||||
"className": "AuthorsController",
|
||||
|
||||
Reference in New Issue
Block a user