Add fresh read variants that await a server round-trip (closes #75) #76

Merged
clawbot merged 1 commits from issue-75-fresh-reads into next 2026-09-22 23:29:15 +02:00
3 changed files with 358 additions and 12 deletions
Showing only changes of commit 0536ecab78 - Show all commits
+77 -12
View File
@@ -6,10 +6,17 @@
// existing copy loaded, that first refresh runs in the background and `open()` // 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 // 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 // server no longer stalls opening. A background timer then refreshes every
// `refreshIntervalSeconds`. Every read is answered from RAM — no read touches // `refreshIntervalSeconds`. Every default read is answered from RAM — no
// the network. There is deliberately no `sync()`, no `refresh()`, no // default read touches the network. There is deliberately no `sync()`, no
// `serverReachable` flag, and no "before each read" mode (design #36): the // `refresh()`, no `serverReachable` flag, and no "before each read" mode
// only ways state changes are the refreshes above. // (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 // 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 // once every fetch has succeeded. A refresh that fails partway therefore never
@@ -40,6 +47,7 @@ import {
type AlbumsAPI, type AlbumsAPI,
type PhotosAPI, type PhotosAPI,
type TimelineAPI, type TimelineAPI,
type FreshReads,
} from "./read.js"; } from "./read.js";
import { import {
ContentCache, ContentCache,
@@ -57,6 +65,7 @@ export {
type AlbumsAPI, type AlbumsAPI,
type PhotosAPI, type PhotosAPI,
type TimelineAPI, type TimelineAPI,
type FreshReads,
type PhotoFilter, type PhotoFilter,
type TimelineGroup, type TimelineGroup,
type GroupBy, type GroupBy,
@@ -250,7 +259,12 @@ export class Library {
private readonly precache?: Precache; private readonly precache?: Precache;
private timer?: ReturnType<typeof setTimeout>; private timer?: ReturnType<typeof setTimeout>;
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<void>;
// Guards the ML fetch pass so a slow backfill never runs twice at once; a // Guards the ML fetch pass so a slow backfill never runs twice at once; a
// refresh whose pass is still running kicks nothing new. // refresh whose pass is still running kicks nothing new.
private mlFetching = false; 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<FreshReads> {
await this.refreshNow();
return {
albums: this.albums,
photos: this.photos,
timeline: this.timeline,
};
}
// Back up every in-scope file to `downloadDirectory` in the historical // Back up every in-scope file to `downloadDirectory` in the historical
// on-disk layout, with a durable failure ledger (issue #51). Refreshes // on-disk layout, with a durable failure ledger (issue #51). Refreshes
// first, fetches pending originals (and optional thumbnails) through the // first, fetches pending originals (and optional thumbnails) through the
@@ -543,11 +572,48 @@ export class Library {
this.timer.unref?.(); this.timer.unref?.();
} }
// One refresh cycle, guarded so a failure never escapes and overlapping // The background loop's refresh: run a cycle unless one is already in flight
// cycles never run. Errors are reported, not thrown. // (or the library is closed), and never let a failure escape — the
private async runRefresh(): Promise<void> { // background path reports errors through `status()`/`onProgress`, it does
if (this.closed || this.refreshing) return; // not throw. Resolves once the cycle it started (or skipped past) settles.
this.refreshing = true; private runRefresh(): Promise<void> {
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<void> {
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<void> {
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<void> {
this.emit({ operation: "refresh", status: "started" }); this.emit({ operation: "refresh", status: "started" });
try { try {
await this.refreshOnce(); await this.refreshOnce();
@@ -563,8 +629,7 @@ export class Library {
const error = err instanceof Error ? err.message : String(err); const error = err instanceof Error ? err.message : String(err);
this.lastError = error; this.lastError = error;
this.emit({ operation: "refresh", status: "failed", error }); this.emit({ operation: "refresh", status: "failed", error });
} finally { throw err;
this.refreshing = false;
} }
} }
+9
View File
@@ -193,6 +193,15 @@ export interface TimelineAPI {
groups(args: { groupBy: GroupBy; filter?: PhotoFilter }): TimelineGroup[]; 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 = ( export const makeAlbumsAPI = (
derive: () => DerivedRecords, derive: () => DerivedRecords,
content?: PhotoContent, content?: PhotoContent,
+272
View File
@@ -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<number, FilesPage[]>();
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<CollectionsPage> {
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<FilesPage> {
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<void>((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();
}
});
});