diigo-cache: diigo counts pages from 0; migrate stored pages (schema v2)
Comparing a cache with diigo's RSS export: the size-100 walk had missed
exactly the 100 newest bookmarks (everything updated since 2026-09-16).
diigo's page_num is 0-based, and we sent our 1-based page number
unchanged, so "page 1" was really page 2 and the newest page was never
fetched. diigo's total (16744) was right; the "total-overstated"
reading was a misdiagnosis.
- page() asks for page_num = n - 1
- BREAKING for stored state, agreed beforehand, migrated in place:
schema v2 renumbers every stored page +1 (pages and page_members, via
negation so no number collides), once, recorded as kv "schema" = 2;
nothing is refetched, page 1 just becomes a miss
- fill jobs record their numbering; a job saved without it fails ("page
numbering changed …; start the fill again") instead of resuming
- the fake pages from 0 like diigo (7 tests failed until the fix);
tests feed v1-shaped storage and a v1-numbered job to the new code
- the next-page peek guard stays; its comments no longer claim diigo
overstates its total
- the fixed test clock moves to 2099: pinned to "today" its alarms came
due in real time and fired on their own
Generated-by: Claude Opus 5.5
This commit is contained in:
1 parent
e6deb9fda0
commit
cffb1fa9b2
5 files changed
+116
-17
No files matched your search
+57
-11
@@ -45,6 +45,22 @@ create table if not exists page_members (
|
|||||||
);
|
);
|
||||||
`;
|
`;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* V2: renumber stored pages +1. Pages are 1-based here, but diigo's
|
||||||
|
* `page_num` counts from 0, and V1 sent our page number unchanged — so a
|
||||||
|
* V1 "page n" holds what is really page n+1 (and the newest page was never
|
||||||
|
* fetched). Via negation, so no row's new number collides with an old one.
|
||||||
|
*/
|
||||||
|
const SCHEMA_V2_RENUMBER = `
|
||||||
|
update page_members set page_num = -(page_num + 1);
|
||||||
|
update page_members set page_num = -page_num;
|
||||||
|
update pages set page_num = -(page_num + 1);
|
||||||
|
update pages set page_num = -page_num;
|
||||||
|
`;
|
||||||
|
|
||||||
|
/** the schema version a cache's storage is at (kv key "schema") */
|
||||||
|
const SCHEMA_VERSION = 2;
|
||||||
|
|
||||||
/** one cached page, raw: `items` are the dump rows (JSON strings), ready
|
/** one cached page, raw: `items` are the dump rows (JSON strings), ready
|
||||||
* to re-join with newlines */
|
* to re-join with newlines */
|
||||||
export type CachePage = {
|
export type CachePage = {
|
||||||
@@ -86,6 +102,9 @@ export type FillStatus = {
|
|||||||
/** the page size `nextPage` and `pages` count in; absent on jobs
|
/** the page size `nextPage` and `pages` count in; absent on jobs
|
||||||
* saved before this field existed (they counted in 24s) */
|
* saved before this field existed (they counted in 24s) */
|
||||||
pageSize?: number;
|
pageSize?: number;
|
||||||
|
/** the page numbering `nextPage` counts in: 2 since diigo's 0-based
|
||||||
|
* page_num is translated; absent on jobs that sent it unchanged */
|
||||||
|
numbering?: number;
|
||||||
/** pages fetched from diigo by this job (cache hits not counted) */
|
/** pages fetched from diigo by this job (cache hits not counted) */
|
||||||
fetched: number;
|
fetched: number;
|
||||||
startedAt: string;
|
startedAt: string;
|
||||||
@@ -99,6 +118,11 @@ function fillPageSize(status: FillStatus): number {
|
|||||||
return status.pageSize ?? LEGACY_FILL_PAGE_SIZE;
|
return status.pageSize ?? LEGACY_FILL_PAGE_SIZE;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** does a stored fill job count pages the way this code does? */
|
||||||
|
function fillCountsAsNow(status: FillStatus): boolean {
|
||||||
|
return fillPageSize(status) === PAGE_SIZE && status.numbering === SCHEMA_VERSION;
|
||||||
|
}
|
||||||
|
|
||||||
/** release an RPC stub, if it is one (plain objects are left alone) */
|
/** release an RPC stub, if it is one (plain objects are left alone) */
|
||||||
function disposeStub(stub: unknown): void {
|
function disposeStub(stub: unknown): void {
|
||||||
const dispose = (stub as { [Symbol.dispose]?: () => void } | null)?.[Symbol.dispose];
|
const dispose = (stub as { [Symbol.dispose]?: () => void } | null)?.[Symbol.dispose];
|
||||||
@@ -182,7 +206,7 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
|
|||||||
*/
|
*/
|
||||||
async startFill(opts: { pagesPerStep?: number } = {}): Promise<FillStatus> {
|
async startFill(opts: { pagesPerStep?: number } = {}): Promise<FillStatus> {
|
||||||
const current = await this.fillStatus();
|
const current = await this.fillStatus();
|
||||||
if (current?.state === "running" && fillPageSize(current) === PAGE_SIZE) return current;
|
if (current?.state === "running" && fillCountsAsNow(current)) return current;
|
||||||
const step = Math.trunc(opts.pagesPerStep ?? 20);
|
const step = Math.trunc(opts.pagesPerStep ?? 20);
|
||||||
if (!(step >= 1 && step <= 50)) throw new Error("pagesPerStep: expected 1–50");
|
if (!(step >= 1 && step <= 50)) throw new Error("pagesPerStep: expected 1–50");
|
||||||
const now = this.#iso();
|
const now = this.#iso();
|
||||||
@@ -191,6 +215,7 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
|
|||||||
nextPage: 1,
|
nextPage: 1,
|
||||||
pages: this.#knownPages(),
|
pages: this.#knownPages(),
|
||||||
pageSize: PAGE_SIZE,
|
pageSize: PAGE_SIZE,
|
||||||
|
numbering: SCHEMA_VERSION,
|
||||||
fetched: 0,
|
fetched: 0,
|
||||||
startedAt: now,
|
startedAt: now,
|
||||||
updatedAt: now,
|
updatedAt: now,
|
||||||
@@ -225,10 +250,13 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
|
|||||||
async alarm(): Promise<void> {
|
async alarm(): Promise<void> {
|
||||||
const step = (await this.ctx.storage.get<number>("fillStep")) ?? 20;
|
const step = (await this.ctx.storage.get<number>("fillStep")) ?? 20;
|
||||||
let status = await this.fillStatus();
|
let status = await this.fillStatus();
|
||||||
// a job saved under another page size counts its pages differently:
|
// a job saved under another page size or numbering counts its pages
|
||||||
// resuming it would skip or repeat items, so it fails instead
|
// differently: resuming it would skip or repeat items, so it fails
|
||||||
if (status?.state === "running" && fillPageSize(status) !== PAGE_SIZE) {
|
if (status?.state === "running" && !fillCountsAsNow(status)) {
|
||||||
const error = `page size changed (from ${fillPageSize(status)} to ${PAGE_SIZE}); start the fill again`;
|
const error =
|
||||||
|
fillPageSize(status) !== PAGE_SIZE
|
||||||
|
? `page size changed (from ${fillPageSize(status)} to ${PAGE_SIZE}); start the fill again`
|
||||||
|
: "page numbering changed (diigo counts pages from 0); start the fill again";
|
||||||
status = { ...status, state: "failed", error, updatedAt: this.#iso() };
|
status = { ...status, state: "failed", error, updatedAt: this.#iso() };
|
||||||
console.error({ event: "fill", kind: this.kind, state: "failed", error });
|
console.error({ event: "fill", kind: this.kind, state: "failed", error });
|
||||||
await this.ctx.storage.put("fill", status);
|
await this.ctx.storage.put("fill", status);
|
||||||
@@ -274,10 +302,27 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
|
|||||||
if (status.state === "running") await this.ctx.storage.setAlarm(this.#clock.now() + FILL_STEP_DELAY_MS);
|
if (status.state === "running") await this.ctx.storage.setAlarm(this.#clock.now() + FILL_STEP_DELAY_MS);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#schemaReady = false;
|
||||||
|
|
||||||
|
/** create tables, and migrate stored state once per activation */
|
||||||
|
#schema(): void {
|
||||||
|
if (this.#schemaReady) return;
|
||||||
|
this.ctx.storage.sql.exec(SCHEMA_V1);
|
||||||
|
const version = this.ctx.storage.kv.get<number>("schema") ?? 1;
|
||||||
|
if (version < 2) {
|
||||||
|
this.ctx.storage.transactionSync(() => {
|
||||||
|
this.ctx.storage.sql.exec(SCHEMA_V2_RENUMBER);
|
||||||
|
this.ctx.storage.kv.put("schema", SCHEMA_VERSION);
|
||||||
|
});
|
||||||
|
console.log({ event: "schema", kind: this.kind, from: version, to: SCHEMA_VERSION });
|
||||||
|
}
|
||||||
|
this.#schemaReady = true;
|
||||||
|
}
|
||||||
|
|
||||||
/** diigo's page count, if any cached page recorded `total` (pages past
|
/** diigo's page count, if any cached page recorded `total` (pages past
|
||||||
* the end record 0, so they don't count) */
|
* the end record 0, so they don't count) */
|
||||||
#knownPages(): number | null {
|
#knownPages(): number | null {
|
||||||
this.ctx.storage.sql.exec(SCHEMA_V1);
|
this.#schema();
|
||||||
const row = this.ctx.storage.sql
|
const row = this.ctx.storage.sql
|
||||||
.exec<{ total: number | null }>("select total from pages where total > 0 order by etched_at desc limit 1")
|
.exec<{ total: number | null }>("select total from pages where total > 0 order by etched_at desc limit 1")
|
||||||
.toArray();
|
.toArray();
|
||||||
@@ -293,7 +338,7 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
|
|||||||
async page(pageNum: number, count: number): Promise<CachePage> {
|
async page(pageNum: number, count: number): Promise<CachePage> {
|
||||||
const init = await this.wiring();
|
const init = await this.wiring();
|
||||||
if (!init) throw new Error("not wired: this cache id was never set up");
|
if (!init) throw new Error("not wired: this cache id was never set up");
|
||||||
this.ctx.storage.sql.exec(SCHEMA_V1);
|
this.#schema();
|
||||||
|
|
||||||
const hit = await this.#pageFromStorage(pageNum, count);
|
const hit = await this.#pageFromStorage(pageNum, count);
|
||||||
if (hit) {
|
if (hit) {
|
||||||
@@ -322,9 +367,9 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
|
|||||||
// page is in hand: undisposed stubs keep this DO from being evicted
|
// page is in hand: undisposed stubs keep this DO from being evicted
|
||||||
const data = await this.#fetchPage(init.webappId, pageNum, count);
|
const data = await this.#fetchPage(init.webappId, pageNum, count);
|
||||||
// diigo may cap the page size below what we asked: a short page that
|
// diigo may cap the page size below what we asked: a short page that
|
||||||
// isn't the last would silently skip items. But diigo's `total` can
|
// isn't the last would silently skip items. If `total` ever
|
||||||
// also overstate what it returns, making the true last page look
|
// overstated what diigo returns, the true last page would look short
|
||||||
// short. The next page tells them apart: empty means the data ends
|
// too. The next page tells them apart: empty means the data ends
|
||||||
// here; anything else means items would be skipped — refuse loudly.
|
// here; anything else means items would be skipped — refuse loudly.
|
||||||
const lastPage = Math.max(1, Math.ceil(data.total / count));
|
const lastPage = Math.max(1, Math.ceil(data.total / count));
|
||||||
if (data.items.length < count && pageNum < lastPage) {
|
if (data.items.length < count && pageNum < lastPage) {
|
||||||
@@ -350,7 +395,8 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
|
|||||||
async #fetchPage(webappId: string, pageNum: number, count: number): Promise<{ total: number; items: Item[] }> {
|
async #fetchPage(webappId: string, pageNum: number, count: number): Promise<{ total: number; items: Item[] }> {
|
||||||
const app = await this.env.WEBAPP.at(webappId);
|
const app = await this.env.WEBAPP.at(webappId);
|
||||||
const joined = app.pathjoin("load_user_items");
|
const joined = app.pathjoin("load_user_items");
|
||||||
const queried = joined.query({ sort: "updated", count, page_num: pageNum });
|
// diigo's page_num counts from 0; ours from 1
|
||||||
|
const queried = joined.query({ sort: "updated", count, page_num: pageNum - 1 });
|
||||||
try {
|
try {
|
||||||
return JSON.parse(await queried.get()) as { total: number; items: Item[] };
|
return JSON.parse(await queried.get()) as { total: number; items: Item[] };
|
||||||
} finally {
|
} finally {
|
||||||
|
|||||||
@@ -88,7 +88,7 @@ describe("page size", () => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it("a short page whose next page is empty is the real end: diigo's total overstated, noted not refused", async () => {
|
it("a short page whose next page is empty is the real end: an overstated total is noted, not refused", async () => {
|
||||||
const spy = vi.spyOn(console, "log");
|
const spy = vi.spyOn(console, "log");
|
||||||
try {
|
try {
|
||||||
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
|
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
|
||||||
@@ -122,6 +122,43 @@ describe("pacing", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("page numbering (schema v2)", () => {
|
||||||
|
it("asks diigo for page_num n-1: page 1 is the newest page", async () => {
|
||||||
|
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
|
||||||
|
await stub.setup({ webappId: WEBAPP_ID });
|
||||||
|
const first = (await stub.page(1, PAGE_SIZE)).items.map((j) => (JSON.parse(j) as { title: string }).title);
|
||||||
|
expect(first[0]).toBe("bookmark 1");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("migrates v1 storage: a stored 'page n' was really page n+1, so it is renumbered, not refetched", async () => {
|
||||||
|
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
|
||||||
|
await stub.setup({ webappId: WEBAPP_ID });
|
||||||
|
// the shape v1 left: tables, a page 1 holding what diigo's
|
||||||
|
// page_num=1 returned (really page 2), and no schema marker
|
||||||
|
await runInDurableObject(stub, (_obj, state) => {
|
||||||
|
const sql = state.storage.sql;
|
||||||
|
sql.exec(`create table items (item_id text primary key, url text not null, title text not null,
|
||||||
|
private integer not null, json text not null, updated text not null)`);
|
||||||
|
sql.exec("create table pages (page_num integer primary key, count integer not null, total integer, etched_at text not null)");
|
||||||
|
sql.exec("create table page_members (page_num integer not null, pos integer not null, item_id text not null, primary key (page_num, pos))");
|
||||||
|
sql.exec("insert into items values ('a', 'https://a.example/', 'old page one, first', 0, '{\"title\":\"A\"}', '1')");
|
||||||
|
sql.exec("insert into items values ('b', 'https://b.example/', 'old page one, second', 0, '{\"title\":\"B\"}', '1')");
|
||||||
|
sql.exec("insert into pages values (1, 100, 250, '2026-10-07T00:00:00.000Z')");
|
||||||
|
sql.exec("insert into page_members values (1, 1, 'a'), (1, 2, 'b')");
|
||||||
|
});
|
||||||
|
const before = await testEnv.WEBAPP.calls();
|
||||||
|
const two = await stub.page(2, PAGE_SIZE);
|
||||||
|
expect(two.cached).toBe(true);
|
||||||
|
expect(two.items.map((j) => (JSON.parse(j) as { title: string }).title)).toEqual(["A", "B"]);
|
||||||
|
expect(await testEnv.WEBAPP.calls()).toBe(before);
|
||||||
|
// page 1 was never fetched under v1: now it is a miss
|
||||||
|
expect((await stub.page(1, PAGE_SIZE)).cached).toBe(false);
|
||||||
|
await runInDurableObject(stub, (_obj, state) => {
|
||||||
|
expect(state.storage.kv.get("schema")).toBe(2);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("eviction", () => {
|
describe("eviction", () => {
|
||||||
it("a cache miss leaves no undisposed RPC stubs to block eviction", async () => {
|
it("a cache miss leaves no undisposed RPC stubs to block eviction", async () => {
|
||||||
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
|
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
|
||||||
|
|||||||
@@ -14,7 +14,8 @@ const TOTAL = 250;
|
|||||||
const CAPPED_WEBAPP = "fake-capped-webapp";
|
const CAPPED_WEBAPP = "fake-capped-webapp";
|
||||||
const CAP = 64;
|
const CAP = 64;
|
||||||
/** the plain account, but its `total` claims 60 more items than it has
|
/** the plain account, but its `total` claims 60 more items than it has
|
||||||
* (as diigo's does, observed 2026-10-07) */
|
* (hypothetical: what looked like this on 2026-10-07 was the 0-based
|
||||||
|
* page_num, but the guard stays) */
|
||||||
const OVERSTATED_WEBAPP = "fake-overstated-webapp";
|
const OVERSTATED_WEBAPP = "fake-overstated-webapp";
|
||||||
const OVERSTATEMENT = 60;
|
const OVERSTATEMENT = 60;
|
||||||
|
|
||||||
@@ -93,7 +94,9 @@ class FakeGet extends RpcTarget {
|
|||||||
const total = this.#dated === "deep" ? DEEP_TOTAL : dated ? DATED_TOTAL : TOTAL;
|
const total = this.#dated === "deep" ? DEEP_TOTAL : dated ? DATED_TOTAL : TOTAL;
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
const items = [];
|
const items = [];
|
||||||
for (let n = (page - 1) * count + 1; n <= Math.min(page * count, total); n += 1) {
|
// diigo's page_num counts from 0 (observed 2026-10-07: page_num=1
|
||||||
|
// skipped the newest page)
|
||||||
|
for (let n = page * count + 1; n <= Math.min((page + 1) * count, total); n += 1) {
|
||||||
items.push(dated ? datedItem(n, now, this.#dated === "deep" ? "deep_dee" : "dated_dan") : item(n));
|
items.push(dated ? datedItem(n, now, this.#dated === "deep" ? "deep_dee" : "dated_dan") : item(n));
|
||||||
}
|
}
|
||||||
// past the end, diigo answers no items and a total of 0 (observed
|
// past the end, diigo answers no items and a total of 0 (observed
|
||||||
|
|||||||
@@ -131,7 +131,7 @@ describe("fill job (goal 1e)", () => {
|
|||||||
expect(status?.error).toMatch(/page size not honored/);
|
expect(status?.error).toMatch(/page size not honored/);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("fills to the real end when diigo's total overstates its items", async () => {
|
it("fills to the real end if a total overstates the items", async () => {
|
||||||
const stub = await wired("fake-overstated-webapp");
|
const stub = await wired("fake-overstated-webapp");
|
||||||
await stub.startFill();
|
await stub.startFill();
|
||||||
await drain(stub);
|
await drain(stub);
|
||||||
@@ -143,6 +143,18 @@ describe("fill job (goal 1e)", () => {
|
|||||||
expect((await stub.page(3, PAGE_SIZE)).items).toHaveLength(50);
|
expect((await stub.page(3, PAGE_SIZE)).items).toHaveLength(50);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("a job saved under the old page numbering fails instead of resuming", async () => {
|
||||||
|
const stub = await wired();
|
||||||
|
await runInDurableObject(stub, async (_obj, state) => {
|
||||||
|
const at = new Date().toISOString();
|
||||||
|
await state.storage.put("fill", { state: "running", nextPage: 40, pages: 168, fetched: 39, pageSize: 100, startedAt: at, updatedAt: at });
|
||||||
|
await state.storage.setAlarm(Date.now() + 60_000);
|
||||||
|
});
|
||||||
|
await drain(stub);
|
||||||
|
expect((await stub.fillStatus())?.error).toMatch(/page numbering changed/);
|
||||||
|
expect(await stub.startFill()).toMatchObject({ state: "running", nextPage: 1, numbering: 2 });
|
||||||
|
});
|
||||||
|
|
||||||
it("a job saved under another page size fails instead of resuming; a new start replaces it", async () => {
|
it("a job saved under another page size fails instead of resuming; a new start replaces it", async () => {
|
||||||
const stub = await wired();
|
const stub = await wired();
|
||||||
// the shape a 24-per-page job left in storage (no pageSize field)
|
// the shape a 24-per-page job left in storage (no pageSize field)
|
||||||
|
|||||||
@@ -9,8 +9,9 @@ import type { CacheEnv } from "../src/cache.js";
|
|||||||
export * from "../src/index.js";
|
export * from "../src/index.js";
|
||||||
export { default } from "../src/index.js";
|
export { default } from "../src/index.js";
|
||||||
|
|
||||||
/** 2026-10-07T12:00:00Z */
|
/** 2099-01-01T00:00:00Z: in the future, so an alarm set by this clock is
|
||||||
export const FIXED_NOW = Date.UTC(2026, 9, 7, 12, 0, 0);
|
* never due in real time — tests run it with runDurableObjectAlarm */
|
||||||
|
export const FIXED_NOW = Date.UTC(2099, 0, 1, 0, 0, 0);
|
||||||
|
|
||||||
/** a fixed clock that records each sleep it is asked for */
|
/** a fixed clock that records each sleep it is asked for */
|
||||||
export class FixedClockCache extends DiigoCacheLogic {
|
export class FixedClockCache extends DiigoCacheLogic {
|
||||||
|
|||||||
Reference in new issue
Block a user