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

README.md

@@ -651,17 +651,17 @@ The CLI stores the snapshot at the platform-appropriate data directory via
 `0600`. The key material is stored in cleartext in the JSON; treat this file as
 you would treat the password itself. A missing file is reported as "not logged
 in"; a file that exists but is corrupt is reported as such, naming the bad
-field. The server answers HTTP 401 when it no longer accepts the saved session's
-token. A command that gets that answer prints one line,
-`quak: the saved session is no longer valid; run "quak login"`, with no stack
-trace. All three exit with status 3, which means the user must run `quak login`
-again. `quak logout` is the exception: with no file it says there is no session
-and exits 0, and it handles a corrupt file or a failed server call as described
-below. `quak backup` meets an expired session on the refresh that starts every
-run, before it touches any file but its lock (see "Backup layout"). A session
-that stops working partway through a backup instead fails each remaining file
-into `failures.json`, so that run exits 1 and the next one stops at its refresh
-with status 3. No command but `quak login` ever prompts.
+field. When the refresh a command starts with gets HTTP 401 from the server,
+because it no longer accepts the saved session's token, the command prints one
+line, `quak: the saved session is no longer valid; run "quak login"`, with no
+stack trace. All three exit with status 3, which means the user must run
+`quak login` again. `quak logout` is the exception: with no file it says there
+is no session and exits 0, and it handles a corrupt file or a failed server call
+as described below. `quak backup` meets an expired session on the refresh that
+starts every run, before it touches any file but its lock (see "Backup layout").
+A session that stops working partway through a backup instead fails each
+remaining file into `failures.json`, so that run exits 1 and the next one stops
+at its refresh with status 3. No command but `quak login` ever prompts.
 
 `quak logout` ends the session on the server, so the token in `session.json`
 stops working even in a copy of the file, and then deletes the file. If the
@@ -708,14 +708,12 @@ fetches no collection's files and leaves the cache as it is. `files` with a
 collection ID asks for the collection list the same way. It then asks for the
 files of that collection alone, and stores them in the cache. It asks for no
 files when the cache's copy of that collection is current. The other read
-commands, and `files` without a collection ID, wait for a refresh. So does
-`files` with the ID of a collection that is new since the last refresh.
+commands, and `files` without a collection ID, wait for a refresh.
 
-The first refresh fetches the files of every collection, one collection after
-another. That includes any collection that `files` with a collection ID stored
-before it. The refresh stores nothing until it has them all, and on a large
-account that takes a while. Meanwhile the command logs a line on stderr for each
-collection whose files it has fetched, such as
+On an empty cache, as on a first run, that refresh fetches the files of every
+collection, one collection after another. It stores nothing until it has them
+all, and on a large account that takes a while. Meanwhile the command logs a
+line on stderr for each collection whose files it has fetched, such as
 `Fetched the files of 3 of 120 collections`. It logs these lines with `--json`
 too. `backup` logs them during the refresh it starts with. stdout holds only the
 command's output.
@@ -779,10 +777,9 @@ its `Failed files:` list. Every other line is logged as it is without `--json`.
 exit status 1.
 
 `files <collectionID>` lists the files in that collection. `files` with no
-argument lists every file in the account once, collection by collection. The
-collections come in the order the server first sent them in a refresh. A file in
-several collections is listed with the first of them, which is its
-`collectionID` in `--json`.
+argument lists every file in the account, collection by collection, each file
+once: a file in several collections is listed with the first of them, which is
+its `collectionID` in `--json`.
 
 An ID given to `files`, `get`, `get-thumb` or
 `helper fix-missing-thumbnails --file` is written in decimal digits, no larger
@@ -1162,8 +1159,7 @@ In between, a refresh sends `"progress"` after each collection whose files it
 fetched. That event's `fetched` is the number of collections whose files the
 refresh has fetched so far. Its `total` is the number of collections whose files
 it will fetch in all. A refresh fetches the files of each collection that is new
-or changed since the cache's copy. On the first refresh, that is every
-collection.
+or changed since the cache's copy. On an empty cache, that is every collection.
 
 Concurrency is set through `pools`: construct
 `new RequestPools({ metadataConcurrency, contentConcurrency, thumbnailConcurrency })`
@@ -1218,18 +1214,17 @@ waits for it to end. When a refresh is already running, background or not,
 `fresh()` waits for that one. Its answer can then come from requests made before
 the call. Only when none is running does it start one. A refresh that fails
 rejects the caller (default reads stay silent and keep serving the last good
-copy). Every CLI read command calls `fresh()`, except `quak collections`.
-`quak files` with a collection ID calls it only when `lib.refreshCollection()`
-resolves to `false`.
+copy). Every CLI read command calls `fresh()`, except `quak collections` and
+`quak files` with a collection ID.
 
 `await lib.freshCollections()` is the fresh read of the collection list alone.
 It sends one collections request. It answers with every live collection that
 decrypts, in the server's order, as the `Collection` objects that
 `listCollections()` returns. It fetches no collection's files and stores
-nothing, not even the list. A refresh from a cursor past 0 fetches a
-collection's files only when the collection is new or has changed since the
-stored copy. So does `lib.refreshCollection()`. Neither would fetch the files of
-a collection stored without them. `quak collections` uses `freshCollections()`.
+nothing, not even the list. A refresh fetches a collection's files only when the
+collection is new or has changed since the stored copy. So a collection stored
+without its files would never get them. `quak collections` uses
+`freshCollections()`.
 
 `await lib.refreshCollection(collection)` brings one collection up to date in
 the cache, given its `Collection` from `freshCollections()`. It does for that
@@ -1237,19 +1232,8 @@ collection what a refresh does for a changed one, and fetches no other
 collection's files. It fetches the collection's files changed since the cache's
 copy, or all of them when the cache has no copy. It then stores and saves them.
 It sends no request when the cache's copy has the same `updationTime` or a later
-one. It resolves to `true` when the cache then holds the collection.
-
-`lib.refreshCollection()` adds a collection the cache does not hold only while
-the refresh cursor is 0. The cursor moves past 0 once a refresh receives the
-account's collections. After that, for a collection the cache does not hold, it
-sends no request and resolves to `false`. Such a collection is new since that
-refresh, or the refresh dropped it. Only a refresh may then add it, after the
-collections the cache holds.
-
-A refresh from a cursor of 0 replaces everything the cache holds. It fetches the
-files of every collection, even those `lib.refreshCollection()` stored. It
-stores the collections in the server's order. The cache then holds what it would
-had `lib.refreshCollection()` never run.
+one. It also sends none when a refresh has already dropped the collection from
+the cache, because the server deleted it or it did not decrypt.
 
 `lib.refreshCollection()` leaves the refresh cursor where it was, so the next
 refresh still fetches the files of every other collection that changed. It never

TODO.md

@@ -33,13 +33,9 @@ declares one.
   one collection up to date through the new `lib.refreshCollection()`. That
   stores and saves its files as a refresh would, and leaves the refresh cursor
   where it was. When the stored copy is current, it asks for none of the files.
-  It never runs at the same time as a refresh. Once a refresh has received the
-  account's collections, it stores no collection the cache does not hold, and
-  `quak files COLLECTION` waits for a refresh instead. A refresh from a cursor
-  of 0 replaces all the cache holds, so collections keep the server's order. A
-  library opened on a cache that only `lib.refreshCollection()` wrote awaits its
-  first refresh. `quak files` with no argument still waits for a refresh. The
-  store lists what did not decrypt by collection ID and file ID.
+  It never runs at the same time as a refresh. A library opened on a cache that
+  only it wrote awaits its first refresh. `quak files` with no argument still
+  waits for a refresh.
 
 - 2026-10-22: The JSON `quak backup` and `quak backup-metadata` write for a file
   holds `keywords` in place of the CLIP embedding in its ML data

src/cli-commands.ts

@@ -131,9 +131,8 @@ const readLibraryClient = (client: Client): LibraryClient => ({
 // 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. `collections` needs none
-// (issue #210). `files COLLECTION` needs one only when `refreshCollection()`
-// resolves to `false` (issue #217). While a refresh runs,
+// 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> =>

src/cli-read.ts

@@ -12,20 +12,16 @@
 // `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. Once a refresh has received the account's collections,
-// `refreshCollection()` stores no collection the library does not hold.
-// `files COLLECTION` then forces a refresh instead.
+// files.
 //
-// `collections` lists in the server's order. `files` lists in the library's
-// own enumeration order: `listCollections()`, then `listFiles()` for each.
-// Those are the orders the pre-library CLI printed. The `albums`/`photos`
-// projection would re-sort the rows newest first. After a refresh, the
-// library holds collections in the order the server first sent them. The
-// field values still come from each record's raw metadata via
-// `cli-output.ts`. `files` without a collection ID lists every file in the
-// account once (issue #211).
-// The library keeps a record of a file for each collection that holds it, and
-// only the first collection's record is listed.
+// `collections` lists in the server's order and `files` in the library's own
+// enumeration order (`listFiles()`), the orders the pre-library CLI printed,
+// rather than the `albums`/`photos` projection's newest-first order, which
+// re-sorts the rows. The field values still come from each record's raw
+// metadata via `cli-output.ts`. `files` without a collection ID lists every
+// file in the account once (issue #211): the library keeps a record of a file
+// for each collection that holds it, and only the first collection's record is
+// listed.
 
 import type { Collection, EnteFile } from "./model/types.js";
 import type { Photo, PhotosAPI } from "./library/index.js";
@@ -37,8 +33,7 @@ import type { Photo, PhotosAPI } from "./library/index.js";
 export interface FreshReadLibrary {
     fresh(): Promise<unknown>;
     freshCollections(): Promise<Collection[]>;
-    refreshCollection(collection: Collection): Promise<boolean>;
-    getCollection(id: number): Collection | undefined;
+    refreshCollection(collection: Collection): Promise<void>;
     listCollections(): Collection[];
     listFiles(collectionID: number): EnteFile[];
     getFileByID(fileID: number): EnteFile | undefined;
@@ -51,14 +46,12 @@ export const freshCollections = (
     lib: FreshReadLibrary,
 ): Promise<Collection[]> => lib.freshCollections();
 
-// The files of one collection, in enumeration order. `undefined` (not an empty
-// list) when the collection does not exist, so the caller can tell "no such
-// collection" from "an empty collection". They are current as of one
+// 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. When `refreshCollection()` resolves to `false`, they are current as of
-// a forced refresh instead. Without 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.
+// 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, 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,
@@ -67,10 +60,7 @@ export const freshFiles = async (
         const collections = await lib.freshCollections();
         const collection = collections.find((c) => c.id === collectionID);
         if (!collection) return undefined;
-        if (!(await lib.refreshCollection(collection))) {
-            await lib.fresh();
-            if (!lib.getCollection(collectionID)) return undefined;
-        }
+        await lib.refreshCollection(collection);
         return lib.listFiles(collectionID);
     }
     await lib.fresh();

src/library/index.ts

@@ -25,13 +25,9 @@
 // `quak collections` uses it. `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. Once the stored cursor is
-// past 0, it only updates a collection the store holds, and only a refresh
-// adds one. A refresh from a stored cursor of 0 replaces all the store holds.
-// So after a refresh, the store holds collections in the order the server
-// first sent them. A caller that asks for
-// each refresh it needs can also open the library with `refreshOnOpen: false`,
-// so that `open()` starts none.
+// 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
@@ -642,10 +638,10 @@ export class Library {
     // The fresh read of the collection list alone (issue #210): every live
     // collection that decrypts, in the server's order, from one collections
     // request. Unlike a refresh, it fetches no collection's files, which on a
-    // large account take long. It stores nothing. A refresh from a stored
-    // cursor past 0 fetches a collection's files only when the collection is
-    // new or has changed since the stored copy. So does `refreshCollection()`.
-    // Neither would fetch the files of a collection stored here without them.
+    // large account take long. It stores nothing. A refresh fetches a
+    // collection's files only when the collection is new or has changed since
+    // the stored copy. A collection stored here without its files would never
+    // have them fetched.
     async freshCollections(): Promise<Collection[]> {
         if (this.closed) throw new Error("the library is closed");
         const { collections } = await this.client.collectionsSince({
@@ -657,25 +653,16 @@ export class Library {
     // 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 resolves to whether the store then holds the collection.
-    //
-    // It adds a collection the store does not hold only while the stored
-    // cursor is 0, before any refresh has received a collection. After that,
-    // it stores nothing for such a collection and resolves to false. Only a
-    // refresh may then add it, after the collections the store holds.
-    //
     // 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. While its own work runs, a fresh read or `backup()` waits for it,
     // and the background loop skips its turn.
-    async refreshCollection(collection: Collection): Promise<boolean> {
+    async refreshCollection(collection: Collection): Promise<void> {
         while (this.cycle) await this.cycle.catch(() => {});
         if (this.closed) throw new Error("the library is closed");
-        let held = false;
-        await this.startCycle(false, async () => {
-            held = await this.refreshCollectionOnce(collection);
-        });
-        return held;
+        await this.startCycle(false, () =>
+            this.refreshCollectionOnce(collection),
+        );
     }
 
     // Back up every in-scope file to `opts.downloadDirectory`, or else the
@@ -832,13 +819,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.
-    //
-    // From a stored cursor of 0, the server lists every live collection. The
-    // refresh then replaces all the store holds, and counts every collection
-    // as new. Only `refreshCollection()` can have stored anything before it.
-    // So the store ends as it would had `refreshCollection()` never run.
     private async refreshOnce(): Promise<void> {
-        const replace = this.store.collectionsSinceTime === 0;
         const page = await this.client.collectionsSince({
             sinceTime: this.store.collectionsSinceTime,
         });
@@ -846,19 +827,16 @@ export class Library {
         // Stage per-collection file diffs. A collection's files are
         // re-enumerated only when its updationTime has advanced past the cached
         // copy; an unchanged album's file list cannot have changed. New
-        // collections enumerate from the beginning of time. When the refresh
-        // replaces the store, every collection counts as new. Each collection
+        // collections enumerate from the beginning of time. Each collection
         // whose files are fetched is reported, with how many there are to
         // fetch.
-        const storedCopy = (id: number): Collection | undefined =>
-            replace ? undefined : this.store.getCollection(id);
         const changedCollections = page.collections.filter((collection) => {
-            const known = storedCopy(collection.id);
+            const known = this.store.getCollection(collection.id);
             return !known || collection.updationTime > known.updationTime;
         });
         const filePages: { collectionID: number; page: FilesPage }[] = [];
         for (const collection of changedCollections) {
-            const known = storedCopy(collection.id);
+            const known = this.store.getCollection(collection.id);
             const filePage = await this.client.filesSince({
                 collectionID: collection.id,
                 collectionKey: collection.key,
@@ -873,23 +851,26 @@ export class Library {
             });
         }
 
-        await this.commit(page, filePages, replace);
+        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. It resolves to
-    // whether the store then holds the collection.
-    private async refreshCollectionOnce(
-        collection: Collection,
-    ): Promise<boolean> {
+    // collection changed since the stored copy, then commit.
+    private async refreshCollectionOnce(collection: Collection): Promise<void> {
         const known = this.store.getCollection(collection.id);
-        // A refresh has received the account's collections, and this one is
-        // not stored. It is new since then, or a refresh dropped it. Leave it
-        // to the next refresh, which adds new collections after the stored
-        // ones, in the server's order.
-        if (!known && this.store.collectionsSinceTime > 0) return false;
         // The stored copy is as new or newer, so there is nothing to fetch.
-        if (known && collection.updationTime <= known.updationTime) return true;
+        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, and no later refresh would see it
+        // again until the server's copy changes.
+        if (
+            !known &&
+            collection.updationTime <= this.store.collectionsSinceTime
+        ) {
+            return;
+        }
         const filePage = await this.client.filesSince({
             collectionID: collection.id,
             collectionKey: collection.key,
@@ -904,22 +885,17 @@ export class Library {
                 cursor: this.store.collectionsSinceTime,
             },
             [{ collectionID: collection.id, page: filePage }],
-            false,
         );
-        return true;
     }
 
     // Commit a collections page and the file pages fetched for its collections
-    // to the store. With `replace`, the page lists every live collection, so
-    // the commit first deletes every stored collection. A collection or file
-    // that does not decrypt is left out, as if deleted. The reason is kept in
-    // the store. Until the server's copy changes, only a refresh from a stored
-    // cursor of 0 fetches it again. Last, save the store when it holds changes
-    // not yet saved.
+    // to the store. 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. Last, save the store when it
+    // holds changes not yet saved.
     private async commit(
         page: CollectionsPage,
         filePages: { collectionID: number; page: FilesPage }[],
-        replace: boolean,
     ): Promise<void> {
         // The network work is done, so change the store now. `changed` says
         // whether this commit changed anything.
@@ -930,13 +906,6 @@ export class Library {
             changed = true;
         }
 
-        if (replace) {
-            for (const { id } of this.store.listCollections()) {
-                this.store.deleteCollection(id);
-                changed = true;
-            }
-        }
-
         for (const id of page.deleted) {
             if (this.store.getCollection(id)) {
                 this.store.deleteCollection(id);

src/library/precache.ts

@@ -122,7 +122,7 @@ export class Precache {
     }
 
     // Recompute the fill orders and the pinned set from the current projection.
-    // Called at open and after every change to the store.
+    // Called at open and after every refresh that changes the library.
     update(records: DerivedRecords): void {
         const photos = [...records.photos.values()].sort(
             (a, b) => b.takenAt - a.takenAt || b.fileID - a.fileID,

src/library/records.ts

@@ -71,14 +71,13 @@ export interface LibrarySnapshot {
     takenAt: number;
 }
 
-// What one refresh or `refreshCollection()` changed in the records.
 export interface LibraryChange {
-    // Full records for albums/photos added or changed.
+    // Full records for albums/photos added or changed by the refresh.
     albumsChanged: AlbumRecord[];
     photosChanged: PhotoRecord[];
     fileIDsRemoved: number[];
     albumIDsRemoved: number[];
-    // Wall-clock milliseconds when the store changed.
+    // Wall-clock milliseconds of the refresh that produced this change.
     refreshedAt: number;
 }
 

src/library/store.ts

@@ -3,7 +3,7 @@
 // The store keeps one `metadata.json` file holding the account's server
 // state: the user id, a schema version, the cursor for the incremental
 // collections listing, the decrypted collection and file records, and the
-// collections and files that did not decrypt. The whole file is read
+// collections and files the refresh could not decrypt. The whole file is read
 // into RAM on load and rewritten as a whole on save; there is no partial
 // update and no lock file. A separate refresh unit populates the store from
 // the server — this module only stores what it is given.
@@ -34,11 +34,9 @@ const FILE_MODE = 0o600;
 type StoredCollection = Omit<Collection, "key"> & { key: string };
 type StoredFile = Omit<EnteFile, "key"> & { key: string };
 
-// A collection, or with `fileID` a file in it, that did not decrypt or parse,
-// and why. A refresh or `refreshCollection()` records it on fetching such a
-// copy. A later fetch drops it on getting a copy that decrypts, or on learning
-// that the server deleted it. Each fetch asks only for what changed, so the
-// store keeps the record between fetches.
+// A collection, or with `fileID` a file in it, that the last refresh to fetch
+// it could not decrypt or parse, and why. It is kept until a later refresh
+// fetches it again, because a refresh fetches only what changed.
 export interface Undecryptable {
     collectionID: number;
     fileID?: number;
@@ -224,14 +222,8 @@ export class MetadataStore {
         this.files.delete(fileKey(collectionID, fileID));
     }
 
-    // Sorted by collection ID, then file ID, so the order does not depend on
-    // when each record was fetched.
     listUndecryptable(): Undecryptable[] {
-        return [...this.undecryptable].sort(
-            (a, b) =>
-                a.collectionID - b.collectionID ||
-                (a.fileID ?? 0) - (b.fileID ?? 0),
-        );
+        return [...this.undecryptable];
     }
 
     // Replaces any earlier record of the same collection or file.

test/cli/commands.test.ts

@@ -124,35 +124,20 @@ const FILES: Record<number, EnteFile[]> = {
 // thumbnail as empty. `withNewFile` adds new.jpg (102) to Vacation, advancing
 // the collection's updationTime as the server does. `beachInWork` adds
 // beach.jpg (100) to Work as well, so that one file is in both albums.
-// `newAlbums` adds the albums A (3) and B (4), in that order, which both hold
-// shared.jpg (300). `refreshError` makes listing collections fail with that
-// message.
+// `refreshError` makes listing collections fail with that message.
 const fakeClient = (
     opts: {
         failID?: number;
         emptyThumbID?: number;
         withNewFile?: boolean;
         beachInWork?: boolean;
-        newAlbums?: boolean;
         refreshError?: string;
     } = {},
 ) => {
-    let collections = opts.withNewFile
+    const collections = opts.withNewFile
         ? [{ ...COLLECTIONS[0], updationTime: 2 }, COLLECTIONS[1]]
         : COLLECTIONS;
     let files = FILES;
-    if (opts.newAlbums) {
-        collections = [
-            ...collections,
-            { ...collection(3, "A"), updationTime: 2 },
-            { ...collection(4, "B"), updationTime: 2 },
-        ];
-        files = {
-            ...files,
-            3: [file(300, 3, "shared.jpg")],
-            4: [file(300, 4, "shared.jpg")],
-        };
-    }
     if (opts.withNewFile) {
         files = {
             ...files,
@@ -584,57 +569,6 @@ describe("files", () => {
         ]);
         expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS);
     });
-
-    // `files 2` on an empty cache stores Work alone. The refresh that `files`
-    // then waits for must still store Vacation first, as the server lists it.
-    it("without a collection ID lists in the server's order after files 2 on an empty cache", async () => {
-        const ctx = context(fakeClient({ beachInWork: true }));
-        expect(await filesCommand(ctx, "2", {})).toBe(0);
-        stdout.text = "";
-        stderr.text = "";
-
-        expect(await filesCommand(ctx, undefined, { json: true })).toBe(0);
-        expect(
-            JSON.parse(stdout.text).map(
-                (row: { id: number; collectionID: number }) => [
-                    row.id,
-                    row.collectionID,
-                ],
-            ),
-        ).toEqual([
-            [100, 1],
-            [101, 1],
-            [200, 2],
-        ]);
-        expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS);
-    });
-
-    // After a refresh, the server adds A and then B. `files 4` must not store B
-    // ahead of A, so it waits for a refresh, which stores both in that order.
-    it("without a collection ID lists new albums in the server's order after files with the second one's ID", async () => {
-        expect(await filesCommand(context(), undefined, {})).toBe(0);
-        const ctx = context(fakeClient({ newAlbums: true }));
-        stdout.text = "";
-
-        expect(await filesCommand(ctx, "4", {})).toBe(0);
-        expect(stdout.text).toBe("300\timage\tshared.jpg\n");
-        stdout.text = "";
-
-        expect(await filesCommand(ctx, undefined, { json: true })).toBe(0);
-        expect(
-            JSON.parse(stdout.text).map(
-                (row: { id: number; collectionID: number }) => [
-                    row.id,
-                    row.collectionID,
-                ],
-            ),
-        ).toEqual([
-            [100, 1],
-            [101, 1],
-            [200, 2],
-            [300, 3],
-        ]);
-    });
 });
 
 describe("get and get-thumb", () => {
@@ -1453,12 +1387,12 @@ describe("backup-metadata --exif", () => {
     });
 });
 
-// Each test first runs `files` so the cache holds the account as it was, then
-// changes the server under it. (`collections` would not do: it reads the
-// collection list alone and stores nothing.)
+// Each test first runs `files 1` so the cache holds Vacation as it was, then
+// adds a file to Vacation on the server. (`collections` would not do: it reads
+// the collection list alone and stores nothing.)
 describe("backup-metadata and the thumbnail helpers refresh first", () => {
     beforeEach(async () => {
-        expect(await filesCommand(context(), undefined, {})).toBe(0);
+        expect(await filesCommand(context(), "1", {})).toBe(0);
         stdout.text = "";
         stderr.text = "";
     });
@@ -1767,8 +1701,8 @@ describe("an account with many albums", () => {
         expect(stderr.text).toBe("");
         expect(paths).toEqual(["/collections/v2?sinceTime=0"]);
 
-        // It stored nothing that would make `refreshCollection()` skip an
-        // album's files: `files` after it still finds the album's photo.
+        // It stored nothing that would make a refresh skip an album's files:
+        // `files` after it still finds the album's photo.
         stdout.text = "";
         expect(await filesCommand(ctx, "7", {})).toBe(0);
         expect(stdout.text).toBe("700\timage\tphoto-7.jpg\n");

test/cli/log.test.ts

@@ -234,8 +234,8 @@ describe("-q", () => {
         });
     });
 
-    // The line saying why the command failed is the fatal one, and it is
-    // printed.
+    // The refresh's progress is left out; the line saying why the command
+    // failed is the fatal one, and it is printed.
     it("prints only the line a failing command exits with", async () => {
         expect(await quak("-q", "files", "9")).toEqual({
             code: 1,

test/cli/read.test.ts

@@ -11,10 +11,8 @@
  * (`freshCollections()`, issue #210). It must not wait for a refresh, which
  * would fetch the files of every collection. `files COLLECTION` reads that list
  * too, then brings that one collection up to date (`refreshCollection()`,
- * issue #217). The stand-in serves a collection's files once `fresh()` has run,
- * or once `refreshCollection()` has resolved to `true` for that collection.
- * When `refreshCollection()` resolves to `false`, `files COLLECTION` forces a
- * refresh instead.
+ * issue #217). The stand-in serves a collection's files once `fresh()` or
+ * `refreshCollection()` for that collection has run.
  *
  * `collections` and `files` also list in the library's enumeration order
  * (`listCollections`/`listFiles`) — the order the pre-library CLI printed — not
@@ -69,18 +67,14 @@ const file = (
 
 // A library that reveals its records only after `fresh()` has been awaited, and
 // serves them in the enumeration order it was given. `freshCollections()`
-// answers the collection list in that order without a refresh. When
-// `refreshCollection()` resolves to true, it reveals the files of that one
-// collection.
+// answers the collection list in that order without a refresh.
+// `refreshCollection()` reveals the files of that one collection.
 // `photos.byID` returns a stand-in `Photo` carrying just the fileID the helper
 // passes through.
 class FakeLibrary implements FreshReadLibrary {
     freshCalls = 0;
     freshCollectionsCalls = 0;
     refreshCollectionIDs: number[] = [];
-    // What `refreshCollection()` resolves to. With false it reveals nothing,
-    // as a library that has had a refresh does for a collection it lacks.
-    refreshCollectionHolds = true;
     private refreshed = false;
 
     constructor(
@@ -99,14 +93,8 @@ class FakeLibrary implements FreshReadLibrary {
         return this.collections;
     }
 
-    async refreshCollection(collection: Collection): Promise<boolean> {
+    async refreshCollection(collection: Collection): Promise<void> {
         this.refreshCollectionIDs.push(collection.id);
-        return this.refreshCollectionHolds;
-    }
-
-    getCollection(id: number): Collection | undefined {
-        if (!this.refreshed) return undefined;
-        return this.collections.find((c) => c.id === id);
     }
 
     listCollections(): Collection[] {
@@ -115,9 +103,7 @@ class FakeLibrary implements FreshReadLibrary {
 
     listFiles(collectionID: number): EnteFile[] {
         const current =
-            this.refreshed ||
-            (this.refreshCollectionHolds &&
-                this.refreshCollectionIDs.includes(collectionID));
+            this.refreshed || this.refreshCollectionIDs.includes(collectionID);
         return current
             ? this.files.filter((f) => f.collectionID === collectionID)
             : [];
@@ -230,19 +216,6 @@ describe("CLI read helpers (issue #36 amendment, issue #52)", () => {
         ]);
     });
 
-    it("freshFiles forces a refresh when refreshCollection resolves to false", async () => {
-        const lib = new FakeLibrary(
-            [collection(1, 100)],
-            [file(10, 1, 1_700_000_000_000_000)],
-        );
-        lib.refreshCollectionHolds = false;
-
-        const rows = await freshFiles(lib, 1);
-        expect(lib.refreshCollectionIDs).toEqual([1]);
-        expect(lib.freshCalls).toBe(1);
-        expect(rows?.map((f) => f.id)).toEqual([10]);
-    });
-
     it("freshFiles returns undefined for an unknown collection", async () => {
         const lib = new FakeLibrary([collection(1, 100)], []);
         const rows = await freshFiles(lib, 999);

test/library/fresh.test.ts

@@ -20,11 +20,9 @@
  *  5. `refreshCollection()` brings one collection up to date in the store
  *     (issue #217). It leaves the stored cursor where it was, so the next
  *     refresh still fetches the files of every other collection that changed.
- *     It never runs at the same time as a refresh. After a refresh, it stores
- *     no collection the store does not hold: not a new one, and not one a
- *     refresh deleted. `close()` waits for it. A library opened on a cache that
- *     only it wrote awaits its first refresh. That refresh replaces what it
- *     stored, in the server's order.
+ *     It never runs at the same time as a refresh, and never brings back a
+ *     collection that a refresh deleted. `close()` waits for it. A library
+ *     opened on a cache that only it wrote awaits its first refresh.
  *
  * 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
@@ -316,10 +314,10 @@ describe("Library.freshCollections", () => {
         rmSync(dir, { recursive: true, force: true });
     });
 
-    // A refresh from a stored cursor past 0 fetches a collection's files only
-    // when the collection is new or its updationTime has advanced past the
-    // stored copy's. So does `refreshCollection()`. Neither would fetch the
-    // files of a collection stored here without them.
+    // A refresh fetches a collection's files only when the collection is new
+    // or its updationTime has advanced past the stored copy's. Had the
+    // collection list been stored, the next refresh would take every
+    // collection for one it already holds the files of, and fetch none.
     it("asks for the collection list alone and stores nothing", async () => {
         const client = new MockClient();
         const page = {
@@ -421,7 +419,7 @@ describe("Library.refreshCollection", () => {
             });
 
             // Collection 1's files alone, since the stored copy.
-            expect(await lib.refreshCollection(collection(1, 110))).toBe(true);
+            await lib.refreshCollection(collection(1, 110));
             expect(client.filesCalls).toEqual([
                 { collectionID: 1, sinceTime: 100 },
             ]);
@@ -503,7 +501,7 @@ describe("Library.refreshCollection", () => {
             await lib.fresh();
             client.filesCalls.length = 0;
 
-            expect(await lib.refreshCollection(listed)).toBe(false);
+            await lib.refreshCollection(listed);
             expect(client.filesCalls).toEqual([]);
             expect(lib.getCollection(1)).toBeUndefined();
         } finally {
@@ -511,21 +509,6 @@ describe("Library.refreshCollection", () => {
         }
     });
 
-    // A refresh has stored collection 1. Collection 2 is new on the server
-    // since then. Only the next refresh may add it, after collection 1.
-    it("stores nothing for a collection that is new since the last refresh", async () => {
-        const { client, lib } = await openSeeded(cacheDirectory);
-        try {
-            client.filesCalls.length = 0;
-
-            expect(await lib.refreshCollection(collection(2, 110))).toBe(false);
-            expect(client.filesCalls).toEqual([]);
-            expect(lib.getCollection(2)).toBeUndefined();
-        } finally {
-            await lib.close();
-        }
-    });
-
     // The caller may remove the cache directory once `close()` resolves, so
     // the cache write must have landed by then.
     it("close() waits for a refreshCollection() in flight", async () => {
@@ -555,13 +538,12 @@ describe("Library.refreshCollection", () => {
         await refreshing;
     });
 
-    // `quak files 2` on an empty cache stores collection 2 alone. A library
-    // opened later on that cache must not serve it as the whole account. Its
-    // first refresh replaces what the cache holds, in the server's order.
+    // `quak files 1` on an empty cache stores collection 1 alone. A library
+    // opened later on that cache must not serve it as the whole account.
     it("open() awaits the first refresh on a cache that only refreshCollection() wrote", async () => {
         const client = new MockClient();
-        client.filesFor(2, {
-            files: [file(2001, 2, 90)],
+        client.filesFor(1, {
+            files: [file(1001, 1, 90)],
             deleted: [],
             cursor: 90,
         });
@@ -571,7 +553,7 @@ describe("Library.refreshCollection", () => {
             refreshOnOpen: false,
             refreshIntervalSeconds: SLOW_INTERVAL,
         });
-        expect(await first.refreshCollection(collection(2, 100))).toBe(true);
+        await first.refreshCollection(collection(1, 100));
         await first.close();
 
         client.collectionsQueue.push({
@@ -579,17 +561,11 @@ describe("Library.refreshCollection", () => {
             deleted: [],
             cursor: 100,
         });
-        client.filesFor(1, {
-            files: [file(1001, 1, 90)],
-            deleted: [],
-            cursor: 90,
-        });
         client.filesFor(2, {
             files: [file(2001, 2, 90)],
             deleted: [],
             cursor: 90,
         });
-        client.filesCalls.length = 0;
         const lib = await Library.open({
             client,
             cacheDirectory,
@@ -598,11 +574,6 @@ describe("Library.refreshCollection", () => {
         try {
             expect(lib.status().lastRefreshAt).toBeDefined();
             expect(lib.listCollections().map((c) => c.id)).toEqual([1, 2]);
-            // It fetches collection 2's files again, from the beginning.
-            expect(client.filesCalls).toEqual([
-                { collectionID: 1, sinceTime: 0 },
-                { collectionID: 2, sinceTime: 0 },
-            ]);
             expect(lib.listFiles(2).map((f) => f.id)).toEqual([2001]);
         } finally {
             await lib.close();

test/library/library.test.ts

@@ -20,9 +20,9 @@
  * 5. `close()` stops the timer and is idempotent, and its promise resolves
  *    only once an in-flight refresh has written the cache file.
  * 6. `cacheDirectory` defaults to the env-paths cache dir plus the user id.
- * 7. `open()` branches on the stored cursor. At 0, as on an empty cache, it
- *    awaits the first refresh. Past 0 it serves the cache's copy at once and
- *    refreshes in the background, so a slow or dead server never stalls
+ * 7. `open()` branches on the cache: an empty cache awaits the first refresh
+ *    (it has nothing to serve yet); an existing cache serves its copy at once
+ *    and refreshes in the background, so a slow or dead server never stalls
  *    opening.
  * 8. A save failure that leaves RAM ahead of disk keeps `status().lastError`
  *    set and keeps retrying the write; a later empty refresh does not clear it.

test/library/snapshot.test.ts

@@ -4,8 +4,7 @@
  * These are the surface the GUI consumes across Electron IPC. `snapshot()` is
  * synchronous — it reads the in-RAM store and projects it into plain records
  * (no keys) — and `subscribe({ onChange })` delivers a `LibraryChange` whenever
- * a refresh or `refreshCollection()` changes the derived records. The
- * contracts:
+ * a background refresh actually changes the derived records. The contracts:
  *
  *  1. `snapshot()` deduplicates a file across memberships into one record with
  *     every album id, orders photos newest first, and carries no key material.

test/library/store.test.ts

@@ -172,7 +172,7 @@ describe("MetadataStore", () => {
         expect(store.listCollections()).toEqual([]);
     });
 
-    it("loads an empty store when the list of what did not decrypt is damaged", async () => {
+    it("loads an empty store when the list of what the refresh could not decrypt is damaged", async () => {
         // A backup reads that list on every run, so a damaged one that loaded
         // would fail every backup until the cache was deleted. A string is
         // damaged too, though JavaScript can loop over its characters.
@@ -266,23 +266,4 @@ describe("MetadataStore", () => {
         expect(store.getFile(12345, 67890)).toBeUndefined();
         expect(store.getFile(55555, 67890)?.key).toEqual(inB.key);
     });
-
-    // A backup reports these in this order. It must not depend on whether a
-    // refresh or `Library.refreshCollection()` fetched each one first.
-    it("lists what did not decrypt by collection ID, then file ID", async () => {
-        const store = await MetadataStore.load(path);
-        store.putUndecryptable({ collectionID: 2, fileID: 7, error: "a" });
-        store.putUndecryptable({ collectionID: 3, error: "b" });
-        store.putUndecryptable({ collectionID: 1, fileID: 9, error: "c" });
-        store.putUndecryptable({ collectionID: 2, fileID: 5, error: "d" });
-
-        expect(
-            store.listUndecryptable().map((r) => [r.collectionID, r.fileID]),
-        ).toEqual([
-            [1, 9],
-            [2, 5],
-            [2, 7],
-            [3, undefined],
-        ]);
-    });
 });