From 43a7c671d91837e7175e4a52b52611818e34f744 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Wed, 2 Sep 2026 00:48:55 +0000 Subject: [PATCH] fix(server): resolve numeric-prefixed Hardcover slugs Require the complete metadata value to be a valid GraphQL integer before using an ID lookup. Keep slug matching and sync change detection consistent through one parser. Fixes #1243 --- .../hardcover-book-match.service.test.ts | 37 +++++++++++++++++ .../hardcover/hardcover-book-match.service.ts | 5 ++- .../hardcover/hardcover-id.utils.test.ts | 21 ++++++++++ .../modules/hardcover/hardcover-id.utils.ts | 7 ++++ .../hardcover/hardcover-import.service.ts | 9 +--- .../hardcover/hardcover-sync.service.test.ts | 41 +++++++++++++++++++ .../hardcover/hardcover-sync.service.ts | 9 +--- 7 files changed, 113 insertions(+), 16 deletions(-) create mode 100644 server/src/modules/hardcover/hardcover-id.utils.test.ts create mode 100644 server/src/modules/hardcover/hardcover-id.utils.ts diff --git a/server/src/modules/hardcover/hardcover-book-match.service.test.ts b/server/src/modules/hardcover/hardcover-book-match.service.test.ts index 52661c7e4..0361c13e1 100644 --- a/server/src/modules/hardcover/hardcover-book-match.service.test.ts +++ b/server/src/modules/hardcover/hardcover-book-match.service.test.ts @@ -144,6 +144,7 @@ describe('HardcoverBookMatchService', () => { const result = await makeService().matchBook(1, 'tok', book); expect(result?.matchMethod).toBe('metadata_id'); expect(result?.hardcoverBookId).toBe(77); + expect(mockClient.query).toHaveBeenCalledWith(1, 'tok', expect.stringContaining('query FindBookById'), { id: 77 }); }); it('matches by metadata slug when hardcoverMetadataId is not numeric', async () => { @@ -168,6 +169,42 @@ describe('HardcoverBookMatchService', () => { ); }); + it('treats a digit-prefixed Hardcover slug as a slug instead of a partial numeric ID', async () => { + mockRepo.findBookState.mockResolvedValue(undefined); + mockClient.query.mockImplementation((_userId, _token, query: string) => { + if (query.includes('query FindBookById')) { + return { books: [{ id: 84, editions: [{ id: 24634818, pages: 284 }] }] }; + } + if (query.includes('query FindBookBySlug')) { + return { books: [{ id: 277100, editions: [{ id: 15528636, pages: 114, isbn_13: '9780140143508' }] }] }; + } + throw new Error('Unexpected query'); + }); + const book = { + ...baseBook, + title: '84, Charing Cross Road', + hardcoverMetadataId: '84-charing-cross-road', + isbn13: '9780140143508', + }; + + const result = await makeService().matchBook(1, 'tok', book); + + expect(result).toEqual({ + hardcoverBookId: 277100, + hardcoverEditionId: 15528636, + editionPages: 114, + editionAudioSeconds: null, + matchMethod: 'metadata_id', + }); + expect(mockClient.query).toHaveBeenCalledTimes(1); + expect(mockClient.query).toHaveBeenCalledWith(1, 'tok', expect.stringContaining('query FindBookBySlug'), { + slug: '84-charing-cross-road', + }); + expect(mockRepo.upsertBookState).toHaveBeenCalledWith( + expect.objectContaining({ hardcoverBookId: 277100, hardcoverEditionId: 15528636, matchMethod: 'metadata_id' }), + ); + }); + it('falls back to isbn13 when metadata_id fails', async () => { mockRepo.findBookState.mockResolvedValue(undefined); mockClient.query.mockRejectedValueOnce(new Error('not found')).mockResolvedValueOnce({ books: [{ id: 88, editions: [{ id: 11 }] }] }); diff --git a/server/src/modules/hardcover/hardcover-book-match.service.ts b/server/src/modules/hardcover/hardcover-book-match.service.ts index 21cfbe411..2424c2d90 100644 --- a/server/src/modules/hardcover/hardcover-book-match.service.ts +++ b/server/src/modules/hardcover/hardcover-book-match.service.ts @@ -3,6 +3,7 @@ import type { HardcoverEdition as HardcoverEditionSummary, HardcoverEditionsResu import { parsePublishedDateKey } from '../../common/utils/published-date.utils'; import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils'; +import { parseHardcoverBookId } from './hardcover-id.utils'; import type { BookSyncData } from './hardcover.repository'; import { HardcoverClientService } from './hardcover-client.service'; import { HardcoverRepository } from './hardcover.repository'; @@ -241,8 +242,8 @@ export class HardcoverBookMatchService { let match: HardcoverBookMatch | null = null; if (book.hardcoverMetadataId) { - const id = parseInt(book.hardcoverMetadataId, 10); - if (!isNaN(id)) { + const id = parseHardcoverBookId(book.hardcoverMetadataId); + if (id !== null) { match = await this.matchByHardcoverId(userId, token, id, book); } else { match = await this.matchByHardcoverSlug(userId, token, book.hardcoverMetadataId, book); diff --git a/server/src/modules/hardcover/hardcover-id.utils.test.ts b/server/src/modules/hardcover/hardcover-id.utils.test.ts new file mode 100644 index 000000000..cd4ac27d6 --- /dev/null +++ b/server/src/modules/hardcover/hardcover-id.utils.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest'; + +import { parseHardcoverBookId } from './hardcover-id.utils'; + +describe('parseHardcoverBookId', () => { + it.each([ + ['1', 1], + ['84', 84], + ['00084', 84], + ['2147483647', 2_147_483_647], + ])('parses the complete positive GraphQL integer %s', (value, expected) => { + expect(parseHardcoverBookId(value)).toBe(expected); + }); + + it.each([null, undefined, '', '0', '-84', '+84', '84.0', '84-charing-cross-road', '84abc', ' 84', '84 ', '2147483648', '9007199254740992'])( + 'rejects a value that is not a complete positive GraphQL integer: %s', + (value) => { + expect(parseHardcoverBookId(value)).toBeNull(); + }, + ); +}); diff --git a/server/src/modules/hardcover/hardcover-id.utils.ts b/server/src/modules/hardcover/hardcover-id.utils.ts new file mode 100644 index 000000000..f10404c86 --- /dev/null +++ b/server/src/modules/hardcover/hardcover-id.utils.ts @@ -0,0 +1,7 @@ +const GRAPHQL_INT_MAX = 2_147_483_647; + +export function parseHardcoverBookId(value: string | null | undefined): number | null { + if (!value || !/^\d+$/.test(value)) return null; + const id = Number(value); + return Number.isSafeInteger(id) && id > 0 && id <= GRAPHQL_INT_MAX ? id : null; +} diff --git a/server/src/modules/hardcover/hardcover-import.service.ts b/server/src/modules/hardcover/hardcover-import.service.ts index 07bfc6a1e..2b319fa71 100644 --- a/server/src/modules/hardcover/hardcover-import.service.ts +++ b/server/src/modules/hardcover/hardcover-import.service.ts @@ -18,6 +18,7 @@ import { UserBookStatusService } from '../user-book-status/user-book-status.serv import { ReadingAttemptService } from '../user-book-status/reading-attempt.service'; import { HARDCOVER_STATUS } from './hardcover.constants'; import { HardcoverClientService } from './hardcover-client.service'; +import { parseHardcoverBookId } from './hardcover-id.utils'; import { HardcoverImportFuzzyIndex } from './hardcover-import-fuzzy-index'; import { type HardcoverImportLocalBook, HardcoverRepository } from './hardcover.repository'; import { HardcoverSettingsService } from './hardcover-settings.service'; @@ -317,7 +318,7 @@ export class HardcoverImportService { for (const book of books) { const metadataId = book.hardcoverMetadataId?.trim(); if (metadataId) { - const numericId = parseNumericId(metadataId); + const numericId = parseHardcoverBookId(metadataId); if (numericId != null) pushUnique(indexes.byHardcoverId, numericId, book); else pushUnique(indexes.byHardcoverSlug, normalizeSlug(metadataId), book); } @@ -617,12 +618,6 @@ function pushUnique(map: Map, key: K, book: Ha if (!current.some((candidate) => candidate.bookId === book.bookId)) current.push(book); } -function parseNumericId(value: string): number | null { - if (!/^\d+$/.test(value)) return null; - const id = Number(value); - return Number.isSafeInteger(id) ? id : null; -} - function normalizeIsbn(value: string | null | undefined): string | null { if (!value) return null; const normalized = value.replace(/[^0-9X]/gi, '').toUpperCase(); diff --git a/server/src/modules/hardcover/hardcover-sync.service.test.ts b/server/src/modules/hardcover/hardcover-sync.service.test.ts index 43f5eee24..9cbd3a267 100644 --- a/server/src/modules/hardcover/hardcover-sync.service.test.ts +++ b/server/src/modules/hardcover/hardcover-sync.service.test.ts @@ -304,6 +304,47 @@ describe('HardcoverSyncService', () => { expect(mockClient.query).not.toHaveBeenCalled(); }); + it('does not treat a digit-prefixed Hardcover slug as a changed numeric book id', async () => { + mockSettingsService.getTokenForUser.mockResolvedValue('tok'); + mockRepo.findBookSyncData.mockResolvedValue({ ...readingBook, hardcoverMetadataId: '84-charing-cross-road' }); + mockRepo.findBookState.mockResolvedValue({ + hardcoverBookId: 277100, + lastSyncedAt: new Date('2024-02-01T00:00:00Z'), + lastSyncedStatus: 'reading', + lastSyncedProgress: 42, + lastSyncedRating: null, + lastSyncedStartedAt: '2024-01-01', + lastSyncedFinishedAt: null, + }); + + await expect(makeService().syncBook(1, 1)).resolves.toBe('skipped'); + + expect(mockMatchService.matchBook).not.toHaveBeenCalled(); + expect(mockClient.query).not.toHaveBeenCalled(); + }); + + it('re-syncs when a complete numeric Hardcover metadata id changes', async () => { + mockSettingsService.getTokenForUser.mockResolvedValue('tok'); + const changedBook = { ...readingBook, hardcoverMetadataId: '84' }; + mockRepo.findBookSyncData.mockResolvedValue(changedBook); + mockRepo.findBookState.mockResolvedValue({ + hardcoverBookId: 277100, + lastSyncedAt: new Date('2024-02-01T00:00:00Z'), + lastSyncedStatus: 'reading', + lastSyncedProgress: 42, + lastSyncedRating: null, + lastSyncedStartedAt: '2024-01-01', + lastSyncedFinishedAt: null, + }); + mockMatchService.matchBook.mockResolvedValue(null); + const warnSpy = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined); + + await expect(makeService().syncBook(1, 1)).resolves.toBe('skipped'); + + expect(mockMatchService.matchBook).toHaveBeenCalledWith(1, 'tok', changedBook); + warnSpy.mockRestore(); + }); + it('stores no_match error when match fails', async () => { const warnSpy = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined); mockSettingsService.getTokenForUser.mockResolvedValue('tok'); diff --git a/server/src/modules/hardcover/hardcover-sync.service.ts b/server/src/modules/hardcover/hardcover-sync.service.ts index ef7484010..b4a211b09 100644 --- a/server/src/modules/hardcover/hardcover-sync.service.ts +++ b/server/src/modules/hardcover/hardcover-sync.service.ts @@ -19,6 +19,7 @@ import { BookService } from '../book/book.service'; import { HARDCOVER_STATUS } from './hardcover.constants'; import { HardcoverBookMatchService } from './hardcover-book-match.service'; import { HardcoverClientService } from './hardcover-client.service'; +import { parseHardcoverBookId } from './hardcover-id.utils'; import { attemptOwnedReadId, type HardcoverReadCandidate, @@ -840,7 +841,7 @@ export class HardcoverSyncService { if (!state?.lastSyncedAt) return true; if (book.attemptsUpdatedAt && book.attemptsUpdatedAt > state.lastSyncedAt) return true; if (book.hardcoverMetadataId && state.syncError === 'no_match') return true; - const metadataHardcoverId = this.parseNumericHardcoverMetadataId(book.hardcoverMetadataId); + const metadataHardcoverId = parseHardcoverBookId(book.hardcoverMetadataId); if (metadataHardcoverId !== null && metadataHardcoverId !== state.hardcoverBookId) return true; if (book.status !== state.lastSyncedStatus) return true; if (book.progress !== state.lastSyncedProgress) return true; @@ -852,12 +853,6 @@ export class HardcoverSyncService { return false; } - private parseNumericHardcoverMetadataId(value: string | null | undefined): number | null { - if (!value) return null; - const id = parseInt(value, 10); - return isNaN(id) ? null : id; - } - private buildAttemptSnapshot(book: BookSyncData) { return { lastSyncedAt: new Date(),