diff --git a/diigo-cache/src/cache.ts b/diigo-cache/src/cache.ts index 87693e3..a47dd4b 100644 --- a/diigo-cache/src/cache.ts +++ b/diigo-cache/src/cache.ts @@ -303,22 +303,23 @@ abstract class PagedCache extends DurableObject { // RPC stubs (the app and each pipelined step) are disposed once the // page is in hand: undisposed stubs keep this DO from being evicted - const app = await this.env.WEBAPP.at(init.webappId); - const joined = app.pathjoin("load_user_items"); - const queried = joined.query({ sort: "updated", count, page_num: pageNum }); - let data: { total: number; items: Item[] }; - try { - data = JSON.parse(await queried.get()) as { total: number; items: Item[] }; - } finally { - for (const stub of [queried, joined, app]) disposeStub(stub); - } + const data = await this.#fetchPage(init.webappId, pageNum, count); // diigo may cap the page size below what we asked: a short page that - // isn't the last would silently skip items, so refuse it loudly + // isn't the last would silently skip items. But diigo's `total` can + // also overstate what it returns, making the true last page look + // short. The next page tells them apart: empty means the data ends + // here; anything else means items would be skipped — refuse loudly. const lastPage = Math.max(1, Math.ceil(data.total / count)); if (data.items.length < count && pageNum < lastPage) { - const error = `diigo returned ${data.items.length} items for page ${pageNum} of ${lastPage} (asked for ${count}): page size not honored?`; - console.error({ event: "cache", kind: this.kind, page: pageNum, outcome: "short-page", fetched: data.items.length, asked: count }); - throw new Error(error); + await this.#clock.sleep(MS_PER_CALL); + const next = await this.#fetchPage(init.webappId, pageNum + 1, count); + if (next.items.length > 0) { + const error = `diigo returned ${data.items.length} items for page ${pageNum} of ${lastPage} (asked for ${count}), then ${next.items.length} for page ${pageNum + 1}: page size not honored?`; + console.error({ event: "cache", kind: this.kind, page: pageNum, outcome: "short-page", fetched: data.items.length, asked: count, nextFetched: next.items.length }); + throw new Error(error); + } + const actual = (pageNum - 1) * count + data.items.length; + console.log({ event: "cache", kind: this.kind, page: pageNum, outcome: "total-overstated", total: data.total, actual }); } const kept = this.keep(toRows(data.items, console)); // fetched vs kept: the public cache's drop of private rows shows here @@ -326,6 +327,20 @@ abstract class PagedCache extends DurableObject { return this.#etch(pageNum, count, data.total, kept); } + /** one diigo page, raw, through the webapp instance; the RPC stubs (the + * app and each pipelined step) are disposed once the page is in hand — + * undisposed stubs keep this DO from being evicted */ + async #fetchPage(webappId: string, pageNum: number, count: number): Promise<{ total: number; items: Item[] }> { + const app = await this.env.WEBAPP.at(webappId); + const joined = app.pathjoin("load_user_items"); + const queried = joined.query({ sort: "updated", count, page_num: pageNum }); + try { + return JSON.parse(await queried.get()) as { total: number; items: Item[] }; + } finally { + for (const stub of [queried, joined, app]) disposeStub(stub); + } + } + /** etch a page (one transaction): upsert items, mark the page, keep * the fetch-time order; returns the cache-page shape */ #etch(pageNum: number, count: number, total: number, rows: ItemRow[]): CachePage { diff --git a/diigo-cache/tests/caches.test.ts b/diigo-cache/tests/caches.test.ts index a2dd670..4559ceb 100644 --- a/diigo-cache/tests/caches.test.ts +++ b/diigo-cache/tests/caches.test.ts @@ -78,7 +78,7 @@ describe("page size", () => { // settle the RPC promise ourselves: expect().rejects on a DO stub's // promise leaves the rejection unhandled const failure = () => stub.page(1, PAGE_SIZE).then(() => null, (e: unknown) => String(e)); - expect(await failure()).toMatch(/returned 64 items for page 1 of 3 \(asked for 100\)/); + expect(await failure()).toMatch(/returned 64 items for page 1 of 3 \(asked for 100\), then 64 for page 2/); expect(spy.mock.calls.map((c) => c[0]).filter((e) => e?.outcome === "short-page")).toHaveLength(1); // nothing was stored: asking again refuses again expect(await failure()).toMatch(/page size not honored/); @@ -87,6 +87,21 @@ describe("page size", () => { } }); + it("a short page whose next page is empty is the real end: diigo's total overstated, noted not refused", async () => { + const spy = vi.spyOn(console, "log"); + try { + const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId()); + await stub.setup({ webappId: "fake-overstated-webapp" }); + // 250 items claimed as 310: page 3 holds the last 50, "of 4" + const page = await stub.page(3, PAGE_SIZE); + expect(page.items).toHaveLength(50); + const notes = spy.mock.calls.map((c) => c[0]).filter((e) => e?.outcome === "total-overstated"); + expect(notes).toEqual([expect.objectContaining({ page: 3, total: 310, actual: 250 })]); + } finally { + spy.mockRestore(); + } + }); + it("the last page may be short", async () => { const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId()); await stub.setup({ webappId: WEBAPP_ID }); diff --git a/diigo-cache/tests/fake-webapp.js b/diigo-cache/tests/fake-webapp.js index 00cd70f..ef4eb74 100644 --- a/diigo-cache/tests/fake-webapp.js +++ b/diigo-cache/tests/fake-webapp.js @@ -13,6 +13,10 @@ const TOTAL = 250; /** the plain account, capped below PAGE_SIZE (as diigo caps at 100) */ const CAPPED_WEBAPP = "fake-capped-webapp"; const CAP = 64; +/** the plain account, but its `total` claims 60 more items than it has + * (as diigo's does, observed 2026-10-07) */ +const OVERSTATED_WEBAPP = "fake-overstated-webapp"; +const OVERSTATEMENT = 60; /** a second account, for date filters (goal 1d): 60 bookmarks, item n * updated n-1 days + 1 hour before the call, in diigo's updated-desc @@ -66,7 +70,7 @@ let calls = 0; class FakeGet extends RpcTarget { #path; #params; - /** null (the plain account), "capped", "dated", or "deep" */ + /** null (the plain account), "capped", "overstated", "dated", or "deep" */ #dated; constructor(path, params, dated) { super(); @@ -92,13 +96,19 @@ class FakeGet extends RpcTarget { for (let n = (page - 1) * count + 1; n <= Math.min(page * count, total); n += 1) { items.push(dated ? datedItem(n, now, this.#dated === "deep" ? "deep_dee" : "dated_dan") : item(n)); } - return JSON.stringify({ total, items }); + return JSON.stringify({ total: this.#dated === "overstated" ? total + OVERSTATEMENT : total, items }); } } export class WebAppFactory extends WorkerEntrypoint { async at(id) { - const kind = { [KNOWN_WEBAPP]: null, [CAPPED_WEBAPP]: "capped", [DATED_WEBAPP]: "dated", [DEEP_WEBAPP]: "deep" }[id]; + const kind = { + [KNOWN_WEBAPP]: null, + [CAPPED_WEBAPP]: "capped", + [OVERSTATED_WEBAPP]: "overstated", + [DATED_WEBAPP]: "dated", + [DEEP_WEBAPP]: "deep", + }[id]; if (kind === undefined) throw new Error("fake webapp: unknown id"); return new FakeGet("", {}, kind); } diff --git a/diigo-cache/tests/fill.test.ts b/diigo-cache/tests/fill.test.ts index c025664..ed738e7 100644 --- a/diigo-cache/tests/fill.test.ts +++ b/diigo-cache/tests/fill.test.ts @@ -131,6 +131,14 @@ describe("fill job (goal 1e)", () => { expect(status?.error).toMatch(/page size not honored/); }); + it("fills to the real end when diigo's total overstates its items", async () => { + const stub = await wired("fake-overstated-webapp"); + await stub.startFill(); + await drain(stub); + expect(await stub.fillStatus()).toMatchObject({ state: "done" }); + expect((await stub.page(3, PAGE_SIZE)).items).toHaveLength(50); + }); + it("a job saved under another page size fails instead of resuming; a new start replaces it", async () => { const stub = await wired(); // the shape a 24-per-page job left in storage (no pageSize field)