quak get and get-thumb stop fetching collections once they find the file (closes #222)

`quak get` and `quak get-thumb` used to wait for a full refresh, which on
an empty cache fetched the files of every collection. `freshFile` now
reads the collection list, then one collection's files at a time, and
stops at the first that holds the file. It tries first the collections
the cache says hold the file, and stores nothing. The commands fetch the
content through the new `Library.originalOf()` and
`Library.thumbnailOf()`, which take the file record itself, since the
store may not hold it.

They no longer print a refresh's progress lines. The `-q` test that
relied on `get` refreshing now runs `helper fix-missing-thumbnails`.

Model: opus-5-5

test/cli/commands.test.ts

@@ -194,9 +194,6 @@ const FETCHED_BOTH_ALBUMS =
     "Fetched the files of 1 of 2 collections\n" +
     "Fetched the files of 2 of 2 collections\n";
 
-// The same for an account with one album.
-const FETCHED_ONE_ALBUM = "Fetched the files of 1 of 1 collections\n";
-
 // Collects everything written to it.
 class Output {
     text = "";
@@ -576,27 +573,27 @@ describe("get and get-thumb", () => {
         const out = join(root, "diagram.png");
         expect(await getCommand(context(), "200", { out })).toBe(0);
         expect(readFileSync(out)).toEqual(Buffer.alloc(7, 200));
-        expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS + `7 bytes -> ${out}\n`);
+        expect(stderr.text).toBe(`7 bytes -> ${out}\n`);
     });
 
     it("get-thumb finds a file in any album without --collection", async () => {
         const out = join(root, "thumb.jpg");
         expect(await getThumbCommand(context(), "200", { out })).toBe(0);
         expect(readFileSync(out)).toEqual(Buffer.alloc(3, 200));
-        expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS + `3 bytes -> ${out}\n`);
+        expect(stderr.text).toBe(`3 bytes -> ${out}\n`);
     });
 
     it("get exits 1 when no album has the file", async () => {
         const out = join(root, "x");
         expect(await getCommand(context(), "999", { out })).toBe(1);
-        expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS + "File 999 not found\n");
+        expect(stderr.text).toBe("File 999 not found\n");
         expect(existsSync(out)).toBe(false);
     });
 
     it("get-thumb exits 1 when no album has the file", async () => {
         const out = join(root, "x");
         expect(await getThumbCommand(context(), "999", { out })).toBe(1);
-        expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS + "File 999 not found\n");
+        expect(stderr.text).toBe("File 999 not found\n");
         expect(existsSync(out)).toBe(false);
     });
 });
@@ -644,7 +641,7 @@ describe("names from an album name or a title over 255 bytes", () => {
         expect(Buffer.byteLength(name)).toBe(255);
         expect(code).toBe(0);
         expect(readdirSync(dir)).toEqual([name]);
-        expect(stderr.text).toBe(FETCHED_ONE_ALBUM + `7 bytes -> ${name}\n`);
+        expect(stderr.text).toBe(`7 bytes -> ${name}\n`);
     });
 
     it("get-thumb cuts the title short so that thumb_ and it fit in 255 bytes, and exits 0", async () => {
@@ -656,7 +653,7 @@ describe("names from an album name or a title over 255 bytes", () => {
         expect(Buffer.byteLength(name)).toBe(255);
         expect(code).toBe(0);
         expect(readdirSync(dir)).toEqual([name]);
-        expect(stderr.text).toBe(FETCHED_ONE_ALBUM + `3 bytes -> ${name}\n`);
+        expect(stderr.text).toBe(`3 bytes -> ${name}\n`);
     });
 
     it("backup-metadata cuts the album name short so that its folder's name fits in 255 bytes, and exits 0", async () => {
@@ -745,8 +742,7 @@ describe("a live photo", () => {
             Buffer.from(VIDEO),
         );
         expect(stderr.text).toBe(
-            FETCHED_ONE_ALBUM +
-                `${IMAGE.length} bytes -> IMG_0300.heic\n` +
+            `${IMAGE.length} bytes -> IMG_0300.heic\n` +
                 `${VIDEO.length} bytes -> IMG_0300.mov\n`,
         );
     });
@@ -800,8 +796,7 @@ describe("a live photo", () => {
         expect(readFileSync(out)).toEqual(Buffer.from(IMAGE));
         expect(readFileSync(video)).toEqual(Buffer.from(VIDEO));
         expect(stderr.text).toBe(
-            FETCHED_ONE_ALBUM +
-                `${IMAGE.length} bytes -> ${out}\n${VIDEO.length} bytes -> ${video}\n`,
+            `${IMAGE.length} bytes -> ${out}\n${VIDEO.length} bytes -> ${video}\n`,
         );
     });
 
@@ -815,8 +810,7 @@ describe("a live photo", () => {
         expect(existsSync(out)).toBe(false);
         expect(existsSync(join(root, "photo.mov"))).toBe(false);
         expect(stderr.text).toBe(
-            FETCHED_ONE_ALBUM +
-                `File 300 is a live photo, and its video would also be written to ${out}\n`,
+            `File 300 is a live photo, and its video would also be written to ${out}\n`,
         );
     });
 
@@ -1575,13 +1569,14 @@ describe("an ID given on the command line", () => {
     );
 });
 
-// An account with many albums (issues #210 and #217). On an empty cache
-// `collections` and `files` with a collection ID used to wait for a refresh
-// that fetched and decrypted the files of every album, one album after another.
-// On such an account they seemed to hang. The account here has 40 albums of one
-// photo each, encrypted as Ente's clients encrypt them, on the fake Ente server
-// of `test/ente-server.ts`. The tests record the path and query of each
-// request quak sends. The session is the one logging in to that server saves.
+// An account with many albums (issues #210, #217 and #222). On an empty cache
+// `collections`, `files` with a collection ID and `get` used to wait for a
+// refresh that fetched and decrypted the files of every album, one album after
+// another. On such an account they seemed to hang. The account here has 40
+// albums of one photo each, encrypted as Ente's clients encrypt them, on the
+// fake Ente server of `test/ente-server.ts`. The tests record the path and query
+// of each request quak sends. The session is the one logging in to that server
+// saves.
 describe("an account with many albums", () => {
     const ALBUMS = 40;
     const PASSWORD = "many-albums";
@@ -1602,14 +1597,15 @@ describe("an account with many albums", () => {
     // The key of each album, by album ID.
     const albumKeys = new Map<number, Uint8Array>();
 
-    // A photo in album `collectionID` as the server sends it. These tests
-    // fetch no original and no thumbnail.
+    // A photo in album `collectionID` as the server sends it, and its original
+    // encrypted as the server stores it. The original's bytes are the title.
+    // These tests fetch no thumbnail.
     const photoRow = (photo: {
         id: number;
         collectionID: number;
         title: string;
         updationTime: number;
-    }): Record<string, unknown> => {
+    }): { row: Record<string, unknown>; original: Uint8Array } => {
         const photoKey = sodium.crypto_secretstream_xchacha20poly1305_keygen();
         const photoKeyBox = encryptSecretbox(
             photoKey,
@@ -1626,7 +1622,11 @@ describe("an account with many albums", () => {
             ),
             photoKey,
         );
-        return {
+        const original = encryptStreamBlob(
+            new TextEncoder().encode(photo.title),
+            photoKey,
+        );
+        const row = {
             id: photo.id,
             collectionID: photo.collectionID,
             ownerID: USER_ID,
@@ -1636,10 +1636,11 @@ describe("an account with many albums", () => {
                 encryptedData: toBase64(metadata.ciphertext),
                 decryptionHeader: toBase64(metadata.header),
             },
-            file: { decryptionHeader: "" },
+            file: { decryptionHeader: toBase64(original.header) },
             thumbnail: { decryptionHeader: "" },
             updationTime: photo.updationTime,
         };
+        return { row, original: original.ciphertext };
     };
 
     // Album n is named `Album n` and holds one photo, n * 100, titled
@@ -1649,6 +1650,7 @@ describe("an account with many albums", () => {
         const account = await buildAccount(PASSWORD);
         const collections: Record<string, unknown>[] = [];
         const filesByCollection: Record<number, Record<string, unknown>[]> = {};
+        const fileCiphertexts: Record<number, Uint8Array> = {};
         for (let id = 1; id <= ALBUMS; id++) {
             const albumKey = sodium.crypto_secretbox_keygen();
             albumKeys.set(id, albumKey);
@@ -1667,14 +1669,14 @@ describe("an account with many albums", () => {
                 type: "album",
                 updationTime: 1,
             });
-            filesByCollection[id] = [
-                photoRow({
-                    id: id * 100,
-                    collectionID: id,
-                    title: `photo-${id}.jpg`,
-                    updationTime: 1,
-                }),
-            ];
+            const { row, original } = photoRow({
+                id: id * 100,
+                collectionID: id,
+                title: `photo-${id}.jpg`,
+                updationTime: 1,
+            });
+            filesByCollection[id] = [row];
+            fileCiphertexts[id * 100] = original;
         }
         const ente = buildMetaFetch({
             verifier: account.verifier,
@@ -1684,7 +1686,7 @@ describe("an account with many albums", () => {
             collections,
             filesByCollection,
             encryptedMLData: {},
-            fileCiphertexts: {},
+            fileCiphertexts,
         });
         server = (async (input: RequestInfo | URL, init?: RequestInit) => {
             const url =
@@ -1758,13 +1760,13 @@ describe("an account with many albums", () => {
                 collectionID: 7,
                 title: "photo-7.jpg",
                 updationTime: 1,
-            }),
+            }).row,
             photoRow({
                 id: 701,
                 collectionID: 7,
                 title: "photo-701.jpg",
                 updationTime: 1,
-            }),
+            }).row,
         ];
         const secondPage = [
             photoRow({
@@ -1772,7 +1774,7 @@ describe("an account with many albums", () => {
                 collectionID: 7,
                 title: "photo-7-edited.jpg",
                 updationTime: 2,
-            }),
+            }).row,
         ];
         const paged = (async (input: RequestInfo | URL, init?: RequestInit) => {
             const url = new URL(input instanceof Request ? input.url : input);
@@ -1812,6 +1814,43 @@ describe("an account with many albums", () => {
         expect(stderr.text).toBe(progress);
     });
 
+    // Photo 200 is in album 2, so `get` asks for no file of album 3 or later.
+    // The last request downloads the photo's original.
+    it("get asks for the albums' files only until one holds the photo", async () => {
+        const out = join(root, "photo.jpg");
+        expect(await getCommand(accountContext(), "200", { out })).toBe(0);
+
+        expect(readFileSync(out, "utf-8")).toBe("photo-2.jpg");
+        expect(stderr.text).toBe(`11 bytes -> ${out}\n`);
+        expect(requests).toEqual([
+            "/collections/v2?sinceTime=0",
+            "/collections/v2/diff?collectionID=1&sinceTime=0",
+            "/collections/v2/diff?collectionID=2&sinceTime=0",
+            "/?fileID=200",
+        ]);
+        expect(existsSync(join(root, "cache", "metadata.json"))).toBe(false);
+    });
+
+    // `files` first fills the cache. The cache then says album 40, the last,
+    // holds photo 4000, so `get` asks for that album's files alone.
+    it("get asks first for the files of the album the cache says holds the photo", async () => {
+        const ctx = accountContext();
+        expect(await filesCommand(ctx, undefined, {})).toBe(0);
+        requests.length = 0;
+        stderr.text = "";
+
+        const out = join(root, "photo.jpg");
+        expect(await getCommand(ctx, "4000", { out })).toBe(0);
+
+        expect(readFileSync(out, "utf-8")).toBe("photo-40.jpg");
+        expect(stderr.text).toBe(`12 bytes -> ${out}\n`);
+        expect(requests).toEqual([
+            "/collections/v2?sinceTime=0",
+            "/collections/v2/diff?collectionID=40&sinceTime=0",
+            "/?fileID=4000",
+        ]);
+    });
+
     it("files counts on stderr each album whose files its refresh fetched", async () => {
         expect(await filesCommand(accountContext(), undefined, {})).toBe(0);
 

test/cli/log.test.ts

@@ -234,13 +234,15 @@ describe("-q", () => {
         });
     });
 
-    // The refresh's progress is left out; the line saying why the command
+    // No album holds file 999, so fixing it fails. The refresh's progress and
+    // the list of failed files are 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", "get", "999")).toEqual({
+        const fix = ["helper", "fix-missing-thumbnails", "--file", "999"];
+        expect(await quak("-q", ...fix)).toEqual({
             code: 1,
             stdout: "",
-            stderr: "File 999 not found\n",
+            stderr: `quak: 1 file(s) failed; see the "Failed files:" list, or the JSON with --json\n`,
         });
     });
 

test/cli/read.test.ts

@@ -3,15 +3,16 @@
  * issue #36, issue #52).
  *
  * The `collections`, `files`, `get`, and `get-thumb` commands must answer for
- * current server state, not the local cache. `get`, `get-thumb` and `files`
- * without a collection ID force a `Library.fresh()` round-trip before they
- * read. The stand-in library below serves no cached record until `fresh()` has
- * been awaited, so a helper that read without refreshing would come back empty
- * and fail here. `collections` asks the library for the collection list alone
+ * current server state, not the local cache. `files` without a collection ID
+ * forces a `Library.fresh()` round-trip before it reads. The stand-in library
+ * below serves no cached record until `fresh()` has been awaited, so a helper
+ * that read without refreshing would come back empty and fail here.
+ * `collections` asks the library for the collection list alone
  * (`freshCollections()`, issue #210). `files` with a collection ID asks for
  * that list, then for that one collection's files (`freshFiles()`, issue
- * #217). Neither may wait for a refresh, which would fetch the files of every
- * collection.
+ * #217). `get` and `get-thumb` ask for that list, then for one collection's
+ * files at a time, until they find the file (issue #222). None of these may
+ * wait for a refresh, which would fetch the files of every collection.
  *
  * `collections` and `files` also list in the order the library gives them, not
  * the albums/photos projection's newest-first order. The fixtures are seeded in
@@ -29,7 +30,6 @@ import {
     type FreshReadLibrary,
 } from "../../src/cli-read.js";
 import { fileListRow } from "../../src/cli-output.js";
-import type { Photo } from "../../src/library/index.js";
 import type { Collection, EnteFile } from "../../src/model/types.js";
 
 const collection = (id: number, updationTime: number): Collection => ({
@@ -66,8 +66,7 @@ const file = (
 // A library that reveals its cached records only after `fresh()` has been
 // awaited, and serves them in the enumeration order it was given.
 // `freshCollections()` and `freshFiles()` answer in that order without a
-// refresh. `photos.byID` returns a stand-in `Photo` carrying just the fileID
-// the helper passes through.
+// refresh.
 class FakeLibrary implements FreshReadLibrary {
     freshCalls = 0;
     freshCollectionsCalls = 0;
@@ -106,18 +105,9 @@ class FakeLibrary implements FreshReadLibrary {
             : [];
     }
 
-    getFileByID(fileID: number): EnteFile | undefined {
-        if (!this.refreshed) return undefined;
-        return this.files.find((f) => f.id === fileID);
+    getFile(collectionID: number, fileID: number): EnteFile | undefined {
+        return this.listFiles(collectionID).find((f) => f.id === fileID);
     }
-
-    photos = {
-        byID: ({ fileID }: { fileID: number }): Photo | undefined => {
-            if (!this.refreshed) return undefined;
-            if (!this.files.some((f) => f.id === fileID)) return undefined;
-            return { fileID } as unknown as Photo;
-        },
-    };
 }
 
 describe("CLI read helpers (issue #36 amendment, issue #52)", () => {
@@ -220,23 +210,44 @@ describe("CLI read helpers (issue #36 amendment, issue #52)", () => {
         expect(rows).toBeUndefined();
     });
 
-    it("freshFile refreshes first, then resolves the photo and its raw record", async () => {
-        const f = file(10, 1, 1_700_000_000_000_000);
-        const lib = new FakeLibrary([collection(1, 100)], [f]);
-
-        const resolved = await freshFile(lib, 10);
-        expect(lib.freshCalls).toBe(1);
-        expect(resolved?.photo.fileID).toBe(10);
-        expect(resolved?.file.metadata.title).toBe("file-10.jpg");
-        expect(resolved?.file.metadata.creationTime).toBe(
-            1_700_000_000_000_000,
+    // Three collections, each holding one file.
+    const threeCollections = (): FakeLibrary =>
+        new FakeLibrary(
+            [collection(1, 100), collection(2, 200), collection(3, 300)],
+            [
+                file(10, 1, 1_700_000_000_000_000),
+                file(20, 2, 1_700_000_000_000_001),
+                file(30, 3, 1_700_000_000_000_002),
+            ],
         );
+
+    it("freshFile reads each collection's files in the server's order until one holds the file", async () => {
+        const lib = threeCollections();
+
+        const found = await freshFile(lib, 20);
+        expect(lib.freshCalls).toBe(0);
+        expect(lib.freshCollectionsCalls).toBe(1);
+        expect(lib.freshFilesCalls).toEqual([1, 2]);
+        expect(found?.metadata.title).toBe("file-20.jpg");
+        expect(found?.metadata.creationTime).toBe(1_700_000_000_000_001);
     });
 
-    it("freshFile returns undefined for an unknown file", async () => {
-        const lib = new FakeLibrary([collection(1, 100)], []);
-        const resolved = await freshFile(lib, 404);
-        expect(lib.freshCalls).toBe(1);
-        expect(resolved).toBeUndefined();
+    // The stand-in's cache holds what the server holds once `fresh()` has run.
+    it("freshFile reads first the files of the collection the cache says holds the file", async () => {
+        const lib = threeCollections();
+        await lib.fresh();
+
+        const found = await freshFile(lib, 30);
+        expect(lib.freshFilesCalls).toEqual([3]);
+        expect(found?.collectionID).toBe(3);
+    });
+
+    it("freshFile returns undefined for an unknown file, after reading every collection's files", async () => {
+        const lib = threeCollections();
+
+        const found = await freshFile(lib, 404);
+        expect(lib.freshCalls).toBe(0);
+        expect(lib.freshFilesCalls).toEqual([1, 2, 3]);
+        expect(found).toBeUndefined();
     });
 });