fix(metadata): stop hardcover sync colliding on reading attempt read ids

Read selection was positional: prefer any open remote read, else a cached book-level id, else the
first read returned, while the primary attempt was re-derived each sync from book dates. The two
could disagree, assigning one Hardcover read to two attempts and violating
reading_attempts_external_uidx, which aborted the sync and persisted the failure.

Treat reading_attempts.external_id as the mapping of record: an attempt keeps the read it owns, an
unlinked attempt adopts only an unclaimed read and claims it locally before the remote write, and a
fresh read is minted when nothing is adoptable. Never merge or delete attempts to resolve a
conflict, and keep soft-deleted ids reserved so importExternalRead tombstones still hold.

Also skip the link write when an attempt already owns its read, and map the aggregated attempt
timestamp to a Date so change detection stops comparing a driver string against lastSyncedAt.

Closes #907
This commit is contained in:
neon
2026-08-21 18:18:29 +00:00
parent 9d116c3e97
commit cbd5f56778
8 changed files with 1136 additions and 46 deletions
@@ -0,0 +1,190 @@
import { describe, expect, it } from 'vitest';
import {
attemptOwnedReadId,
isReadCompatible,
readTargetForAttempt,
readTargetForBook,
selectAdoptableReadId,
selectPrimaryAttempt,
} from './hardcover-read-selection';
function read(id: number, started_at: string | null, finished_at: string | null) {
return { id, started_at, finished_at };
}
function attempt(overrides: Partial<Parameters<typeof readTargetForAttempt>[0]> & { id?: number } = {}) {
return {
id: 1,
startedOn: null,
endedOn: null,
outcome: null,
externalProvider: null,
externalId: null,
...overrides,
} as {
id: number;
startedOn: string | null;
endedOn: string | null;
outcome: 'completed' | 'skimmed' | 'abandoned' | null;
externalProvider: string | null;
externalId: string | null;
};
}
describe('selectPrimaryAttempt', () => {
it('returns null when there are no attempts', () => {
expect(selectPrimaryAttempt([])).toBeNull();
});
it('prefers the open attempt over a higher-id closed one', () => {
const open = attempt({ id: 2, outcome: null });
const closed = attempt({ id: 9, outcome: 'completed' });
expect(selectPrimaryAttempt([closed, open])?.id).toBe(2);
});
it('falls back to the highest id when no attempt is open', () => {
const first = attempt({ id: 3, outcome: 'completed' });
const second = attempt({ id: 7, outcome: 'completed' });
expect(selectPrimaryAttempt([second, first])?.id).toBe(7);
});
it('lets a skimmed or abandoned attempt be primary, since both map to a Hardcover status', () => {
expect(selectPrimaryAttempt([attempt({ id: 4, outcome: 'skimmed' })])?.id).toBe(4);
expect(selectPrimaryAttempt([attempt({ id: 5, outcome: 'abandoned' })])?.id).toBe(5);
});
it('does not depend on input ordering', () => {
const attempts = [attempt({ id: 1, outcome: 'completed' }), attempt({ id: 8, outcome: null }), attempt({ id: 4, outcome: 'abandoned' })];
expect(selectPrimaryAttempt(attempts)?.id).toBe(8);
expect(selectPrimaryAttempt([...attempts].reverse())?.id).toBe(8);
});
});
describe('attemptOwnedReadId', () => {
it('reads the id an attempt already owns', () => {
expect(attemptOwnedReadId({ externalProvider: 'hardcover', externalId: '555' })).toBe(555);
});
it('ignores stamps from other providers', () => {
expect(attemptOwnedReadId({ externalProvider: 'kobo', externalId: '555' })).toBeNull();
});
it('ignores missing or unusable ids', () => {
expect(attemptOwnedReadId({ externalProvider: 'hardcover', externalId: null })).toBeNull();
expect(attemptOwnedReadId({ externalProvider: 'hardcover', externalId: 'abc' })).toBeNull();
expect(attemptOwnedReadId({ externalProvider: 'hardcover', externalId: '0' })).toBeNull();
expect(attemptOwnedReadId({ externalProvider: 'hardcover', externalId: '-3' })).toBeNull();
});
});
describe('readTargetForAttempt', () => {
it('treats only a completed attempt as finished', () => {
expect(readTargetForAttempt(attempt({ startedOn: '2024-01-01', endedOn: '2024-01-05', outcome: 'completed' }))).toEqual({
startedOn: '2024-01-01',
endedOn: '2024-01-05',
finished: true,
});
});
it('keeps a completed attempt with an unknown finish date open remotely', () => {
expect(readTargetForAttempt(attempt({ startedOn: '2024-01-01', endedOn: null, outcome: 'completed' }))).toEqual({
startedOn: '2024-01-01',
endedOn: null,
finished: false,
});
});
it('drops the end date for skimmed and abandoned attempts, matching what the sync sends', () => {
expect(readTargetForAttempt(attempt({ startedOn: '2024-01-01', endedOn: '2024-01-05', outcome: 'skimmed' }))).toEqual({
startedOn: '2024-01-01',
endedOn: null,
finished: false,
});
expect(readTargetForAttempt(attempt({ startedOn: '2024-01-01', endedOn: '2024-01-05', outcome: 'abandoned' }))).toEqual({
startedOn: '2024-01-01',
endedOn: null,
finished: false,
});
});
it('treats an open attempt as unfinished', () => {
expect(readTargetForAttempt(attempt({ startedOn: '2024-02-01', outcome: null }))).toEqual({
startedOn: '2024-02-01',
endedOn: null,
finished: false,
});
});
});
describe('readTargetForBook', () => {
it('is finished exactly when the book has an end date', () => {
expect(readTargetForBook('2024-01-01', '2024-01-05').finished).toBe(true);
expect(readTargetForBook('2024-01-01', null).finished).toBe(false);
});
});
describe('isReadCompatible', () => {
it('requires open and finished state to agree', () => {
const openTarget = readTargetForBook('2024-01-01', null);
expect(isReadCompatible(read(1, '2024-01-01', null), openTarget)).toBe(true);
expect(isReadCompatible(read(1, '2024-01-01', '2024-01-05'), openTarget)).toBe(false);
});
it('rejects a finished read whose end date differs', () => {
const target = readTargetForBook('2024-01-01', '2024-01-05');
expect(isReadCompatible(read(1, '2024-01-01', '2024-01-05'), target)).toBe(true);
expect(isReadCompatible(read(1, '2024-01-01', '2024-02-09'), target)).toBe(false);
});
it('rejects a read whose start date differs', () => {
const target = readTargetForBook('2024-03-01', null);
expect(isReadCompatible(read(1, '2024-01-01', null), target)).toBe(false);
});
it('tolerates dates missing on either side rather than treating absence as evidence', () => {
expect(isReadCompatible(read(1, null, null), readTargetForBook('2024-01-01', null))).toBe(true);
expect(isReadCompatible(read(1, '2024-01-01', null), readTargetForBook(null, null))).toBe(true);
});
});
describe('selectAdoptableReadId', () => {
const openTarget = readTargetForBook('2024-01-01', null);
it('returns null when nothing is compatible', () => {
expect(selectAdoptableReadId([read(1, '2024-01-01', '2024-01-05')], openTarget, new Set())).toBeNull();
});
it('skips reads another attempt already owns', () => {
expect(selectAdoptableReadId([read(555, '2024-01-01', null)], openTarget, new Set([555]))).toBeNull();
});
it('adopts a compatible unclaimed read', () => {
expect(selectAdoptableReadId([read(777, '2024-01-01', null)], openTarget, new Set())).toBe(777);
});
it('prefers the candidate with more explicitly matching dates', () => {
const reads = [read(900, null, null), read(777, '2024-01-01', null)];
expect(selectAdoptableReadId(reads, openTarget, new Set())).toBe(777);
});
it('falls back to the highest id when evidence is equal', () => {
const reads = [read(777, '2024-01-01', null), read(778, '2024-01-01', null)];
expect(selectAdoptableReadId(reads, openTarget, new Set())).toBe(778);
});
it('matches a completed attempt to its finished read', () => {
const target = readTargetForAttempt(attempt({ startedOn: '2024-01-01', endedOn: '2024-01-05', outcome: 'completed' }));
const reads = [read(700, '2024-01-01', '2024-01-05'), read(701, '2024-01-01', null)];
expect(selectAdoptableReadId(reads, target, new Set())).toBe(700);
});
});
@@ -0,0 +1,106 @@
import type { ReadingAttemptOutcome } from '@bookorbit/types';
export const HARDCOVER_EXTERNAL_PROVIDER = 'hardcover';
export interface HardcoverReadCandidate {
id: number;
started_at: string | null;
finished_at: string | null;
}
export interface ReadingAttemptForSync {
id: number;
startedOn: string | null;
endedOn: string | null;
outcome: ReadingAttemptOutcome | null;
externalProvider: string | null;
externalId: string | null;
}
/**
* The shape a Hardcover read must have to represent a given local attempt. `finished` mirrors what
* the sync sends as `finished_at`, which is only ever populated for a completed attempt.
*/
export interface HardcoverReadTarget {
startedOn: string | null;
endedOn: string | null;
finished: boolean;
}
/**
* Mirrors the `active ?? latest` rule that rebuildProjection uses to decide which attempt drives
* user_book_status. The book-level dates the sync pushes to Hardcover are that attempt's dates, so
* both sides must agree on the choice or the sync links a read to the wrong attempt.
*/
export function selectPrimaryAttempt<T extends { id: number; outcome: ReadingAttemptOutcome | null }>(attempts: readonly T[]): T | null {
let active: T | null = null;
let latest: T | null = null;
for (const attempt of attempts) {
if (attempt.outcome === null && (active === null || attempt.id > active.id)) active = attempt;
if (latest === null || attempt.id > latest.id) latest = attempt;
}
return active ?? latest;
}
export function attemptOwnedReadId(attempt: Pick<ReadingAttemptForSync, 'externalProvider' | 'externalId'>): number | null {
if (attempt.externalProvider !== HARDCOVER_EXTERNAL_PROVIDER || !attempt.externalId) return null;
const parsed = Number(attempt.externalId);
return Number.isInteger(parsed) && parsed > 0 ? parsed : null;
}
export function readTargetForAttempt(attempt: Pick<ReadingAttemptForSync, 'startedOn' | 'endedOn' | 'outcome'>): HardcoverReadTarget {
const finished = attempt.outcome === 'completed' && attempt.endedOn !== null;
return { startedOn: attempt.startedOn, endedOn: finished ? attempt.endedOn : null, finished };
}
export function readTargetForBook(startedOn: string | null, endedOn: string | null): HardcoverReadTarget {
return { startedOn, endedOn, finished: endedOn !== null };
}
/**
* Adoption is only safe when the remote read plausibly *is* this target: open/finished state must
* agree, and any date both sides state must be equal. Dates absent on either side are not evidence
* against a match, since Hardcover reads routinely omit them.
*/
export function isReadCompatible(read: HardcoverReadCandidate, target: HardcoverReadTarget): boolean {
if (Boolean(read.finished_at) !== target.finished) return false;
if (read.started_at && target.startedOn && read.started_at !== target.startedOn) return false;
if (read.finished_at && target.endedOn && read.finished_at !== target.endedOn) return false;
return true;
}
function explicitMatchScore(read: HardcoverReadCandidate, target: HardcoverReadTarget): number {
let score = 0;
if (read.started_at && target.startedOn && read.started_at === target.startedOn) score++;
if (read.finished_at && target.endedOn && read.finished_at === target.endedOn) score++;
return score;
}
/**
* Picks a remote read this target may take ownership of. `unavailable` carries every read id already
* owned by another attempt - including soft-deleted ones, whose external ids stay reserved as
* tombstones - plus everything claimed earlier in the same sync run.
*/
export function selectAdoptableReadId(
reads: readonly HardcoverReadCandidate[],
target: HardcoverReadTarget,
unavailable: ReadonlySet<number>,
): number | null {
let best: HardcoverReadCandidate | null = null;
let bestScore = -1;
for (const read of reads) {
if (unavailable.has(read.id)) continue;
if (!isReadCompatible(read, target)) continue;
const score = explicitMatchScore(read, target);
if (score > bestScore) {
best = read;
bestScore = score;
continue;
}
if (score < bestScore || best === null) continue;
// Equal evidence: keep the highest id, matching the descending order Hardcover returns. The
// book-level cached id is deliberately not consulted because it is not attempt identity.
if (read.id > best.id) best = read;
}
return best?.id ?? null;
}
@@ -14,6 +14,7 @@ const mockRepo = {
findBookSyncData: vi.fn(),
findCurrentReadingBooks: vi.fn(),
findReadingAttempts: vi.fn(),
findClaimedHardcoverReadIds: vi.fn(),
linkReadingAttempt: vi.fn(),
};
@@ -76,7 +77,8 @@ describe('HardcoverSyncService', () => {
mockRepo.findBookSyncData.mockResolvedValue(null);
mockRepo.findCurrentReadingBooks.mockResolvedValue([]);
mockRepo.findReadingAttempts.mockResolvedValue([]);
mockRepo.linkReadingAttempt.mockResolvedValue(undefined);
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([]);
mockRepo.linkReadingAttempt.mockResolvedValue('linked');
mockRepo.upsertBookState.mockResolvedValue({});
mockRepo.setBookSyncOverride.mockResolvedValue({});
mockRepo.updateLastSyncedAt.mockResolvedValue(undefined);
@@ -347,6 +349,63 @@ describe('HardcoverSyncService', () => {
logSpy.mockRestore();
});
it('does not fan primary progress out to an open read reserved by another attempt', async () => {
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue({ hardcoverReadId: 900 });
mockRepo.findReadingAttempts.mockResolvedValue([
{
id: 2,
startedOn: '2024-01-01',
endedOn: null,
outcome: null,
externalProvider: 'hardcover',
externalId: '900',
},
]);
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([899, 900]);
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: 20, editionPages: 300, matchMethod: 'cached' });
mockClient.query
.mockResolvedValueOnce({ insert_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ update_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({
user_book_reads: [
{ id: 900, started_at: '2024-01-01', finished_at: null, progress_pages: null },
{ id: 899, started_at: '2023-01-01', finished_at: null, progress_pages: null },
],
})
.mockResolvedValueOnce({ update_user_book_read: { user_book_read: { id: 900 }, error: null } });
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
expect(
mockClient.query.mock.calls.some(
([, , query, variables]) =>
typeof query === 'string' && query.includes('mutation UpdateUserBookRead') && (variables as { id?: number } | undefined)?.id === 899,
),
).toBe(false);
});
it('fails without inserting a read when Hardcover read discovery fails', async () => {
const errorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(undefined);
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: 20, editionPages: 300, matchMethod: 'cached' });
mockClient.query
.mockResolvedValueOnce({ insert_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ update_user_book: { user_book: { id: 55 }, error: null } })
.mockRejectedValueOnce(new Error('read discovery failed'));
await expect(makeService().syncBook(1, 1)).resolves.toBe('failed');
expect(mockClient.query.mock.calls.some(([, , query]) => typeof query === 'string' && query.includes('mutation InsertUserBookRead'))).toBe(
false,
);
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ syncError: 'read discovery failed' }));
errorSpy.mockRestore();
});
it('stores error when edition pages are unavailable', async () => {
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
@@ -807,4 +866,311 @@ describe('HardcoverSyncService', () => {
await expect(makeService().setEdition(1, 1, 200)).resolves.toEqual({ success: true });
});
});
describe('reading attempt to Hardcover read mapping', () => {
type AttemptRow = {
id: number;
startedOn: string | null;
endedOn: string | null;
outcome: 'completed' | 'skimmed' | 'abandoned' | null;
externalProvider: string | null;
externalId: string | null;
};
function attempt(id: number, overrides: Partial<AttemptRow> = {}): AttemptRow {
return { id, startedOn: null, endedOn: null, outcome: null, externalProvider: null, externalId: null, ...overrides };
}
function linked(id: number, readId: number, overrides: Partial<AttemptRow> = {}): AttemptRow {
return attempt(id, { externalProvider: 'hardcover', externalId: String(readId), ...overrides });
}
const rereadingBook = { ...readingBook, status: 'rereading', startedAt: new Date('2024-02-01'), finishedAt: null };
const match = { hardcoverBookId: 10, hardcoverEditionId: 20, editionPages: 300, matchMethod: 'cached' };
function arrange(reads: Array<{ id: number; started_at: string | null; finished_at: string | null; progress_pages: number | null }>) {
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockMatchService.matchBook.mockResolvedValue(match);
mockClient.query
.mockResolvedValueOnce({ insert_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ update_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ user_book_reads: reads });
}
function insertRead(id: number) {
mockClient.query.mockResolvedValueOnce({ insert_user_book_read: { user_book_read: { id }, error: null } });
}
function updateRead(id: number) {
mockClient.query.mockResolvedValueOnce({ update_user_book_read: { user_book_read: { id }, error: null } });
}
function linkCalls() {
return mockRepo.linkReadingAttempt.mock.calls as Array<[number, number, number]>;
}
it("gives a reread its own read instead of stamping the finished attempt's id onto it", async () => {
mockRepo.findBookSyncData.mockResolvedValue(rereadingBook);
mockRepo.findBookState.mockResolvedValue({ hardcoverReadId: 555 });
mockRepo.findReadingAttempts.mockResolvedValue([
linked(1, 555, { startedOn: '2024-01-01', endedOn: '2024-01-05', outcome: 'completed' }),
attempt(2, { startedOn: '2024-02-01' }),
]);
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([555]);
arrange([{ id: 555, started_at: '2024-01-01', finished_at: '2024-01-05', progress_pages: 300 }]);
insertRead(556);
updateRead(555);
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
expect(linkCalls()).toEqual([[1, 2, 556]]);
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ hardcoverReadId: 556, syncError: null }));
});
it('keeps the finished attempt on its original read rather than merging the two', async () => {
mockRepo.findBookSyncData.mockResolvedValue(rereadingBook);
mockRepo.findBookState.mockResolvedValue({ hardcoverReadId: 555 });
mockRepo.findReadingAttempts.mockResolvedValue([
linked(1, 555, { startedOn: '2024-01-01', endedOn: '2024-01-05', outcome: 'completed' }),
attempt(2, { startedOn: '2024-02-01' }),
]);
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([555]);
arrange([{ id: 555, started_at: '2024-01-01', finished_at: '2024-01-05', progress_pages: 300 }]);
insertRead(556);
updateRead(555);
await makeService().syncBook(1, 1);
expect(mockClient.query).toHaveBeenNthCalledWith(
5,
1,
'tok',
expect.stringContaining('mutation UpdateUserBookRead'),
expect.objectContaining({ id: 555 }),
);
expect(linkCalls().some(([, attemptId]) => attemptId === 1)).toBe(false);
});
it("does not let a stale cached read id override an attempt's own mapping", async () => {
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue({ hardcoverReadId: 900 });
mockRepo.findReadingAttempts.mockResolvedValue([linked(1, 777, { startedOn: '2024-01-01' })]);
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([777]);
arrange([
{ id: 900, started_at: '2024-01-01', finished_at: null, progress_pages: null },
{ id: 777, started_at: '2024-01-01', finished_at: null, progress_pages: null },
]);
updateRead(777);
updateRead(900);
await makeService().syncBook(1, 1);
expect(mockClient.query).toHaveBeenNthCalledWith(
4,
1,
'tok',
expect.stringContaining('mutation UpdateUserBookRead'),
expect.objectContaining({ id: 777 }),
);
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ hardcoverReadId: 777 }));
});
it('leaves a read reserved by a soft-deleted attempt alone', async () => {
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(undefined);
mockRepo.findReadingAttempts.mockResolvedValue([attempt(2, { startedOn: '2024-01-01' })]);
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([555]);
arrange([{ id: 555, started_at: '2024-01-01', finished_at: null, progress_pages: null }]);
insertRead(601);
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
expect(linkCalls()).toEqual([[1, 2, 601]]);
});
it('adopts an unclaimed matching read instead of creating a duplicate', async () => {
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(undefined);
mockRepo.findReadingAttempts.mockResolvedValue([attempt(2, { startedOn: '2024-01-01' })]);
arrange([{ id: 777, started_at: '2024-01-01', finished_at: null, progress_pages: null }]);
updateRead(777);
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
expect(linkCalls()).toEqual([[1, 2, 777]]);
expect(mockClient.query).not.toHaveBeenCalledWith(1, 'tok', expect.stringContaining('mutation InsertUserBookRead'), expect.anything());
});
it('claims a candidate locally before editing it on Hardcover', async () => {
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(undefined);
mockRepo.findReadingAttempts.mockResolvedValue([attempt(2, { startedOn: '2024-01-01' })]);
arrange([{ id: 777, started_at: '2024-01-01', finished_at: null, progress_pages: null }]);
const order: string[] = [];
mockRepo.linkReadingAttempt.mockImplementation(() => {
order.push('link');
return Promise.resolve('linked');
});
mockClient.query.mockImplementationOnce(() => {
order.push('remote-write');
return Promise.resolve({ update_user_book_read: { user_book_read: { id: 777 }, error: null } });
});
await makeService().syncBook(1, 1);
expect(order).toEqual(['link', 'remote-write']);
});
it('gives competing unlinked attempts distinct reads', async () => {
mockRepo.findBookSyncData.mockResolvedValue({ ...rereadingBook, startedAt: new Date('2024-03-01') });
mockRepo.findBookState.mockResolvedValue(undefined);
mockRepo.findReadingAttempts.mockResolvedValue([
attempt(1, { startedOn: '2024-01-01', endedOn: '2024-01-05', outcome: 'completed' }),
attempt(2, { startedOn: '2024-01-01', endedOn: '2024-01-05', outcome: 'completed' }),
attempt(3, { startedOn: '2024-03-01' }),
]);
arrange([{ id: 700, started_at: '2024-01-01', finished_at: '2024-01-05', progress_pages: 300 }]);
insertRead(800);
updateRead(700);
insertRead(801);
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
const readIds = linkCalls().map(([, , readId]) => readId);
expect(readIds).toEqual([800, 700, 801]);
expect(new Set(readIds).size).toBe(readIds.length);
});
it('writes no link when every attempt already owns its read, so repeat syncs do not churn', async () => {
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue({ hardcoverReadId: 777 });
mockRepo.findReadingAttempts.mockResolvedValue([
linked(1, 700, { startedOn: '2023-05-01', endedOn: '2023-05-09', outcome: 'completed' }),
linked(2, 777, { startedOn: '2024-01-01' }),
]);
mockRepo.findClaimedHardcoverReadIds.mockResolvedValue([700, 777]);
arrange([
{ id: 777, started_at: '2024-01-01', finished_at: null, progress_pages: null },
{ id: 700, started_at: '2023-05-01', finished_at: '2023-05-09', progress_pages: 300 },
]);
updateRead(777);
updateRead(700);
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
expect(mockRepo.linkReadingAttempt).not.toHaveBeenCalled();
});
it('reselects another read when a claim loses the race', async () => {
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(undefined);
mockRepo.findReadingAttempts.mockResolvedValue([attempt(2, { startedOn: '2024-01-01' })]);
mockRepo.linkReadingAttempt.mockResolvedValueOnce('conflict').mockResolvedValueOnce('linked');
arrange([
{ id: 778, started_at: '2024-01-01', finished_at: null, progress_pages: null },
{ id: 777, started_at: '2024-01-01', finished_at: null, progress_pages: null },
]);
updateRead(777);
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
expect(linkCalls()).toEqual([
[1, 2, 778],
[1, 2, 777],
]);
expect(mockClient.query).toHaveBeenNthCalledWith(
4,
1,
'tok',
expect.stringContaining('mutation UpdateUserBookRead'),
expect.objectContaining({ id: 777 }),
);
});
it('creates a fresh read once reselection is exhausted rather than sharing one', async () => {
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(undefined);
mockRepo.findReadingAttempts.mockResolvedValue([attempt(2, { startedOn: '2024-01-01' })]);
mockRepo.linkReadingAttempt.mockResolvedValueOnce('conflict').mockResolvedValueOnce('conflict').mockResolvedValueOnce('conflict');
arrange([
{ id: 778, started_at: '2024-01-01', finished_at: null, progress_pages: null },
{ id: 777, started_at: '2024-01-01', finished_at: null, progress_pages: null },
{ id: 776, started_at: '2024-01-01', finished_at: null, progress_pages: null },
]);
insertRead(900);
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
expect(mockClient.query).toHaveBeenNthCalledWith(4, 1, 'tok', expect.stringContaining('mutation InsertUserBookRead'), expect.anything());
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ hardcoverReadId: 900 }));
});
it('fails the sync instead of recording success when a brand new read cannot be claimed', async () => {
const errorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(undefined);
mockRepo.findReadingAttempts.mockResolvedValue([attempt(2, { startedOn: '2024-01-01' })]);
mockRepo.linkReadingAttempt.mockResolvedValue('conflict');
arrange([]);
insertRead(900);
await expect(makeService().syncBook(1, 1)).resolves.toBe('failed');
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ syncError: 'read_link_conflict' }));
expect(mockRepo.upsertBookState).not.toHaveBeenCalledWith(expect.objectContaining({ syncError: null }));
errorSpy.mockRestore();
});
it('logs the driver cause so a constraint failure names its constraint', async () => {
const errorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(undefined);
mockRepo.findReadingAttempts.mockResolvedValue([attempt(2, { startedOn: '2024-01-01' })]);
mockRepo.linkReadingAttempt.mockRejectedValue(
Object.assign(new Error('Failed query: update "reading_attempts"'), {
cause: { code: '23505', constraint: 'reading_attempts_external_uidx', message: 'duplicate key value' },
}),
);
arrange([{ id: 777, started_at: '2024-01-01', finished_at: null, progress_pages: null }]);
await expect(makeService().syncBook(1, 1)).resolves.toBe('failed');
expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('cause="23505 reading_attempts_external_uidx duplicate key value"'));
errorSpy.mockRestore();
});
});
describe('change detection', () => {
const syncedState = {
hardcoverBookId: 10,
hardcoverReadId: 700,
lastSyncedAt: new Date('2026-08-21T10:00:00Z'),
lastSyncedStatus: 'reading',
lastSyncedProgress: 42,
lastSyncedRating: null,
lastSyncedStartedAt: '2024-01-01',
lastSyncedFinishedAt: null,
};
it('skips a book whose watched fields all match the last sync', async () => {
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockRepo.findBookSyncData.mockResolvedValue({ ...readingBook, attemptsUpdatedAt: new Date('2026-08-21T09:00:00Z') });
mockRepo.findBookState.mockResolvedValue(syncedState);
await expect(makeService().syncBook(1, 1)).resolves.toBe('skipped');
expect(mockMatchService.matchBook).not.toHaveBeenCalled();
});
it('re-syncs when only the attempt history changed', async () => {
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockRepo.findBookSyncData.mockResolvedValue({ ...readingBook, attemptsUpdatedAt: new Date('2026-08-21T11:00:00Z') });
mockRepo.findBookState.mockResolvedValue(syncedState);
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: 20, editionPages: 300, matchMethod: 'cached' });
mockClient.query
.mockResolvedValueOnce({ insert_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ update_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ user_book_reads: [{ id: 700, started_at: '2024-01-01', finished_at: null, progress_pages: 120 }] })
.mockResolvedValueOnce({ update_user_book_read: { user_book_read: { id: 700 }, error: null } });
await expect(makeService().syncBook(1, 1)).resolves.toBe('synced');
});
});
});
@@ -18,6 +18,16 @@ 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 {
attemptOwnedReadId,
type HardcoverReadCandidate,
type HardcoverReadTarget,
readTargetForAttempt,
readTargetForBook,
type ReadingAttemptForSync,
selectAdoptableReadId,
selectPrimaryAttempt,
} from './hardcover-read-selection';
import { type BookSyncData, HardcoverRepository } from './hardcover.repository';
import { HardcoverSettingsService } from './hardcover-settings.service';
import {
@@ -28,6 +38,9 @@ import {
const LINKED_BOOKS_LIMIT = 200;
// Bounds how many times a lost ownership race may force reselection before the sync mints a new read.
const MAX_READ_ADOPTION_ROUNDS = 3;
const STATUS_MAP: Partial<Record<ReadStatus, number>> = {
want_to_read: HARDCOVER_STATUS.WANT_TO_READ,
reading: HARDCOVER_STATUS.CURRENTLY_READING,
@@ -109,6 +122,20 @@ type FindUserBookReadsResult = {
export type HardcoverSyncBookResult = 'synced' | 'skipped' | 'failed';
type HardcoverBookStateSnapshot = Awaited<ReturnType<HardcoverRepository['findBookState']>>;
/** Drizzle hides the driver error - and with it the constraint name - behind `cause`. */
function describeErrorCause(error: unknown): string {
let current: unknown = error instanceof Error ? error.cause : undefined;
const seen = new Set<unknown>();
while (current && typeof current === 'object' && !seen.has(current)) {
seen.add(current);
const entry = current as { code?: unknown; constraint?: unknown; message?: unknown; cause?: unknown };
const parts = [entry.code, entry.constraint, entry.message].filter((part): part is string => typeof part === 'string' && part.length > 0);
if (parts.length > 0) return parts.join(' ');
current = entry.cause;
}
return '';
}
function toDateString(d: Date | null | undefined): string | null {
if (!d) return null;
return d instanceof Date ? d.toISOString().split('T')[0]! : null;
@@ -476,20 +503,34 @@ export class HardcoverSyncService {
let hardcoverReadId: number | null = syncState?.hardcoverReadId ?? null;
let progressSynced = true;
if (startDate || endDate || book.progress != null) {
if (book.progress != null && !match.editionPages) {
throw new Error('missing_edition_pages');
}
const localAttempts = typeof this.repo.findReadingAttempts === 'function' ? await this.repo.findReadingAttempts(userId, book.bookId) : [];
const primaryAttempt = selectPrimaryAttempt(localAttempts);
// Every read id already owned on this book, tombstones included, so no attempt adopts one that
// reading_attempts_external_uidx would refuse. Grows as this run claims further ids.
const reserved = new Set<number>(
typeof this.repo.findClaimedHardcoverReadIds === 'function' ? await this.repo.findClaimedHardcoverReadIds(userId, book.bookId) : [],
);
const historyAttempts = localAttempts.filter(
(attempt) => attempt.id !== primaryAttempt?.id && attempt.outcome !== 'skimmed' && attempt.outcome !== 'abandoned',
);
const syncsPrimaryRead = startDate !== null || endDate !== null || book.progress != null;
if (syncsPrimaryRead && book.progress != null && !match.editionPages) {
throw new Error('missing_edition_pages');
}
const reads = syncsPrimaryRead || historyAttempts.length > 0 ? await this.findUserBookReads(userId, token, userBookId) : [];
if (syncsPrimaryRead) {
const progressPages = book.progress != null && match.editionPages ? Math.round((book.progress / 100) * match.editionPages) : undefined;
progressSynced = book.progress == null || progressPages != null;
const reads = await this.findUserBookReads(userId, token, userBookId);
const targetReadId = this.resolveTargetReadId(hardcoverReadId, reads);
hardcoverReadId = await this.upsertUserBookRead(userId, token, {
hardcoverReadId = await this.syncAttemptRead(userId, token, {
userBookId,
existingReadId: targetReadId,
attempt: primaryAttempt,
// The projection writes the primary attempt's dates onto the book, so the book-level dates
// this sync sends are the same dates the read must carry.
target: readTargetForBook(startDate, endDate),
reads,
reserved,
startedAt: startDate,
finishedAt: endDate,
progressPages,
@@ -500,6 +541,7 @@ export class HardcoverSyncService {
await this.updateAdditionalOpenReads(userId, token, {
reads,
primaryReadId: hardcoverReadId,
reservedReadIds: reserved,
startedAt: startDate,
finishedAt: endDate,
progressPages,
@@ -512,26 +554,17 @@ export class HardcoverSyncService {
}
}
const localAttempts = typeof this.repo.findReadingAttempts === 'function' ? await this.repo.findReadingAttempts(userId, book.bookId) : [];
const primaryAttempt = [...localAttempts].reverse().find((attempt) => {
const attemptStarted = attempt.startedOn ?? null;
const attemptEnded = attempt.outcome === 'completed' ? (attempt.endedOn ?? null) : null;
return attemptStarted === startDate && attemptEnded === endDate;
});
if (hardcoverReadId != null && primaryAttempt) {
await this.repo.linkReadingAttempt(userId, primaryAttempt.id, hardcoverReadId);
}
for (const attempt of localAttempts) {
if (attempt.id === primaryAttempt?.id || attempt.outcome === 'skimmed' || attempt.outcome === 'abandoned') continue;
const existingReadId = attempt.externalProvider === 'hardcover' && attempt.externalId ? Number(attempt.externalId) : null;
const readId = await this.upsertUserBookRead(userId, token, {
for (const attempt of historyAttempts) {
await this.syncAttemptRead(userId, token, {
userBookId,
existingReadId: Number.isInteger(existingReadId) ? existingReadId : null,
attempt,
target: readTargetForAttempt(attempt),
reads,
reserved,
startedAt: attempt.startedOn,
finishedAt: attempt.outcome === 'completed' ? attempt.endedOn : null,
editionId: match.hardcoverEditionId ?? undefined,
});
if (readId != null) await this.repo.linkReadingAttempt(userId, attempt.id, readId);
}
await this.repo.upsertBookState({
@@ -559,8 +592,10 @@ export class HardcoverSyncService {
} catch (err) {
const errorClass = err instanceof Error ? err.constructor.name : 'Error';
const error = sanitizeLogValue(err instanceof Error ? err.message : String(err));
const cause = describeErrorCause(err);
const causeField = cause ? ` cause="${sanitizeLogValue(cause)}"` : '';
this.logger.error(
`[hardcover.sync_book] [fail] userId=${userId} bookId=${book.bookId} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${error}" - sync failed`,
`[hardcover.sync_book] [fail] userId=${userId} bookId=${book.bookId} durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${error}"${causeField} - sync failed`,
);
await this.repo.upsertBookState({
userId,
@@ -675,19 +710,76 @@ export class HardcoverSyncService {
}
private async findUserBookReads(userId: number, token: string, userBookId: number): Promise<FindUserBookReadsResult['user_book_reads']> {
try {
const result = await this.client.query<FindUserBookReadsResult>(userId, token, FIND_USER_BOOK_READS_QUERY, { userBookId });
return result.user_book_reads ?? [];
} catch {
return [];
}
const result = await this.client.query<FindUserBookReadsResult>(userId, token, FIND_USER_BOOK_READS_QUERY, { userBookId });
return result.user_book_reads ?? [];
}
private resolveTargetReadId(existingReadId: number | null, reads: FindUserBookReadsResult['user_book_reads']): number | null {
const openRead = reads.find((read) => !read.finished_at);
if (openRead) return openRead.id;
if (existingReadId) return existingReadId;
return reads[0]?.id ?? null;
/**
* Resolves the one Hardcover read that represents `attempt` and writes this sync's values to it.
*
* Ownership lives on reading_attempts.external_id, never on the book-level cache. An attempt that
* already owns a read keeps it; an unlinked attempt may only adopt a read nothing else owns, and
* claims it locally before the remote write so a lost race cannot edit another attempt's read.
* When nothing is adoptable the sync mints a fresh read rather than sharing an existing one.
*/
private async syncAttemptRead(
userId: number,
token: string,
input: {
userBookId: number;
attempt: ReadingAttemptForSync | null | undefined;
target: HardcoverReadTarget;
reads: readonly HardcoverReadCandidate[];
reserved: Set<number>;
startedAt?: string | null;
finishedAt?: string | null;
progressPages?: number;
editionId?: number;
},
): Promise<number | null> {
const attempt = input.attempt ?? null;
const write = (existingReadId: number | null): Promise<number | null> =>
this.upsertUserBookRead(userId, token, {
userBookId: input.userBookId,
existingReadId,
startedAt: input.startedAt,
finishedAt: input.finishedAt,
progressPages: input.progressPages,
editionId: input.editionId,
});
const owned = attempt ? attemptOwnedReadId(attempt) : null;
if (owned != null && attempt) {
input.reserved.add(owned);
const readId = await write(owned);
if (readId == null || readId === owned) return readId;
// Hardcover answered with a different read, so move the stamp rather than leave it stale.
input.reserved.add(readId);
if ((await this.repo.linkReadingAttempt(userId, attempt.id, readId)) === 'conflict') {
throw new Error('read_link_conflict');
}
return readId;
}
const excluded = new Set(input.reserved);
for (let round = 0; round < MAX_READ_ADOPTION_ROUNDS; round++) {
const candidate = selectAdoptableReadId(input.reads, input.target, excluded);
if (candidate == null) break;
if (attempt && (await this.repo.linkReadingAttempt(userId, attempt.id, candidate)) === 'conflict') {
excluded.add(candidate);
continue;
}
input.reserved.add(candidate);
return await write(candidate);
}
const created = await write(null);
if (created == null) return null;
input.reserved.add(created);
if (attempt && (await this.repo.linkReadingAttempt(userId, attempt.id, created)) === 'conflict') {
throw new Error('read_link_conflict');
}
return created;
}
private async updateAdditionalOpenReads(
@@ -696,13 +788,16 @@ export class HardcoverSyncService {
input: {
reads: FindUserBookReadsResult['user_book_reads'];
primaryReadId: number;
reservedReadIds: ReadonlySet<number>;
startedAt?: string | null;
finishedAt?: string | null;
progressPages: number;
editionId?: number;
},
): Promise<void> {
const siblingReadIds = input.reads.filter((read) => read.id !== input.primaryReadId && !read.finished_at).map((read) => read.id);
const siblingReadIds = input.reads
.filter((read) => read.id !== input.primaryReadId && !read.finished_at && !input.reservedReadIds.has(read.id))
.map((read) => read.id);
if (siblingReadIds.length === 0) return;
@@ -35,7 +35,9 @@ function makeRepository() {
const settingsInsert = makeReturningChain(settingsRow);
const bookStateInsert = makeReturningChain(bookStateRow);
const deleteChain = makeWhereChain(undefined);
const updateChain = { set: vi.fn().mockReturnValue(makeWhereChain(undefined)) };
const updateReturning = vi.fn().mockResolvedValue([{ id: 3 }]);
const updateWhere = vi.fn().mockReturnValue({ returning: updateReturning });
const updateChain = { set: vi.fn().mockReturnValue({ where: updateWhere }) };
const bookIdLimit = vi.fn().mockResolvedValue([{ bookId: 42 }]);
const bookIdWhere = vi.fn().mockReturnValue({ limit: bookIdLimit });
const bookIdFrom = vi.fn().mockReturnValue({ where: bookIdWhere });
@@ -64,6 +66,8 @@ function makeRepository() {
bookStateInsert,
deleteChain,
updateChain,
updateReturning,
updateWhere,
bookIdLimit,
bookIdWhere,
permissionLimit,
@@ -440,4 +444,101 @@ describe('HardcoverRepository', () => {
expect(progressInsert.values).toHaveBeenCalledWith(expect.objectContaining({ percentage: 0 }));
expect(progressInsert.onConflictDoUpdate).toHaveBeenCalledWith(expect.objectContaining({ set: expect.objectContaining({ percentage: 0 }) }));
});
describe('linkReadingAttempt', () => {
function pgError(fields: Record<string, unknown>) {
return Object.assign(new Error('Failed query: update "reading_attempts"'), { cause: fields });
}
function rejectUpdateWith(updateChain: { set: ReturnType<typeof vi.fn> }, error: unknown) {
updateChain.set.mockReturnValue({ where: vi.fn().mockReturnValue({ returning: vi.fn().mockRejectedValue(error) }) });
}
it('stamps the attempt and reports it linked', async () => {
const { repo, updateChain } = makeRepository();
await expect(repo.linkReadingAttempt(7, 3, 555)).resolves.toBe('linked');
expect(updateChain.set).toHaveBeenCalledWith(expect.objectContaining({ externalProvider: 'hardcover', externalId: '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' }));
await expect(repo.linkReadingAttempt(7, 3, 555)).resolves.toBe('conflict');
});
it('reports a conflict when the attempt was already claimed by a concurrent sync', async () => {
const { repo, updateReturning } = makeRepository();
updateReturning.mockResolvedValue([]);
await expect(repo.linkReadingAttempt(7, 3, 555)).resolves.toBe('conflict');
});
it('recognises the conflict when the driver only names the index in the message', async () => {
const { repo, updateChain } = makeRepository();
rejectUpdateWith(
updateChain,
pgError({ code: '23505', message: 'duplicate key value violates unique constraint "reading_attempts_external_uidx"' }),
);
await expect(repo.linkReadingAttempt(7, 3, 555)).resolves.toBe('conflict');
});
it('recognises a conflict reported directly rather than through a cause', async () => {
const { repo, updateChain } = makeRepository();
rejectUpdateWith(updateChain, Object.assign(new Error('duplicate key'), { code: '23505', constraint: 'reading_attempts_external_uidx' }));
await expect(repo.linkReadingAttempt(7, 3, 555)).resolves.toBe('conflict');
});
it('rethrows a unique violation on a different constraint', async () => {
const { repo, updateChain } = makeRepository();
rejectUpdateWith(updateChain, pgError({ code: '23505', constraint: 'reading_attempts_one_active_uidx' }));
await expect(repo.linkReadingAttempt(7, 3, 555)).rejects.toThrow('Failed query');
});
it('rethrows errors that are not unique violations', async () => {
const { repo, updateChain } = makeRepository();
rejectUpdateWith(updateChain, pgError({ code: '40001', message: 'could not serialize access' }));
await expect(repo.linkReadingAttempt(7, 3, 555)).rejects.toThrow('Failed query');
});
it('does not loop on a self-referencing cause chain', async () => {
const { repo, updateChain } = makeRepository();
const error = new Error('boom') as Error & { cause?: unknown };
error.cause = error;
rejectUpdateWith(updateChain, error);
await expect(repo.linkReadingAttempt(7, 3, 555)).rejects.toThrow('boom');
});
});
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) }) });
}
it('returns the read ids already claimed on the book', async () => {
const { repo, db } = makeRepository();
withRows(db, [{ externalId: '555' }, { externalId: '777' }]);
await expect(repo.findClaimedHardcoverReadIds(7, 42)).resolves.toEqual([555, 777]);
});
it('drops ids that are not usable read identifiers', async () => {
const { repo, db } = makeRepository();
withRows(db, [{ externalId: 'abc' }, { externalId: '0' }, { externalId: null }, { externalId: '-1' }, { externalId: '12' }]);
await expect(repo.findClaimedHardcoverReadIds(7, 42)).resolves.toEqual([12]);
});
it('returns an empty list when nothing is claimed', async () => {
const { repo, db } = makeRepository();
withRows(db, []);
await expect(repo.findClaimedHardcoverReadIds(7, 42)).resolves.toEqual([]);
});
});
});
@@ -1,16 +1,44 @@
import { Inject, Injectable } from '@nestjs/common';
import { Permission } from '@bookorbit/types';
import type { ContentFilterRules, ReadStatus } from '@bookorbit/types';
import { and, asc, eq, inArray, ne, sql } from 'drizzle-orm';
import { and, asc, eq, inArray, isNotNull, isNull, ne, or, sql } from 'drizzle-orm';
import type { NodePgDatabase } from 'drizzle-orm/node-postgres';
import { buildContentFilterClauses } from '../../common/utils/content-filter-sql.utils';
import { DB } from '../../db';
import * as schema from '../../db/schema';
import type { HardcoverBookState, HardcoverUserSetting, NewHardcoverBookState, NewHardcoverUserSetting } from '../../db/schema';
import { HARDCOVER_EXTERNAL_PROVIDER } from './hardcover-read-selection';
type Db = NodePgDatabase<typeof schema>;
const READING_ATTEMPT_EXTERNAL_CONSTRAINT = 'reading_attempts_external_uidx';
export type HardcoverLinkOutcome = 'linked' | 'conflict';
function* iterateErrorChain(error: unknown): Generator<Record<string, unknown>> {
let current: unknown = error;
const seen = new Set<unknown>();
while (current && typeof current === 'object' && !seen.has(current)) {
seen.add(current);
yield current as Record<string, unknown>;
current = (current as { cause?: unknown }).cause;
}
}
function asString(value: unknown): string {
return typeof value === 'string' ? value : '';
}
function isExternalIdConflict(error: unknown): boolean {
for (const entry of iterateErrorChain(error)) {
if (entry['code'] !== '23505') continue;
if (asString(entry['constraint']) === READING_ATTEMPT_EXTERNAL_CONSTRAINT) return true;
if (asString(entry['message']).includes(READING_ATTEMPT_EXTERNAL_CONSTRAINT)) return true;
}
return false;
}
export interface BookSyncData {
bookId: number;
isbn13: string | null;
@@ -208,7 +236,9 @@ export class HardcoverRepository {
const attemptsSq = this.db
.select({
bookId: schema.readingAttempts.bookId,
updatedAt: sql<Date | null>`max(${schema.readingAttempts.updatedAt})`.as('attempts_updated_at'),
// mapWith keeps the driver's raw timestamp string from reaching callers: hasChanges compares
// this against a mapped Date, and a string/Date comparison is silently always false.
updatedAt: sql<Date | null>`max(${schema.readingAttempts.updatedAt})`.mapWith(schema.readingAttempts.updatedAt).as('attempts_updated_at'),
})
.from(schema.readingAttempts)
.where(and(eq(schema.readingAttempts.userId, userId), sql`${schema.readingAttempts.deletedAt} is null`))
@@ -263,11 +293,59 @@ export class HardcoverRepository {
.orderBy(schema.readingAttempts.id);
}
async linkReadingAttempt(userId: number, attemptId: number, hardcoverReadId: number): Promise<void> {
await this.db
.update(schema.readingAttempts)
.set({ externalProvider: 'hardcover', externalId: String(hardcoverReadId), updatedAt: new Date() })
.where(and(eq(schema.readingAttempts.userId, userId), eq(schema.readingAttempts.id, attemptId)));
/**
* Reports an ownership conflict instead of throwing. Another attempt - possibly a soft-deleted
* tombstone - can already own this read, and the caller recovers by selecting a different read
* rather than by merging or reassigning attempts.
*/
async linkReadingAttempt(userId: number, attemptId: number, hardcoverReadId: number): Promise<HardcoverLinkOutcome> {
const externalId = String(hardcoverReadId);
try {
const rows = await this.db
.update(schema.readingAttempts)
.set({ externalProvider: HARDCOVER_EXTERNAL_PROVIDER, externalId, updatedAt: new Date() })
.where(
and(
eq(schema.readingAttempts.userId, userId),
eq(schema.readingAttempts.id, attemptId),
isNull(schema.readingAttempts.deletedAt),
or(
and(isNull(schema.readingAttempts.externalProvider), isNull(schema.readingAttempts.externalId)),
and(eq(schema.readingAttempts.externalProvider, HARDCOVER_EXTERNAL_PROVIDER), eq(schema.readingAttempts.externalId, externalId)),
),
),
)
.returning({ id: schema.readingAttempts.id });
return rows.length > 0 ? 'linked' : 'conflict';
} catch (err) {
if (isExternalIdConflict(err)) return 'conflict';
throw err;
}
}
/**
* 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
* external id is the tombstone importExternalRead relies on to avoid re-importing a removed read.
*/
async findClaimedHardcoverReadIds(userId: number, bookId: number): Promise<number[]> {
const rows = await this.db
.select({ externalId: schema.readingAttempts.externalId })
.from(schema.readingAttempts)
.where(
and(
eq(schema.readingAttempts.userId, userId),
eq(schema.readingAttempts.bookId, bookId),
eq(schema.readingAttempts.externalProvider, HARDCOVER_EXTERNAL_PROVIDER),
isNotNull(schema.readingAttempts.externalId),
),
);
const ids: number[] = [];
for (const row of rows) {
const parsed = Number(row.externalId);
if (Number.isInteger(parsed) && parsed > 0) ids.push(parsed);
}
return ids;
}
async findBookIdByFileId(bookFileId: number): Promise<number | null> {
+133
View File
@@ -494,4 +494,137 @@ describe('Reading attempts main-flow simulation (docker e2e)', { timeout: TIMEOU
}),
).rejects.toThrow();
});
it('19. reports an ownership conflict instead of throwing when another attempt holds the read', async () => {
const book = await createBook();
const attempts = ctx.app.get(ReadingAttemptService);
const hardcover = ctx.app.get(HardcoverRepository);
const owner = await attempts.createHistorical(adminUserId, book.bookId, {
startedOn: '2022-01-01',
endedOn: '2022-01-05',
outcome: 'completed',
});
const rival = await attempts.createHistorical(adminUserId, book.bookId, {
startedOn: '2022-03-01',
endedOn: '2022-03-05',
outcome: 'completed',
});
await expect(hardcover.linkReadingAttempt(adminUserId, owner.id, 9101)).resolves.toBe('linked');
await expect(hardcover.linkReadingAttempt(adminUserId, rival.id, 9101)).resolves.toBe('conflict');
const [rivalRow] = await ctx.db.select().from(schema.readingAttempts).where(eq(schema.readingAttempts.id, rival.id));
expect(rivalRow?.externalId).toBeNull();
const [ownerRow] = await ctx.db.select().from(schema.readingAttempts).where(eq(schema.readingAttempts.id, owner.id));
expect(ownerRow).toMatchObject({ externalId: '9101', deletedAt: null });
});
it('20. keeps a soft-deleted attempt tombstone reserved against reuse', async () => {
const book = await createBook();
const attempts = ctx.app.get(ReadingAttemptService);
const hardcover = ctx.app.get(HardcoverRepository);
const removed = await attempts.createHistorical(adminUserId, book.bookId, {
startedOn: '2022-06-01',
endedOn: '2022-06-05',
outcome: 'completed',
});
await hardcover.linkReadingAttempt(adminUserId, removed.id, 9202);
await attempts.delete(adminUserId, book.bookId, removed.id);
const survivor = await attempts.createHistorical(adminUserId, book.bookId, {
startedOn: '2022-09-01',
endedOn: '2022-09-05',
outcome: 'completed',
});
await expect(hardcover.linkReadingAttempt(adminUserId, survivor.id, 9202)).resolves.toBe('conflict');
await expect(hardcover.findClaimedHardcoverReadIds(adminUserId, book.bookId)).resolves.toContain(9202);
});
it('21. lists claimed read ids across live and soft-deleted attempts', async () => {
const book = await createBook();
const attempts = ctx.app.get(ReadingAttemptService);
const hardcover = ctx.app.get(HardcoverRepository);
const live = await attempts.createHistorical(adminUserId, book.bookId, {
startedOn: '2023-01-01',
endedOn: '2023-01-05',
outcome: 'completed',
});
const deleted = await attempts.createHistorical(adminUserId, book.bookId, {
startedOn: '2023-04-01',
endedOn: '2023-04-05',
outcome: 'completed',
});
await hardcover.linkReadingAttempt(adminUserId, live.id, 9301);
await hardcover.linkReadingAttempt(adminUserId, deleted.id, 9302);
await attempts.delete(adminUserId, book.bookId, deleted.id);
const claimed = await hardcover.findClaimedHardcoverReadIds(adminUserId, book.bookId);
expect(new Set(claimed)).toEqual(new Set([9301, 9302]));
});
it('22. lets an attempt restamp the same read but rejects reassignment', 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: '2023-07-01',
endedOn: '2023-07-05',
outcome: 'completed',
});
await expect(hardcover.linkReadingAttempt(adminUserId, attempt.id, 9401)).resolves.toBe('linked');
await expect(hardcover.linkReadingAttempt(adminUserId, attempt.id, 9401)).resolves.toBe('linked');
await expect(hardcover.linkReadingAttempt(adminUserId, attempt.id, 9402)).resolves.toBe('conflict');
await expect(hardcover.findClaimedHardcoverReadIds(adminUserId, book.bookId)).resolves.toEqual([9401]);
});
it('23. lets exactly one concurrent claim establish attempt ownership', 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: '2023-08-01',
endedOn: '2023-08-05',
outcome: 'completed',
});
const outcomes = await Promise.all([
hardcover.linkReadingAttempt(adminUserId, attempt.id, 9501),
hardcover.linkReadingAttempt(adminUserId, attempt.id, 9502),
]);
expect(outcomes.sort()).toEqual(['conflict', 'linked']);
const claimed = await hardcover.findClaimedHardcoverReadIds(adminUserId, book.bookId);
expect(claimed).toHaveLength(1);
expect([9501, 9502]).toContain(claimed[0]);
});
it('24. surfaces attempt timestamps as Dates so sync change detection can compare them', async () => {
const book = await createBook();
const attempts = ctx.app.get(ReadingAttemptService);
const hardcover = ctx.app.get(HardcoverRepository);
const before = new Date(Date.now() - 60_000);
await attempts.createHistorical(adminUserId, book.bookId, {
startedOn: '2024-02-01',
endedOn: '2024-02-05',
outcome: 'completed',
});
const sync = await hardcover.findBookSyncData(adminUserId, book.bookId);
const updatedAt = sync?.attemptsUpdatedAt ?? null;
// A raw driver timestamp string compares false against a Date, which silently disables the
// attempt-history branch of hasChanges and stops attempt edits from ever re-triggering a sync.
expect(updatedAt).toBeInstanceOf(Date);
expect(updatedAt instanceof Date && updatedAt > before).toBe(true);
});
it('25. leaves the attempt timestamp null for a book with no attempts', async () => {
const book = await createBook();
const hardcover = ctx.app.get(HardcoverRepository);
const sync = await hardcover.findBookSyncData(adminUserId, book.bookId);
expect(sync?.attemptsUpdatedAt ?? null).toBeNull();
});
});