mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(stats): stop deleted KOReader sessions returning and surface reset holds
Deleting a KOReader session left its page events stored, so the next upload within the session gap re-derived it. Record the deleted span in koreader_session_tombstones and skip those events during derivation. Show the reset-hold notice on the Reading Log as well, since Reset is reachable there and a held device otherwise reads as unexplained progress. Refs #1355
This commit is contained in:
@@ -0,0 +1,53 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import {
|
||||
buildDeviceSessionIdPrefix,
|
||||
buildSessionId,
|
||||
buildSessionIdPrefix,
|
||||
koreaderSessionDeviceKey,
|
||||
parseKoreaderSessionId,
|
||||
} from './koreader-session-id.utils';
|
||||
|
||||
const DEVICE_ID = 'abcdef12-3456-7890-abcd-ef1234567890';
|
||||
|
||||
describe('koreader session ids', () => {
|
||||
it('keeps the first eight characters of the device id as its key', () => {
|
||||
expect(koreaderSessionDeviceKey(DEVICE_ID)).toBe('abcdef12');
|
||||
expect(koreaderSessionDeviceKey('dev')).toBe('dev');
|
||||
});
|
||||
|
||||
it('builds prefixes that nest inside each other', () => {
|
||||
expect(buildDeviceSessionIdPrefix(DEVICE_ID)).toBe('kor:abcdef12:');
|
||||
expect(buildSessionIdPrefix(DEVICE_ID, 42)).toBe('kor:abcdef12:42:');
|
||||
expect(buildSessionId(DEVICE_ID, 42, 1_780_000_000)).toBe('kor:abcdef12:42:1780000000');
|
||||
});
|
||||
|
||||
it.each([
|
||||
[DEVICE_ID, 42, 1_780_000_000],
|
||||
['dev', 1, 0],
|
||||
['e2e-stats-0001', 2_147_483_647, 2_147_483_647],
|
||||
['A-B', 7, 5],
|
||||
])('reads back what it builds for device %s', (deviceId, bookFileId, clusterStartEpoch) => {
|
||||
expect(parseKoreaderSessionId(buildSessionId(deviceId, bookFileId, clusterStartEpoch))).toEqual({
|
||||
deviceKey: koreaderSessionDeviceKey(deviceId),
|
||||
bookFileId,
|
||||
clusterStartEpoch,
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
['legacy estimator id', `ks-${'a'.repeat(12)}-${'b'.repeat(32)}`],
|
||||
['web reader id', '3f1c2f9e-0d6b-4d5e-9a51-2b2c1d8e7f60'],
|
||||
['kobo statistics id', 'kst:30:42:1:2'],
|
||||
['missing cluster start', 'kor:abcdef12:42:'],
|
||||
['device key longer than a key can be', 'kor:abcdef123:42:1780000000'],
|
||||
['zero book file id', 'kor:abcdef12:0:1780000000'],
|
||||
['non-numeric book file id', 'kor:abcdef12:x:1780000000'],
|
||||
['trailing segment', 'kor:abcdef12:42:1780000000:9'],
|
||||
['empty string', ''],
|
||||
['null', null],
|
||||
['undefined', undefined],
|
||||
])('rejects a %s', (_label, sessionId) => {
|
||||
expect(parseKoreaderSessionId(sessionId)).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,38 @@
|
||||
/**
|
||||
* Identity of a reading session derived from KOReader page events:
|
||||
* `kor:<device key>:<book file id>:<cluster start>`. Derivation builds it and the session delete
|
||||
* path reads it back, so both sides share one definition of the format.
|
||||
*/
|
||||
|
||||
const KOREADER_SESSION_ID_PATTERN = /^kor:([A-Za-z0-9-]{1,8}):([1-9]\d*):(\d+)$/;
|
||||
|
||||
/** The part of a device id a session id keeps. Two devices sharing it share session identities. */
|
||||
export function koreaderSessionDeviceKey(deviceId: string): string {
|
||||
return deviceId.slice(0, 8);
|
||||
}
|
||||
|
||||
/** Everything one device's derived sessions share, across every book it has read. */
|
||||
export function buildDeviceSessionIdPrefix(deviceId: string): string {
|
||||
return `kor:${koreaderSessionDeviceKey(deviceId)}:`;
|
||||
}
|
||||
|
||||
export function buildSessionIdPrefix(deviceId: string, bookFileId: number): string {
|
||||
return `${buildDeviceSessionIdPrefix(deviceId)}${bookFileId}:`;
|
||||
}
|
||||
|
||||
export function buildSessionId(deviceId: string, bookFileId: number, clusterStartEpoch: number): string {
|
||||
return `${buildSessionIdPrefix(deviceId, bookFileId)}${clusterStartEpoch}`;
|
||||
}
|
||||
|
||||
export interface ParsedKoreaderSessionId {
|
||||
deviceKey: string;
|
||||
bookFileId: number;
|
||||
clusterStartEpoch: number;
|
||||
}
|
||||
|
||||
/** Null for anything that is not a derived KOReader session id, including legacy estimator ids. */
|
||||
export function parseKoreaderSessionId(sessionId: string | null | undefined): ParsedKoreaderSessionId | null {
|
||||
const match = sessionId ? KOREADER_SESSION_ID_PATTERN.exec(sessionId) : null;
|
||||
if (!match) return null;
|
||||
return { deviceKey: match[1]!, bookFileId: Number(match[2]), clusterStartEpoch: Number(match[3]) };
|
||||
}
|
||||
@@ -0,0 +1,15 @@
|
||||
CREATE TABLE "koreader_session_tombstones" (
|
||||
"id" serial PRIMARY KEY NOT NULL,
|
||||
"user_id" integer NOT NULL,
|
||||
"book_file_id" integer NOT NULL,
|
||||
"device_key" varchar(100) NOT NULL,
|
||||
"start_time" bigint NOT NULL,
|
||||
"end_time" bigint NOT NULL,
|
||||
"created_at" timestamp with time zone DEFAULT now() NOT NULL,
|
||||
CONSTRAINT "koreader_session_tombstones_range_chk" CHECK ("koreader_session_tombstones"."end_time" > "koreader_session_tombstones"."start_time")
|
||||
);
|
||||
--> statement-breakpoint
|
||||
ALTER TABLE "koreader_session_tombstones" ADD CONSTRAINT "koreader_session_tombstones_user_id_users_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."users"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
|
||||
ALTER TABLE "koreader_session_tombstones" ADD CONSTRAINT "koreader_session_tombstones_book_file_id_book_files_id_fk" FOREIGN KEY ("book_file_id") REFERENCES "public"."book_files"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
|
||||
CREATE INDEX "kst_user_file_device_start_idx" ON "koreader_session_tombstones" USING btree ("user_id","book_file_id","device_key","start_time");--> statement-breakpoint
|
||||
CREATE INDEX "koreader_session_tombstones_book_file_id_idx" ON "koreader_session_tombstones" USING btree ("book_file_id");
|
||||
File diff suppressed because it is too large
Load Diff
@@ -778,6 +778,13 @@
|
||||
"when": 1791838145626,
|
||||
"tag": "0110_add_email_provider_test_result",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 111,
|
||||
"version": "7",
|
||||
"when": 1791853301183,
|
||||
"tag": "0111_add_koreader_session_tombstones",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -302,6 +302,37 @@ export const koreaderPageStats = pgTable(
|
||||
export type KoreaderPageStat = typeof koreaderPageStats.$inferSelect;
|
||||
export type NewKoreaderPageStat = typeof koreaderPageStats.$inferInsert;
|
||||
|
||||
/**
|
||||
* Spans of page events whose derived session the reader deleted. Sessions are re-derived from the
|
||||
* stored page events whenever a nearby event arrives, and a plugin can upload its history again,
|
||||
* so deleting the session row alone does not stick: derivation skips events inside these spans.
|
||||
*
|
||||
* Keyed on the device key a KOReader session id carries rather than the full device id, because
|
||||
* that is all a deleted session row records about the device that produced it.
|
||||
*/
|
||||
export const koreaderSessionTombstones = pgTable(
|
||||
'koreader_session_tombstones',
|
||||
{
|
||||
id: serial('id').primaryKey(),
|
||||
userId: integer('user_id')
|
||||
.notNull()
|
||||
.references(() => users.id, { onDelete: 'cascade' }),
|
||||
bookFileId: integer('book_file_id')
|
||||
.notNull()
|
||||
.references(() => bookFiles.id, { onDelete: 'cascade' }),
|
||||
deviceKey: varchar('device_key', { length: 100 }).notNull(),
|
||||
// Unix seconds on the device clock, half-open: [startTime, endTime).
|
||||
startTime: bigint('start_time', { mode: 'number' }).notNull(),
|
||||
endTime: bigint('end_time', { mode: 'number' }).notNull(),
|
||||
createdAt: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(),
|
||||
},
|
||||
(t) => [
|
||||
index('kst_user_file_device_start_idx').on(t.userId, t.bookFileId, t.deviceKey, t.startTime),
|
||||
index('koreader_session_tombstones_book_file_id_idx').on(t.bookFileId),
|
||||
check('koreader_session_tombstones_range_chk', sql`${t.endTime} > ${t.startTime}`),
|
||||
],
|
||||
);
|
||||
|
||||
export const koreaderDeviceSweeps = pgTable(
|
||||
'koreader_device_sweeps',
|
||||
{
|
||||
|
||||
@@ -5,6 +5,7 @@ import type { NodePgDatabase } from 'drizzle-orm/node-postgres';
|
||||
import { DB } from '../../db';
|
||||
import * as schema from '../../db/schema';
|
||||
import { chunk } from '../../common/utils/batch.utils';
|
||||
import { buildSessionIdPrefix, koreaderSessionDeviceKey } from '../../common/utils/koreader-session-id.utils';
|
||||
import {
|
||||
aggregateReadingSessionDailyStats,
|
||||
getDayRangeForDateKeys,
|
||||
@@ -14,8 +15,8 @@ import {
|
||||
import {
|
||||
KOREADER_MAX_EVENT_DURATION_SECONDS,
|
||||
KOREADER_SESSION_GAP_SECONDS,
|
||||
buildSessionIdPrefix,
|
||||
deriveKoreaderSessions,
|
||||
excludeTombstonedEvents,
|
||||
type DerivedKoreaderSession,
|
||||
type KoreaderPageEvent,
|
||||
} from './koreader-stats.util';
|
||||
@@ -115,7 +116,22 @@ export class KoreaderPluginRepository {
|
||||
)
|
||||
.orderBy(schema.koreaderPageStats.startTime, schema.koreaderPageStats.page);
|
||||
|
||||
const desired = deriveKoreaderSessions(windowEvents, deviceId, bookFileId);
|
||||
// The window still grows over suppressed events: dropping events only widens the gaps
|
||||
// between the rest, so its edges stay true cluster boundaries.
|
||||
const tombstones = await tx
|
||||
.select({ startTime: schema.koreaderSessionTombstones.startTime, endTime: schema.koreaderSessionTombstones.endTime })
|
||||
.from(schema.koreaderSessionTombstones)
|
||||
.where(
|
||||
and(
|
||||
eq(schema.koreaderSessionTombstones.userId, userId),
|
||||
eq(schema.koreaderSessionTombstones.bookFileId, bookFileId),
|
||||
eq(schema.koreaderSessionTombstones.deviceKey, koreaderSessionDeviceKey(deviceId)),
|
||||
lte(schema.koreaderSessionTombstones.startTime, window.lastStart),
|
||||
gt(schema.koreaderSessionTombstones.endTime, window.firstStart),
|
||||
),
|
||||
);
|
||||
|
||||
const desired = deriveKoreaderSessions(excludeTombstonedEvents(windowEvents, tombstones), deviceId, bookFileId);
|
||||
const prefix = buildSessionIdPrefix(deviceId, bookFileId);
|
||||
|
||||
// Selected by time overlap rather than by recomputed id: a merged cluster changes
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { buildSessionId, buildSessionIdPrefix } from '../../common/utils/koreader-session-id.utils';
|
||||
import {
|
||||
buildSessionId,
|
||||
buildSessionIdPrefix,
|
||||
clusterPageStats,
|
||||
computeClusterMetrics,
|
||||
deriveKoreaderSessions,
|
||||
excludeTombstonedEvents,
|
||||
type KoreaderPageEvent,
|
||||
} from './koreader-stats.util';
|
||||
|
||||
@@ -115,3 +115,49 @@ describe('deriveKoreaderSessions', () => {
|
||||
expect(buildSessionIdPrefix(DEVICE_ID, FILE_ID)).toBe('kor:abcdef12:42:');
|
||||
});
|
||||
});
|
||||
|
||||
describe('excludeTombstonedEvents', () => {
|
||||
const sitting = [event(1000, 60, 1), event(1060, 60, 2), event(1120, 60, 3)];
|
||||
|
||||
it('returns the same array when nothing is tombstoned', () => {
|
||||
expect(excludeTombstonedEvents(sitting, [])).toBe(sitting);
|
||||
});
|
||||
|
||||
it('drops every event that starts inside a span', () => {
|
||||
expect(excludeTombstonedEvents(sitting, [{ startTime: 1000, endTime: 1180 }])).toEqual([]);
|
||||
});
|
||||
|
||||
it('treats the span start as inside and the span end as outside', () => {
|
||||
const atEnd = event(1180, 60, 4);
|
||||
const beforeStart = event(999, 1, 0);
|
||||
const kept = excludeTombstonedEvents([beforeStart, ...sitting, atEnd], [{ startTime: 1000, endTime: 1180 }]);
|
||||
expect(kept).toEqual([beforeStart, atEnd]);
|
||||
});
|
||||
|
||||
it('keeps an event that started before a span even when it runs into it', () => {
|
||||
const longPage = event(900, 300, 0);
|
||||
expect(excludeTombstonedEvents([longPage], [{ startTime: 1000, endTime: 1180 }])).toEqual([longPage]);
|
||||
});
|
||||
|
||||
it('applies several spans at once', () => {
|
||||
const events = [event(100), event(200), event(300), event(400), event(500)];
|
||||
const kept = excludeTombstonedEvents(events, [
|
||||
{ startTime: 150, endTime: 250 },
|
||||
{ startTime: 400, endTime: 401 },
|
||||
]);
|
||||
expect(kept.map((e) => e.startTime)).toEqual([100, 300, 500]);
|
||||
});
|
||||
|
||||
it('leaves the reading after a deleted sitting as its own session', () => {
|
||||
const deleted = [event(1000, 60, 1), event(1060, 60, 2), event(1120, 60, 3)];
|
||||
const readOn = [event(1780, 60, 4), event(1840, 60, 5)];
|
||||
const all = [...deleted, ...readOn];
|
||||
|
||||
expect(deriveKoreaderSessions(all, DEVICE_ID, FILE_ID).map((s) => s.sessionId)).toEqual([buildSessionId(DEVICE_ID, FILE_ID, 1000)]);
|
||||
|
||||
const sessions = deriveKoreaderSessions(excludeTombstonedEvents(all, [{ startTime: 1000, endTime: 1180 }]), DEVICE_ID, FILE_ID);
|
||||
expect(sessions).toHaveLength(1);
|
||||
expect(sessions[0]!.sessionId).toBe(buildSessionId(DEVICE_ID, FILE_ID, 1780));
|
||||
expect(sessions[0]!.durationSeconds).toBe(120);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import { buildSessionId } from '../../common/utils/koreader-session-id.utils';
|
||||
|
||||
export const KOREADER_SESSION_GAP_SECONDS = 1800;
|
||||
/** Upper bound accepted for a single page event, used to bound backward window scans. */
|
||||
export const KOREADER_MAX_EVENT_DURATION_SECONDS = 86400;
|
||||
@@ -20,19 +22,6 @@ export interface DerivedKoreaderSession {
|
||||
endProgress: number | null;
|
||||
}
|
||||
|
||||
/** Everything one device's derived sessions share, across every book it has read. */
|
||||
export function buildDeviceSessionIdPrefix(deviceId: string): string {
|
||||
return `kor:${deviceId.slice(0, 8)}:`;
|
||||
}
|
||||
|
||||
export function buildSessionIdPrefix(deviceId: string, bookFileId: number): string {
|
||||
return `${buildDeviceSessionIdPrefix(deviceId)}${bookFileId}:`;
|
||||
}
|
||||
|
||||
export function buildSessionId(deviceId: string, bookFileId: number, clusterStartEpoch: number): string {
|
||||
return `${buildSessionIdPrefix(deviceId, bookFileId)}${clusterStartEpoch}`;
|
||||
}
|
||||
|
||||
export function clusterPageStats(events: KoreaderPageEvent[], gapSeconds: number = KOREADER_SESSION_GAP_SECONDS): KoreaderPageEvent[][] {
|
||||
const sorted = [...events].sort((a, b) => a.startTime - b.startTime || a.page - b.page);
|
||||
const clusters: KoreaderPageEvent[][] = [];
|
||||
@@ -82,6 +71,22 @@ export function computeClusterMetrics(cluster: KoreaderPageEvent[], deviceId: st
|
||||
};
|
||||
}
|
||||
|
||||
/** A span of device-clock seconds, half-open: [startTime, endTime). */
|
||||
export interface KoreaderEventSpan {
|
||||
startTime: number;
|
||||
endTime: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Drops events that start inside a deleted session's span. Half-open because the page after a
|
||||
* session's last one typically starts on the very second that session ends, and that page belongs
|
||||
* to whatever the reader did next.
|
||||
*/
|
||||
export function excludeTombstonedEvents<T extends { startTime: number }>(events: T[], spans: KoreaderEventSpan[]): T[] {
|
||||
if (spans.length === 0) return events;
|
||||
return events.filter((event) => !spans.some((span) => event.startTime >= span.startTime && event.startTime < span.endTime));
|
||||
}
|
||||
|
||||
export function deriveKoreaderSessions(
|
||||
events: KoreaderPageEvent[],
|
||||
deviceId: string,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { readingSessions, userReadingDailyStats } from '../../db/schema';
|
||||
import { koreaderSessionTombstones, readingSessions, userReadingDailyStats } from '../../db/schema';
|
||||
import { ReadingSessionRepository } from './reading-session.repository';
|
||||
|
||||
function makeDbHarness(options?: {
|
||||
@@ -694,7 +694,17 @@ describe('ReadingSessionRepository - deleteSessionByBook', () => {
|
||||
});
|
||||
|
||||
function makeDeleteHarness(
|
||||
sessionRow: { id: number; startedAt: Date; endedAt: Date; durationSeconds: number; progressDelta: number | null; libraryId: number } | null,
|
||||
sessionRow: {
|
||||
id: number;
|
||||
startedAt: Date;
|
||||
endedAt: Date;
|
||||
durationSeconds: number;
|
||||
progressDelta: number | null;
|
||||
libraryId: number;
|
||||
sessionId?: string | null;
|
||||
source?: string | null;
|
||||
bookFileId?: number | null;
|
||||
} | null,
|
||||
recomputeRows: Array<{ startedAt: Date; endedAt: Date; durationSeconds: number; progressDelta: number | null }> = [],
|
||||
) {
|
||||
const limit = vi.fn().mockResolvedValue(sessionRow ? [sessionRow] : []);
|
||||
@@ -711,19 +721,93 @@ describe('ReadingSessionRepository - deleteSessionByBook', () => {
|
||||
const deleteFn = vi.fn().mockReturnValue({ where: deleteWhere });
|
||||
const dailyConflictUpdate = vi.fn().mockResolvedValue(undefined);
|
||||
const dailyValues = vi.fn().mockReturnValue({ onConflictDoUpdate: dailyConflictUpdate });
|
||||
const tombstoneValues = vi.fn().mockResolvedValue(undefined);
|
||||
const insert = vi.fn((table: unknown) => {
|
||||
if (table === userReadingDailyStats) return { values: dailyValues };
|
||||
if (table === koreaderSessionTombstones) return { values: tombstoneValues };
|
||||
throw new Error('Unexpected table in insert');
|
||||
});
|
||||
|
||||
const tx = { execute: vi.fn().mockResolvedValue(undefined), select, delete: deleteFn, insert };
|
||||
const transaction = vi.fn(async (cb: (trx: typeof tx) => Promise<unknown>) => cb(tx));
|
||||
const db = { transaction };
|
||||
const db = { transaction, insert: vi.fn() };
|
||||
const repo = new ReadingSessionRepository(db as never);
|
||||
|
||||
return { repo, innerJoin: lookupInnerJoin, deleteFn, dailyValues, transaction, tx };
|
||||
return { repo, innerJoin: lookupInnerJoin, deleteFn, dailyValues, tombstoneValues, transaction, tx, db };
|
||||
}
|
||||
|
||||
const KOREADER_ROW = {
|
||||
id: 5,
|
||||
sessionId: 'kor:abcdef12:16:1791594307',
|
||||
source: 'koreader',
|
||||
bookFileId: 16,
|
||||
startedAt: new Date(1_791_594_307_000),
|
||||
endedAt: new Date(1_791_595_507_000),
|
||||
durationSeconds: 1200,
|
||||
progressDelta: 6.67,
|
||||
libraryId: 3,
|
||||
};
|
||||
|
||||
it('tombstones the span of a deleted KOReader session so derivation cannot rebuild it', async () => {
|
||||
const { repo, tombstoneValues, tx, db } = makeDeleteHarness(KOREADER_ROW);
|
||||
|
||||
await expect(repo.deleteSessionByBook(1, 2, 5)).resolves.toEqual({ found: true });
|
||||
|
||||
expect(tx.insert).toHaveBeenCalledWith(koreaderSessionTombstones);
|
||||
expect(db.insert).not.toHaveBeenCalled();
|
||||
expect(tombstoneValues).toHaveBeenCalledExactlyOnceWith({
|
||||
userId: 1,
|
||||
bookFileId: 16,
|
||||
deviceKey: 'abcdef12',
|
||||
startTime: 1_791_594_307,
|
||||
endTime: 1_791_595_507,
|
||||
});
|
||||
});
|
||||
|
||||
it('widens a fractional span outward so no event of the session escapes it', async () => {
|
||||
const { repo, tombstoneValues } = makeDeleteHarness({
|
||||
...KOREADER_ROW,
|
||||
startedAt: new Date(1_791_594_307_400),
|
||||
endedAt: new Date(1_791_595_507_200),
|
||||
});
|
||||
|
||||
await repo.deleteSessionByBook(1, 2, 5);
|
||||
|
||||
expect(tombstoneValues).toHaveBeenCalledWith(expect.objectContaining({ startTime: 1_791_594_307, endTime: 1_791_595_508 }));
|
||||
});
|
||||
|
||||
it('keeps the tombstone on the file the session row points at', async () => {
|
||||
const { repo, tombstoneValues } = makeDeleteHarness({ ...KOREADER_ROW, bookFileId: 99 });
|
||||
|
||||
await repo.deleteSessionByBook(1, 2, 5);
|
||||
|
||||
expect(tombstoneValues).toHaveBeenCalledWith(expect.objectContaining({ bookFileId: 99 }));
|
||||
});
|
||||
|
||||
it.each([
|
||||
['a web reader session', { source: 'web', sessionId: '3f1c2f9e-0d6b-4d5e-9a51-2b2c1d8e7f60' }],
|
||||
['a manual session', { source: 'manual', sessionId: null }],
|
||||
['a legacy KOReader estimate', { source: 'koreader', sessionId: `ks-${'a'.repeat(12)}-${'b'.repeat(32)}` }],
|
||||
['a KOReader-shaped id from another source', { source: 'kobo', sessionId: 'kor:abcdef12:16:1791594307' }],
|
||||
['a session whose file is gone', { bookFileId: null }],
|
||||
['a session with no duration span', { endedAt: KOREADER_ROW.startedAt }],
|
||||
])('writes no tombstone for %s', async (_label, overrides) => {
|
||||
const { repo, tx, tombstoneValues } = makeDeleteHarness({ ...KOREADER_ROW, ...overrides });
|
||||
|
||||
await expect(repo.deleteSessionByBook(1, 2, 5)).resolves.toEqual({ found: true });
|
||||
|
||||
expect(tx.insert).not.toHaveBeenCalledWith(koreaderSessionTombstones);
|
||||
expect(tombstoneValues).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('writes no tombstone when the session is not found', async () => {
|
||||
const { repo, tx } = makeDeleteHarness(null);
|
||||
|
||||
await repo.deleteSessionByBook(1, 2, 5);
|
||||
|
||||
expect(tx.insert).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns { found: false } when session not found', async () => {
|
||||
const { repo } = makeDeleteHarness(null);
|
||||
|
||||
|
||||
@@ -18,10 +18,11 @@ import {
|
||||
splitReadingSessionByDay,
|
||||
type ReadingDailyStatsSegment,
|
||||
} from '../../common/utils/reading-daily-stats.utils';
|
||||
import { parseKoreaderSessionId } from '../../common/utils/koreader-session-id.utils';
|
||||
import { resolveTimeZone, toDateKeyInTimeZone } from '../../common/utils/timezone.utils';
|
||||
import { DB } from '../../db';
|
||||
import * as schema from '../../db/schema';
|
||||
import { bookFiles, books, readingSessionSyncCursors, readingSessions, userReadingDailyStats } from '../../db/schema';
|
||||
import { bookFiles, books, koreaderSessionTombstones, readingSessionSyncCursors, readingSessions, userReadingDailyStats } from '../../db/schema';
|
||||
|
||||
type Db = NodePgDatabase<typeof schema>;
|
||||
type Tx = Parameters<Parameters<Db['transaction']>[0]>[0];
|
||||
@@ -637,6 +638,9 @@ export class ReadingSessionRepository {
|
||||
const [row] = await tx
|
||||
.select({
|
||||
id: readingSessions.id,
|
||||
sessionId: readingSessions.sessionId,
|
||||
source: readingSessions.source,
|
||||
bookFileId: readingSessions.bookFileId,
|
||||
startedAt: readingSessions.startedAt,
|
||||
endedAt: readingSessions.endedAt,
|
||||
durationSeconds: readingSessions.durationSeconds,
|
||||
@@ -655,6 +659,15 @@ export class ReadingSessionRepository {
|
||||
|
||||
await tx.delete(readingSessions).where(eq(readingSessions.id, sessionId));
|
||||
|
||||
// A KOReader session is derived from page events that stay stored, so without this the next
|
||||
// event within the session gap, or a plugin uploading its history again, re-derives it.
|
||||
const derived = row.source === 'koreader' ? parseKoreaderSessionId(row.sessionId) : null;
|
||||
const startTime = Math.floor(startedAt.getTime() / 1000);
|
||||
const endTime = Math.ceil(endedAt.getTime() / 1000);
|
||||
if (derived && row.bookFileId != null && endTime > startTime) {
|
||||
await tx.insert(koreaderSessionTombstones).values({ userId, bookFileId: row.bookFileId, deviceKey: derived.deviceKey, startTime, endTime });
|
||||
}
|
||||
|
||||
await this.recomputeDailyStats(tx, userId, libraryId, affectedDays, timeZone);
|
||||
|
||||
return { found: true };
|
||||
|
||||
@@ -4,13 +4,8 @@ import { join } from 'path';
|
||||
import { and, asc, eq, gte, like } from 'drizzle-orm';
|
||||
|
||||
import * as schema from '../src/db/schema';
|
||||
import {
|
||||
KOREADER_SESSION_GAP_SECONDS,
|
||||
buildSessionId,
|
||||
buildSessionIdPrefix,
|
||||
deriveKoreaderSessions,
|
||||
type KoreaderPageEvent,
|
||||
} from '../src/modules/koreader/koreader-stats.util';
|
||||
import { buildSessionId, buildSessionIdPrefix } from '../src/common/utils/koreader-session-id.utils';
|
||||
import { KOREADER_SESSION_GAP_SECONDS, deriveKoreaderSessions, type KoreaderPageEvent } from '../src/modules/koreader/koreader-stats.util';
|
||||
import { createEpubFixture } from './e2e/reader-state-isolation/reader-state-isolation-fixture-builder';
|
||||
import {
|
||||
authHeader,
|
||||
@@ -265,6 +260,126 @@ describe('KOReader page stats derivation (e2e)', { timeout: 180_000 }, () => {
|
||||
expect(await waitForAchievement(session!.userId, 'power_hour')).toBe(true);
|
||||
});
|
||||
|
||||
async function deleteSessionFromLog(sessionRowId: number) {
|
||||
const response = await ctx.app.inject({
|
||||
method: 'DELETE',
|
||||
url: `/api/v1/books/${epub.bookId}/sessions/${sessionRowId}`,
|
||||
headers: authHeader(ctx.adminToken),
|
||||
});
|
||||
expect(response.statusCode).toBe(204);
|
||||
}
|
||||
|
||||
async function sessionsBetween(fromEpoch: number, toEpoch: number) {
|
||||
return (await storedSessions()).filter((row) => row.startedAt.getTime() >= fromEpoch * 1000 && row.startedAt.getTime() < toEpoch * 1000);
|
||||
}
|
||||
|
||||
async function tombstonesFor(fromEpoch: number) {
|
||||
return ctx.db
|
||||
.select({
|
||||
deviceKey: schema.koreaderSessionTombstones.deviceKey,
|
||||
startTime: schema.koreaderSessionTombstones.startTime,
|
||||
endTime: schema.koreaderSessionTombstones.endTime,
|
||||
})
|
||||
.from(schema.koreaderSessionTombstones)
|
||||
.where(and(eq(schema.koreaderSessionTombstones.bookFileId, epub.bookFileId), gte(schema.koreaderSessionTombstones.startTime, fromEpoch)));
|
||||
}
|
||||
|
||||
it('keeps a deleted session deleted when the device reads on within the session gap', async () => {
|
||||
const base = BASE_EPOCH + 15_000_000;
|
||||
const sitting = cluster(base, 20, 1);
|
||||
await uploadPageStats(sitting);
|
||||
const [derived] = await sessionsBetween(base, base + 86_400);
|
||||
expect(derived?.sessionId).toBe(buildSessionId(DEVICE_ID, epub.bookFileId, base));
|
||||
|
||||
await deleteSessionFromLog(derived!.id);
|
||||
expect(await sessionsBetween(base, base + 86_400)).toEqual([]);
|
||||
expect(await tombstonesFor(base)).toEqual([
|
||||
{ deviceKey: DEVICE_ID.slice(0, 8), startTime: base, endTime: sitting.at(-1)!.startTime + sitting.at(-1)!.durationSeconds },
|
||||
]);
|
||||
|
||||
// Ten minutes after the deleted sitting ended: close enough that derivation used to merge the
|
||||
// stored events back in and re-insert the deleted session under its old id.
|
||||
const later = cluster(sitting.at(-1)!.startTime + sitting.at(-1)!.durationSeconds + 600, 5, 21);
|
||||
await uploadPageStats(later);
|
||||
|
||||
const after = await sessionsBetween(base, base + 86_400);
|
||||
expect(comparable(after)).toEqual(expectedSessions(later));
|
||||
expect(after.some((row) => row.sessionId === derived!.sessionId)).toBe(false);
|
||||
});
|
||||
|
||||
it('counts the page that starts on the second a deleted session ended', async () => {
|
||||
const base = BASE_EPOCH + 15_500_000;
|
||||
const sitting = cluster(base, 10, 1);
|
||||
await uploadPageStats(sitting);
|
||||
const [derived] = await sessionsBetween(base, base + 86_400);
|
||||
await deleteSessionFromLog(derived!.id);
|
||||
|
||||
const sessionEnd = sitting.at(-1)!.startTime + sitting.at(-1)!.durationSeconds;
|
||||
const next = cluster(sessionEnd, 5, 11);
|
||||
await uploadPageStats(next);
|
||||
|
||||
expect(comparable(await sessionsBetween(base, base + 86_400))).toEqual(expectedSessions(next));
|
||||
});
|
||||
|
||||
it('does not bring a deleted session back when its events are uploaded again', async () => {
|
||||
const base = BASE_EPOCH + 16_000_000;
|
||||
const sitting = cluster(base, 15, 30);
|
||||
await uploadPageStats(sitting);
|
||||
const [derived] = await sessionsBetween(base, base + 86_400);
|
||||
await deleteSessionFromLog(derived!.id);
|
||||
|
||||
// KOReader's own statistics sync can deliver more pages for the same span from another copy of
|
||||
// the history, and a plugin that lost its state resends everything it has.
|
||||
const sameSpan = sitting.map((event) => ({ ...event, page: event.page + 500 }));
|
||||
const extra = await uploadPageStats(sameSpan);
|
||||
expect(extra.results[0]!.accepted).toBe(sameSpan.length);
|
||||
expect(await sessionsBetween(base, base + 86_400)).toEqual([]);
|
||||
|
||||
await ctx.db
|
||||
.delete(schema.koreaderPageStats)
|
||||
.where(and(eq(schema.koreaderPageStats.bookFileId, epub.bookFileId), gte(schema.koreaderPageStats.startTime, base)));
|
||||
const resent = await uploadPageStats(sitting);
|
||||
expect(resent.results[0]!.accepted).toBe(sitting.length);
|
||||
expect(await sessionsBetween(base, base + 86_400)).toEqual([]);
|
||||
});
|
||||
|
||||
it('builds a bridged session from the surviving events only', async () => {
|
||||
const base = BASE_EPOCH + 17_000_000;
|
||||
const early = cluster(base, 10, 0);
|
||||
const late = cluster(base + 3 * KOREADER_SESSION_GAP_SECONDS, 10, 40);
|
||||
await uploadPageStats([...early, ...late]);
|
||||
const [earlyRow, lateRow] = await sessionsBetween(base, base + 86_400);
|
||||
expect(earlyRow?.sessionId).toBe(buildSessionId(DEVICE_ID, epub.bookFileId, base));
|
||||
|
||||
await deleteSessionFromLog(earlyRow!.id);
|
||||
|
||||
// Would have merged early, bridge and late into one session starting at the early sitting.
|
||||
const bridge = cluster(early.at(-1)!.startTime + 1_760, 22, 10);
|
||||
await uploadPageStats(bridge);
|
||||
|
||||
const after = await sessionsBetween(base, base + 86_400);
|
||||
expect(comparable(after)).toEqual(expectedSessions([...bridge, ...late]));
|
||||
expect(after.some((row) => row.sessionId === earlyRow!.sessionId)).toBe(false);
|
||||
expect(after.some((row) => row.sessionId === lateRow!.sessionId)).toBe(false);
|
||||
});
|
||||
|
||||
it('leaves the neighbouring sessions of a deleted one untouched', async () => {
|
||||
const base = BASE_EPOCH + 18_000_000;
|
||||
const first = cluster(base, 8, 0);
|
||||
const second = cluster(base + 3 * KOREADER_SESSION_GAP_SECONDS, 8, 10);
|
||||
const third = cluster(base + 6 * KOREADER_SESSION_GAP_SECONDS, 8, 20);
|
||||
await uploadPageStats([...first, ...second, ...third]);
|
||||
const rows = await sessionsBetween(base, base + 86_400);
|
||||
expect(rows).toHaveLength(3);
|
||||
|
||||
await deleteSessionFromLog(rows[1]!.id);
|
||||
await uploadPageStats(cluster(third.at(-1)!.startTime + 120, 4, 30));
|
||||
|
||||
const after = await sessionsBetween(base, base + 86_400);
|
||||
expect(after.map((row) => row.sessionId)).toEqual([rows[0]!.sessionId, rows[2]!.sessionId]);
|
||||
expect(after[0]).toEqual(rows[0]);
|
||||
});
|
||||
|
||||
// A file hash is not unique: identical bytes in two places are two book file rows. The server
|
||||
// must refuse to guess, then honor an accessible file identity previously verified by the
|
||||
// plugin so every later upload has one stable target.
|
||||
|
||||
Reference in New Issue
Block a user