Migrations/bookorbit
1
0
Fork 0
mirror of https://github.com/bookorbit/bookorbit.git synced 2026-11-03 00:11:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(server): normalize and deduplicate narrator names

Browse Source 
Narrator names and sort names are now normalized to remove duplicate
internal whitespace and leading/trailing spaces. Names are deduplicated
case-insensitively when saved to prevent duplicate metadata entries that
are visually identical or very similar in the UI.

Closes #553
This commit is contained in:
neon
2026-07-05 21:12:34 +00:00
parent 947e88c2bc
commit bc7f1f58bc
27 changed files with 938 additions and 194 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/common/services/series-identity.service.test.ts
+8 -4
View File
@@ -29,10 +29,10 @@ describe('SeriesIdentityService', () => {
it('normalizes names for identity while preserving display casing', () => {
const service = new SeriesIdentityService({} as never);
expect(service.normalizeName(' The Expanse ')).toBe('the expanse');
expect(service.normalizeName(' The Expanse ')).toBe('the expanse');
expect(service.normalizeName(' ')).toBeNull();
expect(service.normalizeName(null)).toBeNull();
expect(service.normalizeDisplayName(' The Expanse ')).toBe('The Expanse');
expect(service.normalizeDisplayName(' The Expanse ')).toBe('The Expanse');
expect(service.normalizeDisplayName(' ')).toBeNull();
});
@@ -49,7 +49,7 @@ describe('SeriesIdentityService', () => {
const { db, insert, values, onConflictDoUpdate, returning } = makeInsertDb([{ id: 77 }]);
const service = new SeriesIdentityService(db);
const id = await service.resolveSeriesId(' Dune Chronicles ');
const id = await service.resolveSeriesId(' Dune Chronicles ');
expect(id).toBe(77);
expect(insert).toHaveBeenCalledTimes(1);
@@ -83,7 +83,7 @@ describe('SeriesIdentityService', () => {
const { db } = makeInsertDb([{ id: 88 }]);
const service = new SeriesIdentityService(db);
const patch = await service.resolveMetadataPatch({ bookId: 1, title: 'Caliban War', seriesName: ' The Expanse ' });
const patch = await service.resolveMetadataPatch({ bookId: 1, title: 'Caliban War', seriesName: ' The Expanse ' });
expect(patch).toEqual({ bookId: 1, title: 'Caliban War', seriesName: 'The Expanse', seriesId: 88 });
});
@@ -106,6 +106,10 @@ describe('SeriesIdentityService', () => {
expect(execute).toHaveBeenCalledTimes(3);
const createSeriesSql = flattenSql(execute.mock.calls[0]![0]).replace(/\s+/g, ' ');
expect(createSeriesSql).toContain('regexp_replace');
expect(createSeriesSql).toContain('replace');
expect(createSeriesSql).toContain('chr(160)');
expect(createSeriesSql).toContain('btrim(regexp_replace');
expect(createSeriesSql).toContain('ON CONFLICT (normalized_name) DO NOTHING');
expect(createSeriesSql).not.toContain('DO UPDATE SET');
expect(createSeriesSql).not.toContain('excluded.name');
server/src/common/services/series-identity.service.ts
+28 -14
View File
@@ -6,6 +6,12 @@ import { DB } from '../../db';
import * as schema from '../../db/schema';
import { bookMetadata, bookSeries, bookSeriesMemberships } from '../../db/schema';
import { sanitizeLogValue } from '../utils/log-sanitize.utils';
import {
normalizeMetadataText,
normalizeMetadataTextKey,
normalizeMetadataTextKeySql,
normalizeMetadataTextOrNullSql,
} from '../utils/metadata-text-normalize.utils';
type Db = NodePgDatabase<typeof schema>;
type SeriesWriteExecutor = Pick<Db, 'insert'>;
@@ -35,13 +41,11 @@ export class SeriesIdentityService implements OnModuleInit {
}
normalizeName(name: string | null | undefined): string | null {
const trimmed = name?.trim();
return trimmed ? trimmed.toLowerCase() : null;
return normalizeMetadataTextKey(name);
}
normalizeDisplayName(name: string | null | undefined): string | null {
const trimmed = name?.trim();
return trimmed || null;
return normalizeMetadataText(name);
}
async resolveSeriesId(name: string | null | undefined, executor: SeriesWriteExecutor = this.db): Promise<number | null> {
@@ -73,30 +77,40 @@ export class SeriesIdentityService implements OnModuleInit {
async backfillMissingSeriesIds(executor: Pick<Db, 'execute'> = this.db): Promise<void> {
await executor.execute(sql`
WITH normalized AS (
SELECT ${normalizeMetadataTextOrNullSql(bookMetadata.seriesName)} AS display_name
FROM ${bookMetadata}
WHERE ${bookMetadata.seriesName} IS NOT NULL
)
INSERT INTO book_series (name, normalized_name)
SELECT min(btrim(${bookMetadata.seriesName})), lower(btrim(${bookMetadata.seriesName}))
FROM ${bookMetadata}
WHERE ${bookMetadata.seriesName} IS NOT NULL
AND btrim(${bookMetadata.seriesName}) != ''
GROUP BY lower(btrim(${bookMetadata.seriesName}))
SELECT min(display_name), ${normalizeMetadataTextKeySql(sql`display_name`)}
FROM normalized
WHERE display_name IS NOT NULL
GROUP BY ${normalizeMetadataTextKeySql(sql`display_name`)}
ON CONFLICT (normalized_name)
DO NOTHING
`);
await executor.execute(sql`
WITH normalized AS (
SELECT
${bookMetadata.bookId} AS book_id,
${normalizeMetadataTextOrNullSql(bookMetadata.seriesName)} AS display_name
FROM ${bookMetadata}
)
UPDATE ${bookMetadata}
SET series_id = ${bookSeries.id}
FROM ${bookSeries}
WHERE ${bookMetadata.seriesName} IS NOT NULL
AND btrim(${bookMetadata.seriesName}) != ''
AND lower(btrim(${bookMetadata.seriesName})) = ${bookSeries.normalizedName}
FROM normalized, ${bookSeries}
WHERE ${bookMetadata.bookId} = normalized.book_id
AND normalized.display_name IS NOT NULL
AND ${normalizeMetadataTextKeySql(sql`normalized.display_name`)} = ${bookSeries.normalizedName}
AND (${bookMetadata.seriesId} IS NULL OR ${bookMetadata.seriesId} != ${bookSeries.id})
`);
await executor.execute(sql`
UPDATE ${bookMetadata}
SET series_id = NULL
WHERE (${bookMetadata.seriesName} IS NULL OR btrim(${bookMetadata.seriesName}) = '')
WHERE (${bookMetadata.seriesName} IS NULL OR ${normalizeMetadataTextOrNullSql(bookMetadata.seriesName)} IS NULL)
AND ${bookMetadata.seriesId} IS NOT NULL
`);
}
server/src/common/services/series-membership.service.test.ts
+7 -11
View File
@@ -1,17 +1,12 @@
import { SeriesMembershipService } from './series-membership.service';
import { normalizeMetadataText, normalizeMetadataTextKey } from '../utils/metadata-text-normalize.utils';
function makeSeriesIdentity(ids: Record<string, number>) {
return {
normalizeDisplayName: vi.fn((name: string | null | undefined) => {
const trimmed = name?.trim();
return trimmed || null;
}),
normalizeName: vi.fn((name: string | null | undefined) => {
const trimmed = name?.trim();
return trimmed ? trimmed.toLowerCase() : null;
}),
normalizeDisplayName: vi.fn((name: string | null | undefined) => normalizeMetadataText(name)),
normalizeName: vi.fn((name: string | null | undefined) => normalizeMetadataTextKey(name)),
resolveSeriesId: vi.fn((name: string | null | undefined) => {
const key = name?.trim().toLowerCase() ?? '';
const key = normalizeMetadataTextKey(name) ?? '';
return Promise.resolve(ids[key] ?? null);
}),
};
@@ -59,6 +54,7 @@ describe('SeriesMembershipService', () => {
[
{ seriesName: ' Dune ', seriesIndex: 1 },
{ seriesName: 'dune', seriesIndex: 2 },
{ seriesName: 'The Expanse', seriesIndex: 5 },
{ seriesName: 'The Expanse', seriesIndex: null },
{ seriesName: ' ', seriesIndex: 3 },
],
@@ -67,13 +63,13 @@ describe('SeriesMembershipService', () => {
expect(result).toEqual([
{ seriesId: 101, seriesName: 'Dune', seriesIndex: 1, displayOrder: 0 },
{ seriesId: 202, seriesName: 'The Expanse', seriesIndex: null, displayOrder: 1 },
{ seriesId: 202, seriesName: 'The Expanse', seriesIndex: 5, displayOrder: 1 },
]);
expect(deleteFrom).toHaveBeenCalledTimes(1);
expect(identity.resolveSeriesId).toHaveBeenCalledTimes(2);
expect(insertValues).toHaveBeenCalledWith([
{ bookId: 10, seriesId: 101, seriesIndex: 1, displayOrder: 0 },
{ bookId: 10, seriesId: 202, seriesIndex: null, displayOrder: 1 },
{ bookId: 10, seriesId: 202, seriesIndex: 5, displayOrder: 1 },
]);
expect(updateSet).toHaveBeenNthCalledWith(1, {
seriesId: 101,
server/src/common/utils/metadata-text-normalize.utils.test.ts
+62
View File
@@ -0,0 +1,62 @@
import { describe, expect, it } from 'vitest';
import { sql } from 'drizzle-orm';
import {
chooseCanonicalMetadataTextRow,
normalizeMetadataText,
normalizeMetadataTextKey,
normalizeMetadataTextKeySql,
normalizeMetadataTextOrNullSql,
} from './metadata-text-normalize.utils';
function flattenSql(value: unknown): string {
if (typeof value === 'string') return value;
if (Array.isArray(value)) return value.map(flattenSql).join(' ');
if (!value || typeof value !== 'object') return '';
const record = value as { queryChunks?: unknown[]; value?: unknown };
return [flattenSql(record.value), flattenSql(record.queryChunks)].join(' ');
}
describe('metadata text normalization', () => {
it('collapses internal whitespace and trims edges', () => {
expect(normalizeMetadataText(' James\t\n Islington\u00a0 ')).toBe('James Islington');
});
it('returns null for blank values', () => {
expect(normalizeMetadataText(' \t\n ')).toBeNull();
expect(normalizeMetadataText(null)).toBeNull();
expect(normalizeMetadataText(undefined)).toBeNull();
});
it('builds case-insensitive identity keys from normalized text', () => {
expect(normalizeMetadataTextKey(' Dune Chronicles ')).toBe('dune chronicles');
expect(normalizeMetadataTextKey(' ')).toBeNull();
});
it('builds SQL normalization with matching trim and NBSP handling', () => {
const keySql = flattenSql(normalizeMetadataTextKeySql(sql.raw('authors.name'))).replace(/\s+/g, ' ');
const nullableSql = flattenSql(normalizeMetadataTextOrNullSql(sql.raw('book_metadata.series_name'))).replace(/\s+/g, ' ');
expect(keySql).toContain('lower( btrim(regexp_replace(replace( authors.name , chr(160),');
expect(nullableSql).toContain('NULLIF( btrim(regexp_replace(replace( book_metadata.series_name , chr(160),');
});
it('chooses the desired clean row before legacy whitespace variants', () => {
const rows = [
{ id: 2, name: 'Known Author' },
{ id: 3, name: 'Known Author' },
];
expect(chooseCanonicalMetadataTextRow(rows, { desiredName: 'Known Author' })).toEqual({ id: 3, name: 'Known Author' });
});
it('excludes a current row when choosing a canonical target', () => {
const rows = [
{ id: 1, name: 'Known Author' },
{ id: 3, name: 'Known Author' },
];
expect(chooseCanonicalMetadataTextRow(rows, { desiredName: 'Known Author', excludedId: 1 })).toEqual({ id: 3, name: 'Known Author' });
});
});
server/src/common/utils/metadata-text-normalize.utils.ts
+36
View File
@@ -0,0 +1,36 @@
import { sql, type SQLWrapper, type SQL } from 'drizzle-orm';
export function normalizeMetadataText(value: string | null | undefined): string | null {
const normalized = value?.replace(/\s+/g, ' ').trim();
return normalized ? normalized : null;
}
export function normalizeMetadataTextKey(value: string | null | undefined): string | null {
return normalizeMetadataText(value)?.toLowerCase() ?? null;
}
export function normalizeMetadataTextSql(value: SQLWrapper): SQL<string> {
return sql<string>`btrim(regexp_replace(replace(${value}, chr(160), ' '), '[[:space:]]+', ' ', 'g'))`;
}
export function normalizeMetadataTextOrNullSql(value: SQLWrapper): SQL<string | null> {
return sql<string | null>`NULLIF(${normalizeMetadataTextSql(value)}, '')`;
}
export function normalizeMetadataTextKeySql(value: SQLWrapper): SQL<string> {
return sql<string>`lower(${normalizeMetadataTextSql(value)})`;
}
export function chooseCanonicalMetadataTextRow<T extends { id: number; name: string }>(
rows: readonly T[],
options: { desiredName?: string | null; excludedId?: number } = {},
): T | null {
const desiredName = normalizeMetadataText(options.desiredName);
const candidates = rows.filter((row) => row.id !== options.excludedId).sort((a, b) => a.id - b.id);
return (
(desiredName ? candidates.find((row) => row.name === desiredName) : undefined) ??
candidates.find((row) => normalizeMetadataText(row.name) === row.name) ??
candidates[0] ??
null
);
}
server/src/modules/authors/authors.service.test.ts
+5 -5
View File
@@ -146,7 +146,7 @@ describe('AuthorsService', () => {
expect(result.affectedBookCount).toBe(8);
});
it('update trims values and normalizes blank optional fields to null', async () => {
it('update normalizes author names and blank optional fields', async () => {
authorsRepo.findVisibleAuthorIds.mockResolvedValue([20]);
authorsRepo.findRelatedLibraryIds.mockResolvedValue([1]);
authorsRepo.updateAuthorById.mockResolvedValue({ id: 20, name: 'Updated', sortName: null, description: 'Bio' });
@@ -160,14 +160,14 @@ describe('AuthorsService', () => {
});
await service.update(reqUser(), 20, {
name: ' Updated ',
sortName: ' ',
name: ' Updated Author ',
sortName: ' Author,\tUpdated ',
description: ' Bio ',
});
expect(authorsRepo.updateAuthorById).toHaveBeenCalledWith(20, {
name: 'Updated',
sortName: null,
name: 'Updated Author',
sortName: 'Author, Updated',
description: 'Bio',
});
expect(enrichmentOrchestrator.schedule).toHaveBeenCalledWith(20, AUTHOR_ENRICHMENT_REASONS.AUTHOR_RENAME);
server/src/modules/authors/authors.service.ts
+4 -3
View File
@@ -14,6 +14,7 @@ import type {
import { assembleBookCards } from '../book/utils/assemble-book-cards';
import { MAX_OFFSET_ROWS, isOffsetWithinLimit } from '../../common/constants/pagination.constants';
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
import { normalizeMetadataText } from '../../common/utils/metadata-text-normalize.utils';
import type { RequestUser } from '../../common/types/request-user';
import { books } from '../../db/schema';
import { BookReadService } from '../book/book-read.service';
@@ -206,13 +207,13 @@ export class AuthorsService {
const values: Parameters<AuthorsRepository['updateAuthorById']>[1] = {};
if ('name' in dto) {
const name = dto.name?.trim();
const name = normalizeMetadataText(dto.name);
if (!name) throw new BadRequestException('name cannot be empty');
values.name = name;
}
if ('sortName' in dto) {
values.sortName = dto.sortName?.trim() || null;
values.sortName = normalizeMetadataText(dto.sortName);
}
if ('description' in dto) {
@@ -480,7 +481,7 @@ export class AuthorsService {
try {
onProgress?.({ authorId, updated: didUpdate, imageUpdated, imageUrl, error: errorMessage });
} catch {
// callback threw (e.g. client disconnected) — stop the loop
// callback threw (e.g. client disconnected) - stop the loop
callbackInterrupted = true;
break;
}
server/src/modules/book/book.service.test.ts
+50 -7
View File
@@ -1467,6 +1467,25 @@ describe('BookService', () => {
);
});
it('updateMetadata normalizes publisher and series text before persistence', async () => {
const { service, bookRepo } = makeService();
const user = makeUser();
vi.spyOn(service, 'verifyBookAccess').mockResolvedValue(undefined);
vi.spyOn(service, 'getDetail').mockResolvedValue({ id: 5 } as never);
await service.updateMetadata(5, { publisher: ' Ace\t Books ', seriesName: ' Dune Chronicles ' }, user);
expect(bookRepo.updateMetadataFields).toHaveBeenCalledWith(
5,
expect.objectContaining({
publisher: 'Ace Books',
seriesName: 'Dune Chronicles',
updatedAt: expect.any(Date),
}),
expect.anything(),
);
});
it('updateMetadata with empty communityRatings clears all existing ratings', async () => {
const { service, bookRepo } = makeService();
const user = makeUser();
@@ -2171,7 +2190,7 @@ describe('BookService', () => {
});
});
describe('saveProgress — positionSeconds', () => {
describe('saveProgress - positionSeconds', () => {
it('passes positionSeconds from DTO to repo', async () => {
const { service, bookRepo, libraryService } = makeService();
const user = makeUser();
@@ -3073,6 +3092,26 @@ describe('BookService', () => {
expect(scoreService.calculateAndSave).toHaveBeenNthCalledWith(2, 9);
});
it('bulkSetMetadata normalizes publisher and series text fields', async () => {
const { service, bookRepo } = makeService();
const user = makeUser({ id: 11 });
vi.spyOn(service, 'verifyLibraryAccessForBookIds').mockResolvedValue(undefined);
await service.bulkSetMetadata([7], 'publisher', ' Tor\t Books ', user);
await service.bulkSetMetadata([7], 'seriesName', ' Stormlight Archive ', user);
expect(bookRepo.bulkUpdateMetadataFields).toHaveBeenNthCalledWith(
1,
[7],
expect.objectContaining({ publisher: 'Tor Books', updatedAt: expect.any(Date) }),
);
expect(bookRepo.bulkUpdateMetadataFields).toHaveBeenNthCalledWith(
2,
[7],
expect.objectContaining({ seriesName: 'Stormlight Archive', updatedAt: expect.any(Date) }),
);
});
it('bulkSetMetadata skips books where the field is metadata-locked', async () => {
const { service, bookRepo, fileWriteService, fileRenameService, scoreService, bookMetadataLockService } = makeService();
const user = makeUser({ id: 11 });
@@ -3118,7 +3157,7 @@ describe('BookService', () => {
vi.spyOn(service, 'verifyLibraryAccessForBookIds').mockResolvedValue(undefined);
bookMetadataLockService.getBookIdsWithLockedField.mockResolvedValue(new Set([9]));
await service.bulkSetMetadata([7, 9], 'narrators', ['Narrator A'], user);
await service.bulkSetMetadata([7, 9], 'narrators', ['Narrator A', 'narrator a', ' '], user);
expect(narratorService.replaceForBook).toHaveBeenCalledTimes(1);
expect(narratorService.replaceForBook).toHaveBeenCalledWith(7, ['Narrator A'], { executor: tx });
@@ -3178,13 +3217,17 @@ describe('BookService', () => {
const triggerSpy = vi.spyOn(service, 'triggerPostMetadataUpdateEffects' as never).mockReturnValue(undefined as never);
const fields = makeFields({
publisher: { value: 'Bloomsbury' },
publisher: { value: 'Bloomsbury Books' },
language: { value: 'en' },
});
const result = await service.bulkEditMetadata([7, 9], fields, user);
expect(bookRepo.bulkUpdateMetadataFields).toHaveBeenCalled();
expect(bookRepo.bulkUpdateMetadataFields).toHaveBeenCalledWith(
[7, 9],
expect.objectContaining({ publisher: 'Bloomsbury Books', updatedAt: expect.any(Date) }),
tx,
);
expect(result.updatedBooks).toBe(2);
expect(result.fields.publisher).toEqual({ updated: 2, skippedLocked: 0 });
expect(result.fields.language).toEqual({ updated: 2, skippedLocked: 0 });
@@ -3217,7 +3260,7 @@ describe('BookService', () => {
bookRepo.withTransaction.mockImplementation(async (cb: (value: unknown) => Promise<unknown>) => cb(tx));
const fields = makeFields({
authors: { mode: 'replace', values: ['Author A', 'Author B'] },
authors: { mode: 'replace', values: ['Author A', 'author a', 'Author B'] },
});
await service.bulkEditMetadata([7, 9], fields, user);
@@ -3266,7 +3309,7 @@ describe('BookService', () => {
bookMetadataLockService.getLockedFieldsMap.mockResolvedValue(new Map());
const tx = {};
bookRepo.withTransaction.mockImplementation(async (cb: (value: unknown) => Promise<unknown>) => cb(tx));
bookRepo.findAuthorsByBookIds.mockResolvedValue(new Map([[7, ['Author A', 'Author B']]]));
bookRepo.findAuthorsByBookIds.mockResolvedValue(new Map([[7, ['Author A', 'Author B']]]));
const fields = makeFields({
authors: { mode: 'remove', values: ['Author A'] },
@@ -3340,7 +3383,7 @@ describe('BookService', () => {
bookRepo.withTransaction.mockImplementation(async (cb: (value: unknown) => Promise<unknown>) => cb(tx));
const fields = makeFields({
narrators: { mode: 'replace', values: ['Stephen Fry'] },
narrators: { mode: 'replace', values: ['Stephen Fry', 'stephen fry'] },
});
await service.bulkEditMetadata([7], fields, user);
server/src/modules/book/book.service.ts
+47 -14
View File
@@ -16,6 +16,7 @@ import { inArray, type SQL } from 'drizzle-orm';
import { bookCoverDirPath, bookThumbnailPath, findPreferredBookCoverFileName } from '../../common/book-cover-storage';
import { MAX_BOOK_QUERY_OFFSET_ROWS, isBookQueryOffsetWithinLimit } from '../../common/constants/pagination.constants';
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
import { normalizeMetadataText, normalizeMetadataTextKey } from '../../common/utils/metadata-text-normalize.utils';
import { formatSeriesIndex } from '../../common/utils/series-index-format.utils';
import { SeriesMembershipService } from '../../common/services/series-membership.service';
import { isDateKey, resolveTimeZone, toDateKeyInTimeZone, toTimeZoneStartOfDay } from '../../common/utils/timezone.utils';
@@ -1508,12 +1509,12 @@ export class BookService {
if (dto.title !== undefined) scalarFields.title = dto.title ?? null;
if (dto.subtitle !== undefined) scalarFields.subtitle = dto.subtitle ?? null;
if (dto.description !== undefined) scalarFields.description = dto.description ?? null;
if (dto.publisher !== undefined) scalarFields.publisher = dto.publisher ?? null;
if (dto.publisher !== undefined) scalarFields.publisher = normalizeMetadataText(dto.publisher);
if (dto.publishedYear !== undefined) scalarFields.publishedYear = dto.publishedYear ?? null;
if (dto.language !== undefined) scalarFields.language = dto.language ?? null;
if (dto.pageCount !== undefined) scalarFields.pageCount = dto.pageCount ?? null;
if (dto.seriesMemberships === undefined) {
if (dto.seriesName !== undefined) scalarFields.seriesName = dto.seriesName ?? null;
if (dto.seriesName !== undefined) scalarFields.seriesName = normalizeMetadataText(dto.seriesName);
if (dto.seriesIndex !== undefined) scalarFields.seriesIndex = dto.seriesIndex ?? null;
}
if (dto.isbn10 !== undefined) scalarFields.isbn10 = dto.isbn10 ?? null;
@@ -2059,11 +2060,9 @@ export class BookService {
const lockedIds = await this.bookMetadataLockService.getBookIdsWithLockedField(bookIds, lockField);
const updatableIds = bookIds.filter((bookId) => !lockedIds.has(bookId));
if (updatableIds.length > 0) {
const normalizeListValue = (raw: string | number | string[] | null): string[] => {
if (Array.isArray(raw)) return [...new Set(raw.map((entry) => entry.trim()).filter((entry) => entry.length > 0))];
if (raw === null) return [];
const normalized = String(raw).trim();
return normalized.length > 0 ? [normalized] : [];
const normalizeListValue = (raw: string | number | string[] | null, collapseWhitespace = false): string[] => {
const values = Array.isArray(raw) ? raw.map(String) : raw === null ? [] : [String(raw)];
return collapseWhitespace ? this.normalizePersonNameListValues(values) : this.normalizeListValues(values);
};
if (field === 'seriesName' || field === 'publisher' || field === 'language' || field === 'publishedYear') {
if (field === 'publishedYear') {
@@ -2073,11 +2072,18 @@ export class BookService {
}
await this.bookRepo.bulkUpdateMetadataFields(updatableIds, { publishedYear: year, updatedAt: new Date() });
} else {
const textValue = value === null ? null : String(value).trim();
const textValue =
field === 'language'
? value === null
? null
: String(value).trim() || null
: value === null
? null
: normalizeMetadataText(String(value));
await this.bookRepo.bulkUpdateMetadataFields(updatableIds, { [field]: textValue?.length ? textValue : null, updatedAt: new Date() });
}
} else {
const names = normalizeListValue(value);
const names = normalizeListValue(value, field === 'authors' || field === 'narrators');
await this.bookRepo.withTransaction(async (tx) => {
for (const bookId of updatableIds) {
if (field === 'authors') {
@@ -2193,7 +2199,7 @@ export class BookService {
recordResult(fieldName, ids);
if (ids.length === 0) continue;
const val = fields[fieldName].value;
const textValue = val === null ? null : String(val).trim() || null;
const textValue = fieldName === 'language' ? (val === null ? null : String(val).trim() || null) : normalizeMetadataText(val);
await this.bookRepo.bulkUpdateMetadataFields(ids, { [fieldName]: textValue, updatedAt: new Date() }, tx);
}
@@ -2221,7 +2227,7 @@ export class BookService {
const ids = getUpdatableIds('authors');
recordResult('authors', ids);
if (ids.length > 0) {
const names = this.normalizeListValues(fields.authors.values);
const names = this.normalizePersonNameListValues(fields.authors.values);
if (fields.authors.mode === 'replace') {
for (const bookId of ids) {
await this.metadataService.replaceAuthors(
@@ -2234,7 +2240,10 @@ export class BookService {
const currentMap = await this.bookRepo.findAuthorsByBookIds(ids, tx);
for (const bookId of ids) {
const current = currentMap.get(bookId) ?? [];
const merged = fields.authors!.mode === 'add' ? [...new Set([...current, ...names])] : current.filter((n) => !names.includes(n));
const merged =
fields.authors!.mode === 'add'
? this.normalizePersonNameListValues([...current, ...names])
: this.removePersonNameValues(current, names);
await this.metadataService.replaceAuthors(
bookId,
merged.map((name) => ({ name, sortName: null })),
@@ -2289,7 +2298,7 @@ export class BookService {
const ids = getUpdatableIds('narrators');
recordResult('narrators', ids);
if (ids.length > 0) {
const names = this.normalizeListValues(fields.narrators.values);
const names = this.normalizePersonNameListValues(fields.narrators.values);
if (fields.narrators.mode === 'replace') {
for (const bookId of ids) {
await this.narratorService.replaceForBook(bookId, names, { executor: tx });
@@ -2298,7 +2307,10 @@ export class BookService {
const currentMap = await this.bookRepo.findNarratorsByBookIds(ids, tx);
for (const bookId of ids) {
const current = currentMap.get(bookId) ?? [];
const merged = fields.narrators!.mode === 'add' ? [...new Set([...current, ...names])] : current.filter((n) => !names.includes(n));
const merged =
fields.narrators!.mode === 'add'
? this.normalizePersonNameListValues([...current, ...names])
: this.removePersonNameValues(current, names);
await this.narratorService.replaceForBook(bookId, merged, { executor: tx });
}
}
@@ -2335,6 +2347,27 @@ export class BookService {
return [...new Set(values.map((v) => v.trim()).filter((v) => v.length > 0))];
}
private normalizePersonNameListValues(values: string[]): string[] {
const names: string[] = [];
const seen = new Set<string>();
for (const value of values) {
const name = normalizeMetadataText(value);
const key = normalizeMetadataTextKey(name);
if (!name || !key || seen.has(key)) continue;
seen.add(key);
names.push(name);
}
return names;
}
private removePersonNameValues(current: string[], namesToRemove: string[]): string[] {
const removeKeys = new Set(namesToRemove.map((name) => normalizeMetadataTextKey(name)).filter((key): key is string => key !== null));
return this.normalizePersonNameListValues(current).filter((name) => {
const key = normalizeMetadataTextKey(name);
return key ? !removeKeys.has(key) : false;
});
}
async getKoboState(id: number, user: RequestUser): Promise<BookKoboState> {
await this.verifyBookAccess(id, user);
server/src/modules/entity-manager/strategies/author.strategy.test.ts
+109 -13
View File
@@ -393,9 +393,57 @@ describe('AuthorStrategy', () => {
await expect(strategy.rename({ entityId: 1, newName: '\t\n', userId: 1, libraryIds: [] })).rejects.toBeInstanceOf(BadRequestException);
});
it('triggers implicit merge when another author with the same trimmed name exists', async () => {
const existingAuthorLimit = vi.fn().mockResolvedValue([{ id: 99 }]);
const existingAuthorWhere = vi.fn().mockReturnValue({ limit: existingAuthorLimit });
it('triggers implicit merge when another author with the same normalized name exists', async () => {
const existingAuthorWhere = vi.fn().mockResolvedValue([{ id: 99, name: 'New Name' }]);
const existingAuthorFrom = vi.fn().mockReturnValue({ where: existingAuthorWhere });
const entityLimit = vi.fn().mockResolvedValue([{ name: 'Old Name' }]);
const entityWhere = vi.fn().mockReturnValue({ limit: entityLimit });
const entityFrom = vi.fn().mockReturnValue({ where: entityWhere });
const sourceForMergeLimit = vi.fn().mockResolvedValue([{ id: 99, sortName: null, description: null, hasPhoto: false }]);
const sourceForMergeWhere = vi.fn().mockReturnValue({ limit: sourceForMergeLimit });
const sourceForMergeFrom = vi.fn().mockReturnValue({ where: sourceForMergeWhere });
const sourceFieldsLimit = vi.fn().mockResolvedValue([{ sortName: null, description: null }]);
const sourceFieldsWhere = vi.fn().mockReturnValue({ limit: sourceFieldsLimit });
const sourceFieldsFrom = vi.fn().mockReturnValue({ where: sourceFieldsWhere });
const affectedWhere = vi.fn().mockResolvedValue([{ bookId: 5 }]);
const affectedFrom = vi.fn().mockReturnValue({ where: affectedWhere });
const selectDistinct = vi.fn().mockReturnValue({ from: affectedFrom });
let callIdx = 0;
const select = vi.fn().mockImplementation(() => {
callIdx++;
if (callIdx === 1) return { from: entityFrom };
if (callIdx === 2) return { from: existingAuthorFrom };
if (callIdx === 3) return { from: sourceForMergeFrom };
return { from: sourceFieldsFrom };
});
const updateWhere = vi.fn().mockResolvedValue(undefined);
const updateSet = vi.fn().mockReturnValue({ where: updateWhere });
const update = vi.fn().mockReturnValue({ set: updateSet });
const authorsRepo = { updateAuthorById: vi.fn().mockResolvedValue(undefined), mergeAuthors: vi.fn().mockResolvedValue(undefined) };
const authorImageStorage = { promoteImage: vi.fn().mockResolvedValue(false), deleteAuthorDir: vi.fn().mockResolvedValue(undefined) };
const enrichmentOrchestrator = { schedule: vi.fn().mockResolvedValue(undefined) };
const strategy = makeStrategy({ select, selectDistinct, update }, { authorsRepo, authorImageStorage, enrichmentOrchestrator });
const result = await strategy.rename({ entityId: 1, newName: 'New Name', userId: 5, libraryIds: [] });
expect(result.wasImplicitMerge).toBe(true);
expect(result.mergedEntityId).toBe(99);
expect(result.oldName).toBe('Old Name');
});
it('chooses a non-current merge target when normalized lookup returns the current author first', async () => {
const existingAuthorWhere = vi.fn().mockResolvedValue([
{ id: 1, name: 'New Name' },
{ id: 99, name: 'New Name' },
]);
const existingAuthorFrom = vi.fn().mockReturnValue({ where: existingAuthorWhere });
const entityLimit = vi.fn().mockResolvedValue([{ name: 'Old Name' }]);
@@ -435,9 +483,9 @@ describe('AuthorStrategy', () => {
const result = await strategy.rename({ entityId: 1, newName: 'New Name', userId: 5, libraryIds: [] });
expect(authorsRepo.updateAuthorById).not.toHaveBeenCalledWith(1, { name: 'New Name' });
expect(result.wasImplicitMerge).toBe(true);
expect(result.mergedEntityId).toBe(99);
expect(result.oldName).toBe('Old Name');
});
it('updates author name and returns wasImplicitMerge false when no conflict', async () => {
@@ -445,8 +493,7 @@ describe('AuthorStrategy', () => {
const entityWhere = vi.fn().mockReturnValue({ limit: entityLimit });
const entityFrom = vi.fn().mockReturnValue({ where: entityWhere });
const noExistingLimit = vi.fn().mockResolvedValue([]);
const noExistingWhere = vi.fn().mockReturnValue({ limit: noExistingLimit });
const noExistingWhere = vi.fn().mockResolvedValue([]);
const noExistingFrom = vi.fn().mockReturnValue({ where: noExistingWhere });
const affectedWhere = vi.fn().mockResolvedValue([{ bookId: 7 }]);
@@ -466,7 +513,7 @@ describe('AuthorStrategy', () => {
const strategy = makeStrategy({ select, selectDistinct }, { authorsRepo, authorImageStorage, enrichmentOrchestrator });
const result = await strategy.rename({ entityId: 1, newName: 'New Name', userId: 5, libraryIds: [] });
const result = await strategy.rename({ entityId: 1, newName: ' New Name ', userId: 5, libraryIds: [] });
expect(authorsRepo.updateAuthorById).toHaveBeenCalledWith(1, { name: 'New Name' });
expect(result.oldName).toBe('Old Name');
@@ -573,8 +620,7 @@ describe('AuthorStrategy', () => {
const txInsertValues = vi.fn().mockReturnValue({ onConflictDoNothing: txInsertOnConflict, returning: txInsertReturning });
const txInsert = vi.fn().mockReturnValue({ values: txInsertValues });
const txExistingLimit = vi.fn().mockResolvedValue([]);
const txExistingWhere = vi.fn().mockReturnValue({ limit: txExistingLimit });
const txExistingWhere = vi.fn().mockResolvedValue([]);
const txSelectFrom = vi.fn().mockReturnValue({ where: txExistingWhere });
const txSelect = vi.fn().mockReturnValue({ from: txSelectFrom });
@@ -604,9 +650,11 @@ describe('AuthorStrategy', () => {
const strategy = makeStrategy({ select, selectDistinct, transaction }, { authorsRepo, authorImageStorage });
const result = await strategy.split({ entityId: 1, newNames: ['New A', 'New B'] });
const result = await strategy.split({ entityId: 1, newNames: [' New A ', 'New\tB'] });
expect(transaction).toHaveBeenCalledTimes(1);
expect(txInsertValues).toHaveBeenNthCalledWith(1, { name: 'New A' });
expect(txInsertValues).toHaveBeenNthCalledWith(2, { name: 'New B' });
expect(txDeleteWhere).toHaveBeenCalledTimes(2);
expect(authorImageStorage.deleteAuthorDir).toHaveBeenCalledWith(1);
expect(result.originalName).toBe('Old Author');
@@ -629,8 +677,7 @@ describe('AuthorStrategy', () => {
return { from: affectedFrom };
});
const existingAuthorLimit = vi.fn().mockResolvedValue([{ id: 55 }]);
const existingAuthorWhere = vi.fn().mockReturnValue({ limit: existingAuthorLimit });
const existingAuthorWhere = vi.fn().mockResolvedValue([{ id: 55, name: 'Existing Author' }]);
const existingAuthorFrom = vi.fn().mockReturnValue({ where: existingAuthorWhere });
const bookRowsWhere = vi.fn().mockResolvedValue([]);
@@ -655,11 +702,60 @@ describe('AuthorStrategy', () => {
const strategy = makeStrategy({ select, selectDistinct, transaction }, { authorsRepo, authorImageStorage });
const result = await strategy.split({ entityId: 1, newNames: ['Existing Author'] });
const result = await strategy.split({ entityId: 1, newNames: ['Existing Author'] });
expect(txInsert).not.toHaveBeenCalled();
expect(result.newEntities[0]).toMatchObject({ id: 55, name: 'Existing Author' });
});
it('does not reuse the source author as a normalized split target', async () => {
const entityLimit = vi.fn().mockResolvedValue([{ name: 'Old Author' }]);
const entityWhere = vi.fn().mockReturnValue({ limit: entityLimit });
const entityFrom = vi.fn().mockReturnValue({ where: entityWhere });
const affectedWhere = vi.fn().mockResolvedValue([]);
const affectedFrom = vi.fn().mockReturnValue({ where: affectedWhere });
const selectDistinct = vi.fn().mockReturnValue({ from: affectedFrom });
let outerIdx = 0;
const select = vi.fn().mockImplementation(() => {
outerIdx++;
if (outerIdx === 1) return { from: entityFrom };
return { from: affectedFrom };
});
const existingAuthorWhere = vi.fn().mockResolvedValue([{ id: 1, name: 'Old Author' }]);
const existingAuthorFrom = vi.fn().mockReturnValue({ where: existingAuthorWhere });
const bookRowsWhere = vi.fn().mockResolvedValue([]);
const bookRowsFrom = vi.fn().mockReturnValue({ where: bookRowsWhere });
let txIdx = 0;
const txSelect = vi.fn().mockImplementation(() => {
txIdx++;
if (txIdx === 1) return { from: existingAuthorFrom };
return { from: bookRowsFrom };
});
const txInsertReturning = vi.fn().mockResolvedValue([{ id: 56 }]);
const txInsertValues = vi.fn().mockReturnValue({ returning: txInsertReturning });
const txInsert = vi.fn().mockReturnValue({ values: txInsertValues });
const txDeleteWhere = vi.fn().mockResolvedValue(undefined);
const txDeleteFrom = vi.fn().mockReturnValue({ where: txDeleteWhere });
const tx = { select: txSelect, insert: txInsert, delete: txDeleteFrom };
const transaction = vi.fn().mockImplementation(async (cb: (tx: typeof tx) => Promise<unknown>) => cb(tx));
const authorImageStorage = { deleteAuthorDir: vi.fn().mockResolvedValue(undefined) };
const authorsRepo = { updateAuthorById: vi.fn(), mergeAuthors: vi.fn(), deleteAuthors: vi.fn() };
const strategy = makeStrategy({ select, selectDistinct, transaction }, { authorsRepo, authorImageStorage });
const result = await strategy.split({ entityId: 1, newNames: ['Old Author'] });
expect(txInsertValues).toHaveBeenCalledWith({ name: 'Old Author' });
expect(result.newEntities[0]).toMatchObject({ id: 56, name: 'Old Author' });
});
});
describe('findAffectedBookIds', () => {
server/src/modules/entity-manager/strategies/author.strategy.ts
+43 -12
View File
@@ -7,6 +7,12 @@ import { refreshPrimaryAuthorSortNamesForBooks } from '../../../db/book-author-s
import * as schema from '../../../db/schema';
import { authors, bookAuthors, books, bookMetadata } from '../../../db/schema';
import { buildContentFilterClauses } from '../../../common/utils/content-filter-sql.utils';
import {
chooseCanonicalMetadataTextRow,
normalizeMetadataText,
normalizeMetadataTextKey,
normalizeMetadataTextKeySql,
} from '../../../common/utils/metadata-text-normalize.utils';
import { AuthorImageStorageService } from '../../authors/author-image-storage.service';
import { AuthorsRepository } from '../../authors/authors.repository';
import { AuthorEnrichmentOrchestratorService } from '../../authors/author-enrichment-orchestrator.service';
@@ -28,6 +34,7 @@ import type {
type Db = NodePgDatabase<typeof schema>;
const AUTHOR_ENRICHMENT_REASONS = { AUTHOR_MERGE_TARGET: 'author_merge_target' as const };
const NORMALIZED_AUTHOR_NAME_SQL = normalizeMetadataTextKeySql(authors.name);
function escapeLike(s: string): string {
return s.replace(/[%_\\]/g, '\\$&');
@@ -272,16 +279,21 @@ export class AuthorStrategy implements EntityStrategy {
if (!entity) throw new NotFoundException('Author not found');
const oldName = entity.name;
const trimmed = input.newName.trim();
if (!trimmed) throw new BadRequestException('Name cannot be empty');
const displayName = normalizeMetadataText(input.newName);
const normalizedName = normalizeMetadataTextKey(displayName);
if (!displayName || !normalizedName) throw new BadRequestException('Name cannot be empty');
const [existing] = await this.db.select({ id: authors.id }).from(authors).where(eq(authors.name, trimmed)).limit(1);
if (existing && existing.id !== entityId) {
const mergeResult = await this.merge({ targetId: existing.id, sourceIds: [entityId], userId: input.userId });
return { oldName, affectedBookIds: mergeResult.affectedBookIds, wasImplicitMerge: true, mergedEntityId: existing.id };
const existingRows = await this.db
.select({ id: authors.id, name: authors.name })
.from(authors)
.where(eq(NORMALIZED_AUTHOR_NAME_SQL, normalizedName));
const mergeTarget = this.selectPreferredAuthorMatch(existingRows, entityId, displayName);
if (mergeTarget) {
const mergeResult = await this.merge({ targetId: mergeTarget.id, sourceIds: [entityId], userId: input.userId });
return { oldName, affectedBookIds: mergeResult.affectedBookIds, wasImplicitMerge: true, mergedEntityId: mergeTarget.id };
}
await this.authorsRepo.updateAuthorById(entityId, { name: trimmed });
await this.authorsRepo.updateAuthorById(entityId, { name: displayName });
const affectedBookIds = await this.findAffectedBookIds([entityId]);
return { oldName, affectedBookIds, wasImplicitMerge: false };
}
@@ -312,16 +324,27 @@ export class AuthorStrategy implements EntityStrategy {
const newEntities: { id: number; name: string }[] = [];
await this.db.transaction(async (tx) => {
const seenNewNames = new Set<string>();
for (const name of input.newNames) {
const trimmed = name.trim();
const [existing] = await tx.select({ id: authors.id }).from(authors).where(eq(authors.name, trimmed)).limit(1);
const displayName = normalizeMetadataText(name);
const normalizedName = normalizeMetadataTextKey(displayName);
if (!displayName || !normalizedName) continue;
if (seenNewNames.has(normalizedName)) continue;
seenNewNames.add(normalizedName);
const existingRows = await tx
.select({ id: authors.id, name: authors.name })
.from(authors)
.where(eq(NORMALIZED_AUTHOR_NAME_SQL, normalizedName));
const existing = this.selectPreferredAuthorMatch(existingRows, input.entityId as number, displayName);
if (existing) {
newEntities.push({ id: existing.id, name: trimmed });
newEntities.push({ id: existing.id, name: existing.name });
} else {
const [inserted] = await tx.insert(authors).values({ name: trimmed }).returning({ id: authors.id });
newEntities.push({ id: inserted!.id, name: trimmed });
const [inserted] = await tx.insert(authors).values({ name: displayName }).returning({ id: authors.id });
newEntities.push({ id: inserted!.id, name: displayName });
}
}
if (newEntities.length === 0) throw new BadRequestException('At least one name is required');
const bookRows = await tx
.select({ bookId: bookAuthors.bookId, displayOrder: bookAuthors.displayOrder })
@@ -347,6 +370,14 @@ export class AuthorStrategy implements EntityStrategy {
return { originalName: entity.name, newEntities, affectedBookIds };
}
private selectPreferredAuthorMatch(
rows: { id: number; name: string }[],
excludedAuthorId: number,
displayName: string,
): { id: number; name: string } | null {
return chooseCanonicalMetadataTextRow(rows, { desiredName: displayName, excludedId: excludedAuthorId });
}
async findAffectedBookIds(ids: (number | string)[]): Promise<number[]> {
const numericIds = ids as number[];
if (numericIds.length === 0) return [];
server/src/modules/entity-manager/strategies/inline-entity.strategy.test.ts
+1 -1
View File
@@ -151,7 +151,7 @@ describe('InlineEntityStrategy', () => {
it('sets wasImplicitMerge=false when renaming to same value (same identity)', async () => {
const execute = vi
.fn()
.mockResolvedValueOnce({ rows: [{ cnt: 1 }] })
.mockResolvedValueOnce({ rows: [{ cnt: 0 }] })
.mockResolvedValueOnce({ rows: [{ bookId: 7 }] });
const strategy = makeStrategy({ execute });
server/src/modules/entity-manager/strategies/inline-entity.strategy.ts
+14 -4
View File
@@ -1,5 +1,5 @@
import { BadRequestException, Inject } from '@nestjs/common';
import { asc, count, eq, sql } from 'drizzle-orm';
import { SQL, asc, count, eq, sql } from 'drizzle-orm';
import type { NodePgDatabase } from 'drizzle-orm/node-postgres';
import type { InlineEntityType } from '@bookorbit/types';
@@ -40,6 +40,15 @@ export abstract class InlineEntityStrategy implements EntityStrategy {
return (bookMetadata as any)[this.fieldName];
}
protected normalizeInputValue(value: string): string | null {
const trimmed = value.trim();
return trimmed || null;
}
protected buildIdentityEqualsCondition(alias: string, value: string): SQL {
return sql`${sql.raw(`${alias}.${this.rawFieldName}`)} = ${value}`;
}
async findCandidatePairs(libraryIds: number[], minSimilarity: number): Promise<RawCandidatePair[]> {
const similarityThreshold = Math.max(0.1, Math.min(1, minSimilarity));
const f = sql.raw(this.rawFieldName);
@@ -148,7 +157,7 @@ export abstract class InlineEntityStrategy implements EntityStrategy {
async rename(input: RenameInput): Promise<StrategyRenameResult> {
const currentValue = input.entityId as string;
const trimmed = input.newName.trim();
const trimmed = this.normalizeInputValue(input.newName);
if (!trimmed) throw new BadRequestException('Name cannot be empty');
const idsLiteral = sql.raw(`(${input.libraryIds.join(',')})`);
@@ -158,9 +167,10 @@ export abstract class InlineEntityStrategy implements EntityStrategy {
FROM book_metadata bm
JOIN books b ON b.id = bm.book_id
WHERE b.library_id IN ${idsLiteral}
AND bm.${sql.raw(this.rawFieldName)} = ${trimmed}
AND bm.${sql.raw(this.rawFieldName)} <> ${currentValue}
AND ${this.buildIdentityEqualsCondition('bm', trimmed)}
`);
const wasImplicitMerge = (existingCount.rows[0]?.cnt ?? 0) > 0 && currentValue !== trimmed;
const wasImplicitMerge = (existingCount.rows[0]?.cnt ?? 0) > 0;
const affectedRows = await this.db.execute<{ bookId: number }>(sql`
UPDATE book_metadata bm SET ${sql.raw(this.rawFieldName)} = ${trimmed}
server/src/modules/entity-manager/strategies/publisher.strategy.test.ts
+112
View File
@@ -0,0 +1,112 @@
import { BadRequestException } from '@nestjs/common';
import { PublisherStrategy } from './publisher.strategy';
class TestablePublisherStrategy extends PublisherStrategy {
exposeIdentityCondition(alias: string, value: string) {
return this.buildIdentityEqualsCondition(alias, value);
}
}
function makeStrategy(db: Record<string, unknown> = {}) {
return new PublisherStrategy(db as never);
}
function flattenSql(value: unknown): string {
if (typeof value === 'string') return value;
if (Array.isArray(value)) return value.map(flattenSql).join(' ');
if (!value || typeof value !== 'object') return '';
const record = value as { queryChunks?: unknown[]; value?: unknown };
return [flattenSql(record.value), flattenSql(record.queryChunks)].join(' ');
}
function extractSqlParams(value: unknown): unknown[] {
if (typeof value === 'number' || typeof value === 'string' || value === null || typeof value === 'boolean') return [value];
if (!value || typeof value !== 'object') return [];
const record = value as { queryChunks?: unknown[]; value?: unknown };
return [...(Array.isArray(record.value) ? record.value.flatMap(extractSqlParams) : []), ...(record.queryChunks?.flatMap(extractSqlParams) ?? [])];
}
describe('PublisherStrategy', () => {
describe('rename', () => {
it('normalizes publisher names before updating metadata', async () => {
const execute = vi
.fn()
.mockResolvedValueOnce({ rows: [{ cnt: 0 }] })
.mockResolvedValueOnce({ rows: [{ bookId: 10 }, { bookId: 11 }] });
const strategy = makeStrategy({ execute });
const result = await strategy.rename({
entityId: 'Orbit Books',
newName: ' Orbit\t\nBooks ',
userId: 1,
libraryIds: [7],
});
expect(result).toEqual({
oldName: 'Orbit Books',
affectedBookIds: [10, 11],
wasImplicitMerge: false,
mergedEntityId: undefined,
});
const updateParams = extractSqlParams(execute.mock.calls[1]![0]);
expect(updateParams).toContain('Orbit Books');
expect(updateParams).not.toContain(' Orbit\t\nBooks ');
});
it('uses normalized identity when detecting implicit publisher merges', async () => {
const execute = vi
.fn()
.mockResolvedValueOnce({ rows: [{ cnt: 3 }] })
.mockResolvedValueOnce({ rows: [{ bookId: 20 }] });
const strategy = makeStrategy({ execute });
const result = await strategy.rename({
entityId: 'Orbit Books',
newName: 'Orbit Books',
userId: 1,
libraryIds: [7],
});
expect(result).toMatchObject({
wasImplicitMerge: true,
mergedEntityId: 'Orbit Books',
});
const countSql = execute.mock.calls[0]![0];
const countSqlText = flattenSql(countSql).replace(/\s+/g, ' ');
expect(countSqlText).toContain('regexp_replace');
expect(countSqlText).toContain('chr(160)');
expect(extractSqlParams(countSql)).toContain('orbit books');
});
it('rejects whitespace-only publisher names after normalization', async () => {
const execute = vi.fn();
const strategy = makeStrategy({ execute });
await expect(
strategy.rename({
entityId: 'Orbit Books',
newName: ' \t\n\u00a0 ',
userId: 1,
libraryIds: [7],
}),
).rejects.toBeInstanceOf(BadRequestException);
expect(execute).not.toHaveBeenCalled();
});
});
describe('buildIdentityEqualsCondition', () => {
it('returns a false condition when the normalized lookup key is empty', () => {
const strategy = new TestablePublisherStrategy({} as never);
const condition = strategy.exposeIdentityCondition('bm', ' \t\n\u00a0 ');
expect(flattenSql(condition)).toContain('false');
});
});
});
server/src/modules/entity-manager/strategies/publisher.strategy.ts
+13
View File
@@ -1,6 +1,8 @@
import { Inject, Injectable } from '@nestjs/common';
import { SQL, sql } from 'drizzle-orm';
import type { NodePgDatabase } from 'drizzle-orm/node-postgres';
import { normalizeMetadataText, normalizeMetadataTextKey, normalizeMetadataTextKeySql } from '../../../common/utils/metadata-text-normalize.utils';
import { DB } from '../../../db';
import * as schema from '../../../db/schema';
import { InlineEntityStrategy } from './inline-entity.strategy';
@@ -16,4 +18,15 @@ export class PublisherStrategy extends InlineEntityStrategy {
constructor(@Inject(DB) db: Db) {
super(db);
}
protected override normalizeInputValue(value: string): string | null {
return normalizeMetadataText(value);
}
protected override buildIdentityEqualsCondition(alias: string, value: string): SQL {
const key = normalizeMetadataTextKey(value);
if (!key) return sql`false`;
const field = sql.raw(`${alias}.${this.rawFieldName}`);
return sql`${normalizeMetadataTextKeySql(field)} = ${key}`;
}
}
server/src/modules/entity-manager/strategies/series.strategy.test.ts
+4 -4
View File
@@ -410,20 +410,20 @@ describe('SeriesStrategy', () => {
});
it('upserts a series by normalized name', async () => {
const returning = vi.fn().mockResolvedValue([{ id: 9, name: 'Dune' }]);
const returning = vi.fn().mockResolvedValue([{ id: 9, name: 'Dune Chronicles' }]);
const onConflictDoUpdate = vi.fn().mockReturnValue({ returning });
const values = vi.fn().mockReturnValue({ onConflictDoUpdate });
const insert = vi.fn().mockReturnValue({ values });
const strategy = makeStrategy({ insert });
await expect(
(strategy as unknown as { upsertSeries(name: string): Promise<{ id: number; name: string }> }).upsertSeries('Dune'),
(strategy as unknown as { upsertSeries(name: string): Promise<{ id: number; name: string }> }).upsertSeries(' Dune Chronicles '),
).resolves.toEqual({
id: 9,
name: 'Dune',
name: 'Dune Chronicles',
});
expect(values).toHaveBeenCalledWith({ name: 'Dune', normalizedName: 'dune' });
expect(values).toHaveBeenCalledWith({ name: 'Dune Chronicles', normalizedName: 'dune chronicles' });
});
it('rejects a series upsert when the database returns no row', async () => {
server/src/modules/entity-manager/strategies/series.strategy.ts
+7 -8
View File
@@ -3,6 +3,7 @@ import { SQL, and, asc, count, desc, eq, exists, ilike, inArray, notExists, or,
import type { NodePgDatabase } from 'drizzle-orm/node-postgres';
import { buildContentFilterClauses } from '../../../common/utils/content-filter-sql.utils';
import { normalizeMetadataText, normalizeMetadataTextKey } from '../../../common/utils/metadata-text-normalize.utils';
import { DB } from '../../../db';
import * as schema from '../../../db/schema';
import { bookMetadata, books, bookSeries, bookSeriesMemberships } from '../../../db/schema';
@@ -26,10 +27,6 @@ function escapeLike(s: string): string {
return s.replace(/[%_\\]/g, '\\$&');
}
function normalizedName(name: string): string {
return name.trim().toLowerCase();
}
function numericIds(ids: (number | string)[]): number[] {
return [...new Set(ids.map((id) => Number(id)).filter((id) => Number.isInteger(id) && id > 0))];
}
@@ -203,7 +200,7 @@ export class SeriesStrategy implements EntityStrategy {
const current = await this.findEntityById(entityId);
if (!current) throw new NotFoundException('series not found');
const newName = input.newName.trim();
const newName = normalizeMetadataText(input.newName);
if (!newName) throw new BadRequestException('Name cannot be empty');
const affectedBookIds = await this.findAffectedBookIdsInLibraries([entityId], input.libraryIds);
@@ -298,13 +295,15 @@ export class SeriesStrategy implements EntityStrategy {
}
private async upsertSeries(name: string): Promise<{ id: number; name: string }> {
const normalized = normalizedName(name);
const displayName = normalizeMetadataText(name);
const normalized = normalizeMetadataTextKey(displayName);
if (!displayName || !normalized) throw new BadRequestException('Name cannot be empty');
const [row] = await this.db
.insert(bookSeries)
.values({ name, normalizedName: normalized })
.values({ name: displayName, normalizedName: normalized })
.onConflictDoUpdate({
target: bookSeries.normalizedName,
set: { name, updatedAt: new Date() },
set: { name: displayName, updatedAt: new Date() },
})
.returning({ id: bookSeries.id, name: bookSeries.name });
if (!row) throw new BadRequestException('Unable to resolve series');
server/src/modules/metadata-fetch/metadata-fetch-pipeline.test.ts
+3 -2
View File
@@ -651,8 +651,9 @@ describe('MetadataFetchPipeline', () => {
seriesName: 'Sword of Truth',
seriesIndex: 11,
seriesMemberships: [
{ seriesName: 'Sword of Truth', seriesIndex: 11 },
{ seriesName: 'Chainfire Trilogy', seriesIndex: 3 },
{ seriesName: ' Sword of Truth ', seriesIndex: 11 },
{ seriesName: 'sword of truth', seriesIndex: 12 },
{ seriesName: 'Chainfire\tTrilogy', seriesIndex: 3 },
],
}),
),
server/src/modules/metadata-fetch/metadata-fetch-pipeline.ts
+4 -3
View File
@@ -18,6 +18,7 @@ import { MetadataPreferenceResolver } from '../metadata-preferences/metadata-pre
import { ProviderConfigService } from '../metadata-preferences/provider-config.service';
import { MetadataPreferencesService } from '../metadata-preferences/metadata-preferences.service';
import { createGenreBlocklistTokenSet, filterGenresAgainstBlocklist } from '../../common/utils/genre-blocklist.utils';
import { normalizeMetadataText, normalizeMetadataTextKey } from '../../common/utils/metadata-text-normalize.utils';
import { MetadataFetchService } from './metadata-fetch.service';
import { ProviderRegistry } from './provider-registry';
import { ProviderThrottleTracker } from './provider-throttle.tracker';
@@ -412,9 +413,9 @@ export class MetadataFetchPipeline {
const normalized: MetadataSeriesMembership[] = [];
const seen = new Set<string>();
for (const membership of memberships) {
const seriesName = membership.seriesName.trim();
const key = seriesName.toLowerCase();
if (!seriesName || seen.has(key)) continue;
const seriesName = normalizeMetadataText(membership.seriesName);
const key = normalizeMetadataTextKey(seriesName);
if (!seriesName || !key || seen.has(key)) continue;
const seriesIndex = typeof membership.seriesIndex === 'number' && Number.isFinite(membership.seriesIndex) ? membership.seriesIndex : null;
seen.add(key);
server/src/modules/metadata/metadata.service.test.ts
+67 -17
View File
@@ -119,7 +119,12 @@ const makeDb = () => {
const deleteBuilder = { where: deleteWhere };
const selectLimit = vi.fn().mockResolvedValue([]);
const selectWhere = vi.fn().mockReturnValue({ limit: selectLimit });
const selectWhereResult = {
limit: selectLimit,
then: (onFulfilled: (value: unknown[]) => unknown, onRejected?: (reason: unknown) => unknown) =>
Promise.resolve([]).then(onFulfilled, onRejected),
};
const selectWhere = vi.fn().mockReturnValue(selectWhereResult);
const selectFrom = vi.fn().mockReturnValue({ where: selectWhere });
const insertReturning = vi.fn().mockResolvedValue([]);
@@ -553,10 +558,10 @@ describe('MetadataService', () => {
description: 'Description',
isbn10: '1234567890',
isbn13: '9781234567897',
publisher: 'Publisher',
publisher: ' Publisher\t Name ',
publishedYear: null,
language: 'en',
seriesName: 'Series',
seriesName: 'Series Name',
seriesIndex: 2,
authors: [{ name: 'Author A', sortName: null }],
genres: ['Fantasy'],
@@ -591,7 +596,9 @@ describe('MetadataService', () => {
publishedYear: 1999,
subtitle: 'Subtitle',
description: 'Description',
publisher: 'Publisher Name',
pageCount: 321,
seriesName: 'Series Name',
rating: 5,
googleBooksId: 'google-1',
goodreadsId: 'goodreads-1',
@@ -676,11 +683,11 @@ describe('MetadataService', () => {
subtitle: 'Audio Subtitle',
authors: [{ name: 'Audio Author', sortName: null }],
narrators: ['Audio Narrator'],
publisher: 'Audio Publisher',
publisher: 'Audio Publisher',
publishedYear: 2024,
description: 'Audio Description',
language: 'eng',
seriesName: 'Audio Series',
seriesName: 'Audio\tSeries',
seriesIndex: 2,
genres: ['Fantasy', 'Adventure'],
audibleId: 'B0AUDIBLE',
@@ -740,7 +747,7 @@ describe('MetadataService', () => {
insertedAuthors.push(...rows);
return {
onConflictDoNothing: () => ({
returning: () => Promise.resolve([{ id: 81, name: 'Alice' }]),
returning: () => Promise.resolve([{ id: 81, name: 'Alice Smith' }]),
}),
};
},
@@ -758,28 +765,28 @@ describe('MetadataService', () => {
});
await service.replaceAuthors(5, [
{ name: ' Alice ', sortName: ' ' },
{ name: 'alice', sortName: 'ignored duplicate' },
{ name: ' Alice\t\n Smith ', sortName: ' Smith,\nAlice ' },
{ name: 'alice smith', sortName: 'ignored duplicate' },
{ name: ' ', sortName: null },
]);
expect(transaction).toHaveBeenCalledTimes(1);
expect(db.delete).toHaveBeenCalledWith(bookAuthors);
expect(deleteWhere).toHaveBeenCalledTimes(1);
expect(db.select).not.toHaveBeenCalled();
expect(insertedAuthors).toEqual([{ name: 'Alice', sortName: null }]);
expect(db.select).toHaveBeenCalledTimes(1);
expect(insertedAuthors).toEqual([{ name: 'Alice Smith', sortName: 'Smith, Alice' }]);
expect(insertedBookAuthors).toEqual([{ bookId: 5, authorId: 81, displayOrder: 0 }]);
});
it('replaceAuthors reuses existing authors and only inserts join rows', async () => {
const { db } = makeDb();
it('replaceAuthors canonicalizes a legacy whitespace variant before linking it', async () => {
const { db, updateSet, updateWhere } = makeDb();
const service = makeService(db);
const insertedAuthors: Array<{ name: string; sortName: string | null }> = [];
const insertedBookAuthors: Array<{ bookId: number; authorId: number; displayOrder: number }> = [];
db.select.mockImplementation(() => ({
from: () => ({
where: () => Promise.resolve([{ id: 9, name: 'Known Author' }]),
where: () => Promise.resolve([{ id: 9, name: 'Known Author', normalizedName: 'known author' }]),
}),
}));
db.insert.mockImplementation((table: unknown) => {
@@ -808,10 +815,55 @@ describe('MetadataService', () => {
await service.replaceAuthors(6, [{ name: 'Known Author', sortName: null }]);
expect(insertedAuthors).toEqual([{ name: 'Known Author', sortName: null }]);
expect(insertedAuthors).toEqual([]);
expect(db.update).toHaveBeenCalledWith(authors);
expect(updateSet).toHaveBeenCalledWith({ name: 'Known Author' });
expect(updateWhere).toHaveBeenCalledTimes(1);
expect(db.execute).toHaveBeenCalledTimes(2);
expect(insertedBookAuthors).toEqual([{ bookId: 6, authorId: 9, displayOrder: 0 }]);
});
it('replaceAuthors prefers an existing clean author row over a legacy whitespace variant', async () => {
const { db, updateSet } = makeDb();
const service = makeService(db);
const insertedBookAuthors: Array<{ bookId: number; authorId: number; displayOrder: number }> = [];
db.select.mockImplementation(() => ({
from: () => ({
where: () =>
Promise.resolve([
{ id: 9, name: 'Known Author', normalizedName: 'known author' },
{ id: 10, name: 'Known Author', normalizedName: 'known author' },
]),
}),
}));
db.insert.mockImplementation((table: unknown) => {
if (table === authors) {
return {
values: () => ({
onConflictDoNothing: () => ({
returning: () => Promise.resolve([]),
}),
}),
};
}
if (table === bookAuthors) {
return {
values: (rows: Array<{ bookId: number; authorId: number; displayOrder: number }>) => {
insertedBookAuthors.push(...rows);
return { onConflictDoNothing: () => Promise.resolve(undefined) };
},
};
}
throw new Error('unexpected table in insert');
});
await service.replaceAuthors(6, [{ name: 'Known Author', sortName: null }]);
expect(updateSet).not.toHaveBeenCalledWith({ name: 'Known Author' });
expect(insertedBookAuthors).toEqual([{ bookId: 6, authorId: 10, displayOrder: 0 }]);
});
it('replaceAuthors emits author replaced event with linked author ids', async () => {
const { db } = makeDb();
const metadataEvents = { emit: vi.fn() };
@@ -819,9 +871,7 @@ describe('MetadataService', () => {
db.select.mockImplementation(() => ({
from: () => ({
where: () => ({
limit: () => Promise.resolve([]),
}),
where: () => Promise.resolve([]),
}),
}));
db.insert.mockImplementation((table: unknown) => {
server/src/modules/metadata/metadata.service.ts
+102 -32
View File
@@ -15,9 +15,15 @@ import {
isExtractedBookCoverFileName,
} from '../../common/book-cover-storage';
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
import {
chooseCanonicalMetadataTextRow,
normalizeMetadataText,
normalizeMetadataTextKey,
normalizeMetadataTextKeySql,
} from '../../common/utils/metadata-text-normalize.utils';
import { SeriesIdentityService } from '../../common/services/series-identity.service';
import { SeriesMembershipService } from '../../common/services/series-membership.service';
import { refreshPrimaryAuthorSortNamesForBooks } from '../../db/book-author-sort-key';
import { refreshPrimaryAuthorSortNamesForAuthors, refreshPrimaryAuthorSortNamesForBooks } from '../../db/book-author-sort-key';
import { BookEmbedderService } from '../embedding/book-embedder.service';
import { BookMetadataLockService } from '../book-metadata-lock/book-metadata-lock.service';
import { ComicMetadataRepository } from './comic-metadata.repository';
@@ -39,7 +45,7 @@ import type { PdfParseWarning } from './lib/pdf-parser';
import { MetadataEventsService, METADATA_AUTHORS_REPLACED } from './metadata-events.service';
type Db = NodePgDatabase<typeof schema>;
type RelationMutationExecutor = Pick<Db, 'delete' | 'execute' | 'insert' | 'select'>;
type RelationMutationExecutor = Pick<Db, 'delete' | 'execute' | 'insert' | 'select' | 'update'>;
interface RelationMutationOptions {
executor?: RelationMutationExecutor;
@@ -51,6 +57,7 @@ const MAX_RELATION_NAME_LENGTH = 200;
const EXTRACTED_COVER_SOURCE = 'extracted';
const MIN_PUBLISHED_YEAR = 1000;
const MAX_PUBLISHED_YEAR = 2200;
const NORMALIZED_AUTHOR_NAME_SQL = normalizeMetadataTextKeySql(authors.name);
function normalizePublishedYear(year: number | null | undefined): number | null | undefined {
if (year === undefined) return undefined;
@@ -312,14 +319,15 @@ export class MetadataService {
): Promise<number[]> {
const normalized = parsedAuthors
.map((author) => ({
name: author.name.trim(),
sortName: author.sortName?.trim() || null,
name: normalizeMetadataText(author.name),
sortName: normalizeMetadataText(author.sortName),
}))
.filter((author) => author.name.length > 0);
.filter((author): author is { name: string; sortName: string | null } => author.name !== null);
const seen = new Set<string>();
const unique = normalized.filter((author) => {
const key = author.name.toLowerCase();
const key = normalizeMetadataTextKey(author.name);
if (!key) return false;
if (seen.has(key)) return false;
seen.add(key);
return true;
@@ -392,29 +400,39 @@ export class MetadataService {
return [];
}
const authorByName = new Map<string, { id: number }>();
const insertedAuthors = await executor
.insert(authors)
.values(uniqueAuthors.map((author) => ({ name: author.name, sortName: author.sortName })))
.onConflictDoNothing()
.returning({ id: authors.id, name: authors.name });
for (const row of insertedAuthors) {
authorByName.set(row.name, { id: row.id });
}
const authorByNameKey = new Map<string, { id: number }>();
await this.addExistingAuthorsByNormalizedName(
executor,
authorByNameKey,
uniqueAuthors.map((author) => author.name),
);
const unresolvedNames = [...new Set(uniqueAuthors.map((author) => author.name))].filter((name) => !authorByName.has(name));
if (unresolvedNames.length > 0) {
const existingAuthors = await executor
.select({ id: authors.id, name: authors.name })
.from(authors)
.where(inArray(authors.name, unresolvedNames));
for (const row of existingAuthors) {
authorByName.set(row.name, { id: row.id });
const missingAuthors = uniqueAuthors.filter((author) => {
const key = normalizeMetadataTextKey(author.name);
return key ? !authorByNameKey.has(key) : false;
});
if (missingAuthors.length > 0) {
const insertedAuthors = await executor
.insert(authors)
.values(missingAuthors.map((author) => ({ name: author.name, sortName: author.sortName })))
.onConflictDoNothing()
.returning({ id: authors.id, name: authors.name });
for (const row of insertedAuthors) {
const key = normalizeMetadataTextKey(row.name);
if (key) authorByNameKey.set(key, { id: row.id });
}
}
await this.addExistingAuthorsByNormalizedName(
executor,
authorByNameKey,
uniqueAuthors.filter((author) => !authorByNameKey.has(normalizeMetadataTextKey(author.name) ?? '')).map((author) => author.name),
);
const links = uniqueAuthors.flatMap((author, index) => {
const match = authorByName.get(author.name);
const key = normalizeMetadataTextKey(author.name);
const match = key ? authorByNameKey.get(key) : undefined;
if (!match) return [];
return [{ bookId, authorId: match.id, displayOrder: index }];
});
@@ -428,6 +446,58 @@ export class MetadataService {
return links.map((link) => link.authorId);
}
private async addExistingAuthorsByNormalizedName(
executor: RelationMutationExecutor,
authorByNameKey: Map<string, { id: number }>,
names: string[],
): Promise<void> {
const keys = [...new Set(names.map((name) => normalizeMetadataTextKey(name)).filter((key): key is string => key !== null))].filter(
(key) => !authorByNameKey.has(key),
);
if (keys.length === 0) return;
const existingAuthors = await executor
.select({ id: authors.id, name: authors.name, normalizedName: NORMALIZED_AUTHOR_NAME_SQL })
.from(authors)
.where(inArray(NORMALIZED_AUTHOR_NAME_SQL, keys));
const desiredNameByKey = new Map<string, string>();
for (const name of names) {
const key = normalizeMetadataTextKey(name);
const displayName = normalizeMetadataText(name);
if (key && displayName && !desiredNameByKey.has(key)) {
desiredNameByKey.set(key, displayName);
}
}
const rowsByKey = new Map<string, typeof existingAuthors>();
for (const row of existingAuthors) {
const key = normalizeMetadataTextKey(row.normalizedName) ?? normalizeMetadataTextKey(row.name);
if (!key || authorByNameKey.has(key)) continue;
const rows = rowsByKey.get(key) ?? [];
rows.push(row);
rowsByKey.set(key, rows);
}
const canonicalizedAuthorIds: number[] = [];
for (const [key, rows] of rowsByKey) {
const desiredName = desiredNameByKey.get(key);
const match = chooseCanonicalMetadataTextRow(rows, { desiredName });
if (!match) continue;
const canonicalName = normalizeMetadataText(match.name);
if (canonicalName && match.name !== canonicalName && !rows.some((row) => row.id !== match.id && row.name === canonicalName)) {
await executor.update(authors).set({ name: canonicalName }).where(eq(authors.id, match.id));
canonicalizedAuthorIds.push(match.id);
}
authorByNameKey.set(key, { id: match.id });
}
if (canonicalizedAuthorIds.length > 0) {
await refreshPrimaryAuthorSortNamesForAuthors(executor, canonicalizedAuthorIds);
}
}
private async replaceGenresInExecutor(executor: RelationMutationExecutor, bookId: number, uniqueGenres: string[]): Promise<void> {
await executor.delete(bookGenres).where(eq(bookGenres.bookId, bookId));
if (uniqueGenres.length === 0) return;
@@ -514,10 +584,10 @@ export class MetadataService {
title: data.title,
subtitle: data.subtitle,
description: data.description,
publisher: data.publisher,
publisher: normalizeMetadataText(data.publisher),
publishedYear: data.publishedYear,
language: data.language,
seriesName: data.seriesName,
seriesName: normalizeMetadataText(data.seriesName),
seriesIndex: data.seriesIndex,
authors: data.authors.map((author) => author.name),
genres: data.genres,
@@ -533,10 +603,10 @@ export class MetadataService {
if (filtered.title !== undefined) scalarFields.title = filtered.title;
if (filtered.subtitle !== undefined) scalarFields.subtitle = filtered.subtitle;
if (filtered.description !== undefined) scalarFields.description = filtered.description;
if (filtered.publisher !== undefined) scalarFields.publisher = filtered.publisher;
if (filtered.publisher !== undefined) scalarFields.publisher = normalizeMetadataText(filtered.publisher);
if (filtered.publishedYear !== undefined) scalarFields.publishedYear = normalizePublishedYear(filtered.publishedYear);
if (filtered.language !== undefined) scalarFields.language = filtered.language;
if (filtered.seriesName !== undefined) scalarFields.seriesName = filtered.seriesName;
if (filtered.seriesName !== undefined) scalarFields.seriesName = normalizeMetadataText(filtered.seriesName);
if (filtered.seriesIndex !== undefined) scalarFields.seriesIndex = filtered.seriesIndex;
if (filtered.audibleId !== undefined) scalarFields.audibleId = filtered.audibleId;
if (filtered.audioMetadata?.durationSeconds !== undefined) scalarFields.durationSeconds = filtered.audioMetadata.durationSeconds;
@@ -573,10 +643,10 @@ export class MetadataService {
description: data.description,
isbn10: data.isbn10 ? data.isbn10.replace(/[^0-9Xx]/g, '') : data.isbn10,
isbn13: data.isbn13 ? data.isbn13.replace(/[^0-9]/g, '') : data.isbn13,
publisher: data.publisher,
publisher: normalizeMetadataText(data.publisher),
publishedYear: data.publishedYear,
language: data.language,
seriesName: data.seriesName,
seriesName: normalizeMetadataText(data.seriesName),
seriesIndex: data.seriesIndex,
authors: data.authors.map((author) => author.name),
genres: data.genres,
@@ -603,10 +673,10 @@ export class MetadataService {
if (filtered.description !== undefined) scalarFields.description = filtered.description;
if (filtered.isbn10 !== undefined) scalarFields.isbn10 = filtered.isbn10;
if (filtered.isbn13 !== undefined) scalarFields.isbn13 = filtered.isbn13;
if (filtered.publisher !== undefined) scalarFields.publisher = filtered.publisher;
if (filtered.publisher !== undefined) scalarFields.publisher = normalizeMetadataText(filtered.publisher);
if (filtered.publishedYear !== undefined) scalarFields.publishedYear = normalizePublishedYear(filtered.publishedYear);
if (filtered.language !== undefined) scalarFields.language = filtered.language;
if (filtered.seriesName !== undefined) scalarFields.seriesName = filtered.seriesName;
if (filtered.seriesName !== undefined) scalarFields.seriesName = normalizeMetadataText(filtered.seriesName);
if (filtered.seriesIndex !== undefined) scalarFields.seriesIndex = filtered.seriesIndex;
if (filtered.rating !== undefined) scalarFields.rating = filtered.rating;
if (filtered.pageCount !== undefined) scalarFields.pageCount = filtered.pageCount;
server/src/modules/migration/executor/executor-utils.test.ts
+25 -8
View File
@@ -116,16 +116,33 @@ describe('migration executor utils', () => {
expect(patch).toEqual({ koboId: 'kobo-book-slug' });
});
it('builds contributor values with truncation and defaulted sort name', () => {
it('normalizes publisher and series metadata text in migration patches', () => {
const patch = buildMetadataPatch({
title: null,
subtitle: null,
isbn10: null,
isbn13: null,
description: null,
publisher: ' Tor\t Books ',
publishedYear: null,
language: null,
seriesName: ' Dune Chronicles ',
presentFields: ['publisher', 'seriesName'],
});
expect(patch).toEqual({ publisher: 'Tor Books', seriesName: 'Dune Chronicles' });
});
it('builds contributor values with truncation, normalization, and defaulted sort name', () => {
const values = buildContributorValues({
name: 'Name',
name: ' Name\t Value ',
sortName: null,
description: 'Bio',
});
expect(values).toEqual({
name: 'Name',
sortName: 'Name',
name: 'Name Value',
sortName: 'Name Value',
description: 'Bio',
});
});
@@ -133,7 +150,7 @@ describe('migration executor utils', () => {
it('derives source contributors from structured data then falls back to parsed legacy names', () => {
const structured = getSourceContributors(
[
{ name: ' B ', sortName: 'Bee', description: null, displayOrder: 2 },
{ name: ' B\t Value ', sortName: 'Bee Value', description: null, displayOrder: 2 },
{ name: 'A', sortName: null, description: 'Bio', displayOrder: 1 },
{ name: 'a', sortName: null, description: null, displayOrder: 3 },
],
@@ -142,12 +159,12 @@ describe('migration executor utils', () => {
expect(structured).toEqual([
{ name: 'A', sortName: null, description: 'Bio' },
{ name: 'B', sortName: 'Bee', description: null },
{ name: 'B Value', sortName: 'Bee Value', description: null },
]);
const legacy = getSourceContributors(undefined, '["Ann", "Bob", "ann"]');
const legacy = getSourceContributors(undefined, '["Ann Marie", "Bob", "ann marie"]');
expect(legacy).toEqual([
{ name: 'Ann', sortName: 'Ann', description: null },
{ name: 'Ann Marie', sortName: 'Ann Marie', description: null },
{ name: 'Bob', sortName: 'Bob', description: null },
]);
});
server/src/modules/migration/executor/executor-utils.ts
+21 -9
View File
@@ -1,4 +1,5 @@
import * as schema from '../../../db/schema';
import { normalizeMetadataText, normalizeMetadataTextKey } from '../../../common/utils/metadata-text-normalize.utils';
import { applyPathMappings } from '../planner/matching.service';
import type { PlannerResult } from '../planner/planner.types';
@@ -99,11 +100,11 @@ export function buildMetadataPatch(sourceBook: {
isbn10: field('isbn10', truncateNullableText(sourceBook.isbn10, 10)),
isbn13: field('isbn13', truncateNullableText(sourceBook.isbn13, 13)),
description: field('description', sourceBook.description),
publisher: field('publisher', truncateNullableText(sourceBook.publisher, 500)),
publisher: field('publisher', truncateNullableMetadataText(sourceBook.publisher, 500)),
publishedYear: field('publishedYear', sanitizeBoundedInteger(sourceBook.publishedYear, 1000, 2200)),
language: field('language', truncateNullableText(sourceBook.language, 100)),
pageCount: field('pageCount', sanitizeNonNegativeInteger(sourceBook.pageCount)),
seriesName: field('seriesName', truncateNullableText(sourceBook.seriesName, 500)),
seriesName: field('seriesName', truncateNullableMetadataText(sourceBook.seriesName, 500)),
seriesIndex: field('seriesIndex', sourceBook.seriesIndex),
rating: field('rating', sanitizeBoundedInteger(sourceBook.rating, 1, 10)),
googleBooksId: field('googleBooksId', truncateNullableText(sourceBook.googleBooksId, 50)),
@@ -137,9 +138,11 @@ export function buildContributorValues(contributor: {
sortName?: string | null;
description?: string | null;
}): typeof schema.authors.$inferInsert {
const name = normalizeMetadataText(contributor.name) ?? '';
const sortName = normalizeMetadataText(contributor.sortName ?? contributor.name) ?? name;
return pruneUndefined({
name: truncateText(contributor.name, 500),
sortName: truncateText(contributor.sortName ?? contributor.name, 500),
name: truncateText(name, 500),
sortName: truncateText(sortName, 500),
description: contributor.description ?? undefined,
});
}
@@ -150,9 +153,13 @@ export function getSourceContributors(
): Array<{ name: string; sortName: string | null; description: string | null }> {
const structured =
contributors
?.filter((c) => c.name.trim().length > 0)
?.filter((c) => normalizeMetadataText(c.name) !== null)
.sort((a, b) => (a.displayOrder ?? 0) - (b.displayOrder ?? 0))
.map((c) => ({ name: c.name.trim(), sortName: c.sortName ?? null, description: c.description ?? null })) ?? [];
.map((c) => ({
name: normalizeMetadataText(c.name)!,
sortName: normalizeMetadataText(c.sortName),
description: c.description ?? null,
})) ?? [];
if (structured.length > 0) return dedupeContributors(structured);
return parseAuthorNames(legacyValue).map((name) => ({ name, sortName: name, description: null }));
}
@@ -180,9 +187,9 @@ function parseAuthorNames(value: string | null): string[] {
const seen = new Set<string>();
const names: string[] = [];
for (const rawName of rawNames) {
const name = rawName.trim();
if (!name) continue;
const key = name.toLowerCase();
const name = normalizeMetadataText(rawName);
const key = normalizeMetadataTextKey(name);
if (!name || !key) continue;
if (seen.has(key)) continue;
seen.add(key);
names.push(name);
@@ -240,6 +247,11 @@ export function truncateNullableText(value: string | null | undefined, maxLength
return truncateText(value, maxLength);
}
function truncateNullableMetadataText(value: string | null | undefined, maxLength: number): string | null | undefined {
if (value === undefined) return undefined;
return normalizeMetadataText(value)?.slice(0, maxLength) ?? null;
}
export function normalizeReadStatus(
rawStatus: string | null,
percentage: number | null,
server/src/modules/narrator/narrator.repository.test.ts
+42 -4
View File
@@ -6,8 +6,8 @@ function makeExecutor() {
const deleteFn = vi.fn().mockReturnValue({ where: deleteWhere });
const narratorReturning = vi.fn();
const narratorOnConflictDoUpdate = vi.fn().mockReturnValue({ returning: narratorReturning });
const narratorValues = vi.fn().mockReturnValue({ onConflictDoUpdate: narratorOnConflictDoUpdate });
const narratorOnConflictDoNothing = vi.fn().mockReturnValue({ returning: narratorReturning });
const narratorValues = vi.fn().mockReturnValue({ onConflictDoNothing: narratorOnConflictDoNothing });
const linkOnConflictDoNothing = vi.fn().mockResolvedValue(undefined);
const linkValues = vi.fn().mockReturnValue({ onConflictDoNothing: linkOnConflictDoNothing });
@@ -22,15 +22,28 @@ function makeExecutor() {
throw new Error('unexpected table passed to insert');
});
const selectWhere = vi.fn().mockResolvedValue([]);
const selectFrom = vi.fn().mockReturnValue({ where: selectWhere });
const selectFn = vi.fn().mockReturnValue({ from: selectFrom });
const updateWhere = vi.fn().mockResolvedValue(undefined);
const updateSet = vi.fn().mockReturnValue({ where: updateWhere });
const updateFn = vi.fn().mockReturnValue({ set: updateSet });
return {
executor: {
delete: deleteFn,
insert: insertFn,
select: selectFn,
update: updateFn,
},
deleteWhere,
narratorValues,
narratorReturning,
linkValues,
selectWhere,
updateSet,
updateWhere,
};
}
@@ -51,7 +64,7 @@ function makeRepository() {
}
describe('NarratorRepository', () => {
it('replaces narrators in a transaction and preserves original display order', async () => {
it('replaces narrators in a transaction and preserves first-seen display order', async () => {
const { repo, transaction, narratorReturning, narratorValues, linkValues } = makeRepository();
narratorReturning.mockResolvedValue([
{ id: 11, name: 'Narrator A' },
@@ -72,7 +85,6 @@ describe('NarratorRepository', () => {
expect(linkValues).toHaveBeenCalledWith([
{ bookId: 91, narratorId: 11, displayOrder: 0 },
{ bookId: 91, narratorId: 22, displayOrder: 1 },
{ bookId: 91, narratorId: 11, displayOrder: 2 },
]);
});
@@ -109,4 +121,30 @@ describe('NarratorRepository', () => {
expect(linkValues).toHaveBeenCalledWith([{ bookId: 42, narratorId: 11, displayOrder: 0 }]);
});
it('canonicalizes an existing narrator with equivalent collapsed whitespace before linking it', async () => {
const { repo, selectWhere, narratorValues, linkValues, updateSet, updateWhere } = makeRepository();
selectWhere.mockResolvedValueOnce([{ id: 44, name: 'Narrator A', normalizedName: 'narrator a' }]);
await repo.replaceForBook(43, [{ name: 'Narrator A', sortName: null }]);
expect(narratorValues).not.toHaveBeenCalled();
expect(updateSet).toHaveBeenCalledWith({ name: 'Narrator A' });
expect(updateWhere).toHaveBeenCalledTimes(1);
expect(linkValues).toHaveBeenCalledWith([{ bookId: 43, narratorId: 44, displayOrder: 0 }]);
});
it('prefers an existing clean narrator row over a legacy whitespace variant', async () => {
const { repo, selectWhere, narratorValues, linkValues, updateSet } = makeRepository();
selectWhere.mockResolvedValueOnce([
{ id: 44, name: 'Narrator A', normalizedName: 'narrator a' },
{ id: 45, name: 'Narrator A', normalizedName: 'narrator a' },
]);
await repo.replaceForBook(43, [{ name: 'Narrator A', sortName: null }]);
expect(narratorValues).not.toHaveBeenCalled();
expect(updateSet).not.toHaveBeenCalledWith({ name: 'Narrator A' });
expect(linkValues).toHaveBeenCalledWith([{ bookId: 43, narratorId: 45, displayOrder: 0 }]);
});
});
server/src/modules/narrator/narrator.repository.ts
+88 -17
View File
@@ -1,13 +1,20 @@
import { Inject, Injectable } from '@nestjs/common';
import { eq, sql } from 'drizzle-orm';
import { eq, inArray } from 'drizzle-orm';
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
import { DB } from '../../db';
import * as schema from '../../db/schema';
import { bookNarrators, narrators } from '../../db/schema';
import {
chooseCanonicalMetadataTextRow,
normalizeMetadataText,
normalizeMetadataTextKey,
normalizeMetadataTextKeySql,
} from '../../common/utils/metadata-text-normalize.utils';
type Db = NodePgDatabase<typeof schema>;
type NarratorMutationExecutor = Pick<Db, 'delete' | 'insert' | 'select'>;
type NarratorMutationExecutor = Pick<Db, 'delete' | 'insert' | 'select' | 'update'>;
const NORMALIZED_NARRATOR_NAME_SQL = normalizeMetadataTextKeySql(narrators.name);
@Injectable()
export class NarratorRepository {
@@ -35,28 +42,46 @@ export class NarratorRepository {
const uniqueByName = new Map<string, { name: string; sortName: string | null }>();
for (const value of names) {
if (!uniqueByName.has(value.name)) {
uniqueByName.set(value.name, value);
const key = normalizeMetadataTextKey(value.name);
if (key && !uniqueByName.has(key)) {
uniqueByName.set(key, value);
}
}
const uniqueNames = [...uniqueByName.values()];
const narratorByName = new Map<string, { id: number }>();
const narratorByNameKey = new Map<string, { id: number }>();
await this.addExistingNarratorsByNormalizedName(
executor,
narratorByNameKey,
uniqueNames.map((narrator) => narrator.name),
);
const upsertedNarrators = await executor
.insert(narrators)
.values(uniqueNames)
.onConflictDoUpdate({
target: narrators.name,
set: { name: sql`excluded.name` },
})
.returning({ id: narrators.id, name: narrators.name });
for (const row of upsertedNarrators) {
narratorByName.set(row.name, { id: row.id });
const missingNames = uniqueNames.filter((narrator) => {
const key = normalizeMetadataTextKey(narrator.name);
return key ? !narratorByNameKey.has(key) : false;
});
if (missingNames.length > 0) {
const insertedNarrators = await executor
.insert(narrators)
.values(missingNames)
.onConflictDoNothing()
.returning({ id: narrators.id, name: narrators.name });
for (const row of insertedNarrators) {
const key = normalizeMetadataTextKey(row.name);
if (key) narratorByNameKey.set(key, { id: row.id });
}
}
const links = names.flatMap((value, index) => {
const match = narratorByName.get(value.name);
await this.addExistingNarratorsByNormalizedName(
executor,
narratorByNameKey,
uniqueNames.filter((narrator) => !narratorByNameKey.has(normalizeMetadataTextKey(narrator.name) ?? '')).map((narrator) => narrator.name),
);
const links = uniqueNames.flatMap((value, index) => {
const key = normalizeMetadataTextKey(value.name);
const match = key ? narratorByNameKey.get(key) : undefined;
if (!match) return [];
return [{ bookId, narratorId: match.id, displayOrder: index }];
});
@@ -65,4 +90,50 @@ export class NarratorRepository {
await executor.insert(bookNarrators).values(links).onConflictDoNothing();
}
}
private async addExistingNarratorsByNormalizedName(
executor: NarratorMutationExecutor,
narratorByNameKey: Map<string, { id: number }>,
names: string[],
): Promise<void> {
const keys = [...new Set(names.map((name) => normalizeMetadataTextKey(name)).filter((key): key is string => key !== null))].filter(
(key) => !narratorByNameKey.has(key),
);
if (keys.length === 0) return;
const existingNarrators = await executor
.select({ id: narrators.id, name: narrators.name, normalizedName: NORMALIZED_NARRATOR_NAME_SQL })
.from(narrators)
.where(inArray(NORMALIZED_NARRATOR_NAME_SQL, keys));
const desiredNameByKey = new Map<string, string>();
for (const name of names) {
const key = normalizeMetadataTextKey(name);
const displayName = normalizeMetadataText(name);
if (key && displayName && !desiredNameByKey.has(key)) {
desiredNameByKey.set(key, displayName);
}
}
const rowsByKey = new Map<string, typeof existingNarrators>();
for (const row of existingNarrators) {
const key = normalizeMetadataTextKey(row.normalizedName) ?? normalizeMetadataTextKey(row.name);
if (!key || narratorByNameKey.has(key)) continue;
const rows = rowsByKey.get(key) ?? [];
rows.push(row);
rowsByKey.set(key, rows);
}
for (const [key, rows] of rowsByKey) {
const desiredName = desiredNameByKey.get(key);
const match = chooseCanonicalMetadataTextRow(rows, { desiredName });
if (!match) continue;
const canonicalName = normalizeMetadataText(match.name);
if (canonicalName && match.name !== canonicalName && !rows.some((row) => row.id !== match.id && row.name === canonicalName)) {
await executor.update(narrators).set({ name: canonicalName }).where(eq(narrators.id, match.id));
}
narratorByNameKey.set(key, { id: match.id });
}
}
}
server/src/modules/narrator/narrator.service.test.ts
+19
View File
@@ -54,5 +54,24 @@ describe('NarratorService', () => {
const [, names] = (narratorRepo.replaceForBook as ReturnType<typeof vi.fn>).mock.calls[0];
expect(names[0].sortName).toBeNull();
});
it('normalizes whitespace, filters blanks, and deduplicates names case-insensitively', async () => {
const { service, narratorRepo } = makeService();
await service.replaceForBook(1, [' Michael\t Kramer ', 'michael kramer', ' ', 'Kate Reading']);
expect(narratorRepo.replaceForBook).toHaveBeenCalledWith(1, [
{ name: 'Michael Kramer', sortName: null },
{ name: 'Kate Reading', sortName: null },
]);
});
it('normalizes object-form narrator names and sort names', async () => {
const { service, narratorRepo } = makeService();
await service.replaceForBook(1, [{ name: ' Simon Vance ', sortName: ' Vance,\tSimon ' }]);
expect(narratorRepo.replaceForBook).toHaveBeenCalledWith(1, [{ name: 'Simon Vance', sortName: 'Vance, Simon' }]);
});
});
});
server/src/modules/narrator/narrator.service.ts
+17 -2
View File
@@ -2,10 +2,11 @@ import { Injectable } from '@nestjs/common';
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
import * as schema from '../../db/schema';
import { normalizeMetadataText, normalizeMetadataTextKey } from '../../common/utils/metadata-text-normalize.utils';
import { NarratorRepository } from './narrator.repository';
type Db = NodePgDatabase<typeof schema>;
type NarratorMutationExecutor = Pick<Db, 'delete' | 'insert' | 'select'>;
type NarratorMutationExecutor = Pick<Db, 'delete' | 'insert' | 'select' | 'update'>;
@Injectable()
export class NarratorService {
@@ -16,10 +17,11 @@ export class NarratorService {
names: string[] | { name: string; sortName: string | null }[],
options: { executor?: NarratorMutationExecutor } = {},
): Promise<void> {
const normalized =
const input =
names.length > 0 && typeof names[0] !== 'string'
? (names as { name: string; sortName: string | null }[])
: (names as string[]).map((name) => ({ name, sortName: null }));
const normalized = this.normalizeNarratorNames(input);
if (options.executor) {
await this.narratorRepo.replaceForBook(bookId, normalized, options.executor);
@@ -27,4 +29,17 @@ export class NarratorService {
}
await this.narratorRepo.replaceForBook(bookId, normalized);
}
private normalizeNarratorNames(names: { name: string; sortName: string | null }[]): { name: string; sortName: string | null }[] {
const normalized: { name: string; sortName: string | null }[] = [];
const seen = new Set<string>();
for (const narrator of names) {
const name = normalizeMetadataText(narrator.name);
const key = normalizeMetadataTextKey(name);
if (!name || !key || seen.has(key)) continue;
seen.add(key);
normalized.push({ name, sortName: normalizeMetadataText(narrator.sortName) });
}
return normalized;
}
}