quak files COLLECTION fetches that collection's files alone (closes #217)

`quak files COLLECTION` used to wait for a full refresh, which on an
empty cache fetched the files of every collection before it listed one.
It now reads the collection list, then calls the new
`Library.refreshCollection()`. That brings one collection up to date in
the store as a refresh would, and saves it. It runs in the same slot as
a refresh, so the two never run at the same time, and `close()` waits
for it. It never stores a collection that a refresh has already dropped.
It leaves the refresh cursor alone. `open()` therefore awaits the first
refresh on a cache whose cursor is still 0. `quak files` with no
argument still waits for a refresh.

Model: opus-5-5

src/cli-commands.ts

@@ -128,12 +128,13 @@ const readLibraryClient = (client: Client): LibraryClient => ({
     contentSource: () => client.contentSource(),
 });
 
-// Open a library for a single point command: the aggressive background precache
-// (issue #48) is off — a one-shot `collections` or `get` must not start
-// downloading the whole account — and no refresh starts as it opens or on a
-// timer: each command asks for the one it needs, and `collections` needs none
-// (issue #210). While a refresh runs, each collection whose files it has
-// fetched is logged, so a large account shows progress rather than nothing.
+// Open a library for a single point command. The aggressive background
+// precache (issue #48) is off, because a one-shot `collections` or `get` must
+// not start downloading the whole account. No refresh starts as it opens or on
+// a timer. Each command asks for the refresh it needs, and `collections` and
+// `files COLLECTION` need none (issues #210 and #217). While a refresh runs,
+// each collection whose files it has fetched is logged, so a large account
+// shows progress rather than nothing.
 const openReadLibrary = (ctx: CliContext, client: Client): Promise<Library> =>
     Library.open({
         client: readLibraryClient(client),

src/cli-read.ts

@@ -2,13 +2,17 @@
 //
 // `collections`, `files`, `get`, and `get-thumb` must answer for
 // the account's state at the moment the command runs, not for whatever the
-// local cache last happened to hold (owner amendment, issue #36). `files`,
-// `get` and `get-thumb` therefore force a server round-trip through
-// `Library.fresh()` and only then read — so a collection, file, or metadata
-// change made elsewhere is visible. `collections` asks the library for the
-// collection list alone through `Library.freshCollections()` (issue #210): a
-// refresh would also fetch the files of every collection, which on a large
-// account takes long and which `collections` does not print.
+// local cache last happened to hold (owner amendment, issue #36). `files`
+// without a collection ID, `get` and `get-thumb` therefore force a server
+// round-trip through `Library.fresh()`, and only then read. A collection, file,
+// or metadata change made elsewhere is then visible. `collections` asks the
+// library for the collection list alone through `Library.freshCollections()`
+// (issue #210). A refresh would also fetch the files of every collection, which
+// `collections` does not print, and on a large account that takes long.
+// `files COLLECTION` reads the collection list the same way, for the same
+// reason. It then brings that one collection up to date through
+// `Library.refreshCollection()` (issue #217), and fetches no other collection's
+// files.
 //
 // `collections` lists in the server's order and `files` in the library's own
 // enumeration order (`listFiles()`), the orders the pre-library CLI printed,
@@ -23,13 +27,13 @@ import type { Collection, EnteFile } from "./model/types.js";
 import type { Photo, PhotosAPI } from "./library/index.js";
 
 // The slice of `Library` these helpers read. `Library` satisfies it
-// structurally; a test can drive them with a stand-in that records the
-// `fresh()` and `freshCollections()` calls and serves records in a known
-// enumeration order.
+// structurally. A test can drive them with a stand-in that records the
+// `fresh()`, `freshCollections()` and `refreshCollection()` calls. The
+// stand-in serves records in a known enumeration order.
 export interface FreshReadLibrary {
     fresh(): Promise<unknown>;
     freshCollections(): Promise<Collection[]>;
-    getCollection(id: number): Collection | undefined;
+    refreshCollection(collection: Collection): Promise<void>;
     listCollections(): Collection[];
     listFiles(collectionID: number): EnteFile[];
     getFileByID(fileID: number): EnteFile | undefined;
@@ -42,21 +46,24 @@ export const freshCollections = (
     lib: FreshReadLibrary,
 ): Promise<Collection[]> => lib.freshCollections();
 
-// The files of one collection, current as of a forced refresh, in enumeration
-// order. `undefined` (not an empty list) when the collection does not exist, so
+// The files of one collection, in enumeration order, current as of one
+// collections request and the requests that bring that collection alone up to
+// date. `undefined` (not an empty list) when the collection does not exist, so
 // the caller can tell "no such collection" from "an empty collection". Without
-// a collection ID, every file in the account, each once, as held by the first
-// collection that holds it.
+// a collection ID, every file in the account, current as of a forced refresh.
+// Each file is listed once, as held by the first collection that holds it.
 export const freshFiles = async (
     lib: FreshReadLibrary,
     collectionID?: number,
 ): Promise<EnteFile[] | undefined> => {
-    await lib.fresh();
     if (collectionID !== undefined) {
-        return lib.getCollection(collectionID)
-            ? lib.listFiles(collectionID)
-            : undefined;
+        const collections = await lib.freshCollections();
+        const collection = collections.find((c) => c.id === collectionID);
+        if (!collection) return undefined;
+        await lib.refreshCollection(collection);
+        return lib.listFiles(collectionID);
     }
+    await lib.fresh();
     const listed = new Set<number>();
     const files: EnteFile[] = [];
     for (const collection of lib.listCollections()) {

src/library/index.ts

@@ -1,15 +1,16 @@
 // The library surface over the local cache.
 //
 // `Library.open()` loads the on-disk metadata store (issue #41), then starts
-// the refresh loop. When the cache loaded empty it awaits the first refresh,
-// so the library never opens onto an empty store it could have filled; when an
-// 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 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).
+// the refresh loop. When no refresh has stored a collection yet (the stored
+// cursor is 0), `open()` awaits the first refresh. The cache is then empty, or
+// holds only what `refreshCollection()` stored, and the library must not serve
+// it as the whole account. Otherwise that first refresh runs in the
+// background. `open()` returns as soon as the cached data is ready to serve,
+// so a slow or unreachable server does not stall opening. A background timer
+// then refreshes every `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
@@ -20,9 +21,12 @@
 //
 // `freshCollections()` is the fresh read of the collection list alone (issue
 // #210): one collections request, no collection's files, and nothing stored,
-// for a caller such as `quak collections` that needs no more. A caller that
-// asks for each refresh it needs can also open the library with
-// `refreshOnOpen: false`, so that `open()` starts none.
+// for a caller such as `quak collections` that needs no more.
+// `refreshCollection()` brings one collection from that list up to date in the
+// store (issue #217). It fetches that collection's files alone, as a refresh
+// would. `quak files COLLECTION` uses it. It never runs at the same time as a
+// refresh. A caller that asks for each refresh it needs can also open the
+// library with `refreshOnOpen: false`, so that `open()` starts none.
 //
 // A refresh reports through `onProgress` each collection whose files it has
 // fetched, so a large account, whose first refresh fetches the files of every
@@ -306,12 +310,16 @@ export class Library {
     private readonly precache?: Precache;
 
     private timer?: ReturnType<typeof setTimeout>;
-    // 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.
+    // The in-flight refresh cycle or `refreshCollection()`, or undefined when
+    // neither runs. One slot holds both, so they never run at the same time.
+    // The background loop skips while it is set. A fresh read (issue #75)
+    // coalesces onto a running refresh. It waits for a running
+    // `refreshCollection()` to end, and then joins or starts a refresh. The
+    // promise carries the real outcome (it rejects on failure); the background
+    // loop ignores that, a fresh read propagates it.
     private cycle?: Promise<void>;
+    // Whether `cycle` is a refresh cycle rather than a `refreshCollection()`.
+    private cycleIsRefresh = false;
     // Guards the ML fetch pass so a slow backfill never runs twice at once; a
     // refresh whose pass is still running kicks nothing new. Holds the running
     // pass, so `close()` and `backup()` can wait for it. It rejects when the
@@ -393,14 +401,16 @@ export class Library {
         this.mldata = makeMLDataAPI(() => this.mlStore);
     }
 
-    // Load the cache and start the refresh loop. With an empty cache the first
-    // refresh is awaited, so `open()` resolves onto populated data whenever the
-    // server is reachable; that awaited refresh may still fail, and the library
-    // then opens empty with the failure recorded in `status()`. With an
-    // existing cache the first refresh runs in the background and `open()`
-    // returns as soon as the cached data is ready — an unreachable server does
-    // not block opening. With `refreshOnOpen: false` there is no first
-    // refresh: `open()` returns onto the cache as it is.
+    // Load the cache and start the refresh loop. When the stored cursor is 0,
+    // the first refresh is awaited. Such a cache is empty, or holds only what
+    // `refreshCollection()` stored. `open()` then resolves onto the whole
+    // account whenever the server is reachable. If that refresh fails, the
+    // library opens onto what the cache holds, with the failure recorded in
+    // `status()`. With any other cache the first refresh runs in the
+    // background, and `open()` returns as soon as the cached data is ready.
+    // An unreachable server then does not block opening. With
+    // `refreshOnOpen: false` there is no first refresh: `open()` returns onto
+    // the cache as it is.
     static async open(opts: LibraryOptions): Promise<Library> {
         const { userID } = opts.client.whoami();
         const cacheDirectory =
@@ -506,13 +516,17 @@ export class Library {
         if (opts.refreshOnOpen === false) {
             // The caller asks for each refresh it needs.
             lib.scheduleNext();
-        } else if (store.loadedFromDisk) {
-            // An existing copy already answers reads; refresh in the background
-            // and start the interval once that first cycle settles.
+        } else if (store.collectionsSinceTime > 0) {
+            // The stored cursor has moved, so a refresh has stored the
+            // account's collections and the cache's copy answers reads.
+            // Refresh in the background, and start the interval once that
+            // first cycle settles.
             void lib.runRefresh().then(() => lib.scheduleNext());
         } else {
-            // Nothing was cached: wait for the first refresh to fill the store
-            // (or fail) rather than resolve onto an empty library.
+            // The stored cursor is still 0, so no refresh has stored any
+            // collection: the cache is empty, or holds only what
+            // `refreshCollection()` stored. Wait for the first refresh to fill
+            // the store (or fail) rather than resolve onto part of the account.
             await lib.runRefresh();
             lib.scheduleNext();
         }
@@ -625,6 +639,21 @@ export class Library {
         return collections;
     }
 
+    // Bring one collection up to date in the store (issue #217), given its
+    // record from `freshCollections()`. It does for that collection what a
+    // refresh does for a changed one. It fetches no other collection's files.
+    // It runs in the same slot as a refresh, so the two never run at the same
+    // time. It first waits for a running refresh or `refreshCollection()` to
+    // end. A fresh read or `backup()` that starts meanwhile waits for it, and
+    // the background loop skips its turn.
+    async refreshCollection(collection: Collection): Promise<void> {
+        while (this.cycle) await this.cycle.catch(() => {});
+        if (this.closed) throw new Error("the library is closed");
+        await this.startCycle(false, () =>
+            this.refreshCollectionOnce(collection),
+        );
+    }
+
     // Back up every in-scope file to `opts.downloadDirectory`, or else the
     // library's, each original at its save path, with a durable failure
     // ledger (issue #51). Takes the lock in that directory first, unless the
@@ -674,11 +703,13 @@ export class Library {
         );
     }
 
-    // Stop the background timer. Idempotent. An in-flight refresh is left to
-    // finish; it will not schedule another cycle once closed. The returned
-    // promise resolves once that refresh (including its cache write), the ML
-    // fetch pass and the precache fetches already running have all finished,
-    // so a caller can then remove the cache directory. A refresh failure is
+    // Stop the background timer. Idempotent. A refresh or `refreshCollection()`
+    // in flight is left to finish, and nothing starts after it. The returned
+    // promise resolves once all of these have finished:
+    //  - that refresh or `refreshCollection()`, with its cache write;
+    //  - the ML fetch pass;
+    //  - the precache fetches already running.
+    // A caller can then remove the cache directory. A refresh failure is
     // reported through `status()`, not thrown here.
     async close(): Promise<void> {
         this.closed = true;
@@ -702,33 +733,40 @@ export class Library {
         this.timer.unref?.();
     }
 
-    // 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.
+    // The background loop's refresh: run a cycle unless a refresh or
+    // `refreshCollection()` is 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(() => {});
+        return this.startCycle(true, () => this.refreshCycle()).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"));
+    // server round-trips. A running `refreshCollection()` is waited for first.
+    private async refreshNow(): Promise<void> {
+        while (this.cycle && !this.cycleIsRefresh) {
+            await this.cycle.catch(() => {});
         }
-        return this.cycle ?? this.startCycle();
+        if (this.closed) throw new Error("the library is closed");
+        return this.cycle ?? this.startCycle(true, () => this.refreshCycle());
     }
 
-    // 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();
+    // Start `work` and record it in the in-flight slot, so every other caller
+    // sees it. `isRefresh` says whether it is a refresh cycle, which a fresh
+    // read can coalesce onto. The returned promise carries the real outcome.
+    // Each caller attaches the handling its own path needs, and the slot is
+    // cleared once the work settles.
+    private startCycle(
+        isRefresh: boolean,
+        work: () => Promise<void>,
+    ): Promise<void> {
+        const cycle = work();
         this.cycle = cycle;
+        this.cycleIsRefresh = isRefresh;
         void cycle.then(
             () => {
                 if (this.cycle === cycle) this.cycle = undefined;
@@ -766,10 +804,7 @@ export class Library {
 
     // Fetch every change since the stored cursor, then commit. All network
     // reads happen before any store mutation, so a fetch that throws leaves the
-    // store untouched and the previous snapshot intact. A collection or file
-    // that does not decrypt is left out instead, as if deleted, with the reason
-    // kept in the store: the cursor moves past it, so no later refresh sees it
-    // again until the server's copy changes.
+    // store untouched and the previous snapshot intact.
     private async refreshOnce(): Promise<void> {
         const page = await this.client.collectionsSince({
             sinceTime: this.store.collectionsSinceTime,
@@ -802,6 +837,51 @@ export class Library {
             });
         }
 
+        await this.commit(page, filePages);
+    }
+
+    // `refreshCollection()`'s work, run in the slot: fetch the files of one
+    // collection changed since the stored copy, then commit.
+    private async refreshCollectionOnce(collection: Collection): Promise<void> {
+        const known = this.store.getCollection(collection.id);
+        // The stored copy is as new or newer, so there is nothing to fetch.
+        if (known && collection.updationTime <= known.updationTime) return;
+        // No copy is stored, yet the stored cursor has reached this
+        // `updationTime`. A refresh has then seen this version or a later one,
+        // and dropped the collection: it was deleted, or did not decrypt.
+        // Storing it would bring it back for good, since no later refresh sees
+        // it again.
+        if (
+            !known &&
+            collection.updationTime <= this.store.collectionsSinceTime
+        ) {
+            return;
+        }
+        const filePage = await this.client.filesSince({
+            collectionID: collection.id,
+            collectionKey: collection.key,
+            sinceTime: known ? known.updationTime : 0,
+        });
+        // The stored cursor stays where it is, so the next refresh still
+        // fetches the files of every other collection that changed.
+        await this.commit(
+            {
+                collections: [collection],
+                deleted: [],
+                cursor: this.store.collectionsSinceTime,
+            },
+            [{ collectionID: collection.id, page: filePage }],
+        );
+    }
+
+    // Commit a collections page and the file pages fetched for its collections
+    // to the store, then save. A collection or file that does not decrypt is
+    // left out, as if deleted. The reason is kept in the store. No later
+    // refresh sees it again until the server's copy changes.
+    private async commit(
+        page: CollectionsPage,
+        filePages: { collectionID: number; page: FilesPage }[],
+    ): Promise<void> {
         // Network work done; commit to the store and persist only if something
         // actually changed.
         let changed = false;

src/library/store.ts

@@ -70,12 +70,6 @@ export class MetadataStore {
     userID = 0;
     collectionsSinceTime: Microseconds = 0;
 
-    // True when `load` populated this store from a valid existing file; false
-    // on a first run or a missing/corrupt/wrong-version file that loaded empty.
-    // `Library.open` reads it to decide whether the first refresh may run in
-    // the background (an existing copy already serves reads) or must be awaited.
-    loadedFromDisk = false;
-
     private readonly collections = new Map<number, Collection>();
     private readonly files = new Map<string, EnteFile>();
     private undecryptable: Undecryptable[] = [];
@@ -102,7 +96,6 @@ export class MetadataStore {
             if (parsed.schemaVersion !== METADATA_SCHEMA_VERSION) {
                 return store;
             }
-            store.loadedFromDisk = true;
             store.userID = parsed.userID ?? 0;
             store.collectionsSinceTime = parsed.collectionsSinceTime ?? 0;
             for (const stored of parsed.collections ?? []) {