mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(metadata): stop Hardcover sync following a stale cached match
A cached Hardcover match outranked a corrected Hardcover ID or ISBN, so progress kept going to the wrong book. Record the identifiers each link came from and re-check it when they change, honour the metadata edition on a match, and release the old book's read links so they are never reused. Closes #1488
This commit is contained in:
@@ -0,0 +1 @@
|
||||
ALTER TABLE "hardcover_book_state" ADD COLUMN "match_basis" text;
|
||||
File diff suppressed because it is too large
Load Diff
@@ -764,6 +764,13 @@
|
||||
"when": 1791684796856,
|
||||
"tag": "0108_add_read_aloud_delivery",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 109,
|
||||
"version": "7",
|
||||
"when": 1791821546946,
|
||||
"tag": "0109_add_hardcover_match_basis",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -39,6 +39,8 @@ export const hardcoverBookState = pgTable(
|
||||
hardcoverUserBookId: integer('hardcover_user_book_id'),
|
||||
hardcoverReadId: integer('hardcover_read_id'),
|
||||
matchMethod: varchar('match_method', { length: 20 }),
|
||||
// Identifiers the current link was resolved from; null on links made before it was recorded.
|
||||
matchBasis: text('match_basis'),
|
||||
matchError: text('match_error'),
|
||||
lastSyncedAt: timestamp('last_synced_at', { withTimezone: true }),
|
||||
lastSyncedStatus: varchar('last_synced_status', { length: 20 }),
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
|
||||
import { EDITION_DISPLAY_LIMIT, HardcoverBookMatchService } from './hardcover-book-match.service';
|
||||
import { buildHardcoverMatchBasis } from './hardcover-id.utils';
|
||||
|
||||
const mockRepo = {
|
||||
findBookState: vi.fn(),
|
||||
upsertBookState: vi.fn(),
|
||||
relinkBook: vi.fn(),
|
||||
};
|
||||
|
||||
const mockClient = {
|
||||
@@ -31,10 +33,14 @@ const baseBook = {
|
||||
format: null,
|
||||
};
|
||||
|
||||
// A link whose identifiers are unchanged since it was made, so the cached path runs untouched.
|
||||
const BASE_BASIS = buildHardcoverMatchBasis(baseBook);
|
||||
|
||||
describe('HardcoverBookMatchService', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockRepo.upsertBookState.mockResolvedValue({});
|
||||
mockRepo.relinkBook.mockResolvedValue(0);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -46,6 +52,7 @@ describe('HardcoverBookMatchService', () => {
|
||||
hardcoverBookId: 100,
|
||||
hardcoverEditionId: 200,
|
||||
matchError: null,
|
||||
matchBasis: BASE_BASIS,
|
||||
});
|
||||
mockClient.query.mockResolvedValue({
|
||||
books: [{ id: 100, editions: [{ id: 200, pages: 320 }] }],
|
||||
@@ -67,6 +74,7 @@ describe('HardcoverBookMatchService', () => {
|
||||
hardcoverBookId: 100,
|
||||
hardcoverEditionId: 200,
|
||||
matchError: null,
|
||||
matchBasis: BASE_BASIS,
|
||||
});
|
||||
mockClient.query.mockResolvedValue({
|
||||
books: [
|
||||
@@ -97,6 +105,7 @@ describe('HardcoverBookMatchService', () => {
|
||||
hardcoverBookId: 100,
|
||||
hardcoverEditionId: 200,
|
||||
matchError: null,
|
||||
matchBasis: BASE_BASIS,
|
||||
});
|
||||
mockClient.query.mockResolvedValue({
|
||||
books: [{ id: 100, editions: [{ id: 200, pages: null, audio_seconds: 3600 }] }],
|
||||
@@ -116,6 +125,7 @@ describe('HardcoverBookMatchService', () => {
|
||||
hardcoverBookId: 100,
|
||||
hardcoverEditionId: 200,
|
||||
matchError: null,
|
||||
matchBasis: BASE_BASIS,
|
||||
});
|
||||
mockClient.query.mockResolvedValue({
|
||||
books: [{ id: 100, editions: [{ id: 301, pages: 410, isbn_13: '9781234567890', isbn_10: null, audio_seconds: null }] }],
|
||||
@@ -134,6 +144,7 @@ describe('HardcoverBookMatchService', () => {
|
||||
hardcoverBookId: 100,
|
||||
hardcoverEditionId: 9999,
|
||||
matchError: null,
|
||||
matchBasis: BASE_BASIS,
|
||||
});
|
||||
const cappedWindow = Array.from({ length: 50 }, (_, index) => ({
|
||||
id: index + 1,
|
||||
@@ -168,6 +179,7 @@ describe('HardcoverBookMatchService', () => {
|
||||
hardcoverBookId: 100,
|
||||
hardcoverEditionId: 200,
|
||||
matchError: null,
|
||||
matchBasis: BASE_BASIS,
|
||||
});
|
||||
mockClient.query.mockImplementation((_userId, _token, query: string) => {
|
||||
if (query.includes('FindBookEditionMetricsById')) {
|
||||
@@ -193,6 +205,7 @@ describe('HardcoverBookMatchService', () => {
|
||||
hardcoverBookId: 100,
|
||||
hardcoverEditionId: null,
|
||||
matchError: null,
|
||||
matchBasis: BASE_BASIS,
|
||||
});
|
||||
mockClient.query.mockResolvedValue({
|
||||
books: [{ id: 100, editions: [{ id: 301, pages: 410, isbn_13: null, isbn_10: null, audio_seconds: null }] }],
|
||||
@@ -444,6 +457,364 @@ describe('HardcoverBookMatchService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('records the identifiers a new link was resolved from', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue(undefined);
|
||||
mockClient.query.mockResolvedValue({ books: [{ id: 5, editions: [{ id: 6 }] }] });
|
||||
|
||||
await makeService().matchBook(1, 'tok', baseBook);
|
||||
|
||||
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ hardcoverBookId: 5, matchBasis: BASE_BASIS }));
|
||||
});
|
||||
|
||||
it('records no basis when nothing matched', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue(undefined);
|
||||
mockClient.query.mockResolvedValue({ books: [] });
|
||||
|
||||
await makeService().matchBook(1, 'tok', baseBook);
|
||||
|
||||
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ matchError: 'no_match', matchBasis: null }));
|
||||
});
|
||||
|
||||
describe('when the identifiers change after a link was made', () => {
|
||||
const WRONG_BOOK = 1554702;
|
||||
const WRONG_EDITION = 31616496;
|
||||
const RIGHT_BOOK = 2973670;
|
||||
const RIGHT_EDITION = 33314466;
|
||||
const staleBook = { ...baseBook, title: 'ok, amen', hardcoverMetadataId: 'hell-be-ok-growing-gorgeous-boys-into-good-men', isbn13: null };
|
||||
const correctedBook = {
|
||||
...staleBook,
|
||||
hardcoverMetadataId: 'ok-amen',
|
||||
hardcoverEditionId: String(RIGHT_EDITION),
|
||||
isbn13: '9789146223856',
|
||||
format: 'epub',
|
||||
};
|
||||
const rightBookAnswer = {
|
||||
books: [{ id: RIGHT_BOOK, editions: [{ id: RIGHT_EDITION, pages: 321, isbn_13: '9789146223856', isbn_10: null, audio_seconds: null }] }],
|
||||
};
|
||||
const wrongBookMetrics = {
|
||||
books: [{ id: WRONG_BOOK, editions: [{ id: WRONG_EDITION, pages: 217, isbn_13: null, isbn_10: null, audio_seconds: null }] }],
|
||||
};
|
||||
|
||||
function answer(routes: Record<string, unknown>) {
|
||||
mockClient.query.mockImplementation((_userId: number, _token: string, query: string) => {
|
||||
const route = Object.keys(routes).find((name) => query.includes(name));
|
||||
if (!route) return Promise.reject(new Error(`unexpected query ${query.trim().split('(')[0]}`));
|
||||
const value = routes[route];
|
||||
return value instanceof Error ? Promise.reject(value) : Promise.resolve(value);
|
||||
});
|
||||
}
|
||||
|
||||
function queryNames(): string[] {
|
||||
return mockClient.query.mock.calls.map((call) => /query (\w+)/.exec(call[2] as string)?.[1] ?? '');
|
||||
}
|
||||
|
||||
function silenceWarnings(service: HardcoverBookMatchService) {
|
||||
const logger = Reflect.get(service, 'logger') as { warn: (message: string) => void; log: (message: string) => void };
|
||||
vi.spyOn(logger, 'warn').mockImplementation(() => undefined);
|
||||
vi.spyOn(logger, 'log').mockImplementation(() => undefined);
|
||||
return logger;
|
||||
}
|
||||
|
||||
it('moves the link to the book the corrected metadata identifies (issue 1488)', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue({
|
||||
hardcoverBookId: WRONG_BOOK,
|
||||
hardcoverEditionId: WRONG_EDITION,
|
||||
matchError: null,
|
||||
matchBasis: buildHardcoverMatchBasis(staleBook),
|
||||
});
|
||||
answer({ 'query FindBookBySlug': rightBookAnswer });
|
||||
const service = makeService();
|
||||
const logger = silenceWarnings(service);
|
||||
|
||||
const result = await service.matchBook(1, 'tok', correctedBook);
|
||||
|
||||
expect(result).toEqual({
|
||||
hardcoverBookId: RIGHT_BOOK,
|
||||
hardcoverEditionId: RIGHT_EDITION,
|
||||
editionPages: 321,
|
||||
editionAudioSeconds: null,
|
||||
matchMethod: 'metadata_id',
|
||||
replacedHardcoverBookId: WRONG_BOOK,
|
||||
});
|
||||
expect(mockRepo.relinkBook).toHaveBeenCalledWith({
|
||||
userId: 1,
|
||||
bookId: 42,
|
||||
hardcoverBookId: RIGHT_BOOK,
|
||||
hardcoverEditionId: RIGHT_EDITION,
|
||||
matchMethod: 'metadata_id',
|
||||
matchBasis: buildHardcoverMatchBasis(correctedBook),
|
||||
});
|
||||
expect(queryNames()).toEqual(['FindBookBySlug']);
|
||||
expect(mockRepo.upsertBookState).not.toHaveBeenCalled();
|
||||
expect(logger.log).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
`[hardcover.book_relink] [end] userId=1 bookId=42 previousHardcoverBookId=${WRONG_BOOK} hardcoverBookId=${RIGHT_BOOK}`,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it('heals a link recorded before its basis existed when the Hardcover ID disagrees with it', async () => {
|
||||
// The reporter's row: the corrected edition was written onto the stale book by the metadata save.
|
||||
mockRepo.findBookState.mockResolvedValue({
|
||||
hardcoverBookId: WRONG_BOOK,
|
||||
hardcoverEditionId: RIGHT_EDITION,
|
||||
matchError: null,
|
||||
matchBasis: null,
|
||||
});
|
||||
answer({ 'query FindBookBySlug': rightBookAnswer });
|
||||
const service = makeService();
|
||||
silenceWarnings(service);
|
||||
const result = await service.matchBook(1, 'tok', correctedBook);
|
||||
|
||||
expect(result).toMatchObject({ hardcoverBookId: RIGHT_BOOK, hardcoverEditionId: RIGHT_EDITION, replacedHardcoverBookId: WRONG_BOOK });
|
||||
expect(mockRepo.relinkBook).toHaveBeenCalledWith(expect.objectContaining({ hardcoverBookId: RIGHT_BOOK, hardcoverEditionId: RIGHT_EDITION }));
|
||||
});
|
||||
|
||||
it('lets only the Hardcover ID move a link recorded before its basis existed', async () => {
|
||||
// Import may have confirmed such a link against an ISBN that disagrees with it.
|
||||
const isbnOnlyBook = { ...baseBook, isbn13: '9789146223856' };
|
||||
mockRepo.findBookState.mockResolvedValue({
|
||||
hardcoverBookId: WRONG_BOOK,
|
||||
hardcoverEditionId: WRONG_EDITION,
|
||||
matchError: null,
|
||||
matchBasis: null,
|
||||
});
|
||||
answer({ FindBookEditionMetricsById: wrongBookMetrics });
|
||||
|
||||
const result = await makeService().matchBook(1, 'tok', isbnOnlyBook);
|
||||
|
||||
expect(result).toMatchObject({ hardcoverBookId: WRONG_BOOK, hardcoverEditionId: WRONG_EDITION, matchMethod: 'cached' });
|
||||
expect(queryNames()).toEqual(['FindBookEditionMetricsById']);
|
||||
expect(mockRepo.upsertBookState).toHaveBeenCalledWith({ userId: 1, bookId: 42, matchBasis: buildHardcoverMatchBasis(isbnOnlyBook) });
|
||||
expect(mockRepo.relinkBook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('moves a recorded link when a changed ISBN resolves to another book', async () => {
|
||||
const before = { ...baseBook, isbn13: '9780000000001' };
|
||||
const after = { ...baseBook, isbn13: '9789146223856' };
|
||||
mockRepo.findBookState.mockResolvedValue({
|
||||
hardcoverBookId: WRONG_BOOK,
|
||||
hardcoverEditionId: WRONG_EDITION,
|
||||
matchError: null,
|
||||
matchBasis: buildHardcoverMatchBasis(before),
|
||||
});
|
||||
answer({ FindBookByISBN13: rightBookAnswer });
|
||||
const service = makeService();
|
||||
silenceWarnings(service);
|
||||
|
||||
const result = await service.matchBook(1, 'tok', after);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
hardcoverBookId: RIGHT_BOOK,
|
||||
hardcoverEditionId: RIGHT_EDITION,
|
||||
matchMethod: 'isbn',
|
||||
replacedHardcoverBookId: WRONG_BOOK,
|
||||
});
|
||||
expect(mockRepo.relinkBook).toHaveBeenCalledWith(expect.objectContaining({ matchMethod: 'isbn', matchBasis: buildHardcoverMatchBasis(after) }));
|
||||
});
|
||||
|
||||
it("keeps the link and the user's edition when the changed identifiers resolve to the same book", async () => {
|
||||
const before = { ...baseBook, hardcoverMetadataId: '100', isbn13: '9780000000001' };
|
||||
const after = { ...before, isbn13: '9780000000002' };
|
||||
mockRepo.findBookState.mockResolvedValue({
|
||||
hardcoverBookId: 100,
|
||||
hardcoverEditionId: 200,
|
||||
matchError: null,
|
||||
matchBasis: buildHardcoverMatchBasis(before),
|
||||
});
|
||||
answer({
|
||||
'query FindBookById': {
|
||||
books: [{ id: 100, editions: [{ id: 201, pages: 300, isbn_13: '9780000000002', isbn_10: null, audio_seconds: null }] }],
|
||||
},
|
||||
FindBookEditionMetricsById: { books: [{ id: 100, editions: [{ id: 200, pages: 320, isbn_13: null, isbn_10: null, audio_seconds: null }] }] },
|
||||
});
|
||||
|
||||
const result = await makeService().matchBook(1, 'tok', after);
|
||||
|
||||
expect(result).toEqual({ hardcoverBookId: 100, hardcoverEditionId: 200, editionPages: 320, editionAudioSeconds: null, matchMethod: 'cached' });
|
||||
// The Hardcover ID outranks the ISBN, so the ISBN is never looked up.
|
||||
expect(queryNames()).toEqual(['FindBookById', 'FindBookEditionMetricsById']);
|
||||
expect(mockRepo.upsertBookState).toHaveBeenCalledWith({ userId: 1, bookId: 42, matchBasis: buildHardcoverMatchBasis(after) });
|
||||
expect(mockRepo.relinkBook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps the link when Hardcover knows none of the corrected identifiers, and settles the check', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue({
|
||||
hardcoverBookId: WRONG_BOOK,
|
||||
hardcoverEditionId: WRONG_EDITION,
|
||||
matchError: null,
|
||||
matchBasis: buildHardcoverMatchBasis(staleBook),
|
||||
});
|
||||
answer({ 'query FindBookBySlug': { books: [] }, FindBookByISBN13: { books: [] }, FindBookEditionMetricsById: wrongBookMetrics });
|
||||
|
||||
const result = await makeService().matchBook(1, 'tok', correctedBook);
|
||||
|
||||
expect(result).toMatchObject({ hardcoverBookId: WRONG_BOOK, matchMethod: 'cached' });
|
||||
expect(queryNames()).toEqual(['FindBookBySlug', 'FindBookByISBN13', 'FindBookEditionMetricsById']);
|
||||
expect(mockRepo.upsertBookState).toHaveBeenCalledWith({ userId: 1, bookId: 42, matchBasis: buildHardcoverMatchBasis(correctedBook) });
|
||||
expect(mockRepo.relinkBook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls through to the ISBN when Hardcover does not know the corrected Hardcover ID', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue({
|
||||
hardcoverBookId: WRONG_BOOK,
|
||||
hardcoverEditionId: WRONG_EDITION,
|
||||
matchError: null,
|
||||
matchBasis: buildHardcoverMatchBasis(staleBook),
|
||||
});
|
||||
answer({ 'query FindBookBySlug': { books: [] }, FindBookByISBN13: rightBookAnswer });
|
||||
const service = makeService();
|
||||
silenceWarnings(service);
|
||||
|
||||
const result = await service.matchBook(1, 'tok', correctedBook);
|
||||
|
||||
expect(result).toMatchObject({ hardcoverBookId: RIGHT_BOOK, matchMethod: 'isbn', replacedHardcoverBookId: WRONG_BOOK });
|
||||
});
|
||||
|
||||
it('retries on a later sync instead of letting a weaker identifier decide when a lookup fails', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue({
|
||||
hardcoverBookId: WRONG_BOOK,
|
||||
hardcoverEditionId: WRONG_EDITION,
|
||||
matchError: null,
|
||||
matchBasis: buildHardcoverMatchBasis(staleBook),
|
||||
});
|
||||
answer({
|
||||
'query FindBookBySlug': new Error('Hardcover unavailable'),
|
||||
FindBookByISBN13: rightBookAnswer,
|
||||
FindBookEditionMetricsById: wrongBookMetrics,
|
||||
});
|
||||
const service = makeService();
|
||||
silenceWarnings(service);
|
||||
|
||||
const result = await service.matchBook(1, 'tok', correctedBook);
|
||||
|
||||
expect(result).toMatchObject({ hardcoverBookId: WRONG_BOOK, matchMethod: 'cached' });
|
||||
expect(queryNames()).toEqual(['FindBookBySlug', 'FindBookEditionMetricsById']);
|
||||
expect(mockRepo.upsertBookState).not.toHaveBeenCalled();
|
||||
expect(mockRepo.relinkBook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('never lets a title search move a link', async () => {
|
||||
const before = { ...baseBook, isbn13: '9780000000001' };
|
||||
const cleared = { ...baseBook, isbn13: null };
|
||||
mockRepo.findBookState.mockResolvedValue({
|
||||
hardcoverBookId: 100,
|
||||
hardcoverEditionId: 200,
|
||||
matchError: null,
|
||||
matchBasis: buildHardcoverMatchBasis(before),
|
||||
});
|
||||
answer({ FindBookEditionMetricsById: { books: [{ id: 100, editions: [{ id: 200, pages: 320 }] }] } });
|
||||
|
||||
const result = await makeService().matchBook(1, 'tok', cleared);
|
||||
|
||||
expect(result).toMatchObject({ hardcoverBookId: 100, hardcoverEditionId: 200, matchMethod: 'cached' });
|
||||
expect(queryNames()).toEqual(['FindBookEditionMetricsById']);
|
||||
expect(mockRepo.relinkBook).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('metadata edition', () => {
|
||||
const editions = [
|
||||
{ id: 900, pages: null, isbn_13: null, isbn_10: null, audio_seconds: 36000 },
|
||||
{ id: 901, pages: 512, isbn_13: '9781234567890', isbn_10: null, audio_seconds: null },
|
||||
{ id: 902, pages: 300, isbn_13: null, isbn_10: null, audio_seconds: null },
|
||||
];
|
||||
|
||||
it('wins over the best-guess edition on a fresh Hardcover ID match', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue(undefined);
|
||||
mockClient.query.mockResolvedValue({ books: [{ id: 700, editions }] });
|
||||
|
||||
const result = await makeService().matchBook(1, 'tok', { ...baseBook, hardcoverMetadataId: '700', hardcoverEditionId: '902', format: 'epub' });
|
||||
|
||||
expect(result).toEqual({
|
||||
hardcoverBookId: 700,
|
||||
hardcoverEditionId: 902,
|
||||
editionPages: 300,
|
||||
editionAudioSeconds: null,
|
||||
matchMethod: 'metadata_id',
|
||||
});
|
||||
expect(mockClient.query).toHaveBeenCalledTimes(1);
|
||||
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ hardcoverBookId: 700, hardcoverEditionId: 902 }));
|
||||
});
|
||||
|
||||
it('is looked up directly when it sits past the capped edition list', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue(undefined);
|
||||
mockClient.query.mockImplementation((_userId, _token, query: string) =>
|
||||
Promise.resolve(
|
||||
query.includes('FindBookEditionMetricsById')
|
||||
? { books: [{ id: 700, editions: [{ id: 9999, pages: 250, isbn_13: null, isbn_10: null, audio_seconds: null }] }] }
|
||||
: { books: [{ id: 700, editions }] },
|
||||
),
|
||||
);
|
||||
|
||||
const result = await makeService().matchBook(1, 'tok', { ...baseBook, hardcoverMetadataId: '700', hardcoverEditionId: '9999' });
|
||||
|
||||
expect(result).toMatchObject({ hardcoverEditionId: 9999, editionPages: 250 });
|
||||
expect(mockClient.query).toHaveBeenCalledWith(1, 'tok', expect.stringContaining('FindBookEditionMetricsById'), { id: 700, editionId: 9999 });
|
||||
});
|
||||
|
||||
it('is ignored when it belongs to another book', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue(undefined);
|
||||
mockClient.query.mockImplementation((_userId, _token, query: string) =>
|
||||
Promise.resolve(query.includes('FindBookEditionMetricsById') ? { books: [{ id: 700, editions: [] }] } : { books: [{ id: 700, editions }] }),
|
||||
);
|
||||
|
||||
const result = await makeService().matchBook(1, 'tok', { ...baseBook, hardcoverMetadataId: '700', hardcoverEditionId: '31616496' });
|
||||
|
||||
expect(result).toMatchObject({ hardcoverBookId: 700, hardcoverEditionId: 901 });
|
||||
});
|
||||
|
||||
it("wins over the ISBN's own edition on an ISBN match", async () => {
|
||||
mockRepo.findBookState.mockResolvedValue(undefined);
|
||||
mockClient.query.mockImplementation((_userId, _token, query: string) =>
|
||||
Promise.resolve(
|
||||
query.includes('FindBookEditionMetricsById')
|
||||
? { books: [{ id: 88, editions: [{ id: 12, pages: 350, isbn_13: null, isbn_10: null, audio_seconds: null }] }] }
|
||||
: { books: [{ id: 88, editions: [{ id: 11, pages: 200 }] }] },
|
||||
),
|
||||
);
|
||||
|
||||
const result = await makeService().matchBook(1, 'tok', { ...baseBook, hardcoverEditionId: '12' });
|
||||
|
||||
expect(result).toEqual({ hardcoverBookId: 88, hardcoverEditionId: 12, editionPages: 350, editionAudioSeconds: null, matchMethod: 'isbn' });
|
||||
});
|
||||
|
||||
it('falls back without failing the match when its lookup fails', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue(undefined);
|
||||
mockClient.query.mockImplementation((_userId, _token, query: string) =>
|
||||
query.includes('FindBookEditionMetricsById') ? Promise.reject(new Error('boom')) : Promise.resolve({ books: [{ id: 700, editions }] }),
|
||||
);
|
||||
const service = makeService();
|
||||
const warn = vi.spyOn(Reflect.get(service, 'logger') as { warn: (message: string) => void }, 'warn').mockImplementation(() => undefined);
|
||||
|
||||
const result = await service.matchBook(1, 'tok', { ...baseBook, hardcoverMetadataId: '700', hardcoverEditionId: '9999' });
|
||||
|
||||
expect(result).toMatchObject({ hardcoverBookId: 700, hardcoverEditionId: 901, matchMethod: 'metadata_id' });
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('method=metadata_edition'));
|
||||
});
|
||||
|
||||
it('fills a cached link that has no edition yet', async () => {
|
||||
mockRepo.findBookState.mockResolvedValue({ hardcoverBookId: 700, hardcoverEditionId: null, matchError: null, matchBasis: BASE_BASIS });
|
||||
mockClient.query.mockResolvedValue({ books: [{ id: 700, editions }] });
|
||||
|
||||
const result = await makeService().matchBook(1, 'tok', { ...baseBook, hardcoverEditionId: '902' });
|
||||
|
||||
expect(result).toMatchObject({ hardcoverBookId: 700, hardcoverEditionId: 902, matchMethod: 'cached' });
|
||||
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ hardcoverEditionId: 902, matchMethod: 'cached' }));
|
||||
});
|
||||
|
||||
it("leaves a user's cached edition alone", async () => {
|
||||
mockRepo.findBookState.mockResolvedValue({ hardcoverBookId: 700, hardcoverEditionId: 901, matchError: null, matchBasis: BASE_BASIS });
|
||||
mockClient.query.mockResolvedValue({ books: [{ id: 700, editions: [editions[1]] }] });
|
||||
|
||||
const result = await makeService().matchBook(1, 'tok', { ...baseBook, hardcoverEditionId: '902' });
|
||||
|
||||
expect(result).toMatchObject({ hardcoverEditionId: 901, matchMethod: 'cached' });
|
||||
expect(mockClient.query).toHaveBeenCalledTimes(1);
|
||||
expect(mockRepo.upsertBookState).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('listEditions', () => {
|
||||
it('maps editions for display, deriving a format label and published date', async () => {
|
||||
mockClient.query.mockResolvedValue({
|
||||
|
||||
@@ -3,7 +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 { buildHardcoverMatchBasis, parseHardcoverBookId, parseHardcoverEditionId } from './hardcover-id.utils';
|
||||
import type { BookSyncData } from './hardcover.repository';
|
||||
import { HardcoverClientService } from './hardcover-client.service';
|
||||
import { HardcoverRepository } from './hardcover.repository';
|
||||
@@ -14,6 +14,15 @@ export interface HardcoverBookMatch {
|
||||
editionPages: number | null;
|
||||
editionAudioSeconds: number | null;
|
||||
matchMethod: 'isbn' | 'title' | 'cached' | 'metadata_id';
|
||||
// Set when this match moved the link off another Hardcover book, whose read ids no longer apply.
|
||||
replacedHardcoverBookId?: number;
|
||||
}
|
||||
|
||||
// `failed` tells a lookup that could not run apart from one Hardcover answered with no book: only
|
||||
// the second may settle a re-check, the first has to be retried.
|
||||
interface HardcoverLookup {
|
||||
match: HardcoverBookMatch | null;
|
||||
failed: boolean;
|
||||
}
|
||||
|
||||
const FIND_BOOK_BY_ISBN13_QUERY = `
|
||||
@@ -229,6 +238,12 @@ export class HardcoverBookMatchService {
|
||||
async matchBook(userId: number, token: string, book: BookSyncData): Promise<HardcoverBookMatch | null> {
|
||||
const cached = await this.repo.findBookState(userId, book.bookId);
|
||||
if (cached?.hardcoverBookId && !cached.matchError) {
|
||||
const matchBasis = buildHardcoverMatchBasis(book);
|
||||
if (cached.matchBasis !== matchBasis) {
|
||||
const relinked = await this.relinkIfIdentifiersMoved(userId, token, book, cached.hardcoverBookId, cached.matchBasis ?? null, matchBasis);
|
||||
if (relinked) return relinked;
|
||||
}
|
||||
|
||||
const cachedMatch = await this.resolveCachedMatch(userId, token, book, cached.hardcoverBookId, cached.hardcoverEditionId ?? null);
|
||||
|
||||
if ((cached.hardcoverEditionId ?? null) !== cachedMatch.hardcoverEditionId) {
|
||||
@@ -254,20 +269,15 @@ export class HardcoverBookMatchService {
|
||||
let match: HardcoverBookMatch | null = null;
|
||||
|
||||
if (book.hardcoverMetadataId) {
|
||||
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);
|
||||
}
|
||||
match = (await this.matchByMetadataId(userId, token, book, book.hardcoverMetadataId)).match;
|
||||
}
|
||||
|
||||
if (!match && book.isbn13) {
|
||||
match = await this.matchByIsbn(userId, token, book.isbn13, book.bookId, 13);
|
||||
match = (await this.matchByIsbn(userId, token, book, book.isbn13, 13)).match;
|
||||
}
|
||||
|
||||
if (!match && book.isbn10) {
|
||||
match = await this.matchByIsbn(userId, token, book.isbn10, book.bookId, 10);
|
||||
match = (await this.matchByIsbn(userId, token, book, book.isbn10, 10)).match;
|
||||
}
|
||||
|
||||
if (!match && book.title && book.authorName) {
|
||||
@@ -281,6 +291,7 @@ export class HardcoverBookMatchService {
|
||||
hardcoverBookId: match.hardcoverBookId,
|
||||
hardcoverEditionId: match.hardcoverEditionId ?? null,
|
||||
matchMethod: match.matchMethod,
|
||||
matchBasis: buildHardcoverMatchBasis(book),
|
||||
matchError: null,
|
||||
});
|
||||
} else {
|
||||
@@ -288,6 +299,7 @@ export class HardcoverBookMatchService {
|
||||
userId,
|
||||
bookId: book.bookId,
|
||||
hardcoverBookId: null,
|
||||
matchBasis: null,
|
||||
matchError: 'no_match',
|
||||
});
|
||||
}
|
||||
@@ -295,68 +307,109 @@ export class HardcoverBookMatchService {
|
||||
return match;
|
||||
}
|
||||
|
||||
private async matchByHardcoverId(userId: number, token: string, id: number, book: BookSyncData): Promise<HardcoverBookMatch | null> {
|
||||
/**
|
||||
* Re-checks a cached link whose identifiers changed since it was made. Only an explicit identifier
|
||||
* can move it, and only to a book Hardcover positively resolves: a title search, or an identifier
|
||||
* Hardcover does not know, must never replace a link that works.
|
||||
*/
|
||||
private async relinkIfIdentifiersMoved(
|
||||
userId: number,
|
||||
token: string,
|
||||
book: BookSyncData,
|
||||
cachedBookId: number,
|
||||
previousBasis: string | null,
|
||||
matchBasis: string,
|
||||
): Promise<HardcoverBookMatch | null> {
|
||||
// A link made before its basis was recorded may have been confirmed through import against an
|
||||
// ISBN that disagrees with it, so only the explicit Hardcover ID is trusted to move it.
|
||||
const lookup = await this.findIdentifiedBook(userId, token, book, previousBasis !== null);
|
||||
if (lookup.failed) return null;
|
||||
|
||||
const match = lookup.match;
|
||||
if (!match || match.hardcoverBookId === cachedBookId) {
|
||||
await this.repo.upsertBookState({ userId, bookId: book.bookId, matchBasis });
|
||||
return null;
|
||||
}
|
||||
|
||||
const releasedAttempts = await this.repo.relinkBook({
|
||||
userId,
|
||||
bookId: book.bookId,
|
||||
hardcoverBookId: match.hardcoverBookId,
|
||||
hardcoverEditionId: match.hardcoverEditionId,
|
||||
matchMethod: match.matchMethod,
|
||||
matchBasis,
|
||||
});
|
||||
this.logger.log(
|
||||
`[hardcover.book_relink] [end] userId=${userId} bookId=${book.bookId} previousHardcoverBookId=${cachedBookId} hardcoverBookId=${match.hardcoverBookId} hardcoverEditionId=${match.hardcoverEditionId ?? null} matchMethod=${match.matchMethod} releasedAttempts=${releasedAttempts} - link moved to the book the metadata identifies`,
|
||||
);
|
||||
return { ...match, replacedHardcoverBookId: cachedBookId };
|
||||
}
|
||||
|
||||
private async findIdentifiedBook(userId: number, token: string, book: BookSyncData, includeIsbn: boolean): Promise<HardcoverLookup> {
|
||||
const lookups: Array<() => Promise<HardcoverLookup>> = [];
|
||||
const { hardcoverMetadataId, isbn13, isbn10 } = book;
|
||||
if (hardcoverMetadataId) lookups.push(() => this.matchByMetadataId(userId, token, book, hardcoverMetadataId));
|
||||
if (includeIsbn && isbn13) lookups.push(() => this.matchByIsbn(userId, token, book, isbn13, 13));
|
||||
if (includeIsbn && isbn10) lookups.push(() => this.matchByIsbn(userId, token, book, isbn10, 10));
|
||||
|
||||
for (const lookup of lookups) {
|
||||
const result = await lookup();
|
||||
// A stronger identifier that could not be checked must not let a weaker one decide instead.
|
||||
if (result.match || result.failed) return result;
|
||||
}
|
||||
return { match: null, failed: false };
|
||||
}
|
||||
|
||||
private matchByMetadataId(userId: number, token: string, book: BookSyncData, metadataId: string): Promise<HardcoverLookup> {
|
||||
const id = parseHardcoverBookId(metadataId);
|
||||
return id !== null ? this.matchByHardcoverId(userId, token, id, book) : this.matchByHardcoverSlug(userId, token, metadataId, book);
|
||||
}
|
||||
|
||||
private async matchByHardcoverId(userId: number, token: string, id: number, book: BookSyncData): Promise<HardcoverLookup> {
|
||||
try {
|
||||
const data = await this.client.query<BooksQueryResult>(userId, token, FIND_BOOK_BY_HARDCOVER_ID_QUERY, { id });
|
||||
const hardcoverBook = data.books?.[0];
|
||||
if (!hardcoverBook) return null;
|
||||
const edition = this.pickBestEdition(hardcoverBook.editions ?? [], book);
|
||||
return {
|
||||
hardcoverBookId: hardcoverBook.id,
|
||||
hardcoverEditionId: edition?.id ?? null,
|
||||
editionPages: this.normalizeEditionPages(edition?.pages),
|
||||
editionAudioSeconds: this.normalizeEditionAudioSeconds(edition?.audio_seconds),
|
||||
matchMethod: 'metadata_id',
|
||||
};
|
||||
if (!hardcoverBook) return { match: null, failed: false };
|
||||
const fallback = this.pickBestEdition(hardcoverBook.editions ?? [], book);
|
||||
return { match: await this.buildMatch(userId, token, hardcoverBook, book, 'metadata_id', fallback), failed: false };
|
||||
} catch (err) {
|
||||
const error = sanitizeLogValue(err instanceof Error ? err.message : String(err));
|
||||
this.logger.warn(
|
||||
`[hardcover.book_match] [fail] userId=${userId} bookId=${book.bookId} method=metadata_id error="${error}" - metadata_id lookup failed`,
|
||||
);
|
||||
return null;
|
||||
return { match: null, failed: true };
|
||||
}
|
||||
}
|
||||
|
||||
private async matchByHardcoverSlug(userId: number, token: string, slug: string, book: BookSyncData): Promise<HardcoverBookMatch | null> {
|
||||
private async matchByHardcoverSlug(userId: number, token: string, slug: string, book: BookSyncData): Promise<HardcoverLookup> {
|
||||
try {
|
||||
const data = await this.client.query<BooksQueryResult>(userId, token, FIND_BOOK_BY_HARDCOVER_SLUG_QUERY, { slug });
|
||||
const hardcoverBook = data.books?.[0];
|
||||
if (!hardcoverBook) return null;
|
||||
const edition = this.pickBestEdition(hardcoverBook.editions ?? [], book);
|
||||
return {
|
||||
hardcoverBookId: hardcoverBook.id,
|
||||
hardcoverEditionId: edition?.id ?? null,
|
||||
editionPages: this.normalizeEditionPages(edition?.pages),
|
||||
editionAudioSeconds: this.normalizeEditionAudioSeconds(edition?.audio_seconds),
|
||||
matchMethod: 'metadata_id',
|
||||
};
|
||||
if (!hardcoverBook) return { match: null, failed: false };
|
||||
const fallback = this.pickBestEdition(hardcoverBook.editions ?? [], book);
|
||||
return { match: await this.buildMatch(userId, token, hardcoverBook, book, 'metadata_id', fallback), failed: false };
|
||||
} catch (err) {
|
||||
const error = sanitizeLogValue(err instanceof Error ? err.message : String(err));
|
||||
this.logger.warn(
|
||||
`[hardcover.book_match] [fail] userId=${userId} bookId=${book.bookId} method=metadata_slug error="${error}" - metadata_slug lookup failed`,
|
||||
);
|
||||
return null;
|
||||
return { match: null, failed: true };
|
||||
}
|
||||
}
|
||||
|
||||
private async matchByIsbn(userId: number, token: string, isbn: string, bookId: number, version: 10 | 13): Promise<HardcoverBookMatch | null> {
|
||||
private async matchByIsbn(userId: number, token: string, book: BookSyncData, isbn: string, version: 10 | 13): Promise<HardcoverLookup> {
|
||||
const query = version === 13 ? FIND_BOOK_BY_ISBN13_QUERY : FIND_BOOK_BY_ISBN10_QUERY;
|
||||
try {
|
||||
const data = await this.client.query<BooksQueryResult>(userId, token, query, { isbn });
|
||||
const hardcoverBook = data.books?.[0];
|
||||
if (!hardcoverBook) return null;
|
||||
const edition = hardcoverBook.editions?.[0];
|
||||
return {
|
||||
hardcoverBookId: hardcoverBook.id,
|
||||
hardcoverEditionId: edition?.id ?? null,
|
||||
editionPages: this.normalizeEditionPages(edition?.pages),
|
||||
editionAudioSeconds: this.normalizeEditionAudioSeconds(edition?.audio_seconds),
|
||||
matchMethod: 'isbn',
|
||||
};
|
||||
if (!hardcoverBook) return { match: null, failed: false };
|
||||
return { match: await this.buildMatch(userId, token, hardcoverBook, book, 'isbn', hardcoverBook.editions?.[0]), failed: false };
|
||||
} catch (err) {
|
||||
const error = sanitizeLogValue(err instanceof Error ? err.message : String(err));
|
||||
this.logger.warn(`[hardcover.book_match] [fail] userId=${userId} bookId=${bookId} method=isbn${version} error="${error}" - ISBN lookup failed`);
|
||||
return null;
|
||||
this.logger.warn(
|
||||
`[hardcover.book_match] [fail] userId=${userId} bookId=${book.bookId} method=isbn${version} error="${error}" - ISBN lookup failed`,
|
||||
);
|
||||
return { match: null, failed: true };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -386,14 +439,8 @@ export class HardcoverBookMatchService {
|
||||
const booksById = new Map((data.books ?? []).map((candidate) => [candidate.id, candidate]));
|
||||
const hardcoverBook = ids.map((id) => booksById.get(id)).find((candidate): candidate is BooksQueryResult['books'][number] => candidate != null);
|
||||
if (!hardcoverBook) return null;
|
||||
const edition = this.pickBestEdition(hardcoverBook.editions ?? [], book);
|
||||
return {
|
||||
hardcoverBookId: hardcoverBook.id,
|
||||
hardcoverEditionId: edition?.id ?? null,
|
||||
editionPages: this.normalizeEditionPages(edition?.pages),
|
||||
editionAudioSeconds: this.normalizeEditionAudioSeconds(edition?.audio_seconds),
|
||||
matchMethod: 'title',
|
||||
};
|
||||
const fallback = this.pickBestEdition(hardcoverBook.editions ?? [], book);
|
||||
return await this.buildMatch(userId, token, hardcoverBook, book, 'title', fallback);
|
||||
} catch (err) {
|
||||
const errorClass = err instanceof Error ? err.constructor.name : 'Error';
|
||||
const error = sanitizeLogValue(err instanceof Error ? err.message : String(err));
|
||||
@@ -404,6 +451,57 @@ export class HardcoverBookMatchService {
|
||||
}
|
||||
}
|
||||
|
||||
private async buildMatch(
|
||||
userId: number,
|
||||
token: string,
|
||||
hardcoverBook: BooksQueryResult['books'][number],
|
||||
book: BookSyncData,
|
||||
matchMethod: HardcoverBookMatch['matchMethod'],
|
||||
fallback: HardcoverEdition | undefined,
|
||||
): Promise<HardcoverBookMatch> {
|
||||
const edition = await this.chooseEdition(userId, token, hardcoverBook.id, hardcoverBook.editions ?? [], book, fallback);
|
||||
return {
|
||||
hardcoverBookId: hardcoverBook.id,
|
||||
hardcoverEditionId: edition?.id ?? null,
|
||||
editionPages: this.normalizeEditionPages(edition?.pages),
|
||||
editionAudioSeconds: this.normalizeEditionAudioSeconds(edition?.audio_seconds),
|
||||
matchMethod,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The edition named in the book's metadata wins whenever it belongs to the matched book, looked up
|
||||
* directly when the capped list does not carry it. An edition of another book is ignored.
|
||||
*/
|
||||
private async chooseEdition(
|
||||
userId: number,
|
||||
token: string,
|
||||
hardcoverBookId: number,
|
||||
editions: HardcoverEdition[],
|
||||
book: BookSyncData,
|
||||
fallback: HardcoverEdition | undefined,
|
||||
): Promise<HardcoverEdition | undefined> {
|
||||
const preferredId = parseHardcoverEditionId(book.hardcoverEditionId);
|
||||
if (preferredId === null || preferredId === fallback?.id) return fallback;
|
||||
|
||||
const listed = editions.find((edition) => edition.id === preferredId);
|
||||
if (listed) return listed;
|
||||
|
||||
try {
|
||||
const data = await this.client.query<BooksQueryResult>(userId, token, FIND_BOOK_EDITION_METRICS_BY_ID_QUERY, {
|
||||
id: hardcoverBookId,
|
||||
editionId: preferredId,
|
||||
});
|
||||
return data.books?.[0]?.editions?.find((edition) => edition.id === preferredId) ?? fallback;
|
||||
} catch (err) {
|
||||
const error = sanitizeLogValue(err instanceof Error ? err.message : String(err));
|
||||
this.logger.warn(
|
||||
`[hardcover.book_match] [fail] userId=${userId} bookId=${book.bookId} method=metadata_edition error="${error}" - metadata edition lookup failed`,
|
||||
);
|
||||
return fallback;
|
||||
}
|
||||
}
|
||||
|
||||
// A failed upstream lookup must not be reported as "no editions" - the caller (and the user)
|
||||
// needs to be able to tell an empty catalog apart from a provider outage and retry.
|
||||
async listEditions(userId: number, token: string, hardcoverBookId: number): Promise<HardcoverEditionsResult> {
|
||||
@@ -490,7 +588,8 @@ export class HardcoverBookMatchService {
|
||||
return { hardcoverEditionId: cachedEditionId, editionPages: null, editionAudioSeconds: null };
|
||||
}
|
||||
|
||||
const edition = this.pickBestEdition(hardcoverBook.editions ?? [], book);
|
||||
const editions = hardcoverBook.editions ?? [];
|
||||
const edition = await this.chooseEdition(userId, token, hardcoverBookId, editions, book, this.pickBestEdition(editions, book));
|
||||
if (!edition) {
|
||||
return { hardcoverEditionId: cachedEditionId, editionPages: null, editionAudioSeconds: null };
|
||||
}
|
||||
|
||||
@@ -5,3 +5,13 @@ export function parseHardcoverBookId(value: string | null | undefined): number |
|
||||
const id = Number(value);
|
||||
return Number.isSafeInteger(id) && id > 0 && id <= GRAPHQL_INT_MAX ? id : null;
|
||||
}
|
||||
|
||||
export const parseHardcoverEditionId = parseHardcoverBookId;
|
||||
|
||||
/**
|
||||
* The identifiers a Hardcover link was resolved from. A cached link is only re-checked when this
|
||||
* changes, so a corrected Hardcover ID or ISBN moves the link without querying on every sync.
|
||||
*/
|
||||
export function buildHardcoverMatchBasis(book: { hardcoverMetadataId: string | null; isbn13: string | null; isbn10: string | null }): string {
|
||||
return JSON.stringify([book.hardcoverMetadataId ?? null, book.isbn13 ?? null, book.isbn10 ?? null]);
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ import type { ReadingAttemptService } from '../user-book-status/reading-attempt.
|
||||
import { BadRequestException, Logger } from '@nestjs/common';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { buildHardcoverMatchBasis } from './hardcover-id.utils';
|
||||
import { HardcoverImportService } from './hardcover-import.service';
|
||||
|
||||
const mockRepo = {
|
||||
@@ -508,6 +509,8 @@ describe('HardcoverImportService', () => {
|
||||
hardcoverUserBookId: 1000,
|
||||
hardcoverReadId: 501,
|
||||
matchMethod: 'isbn',
|
||||
// Recorded so the first sync does not second-guess a link the import just made.
|
||||
matchBasis: buildHardcoverMatchBasis(localBook()),
|
||||
lastSyncedStatus: 'read',
|
||||
lastSyncedProgress: null,
|
||||
}),
|
||||
@@ -920,16 +923,19 @@ describe('HardcoverImportService', () => {
|
||||
const service = makeService(attempts as never);
|
||||
const preview = await service.previewImport(user as never);
|
||||
vi.spyOn(service as any, 'buildPreview').mockResolvedValue({
|
||||
...preview,
|
||||
rows: Array.from({ length: 121 }, (_, index) => ({
|
||||
...preview.rows[0],
|
||||
hardcoverUserBookId: 1000 + index,
|
||||
localBookId: 42 + index,
|
||||
hardcoverReads: [
|
||||
{ id: index * 2 + 1, startedAt: null, finishedAt: '2026-01-15' },
|
||||
{ id: index * 2 + 2, startedAt: null, finishedAt: '2026-01-16' },
|
||||
],
|
||||
})),
|
||||
preview: {
|
||||
...preview,
|
||||
rows: Array.from({ length: 121 }, (_, index) => ({
|
||||
...preview.rows[0],
|
||||
hardcoverUserBookId: 1000 + index,
|
||||
localBookId: 42 + index,
|
||||
hardcoverReads: [
|
||||
{ id: index * 2 + 1, startedAt: null, finishedAt: '2026-01-15' },
|
||||
{ id: index * 2 + 2, startedAt: null, finishedAt: '2026-01-16' },
|
||||
],
|
||||
})),
|
||||
},
|
||||
matchBasisByBookId: new Map(),
|
||||
});
|
||||
mockUserBookStatusService.updateManual.mockImplementation(() => {
|
||||
events.notifyChanged(7);
|
||||
|
||||
@@ -18,7 +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 { buildHardcoverMatchBasis, 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';
|
||||
@@ -150,7 +150,7 @@ export class HardcoverImportService {
|
||||
this.logger.log(`[hardcover.import_status] [start] userId=${user.id} mode=preview - import preview started`);
|
||||
|
||||
try {
|
||||
const preview = await this.buildPreview(user);
|
||||
const { preview } = await this.buildPreview(user);
|
||||
this.logger.log(
|
||||
`[hardcover.import_status] [end] userId=${user.id} mode=preview durationMs=${Date.now() - startedAt} totalHardcoverBooks=${preview.summary.totalHardcoverBooks} willUpdate=${preview.summary.willUpdate} needsReview=${preview.summary.needsReview} conflicts=${preview.summary.conflicts} unmatched=${preview.summary.unmatched} skipped=${preview.summary.skipped} - import preview completed`,
|
||||
);
|
||||
@@ -166,7 +166,7 @@ export class HardcoverImportService {
|
||||
this.logger.log(`[hardcover.import_status] [start] userId=${user.id} mode=apply - import apply started`);
|
||||
|
||||
try {
|
||||
const preview = await this.buildPreview(user);
|
||||
const { preview, matchBasisByBookId } = await this.buildPreview(user);
|
||||
const selectedIds = payload.hardcoverUserBookIds ? new Set(payload.hardcoverUserBookIds) : null;
|
||||
const importProgress = payload.importProgress === true;
|
||||
let applied = 0;
|
||||
@@ -212,6 +212,7 @@ export class HardcoverImportService {
|
||||
hardcoverUserBookId: row.hardcoverUserBookId,
|
||||
hardcoverReadId: row.hardcoverReadId,
|
||||
matchMethod: row.matchMethod,
|
||||
matchBasis: matchBasisByBookId.get(row.localBookId) ?? null,
|
||||
matchError: null,
|
||||
syncError: null,
|
||||
lastSyncedAt: new Date(),
|
||||
@@ -257,7 +258,7 @@ export class HardcoverImportService {
|
||||
}
|
||||
}
|
||||
|
||||
private async buildPreview(user: RequestUser): Promise<HardcoverImportPreview> {
|
||||
private async buildPreview(user: RequestUser): Promise<{ preview: HardcoverImportPreview; matchBasisByBookId: Map<number, string> }> {
|
||||
const token = await this.settingsService.getTokenForUser(user.id);
|
||||
if (!token) {
|
||||
throw new BadRequestException('Hardcover sync is not available right now');
|
||||
@@ -277,8 +278,8 @@ export class HardcoverImportService {
|
||||
const rows: HardcoverImportPreviewRow[] = [];
|
||||
for (const book of hardcoverBooks) rows.push(await this.buildPreviewRow(book, indexes));
|
||||
return {
|
||||
summary: summarize(rows),
|
||||
rows,
|
||||
preview: { summary: summarize(rows), rows },
|
||||
matchBasisByBookId: new Map(localBooks.map((book) => [book.bookId, buildHardcoverMatchBasis(book)])),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { BookService } from '../book/book.service';
|
||||
import { HardcoverBookMatchService } from './hardcover-book-match.service';
|
||||
import { HardcoverClientService } from './hardcover-client.service';
|
||||
import { buildHardcoverMatchBasis } from './hardcover-id.utils';
|
||||
import { HardcoverRepository } from './hardcover.repository';
|
||||
import { HardcoverSettingsService } from './hardcover-settings.service';
|
||||
import { HardcoverSyncService } from './hardcover-sync.service';
|
||||
@@ -22,6 +23,7 @@ const mockRepo = {
|
||||
findReadingAttempts: vi.fn(),
|
||||
findClaimedHardcoverReadIds: vi.fn(),
|
||||
linkReadingAttempt: vi.fn(),
|
||||
unlinkReadingAttempt: vi.fn(),
|
||||
};
|
||||
|
||||
const mockClient = {
|
||||
@@ -99,6 +101,7 @@ describe('HardcoverSyncService', () => {
|
||||
mockRepo.findReadingAttempts.mockResolvedValue([]);
|
||||
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([]);
|
||||
mockRepo.linkReadingAttempt.mockResolvedValue('linked');
|
||||
mockRepo.unlinkReadingAttempt.mockResolvedValue(true);
|
||||
mockRepo.upsertBookState.mockResolvedValue({});
|
||||
mockRepo.setBookSyncOverride.mockResolvedValue({});
|
||||
mockRepo.updateLastSyncedAt.mockResolvedValue(undefined);
|
||||
@@ -1317,7 +1320,132 @@ describe('HardcoverSyncService', () => {
|
||||
expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('cause="23505 reading_attempts_external_uidx duplicate key value"'));
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
|
||||
describe('a linked read filed under another Hardcover book', () => {
|
||||
const FOREIGN_READ = 7159741;
|
||||
const FOREIGN_USER_BOOK = 19544253;
|
||||
|
||||
function owner(userBookId: number | null) {
|
||||
mockClient.query.mockResolvedValueOnce({ user_book_reads: userBookId == null ? [] : [{ id: FOREIGN_READ, user_book_id: userBookId }] });
|
||||
}
|
||||
|
||||
it('is released, and the attempt gets a read on this book instead of a rejected edit', async () => {
|
||||
const logSpy = vi.spyOn(Logger.prototype, 'log').mockImplementation(() => undefined);
|
||||
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
|
||||
mockRepo.findReadingAttempts.mockResolvedValue([linked(1, FOREIGN_READ, { startedOn: '2024-01-01' })]);
|
||||
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([FOREIGN_READ]);
|
||||
arrange([]);
|
||||
owner(FOREIGN_USER_BOOK);
|
||||
insertRead(601);
|
||||
|
||||
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
|
||||
|
||||
expect(mockClient.query).toHaveBeenNthCalledWith(4, 1, 'tok', expect.stringContaining('query FindUserBookReadOwner'), { id: FOREIGN_READ });
|
||||
expect(mockRepo.unlinkReadingAttempt).toHaveBeenCalledWith(1, 1, FOREIGN_READ);
|
||||
expect(linkCalls()).toEqual([[1, 1, 601]]);
|
||||
expect(mockClient.query).not.toHaveBeenCalledWith(
|
||||
1,
|
||||
'tok',
|
||||
expect.stringContaining('mutation UpdateUserBookRead'),
|
||||
expect.objectContaining({ id: FOREIGN_READ }),
|
||||
);
|
||||
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ hardcoverReadId: 601, syncError: null }));
|
||||
expect(logSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining(`[hardcover.read_unlink] [end] userId=1 attemptId=1 hardcoverReadId=${FOREIGN_READ}`),
|
||||
);
|
||||
logSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('is kept when Hardcover files it under this book, past the fetched window', async () => {
|
||||
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
|
||||
mockRepo.findReadingAttempts.mockResolvedValue([linked(1, FOREIGN_READ, { startedOn: '2024-01-01' })]);
|
||||
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([FOREIGN_READ]);
|
||||
arrange([]);
|
||||
owner(55);
|
||||
updateRead(FOREIGN_READ);
|
||||
|
||||
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
|
||||
|
||||
expect(mockRepo.unlinkReadingAttempt).not.toHaveBeenCalled();
|
||||
expect(mockClient.query).toHaveBeenNthCalledWith(
|
||||
5,
|
||||
1,
|
||||
'tok',
|
||||
expect.stringContaining('mutation UpdateUserBookRead'),
|
||||
expect.objectContaining({ id: FOREIGN_READ }),
|
||||
);
|
||||
});
|
||||
|
||||
it('is kept when Hardcover cannot place it', async () => {
|
||||
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
|
||||
mockRepo.findReadingAttempts.mockResolvedValue([linked(1, FOREIGN_READ, { startedOn: '2024-01-01' })]);
|
||||
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([FOREIGN_READ]);
|
||||
arrange([]);
|
||||
owner(null);
|
||||
updateRead(FOREIGN_READ);
|
||||
|
||||
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
|
||||
|
||||
expect(mockRepo.unlinkReadingAttempt).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('is kept when the owner lookup fails', async () => {
|
||||
const warnSpy = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);
|
||||
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
|
||||
mockRepo.findReadingAttempts.mockResolvedValue([linked(1, FOREIGN_READ, { startedOn: '2024-01-01' })]);
|
||||
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([FOREIGN_READ]);
|
||||
arrange([]);
|
||||
mockClient.query.mockRejectedValueOnce(new Error('Hardcover unavailable'));
|
||||
updateRead(FOREIGN_READ);
|
||||
|
||||
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
|
||||
|
||||
expect(mockRepo.unlinkReadingAttempt).not.toHaveBeenCalled();
|
||||
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining(`[hardcover.read_owner] [fail] userId=1 hardcoverReadId=${FOREIGN_READ}`));
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('after the match moves to another Hardcover book', () => {
|
||||
const wantToRead = { ...readingBook, status: 'want_to_read', startedAt: null, progress: null };
|
||||
const staleState = { hardcoverBookId: 1554702, hardcoverUserBookId: 19544253, hardcoverReadId: 7159741 };
|
||||
|
||||
function arrangeWantToRead(match: Record<string, unknown>) {
|
||||
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
|
||||
mockRepo.findBookSyncData.mockResolvedValue(wantToRead);
|
||||
mockRepo.findBookState.mockResolvedValue(staleState);
|
||||
mockMatchService.matchBook.mockResolvedValue(match);
|
||||
mockClient.query
|
||||
.mockResolvedValueOnce({ insert_user_book: { user_book: { id: 19544262 }, error: null } })
|
||||
.mockResolvedValueOnce({ update_user_book: { user_book: { id: 19544262 }, error: null } });
|
||||
}
|
||||
|
||||
it("does not carry the old book's read id over", async () => {
|
||||
arrangeWantToRead({
|
||||
hardcoverBookId: 2973670,
|
||||
hardcoverEditionId: 33314466,
|
||||
editionPages: 321,
|
||||
matchMethod: 'metadata_id',
|
||||
replacedHardcoverBookId: 1554702,
|
||||
});
|
||||
|
||||
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
|
||||
|
||||
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ hardcoverBookId: 2973670, hardcoverUserBookId: 19544262, hardcoverReadId: null, syncError: null }),
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps the read id while the match stays on the same book', async () => {
|
||||
arrangeWantToRead({ hardcoverBookId: 1554702, hardcoverEditionId: 31616496, editionPages: 217, matchMethod: 'cached' });
|
||||
|
||||
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
|
||||
|
||||
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ hardcoverBookId: 1554702, hardcoverReadId: 7159741 }));
|
||||
});
|
||||
});
|
||||
|
||||
describe('change detection', () => {
|
||||
const syncedState = {
|
||||
hardcoverBookId: 10,
|
||||
@@ -1352,5 +1480,48 @@ describe('HardcoverSyncService', () => {
|
||||
|
||||
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
|
||||
});
|
||||
|
||||
it('re-syncs a settled link whose identifiers changed, even with nothing else to send', async () => {
|
||||
const before = { ...readingBook, hardcoverMetadataId: 'hell-be-ok-growing-gorgeous-boys-into-good-men' };
|
||||
const corrected = { ...readingBook, hardcoverMetadataId: 'ok-amen', isbn13: '9789146223856' };
|
||||
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
|
||||
mockRepo.findBookSyncData.mockResolvedValue(corrected);
|
||||
mockRepo.findBookState.mockResolvedValue({ ...syncedState, matchBasis: buildHardcoverMatchBasis(before) });
|
||||
mockMatchService.matchBook.mockResolvedValue(null);
|
||||
const warnSpy = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);
|
||||
|
||||
await makeService().syncBook(1, 1);
|
||||
|
||||
expect(mockMatchService.matchBook).toHaveBeenCalledWith(1, 'tok', corrected);
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('skips a settled link whose identifiers are unchanged', async () => {
|
||||
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
|
||||
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
|
||||
mockRepo.findBookState.mockResolvedValue({ ...syncedState, matchBasis: buildHardcoverMatchBasis(readingBook) });
|
||||
|
||||
await expect(makeService().syncBook(1, 1)).resolves.toBe('skipped');
|
||||
expect(mockMatchService.matchBook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('stops re-syncing a numeric Hardcover ID once a check settled it without moving the link', async () => {
|
||||
const unresolvable = { ...readingBook, hardcoverMetadataId: '84' };
|
||||
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
|
||||
mockRepo.findBookSyncData.mockResolvedValue(unresolvable);
|
||||
mockRepo.findBookState.mockResolvedValue({ ...syncedState, hardcoverBookId: 277100, matchBasis: buildHardcoverMatchBasis(unresolvable) });
|
||||
|
||||
await expect(makeService().syncBook(1, 1)).resolves.toBe('skipped');
|
||||
expect(mockMatchService.matchBook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not queue every older link for a re-sync because no basis was recorded for it', async () => {
|
||||
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
|
||||
mockRepo.findBookSyncData.mockResolvedValue({ ...readingBook, isbn13: '9789146223856', hardcoverMetadataId: 'ok-amen' });
|
||||
mockRepo.findBookState.mockResolvedValue({ ...syncedState, matchBasis: null });
|
||||
|
||||
await expect(makeService().syncBook(1, 1)).resolves.toBe('skipped');
|
||||
expect(mockMatchService.matchBook).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -19,7 +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 { buildHardcoverMatchBasis, parseHardcoverBookId } from './hardcover-id.utils';
|
||||
import {
|
||||
attemptOwnedReadId,
|
||||
type HardcoverReadCandidate,
|
||||
@@ -113,11 +113,21 @@ query FindUserBookReads($userBookId: Int!) {
|
||||
}
|
||||
}`;
|
||||
|
||||
// Answers which user book a read belongs to, for a linked read the window above did not return.
|
||||
const FIND_USER_BOOK_READ_OWNER_QUERY = `
|
||||
query FindUserBookReadOwner($id: Int!) {
|
||||
user_book_reads(where: { id: { _eq: $id } }, limit: 1) {
|
||||
id
|
||||
user_book_id
|
||||
}
|
||||
}`;
|
||||
|
||||
type InsertUserBookResult = { insert_user_book: { user_book: { id: number } | null; error: string | null } };
|
||||
type UpdateUserBookResult = { update_user_book: { user_book: { id: number } | null; error: string | null } };
|
||||
type FindUserBookResult = { me: { user_books: { id: number }[] } };
|
||||
type InsertUserBookReadResult = { insert_user_book_read: { user_book_read: { id: number } | null; error: string | null } };
|
||||
type UpdateUserBookReadResult = { update_user_book_read: { user_book_read: { id: number } | null; error: string | null } };
|
||||
type FindUserBookReadOwnerResult = { user_book_reads: Array<{ id: number; user_book_id: number | null }> };
|
||||
type FindUserBookReadsResult = {
|
||||
user_book_reads: Array<{
|
||||
id: number;
|
||||
@@ -533,7 +543,7 @@ export class HardcoverSyncService {
|
||||
const syncState = activeInitialState ?? state;
|
||||
const startDate = toDateString(book.startedAt);
|
||||
const endDate = toDateString(book.finishedAt);
|
||||
let hardcoverReadId: number | null = syncState?.hardcoverReadId ?? null;
|
||||
let hardcoverReadId: number | null = match.replacedHardcoverBookId != null ? null : (syncState?.hardcoverReadId ?? null);
|
||||
let progressSynced = true;
|
||||
|
||||
const localAttempts = typeof this.repo.findReadingAttempts === 'function' ? await this.repo.findReadingAttempts(userId, book.bookId) : [];
|
||||
@@ -794,7 +804,15 @@ export class HardcoverSyncService {
|
||||
editionId: input.editionId,
|
||||
});
|
||||
|
||||
const owned = attempt ? attemptOwnedReadId(attempt) : null;
|
||||
let owned = attempt ? attemptOwnedReadId(attempt) : null;
|
||||
if (owned != null && attempt && (await this.readBelongsToAnotherUserBook(userId, token, input.userBookId, input.reads, owned))) {
|
||||
// Hardcover rejects edits to a read filed under another book, so the link could never sync again.
|
||||
await this.repo.unlinkReadingAttempt(userId, attempt.id, owned);
|
||||
this.logger.log(
|
||||
`[hardcover.read_unlink] [end] userId=${userId} attemptId=${attempt.id} hardcoverReadId=${owned} hardcoverUserBookId=${input.userBookId} - released a read that belongs to another Hardcover book`,
|
||||
);
|
||||
owned = null;
|
||||
}
|
||||
if (owned != null && attempt) {
|
||||
input.reserved.add(owned);
|
||||
const readId = await write(owned);
|
||||
@@ -828,6 +846,30 @@ export class HardcoverSyncService {
|
||||
return created;
|
||||
}
|
||||
|
||||
/**
|
||||
* Only a read missing from the fetched window is checked, and only Hardcover naming a different
|
||||
* user book counts: a read it cannot place keeps the link, as before.
|
||||
*/
|
||||
private async readBelongsToAnotherUserBook(
|
||||
userId: number,
|
||||
token: string,
|
||||
userBookId: number,
|
||||
reads: readonly HardcoverReadCandidate[],
|
||||
readId: number,
|
||||
): Promise<boolean> {
|
||||
if (reads.some((read) => read.id === readId)) return false;
|
||||
try {
|
||||
const data = await this.client.query<FindUserBookReadOwnerResult>(userId, token, FIND_USER_BOOK_READ_OWNER_QUERY, { id: readId });
|
||||
const owner = data.user_book_reads?.[0]?.user_book_id;
|
||||
return owner != null && owner !== userBookId;
|
||||
} catch (err) {
|
||||
this.logger.warn(
|
||||
`[hardcover.read_owner] [fail] userId=${userId} hardcoverReadId=${readId} errorClass=${err?.constructor?.name ?? 'Error'} error="${sanitizeLogValue(err instanceof Error ? err.message : String(err))}" - read owner lookup failed`,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private async updateAdditionalOpenReads(
|
||||
userId: number,
|
||||
token: string,
|
||||
@@ -865,8 +907,13 @@ 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 = parseHardcoverBookId(book.hardcoverMetadataId);
|
||||
if (metadataHardcoverId !== null && metadataHardcoverId !== state.hardcoverBookId) return true;
|
||||
if (state.matchBasis != null) {
|
||||
// Corrected identifiers have to reach the match even when nothing else changed.
|
||||
if (state.hardcoverBookId != null && state.matchBasis !== buildHardcoverMatchBasis(book)) return true;
|
||||
} else {
|
||||
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;
|
||||
if (book.rating !== state.lastSyncedRating) return true;
|
||||
|
||||
@@ -485,6 +485,16 @@ describe('HardcoverRepository', () => {
|
||||
expect(updateChain.set).toHaveBeenCalledWith(expect.objectContaining({ externalProvider: 'hardcover', externalId: '555' }));
|
||||
});
|
||||
|
||||
it('accepts an attempt whose Hardcover link was cleared down to the provider', async () => {
|
||||
const { repo, updateWhere } = makeRepository();
|
||||
|
||||
await repo.linkReadingAttempt(7, 3, 555);
|
||||
|
||||
const where = new PgDialect().sqlToQuery(updateWhere.mock.calls[0]![0]);
|
||||
expect(where.sql).toContain('("reading_attempts"."external_provider" is null or "reading_attempts"."external_provider" = $');
|
||||
expect(where.params).toEqual([7, 3, 'hardcover', 'hardcover', '555']);
|
||||
});
|
||||
|
||||
it('reports a conflict when another attempt already owns the read', async () => {
|
||||
const { repo, updateChain } = makeRepository();
|
||||
rejectUpdateWith(updateChain, pgError({ code: '23505', constraint: 'reading_attempts_external_uidx' }));
|
||||
@@ -540,6 +550,78 @@ describe('HardcoverRepository', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('relinkBook', () => {
|
||||
function makeTransactionalRepository(releasedIds: number[]) {
|
||||
const stateSet = vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) });
|
||||
const attemptWhere = vi.fn().mockReturnValue({ returning: vi.fn().mockResolvedValue(releasedIds.map((id) => ({ id }))) });
|
||||
const attemptSet = vi.fn().mockReturnValue({ where: attemptWhere });
|
||||
const tx = { update: vi.fn().mockReturnValueOnce({ set: stateSet }).mockReturnValueOnce({ set: attemptSet }) };
|
||||
const db = { transaction: vi.fn((run: (handle: typeof tx) => Promise<unknown>) => run(tx)) };
|
||||
return { repo: new HardcoverRepository(db as never), db, tx, stateSet, attemptSet, attemptWhere };
|
||||
}
|
||||
|
||||
const input = {
|
||||
userId: 7,
|
||||
bookId: 42,
|
||||
hardcoverBookId: 2973670,
|
||||
hardcoverEditionId: 33314466,
|
||||
matchMethod: 'metadata_id',
|
||||
matchBasis: '["ok-amen","9789146223856",null]',
|
||||
};
|
||||
|
||||
it("moves the link and drops the old book's user book and read ids", async () => {
|
||||
const { repo, db, tx, stateSet } = makeTransactionalRepository([]);
|
||||
|
||||
await repo.relinkBook(input);
|
||||
|
||||
expect(db.transaction).toHaveBeenCalledTimes(1);
|
||||
expect(tx.update).toHaveBeenNthCalledWith(1, schema.hardcoverBookState);
|
||||
expect(stateSet).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
hardcoverBookId: 2973670,
|
||||
hardcoverEditionId: 33314466,
|
||||
hardcoverUserBookId: null,
|
||||
hardcoverReadId: null,
|
||||
matchMethod: 'metadata_id',
|
||||
matchBasis: input.matchBasis,
|
||||
matchError: null,
|
||||
lastSyncedAt: null,
|
||||
}),
|
||||
);
|
||||
expect(stateSet.mock.calls[0]![0]).not.toHaveProperty('syncOverride');
|
||||
});
|
||||
|
||||
it('releases only live Hardcover links on that user and book, and counts them', async () => {
|
||||
const { repo, tx, attemptSet, attemptWhere } = makeTransactionalRepository([3, 4]);
|
||||
|
||||
await expect(repo.relinkBook(input)).resolves.toBe(2);
|
||||
|
||||
expect(tx.update).toHaveBeenNthCalledWith(2, schema.readingAttempts);
|
||||
expect(attemptSet).toHaveBeenCalledWith(expect.objectContaining({ externalProvider: null, externalId: null }));
|
||||
const where = new PgDialect().sqlToQuery(attemptWhere.mock.calls[0]![0]);
|
||||
expect(where.sql).toContain('"reading_attempts"."deleted_at" is null');
|
||||
expect(where.params).toEqual([7, 42, 'hardcover']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('unlinkReadingAttempt', () => {
|
||||
it('releases the attempt only while it still points at the rejected read', async () => {
|
||||
const { repo, updateChain, updateWhere } = makeRepository();
|
||||
|
||||
await expect(repo.unlinkReadingAttempt(7, 3, 555)).resolves.toBe(true);
|
||||
|
||||
expect(updateChain.set).toHaveBeenCalledWith(expect.objectContaining({ externalProvider: null, externalId: null }));
|
||||
expect(new PgDialect().sqlToQuery(updateWhere.mock.calls[0]![0]).params).toEqual([7, 3, 'hardcover', '555']);
|
||||
});
|
||||
|
||||
it('reports nothing released when the link already moved on', async () => {
|
||||
const { repo, updateReturning } = makeRepository();
|
||||
updateReturning.mockResolvedValue([]);
|
||||
|
||||
await expect(repo.unlinkReadingAttempt(7, 3, 555)).resolves.toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('findClaimedHardcoverReadIds', () => {
|
||||
function withRows(db: { select: ReturnType<typeof vi.fn> }, rows: Array<{ externalId: string | null }>) {
|
||||
db.select.mockReturnValue({ from: vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(rows) }) });
|
||||
|
||||
@@ -46,6 +46,7 @@ export interface BookSyncData {
|
||||
title: string | null;
|
||||
authorName: string | null;
|
||||
hardcoverMetadataId: string | null;
|
||||
hardcoverEditionId?: string | null;
|
||||
pageCount: number | null;
|
||||
format: string | null;
|
||||
status: string;
|
||||
@@ -157,6 +158,53 @@ export class HardcoverRepository {
|
||||
return row != null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Moves a user's link to a different Hardcover book. The user book, read and every live attempt's
|
||||
* read link all point into the old book's Hardcover entry, so they are released with it; reusing
|
||||
* them would keep writing progress to the old book. Soft-deleted attempts keep their ids as
|
||||
* import tombstones. The new book counts as never synced, so a failed first sync is retried.
|
||||
* Returns how many attempts were released.
|
||||
*/
|
||||
async relinkBook(input: {
|
||||
userId: number;
|
||||
bookId: number;
|
||||
hardcoverBookId: number;
|
||||
hardcoverEditionId: number | null;
|
||||
matchMethod: string;
|
||||
matchBasis: string;
|
||||
}): Promise<number> {
|
||||
return this.db.transaction(async (tx) => {
|
||||
await tx
|
||||
.update(schema.hardcoverBookState)
|
||||
.set({
|
||||
hardcoverBookId: input.hardcoverBookId,
|
||||
hardcoverEditionId: input.hardcoverEditionId,
|
||||
hardcoverUserBookId: null,
|
||||
hardcoverReadId: null,
|
||||
matchMethod: input.matchMethod,
|
||||
matchBasis: input.matchBasis,
|
||||
matchError: null,
|
||||
lastSyncedAt: null,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(and(eq(schema.hardcoverBookState.userId, input.userId), eq(schema.hardcoverBookState.bookId, input.bookId)));
|
||||
|
||||
const released = await tx
|
||||
.update(schema.readingAttempts)
|
||||
.set({ externalProvider: null, externalId: null, updatedAt: new Date() })
|
||||
.where(
|
||||
and(
|
||||
eq(schema.readingAttempts.userId, input.userId),
|
||||
eq(schema.readingAttempts.bookId, input.bookId),
|
||||
eq(schema.readingAttempts.externalProvider, HARDCOVER_EXTERNAL_PROVIDER),
|
||||
isNull(schema.readingAttempts.deletedAt),
|
||||
),
|
||||
)
|
||||
.returning({ id: schema.readingAttempts.id });
|
||||
return released.length;
|
||||
});
|
||||
}
|
||||
|
||||
async setBookSyncOverride(userId: number, bookId: number, syncOverride: 'included' | 'excluded' | null): Promise<HardcoverBookState> {
|
||||
const syncExcluded = syncOverride === 'excluded';
|
||||
const [row] = await this.db
|
||||
@@ -253,6 +301,7 @@ export class HardcoverRepository {
|
||||
title: schema.bookMetadata.title,
|
||||
authorName: firstAuthorSq.authorName,
|
||||
hardcoverMetadataId: schema.bookMetadata.hardcoverId,
|
||||
hardcoverEditionId: schema.bookMetadata.hardcoverEditionId,
|
||||
pageCount: schema.bookMetadata.pageCount,
|
||||
format: schema.bookFiles.format,
|
||||
status: includeUnread ? sql<string>`coalesce(${schema.userBookStatus.status}, 'unread')` : schema.userBookStatus.status,
|
||||
@@ -316,7 +365,11 @@ export class HardcoverRepository {
|
||||
eq(schema.readingAttempts.id, attemptId),
|
||||
isNull(schema.readingAttempts.deletedAt),
|
||||
or(
|
||||
and(isNull(schema.readingAttempts.externalProvider), isNull(schema.readingAttempts.externalId)),
|
||||
// A Hardcover provider without an id owns nothing; hand-cleared links are left in this shape.
|
||||
and(
|
||||
or(isNull(schema.readingAttempts.externalProvider), eq(schema.readingAttempts.externalProvider, HARDCOVER_EXTERNAL_PROVIDER)),
|
||||
isNull(schema.readingAttempts.externalId),
|
||||
),
|
||||
and(eq(schema.readingAttempts.externalProvider, HARDCOVER_EXTERNAL_PROVIDER), eq(schema.readingAttempts.externalId, externalId)),
|
||||
),
|
||||
),
|
||||
@@ -329,6 +382,23 @@ export class HardcoverRepository {
|
||||
}
|
||||
}
|
||||
|
||||
/** Releases one attempt's link, but only while it still points at the read the caller rejected. */
|
||||
async unlinkReadingAttempt(userId: number, attemptId: number, hardcoverReadId: number): Promise<boolean> {
|
||||
const rows = await this.db
|
||||
.update(schema.readingAttempts)
|
||||
.set({ externalProvider: null, externalId: null, updatedAt: new Date() })
|
||||
.where(
|
||||
and(
|
||||
eq(schema.readingAttempts.userId, userId),
|
||||
eq(schema.readingAttempts.id, attemptId),
|
||||
eq(schema.readingAttempts.externalProvider, HARDCOVER_EXTERNAL_PROVIDER),
|
||||
eq(schema.readingAttempts.externalId, String(hardcoverReadId)),
|
||||
),
|
||||
)
|
||||
.returning({ id: schema.readingAttempts.id });
|
||||
return rows.length > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Read ids already spoken for on this book. Soft-deleted attempts are included on purpose:
|
||||
* reading_attempts_external_uidx has no deleted_at predicate, and a deleted attempt keeping its
|
||||
|
||||
@@ -629,6 +629,136 @@ describe('Reading attempts main-flow simulation (docker e2e)', { timeout: TIMEOU
|
||||
expect(sync?.attemptsUpdatedAt ?? null).toBeNull();
|
||||
});
|
||||
|
||||
it('25a. moves a Hardcover link to another book and releases only its live read links', async () => {
|
||||
const book = await createBook();
|
||||
const attempts = ctx.app.get(ReadingAttemptService);
|
||||
const hardcover = ctx.app.get(HardcoverRepository);
|
||||
const finished = await attempts.createHistorical(adminUserId, book.bookId, {
|
||||
startedOn: '2025-01-01',
|
||||
endedOn: '2025-01-09',
|
||||
outcome: 'completed',
|
||||
});
|
||||
const removed = await attempts.createHistorical(adminUserId, book.bookId, {
|
||||
startedOn: '2025-02-01',
|
||||
endedOn: '2025-02-09',
|
||||
outcome: 'completed',
|
||||
});
|
||||
const otherProvider = await attempts.createHistorical(adminUserId, book.bookId, {
|
||||
startedOn: '2025-03-01',
|
||||
endedOn: '2025-03-09',
|
||||
outcome: 'completed',
|
||||
});
|
||||
await hardcover.linkReadingAttempt(adminUserId, finished.id, 9601);
|
||||
await hardcover.linkReadingAttempt(adminUserId, removed.id, 9602);
|
||||
await attempts.delete(adminUserId, book.bookId, removed.id);
|
||||
await ctx.db
|
||||
.update(schema.readingAttempts)
|
||||
.set({ externalProvider: 'storygraph', externalId: 'sg-1' })
|
||||
.where(eq(schema.readingAttempts.id, otherProvider.id));
|
||||
await hardcover.upsertBookState({
|
||||
userId: adminUserId,
|
||||
bookId: book.bookId,
|
||||
hardcoverBookId: 1554702,
|
||||
hardcoverEditionId: 31616496,
|
||||
hardcoverUserBookId: 19544253,
|
||||
hardcoverReadId: 9601,
|
||||
matchMethod: 'cached',
|
||||
matchBasis: '["hell-be-ok",null,null]',
|
||||
syncOverride: 'included',
|
||||
lastSyncedAt: new Date('2026-10-01T00:00:00Z'),
|
||||
lastSyncedProgress: 13.82,
|
||||
});
|
||||
|
||||
const released = await hardcover.relinkBook({
|
||||
userId: adminUserId,
|
||||
bookId: book.bookId,
|
||||
hardcoverBookId: 2973670,
|
||||
hardcoverEditionId: 33314466,
|
||||
matchMethod: 'metadata_id',
|
||||
matchBasis: '["ok-amen","9789146223856",null]',
|
||||
});
|
||||
|
||||
expect(released).toBe(1);
|
||||
await expect(hardcover.findBookState(adminUserId, book.bookId)).resolves.toMatchObject({
|
||||
hardcoverBookId: 2973670,
|
||||
hardcoverEditionId: 33314466,
|
||||
hardcoverUserBookId: null,
|
||||
hardcoverReadId: null,
|
||||
matchMethod: 'metadata_id',
|
||||
matchBasis: '["ok-amen","9789146223856",null]',
|
||||
syncOverride: 'included',
|
||||
lastSyncedAt: null,
|
||||
lastSyncedProgress: 13.82,
|
||||
});
|
||||
const rows = await ctx.db.select().from(schema.readingAttempts).where(eq(schema.readingAttempts.bookId, book.bookId));
|
||||
const byId = new Map(rows.map((row) => [row.id, row]));
|
||||
expect(byId.get(finished.id)).toMatchObject({ externalProvider: null, externalId: null });
|
||||
// The soft-deleted attempt stays a tombstone, and another provider's link is not ours to drop.
|
||||
expect(byId.get(removed.id)).toMatchObject({ externalProvider: 'hardcover', externalId: '9602' });
|
||||
expect(byId.get(otherProvider.id)).toMatchObject({ externalProvider: 'storygraph', externalId: 'sg-1' });
|
||||
await expect(hardcover.findClaimedHardcoverReadIds(adminUserId, book.bookId)).resolves.toEqual([9602]);
|
||||
await expect(hardcover.linkReadingAttempt(adminUserId, finished.id, 9603)).resolves.toBe('linked');
|
||||
});
|
||||
|
||||
it('25b. releases one attempt link only while it still points at the rejected read', async () => {
|
||||
const book = await createBook();
|
||||
const attempts = ctx.app.get(ReadingAttemptService);
|
||||
const hardcover = ctx.app.get(HardcoverRepository);
|
||||
const attempt = await attempts.createHistorical(adminUserId, book.bookId, {
|
||||
startedOn: '2025-04-01',
|
||||
endedOn: '2025-04-09',
|
||||
outcome: 'completed',
|
||||
});
|
||||
await hardcover.linkReadingAttempt(adminUserId, attempt.id, 9701);
|
||||
|
||||
await expect(hardcover.unlinkReadingAttempt(adminUserId, attempt.id, 9799)).resolves.toBe(false);
|
||||
await expect(hardcover.findClaimedHardcoverReadIds(adminUserId, book.bookId)).resolves.toEqual([9701]);
|
||||
|
||||
await expect(hardcover.unlinkReadingAttempt(adminUserId, attempt.id, 9701)).resolves.toBe(true);
|
||||
const [row] = await ctx.db.select().from(schema.readingAttempts).where(eq(schema.readingAttempts.id, attempt.id));
|
||||
expect(row).toMatchObject({ externalProvider: null, externalId: null });
|
||||
});
|
||||
|
||||
it('25c. lets a Hardcover link cleared down to its provider be claimed again', async () => {
|
||||
const book = await createBook();
|
||||
const attempts = ctx.app.get(ReadingAttemptService);
|
||||
const hardcover = ctx.app.get(HardcoverRepository);
|
||||
const halfCleared = await attempts.createHistorical(adminUserId, book.bookId, {
|
||||
startedOn: '2025-05-01',
|
||||
endedOn: '2025-05-09',
|
||||
outcome: 'completed',
|
||||
});
|
||||
const otherProvider = await attempts.createHistorical(adminUserId, book.bookId, {
|
||||
startedOn: '2025-06-01',
|
||||
endedOn: '2025-06-09',
|
||||
outcome: 'completed',
|
||||
});
|
||||
await ctx.db
|
||||
.update(schema.readingAttempts)
|
||||
.set({ externalProvider: 'hardcover', externalId: null })
|
||||
.where(eq(schema.readingAttempts.id, halfCleared.id));
|
||||
await ctx.db
|
||||
.update(schema.readingAttempts)
|
||||
.set({ externalProvider: 'storygraph', externalId: null })
|
||||
.where(eq(schema.readingAttempts.id, otherProvider.id));
|
||||
|
||||
await expect(hardcover.linkReadingAttempt(adminUserId, halfCleared.id, 9801)).resolves.toBe('linked');
|
||||
await expect(hardcover.linkReadingAttempt(adminUserId, otherProvider.id, 9802)).resolves.toBe('conflict');
|
||||
});
|
||||
|
||||
it('25d. carries the metadata Hardcover edition into the sync snapshot', async () => {
|
||||
const book = await createBook();
|
||||
const hardcover = ctx.app.get(HardcoverRepository);
|
||||
await ctx.db
|
||||
.insert(schema.bookMetadata)
|
||||
.values({ bookId: book.bookId, title: 'ok, amen', hardcoverId: 'ok-amen', hardcoverEditionId: '33314466' });
|
||||
|
||||
await expect(hardcover.findBookSyncData(adminUserId, book.bookId)).resolves.toMatchObject({
|
||||
hardcoverMetadataId: 'ok-amen',
|
||||
hardcoverEditionId: '33314466',
|
||||
});
|
||||
});
|
||||
|
||||
it('26. lets lifecycle dates create and complete attempts through the status API', async () => {
|
||||
const startedOnly = await createBook();
|
||||
const startResponse = await patchStatus(startedOnly.bookId, { startedAt: '2026-01-10' });
|
||||
|
||||
Reference in New Issue
Block a user