commit 34c6d9caaf933f4462b6472f133a90384eb671a5 truex <truex@equwal.com> 2026-09-26 23:18:11 -0700 Let a book that the user opens win over the restore of the last book restoreLastBook did not know about the opens of the user: - The last book L is large, and the user picks a small PDF B while L loads. B shows first, and its copy replaces L. Then the open of L gives false, and the restore read that as "the copy cannot be read": clearLastBook deleted the copy, which was the copy of B now. - On the slow first start (the database upgrade), the start card shows after 2 s, and the user picks B. The database opens while B loads. state.pdf is still null, so the restore opened L as a later attempt, and L won over the pick of the user. Opens now counts the opens of the user (byUser). The restore notes the count when it starts, and userOpened tells when the user started an open after that. Then the restore does not open L, and does not forget the copy. clearLastBook(key) also deletes the copy only while it is of the book `key`, in one transaction.
README.md | 4 +- src/book-open.ts | 25 +++++++++--- src/main.ts | 32 ++++++++++------ src/token-cache.ts | 18 +++++++-- test/book-open.test.ts | 99 +++++++++++++++++++++++++++++++++++++++--------- test/token-cache.test.ts | 20 ++++++++-- 6 files changed, 157 insertions(+), 41 deletions(-)
diff --git a/README.md b/README.md index 34f6cc8..67905fc 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,9 @@ its page, with its subtitles, without a picker. Android stops the reader while you are in the dictionary or in the player app, and the reader comes back where you were. When the copy cannot be read, for example after the app storage was cleared, the app forgets it and shows -the start card. +the start card. A book that you open while the last book loads, or while +the start waits for the database, wins: the app does not open the last +book over it, and it keeps the copy of your book. Subtitles belong to a book. Another book opens with its own subtitles, or with none. Subtitles that you load while no book is open are for the diff --git a/src/book-open.ts b/src/book-open.ts index ad0249c..fab6fa7 100644 --- a/src/book-open.ts +++ b/src/book-open.ts @@ -1,5 +1,5 @@ -// The open of a book: the open attempts, and the owner of subtitles that -// load during an open. Pure: no DOM. +// The open of a book: the open attempts, the owner of subtitles that load +// during an open, and the restore of the last book. Pure: no DOM. /** * The open attempts of books. Each attempt gets the next number: 1, 2, 3 ... @@ -11,17 +11,20 @@ export interface Opens { tried: number; /** The attempt of the book that shows, or 0. */ shown: number; + /** The count of attempts that the user started: a file of the file picker. */ + byUser: number; /** The attempts that still load. */ loading: Set<number>; } export function createOpens(): Opens { - return { tried: 0, shown: 0, loading: new Set() }; + return { tried: 0, shown: 0, byUser: 0, loading: new Set() }; } -/** Starts an attempt, and gives its number. */ -export function beginOpen(o: Opens): number { +/** Starts an attempt, and gives its number. `byUser` is true for a file that the user picked. */ +export function beginOpen(o: Opens, byUser: boolean): number { const attempt = ++o.tried; + if (byUser) o.byUser++; o.loading.add(attempt); return attempt; } @@ -55,3 +58,15 @@ export function opening(o: Opens): boolean { export function subtitlesOwner(o: Opens, book: string | null): string | null { return opening(o) ? null : book; } + +/** + * True when the user started an open after `since`, the value of o.byUser + * when the restore of the last book started. The restore waits for the + * database, and the first open after an upgrade can take many seconds. A + * book that the user picks meanwhile wins: the restore does not open the + * last book over it, also while it still loads, and does not forget the copy + * of the last book, because the copy can be of the user's book now. + */ +export function userOpened(o: Opens, since: number): boolean { + return o.byUser !== since; +} diff --git a/src/main.ts b/src/main.ts index 51f56cd..c74f97a 100644 --- a/src/main.ts +++ b/src/main.ts @@ -9,7 +9,7 @@ import { App } from '@capacitor/app'; import { createAligner, shiftSpans, type Aligner, type OcrToken, type TokenSpan } from './align'; import { ankiCard, cardSource } from './anki-card'; import { backStep, keepScreenOn } from './app-state'; -import { beginOpen, createOpens, endOpen, subtitlesOwner } from './book-open'; +import { beginOpen, createOpens, endOpen, subtitlesOwner, userOpened } from './book-open'; import { resultName, srtName, subreadConcerns, subtitlesOnOpen } from './book-subtitles'; import { bookText } from './book-text'; import { LocalAudioClock, OverlayClock, type ClockSource, type ClockState } from './clock-source'; @@ -313,17 +313,19 @@ const opens = createOpens(); * file loads: a file that is not a PDF, or is damaged, changes nothing. * * `meta` is what the reader keeps for the book, when the caller has it: - * the book opens at its page. Else the meta data comes from IndexedDB - * after the open. The page shows and the reading starts at once, and the - * meta data applies when it comes: an upgrade of the database can hold - * IndexedDB for many seconds. Gives true when the new book is open. + * the book opens at its page. Only the restore of the last book gives it, + * so an open without it is a file that the user picked. Else the meta data + * comes from IndexedDB after the open. The page shows and the reading + * starts at once, and the meta data applies when it comes: an upgrade of + * the database can hold IndexedDB for many seconds. Gives true when the new + * book is open. */ async function openBook(file: File, meta?: BookMeta): Promise<boolean> { if (file.size === 0) { say(EMPTY_FILE_TEXT); return false; } - const attempt = beginOpen(opens); + const attempt = beginOpen(opens, meta === undefined); let bytes: ArrayBuffer; try { bytes = await file.arrayBuffer(); @@ -485,20 +487,26 @@ function showStart(): void { * Opens the last book at its page, with its subtitles. Android stops the * reader while the user is in the dictionary or in the player app, and a * reload forgets the file that the user opened. + * + * A book that the user opens while the restore waits for the database + * wins, also while it still loads (userOpened). */ async function restoreLastBook(): Promise<void> { + const since = opens.byUser; const file = await getLastBook(); - // The user opened a book while the database was busy. - if (state.pdf) return; - if (!file) { + if (!file || userOpened(opens, since)) { showStart(); return; } const meta = await getMeta(bookKey(file)); - if (state.pdf) return; + if (userOpened(opens, since)) { + showStart(); + return; + } if (await openBook(file, meta)) return; - // The copy cannot be read, or it is no PDF now: forget it. - await clearLastBook(); + // The copy cannot be read, or it is no PDF now: forget it. After an open + // by the user, the copy can be of the user's book, so it stays. + if (!userOpened(opens, since)) await clearLastBook(bookKey(file)); showStart(); } diff --git a/src/token-cache.ts b/src/token-cache.ts index f34f38f..659201b 100644 --- a/src/token-cache.ts +++ b/src/token-cache.ts @@ -198,9 +198,21 @@ export function getLastBook(): Promise<File | undefined> { }); } -/** Removes the copy of the last book, for example when its data cannot be read. */ -export async function clearLastBook(): Promise<void> { - await tryDb((d) => d.delete('books', LAST)); +/** + * Removes the copy of the last book, for example when its data cannot be + * read, but only while the copy is of the book `key` (see bookKey). The copy + * of another book stays: the user can open a book while the restore of the + * last book runs, and the copy of that book replaces the last book. + */ +export async function clearLastBook(key: string): Promise<void> { + await tryDb((d) => { + // One transaction reads and deletes, so a new copy cannot come between. + const tx = d.transaction('books', 'readwrite'); + const remove = async (): Promise<void> => { + if ((await tx.store.get(LAST))?.key === key) await tx.store.delete(LAST); + }; + return Promise.all([remove(), tx.done]); + }); } /** diff --git a/test/book-open.test.ts b/test/book-open.test.ts index 6241e83..5d9e51b 100644 --- a/test/book-open.test.ts +++ b/test/book-open.test.ts @@ -6,14 +6,18 @@ import { endOpen, opening, subtitlesOwner, + userOpened, type Opens, } from '../src/book-open'; /** One step of a script of opens: start the next attempt, or end one that loads. */ interface Step { begin: boolean; - /** The attempt that ends, when `begin` is false. */ + /** The attempt that starts or ends. */ attempt: number; + /** For a start: the user picked the file. */ + byUser: boolean; + /** For an end: the file loaded as a PDF. */ loaded: boolean; } @@ -24,29 +28,39 @@ interface Step { const scripts: fc.Arbitrary<Step[]> = fc .record({ count: fc.integer({ min: 1, max: 6 }), + byUser: fc.array(fc.boolean(), { minLength: 6, maxLength: 6 }), loaded: fc.array(fc.boolean(), { minLength: 6, maxLength: 6 }), picks: fc.array(fc.nat(), { minLength: 12, maxLength: 12 }), }) - .map(({ count, loaded, picks }) => { + .map(({ count, byUser, loaded, picks }) => { const steps: Step[] = []; const loading: number[] = []; let begun = 0; + const end = (attempt: number): Step => ({ + begin: false, + attempt, + byUser: false, + loaded: loaded[attempt - 1]!, + }); for (const pick of picks) { if (begun < count && (loading.length === 0 || pick % 2 === 0)) { loading.push(++begun); - steps.push({ begin: true, attempt: begun, loaded: false }); + steps.push({ begin: true, attempt: begun, byUser: byUser[begun - 1]!, loaded: false }); } else if (loading.length > 0) { - const [attempt] = loading.splice(pick % loading.length, 1); - steps.push({ begin: false, attempt: attempt!, loaded: loaded[attempt! - 1]! }); + steps.push(end(loading.splice(pick % loading.length, 1)[0]!)); } } // The attempts that still load end in their order. - for (const attempt of loading) { - steps.push({ begin: false, attempt, loaded: loaded[attempt - 1]! }); - } + for (const attempt of loading) steps.push(end(attempt)); return steps; }); +/** Runs one step of a script. */ +function run(o: Opens, step: Step): void { + if (step.begin) beginOpen(o, step.byUser); + else endOpen(o, step.attempt, step.loaded); +} + describe('open attempts', () => { it('shows the book of the last attempt that loaded, whatever the order of the loads', () => { fc.assert( @@ -56,7 +70,7 @@ describe('open attempts', () => { for (const step of steps) { const before = o.shown; if (step.begin) { - expect(beginOpen(o)).toBe(step.attempt); + expect(beginOpen(o, step.byUser)).toBe(step.attempt); } else { const shows = endOpen(o, step.attempt, step.loaded); // An attempt shows when it loaded and no later attempt shows. @@ -78,7 +92,7 @@ describe('open attempts', () => { /** Opens where book P shows, and the attempt of P. */ function withP(): { o: Opens; p: number } { const o = createOpens(); - const p = beginOpen(o); + const p = beginOpen(o, true); endOpen(o, p, true); return { o, p }; } @@ -90,7 +104,7 @@ describe('subtitlesOwner', () => { // over the subtitles of P, and the open of A then cleared it. const { o } = withP(); expect(subtitlesOwner(o, 'P')).toBe('P'); - const a = beginOpen(o); + const a = beginOpen(o, true); expect(subtitlesOwner(o, 'P')).toBeNull(); expect(endOpen(o, a, true)).toBe(true); expect(subtitlesOwner(o, 'A')).toBe('A'); @@ -98,7 +112,7 @@ describe('subtitlesOwner', () => { it('gives them to the book that stays open when the open fails', () => { const { o } = withP(); - const a = beginOpen(o); + const a = beginOpen(o, true); expect(subtitlesOwner(o, 'P')).toBeNull(); expect(endOpen(o, a, false)).toBe(false); expect(subtitlesOwner(o, 'P')).toBe('P'); @@ -107,8 +121,8 @@ describe('subtitlesOwner', () => { it('does not wait for an older open that cannot show', () => { // The user picks a large A, then a small B. B shows while A loads. const o = createOpens(); - const a = beginOpen(o); - const b = beginOpen(o); + const a = beginOpen(o, true); + const b = beginOpen(o, true); expect(endOpen(o, b, true)).toBe(true); expect(subtitlesOwner(o, 'B')).toBe('B'); expect(endOpen(o, a, true)).toBe(false); @@ -117,7 +131,7 @@ describe('subtitlesOwner', () => { it('gives no book while no book is open', () => { const o = createOpens(); expect(subtitlesOwner(o, null)).toBeNull(); - beginOpen(o); + beginOpen(o, true); expect(subtitlesOwner(o, null)).toBeNull(); }); @@ -126,11 +140,62 @@ describe('subtitlesOwner', () => { fc.property(scripts, (steps) => { const o = createOpens(); for (const step of steps) { - if (step.begin) beginOpen(o); - else endOpen(o, step.attempt, step.loaded); + run(o, step); expect(subtitlesOwner(o, 'book')).toBe(opening(o) ? null : 'book'); } }), ); }); }); + +describe('userOpened', () => { + it('keeps the copy of a book that the user opened while the last book loaded', () => { + // The finding: the restore opens a large last book L. The user picks a + // small B, B shows first, and the copy of B replaces L. The open of L + // then gives false, and the restore deleted the copy of B. + const o = createOpens(); + const since = o.byUser; + const l = beginOpen(o, false); + const b = beginOpen(o, true); + expect(endOpen(o, b, true)).toBe(true); + expect(endOpen(o, l, true)).toBe(false); + expect(userOpened(o, since)).toBe(true); + }); + + it('does not open the last book over a book that the user picked while the database opened', () => { + // The finding: the database upgrade holds the restore, and the start + // card shows. The user picks B. When the database opens, B still loads + // and no book shows, so the restore opened the last book as a later + // attempt, and it won over B. + const o = createOpens(); + const since = o.byUser; + beginOpen(o, true); + expect(o.shown).toBe(0); + expect(userOpened(o, since)).toBe(true); + }); + + it('lets the restore open and forget the last book when the user opened nothing', () => { + const o = createOpens(); + const since = o.byUser; + const l = beginOpen(o, false); + expect(userOpened(o, since)).toBe(false); + expect(endOpen(o, l, false)).toBe(false); + expect(userOpened(o, since)).toBe(false); + }); + + it('is true exactly when the user started an open after the restore started', () => { + const input = scripts.chain((steps) => + fc.tuple(fc.constant(steps), fc.nat({ max: steps.length })), + ); + fc.assert( + fc.property(input, ([steps, start]) => { + const o = createOpens(); + steps.slice(0, start).forEach((step) => run(o, step)); + const since = o.byUser; + steps.slice(start).forEach((step) => run(o, step)); + const later = steps.slice(start).some((step) => step.begin && step.byUser); + expect(userOpened(o, since)).toBe(later); + }), + ); + }); +}); diff --git a/test/token-cache.test.ts b/test/token-cache.test.ts index 3f8e61a..5e32219 100644 --- a/test/token-cache.test.ts +++ b/test/token-cache.test.ts @@ -143,12 +143,26 @@ describe('token cache', () => { await cache.putLastBook(neko); await cache.putMeta(cache.bookKey(neko), { page: 2 }); await cache.putPage(cache.pageKey(neko, 0, 'jpn', 'ocr'), entry); - await cache.clearLastBook(); + await cache.clearLastBook(cache.bookKey(neko)); expect(await cache.getLastBook()).toBeUndefined(); expect(await cache.getMeta(cache.bookKey(neko))).toEqual({ page: 2 }); expect(await cache.getPage(cache.pageKey(neko, 0, 'jpn', 'ocr'))).toEqual(entry); // No last book: nothing to forget. - await expect(cache.clearLastBook()).resolves.toBeUndefined(); + await expect(cache.clearLastBook(cache.bookKey(neko))).resolves.toBeUndefined(); + }); + + it('forgets the last book only while the copy is of that book', async () => { + // The finding: the restore of the last book failed after the user had + // opened another book, and it deleted the copy of the user's book. + const cache = await freshCache(); + const last = new File(['%PDF-1.7 猫'], 'neko.pdf', { type: 'application/pdf' }); + const picked = new File(['%PDF-1.7 犬と猫'], 'inu.pdf', { type: 'application/pdf' }); + await cache.putLastBook(last); + await cache.putLastBook(picked); + await cache.clearLastBook(cache.bookKey(last)); + expect((await cache.getLastBook())?.name).toBe('inu.pdf'); + await cache.clearLastBook(cache.bookKey(picked)); + expect(await cache.getLastBook()).toBeUndefined(); }); it('tells when the database is open, after the upgrade', async () => { @@ -229,7 +243,7 @@ describe('token cache', () => { await expect(cache.putMeta('b', { page: 1 })).resolves.toBeUndefined(); await expect(cache.putLastBook(new File(['x'], 'b.pdf'))).resolves.toBe(false); await expect(cache.getLastBook()).resolves.toBeUndefined(); - await expect(cache.clearLastBook()).resolves.toBeUndefined(); + await expect(cache.clearLastBook('b.pdf|1')).resolves.toBeUndefined(); await expect(cache.dbReady()).resolves.toBeUndefined(); expect(warn).toHaveBeenCalledTimes(1); });