diff --git a/client/src/components/ui/IdentifierInput.vue b/client/src/components/ui/IdentifierInput.vue new file mode 100644 index 000000000..f9941646a --- /dev/null +++ b/client/src/components/ui/IdentifierInput.vue @@ -0,0 +1,43 @@ + + + diff --git a/client/src/components/ui/__tests__/IdentifierInput.spec.ts b/client/src/components/ui/__tests__/IdentifierInput.spec.ts new file mode 100644 index 000000000..58af25602 --- /dev/null +++ b/client/src/components/ui/__tests__/IdentifierInput.spec.ts @@ -0,0 +1,181 @@ +import { mount, type VueWrapper } from '@vue/test-utils' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { IdentifierInputKind } from '@bookorbit/types' +import IdentifierInput from '../IdentifierInput.vue' + +let wrapper: VueWrapper | undefined + +afterEach(() => { + wrapper?.unmount() + wrapper = undefined +}) + +function mountInput(kind: IdentifierInputKind, modelValue: string | null = null, attrs: Record = {}) { + wrapper = mount(IdentifierInput, { + props: { kind, modelValue, 'onUpdate:modelValue': (value: string) => wrapper!.setProps({ modelValue: value }) }, + attrs, + attachTo: document.body, + }) + return wrapper +} + +function field(target: VueWrapper) { + return target.get('input') +} + +function emitted(target: VueWrapper): string[] { + return (target.emitted('update:modelValue') ?? []).map(([value]) => value as string) +} + +async function typeAt(target: VueWrapper, value: string, caret: number) { + const input = field(target).element + input.focus() + input.value = value + input.setSelectionRange(caret, caret) + await field(target).trigger('input') + return input +} + +describe('IdentifierInput', () => { + it('renders the bound value, and an empty field for null', async () => { + const target = mountInput('isbn', '9783426226223') + expect(field(target).element.value).toBe('9783426226223') + + await target.setProps({ modelValue: null }) + expect(field(target).element.value).toBe('') + }) + + it('has no length limit that could cut a pasted identifier', () => { + const target = mountInput('isbn') + expect(field(target).attributes('maxlength')).toBeUndefined() + expect(field(target).attributes('type')).toBe('text') + }) + + it('cleans a pasted hyphenated ISBN instead of truncating it', async () => { + const target = mountInput('isbn') + + await field(target).setValue('978-3426226223') + + expect(field(target).element.value).toBe('9783426226223') + expect(emitted(target)).toEqual(['9783426226223']) + }) + + it('cleans an ISBN copied with its Amazon label and invisible characters', async () => { + const target = mountInput('isbn') + + await field(target).setValue('\u200BISBN-13 : 978-3426226223\u200E') + + expect(field(target).element.value).toBe('9783426226223') + expect(emitted(target)).toEqual(['9783426226223']) + }) + + it('removes the invisible character copied in front of an ASIN', async () => { + const target = mountInput('asin') + + await field(target).setValue('\u200Bb00x47zvxm') + + expect(field(target).element.value).toBe('B00X47ZVXM') + expect(emitted(target)).toEqual(['B00X47ZVXM']) + }) + + it('keeps the punctuation of other provider ids while dropping whitespace', async () => { + const target = mountInput('provider-id') + + await field(target).setValue(' zyTC-Al_FPjgYC\uFEFF') + + expect(field(target).element.value).toBe('zyTC-Al_FPjgYC') + expect(emitted(target)).toEqual(['zyTC-Al_FPjgYC']) + }) + + it('shows an over-long value in full so validation can name it', async () => { + const target = mountInput('isbn') + + await field(target).setValue('978-3426226223-0000') + + expect(field(target).element.value).toBe('97834262262230000') + expect(emitted(target)).toEqual(['97834262262230000']) + }) + + it('swallows a typed separator without emitting an unchanged value', async () => { + const target = mountInput('isbn', '978') + + const input = await typeAt(target, '978-', 4) + + expect(input.value).toBe('978') + expect(input.selectionStart).toBe(3) + expect(emitted(target)).toEqual([]) + }) + + it('keeps the caret after the same character when text is cleaned mid-value', async () => { + const target = mountInput('isbn', '978226223') + + const input = await typeAt(target, '978-3426-226223', 8) + + expect(input.value).toBe('9783426226223') + expect(input.selectionStart).toBe(7) + expect(input.selectionEnd).toBe(7) + }) + + it('leaves the caret alone when nothing needed cleaning', async () => { + const target = mountInput('isbn', '978') + + const input = await typeAt(target, '9783', 4) + + expect(input.selectionStart).toBe(4) + expect(emitted(target)).toEqual(['9783']) + }) + + it('emits an empty string when an existing value is cleared', async () => { + const target = mountInput('isbn', '9783426226223') + + await field(target).setValue('') + + expect(emitted(target)).toEqual(['']) + }) + + it('does not turn an empty field into an edit when only separators were typed', async () => { + const target = mountInput('isbn', null) + + await field(target).setValue('-') + + expect(field(target).element.value).toBe('') + expect(emitted(target)).toEqual([]) + }) + + it('waits for an input method composition to finish before cleaning', async () => { + const target = mountInput('isbn') + const input = field(target).element + + input.value = '\uFF19\uFF17\uFF18\uFF0D' + input.dispatchEvent(new InputEvent('input', { isComposing: true })) + expect(input.value).toBe('\uFF19\uFF17\uFF18\uFF0D') + expect(emitted(target)).toEqual([]) + + input.dispatchEvent(new CompositionEvent('compositionend')) + expect(input.value).toBe('978') + expect(emitted(target)).toEqual(['978']) + }) + + it('reflects a value set from outside, such as an applied metadata match', async () => { + const target = mountInput('isbn', '') + + await target.setProps({ modelValue: '9780306406157' }) + + expect(field(target).element.value).toBe('9780306406157') + }) + + it('passes attributes and listeners through to the input', async () => { + const onInput = vi.fn<(event: Event) => void>() + const target = mountInput('isbn', null, { id: 'isbn-field', class: 'font-mono', disabled: true, onInput }) + + expect(field(target).attributes('id')).toBe('isbn-field') + expect(field(target).classes()).toContain('font-mono') + expect(field(target).element.disabled).toBe(true) + + field(target).element.disabled = false + await field(target).setValue('978-0') + + expect(onInput).toHaveBeenCalledTimes(1) + expect(emitted(target)).toEqual(['9780']) + }) +}) diff --git a/client/src/features/book-dock/components/BookDockBulkEditDialog.vue b/client/src/features/book-dock/components/BookDockBulkEditDialog.vue index 07a3e020b..05e723dad 100644 --- a/client/src/features/book-dock/components/BookDockBulkEditDialog.vue +++ b/client/src/features/book-dock/components/BookDockBulkEditDialog.vue @@ -2,8 +2,9 @@ import { computed, reactive, ref } from 'vue' import { useI18n } from 'vue-i18n' import { X, Loader2 } from '@lucide/vue' -import { isValidSeriesIndex, type BookDockBulkEditResult, type BookDockMetadata } from '@bookorbit/types' +import { isValidSeriesIndex, type BookDockBulkEditResult, type BookDockMetadata, type IdentifierInputKind } from '@bookorbit/types' import { api } from '@/lib/api' +import IdentifierInput from '@/components/ui/IdentifierInput.vue' const { t } = useI18n() @@ -29,19 +30,22 @@ const loading = ref(false) const error = ref(null) const result = ref(null) -const FIELDS = computed<{ key: keyof BookDockMetadata; label: string; type: 'text' | 'number' | 'array' }[]>(() => [ +const FIELDS = computed<{ key: keyof BookDockMetadata; label: string; type: 'text' | 'number' | 'array'; identifier?: IdentifierInputKind }[]>(() => [ { key: 'title', label: t('bookDock.field.title'), type: 'text' }, { key: 'authors', label: t('bookDock.field.authors'), type: 'array' }, { key: 'publisher', label: t('bookDock.field.publisher'), type: 'text' }, { key: 'publishedYear', label: t('bookDock.field.year'), type: 'number' }, { key: 'language', label: t('bookDock.field.language'), type: 'text' }, - { key: 'isbn13', label: t('bookDock.field.isbn13'), type: 'text' }, - { key: 'isbn10', label: t('bookDock.field.isbn10'), type: 'text' }, + { key: 'isbn13', label: t('bookDock.field.isbn13'), type: 'text', identifier: 'isbn' }, + { key: 'isbn10', label: t('bookDock.field.isbn10'), type: 'text', identifier: 'isbn' }, { key: 'seriesName', label: t('bookDock.field.series'), type: 'text' }, { key: 'seriesIndex', label: t('bookDock.field.seriesIndex'), type: 'text' }, { key: 'genres', label: t('bookDock.field.genres'), type: 'array' }, ]) +const FIELD_INPUT_CLASS = + 'w-full h-8 rounded-lg border border-input bg-background px-3 text-sm outline-none focus:ring-1 focus:ring-ring disabled:opacity-40' + const enabledFields = reactive>(new Set()) const values = reactive>({}) const mergeArrays = ref(true) @@ -141,11 +145,19 @@ function handleClose() { /> diff --git a/client/src/features/book-dock/components/BookDockFileSheet.vue b/client/src/features/book-dock/components/BookDockFileSheet.vue index 1a2bd123a..eb44b6f29 100644 --- a/client/src/features/book-dock/components/BookDockFileSheet.vue +++ b/client/src/features/book-dock/components/BookDockFileSheet.vue @@ -17,6 +17,7 @@ import { SERIES_INDEX_MAX_LENGTH, } from '@bookorbit/types' import BookDockStatusBadge from './BookDockStatusBadge.vue' +import IdentifierInput from '@/components/ui/IdentifierInput.vue' import MetadataMatchWorkspace from '@/features/book/components/metadata-match/MetadataMatchWorkspace.vue' import type { MetadataQuery } from '@/features/book/components/metadata-match/MetadataMatchQuery.vue' import { useBookDockDetail } from '../composables/useBookDockDetail' @@ -574,16 +575,18 @@ onMounted(() => { - diff --git a/client/src/features/book/components/detail/tabs/__tests__/EditMetadataTab.spec.ts b/client/src/features/book/components/detail/tabs/__tests__/EditMetadataTab.spec.ts index 6a2fee0f7..ca59a9bf9 100644 --- a/client/src/features/book/components/detail/tabs/__tests__/EditMetadataTab.spec.ts +++ b/client/src/features/book/components/detail/tabs/__tests__/EditMetadataTab.spec.ts @@ -225,6 +225,42 @@ describe('EditMetadataTab cover tiles', () => { expect(body.metadata).toMatchObject({ isbn10: '0306406152', isbn13: '9780306406157' }) }) + // Issue #1624: identifiers pasted from Amazon carry hyphens and invisible characters. + it('saves pasted identifiers cleaned instead of truncated', async () => { + vi.mocked(api).mockImplementation(async (url, init) => { + if (init?.method === 'PATCH') return json({ book: savedBook, write: null, libraryAutoWriteEnabled: false }) + if (String(url).includes('/metadata-fetch/providers')) return json([{ key: 'amazon' }, { key: 'google' }, { key: 'audible' }]) + return json({}) + }) + const wrapper = mountTab(makeBook()) + await flushPromises() + + const isbn13 = wrapper.get('[data-field="isbn13"] input') + const isbn10 = wrapper.get('[data-field="isbn10"] input') + expect(isbn13.attributes('maxlength')).toBeUndefined() + expect(isbn10.attributes('maxlength')).toBeUndefined() + + await isbn13.setValue('978-3426226223') + await isbn10.setValue('\u200B0-8044-2957-x') + await wrapper.get('#provider-id-amazonId').setValue('\u200BB00X47ZVXM') + await wrapper.get('#provider-id-audibleId').setValue(' b002v0qk4c ') + await wrapper.get('#provider-id-googleBooksId').setValue('zyTC-Al_FPjgYC\u200E') + + expect(isbn13.element.value).toBe('9783426226223') + expect(isbn10.element.value).toBe('080442957X') + + await saveButton(wrapper).trigger('click') + await flushPromises() + const body = JSON.parse(patchCalls()[0]![1]!.body as string) + expect(body.metadata).toMatchObject({ + isbn13: '9783426226223', + isbn10: '080442957X', + amazonId: 'B00X47ZVXM', + audibleId: 'B002V0QK4C', + googleBooksId: 'zyTC-Al_FPjgYC', + }) + }) + it('warns when auto-fill used saved results for a provider that was unavailable', async () => { vi.mocked(toast.warning).mockClear() vi.mocked(api).mockImplementation(async (url) => { diff --git a/packages/types/src/identifier-input.ts b/packages/types/src/identifier-input.ts new file mode 100644 index 000000000..4b1b0bf42 --- /dev/null +++ b/packages/types/src/identifier-input.ts @@ -0,0 +1,82 @@ +/** + * How a hand-entered identifier is cleaned before it is stored. An identifier copied from a store + * page carries its print formatting (hyphens, spaces) and often an invisible character such as a + * zero-width space or a bidi mark. None of these belong to the identifier, and a field that looks + * right while holding one is very hard to diagnose. Cleaning never truncates: an over-long result + * stays over-long so validation can name it. + */ +export type IdentifierInputKind = "isbn" | "asin" | "provider-id"; + +/** + * The cleaning rule for each editable identifier field. Only ISBNs and ASINs have a fixed alphabet; + * every other provider id keeps its punctuation, because Google Books ids and Hardcover and Kobo + * slugs legitimately contain `-` and `_`. + */ +export const IDENTIFIER_INPUT_KIND_BY_FIELD = { + isbn10: "isbn", + isbn13: "isbn", + amazonId: "asin", + audibleId: "asin", + googleBooksId: "provider-id", + goodreadsId: "provider-id", + hardcoverId: "provider-id", + hardcoverEditionId: "provider-id", + openLibraryId: "provider-id", + itunesId: "provider-id", + librofmId: "provider-id", + koboId: "provider-id", + comicvineId: "provider-id", + ranobedbId: "provider-id", + lubimyczytacId: "provider-id", + aladinId: "provider-id", +} as const satisfies Record; + +export type IdentifierInputField = keyof typeof IDENTIFIER_INPUT_KIND_BY_FIELD; + +// Whitespace plus Unicode control (Cc) and format (Cf) characters: zero-width spaces and joiners, +// bidi marks, the byte-order mark and the soft hyphen. +const INVISIBLE_OR_WHITESPACE = /[\s\p{Cc}\p{Cf}]/gu; + +// Amazon prints "ISBN-13 : 978-...". Without this the label's digits survive the digit filter and +// prefix the real number. The colon is required so an ISBN-10 starting with 10 or 13 is never cut. +const NUMBERED_ISBN_LABEL = /^ISBN\p{Pd}?1[03]:/iu; + +/** Keeps the digits and the ISBN-10 check character `X`, so `978-3426226223` becomes `9783426226223`. */ +export function normalizeIsbnInput(value: string): string { + return value + .normalize("NFKC") + .replace(INVISIBLE_OR_WHITESPACE, "") + .replace(NUMBERED_ISBN_LABEL, "") + .toUpperCase() + .replace(/[^0-9X]/g, ""); +} + +/** ASINs are upper-case alphanumerics; anything else is formatting or an invisible character. */ +export function normalizeAsinInput(value: string): string { + return value + .normalize("NFKC") + .toUpperCase() + .replace(/[^0-9A-Z]/g, ""); +} + +/** Removes only whitespace and invisible characters, which no provider id contains. */ +export function normalizeProviderIdInput(value: string): string { + return value.replace(INVISIBLE_OR_WHITESPACE, ""); +} + +export function normalizeIdentifierInput(kind: IdentifierInputKind, value: string): string { + switch (kind) { + case "isbn": + return normalizeIsbnInput(value); + case "asin": + return normalizeAsinInput(value); + case "provider-id": + return normalizeProviderIdInput(value); + } +} + +export function identifierInputKindForField(field: string): IdentifierInputKind | null { + return Object.prototype.hasOwnProperty.call(IDENTIFIER_INPUT_KIND_BY_FIELD, field) + ? IDENTIFIER_INPUT_KIND_BY_FIELD[field as IdentifierInputField] + : null; +} diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index c15a0475b..b19617eca 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -12,6 +12,7 @@ export * from "./metadata-preferences"; export * from "./custom-metadata"; export * from "./custom-icon"; export * from "./metadata-lock"; +export * from "./identifier-input"; export * from "./dashboard"; export * from "./book"; export * from "./book-duplicates"; diff --git a/server/src/common/decorators/normalize-identifier.decorator.test.ts b/server/src/common/decorators/normalize-identifier.decorator.test.ts new file mode 100644 index 000000000..d3201eb83 --- /dev/null +++ b/server/src/common/decorators/normalize-identifier.decorator.test.ts @@ -0,0 +1,237 @@ +import 'reflect-metadata'; + +import { plainToInstance } from 'class-transformer'; +import { + IDENTIFIER_INPUT_KIND_BY_FIELD, + identifierInputKindForField, + normalizeAsinInput, + normalizeIdentifierInput, + normalizeIsbnInput, + normalizeProviderIdInput, +} from '@bookorbit/types'; + +import { PROVIDER_ID_MAX_LENGTHS } from '../utils/provider-id.utils'; +import { NormalizeIdentifier } from './normalize-identifier.decorator'; + +const INVISIBLE_CHARACTERS = { + 'zero-width space': '\u200B', + 'zero-width non-joiner': '\u200C', + 'zero-width joiner': '\u200D', + 'left-to-right mark': '\u200E', + 'right-to-left mark': '\u200F', + 'left-to-right embedding': '\u202A', + 'pop directional formatting': '\u202C', + 'left-to-right isolate': '\u2066', + 'word joiner': '\u2060', + 'byte-order mark': '\uFEFF', + 'soft hyphen': '\u00AD', + 'null character': '\u0000', + 'delete character': '\u007F', +}; + +const WHITESPACE_CHARACTERS = { + space: ' ', + tab: '\t', + 'line feed': '\n', + 'carriage return': '\r', + 'no-break space': '\u00A0', + 'thin space': '\u2009', + 'narrow no-break space': '\u202F', + 'ideographic space': '\u3000', + 'line separator': '\u2028', +}; + +describe('normalizeIsbnInput', () => { + it('removes the hyphens Amazon prints in an ISBN-13', () => { + expect(normalizeIsbnInput('978-3426226223')).toBe('9783426226223'); + expect(normalizeIsbnInput('978-3-426-22622-3')).toBe('9783426226223'); + }); + + it('removes every dash and separator an ISBN is printed with', () => { + for (const separator of ['-', '\u2010', '\u2011', '\u2012', '\u2013', '\u2212', '\uFE63', '\uFF0D', '.', '/', '_', '\u00B7']) { + expect({ separator, value: normalizeIsbnInput(`978${separator}0${separator}306${separator}40615${separator}7`) }).toEqual({ + separator, + value: '9780306406157', + }); + } + }); + + it.each(Object.entries(INVISIBLE_CHARACTERS))('removes a %s wherever it appears', (_name, character) => { + expect(normalizeIsbnInput(`${character}978030640615${character}7${character}`)).toBe('9780306406157'); + }); + + it.each(Object.entries(WHITESPACE_CHARACTERS))('removes a %s wherever it appears', (_name, character) => { + expect(normalizeIsbnInput(`${character}978 0306${character}406157${character}`)).toBe('9780306406157'); + }); + + it('upper-cases the ISBN-10 check character', () => { + expect(normalizeIsbnInput('0-8044-2957-x')).toBe('080442957X'); + expect(normalizeIsbnInput('080442957X')).toBe('080442957X'); + }); + + it('folds full-width digits, hyphens and check characters to ASCII', () => { + expect(normalizeIsbnInput('\uFF19\uFF17\uFF18\uFF0D\uFF14\uFF10\uFF18\uFF18\uFF10\uFF16\uFF16\uFF13\uFF17\uFF16')).toBe('9784088066376'); + expect(normalizeIsbnInput('080442957\uFF58')).toBe('080442957X'); + }); + + it('drops a numbered ISBN label together with its digits', () => { + expect(normalizeIsbnInput('ISBN-13 : 978-3426226223')).toBe('9783426226223'); + expect(normalizeIsbnInput('ISBN13: 9783426226223')).toBe('9783426226223'); + expect(normalizeIsbnInput('ISBN 13: 9783426226223')).toBe('9783426226223'); + expect(normalizeIsbnInput('isbn-10: 0-306-40615-2')).toBe('0306406152'); + expect(normalizeIsbnInput('ISBN\u201013\uFF1A978-3426226223')).toBe('9783426226223'); + expect(normalizeIsbnInput('\u200BISBN-13 : 978-3426226223')).toBe('9783426226223'); + }); + + it('drops a bare ISBN label', () => { + expect(normalizeIsbnInput('ISBN 978-3-426-22622-3')).toBe('9783426226223'); + expect(normalizeIsbnInput('ISBN: 0-306-40615-2')).toBe('0306406152'); + }); + + it('keeps the leading digits of an ISBN-10 that starts with 10 or 13', () => { + expect(normalizeIsbnInput('1302123456')).toBe('1302123456'); + expect(normalizeIsbnInput('13-02123456')).toBe('1302123456'); + expect(normalizeIsbnInput('ISBN 1302123456')).toBe('1302123456'); + expect(normalizeIsbnInput('ISBN: 1034567890')).toBe('1034567890'); + expect(normalizeIsbnInput('ISBN-1302123456')).toBe('1302123456'); + }); + + it('never truncates an over-long value', () => { + expect(normalizeIsbnInput('978-3426226223-0000')).toBe('97834262262230000'); + expect(normalizeIsbnInput('9'.repeat(40))).toBe('9'.repeat(40)); + }); + + it('returns an empty string when nothing of the identifier remains', () => { + expect(normalizeIsbnInput('')).toBe(''); + expect(normalizeIsbnInput(' \u200B\uFEFF\t')).toBe(''); + expect(normalizeIsbnInput('---')).toBe(''); + }); + + it('is idempotent', () => { + for (const value of ['978-3426226223', '\u200BISBN-13 : 978-3426226223', '0-8044-2957-x', 'ISBN 1302123456', '']) { + const once = normalizeIsbnInput(value); + expect(normalizeIsbnInput(once)).toBe(once); + } + }); +}); + +describe('normalizeAsinInput', () => { + it('removes the zero-width space Amazon copies in front of an ASIN', () => { + expect(normalizeAsinInput('\u200BB00X47ZVXM')).toBe('B00X47ZVXM'); + }); + + it.each(Object.entries(INVISIBLE_CHARACTERS))('removes a %s wherever it appears', (_name, character) => { + expect(normalizeAsinInput(`${character}B00X4${character}7ZVXM${character}`)).toBe('B00X47ZVXM'); + }); + + it.each(Object.entries(WHITESPACE_CHARACTERS))('removes a %s wherever it appears', (_name, character) => { + expect(normalizeAsinInput(`${character}B00X4${character}7ZVXM${character}`)).toBe('B00X47ZVXM'); + }); + + it('removes separators and upper-cases the result', () => { + expect(normalizeAsinInput('b00x-47zv-xm')).toBe('B00X47ZVXM'); + expect(normalizeAsinInput('B00X47ZVXM.')).toBe('B00X47ZVXM'); + }); + + it('keeps an ISBN-10 shaped ASIN intact, including its check character', () => { + expect(normalizeAsinInput('0-8044-2957-x')).toBe('080442957X'); + }); + + it('folds full-width characters to ASCII', () => { + expect(normalizeAsinInput('\uFF22\uFF10\uFF10\uFF58\uFF14\uFF17\uFF3A\uFF36\uFF38\uFF2D')).toBe('B00X47ZVXM'); + }); + + it('never truncates an over-long value', () => { + expect(normalizeAsinInput('B00X47ZVXM-B00X47ZVXM')).toBe('B00X47ZVXMB00X47ZVXM'); + }); + + it('is idempotent', () => { + for (const value of ['\u200Bb00x47zvxm ', 'B00X47ZVXM', '']) { + const once = normalizeAsinInput(value); + expect(normalizeAsinInput(once)).toBe(once); + } + }); +}); + +describe('normalizeProviderIdInput', () => { + it.each(Object.entries(INVISIBLE_CHARACTERS))('removes a %s wherever it appears', (_name, character) => { + expect(normalizeProviderIdInput(`${character}zyTC${character}AlFPjgYC${character}`)).toBe('zyTCAlFPjgYC'); + }); + + it.each(Object.entries(WHITESPACE_CHARACTERS))('removes a %s wherever it appears', (_name, character) => { + expect(normalizeProviderIdInput(`${character}12345${character}678${character}`)).toBe('12345678'); + }); + + it('keeps the punctuation and case that provider ids legitimately use', () => { + expect(normalizeProviderIdInput('zyTC-Al_FPjgYC')).toBe('zyTC-Al_FPjgYC'); + expect(normalizeProviderIdInput('the-name-of-the-wind')).toBe('the-name-of-the-wind'); + expect(normalizeProviderIdInput('OL7353617M')).toBe('OL7353617M'); + expect(normalizeProviderIdInput('12345.The_Book')).toBe('12345.The_Book'); + }); + + it('never truncates an over-long value', () => { + expect(normalizeProviderIdInput(` ${'a'.repeat(600)} `)).toBe('a'.repeat(600)); + }); +}); + +describe('normalizeIdentifierInput', () => { + it('applies the rule for the requested kind', () => { + expect(normalizeIdentifierInput('isbn', ' 978-3426226223 ')).toBe('9783426226223'); + expect(normalizeIdentifierInput('asin', '\u200Bb00x47zvxm')).toBe('B00X47ZVXM'); + expect(normalizeIdentifierInput('provider-id', '\u200Bzy-TC ')).toBe('zy-TC'); + }); +}); + +describe('identifierInputKindForField', () => { + it('cleans ISBNs as ISBNs and Amazon and Audible ids as ASINs', () => { + expect(identifierInputKindForField('isbn10')).toBe('isbn'); + expect(identifierInputKindForField('isbn13')).toBe('isbn'); + expect(identifierInputKindForField('amazonId')).toBe('asin'); + expect(identifierInputKindForField('audibleId')).toBe('asin'); + }); + + // A provider id column without a rule would reach the DTO decorator and throw at boot. + it('has a rule for every stored provider id', () => { + for (const field of Object.keys(PROVIDER_ID_MAX_LENGTHS)) { + expect({ field, kind: identifierInputKindForField(field) }).toEqual({ field, kind: expect.any(String) }); + } + expect(Object.keys(IDENTIFIER_INPUT_KIND_BY_FIELD).sort()).toEqual([...Object.keys(PROVIDER_ID_MAX_LENGTHS), 'isbn10', 'isbn13'].sort()); + }); + + it('returns null for fields that are not identifiers, including inherited object keys', () => { + for (const field of ['title', 'publisher', 'toString', 'constructor', '__proto__', 'hasOwnProperty', '']) { + expect({ field, kind: identifierInputKindForField(field) }).toEqual({ field, kind: null }); + } + }); +}); + +describe('NormalizeIdentifier', () => { + class IdentifierProbe { + @NormalizeIdentifier() isbn13?: unknown; + @NormalizeIdentifier() amazonId?: unknown; + @NormalizeIdentifier() googleBooksId?: unknown; + } + + it('cleans each string by the rule its property name selects', () => { + const probe = plainToInstance(IdentifierProbe, { + isbn13: '\u200B978-3426226223', + amazonId: '\u200Bb00x47zvxm ', + googleBooksId: ' zyTC-Al_FPjgYC\u200E', + }); + + expect(probe).toEqual({ isbn13: '9783426226223', amazonId: 'B00X47ZVXM', googleBooksId: 'zyTC-Al_FPjgYC' }); + }); + + it('passes non-string values through for the validators to judge', () => { + expect(plainToInstance(IdentifierProbe, { isbn13: null, amazonId: 9780306406157, googleBooksId: ['x'] })).toEqual({ + isbn13: null, + amazonId: 9780306406157, + googleBooksId: ['x'], + }); + expect(plainToInstance(IdentifierProbe, {}).isbn13).toBeUndefined(); + }); + + it('refuses a property that has no identifier rule', () => { + expect(() => NormalizeIdentifier()(Object.prototype, 'title')).toThrow('@NormalizeIdentifier() has no identifier rule for "title"'); + }); +}); diff --git a/server/src/common/decorators/normalize-identifier.decorator.ts b/server/src/common/decorators/normalize-identifier.decorator.ts new file mode 100644 index 000000000..03158f5c5 --- /dev/null +++ b/server/src/common/decorators/normalize-identifier.decorator.ts @@ -0,0 +1,15 @@ +import { Transform } from 'class-transformer'; +import { identifierInputKindForField, normalizeIdentifierInput } from '@bookorbit/types'; + +/** + * Cleans a hand-entered identifier before validation, using the rule the client applies as the + * user types, so `978-3426226223` is stored as `9783426226223` instead of failing `@MaxLength(13)`. + * The rule comes from the property name; non-strings pass through so `@IsString()` still rejects them. + */ +export function NormalizeIdentifier(): PropertyDecorator { + return (target, propertyKey) => { + const kind = identifierInputKindForField(String(propertyKey)); + if (!kind) throw new Error(`@NormalizeIdentifier() has no identifier rule for "${String(propertyKey)}"`); + Transform(({ value }: { value: unknown }) => (typeof value === 'string' ? normalizeIdentifierInput(kind, value) : value))(target, propertyKey); + }; +} diff --git a/server/src/modules/book-dock/book-dock.dto.test.ts b/server/src/modules/book-dock/book-dock.dto.test.ts index a32daee29..aad2e2b45 100644 --- a/server/src/modules/book-dock/book-dock.dto.test.ts +++ b/server/src/modules/book-dock/book-dock.dto.test.ts @@ -64,12 +64,12 @@ describe('BookDock DTO validation', () => { communityRatings: [{ provider: 'hardcover', rating: 4.5, ratingCount: 1000 }], googleBooksId: 'google-id', goodreadsId: 'goodreads-id', - amazonId: 'amazon-id', + amazonId: 'B00X47ZVXM', hardcoverId: 'hardcover-id', hardcoverEditionId: 'hardcover-edition-id', openLibraryId: 'OL1W', itunesId: 'itunes-id', - audibleId: 'audible-id', + audibleId: 'B002V0QK4C', librofmId: 'librofm-id', koboId: 'kobo-id', comicvineId: 'comicvine-id', @@ -154,4 +154,39 @@ describe('BookDock DTO validation', () => { (await errorsFor(BulkEditBookDockDto, { fields: { seriesIndex: 5.1 }, enabledFields: ['seriesIndex'], mergeArrays: false })).length, ).toBeGreaterThan(0); }); + + // Issue #1624: a hyphenated ISBN used to fail @MaxLength(13) instead of being saved. + it('UpdateBookDockFileDto cleans pasted identifiers through the production validation path', async () => { + const pipe = new ValidationPipe({ whitelist: true, forbidNonWhitelisted: true, transform: true }); + + await expect( + pipe.transform( + { + selectedMetadata: { + isbn13: 'ISBN-13 : 978-3426226223', + isbn10: '0 8044 2957 x', + amazonId: '\u200BB00X47ZVXM', + goodreadsId: ' 12345.The_Book\u200E', + }, + }, + { type: 'body', metatype: UpdateBookDockFileDto, data: undefined }, + ), + ).resolves.toMatchObject({ + selectedMetadata: { isbn13: '9783426226223', isbn10: '080442957X', amazonId: 'B00X47ZVXM', goodreadsId: '12345.The_Book' }, + }); + }); + + it('BulkEditBookDockDto cleans pasted ISBNs and still rejects ones too long to be an ISBN', async () => { + const valid = plainToInstance(BulkEditBookDockDto, { + fields: { isbn13: '978-3-426-22622-3', isbn10: '\uFEFF0-306-40615-2' }, + enabledFields: ['isbn13', 'isbn10'], + mergeArrays: false, + }); + expect(await validate(valid)).toHaveLength(0); + expect(valid.fields).toMatchObject({ isbn13: '9783426226223', isbn10: '0306406152' }); + + const tooLong = plainToInstance(BulkEditBookDockDto, { fields: { isbn13: '978-3426226223-9' }, enabledFields: ['isbn13'], mergeArrays: false }); + expect(tooLong.fields.isbn13).toBe('97834262262239'); + expect(await validate(tooLong)).toHaveLength(1); + }); }); diff --git a/server/src/modules/book-dock/dto/index.ts b/server/src/modules/book-dock/dto/index.ts index 4f72b73d8..e7cd2fc82 100644 --- a/server/src/modules/book-dock/dto/index.ts +++ b/server/src/modules/book-dock/dto/index.ts @@ -22,6 +22,7 @@ import { } from 'class-validator'; import { Type } from 'class-transformer'; import { MetadataProviderKey, SERIES_INDEX_MAX_LENGTH, SERIES_INDEX_PATTERN, type SeriesIndex } from '@bookorbit/types'; +import { NormalizeIdentifier } from '../../../common/decorators/normalize-identifier.decorator'; @ValidatorConstraint({ name: 'hasCompleteDefaultDestination', async: false }) class HasCompleteDefaultDestinationConstraint implements ValidatorConstraintInterface { @@ -86,8 +87,8 @@ export class BookDockMetadataFieldsDto { @IsOptional() @IsString() description?: string; @IsOptional() @IsString() @MaxLength(500) publisher?: string | null; @IsOptional() @IsString() @MaxLength(100) language?: string | null; - @IsOptional() @IsString() @MaxLength(10) isbn10?: string | null; - @IsOptional() @IsString() @MaxLength(13) isbn13?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(10) isbn10?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(13) isbn13?: string | null; @IsOptional() @IsString() @MaxLength(500) seriesName?: string | null; @IsOptional() @IsString() coverUrl?: string; @IsOptional() @IsString() @Matches(/^\d{4}-\d{2}-\d{2}$/) publishedDate?: string | null; @@ -110,20 +111,20 @@ export class BookDockMetadataFieldsDto { @ValidateNested({ each: true }) @Type(() => BookDockCommunityRatingDto) communityRatings?: BookDockCommunityRatingDto[] | null; - @IsOptional() @IsString() @MaxLength(50) googleBooksId?: string | null; - @IsOptional() @IsString() @MaxLength(50) goodreadsId?: string | null; - @IsOptional() @IsString() @MaxLength(20) amazonId?: string | null; - @IsOptional() @IsString() @MaxLength(255) hardcoverId?: string | null; - @IsOptional() @IsString() @MaxLength(50) hardcoverEditionId?: string | null; - @IsOptional() @IsString() @MaxLength(50) openLibraryId?: string | null; - @IsOptional() @IsString() @MaxLength(50) itunesId?: string | null; - @IsOptional() @IsString() @MaxLength(20) audibleId?: string | null; - @IsOptional() @IsString() @MaxLength(50) librofmId?: string | null; - @IsOptional() @IsString() @MaxLength(255) koboId?: string | null; - @IsOptional() @IsString() @MaxLength(50) comicvineId?: string | null; - @IsOptional() @IsString() @MaxLength(50) ranobedbId?: string | null; - @IsOptional() @IsString() @MaxLength(512) lubimyczytacId?: string | null; - @IsOptional() @IsString() @MaxLength(20) aladinId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(50) googleBooksId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(50) goodreadsId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(20) amazonId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(255) hardcoverId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(50) hardcoverEditionId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(50) openLibraryId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(50) itunesId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(20) audibleId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(50) librofmId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(255) koboId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(50) comicvineId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(50) ranobedbId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(512) lubimyczytacId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(20) aladinId?: string | null; @IsOptional() @ValidateNested() @Type(() => BookDockComicMetadataDto) comicMetadata?: BookDockComicMetadataDto | null; } diff --git a/server/src/modules/book/dto/book-dto-validation.test.ts b/server/src/modules/book/dto/book-dto-validation.test.ts index 41d53283b..96dd9896e 100644 --- a/server/src/modules/book/dto/book-dto-validation.test.ts +++ b/server/src/modules/book/dto/book-dto-validation.test.ts @@ -1,5 +1,6 @@ import 'reflect-metadata'; +import { ValidationPipe } from '@nestjs/common'; import { plainToInstance } from 'class-transformer'; import { validate } from 'class-validator'; import { MetadataProviderKey } from '@bookorbit/types'; @@ -22,6 +23,7 @@ import { SaveProgressDto } from './save-progress.dto'; import { SearchBooksDto } from './search-books.dto'; import { UpsertAudioProgressDto } from './upsert-audio-progress.dto'; import { UpdateBookMetadataDto } from './update-book-metadata.dto'; +import { UpdateBookMetadataAndLocksDto } from './update-book-metadata-and-locks.dto'; import { UpdateBookAddedAtDto } from './update-book-added-at.dto'; import { UpdatePersonalNoteDto } from './update-personal-note.dto'; import { UpdateRatingDto } from './update-rating.dto'; @@ -260,6 +262,54 @@ describe('Book DTO validation', () => { expect(Object.values(error?.constraints ?? {}).join(' ')).toBe('amazonId must be shorter than or equal to 20 characters'); }); + // Issue #1624: identifiers pasted from Amazon carry hyphens and invisible characters. + it('cleans pasted identifiers before validating them instead of rejecting them', async () => { + const pipe = new ValidationPipe({ whitelist: true, forbidNonWhitelisted: true, transform: true }); + + const result = (await pipe.transform( + { + metadata: { + isbn13: '978-3426226223', + isbn10: '\u200B0-8044-2957-x', + amazonId: '\u200BB00X47ZVXM', + audibleId: ' b002v0qk4c\u200E', + googleBooksId: '\u00A0zyTC-Al_FPjgYC\uFEFF', + hardcoverId: 'the-name-of-the-wind ', + }, + lockedFields: [], + }, + { type: 'body', metatype: UpdateBookMetadataAndLocksDto, data: undefined }, + )) as UpdateBookMetadataAndLocksDto; + + expect(result.metadata).toEqual({ + isbn13: '9783426226223', + isbn10: '080442957X', + amazonId: 'B00X47ZVXM', + audibleId: 'B002V0QK4C', + googleBooksId: 'zyTC-Al_FPjgYC', + hardcoverId: 'the-name-of-the-wind', + }); + }); + + it('rejects an identifier that is still too long once cleaned, without truncating it', async () => { + const dto = plainToInstance(UpdateBookMetadataDto, { isbn13: '978-3426226223-1', isbn10: '0-306-40615-22' }); + const errors = await validate(dto); + + expect(dto.isbn13).toBe('97834262262231'); + expect(dto.isbn10).toBe('03064061522'); + expect(errors.map((error) => error.property).sort()).toEqual(['isbn10', 'isbn13']); + }); + + it('leaves cleared and non-string identifiers to the existing validators', async () => { + const cleared = plainToInstance(UpdateBookMetadataDto, { isbn13: null, amazonId: '', goodreadsId: ' \u200B ' }); + expect(await validate(cleared)).toHaveLength(0); + expect(cleared).toMatchObject({ isbn13: null, amazonId: '', goodreadsId: '' }); + + const [error] = await errorsFor(UpdateBookMetadataDto, { isbn13: 9783426226223 }); + expect(error?.property).toBe('isbn13'); + expect(error?.constraints).toHaveProperty('isString'); + }); + it('normalizes a zero page count to null and validates other page count values', async () => { // A provider can return a page count of 0 (issue #329); it must normalize to null instead of failing validation. const zeroed = plainToInstance(UpdateBookMetadataDto, { pageCount: 0 }); diff --git a/server/src/modules/book/dto/update-book-metadata.dto.ts b/server/src/modules/book/dto/update-book-metadata.dto.ts index 4fc9368ca..f5ad17f44 100644 --- a/server/src/modules/book/dto/update-book-metadata.dto.ts +++ b/server/src/modules/book/dto/update-book-metadata.dto.ts @@ -3,6 +3,7 @@ import { Transform, Type } from 'class-transformer'; import { MetadataProviderKey, SERIES_INDEX_MAX_LENGTH, SERIES_INDEX_PATTERN, type SeriesIndex } from '@bookorbit/types'; import { MAX_SERIES_TOTAL_BOOKS } from '../../../common/utils/series-total-books.utils'; import { PROVIDER_ID_MAX_LENGTHS } from '../../../common/utils/provider-id.utils'; +import { NormalizeIdentifier } from '../../../common/decorators/normalize-identifier.decorator'; import { CustomMetadataValueDto } from '../../custom-metadata/dto/custom-metadata-value.dto'; export class AudiobookChapterDto { @@ -59,28 +60,28 @@ export class UpdateBookMetadataDto { @IsOptional() @IsString() @MaxLength(500) seriesName?: string | null; @IsOptional() @IsString() @MaxLength(SERIES_INDEX_MAX_LENGTH) @Matches(SERIES_INDEX_PATTERN) seriesIndex?: SeriesIndex | null; @IsOptional() @IsArray() @ValidateNested({ each: true }) @Type(() => BookSeriesMembershipDto) seriesMemberships?: BookSeriesMembershipDto[] | null; - @IsOptional() @IsString() @MaxLength(10) isbn10?: string | null; - @IsOptional() @IsString() @MaxLength(13) isbn13?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(10) isbn10?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(13) isbn13?: string | null; @IsOptional() @IsInt() @Min(1) @Max(5) rating?: number | null; @IsOptional() @IsArray() @ValidateNested({ each: true }) @Type(() => CommunityRatingDto) communityRatings?: CommunityRatingDto[] | null; @IsOptional() @IsArray() @IsString({ each: true }) authors?: string[]; @IsOptional() @IsArray() @IsString({ each: true }) genres?: string[]; @IsOptional() @IsArray() @IsString({ each: true }) tags?: string[]; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.googleBooksId) googleBooksId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.goodreadsId) goodreadsId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.amazonId) amazonId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.hardcoverId) hardcoverId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.hardcoverEditionId) hardcoverEditionId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.openLibraryId) openLibraryId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.itunesId) itunesId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.audibleId) audibleId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.librofmId) librofmId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.koboId) koboId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.googleBooksId) googleBooksId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.goodreadsId) goodreadsId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.amazonId) amazonId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.hardcoverId) hardcoverId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.hardcoverEditionId) hardcoverEditionId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.openLibraryId) openLibraryId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.itunesId) itunesId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.audibleId) audibleId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.librofmId) librofmId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.koboId) koboId?: string | null; @IsOptional() @ValidateNested() @Type(() => AudioMetadataDto) audioMetadata?: AudioMetadataDto; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.comicvineId) comicvineId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.ranobedbId) ranobedbId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.lubimyczytacId) lubimyczytacId?: string | null; - @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.aladinId) aladinId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.comicvineId) comicvineId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.ranobedbId) ranobedbId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.lubimyczytacId) lubimyczytacId?: string | null; + @NormalizeIdentifier() @IsOptional() @IsString() @MaxLength(PROVIDER_ID_MAX_LENGTHS.aladinId) aladinId?: string | null; @IsOptional() @ValidateNested() @Type(() => ComicMetadataDto) comicMetadata?: ComicMetadataDto; @IsOptional() @IsArray() @ValidateNested({ each: true }) @Type(() => CustomMetadataValueDto) customMetadata?: CustomMetadataValueDto[]; }