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 first. 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.
Once the stored cursor is past 0, it only updates a collection the store
holds. For a new one, `quak files COLLECTION` waits for a refresh. A
refresh from a cursor of 0 replaces all the store holds, so collections
keep the server's order. `open()` awaits the first refresh while the
cursor is 0. The store lists what did not decrypt in a fixed order.

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
@@ -1226,10 +1226,10 @@ resolves to `false`.
 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

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

@@ -55,8 +55,8 @@ export const freshCollections = (
 // 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
 // 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,
+// date. When `refreshCollection()` stores nothing, 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.
 export const freshFiles = async (

src/library/index.ts

@@ -28,8 +28,7 @@
 // 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
+// So the store keeps collections in the server's order. A caller that asks for
 // each refresh it needs can also open the library with `refreshOnOpen: false`,
 // so that `open()` starts none.
 //
@@ -642,10 +641,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({
@@ -913,9 +912,8 @@ export class Library {
     // 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.
+    // 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 }[],

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;

test/cli/commands.test.ts

@@ -1767,8 +1767,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,9 @@
  * (`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. When `refreshCollection()`
+ * stores nothing, `files COLLECTION` forces a refresh instead.
  *
  * `collections` and `files` also list in the library's enumeration order
  * (`listCollections`/`listFiles`) — the order the pre-library CLI printed — not
@@ -69,9 +68,8 @@ 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 {
@@ -230,7 +228,7 @@ describe("CLI read helpers (issue #36 amendment, issue #52)", () => {
         ]);
     });
 
-    it("freshFiles forces a refresh when refreshCollection resolves to false", async () => {
+    it("freshFiles forces a refresh when refreshCollection stores nothing", async () => {
         const lib = new FakeLibrary(
             [collection(1, 100)],
             [file(10, 1, 1_700_000_000_000_000)],

test/library/fresh.test.ts

@@ -316,10 +316,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 = {

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.