From aeccb489b527f6579a5f71ed084b78f1bdcce627 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 22 Sep 2026 23:29:15 +0200 Subject: [PATCH] Fresh read variants that await a server round-trip (closes #75) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds Library.fresh(): forces a refresh, awaits its completion and persist, then returns the albums/photos/timeline read namespaces now reflecting a completed server round trip — the caller awaits and is guaranteed current at resolve. Default reads and the background loop are unchanged (immediate-from-cache). Concurrent fresh reads coalesce to one in-flight refresh; a fresh read whose refresh fails rejects rather than answering stale. The CLI adopts fresh reads separately (#52). Model: opus-4-8 --- src/library/index.ts | 89 ++++++++++-- src/library/read.ts | 9 ++ test/library/fresh.test.ts | 272 +++++++++++++++++++++++++++++++++++++ 3 files changed, 358 insertions(+), 12 deletions(-) create mode 100644 test/library/fresh.test.ts diff --git a/src/library/index.ts b/src/library/index.ts index d9aa5e3..4a101dc 100644 --- a/src/library/index.ts +++ b/src/library/index.ts @@ -6,10 +6,17 @@ // existing copy loaded, that first refresh runs in the background and `open()` // returns as soon as the cached data is ready to serve — a slow or unreachable // server no longer stalls opening. A background timer then refreshes every -// `refreshIntervalSeconds`. Every read is answered from RAM — no read touches -// the network. There is deliberately no `sync()`, no `refresh()`, no -// `serverReachable` flag, and no "before each read" mode (design #36): the -// only ways state changes are the refreshes above. +// `refreshIntervalSeconds`. Every default read is answered from RAM — no +// default read touches the network. There is deliberately no `sync()`, no +// `refresh()`, no `serverReachable` flag, and no "before each read" mode +// (design #36). +// +// `fresh()` is the one exception (issue #75, an owner amendment to #36): it +// forces a refresh, awaits it, and only then hands back the read namespaces, so +// a caller that needs server-current data can ask for it. Concurrent `fresh()` +// calls coalesce onto one in-flight refresh, and a refresh that fails rejects +// the caller (the default reads stay silent and serve the last good copy). The +// default methods and the background loop are unchanged. // // A refresh stages all of its network work first and only mutates the store // once every fetch has succeeded. A refresh that fails partway therefore never @@ -40,6 +47,7 @@ import { type AlbumsAPI, type PhotosAPI, type TimelineAPI, + type FreshReads, } from "./read.js"; import { ContentCache, @@ -57,6 +65,7 @@ export { type AlbumsAPI, type PhotosAPI, type TimelineAPI, + type FreshReads, type PhotoFilter, type TimelineGroup, type GroupBy, @@ -250,7 +259,12 @@ export class Library { private readonly precache?: Precache; private timer?: ReturnType; - private refreshing = false; + // The in-flight refresh cycle, or undefined when none runs. One slot serves + // both paths: the background loop skips when it is set, and a fresh read + // (issue #75) coalesces onto it or starts one. The promise carries the + // cycle's real outcome (it rejects on failure); the background loop ignores + // that, a fresh read propagates it. + private cycle?: Promise; // Guards the ML fetch pass so a slow backfill never runs twice at once; a // refresh whose pass is still running kicks nothing new. private mlFetching = false; @@ -484,6 +498,21 @@ export class Library { }; } + // Fresh reads (issue #75, owner amendment to design #36). Force a refresh, + // wait for it to complete and persist, then hand back the same + // `albums`/`photos`/`timeline` namespaces — now guaranteed to reflect a + // completed server round-trip. Concurrent calls coalesce onto one refresh; + // a refresh that fails rejects here, where the default namespaces would + // instead stay silent and serve the last good copy. + async fresh(): Promise { + await this.refreshNow(); + return { + albums: this.albums, + photos: this.photos, + timeline: this.timeline, + }; + } + // Back up every in-scope file to `downloadDirectory` in the historical // on-disk layout, with a durable failure ledger (issue #51). Refreshes // first, fetches pending originals (and optional thumbnails) through the @@ -543,11 +572,48 @@ export class Library { this.timer.unref?.(); } - // One refresh cycle, guarded so a failure never escapes and overlapping - // cycles never run. Errors are reported, not thrown. - private async runRefresh(): Promise { - if (this.closed || this.refreshing) return; - this.refreshing = true; + // The background loop's refresh: run a cycle unless one is already in flight + // (or the library is closed), and never let a failure escape — the + // background path reports errors through `status()`/`onProgress`, it does + // not throw. Resolves once the cycle it started (or skipped past) settles. + private runRefresh(): Promise { + if (this.closed || this.cycle) return Promise.resolve(); + return this.startCycle().catch(() => {}); + } + + // A fresh read's refresh (issue #75): force a cycle and await it, rejecting + // if it fails. Concurrent fresh reads coalesce onto the one in-flight cycle + // — the background loop's included — so they never fan out into redundant + // server round-trips. + private refreshNow(): Promise { + if (this.closed) { + return Promise.reject(new Error("the library is closed")); + } + return this.cycle ?? this.startCycle(); + } + + // Start one refresh cycle and record it as the in-flight cycle so every + // caller coalesces onto it. The returned promise carries the cycle's real + // outcome; each caller attaches the handling its own path needs, and the + // slot is cleared once the cycle settles. + private startCycle(): Promise { + const cycle = this.refreshCycle(); + this.cycle = cycle; + void cycle.then( + () => { + if (this.cycle === cycle) this.cycle = undefined; + }, + () => { + if (this.cycle === cycle) this.cycle = undefined; + }, + ); + return cycle; + } + + // One refresh cycle: the network fetch and commit, wrapped in the progress + // events and status bookkeeping. Throws when the refresh fails so a fresh + // read can reject; `runRefresh` swallows that throw for the background loop. + private async refreshCycle(): Promise { this.emit({ operation: "refresh", status: "started" }); try { await this.refreshOnce(); @@ -563,8 +629,7 @@ export class Library { const error = err instanceof Error ? err.message : String(err); this.lastError = error; this.emit({ operation: "refresh", status: "failed", error }); - } finally { - this.refreshing = false; + throw err; } } diff --git a/src/library/read.ts b/src/library/read.ts index b8f8678..7443ead 100644 --- a/src/library/read.ts +++ b/src/library/read.ts @@ -193,6 +193,15 @@ export interface TimelineAPI { groups(args: { groupBy: GroupBy; filter?: PhotoFilter }): TimelineGroup[]; } +// The surface `Library.fresh()` resolves to (issue #75). It is the same three +// read namespaces as the default `albums`/`photos`/`timeline`, handed back only +// after a forced refresh has brought the local copy current. +export interface FreshReads { + albums: AlbumsAPI; + photos: PhotosAPI; + timeline: TimelineAPI; +} + export const makeAlbumsAPI = ( derive: () => DerivedRecords, content?: PhotoContent, diff --git a/test/library/fresh.test.ts b/test/library/fresh.test.ts new file mode 100644 index 0000000..15b71b7 --- /dev/null +++ b/test/library/fresh.test.ts @@ -0,0 +1,272 @@ +/** + * Tests for fresh reads (issue #75, an owner amendment to design #36). + * + * The default read namespaces answer from RAM and never touch the network; a + * background loop keeps the local copy current. `fresh()` adds an awaited path: + * it forces a refresh, waits for it to complete and persist, and only then + * hands back the `albums`/`photos`/`timeline` namespaces, guaranteeing the + * local copy reflects a completed server round-trip. The contracts here: + * + * 1. A fresh read observes a server change that a same-instant default read + * would miss (the default path has not refreshed yet). + * 2. Concurrent fresh reads coalesce onto one in-flight refresh — three + * concurrent `fresh()` calls make exactly one collections round-trip, not + * three. + * 3. A refresh that fails rejects the fresh read (currency was unavailable), + * while the default reads stay silent and keep serving the last good copy. + * + * The client is the same metadata-only mock the background-refresh tests use: + * no crypto, no network, scripted pages, and a record of each call. A long + * refresh interval keeps the background timer out of the way so each test's + * refreshes are exactly the ones it triggers. + */ + +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { Library } from "../../src/library/index.js"; +import type { CollectionsPage, FilesPage } from "../../src/client.js"; +import type { Collection, EnteFile } from "../../src/model/types.js"; + +const USER_ID = 42; + +// Long enough that no background tick fires during a test; each test's +// refreshes are only the ones its own `fresh()` calls force. +const SLOW_INTERVAL = 3600; + +const collection = (id: number, updationTime: number): Collection => ({ + id, + ownerID: USER_ID, + key: new Uint8Array([id & 0xff]), + name: `album-${id}`, + type: "album", + updationTime, + isShared: false, +}); + +const file = ( + id: number, + collectionID: number, + updationTime: number, +): EnteFile => ({ + id, + collectionID, + ownerID: USER_ID, + key: new Uint8Array([id & 0xff]), + metadata: { + title: `file-${id}.jpg`, + fileType: "image", + creationTime: updationTime, + modificationTime: updationTime, + }, + file: { decryptionHeader: "aGVhZGVy" }, + thumbnail: { decryptionHeader: "dGh1bWI=" }, + updationTime, +}); + +class MockClient { + userID = USER_ID; + failCollections = false; + collectionsQueue: CollectionsPage[] = []; + filesByCollection = new Map(); + + collectionsSinceTimes: number[] = []; + filesCalls: { collectionID: number; sinceTime: number }[] = []; + + whoami(): { email: string; userID: number } { + return { email: "user@example.com", userID: this.userID }; + } + + async collectionsSince(args: { + sinceTime: number; + }): Promise { + this.collectionsSinceTimes.push(args.sinceTime); + if (this.failCollections) throw new Error("network down"); + return ( + this.collectionsQueue.shift() ?? { + collections: [], + deleted: [], + cursor: args.sinceTime, + } + ); + } + + async filesSince(args: { + collectionID: number; + collectionKey: Uint8Array; + sinceTime: number; + }): Promise { + this.filesCalls.push({ + collectionID: args.collectionID, + sinceTime: args.sinceTime, + }); + const queue = this.filesByCollection.get(args.collectionID); + return ( + queue?.shift() ?? { + files: [], + deleted: [], + cursor: args.sinceTime, + } + ); + } + + filesFor(collectionID: number, ...pages: FilesPage[]): void { + this.filesByCollection.set(collectionID, pages); + } +} + +// A client seeded with one collection and one file, opened with an empty cache +// so the initial refresh is awaited and post-`open()` state is deterministic. +const openSeeded = async ( + cacheDirectory: string, +): Promise<{ client: MockClient; lib: Library }> => { + const client = new MockClient(); + client.collectionsQueue.push({ + collections: [collection(1, 100)], + deleted: [], + cursor: 100, + }); + client.filesFor(1, { + files: [file(1001, 1, 90)], + deleted: [], + cursor: 90, + }); + const lib = await Library.open({ + client, + cacheDirectory, + refreshIntervalSeconds: SLOW_INTERVAL, + }); + return { client, lib }; +}; + +describe("Library.fresh", () => { + let dir: string; + let cacheDirectory: string; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "quak-fresh-")); + cacheDirectory = join(dir, "cache"); + }); + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("observes a server change a same-instant default read would miss", async () => { + const { client, lib } = await openSeeded(cacheDirectory); + try { + // A new file appears on the server after open, advancing its + // collection so the next refresh re-enumerates it. + client.collectionsQueue.push({ + collections: [collection(1, 200)], + deleted: [], + cursor: 200, + }); + client.filesFor(1, { + files: [file(1002, 1, 190)], + deleted: [], + cursor: 190, + }); + + // A default read at this instant has not refreshed: it misses 1002. + expect(lib.photos.byID({ fileID: 1002 })).toBeUndefined(); + + // A fresh read forces the round-trip and sees it. + const reads = await lib.fresh(); + expect(reads.photos.byID({ fileID: 1002 })?.fileID).toBe(1002); + // And the change is now live for the default namespaces too. + expect(lib.photos.byID({ fileID: 1002 })?.fileID).toBe(1002); + } finally { + lib.close(); + } + }); + + it("coalesces concurrent fresh reads onto one in-flight refresh", async () => { + const { client, lib } = await openSeeded(cacheDirectory); + try { + client.collectionsQueue.push({ + collections: [collection(1, 200)], + deleted: [], + cursor: 200, + }); + client.filesFor(1, { + files: [file(1002, 1, 190)], + deleted: [], + cursor: 190, + }); + + const collectionsBefore = client.collectionsSinceTimes.length; + const filesBefore = client.filesCalls.length; + + // Gate the next collections fetch so all three fresh reads are in + // flight together before any of them completes. + let release: () => void = () => {}; + const gate = new Promise((r) => { + release = r; + }); + const inner = client.collectionsSince.bind(client); + client.collectionsSince = async (args: { sinceTime: number }) => { + await gate; + return inner(args); + }; + + const all = Promise.all([lib.fresh(), lib.fresh(), lib.fresh()]); + release(); + const [a, b, c] = await all; + + // Exactly one collections round-trip and one file round-trip served + // all three fresh reads. + expect(client.collectionsSinceTimes.length).toBe( + collectionsBefore + 1, + ); + expect(client.filesCalls.length).toBe(filesBefore + 1); + + // All three observed the change. + for (const reads of [a, b, c]) { + expect(reads.photos.byID({ fileID: 1002 })?.fileID).toBe(1002); + } + } finally { + lib.close(); + } + }); + + it("rejects the fresh read when the refresh fails, leaving defaults intact", async () => { + const { client, lib } = await openSeeded(cacheDirectory); + try { + client.failCollections = true; + + // Two concurrent fresh reads both reject, and share one failed + // round-trip rather than each making its own. + const collectionsBefore = client.collectionsSinceTimes.length; + const first = lib.fresh(); + const second = lib.fresh(); + await expect(first).rejects.toThrow(/network down/); + await expect(second).rejects.toThrow(/network down/); + expect(client.collectionsSinceTimes.length).toBe( + collectionsBefore + 1, + ); + + // The default reads never rejected: they still serve the last good + // copy, and the failure surfaced through status(). + expect(lib.photos.byID({ fileID: 1001 })?.fileID).toBe(1001); + expect(lib.status().lastError).toMatch(/network down/); + + // Recovery: once the server answers, a fresh read resolves current. + client.failCollections = false; + client.collectionsQueue.push({ + collections: [collection(2, 300)], + deleted: [], + cursor: 300, + }); + const reads = await lib.fresh(); + expect(reads.albums.byID({ collectionID: 2 })?.collectionID).toBe( + 2, + ); + expect(lib.status().lastError).toBeUndefined(); + } finally { + lib.close(); + } + }); +});