Migrations/bookorbit
1
0
Fork 0
mirror of https://github.com/bookorbit/bookorbit.git synced 2026-11-03 00:11:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(koreader): rank synced books by when the device read them

Browse Source 
Stamp reading_progress.last_read_at with the device's reading time instead of the
server's write time. A sweep pushes a whole shelf at once, so sorting by last read
came out in processing order. Keep the later of the stored and pushed times, so a
late or offline push never sinks a newer read from another client.

Fixes #1433
This commit is contained in:
neon
2026-10-20 15:44:22 +00:00
parent 98415b90cc
commit 2e454cc846
5 changed files with 145 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/modules/koreader/koreader.repository.test.ts
+54 -21
View File
@@ -1296,46 +1296,79 @@ describe('KoreaderRepository', () => {
// freezes on purpose so it stays a "last local write" marker. A KOReader-only reader therefore
// kept the timestamp of their first ever sync forever and the sort looked random.
describe('last-read timestamps in the compiled SQL', () => {
async function compileUpsert() {
type Query = { sql: string; params: unknown[] };
async function compileUpsert(timing: { readAt?: Date; lastReadAt?: Date } = {}) {
const { db: capturingDb, queries } = createCapturingDb();
const capturingRepo = new KoreaderRepository(capturingDb as never);
await capturingRepo.upsertReadingProgress({ bookFileId: 44, userId: 12, percentage: 41.25 });
await capturingRepo.upsertReadingProgress({ bookFileId: 44, userId: 12, percentage: 41.25, ...timing });
expect(queries).toHaveLength(1);
return queries[0]!;
}
function stampedLastReadAt({ sql: text, params }: { sql: string; params: unknown[] }) {
const match = /"last_read_at" = \$(\d+)/.exec(text);
expect(match).not.toBeNull();
return new Date(String(params[Number(match![1]) - 1])).getTime();
function paramTime({ params }: Query, placeholder: string | undefined) {
expect(placeholder).toMatch(/^\$\d+$/);
return new Date(String(params[Number(placeholder!.slice(1)) - 1])).getTime();
}
it('freezes updated_at but advances last_read_at on conflict', async () => {
function insertedTime(query: Query, column: string) {
const match = /^insert into "reading_progress" \(([^)]*)\) values \(([^)]*)\)/.exec(query.sql);
expect(match).not.toBeNull();
const columns = match![1]!.split(', ');
return paramTime(query, match![2]!.split(', ')[columns.indexOf(`"${column}"`)]);
}
function conflictLastReadAt(query: Query) {
const match = /"last_read_at" = greatest\("reading_progress"\."last_read_at", (\$\d+)::timestamptz\)/.exec(query.sql);
expect(match).not.toBeNull();
return paramTime(query, match![1]);
}
function conflictTextUpdatedAt(query: Query) {
return paramTime(query, /"text_updated_at" = (\$\d+)/.exec(query.sql)?.[1]);
}
it('freezes updated_at and only ever advances last_read_at on conflict', async () => {
const query = await compileUpsert();
expect(query.sql).toContain('"updated_at" = "reading_progress"."updated_at"');
expect(query.sql).toMatch(/"last_read_at" = \$\d+/);
expect(query.sql).not.toMatch(/"last_read_at" = "reading_progress"\."last_read_at"/);
expect(stampedLastReadAt(query)).not.toBeNaN();
expect(conflictLastReadAt(query)).not.toBeNaN();
});
it('stamps last_read_at close to now rather than reusing a stored value', async () => {
// Regression: a sweep pushes a device's whole shelf at once, and stamping each
// write with the server's clock ranked every book in it as read in the same few seconds.
it('stamps last_read_at with when the device read, not when the push arrived', async () => {
const readAt = new Date('2026-03-04T21:15:00.000Z');
const query = await compileUpsert({ readAt });
expect(insertedTime(query, 'last_read_at')).toBe(readAt.getTime());
expect(conflictLastReadAt(query)).toBe(readAt.getTime());
});
it('keeps the reading time for last_read_at when the text position is stamped later', async () => {
const readAt = new Date('2026-10-17T06:44:16.000Z');
const lastReadAt = new Date('2026-10-10T17:00:00.000Z');
const query = await compileUpsert({ readAt, lastReadAt });
expect(insertedTime(query, 'last_read_at')).toBe(lastReadAt.getTime());
expect(conflictLastReadAt(query)).toBe(lastReadAt.getTime());
expect(insertedTime(query, 'text_updated_at')).toBe(readAt.getTime());
expect(conflictTextUpdatedAt(query)).toBe(readAt.getTime());
});
it("falls back to the write's own time when the device gives none", async () => {
const before = Date.now();
const query = await compileUpsert();
const after = Date.now();
const stamped = stampedLastReadAt(query);
expect(stamped).toBeGreaterThanOrEqual(before);
expect(stamped).toBeLessThanOrEqual(after);
});
it('lets the insert branch default last_read_at instead of leaving it unset', async () => {
const { sql: text } = await compileUpsert();
expect(text).toContain('"last_read_at"');
expect(text.indexOf('"last_read_at"')).toBeLessThan(text.indexOf('on conflict'));
for (const stamped of [insertedTime(query, 'last_read_at'), conflictLastReadAt(query)]) {
expect(stamped).toBeGreaterThanOrEqual(before);
expect(stamped).toBeLessThanOrEqual(after);
}
});
});
});
server/src/modules/koreader/koreader.repository.ts
+8 -2
View File
@@ -82,6 +82,8 @@ export interface ReadingProgressUpsert {
pageNumber?: number | null;
/** When the device read to this position; the write's own time when it does not say. */
readAt?: Date;
/** When the reading happened, for last_read_at; defaults to readAt. Never moves the stored value back. */
lastReadAt?: Date;
}
export interface DeviceProgressUpsert {
@@ -720,11 +722,11 @@ export class KoreaderRepository {
}
async upsertReadingProgress(entry: ReadingProgressUpsert) {
const { bookFileId, userId, percentage, cfi = null, xpointer = null, pageNumber = null, readAt = new Date() } = entry;
const { bookFileId, userId, percentage, cfi = null, xpointer = null, pageNumber = null, readAt = new Date(), lastReadAt = readAt } = entry;
await this.db
.insert(schema.readingProgress)
.values({ bookFileId, userId, percentage, cfi, pageNumber, koreaderProgress: xpointer, textUpdatedAt: readAt })
.values({ bookFileId, userId, percentage, cfi, pageNumber, koreaderProgress: xpointer, textUpdatedAt: readAt, lastReadAt })
.onConflictDoUpdate({
target: [schema.readingProgress.bookFileId, schema.readingProgress.userId],
// Deliberately do NOT update updatedAt here. reading_progress.updatedAt must only
@@ -747,6 +749,10 @@ export class KoreaderRepository {
koboContentSourceProgressPercent: null,
textUpdatedAt: readAt,
updatedAt: sql`"reading_progress"."updated_at"`,
// The device's reading time, not the write's: a sweep pushes a whole shelf at once and
// an offline book arrives on reconnection, so the write's own time ranked them all as
// just read. Held to the stored value so an older push never sinks a newer read elsewhere.
lastReadAt: sql`greatest("reading_progress"."last_read_at", ${lastReadAt.toISOString()}::timestamptz)`,
},
});
}
server/src/modules/koreader/koreader.service.test.ts
+31 -1
View File
@@ -421,9 +421,11 @@ describe('KoreaderService', () => {
});
expect(mockRepo.upsertDeviceProgress).toHaveBeenCalled();
const written = mockRepo.upsertReadingProgress.mock.calls[0]![0] as { percentage: number; readAt: Date };
const written = mockRepo.upsertReadingProgress.mock.calls[0]![0] as { percentage: number; readAt: Date; lastReadAt: Date };
expect(written.percentage).toBe(70);
expect(written.readAt.getTime()).toBeGreaterThanOrEqual(before);
// Still ranked by when it was read, so reconnecting does not lift it above books read since.
expect(written.lastReadAt).toEqual(new Date('2026-10-10T17:00:00Z'));
expect(mockBookService.syncAudioProgressForExternalEbookProgress).toHaveBeenCalledWith(
12,
55,
@@ -524,6 +526,7 @@ describe('KoreaderService', () => {
xpointer: '/body/DocFragment[7]',
pageNumber: null,
readAt: new Date(1700000000 * 1000),
lastReadAt: new Date(1700000000 * 1000),
});
expect(mockRepo.restoreDevice).toHaveBeenCalledWith(12, 'device-12');
expect(mockBookService.syncKoboReadingStateForExternalProgress).toHaveBeenCalledWith(12, 44, 50);
@@ -738,6 +741,33 @@ describe('KoreaderService', () => {
expect(written.get(12)).toEqual(expect.objectContaining({ pageNumber: 31, cfi: null }));
});
// Regression: a first sweep pushes every book on the device, and each one was ranked as read
// the moment the sweep arrived, so sorting by last read came out in processing order.
it("stamps each swept book's last read with when the device last opened it", async () => {
const appliedAt = Math.floor(Date.now() / 1000);
await service.applyBulkProgress(
7,
[
{ bookFile: bookFile(10, 100), percentage: 0.5, timestamp: 1700000000 },
{ bookFile: bookFile(11, 110), percentage: 0.2, timestamp: 1750000000 },
{ bookFile: bookFile(12, 120), percentage: 0.9 },
],
device,
);
const written = new Map(
mockRepo.upsertReadingProgress.mock.calls.map((call) => {
const entry = call[0] as { bookFileId: number; lastReadAt: Date };
return [entry.bookFileId, entry.lastReadAt];
}),
);
expect(written.get(10)).toEqual(new Date(1700000000 * 1000));
expect(written.get(11)).toEqual(new Date(1750000000 * 1000));
expect(written.get(12)!.getTime()).toBeGreaterThanOrEqual(appliedAt * 1000);
});
it('issues no query for an empty entry list', async () => {
await expect(service.applyBulkProgress(7, [], device)).resolves.toEqual({ shared: 0, stale: 0, held: 0 });
server/src/modules/koreader/koreader.service.ts
+3 -1
View File
@@ -501,8 +501,10 @@ export class KoreaderService {
xpointer: data.progress ?? null,
pageNumber,
// A late push moves the page now, which is what other clients need to see; the narration
// below still judges it by when it was read, so newer listening keeps its marker.
// below and last-read ordering still judge it by when it was read, so newer listening
// keeps its marker.
readAt: late ? new Date() : readAt,
lastReadAt: readAt,
});
await this.bookService.syncKoboReadingStateForExternalProgress(userId, bookFile.id, bookorbitPercentage).catch(() => undefined);
await this.bookService
server/test/koreader-progress-position.e2e-spec.ts
+49
View File
@@ -504,4 +504,53 @@ describe('KOReader progress position routing (e2e)', { timeout: 180_000 }, () =>
expect(pull.json()).toEqual(expect.objectContaining({ percentage: 0.7, progress: null }));
}
});
// Regression: a sweep pushes the device's whole shelf at once, and every book in it was ranked
// as read the moment it arrived, so sorting by last read came out in processing order.
it('ranks swept books by when the device read them, and never below a newer read elsewhere', async () => {
const [olderPath, newerPath] = await Promise.all([
createEpubFixture(library.folderPath, 'last-read/Older.epub', { title: `Last Read Older ${randomUUID()}`, uid: `urn:uuid:${randomUUID()}` }),
createEpubFixture(library.folderPath, 'last-read/Newer.epub', { title: `Last Read Newer ${randomUUID()}`, uid: `urn:uuid:${randomUUID()}` }),
]);
await triggerAndWaitForLibraryScan(ctx, library.libraryId);
const older = await locateBookByAbsolutePath(ctx, olderPath);
const newer = await locateBookByAbsolutePath(ctx, newerPath);
const lastReadAt = async (bookFileId: number) => {
const [row] = await ctx.db
.select({ lastReadAt: schema.readingProgress.lastReadAt })
.from(schema.readingProgress)
.where(eq(schema.readingProgress.bookFileId, bookFileId));
return row?.lastReadAt.getTime();
};
const now = Math.floor(Date.now() / 1000);
const olderReadAt = now - 30 * 86400;
const newerReadAt = now - 2 * 86400;
const sweep = await ctx.app.inject({
method: 'POST',
url: '/api/v1/koreader/plugin/progress',
headers: deviceHeaders(),
payload: {
deviceId: DEVICE_ID,
deviceModel: 'Regression device',
pluginVersion: '1.7.0',
items: [
{ hash: await fileHashFor(older.bookFileId), percentage: 0.4, timestamp: olderReadAt },
{ hash: await fileHashFor(newer.bookFileId), percentage: 0.3, timestamp: newerReadAt },
],
},
});
expect(sweep.statusCode).toBe(201);
expect(await lastReadAt(older.bookFileId)).toBe(olderReadAt * 1000);
expect(await lastReadAt(newer.bookFileId)).toBe(newerReadAt * 1000);
await saveFromWebReader(older.bookFileId, { percentage: 45 });
const webReadAt = await lastReadAt(older.bookFileId);
expect(webReadAt).toBeGreaterThanOrEqual(now * 1000);
// Read further offline an hour ago and pushed late: the page is taken, the newer web read stays the last one.
await syncFromDevice(await fileHashFor(older.bookFileId), 0.7, undefined, now - 3600);
expect((await storedProgress(older.bookFileId))?.percentage).toBeCloseTo(70, 5);
expect(await lastReadAt(older.bookFileId)).toBe(webReadAt);
});
});