Fresh read variants that await a server round-trip (closes #75)
check / check (push) Successful in 28s
check / check (push) Successful in 28s
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
This commit was merged in pull request #76.
This commit is contained in:
+77
-12
@@ -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<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
|
||||
// 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<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
|
||||
// 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<void> {
|
||||
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<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" });
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user