fix(reader): route KOReader progress by format so paged books resume

KOReader reports a paged document's position as a page number and a reflowable one's as an
xpointer, but applySharedProgress treated every incoming position as an xpointer and
upsertReadingProgress hardcoded pageNumber to null on every write. A PDF synced from a device
therefore stored a correct percentage with no page, and the web reader reopened it at page 1.
The same write also destroyed a page the web reader had saved earlier, including during an
unattended bulk sweep, so this lost positions that were never synced from a device at all.

Carry the book file's format through hash resolution and pick the column from it. CBZ/CBR were
affected identically at the data level but masked in the UI, because the comic reader estimates
a page from the percentage when none is stored.

Closes #954
This commit is contained in:
neon
2026-08-09 17:29:19 +00:00
parent 4607ed8235
commit 68fae9a121
10 changed files with 553 additions and 49 deletions
+19
View File
@@ -513,6 +513,25 @@ export const E2E_SUITES = Object.freeze({
...SHARED_DB_AND_HELPER_PATHS, ...SHARED_DB_AND_HELPER_PATHS,
], ],
}, },
"koreader-progress-position": {
id: "koreader-progress-position",
name: "KOReader Progress Position Routing",
timeout: 45,
lane: "full",
description: "KOReader progress position routing into cfi or pageNumber per book format",
vitestTarget: "test/koreader-progress-position.e2e-spec.ts",
junitOutput: `${TEST_RESULTS_DIR}/koreader-progress-position-e2e-junit.xml`,
prepareDedicatedDatabase: true,
useDedicatedDatabase: true,
changedPaths: [
"server/src/modules/koreader/**",
"server/src/modules/book/book.service.ts",
"server/src/db/schema/reader.ts",
"server/test/koreader-progress-position.e2e-spec.ts",
"server/test/e2e/reader-state-isolation/**",
...SHARED_DB_AND_HELPER_PATHS,
],
},
"koreader-file-naming": { "koreader-file-naming": {
id: "koreader-file-naming", id: "koreader-file-naming",
name: "KOReader File Naming", name: "KOReader File Naming",
+1 -1
View File
@@ -152,7 +152,7 @@ export const readingProgress = pgTable(
percentage: real('percentage').notNull().default(0), percentage: real('percentage').notNull().default(0),
// EPUB: CFI string pinpoints exact location // EPUB: CFI string pinpoints exact location
cfi: varchar('cfi', { length: 2000 }), cfi: varchar('cfi', { length: 2000 }),
// PDF / CBX / CBR: zero-based page index // PDF / CBX / CBR: 1-based page number
pageNumber: integer('page_number'), pageNumber: integer('page_number'),
// Audio: playback position in seconds // Audio: playback position in seconds
positionSeconds: real('position_seconds'), positionSeconds: real('position_seconds'),
@@ -224,7 +224,7 @@ export class KoreaderPluginService {
} }
entries.push({ entries.push({
bookFile: { id: match.bookFileId, bookId: match.bookId, libraryId: match.libraryId }, bookFile: { id: match.bookFileId, bookId: match.bookId, libraryId: match.libraryId, format: match.format },
percentage: item.percentage, percentage: item.percentage,
progress: item.progress, progress: item.progress,
timestamp: item.timestamp, timestamp: item.timestamp,
@@ -0,0 +1,68 @@
import { describe, expect, it } from 'vitest';
import { isPagedReadingFormat, parseKoreaderPageNumber } from './koreader-progress-position.util';
describe('isPagedReadingFormat', () => {
it.each(['pdf', 'PDF', 'cbz', 'cbr', 'cb7', 'cbx', 'CBZ'])('treats %s as paged', (format) => {
expect(isPagedReadingFormat(format)).toBe(true);
});
it.each(['epub', 'mobi', 'azw3', 'azw', 'fb2', 'txt', 'EPUB'])('treats %s as reflowable', (format) => {
expect(isPagedReadingFormat(format)).toBe(false);
});
it.each(['m4b', 'mp3', 'm4a', 'opus', 'ogg', 'flac'])('treats %s as non-paged', (format) => {
expect(isPagedReadingFormat(format)).toBe(false);
});
it.each([null, undefined, '', ' ', 'djvu'])('falls back to non-paged for %s', (format) => {
expect(isPagedReadingFormat(format)).toBe(false);
});
});
describe('parseKoreaderPageNumber', () => {
it('parses the page number KOReader sends for a paged document', () => {
expect(parseKoreaderPageNumber('117')).toBe(117);
});
it('accepts the first page', () => {
expect(parseKoreaderPageNumber('1')).toBe(1);
});
it('tolerates surrounding whitespace and leading zeros', () => {
expect(parseKoreaderPageNumber(' 42 ')).toBe(42);
expect(parseKoreaderPageNumber('0042')).toBe(42);
});
it('rejects an xpointer that reached a paged file by mistake', () => {
expect(parseKoreaderPageNumber('/body/DocFragment[8]/body/p[12]/text().0')).toBeNull();
});
it.each([null, undefined, '', ' '])('returns null for %s so the stored page is cleared', (progress) => {
expect(parseKoreaderPageNumber(progress)).toBeNull();
});
it.each(['0', '-1', '-12'])('rejects %s because KOReader pages start at 1', (progress) => {
expect(parseKoreaderPageNumber(progress)).toBeNull();
});
it.each(['12.5', '12.0', '1e3', '0x2a', '4 2', '12abc', 'NaN', 'Infinity', '+7'])('rejects the non-integer %s', (progress) => {
expect(parseKoreaderPageNumber(progress)).toBeNull();
});
it('accepts the largest page a PostgreSQL integer column can hold', () => {
expect(parseKoreaderPageNumber('2147483647')).toBe(2_147_483_647);
});
it('rejects a page that would overflow the integer column', () => {
expect(parseKoreaderPageNumber('2147483648')).toBeNull();
});
it('rejects a digit string too long to survive as a safe integer', () => {
expect(parseKoreaderPageNumber('9'.repeat(400))).toBeNull();
});
it('rejects non-string input defensively', () => {
expect(parseKoreaderPageNumber(42 as unknown as string)).toBeNull();
});
});
@@ -0,0 +1,35 @@
import { getFormatGroup } from '@bookorbit/types';
/** reading_progress.page_number is a PostgreSQL integer, so anything above int4 would fail the insert. */
const MAX_PAGE_NUMBER = 2_147_483_647;
/**
* KOReader reports a reading position two different ways: reflowable documents get a
* CREngine xpointer, paged ones (PDF and the CBX comic containers) get a plain page number.
* The two land in different reading_progress columns, so every incoming position has
* to be routed by the book file's format.
*
* An unknown or missing format resolves to the reflowable group, matching getFormatGroup.
*/
export function isPagedReadingFormat(format: string | null | undefined): boolean {
const group = getFormatGroup(format ?? '');
return group === 'pdf' || group === 'cbx';
}
/**
* Parses the 1-based page number KOReader sends for paged documents
* (ReaderPaging:getLastProgress). Returns null for anything else, including an xpointer
* that reached a paged file by mistake, so the caller clears the stored page instead of
* replacing it with a position the reader would misinterpret.
*/
export function parseKoreaderPageNumber(progress: string | null | undefined): number | null {
if (typeof progress !== 'string') return null;
const trimmed = progress.trim();
if (!/^\d+$/.test(trimmed)) return null;
const page = Number(trimmed);
if (!Number.isSafeInteger(page) || page < 1 || page > MAX_PAGE_NUMBER) return null;
return page;
}
@@ -61,7 +61,7 @@ describe('KoreaderRepository', () => {
}); });
it('returns the book file when found by current hash', async () => { it('returns the book file when found by current hash', async () => {
const file = { id: 10, bookId: 20, libraryId: 1 }; const file = { id: 10, bookId: 20, libraryId: 1, format: 'epub' };
db.select.mockReturnValue(makeQueryChain([file])); db.select.mockReturnValue(makeQueryChain([file]));
const result = await repo.resolveBookFileByHash('abc123', null); const result = await repo.resolveBookFileByHash('abc123', null);
@@ -71,7 +71,7 @@ describe('KoreaderRepository', () => {
}); });
it('falls back to hash history when current hash lookup returns nothing', async () => { it('falls back to hash history when current hash lookup returns nothing', async () => {
const file = { id: 10, bookId: 20, libraryId: 1 }; const file = { id: 10, bookId: 20, libraryId: 1, format: 'pdf' };
db.select.mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([file])); db.select.mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([file]));
const result = await repo.resolveBookFileByHash('oldhash', null); const result = await repo.resolveBookFileByHash('oldhash', null);
@@ -81,7 +81,7 @@ describe('KoreaderRepository', () => {
}); });
it('falls back to a user-scoped manual link after direct and history lookups miss', async () => { it('falls back to a user-scoped manual link after direct and history lookups miss', async () => {
const file = { id: 10, bookId: 20, libraryId: 1 }; const file = { id: 10, bookId: 20, libraryId: 1, format: 'cbz' };
db.select db.select
.mockReturnValueOnce(makeQueryChain([])) .mockReturnValueOnce(makeQueryChain([]))
.mockReturnValueOnce(makeQueryChain([])) .mockReturnValueOnce(makeQueryChain([]))
@@ -93,6 +93,17 @@ describe('KoreaderRepository', () => {
expect(db.select).toHaveBeenCalledTimes(3); expect(db.select).toHaveBeenCalledTimes(3);
}); });
it('selects the file format on every lookup so callers can route the position', async () => {
db.select.mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([]));
await repo.resolveBookFileByHash('manualhash', [1], 7);
expect(db.select).toHaveBeenCalledTimes(3);
for (const call of db.select.mock.calls) {
expect(call[0]).toHaveProperty('format');
}
});
it('returns null when a user-scoped manual link lookup also misses', async () => { it('returns null when a user-scoped manual link lookup also misses', async () => {
db.select.mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([])); db.select.mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([]));
@@ -122,41 +133,52 @@ describe('KoreaderRepository', () => {
db.select db.select
.mockReturnValueOnce( .mockReturnValueOnce(
makeQueryChain([ makeQueryChain([
{ hash: 'current', bookFileId: 11, bookId: 21, libraryId: 31 }, { hash: 'current', bookFileId: 11, bookId: 21, libraryId: 31, format: 'epub' },
{ hash: null, bookFileId: 12, bookId: 22, libraryId: 32 }, { hash: null, bookFileId: 12, bookId: 22, libraryId: 32, format: 'epub' },
]), ]),
) )
.mockReturnValueOnce(makeQueryChain([{ hash: 'old', bookFileId: 13, bookId: 23, libraryId: 33 }])); .mockReturnValueOnce(makeQueryChain([{ hash: 'old', bookFileId: 13, bookId: 23, libraryId: 33, format: 'pdf' }]));
const result = await repo.resolveBookFilesByHashes(['current', 'old'], [31, 33]); const result = await repo.resolveBookFilesByHashes(['current', 'old'], [31, 33]);
expect(result.get('current')).toEqual({ bookFileId: 11, bookId: 21, libraryId: 31 }); expect(result.get('current')).toEqual({ bookFileId: 11, bookId: 21, libraryId: 31, format: 'epub' });
expect(result.get('old')).toEqual({ bookFileId: 13, bookId: 23, libraryId: 33 }); expect(result.get('old')).toEqual({ bookFileId: 13, bookId: 23, libraryId: 33, format: 'pdf' });
expect(result.size).toBe(2); expect(result.size).toBe(2);
expect(db.select).toHaveBeenCalledTimes(2); expect(db.select).toHaveBeenCalledTimes(2);
}); });
it('skips hash history lookup when all hashes resolve directly', async () => { it('skips hash history lookup when all hashes resolve directly', async () => {
db.select.mockReturnValueOnce(makeQueryChain([{ hash: 'current', bookFileId: 11, bookId: 21, libraryId: 31 }])); db.select.mockReturnValueOnce(makeQueryChain([{ hash: 'current', bookFileId: 11, bookId: 21, libraryId: 31, format: 'pdf' }]));
const result = await repo.resolveBookFilesByHashes(['current'], null); const result = await repo.resolveBookFilesByHashes(['current'], null);
expect(result.get('current')).toEqual({ bookFileId: 11, bookId: 21, libraryId: 31 }); expect(result.get('current')).toEqual({ bookFileId: 11, bookId: 21, libraryId: 31, format: 'pdf' });
expect(db.select).toHaveBeenCalledTimes(1); expect(db.select).toHaveBeenCalledTimes(1);
}); });
it('resolves remaining hashes from user-scoped manual links', async () => { it('resolves remaining hashes from user-scoped manual links', async () => {
db.select db.select
.mockReturnValueOnce(makeQueryChain([{ hash: 'current', bookFileId: 11, bookId: 21, libraryId: 31 }])) .mockReturnValueOnce(makeQueryChain([{ hash: 'current', bookFileId: 11, bookId: 21, libraryId: 31, format: 'epub' }]))
.mockReturnValueOnce(makeQueryChain([])) .mockReturnValueOnce(makeQueryChain([]))
.mockReturnValueOnce(makeQueryChain([{ hash: 'manual', bookFileId: 12, bookId: 22, libraryId: 32 }])); .mockReturnValueOnce(makeQueryChain([{ hash: 'manual', bookFileId: 12, bookId: 22, libraryId: 32, format: 'cbr' }]));
const result = await repo.resolveBookFilesByHashes(['current', 'manual'], [31, 32], 7); const result = await repo.resolveBookFilesByHashes(['current', 'manual'], [31, 32], 7);
expect(result.get('current')).toEqual({ bookFileId: 11, bookId: 21, libraryId: 31 }); expect(result.get('current')).toEqual({ bookFileId: 11, bookId: 21, libraryId: 31, format: 'epub' });
expect(result.get('manual')).toEqual({ bookFileId: 12, bookId: 22, libraryId: 32 }); expect(result.get('manual')).toEqual({ bookFileId: 12, bookId: 22, libraryId: 32, format: 'cbr' });
expect(db.select).toHaveBeenCalledTimes(3); expect(db.select).toHaveBeenCalledTimes(3);
}); });
it('selects the file format on every lookup so callers can route the position', async () => {
db.select.mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([])).mockReturnValueOnce(makeQueryChain([]));
await repo.resolveBookFilesByHashes(['current', 'manual'], [31, 32], 7);
expect(db.select).toHaveBeenCalledTimes(3);
for (const call of db.select.mock.calls) {
expect(call[0]).toHaveProperty('format');
}
});
}); });
describe('unmatched books and manual hash links', () => { describe('unmatched books and manual hash links', () => {
@@ -743,12 +765,22 @@ describe('KoreaderRepository', () => {
}); });
describe('upsertReadingProgress', () => { describe('upsertReadingProgress', () => {
it('upserts percentage and clears stale web locator fields on conflict', async () => { function mockUpsertChain() {
const onConflictDoUpdate = vi.fn().mockResolvedValue(undefined); const onConflictDoUpdate = vi.fn().mockResolvedValue(undefined);
const values = vi.fn().mockReturnValue({ onConflictDoUpdate }); const values = vi.fn().mockReturnValue({ onConflictDoUpdate });
db.insert.mockReturnValue({ values }); db.insert.mockReturnValue({ values });
return { onConflictDoUpdate, values };
}
await repo.upsertReadingProgress(44, 12, 41.25); function conflictSet(onConflictDoUpdate: ReturnType<typeof vi.fn>) {
const arg = onConflictDoUpdate.mock.calls[0]?.[0] as { set?: Record<string, unknown> } | undefined;
return arg?.set;
}
it('upserts percentage and clears stale web locator fields on conflict', async () => {
const { onConflictDoUpdate, values } = mockUpsertChain();
await repo.upsertReadingProgress({ bookFileId: 44, userId: 12, percentage: 41.25 });
expect(db.insert).toHaveBeenCalledTimes(1); expect(db.insert).toHaveBeenCalledTimes(1);
expect(values).toHaveBeenCalledWith( expect(values).toHaveBeenCalledWith(
@@ -756,6 +788,9 @@ describe('KoreaderRepository', () => {
bookFileId: 44, bookFileId: 44,
userId: 12, userId: 12,
percentage: 41.25, percentage: 41.25,
cfi: null,
pageNumber: null,
koreaderProgress: null,
}), }),
); );
@@ -767,12 +802,47 @@ describe('KoreaderRepository', () => {
cfi: null, cfi: null,
pageNumber: null, pageNumber: null,
koreaderProgress: null, koreaderProgress: null,
koboLocationSource: null,
koboLocationType: null,
koboLocationValue: null,
koboContentSourceProgressPercent: null,
}), }),
}), }),
); );
const conflictArg = onConflictDoUpdate.mock.calls[0]?.[0] as { set?: Record<string, unknown> } | undefined; expect(conflictSet(onConflictDoUpdate)?.['updatedAt']).toBeDefined();
expect(conflictArg?.set?.['updatedAt']).toBeDefined(); });
it('persists a paged position on both the insert and the conflict update', async () => {
const { onConflictDoUpdate, values } = mockUpsertChain();
await repo.upsertReadingProgress({ bookFileId: 44, userId: 12, percentage: 30, xpointer: '117', pageNumber: 117 });
expect(values).toHaveBeenCalledWith(expect.objectContaining({ pageNumber: 117, cfi: null, koreaderProgress: '117' }));
expect(conflictSet(onConflictDoUpdate)).toEqual(expect.objectContaining({ pageNumber: 117, cfi: null, koreaderProgress: '117' }));
});
it('persists a reflowable position without touching the page column', async () => {
const { onConflictDoUpdate, values } = mockUpsertChain();
await repo.upsertReadingProgress({
bookFileId: 44,
userId: 12,
percentage: 30,
cfi: 'epubcfi(/6/14!/4/2/6)',
xpointer: '/body/DocFragment[7]',
});
expect(values).toHaveBeenCalledWith(expect.objectContaining({ cfi: 'epubcfi(/6/14!/4/2/6)', pageNumber: null }));
expect(conflictSet(onConflictDoUpdate)).toEqual(expect.objectContaining({ cfi: 'epubcfi(/6/14!/4/2/6)', pageNumber: null }));
});
it('clears a stale stored page when the sync carries no page', async () => {
const { onConflictDoUpdate } = mockUpsertChain();
await repo.upsertReadingProgress({ bookFileId: 44, userId: 12, percentage: 30, pageNumber: null });
expect(conflictSet(onConflictDoUpdate)).toEqual(expect.objectContaining({ pageNumber: null }));
}); });
}); });
@@ -7,7 +7,8 @@ import { DB } from '../../db';
import * as schema from '../../db/schema'; import * as schema from '../../db/schema';
type Db = NodePgDatabase<typeof schema>; type Db = NodePgDatabase<typeof schema>;
type ResolvedBookFileByHash = { id: number; bookId: number; libraryId: number }; type ResolvedBookFileByHash = { id: number; bookId: number; libraryId: number; format: string | null };
type ResolvedBookFileByHashes = { bookFileId: number; bookId: number; libraryId: number; format: string | null };
type KoreaderUnmatchedSource = 'current_file' | 'file' | 'statistics'; type KoreaderUnmatchedSource = 'current_file' | 'file' | 'statistics';
type KoreaderUnmatchedCandidate = { type KoreaderUnmatchedCandidate = {
hash: string; hash: string;
@@ -23,6 +24,18 @@ type KoreaderHashLinkMetadata = {
lastOpen?: number | null; lastOpen?: number | null;
}; };
export interface ReadingProgressUpsert {
bookFileId: number;
userId: number;
percentage: number;
/** Reflowable position converted from the device xpointer; null for paged formats or a failed conversion. */
cfi?: string | null;
/** The device position string verbatim, so the next pull hands KOReader back what it sent. */
xpointer?: string | null;
/** 1-based page for paged formats; null for reflowable and audio. */
pageNumber?: number | null;
}
export interface DeviceProgressUpsert { export interface DeviceProgressUpsert {
bookFileId: number; bookFileId: number;
userId: number; userId: number;
@@ -71,7 +84,7 @@ export class KoreaderRepository {
const libraryFilter = accessibleLibraryIds ? inArray(schema.books.libraryId, accessibleLibraryIds) : undefined; const libraryFilter = accessibleLibraryIds ? inArray(schema.books.libraryId, accessibleLibraryIds) : undefined;
const [byFileHash] = await this.db const [byFileHash] = await this.db
.select({ id: schema.bookFiles.id, bookId: schema.bookFiles.bookId, libraryId: schema.books.libraryId }) .select({ id: schema.bookFiles.id, bookId: schema.bookFiles.bookId, libraryId: schema.books.libraryId, format: schema.bookFiles.format })
.from(schema.bookFiles) .from(schema.bookFiles)
.innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId)) .innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId))
.where(and(eq(schema.bookFiles.fileHash, hash), libraryFilter)) .where(and(eq(schema.bookFiles.fileHash, hash), libraryFilter))
@@ -80,7 +93,7 @@ export class KoreaderRepository {
if (byFileHash) return byFileHash; if (byFileHash) return byFileHash;
const [byFileHashHistory] = await this.db const [byFileHashHistory] = await this.db
.select({ id: schema.bookFiles.id, bookId: schema.bookFiles.bookId, libraryId: schema.books.libraryId }) .select({ id: schema.bookFiles.id, bookId: schema.bookFiles.bookId, libraryId: schema.books.libraryId, format: schema.bookFiles.format })
.from(schema.bookFileHashHistory) .from(schema.bookFileHashHistory)
.innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.bookFileHashHistory.bookFileId)) .innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.bookFileHashHistory.bookFileId))
.innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId)) .innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId))
@@ -91,7 +104,7 @@ export class KoreaderRepository {
if (userId !== undefined) { if (userId !== undefined) {
const [byManualLink] = await this.db const [byManualLink] = await this.db
.select({ id: schema.bookFiles.id, bookId: schema.bookFiles.bookId, libraryId: schema.books.libraryId }) .select({ id: schema.bookFiles.id, bookId: schema.bookFiles.bookId, libraryId: schema.books.libraryId, format: schema.bookFiles.format })
.from(schema.koreaderBookHashLinks) .from(schema.koreaderBookHashLinks)
.innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.koreaderBookHashLinks.bookFileId)) .innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.koreaderBookHashLinks.bookFileId))
.innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId)) .innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId))
@@ -108,8 +121,8 @@ export class KoreaderRepository {
hashes: string[], hashes: string[],
accessibleLibraryIds: number[] | null, accessibleLibraryIds: number[] | null,
userId?: number, userId?: number,
): Promise<Map<string, { bookFileId: number; bookId: number; libraryId: number }>> { ): Promise<Map<string, ResolvedBookFileByHashes>> {
const result = new Map<string, { bookFileId: number; bookId: number; libraryId: number }>(); const result = new Map<string, ResolvedBookFileByHashes>();
if (hashes.length === 0) return result; if (hashes.length === 0) return result;
if (accessibleLibraryIds !== null && accessibleLibraryIds.length === 0) return result; if (accessibleLibraryIds !== null && accessibleLibraryIds.length === 0) return result;
@@ -121,6 +134,7 @@ export class KoreaderRepository {
bookFileId: schema.bookFiles.id, bookFileId: schema.bookFiles.id,
bookId: schema.bookFiles.bookId, bookId: schema.bookFiles.bookId,
libraryId: schema.books.libraryId, libraryId: schema.books.libraryId,
format: schema.bookFiles.format,
}) })
.from(schema.bookFiles) .from(schema.bookFiles)
.innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId)) .innerJoin(schema.books, eq(schema.books.id, schema.bookFiles.bookId))
@@ -128,7 +142,7 @@ export class KoreaderRepository {
for (const row of direct) { for (const row of direct) {
if (row.hash && !result.has(row.hash)) { if (row.hash && !result.has(row.hash)) {
result.set(row.hash, { bookFileId: row.bookFileId, bookId: row.bookId, libraryId: row.libraryId }); result.set(row.hash, { bookFileId: row.bookFileId, bookId: row.bookId, libraryId: row.libraryId, format: row.format });
} }
} }
@@ -141,6 +155,7 @@ export class KoreaderRepository {
bookFileId: schema.bookFiles.id, bookFileId: schema.bookFiles.id,
bookId: schema.bookFiles.bookId, bookId: schema.bookFiles.bookId,
libraryId: schema.books.libraryId, libraryId: schema.books.libraryId,
format: schema.bookFiles.format,
}) })
.from(schema.bookFileHashHistory) .from(schema.bookFileHashHistory)
.innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.bookFileHashHistory.bookFileId)) .innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.bookFileHashHistory.bookFileId))
@@ -149,7 +164,7 @@ export class KoreaderRepository {
for (const row of history) { for (const row of history) {
if (!result.has(row.hash)) { if (!result.has(row.hash)) {
result.set(row.hash, { bookFileId: row.bookFileId, bookId: row.bookId, libraryId: row.libraryId }); result.set(row.hash, { bookFileId: row.bookFileId, bookId: row.bookId, libraryId: row.libraryId, format: row.format });
} }
} }
@@ -162,6 +177,7 @@ export class KoreaderRepository {
bookFileId: schema.bookFiles.id, bookFileId: schema.bookFiles.id,
bookId: schema.bookFiles.bookId, bookId: schema.bookFiles.bookId,
libraryId: schema.books.libraryId, libraryId: schema.books.libraryId,
format: schema.bookFiles.format,
}) })
.from(schema.koreaderBookHashLinks) .from(schema.koreaderBookHashLinks)
.innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.koreaderBookHashLinks.bookFileId)) .innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.koreaderBookHashLinks.bookFileId))
@@ -170,7 +186,7 @@ export class KoreaderRepository {
for (const row of manualLinks) { for (const row of manualLinks) {
if (!result.has(row.hash)) { if (!result.has(row.hash)) {
result.set(row.hash, { bookFileId: row.bookFileId, bookId: row.bookId, libraryId: row.libraryId }); result.set(row.hash, { bookFileId: row.bookFileId, bookId: row.bookId, libraryId: row.libraryId, format: row.format });
} }
} }
@@ -533,24 +549,27 @@ export class KoreaderRepository {
return row ?? null; return row ?? null;
} }
async upsertReadingProgress(bookFileId: number, userId: number, percentage: number, cfi: string | null = null, xpointer: string | null = null) { async upsertReadingProgress(entry: ReadingProgressUpsert) {
const { bookFileId, userId, percentage, cfi = null, xpointer = null, pageNumber = null } = entry;
await this.db await this.db
.insert(schema.readingProgress) .insert(schema.readingProgress)
.values({ bookFileId, userId, percentage, cfi, koreaderProgress: xpointer }) .values({ bookFileId, userId, percentage, cfi, pageNumber, koreaderProgress: xpointer })
.onConflictDoUpdate({ .onConflictDoUpdate({
target: [schema.readingProgress.bookFileId, schema.readingProgress.userId], target: [schema.readingProgress.bookFileId, schema.readingProgress.userId],
// Deliberately do NOT update updatedAt here. reading_progress.updatedAt must only // Deliberately do NOT update updatedAt here. reading_progress.updatedAt must only
// change when the web reader writes it, so getProgress can use it as an accurate // change when the web reader writes it, so getProgress can use it as an accurate
// "last web-reader sync time" for comparison against koreader_device_progress.updatedAt. // "last web-reader sync time" for comparison against koreader_device_progress.updatedAt.
// The cfi stored here is the server-side conversion of KOReader's XPointer (null when // Exactly one of cfi and pageNumber carries the position, chosen by the book file's
// conversion fails) so the web reader resumes at the same paragraph; stale web locator // format: cfi is the server-side conversion of KOReader's XPointer for reflowable
// fields are never kept, or clients may resume at an older location. Kobo location // documents, pageNumber the page KOReader reports for paged ones. Both are written on
// fields clear because the position no longer matches the device's bookmark; the Kobo // every sync so stale web locator fields are never kept, or clients may resume at an
// pull path recomputes a precise Location from the cfi. // older location. Kobo location fields clear because the position no longer matches the
// device's bookmark; the Kobo pull path recomputes a precise Location from the cfi.
set: { set: {
percentage, percentage,
cfi, cfi,
pageNumber: null, pageNumber,
koreaderProgress: xpointer, koreaderProgress: xpointer,
koboLocationSource: null, koboLocationSource: null,
koboLocationType: null, koboLocationType: null,
@@ -357,7 +357,7 @@ describe('KoreaderService', () => {
describe('saveProgress', () => { describe('saveProgress', () => {
it('resolves the book file, parses progress, extracts chapters, and updates synced progress', async () => { it('resolves the book file, parses progress, extracts chapters, and updates synced progress', async () => {
mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 44, bookId: 55, libraryId: 3 }); mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 44, bookId: 55, libraryId: 3, format: 'epub' });
mockChapterService.parseChapterIndexFromProgress.mockReturnValue(6); mockChapterService.parseChapterIndexFromProgress.mockReturnValue(6);
mockChapterExtractor.extractAndStoreChapters.mockRejectedValueOnce(new Error('extract failed')); mockChapterExtractor.extractAndStoreChapters.mockRejectedValueOnce(new Error('extract failed'));
@@ -383,11 +383,18 @@ describe('KoreaderService', () => {
chapterIndex: 6, chapterIndex: 6,
syncTimestamp: 1700000000, syncTimestamp: 1700000000,
}); });
expect(mockRepo.upsertReadingProgress).toHaveBeenCalledWith(44, 12, 50, null, '/body/DocFragment[7]'); expect(mockRepo.upsertReadingProgress).toHaveBeenCalledWith({
bookFileId: 44,
userId: 12,
percentage: 50,
cfi: null,
xpointer: '/body/DocFragment[7]',
pageNumber: null,
});
expect(mockBookService.syncKoboReadingStateForExternalProgress).toHaveBeenCalledWith(12, 44, 50); expect(mockBookService.syncKoboReadingStateForExternalProgress).toHaveBeenCalledWith(12, 44, 50);
expect(mockBookService.autoUpdateReadStatusForProgress).toHaveBeenCalledWith( expect(mockBookService.autoUpdateReadStatusForProgress).toHaveBeenCalledWith(
12, 12,
{ id: 44, bookId: 55, libraryId: 3 }, { id: 44, bookId: 55, libraryId: 3, format: 'epub' },
50, 50,
expect.objectContaining({ origin: 'koreader', strongRereadEvidence: false }), expect.objectContaining({ origin: 'koreader', strongRereadEvidence: false }),
); );
@@ -438,7 +445,7 @@ describe('KoreaderService', () => {
}); });
it('uses the default device and generated device id when the payload leaves them empty', async () => { it('uses the default device and generated device id when the payload leaves them empty', async () => {
mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 88, bookId: 99, libraryId: 4 }); mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 88, bookId: 99, libraryId: 4, format: 'epub' });
await service.saveProgress(12, { await service.saveProgress(12, {
document: 'default-device-document', document: 'default-device-document',
@@ -458,11 +465,71 @@ describe('KoreaderService', () => {
}); });
}); });
describe('shared progress position routing', () => {
async function syncFormat(format: string | null, progress?: string) {
mockRepo.resolveBookFileByHash.mockResolvedValue({ id: 44, bookId: 55, libraryId: 3, format });
await service.saveProgress(12, { document: 'abcdef1234567890fedcba', percentage: 0.5, progress });
return mockRepo.upsertReadingProgress.mock.calls[0]![0] as { cfi: string | null; pageNumber: number | null; xpointer: string | null };
}
it.each(['pdf', 'cbz', 'cbr', 'cb7', 'PDF'])('stores the KOReader page as pageNumber for %s', async (format) => {
const written = await syncFormat(format, '117');
expect(written.pageNumber).toBe(117);
expect(written.cfi).toBeNull();
expect(mockPositionConverter.xpointerPointToCfi).not.toHaveBeenCalled();
});
it('keeps the raw device position alongside the parsed page', async () => {
const written = await syncFormat('pdf', '117');
expect(written.xpointer).toBe('117');
});
it('converts the xpointer to a cfi and writes no page for a reflowable format', async () => {
mockPositionConverter.xpointerPointToCfi.mockResolvedValue({ status: 'exact', cfi: 'epubcfi(/6/14!/4/2/6)' });
const written = await syncFormat('epub', '/body/DocFragment[7]/body/p[3]/text().0');
expect(written.cfi).toBe('epubcfi(/6/14!/4/2/6)');
expect(written.pageNumber).toBeNull();
expect(mockPositionConverter.xpointerPointToCfi).toHaveBeenCalledWith({ bookFileId: 44, pos: '/body/DocFragment[7]/body/p[3]/text().0' });
});
it('treats an unknown format as reflowable', async () => {
const written = await syncFormat(null, '/body/DocFragment[7]');
expect(written.pageNumber).toBeNull();
expect(mockPositionConverter.xpointerPointToCfi).toHaveBeenCalled();
});
it('clears the stored page when a paged sync carries no position', async () => {
const written = await syncFormat('pdf', undefined);
expect(written.pageNumber).toBeNull();
expect(written.cfi).toBeNull();
expect(written.xpointer).toBeNull();
});
it('clears the stored page rather than storing an xpointer that reached a paged file', async () => {
const written = await syncFormat('pdf', '/body/DocFragment[7]/body/p[3]/text().0');
expect(written.pageNumber).toBeNull();
expect(written.cfi).toBeNull();
});
it('still records the percentage when the page cannot be parsed', async () => {
const written = (await syncFormat('pdf', 'not-a-page')) as unknown as { percentage: number };
expect(written.percentage).toBe(50);
});
});
describe('applyBulkProgress', () => { describe('applyBulkProgress', () => {
const device = { device: 'Kobo Libra 2', deviceId: 'device-a' }; const device = { device: 'Kobo Libra 2', deviceId: 'device-a' };
function bookFile(id: number, bookId = id * 10) { function bookFile(id: number, bookId = id * 10, format: string | null = 'epub') {
return { id, bookId, libraryId: 1 }; return { id, bookId, libraryId: 1, format };
} }
it('reads state once, writes one device statement, and applies shared progress per entry', async () => { it('reads state once, writes one device statement, and applies shared progress per entry', async () => {
@@ -488,6 +555,31 @@ describe('KoreaderService', () => {
expect(result).toEqual({ shared: 2, stale: 0 }); expect(result).toEqual({ shared: 2, stale: 0 });
}); });
it('routes each entry by its own format within a single sweep', async () => {
mockPositionConverter.xpointerPointToCfi.mockResolvedValue({ status: 'exact', cfi: 'epubcfi(/6/14!/4/2/6)' });
await service.applyBulkProgress(
7,
[
{ bookFile: bookFile(10, 100, 'pdf'), percentage: 0.5, progress: '84' },
{ bookFile: bookFile(11, 110, 'epub'), percentage: 0.2, progress: '/body/DocFragment[3]' },
{ bookFile: bookFile(12, 120, 'cbz'), percentage: 0.9, progress: '31' },
],
device,
);
const written = new Map(
mockRepo.upsertReadingProgress.mock.calls.map((call) => {
const entry = call[0] as { bookFileId: number };
return [entry.bookFileId, entry];
}),
);
expect(written.get(10)).toEqual(expect.objectContaining({ pageNumber: 84, cfi: null }));
expect(written.get(11)).toEqual(expect.objectContaining({ pageNumber: null, cfi: 'epubcfi(/6/14!/4/2/6)' }));
expect(written.get(12)).toEqual(expect.objectContaining({ pageNumber: 31, cfi: null }));
});
it('issues no query for an empty entry list', async () => { it('issues no query for an empty entry list', async () => {
await expect(service.applyBulkProgress(7, [], device)).resolves.toEqual({ shared: 0, stale: 0 }); await expect(service.applyBulkProgress(7, [], device)).resolves.toEqual({ shared: 0, stale: 0 });
@@ -550,7 +642,7 @@ describe('KoreaderService', () => {
expect(mockBookService.autoUpdateReadStatusForProgress).toHaveBeenCalledWith( expect(mockBookService.autoUpdateReadStatusForProgress).toHaveBeenCalledWith(
7, 7,
{ id: 10, bookId: 100, libraryId: 1 }, { id: 10, bookId: 100, libraryId: 1, format: 'epub' },
5, 5,
expect.objectContaining({ origin: 'koreader', strongRereadEvidence: true }), expect.objectContaining({ origin: 'koreader', strongRereadEvidence: true }),
); );
@@ -13,6 +13,7 @@ import { KoreaderChapterExtractorService } from './koreader-chapter-extractor.se
import { KoreaderPackageService } from './koreader-package.service'; import { KoreaderPackageService } from './koreader-package.service';
import { pluginRequiresManualUpdate } from './koreader-plugin-update.util'; import { pluginRequiresManualUpdate } from './koreader-plugin-update.util';
import { KoreaderPluginRepository } from './koreader-plugin.repository'; import { KoreaderPluginRepository } from './koreader-plugin.repository';
import { isPagedReadingFormat, parseKoreaderPageNumber } from './koreader-progress-position.util';
import { BookService } from '../book/book.service'; import { BookService } from '../book/book.service';
import { PositionConverterService } from '../position-converter/position-converter.service'; import { PositionConverterService } from '../position-converter/position-converter.service';
import { AchievementEventsService, ACHIEVEMENT_EVENT_BOOK_PROGRESS_CHANGED } from '../achievement/achievement-events.service'; import { AchievementEventsService, ACHIEVEMENT_EVENT_BOOK_PROGRESS_CHANGED } from '../achievement/achievement-events.service';
@@ -28,8 +29,16 @@ const FILE_NAMING_CACHE_MAX_ENTRIES = 5_000;
const SHARED_PROGRESS_CONCURRENCY = 4; const SHARED_PROGRESS_CONCURRENCY = 4;
const CHAPTER_EXTRACTION_CONCURRENCY = 2; const CHAPTER_EXTRACTION_CONCURRENCY = 2;
/** `format` routes the incoming position to the cfi or the pageNumber column. */
export interface KoreaderProgressBookFile {
id: number;
bookId: number;
libraryId: number;
format: string | null;
}
export interface BulkProgressEntry { export interface BulkProgressEntry {
bookFile: { id: number; bookId: number; libraryId: number }; bookFile: KoreaderProgressBookFile;
percentage: number; percentage: number;
progress?: string; progress?: string;
timestamp?: number; timestamp?: number;
@@ -160,7 +169,7 @@ export class KoreaderService {
async applyProgressForResolvedFile( async applyProgressForResolvedFile(
userId: number, userId: number,
bookFile: { id: number; bookId: number; libraryId: number }, bookFile: KoreaderProgressBookFile,
data: { percentage: number; progress?: string; device: string; deviceId: string; timestamp?: number }, data: { percentage: number; progress?: string; device: string; deviceId: string; timestamp?: number },
options?: { skipSharedProgress?: boolean }, options?: { skipSharedProgress?: boolean },
) { ) {
@@ -287,13 +296,25 @@ export class KoreaderService {
private async applySharedProgress( private async applySharedProgress(
userId: number, userId: number,
bookFile: { id: number; bookId: number; libraryId: number }, bookFile: KoreaderProgressBookFile,
data: { percentage: number; progress?: string; timestamp?: number }, data: { percentage: number; progress?: string; timestamp?: number },
previousPercentage: number | null, previousPercentage: number | null,
) { ) {
const bookorbitPercentage = toBookorbitPercentage(data.percentage); const bookorbitPercentage = toBookorbitPercentage(data.percentage);
const cfi = data.progress ? await this.convertProgressToCfi(bookFile.id, data.progress) : null; // KOReader reports a paged document's position as a page number and a reflowable one's as
await this.repo.upsertReadingProgress(bookFile.id, userId, bookorbitPercentage, cfi, data.progress ?? null); // an xpointer, and the web reader resumes from a different column for each, so the format
// decides which one this position becomes.
const paged = isPagedReadingFormat(bookFile.format);
const cfi = !paged && data.progress ? await this.convertProgressToCfi(bookFile.id, data.progress) : null;
const pageNumber = paged ? parseKoreaderPageNumber(data.progress) : null;
await this.repo.upsertReadingProgress({
bookFileId: bookFile.id,
userId,
percentage: bookorbitPercentage,
cfi,
xpointer: data.progress ?? null,
pageNumber,
});
await this.bookService.syncKoboReadingStateForExternalProgress(userId, bookFile.id, bookorbitPercentage).catch(() => undefined); await this.bookService.syncKoboReadingStateForExternalProgress(userId, bookFile.id, bookorbitPercentage).catch(() => undefined);
const strongRereadEvidence = previousPercentage !== null && previousPercentage - bookorbitPercentage >= 10; const strongRereadEvidence = previousPercentage !== null && previousPercentage - bookorbitPercentage >= 10;
await this.bookService.autoUpdateReadStatusForProgress(userId, bookFile, bookorbitPercentage, { await this.bookService.autoUpdateReadStatusForProgress(userId, bookFile, bookorbitPercentage, {
@@ -0,0 +1,180 @@
import { randomUUID } from 'crypto';
import { eq } from 'drizzle-orm';
import * as schema from '../src/db/schema';
import { createCbzFixture, createEpubFixture } from './e2e/reader-state-isolation/reader-state-isolation-fixture-builder';
import {
authHeader,
closeReaderStateIsolationE2EContext,
createLibraryWithFolder,
createReaderStateIsolationE2EContext,
locateBookByAbsolutePath,
triggerAndWaitForLibraryScan,
type CreatedLibrary,
type LocatedBookFile,
type ReaderStateIsolationE2EContext,
} from './e2e/reader-state-isolation/reader-state-isolation-harness';
const KOREADER_USERNAME = `progress-position-device-${randomUUID().slice(0, 8)}`;
const KOREADER_PASSWORD = 'ProgressPositionPass123';
const XPOINTER = '/body/DocFragment[8]/body/p[12]/text().0';
/**
* KOReader reports a paged document's position as a page number and a reflowable one's as an
* xpointer. Both arrive on the same endpoint, so these cases assert the position lands in the
* column the web reader actually resumes from for that format, against a real database.
*/
describe('KOReader progress position routing (e2e)', { timeout: 180_000 }, () => {
let ctx!: ReaderStateIsolationE2EContext;
let library!: CreatedLibrary;
let comic!: LocatedBookFile;
let epub!: LocatedBookFile;
let comicHash!: string;
let epubHash!: string;
function deviceHeaders(): Record<string, string> {
return { 'x-auth-user': KOREADER_USERNAME, 'x-auth-key': KOREADER_PASSWORD };
}
async function fileHashFor(bookFileId: number): Promise<string> {
const [row] = await ctx.db.select({ fileHash: schema.bookFiles.fileHash }).from(schema.bookFiles).where(eq(schema.bookFiles.id, bookFileId));
expect(row?.fileHash).toBeTruthy();
return row!.fileHash!;
}
async function syncFromDevice(hash: string, percentage: number, progress?: string | number) {
const response = await ctx.app.inject({
method: 'PUT',
url: '/api/v1/koreader/syncs/progress',
headers: deviceHeaders(),
payload: { document: hash, percentage, ...(progress === undefined ? {} : { progress }) },
});
expect(response.statusCode).toBe(200);
}
async function storedProgress(bookFileId: number) {
const [row] = await ctx.db
.select({
percentage: schema.readingProgress.percentage,
cfi: schema.readingProgress.cfi,
pageNumber: schema.readingProgress.pageNumber,
koreaderProgress: schema.readingProgress.koreaderProgress,
})
.from(schema.readingProgress)
.where(eq(schema.readingProgress.bookFileId, bookFileId));
return row;
}
/** What the web reader loads when it opens the book. */
async function readerProgress(bookFileId: number) {
const response = await ctx.app.inject({
method: 'GET',
url: `/api/v1/books/files/${bookFileId}/progress`,
headers: authHeader(ctx.adminToken),
});
expect(response.statusCode).toBe(200);
return response.json() as { percentage: number; cfi: string | null; pageNumber: number | null };
}
async function saveFromWebReader(bookFileId: number, payload: Record<string, unknown>) {
const response = await ctx.app.inject({
method: 'POST',
url: `/api/v1/books/files/${bookFileId}/progress`,
headers: authHeader(ctx.adminToken),
payload,
});
expect([200, 201, 204]).toContain(response.statusCode);
}
beforeAll(async () => {
ctx = await createReaderStateIsolationE2EContext();
library = await createLibraryWithFolder(ctx, { name: `koreader-progress-position-${randomUUID()}` });
const comicPath = await createCbzFixture(library.folderPath, 'progress-position-comic.cbz', {
title: `Progress Position Comic ${randomUUID()}`,
});
const epubPath = await createEpubFixture(library.folderPath, 'progress-position-book.epub', {
title: `Progress Position Book ${randomUUID()}`,
uid: `urn:uuid:${randomUUID()}`,
});
await triggerAndWaitForLibraryScan(ctx, library.libraryId);
comic = await locateBookByAbsolutePath(ctx, comicPath);
epub = await locateBookByAbsolutePath(ctx, epubPath);
comicHash = await fileHashFor(comic.bookFileId);
epubHash = await fileHashFor(epub.bookFileId);
const credentials = await ctx.app.inject({
method: 'POST',
url: '/api/v1/koreader/credentials',
headers: authHeader(ctx.adminToken),
payload: { username: KOREADER_USERNAME, password: KOREADER_PASSWORD },
});
expect([200, 201]).toContain(credentials.statusCode);
}, 180_000);
afterAll(async () => {
if (ctx) await closeReaderStateIsolationE2EContext(ctx);
});
it('stores a paged position as a page number the web reader can resume from', async () => {
await syncFromDevice(comicHash, 0.4, '117');
const stored = await storedProgress(comic.bookFileId);
expect(stored?.pageNumber).toBe(117);
expect(stored?.cfi).toBeNull();
expect(stored?.koreaderProgress).toBe('117');
expect(stored?.percentage).toBeCloseTo(40, 5);
await expect(readerProgress(comic.bookFileId)).resolves.toEqual(expect.objectContaining({ pageNumber: 117, cfi: null }));
});
it('accepts the numeric page KOReader serializes for a paged document', async () => {
await syncFromDevice(comicHash, 0.5, 208);
expect((await storedProgress(comic.bookFileId))?.pageNumber).toBe(208);
});
it('advances the stored page on the next sync of the same book', async () => {
await syncFromDevice(comicHash, 0.6, '242');
expect((await storedProgress(comic.bookFileId))?.pageNumber).toBe(242);
});
it('leaves the page column empty for a reflowable document', async () => {
await syncFromDevice(epubHash, 0.3, XPOINTER);
const stored = await storedProgress(epub.bookFileId);
expect(stored?.pageNumber).toBeNull();
expect(stored?.koreaderProgress).toBe(XPOINTER);
expect((await readerProgress(epub.bookFileId)).pageNumber).toBeNull();
});
it('replaces a page saved by the web reader instead of discarding it', async () => {
await saveFromWebReader(comic.bookFileId, { percentage: 12, pageNumber: 34 });
expect((await storedProgress(comic.bookFileId))?.pageNumber).toBe(34);
await syncFromDevice(comicHash, 0.55, '221');
expect((await storedProgress(comic.bookFileId))?.pageNumber).toBe(221);
});
it('clears a stale page when the device sends a position it cannot supply', async () => {
await syncFromDevice(comicHash, 0.7, '300');
expect((await storedProgress(comic.bookFileId))?.pageNumber).toBe(300);
await syncFromDevice(comicHash, 0.75);
const stored = await storedProgress(comic.bookFileId);
expect(stored?.pageNumber).toBeNull();
expect(stored?.percentage).toBeCloseTo(75, 5);
});
it('rejects a page that would overflow the column rather than failing the sync', async () => {
await syncFromDevice(comicHash, 0.8, '99999999999999');
const stored = await storedProgress(comic.bookFileId);
expect(stored?.pageNumber).toBeNull();
expect(stored?.percentage).toBeCloseTo(80, 5);
});
});