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

`quak files` with a collection ID used to wait for a full refresh, which
on an empty cache fetched the files of every collection in the account.
It now reads the collection list, then that one collection's files
through the new `Library.freshFiles()`, and stores nothing. The files
print in the server's order, and no refresh progress lines appear. Each
file prints once, as the latest copy the server sent. `quak files`
without a collection ID still waits for a refresh.

Some tests ran `quak files` with a collection ID to fill the cache, to
show a refresh's progress or to show what `-q` hides. They now run a
command that still refreshes.

Model: opus-5-5

test/cli/commands.test.ts

@@ -535,9 +535,7 @@ describe("files", () => {
 
     it("exits 1 for an unknown collection", async () => {
         expect(await filesCommand(context(), "9", {})).toBe(1);
-        expect(stderr.text).toBe(
-            FETCHED_BOTH_ALBUMS + "Collection 9 not found\n",
-        );
+        expect(stderr.text).toBe("Collection 9 not found\n");
     });
 
     // Without a collection ID it lists every file in the account once (issue
@@ -1390,11 +1388,11 @@ 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.)
+// changes the server under it. (`collections` and `files` with a collection ID
+// would not do: they store nothing.)
 describe("backup-metadata and the thumbnail helpers refresh first", () => {
     beforeEach(async () => {
-        expect(await filesCommand(context(), "1", {})).toBe(0);
+        expect(await filesCommand(context(), undefined, {})).toBe(0);
         stdout.text = "";
         stderr.text = "";
     });
@@ -1577,20 +1575,73 @@ describe("an ID given on the command line", () => {
     );
 });
 
-// An account with many albums (issue #210). On an empty cache `collections`
-// used to wait for a refresh that fetched and decrypted the files of every
-// album, one album after another, before it printed anything, so on such an
-// account it 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`, which also records the path of each request quak
-// sends. The session is the one logging in to that server saves.
+// 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.
 describe("an account with many albums", () => {
     const ALBUMS = 40;
     const PASSWORD = "many-albums";
-    const paths: string[] = [];
+    const requests: string[] = [];
     let server: typeof globalThis.fetch;
     let snapshot: ClientSnapshot;
 
+    // What `files` without a collection ID prints on an empty cache: every
+    // album's photo on stdout, and on stderr each album whose files its
+    // refresh fetched.
+    let everyPhoto = "";
+    let progress = "";
+    for (let n = 1; n <= ALBUMS; n++) {
+        everyPhoto += `${n * 100}\timage\tphoto-${n}.jpg\n`;
+        progress += `Fetched the files of ${n} of ${ALBUMS} collections\n`;
+    }
+
+    // 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.
+    const photoRow = (photo: {
+        id: number;
+        collectionID: number;
+        title: string;
+        updationTime: number;
+    }): Record<string, unknown> => {
+        const photoKey = sodium.crypto_secretstream_xchacha20poly1305_keygen();
+        const photoKeyBox = encryptSecretbox(
+            photoKey,
+            albumKeys.get(photo.collectionID)!,
+        );
+        const metadata = encryptStreamBlob(
+            new TextEncoder().encode(
+                JSON.stringify({
+                    title: photo.title,
+                    fileType: 0,
+                    creationTime: 1000,
+                    modificationTime: 1000,
+                }),
+            ),
+            photoKey,
+        );
+        return {
+            id: photo.id,
+            collectionID: photo.collectionID,
+            ownerID: USER_ID,
+            encryptedKey: toBase64(photoKeyBox.ciphertext),
+            keyDecryptionNonce: toBase64(photoKeyBox.nonce),
+            metadata: {
+                encryptedData: toBase64(metadata.ciphertext),
+                decryptionHeader: toBase64(metadata.header),
+            },
+            file: { decryptionHeader: "" },
+            thumbnail: { decryptionHeader: "" },
+            updationTime: photo.updationTime,
+        };
+    };
+
     // Album n is named `Album n` and holds one photo, n * 100, titled
     // `photo-n.jpg`.
     beforeAll(async () => {
@@ -1600,6 +1651,7 @@ describe("an account with many albums", () => {
         const filesByCollection: Record<number, Record<string, unknown>[]> = {};
         for (let id = 1; id <= ALBUMS; id++) {
             const albumKey = sodium.crypto_secretbox_keygen();
+            albumKeys.set(id, albumKey);
             const albumKeyBox = encryptSecretbox(albumKey, account.masterKey);
             const name = encryptSecretbox(
                 new TextEncoder().encode(`Album ${id}`),
@@ -1615,36 +1667,13 @@ describe("an account with many albums", () => {
                 type: "album",
                 updationTime: 1,
             });
-            const photoKey =
-                sodium.crypto_secretstream_xchacha20poly1305_keygen();
-            const photoKeyBox = encryptSecretbox(photoKey, albumKey);
-            const metadata = encryptStreamBlob(
-                new TextEncoder().encode(
-                    JSON.stringify({
-                        title: `photo-${id}.jpg`,
-                        fileType: 0,
-                        creationTime: 1000,
-                        modificationTime: 1000,
-                    }),
-                ),
-                photoKey,
-            );
             filesByCollection[id] = [
-                {
+                photoRow({
                     id: id * 100,
                     collectionID: id,
-                    ownerID: USER_ID,
-                    encryptedKey: toBase64(photoKeyBox.ciphertext),
-                    keyDecryptionNonce: toBase64(photoKeyBox.nonce),
-                    metadata: {
-                        encryptedData: toBase64(metadata.ciphertext),
-                        decryptionHeader: toBase64(metadata.header),
-                    },
-                    // These tests fetch no original and no thumbnail.
-                    file: { decryptionHeader: "" },
-                    thumbnail: { decryptionHeader: "" },
+                    title: `photo-${id}.jpg`,
                     updationTime: 1,
-                },
+                }),
             ];
         }
         const ente = buildMetaFetch({
@@ -1664,7 +1693,8 @@ describe("an account with many albums", () => {
                     : input instanceof URL
                       ? input.href
                       : input.url;
-            paths.push(new URL(url).pathname);
+            const { pathname, search } = new URL(url);
+            requests.push(pathname + search);
             return ente(input, init);
         }) as typeof globalThis.fetch;
         const client = await Client.login({
@@ -1676,16 +1706,17 @@ describe("an account with many albums", () => {
     });
 
     beforeEach(() => {
-        paths.length = 0;
+        requests.length = 0;
     });
 
-    // A context whose saved session sends its requests to the fake server.
-    const accountContext = (): CliContext => {
+    // A context whose saved session sends its requests to `fetch`, by default
+    // the fake server.
+    const accountContext = (fetch = server): CliContext => {
         const ctx = context();
         saveSession(ctx.sessionDir, snapshot);
         return {
             ...ctx,
-            loadSession: (path) => loadSession(path, { fetch: server }),
+            loadSession: (path) => loadSession(path, { fetch }),
         };
     };
 
@@ -1698,46 +1729,103 @@ describe("an account with many albums", () => {
             albums += `${id}\talbum\tAlbum ${id}\n`;
         expect(stdout.text).toBe(albums);
         expect(stderr.text).toBe("");
-        expect(paths).toEqual(["/collections/v2"]);
+        expect(requests).toEqual(["/collections/v2?sinceTime=0"]);
 
         // It stored nothing that would make a refresh skip an album's files:
-        // `files` after it still finds the album's photo.
+        // `files` after it still lists every album's photo.
         stdout.text = "";
-        expect(await filesCommand(ctx, "7", {})).toBe(0);
-        expect(stdout.text).toBe("700\timage\tphoto-7.jpg\n");
+        expect(await filesCommand(ctx, undefined, {})).toBe(0);
+        expect(stdout.text).toBe(everyPhoto);
     });
 
-    it("files counts on stderr each album whose files its refresh fetched", async () => {
+    it("files with a collection ID asks for the album list and that album's files alone", async () => {
         expect(await filesCommand(accountContext(), "7", {})).toBe(0);
 
         expect(stdout.text).toBe("700\timage\tphoto-7.jpg\n");
-        let progress = "";
-        for (let n = 1; n <= ALBUMS; n++) {
-            progress += `Fetched the files of ${n} of ${ALBUMS} collections\n`;
-        }
+        expect(stderr.text).toBe("");
+        expect(requests).toEqual([
+            "/collections/v2?sinceTime=0",
+            "/collections/v2/diff?collectionID=7&sinceTime=0",
+        ]);
+    });
+
+    // Here the server sends album 7's files in two pages. Photo 700 changed
+    // between them, so both pages hold a copy of it.
+    it("files with a collection ID lists only the later copy of a photo sent in two pages", async () => {
+        const firstPage = [
+            photoRow({
+                id: 700,
+                collectionID: 7,
+                title: "photo-7.jpg",
+                updationTime: 1,
+            }),
+            photoRow({
+                id: 701,
+                collectionID: 7,
+                title: "photo-701.jpg",
+                updationTime: 1,
+            }),
+        ];
+        const secondPage = [
+            photoRow({
+                id: 700,
+                collectionID: 7,
+                title: "photo-7-edited.jpg",
+                updationTime: 2,
+            }),
+        ];
+        const paged = (async (input: RequestInfo | URL, init?: RequestInit) => {
+            const url = new URL(input instanceof Request ? input.url : input);
+            if (
+                url.pathname !== "/collections/v2/diff" ||
+                url.searchParams.get("collectionID") !== "7"
+            ) {
+                return server(input, init);
+            }
+            const first = url.searchParams.get("sinceTime") === "0";
+            return new Response(
+                JSON.stringify({
+                    diff: first ? firstPage : secondPage,
+                    hasMore: first,
+                }),
+            );
+        }) as typeof globalThis.fetch;
+
+        expect(await filesCommand(accountContext(paged), "7", {})).toBe(0);
+
+        // Photo 700 is listed once, with the later title, before photo 701.
+        expect(stdout.text).toBe(
+            "700\timage\tphoto-7-edited.jpg\n701\timage\tphoto-701.jpg\n",
+        );
+    });
+
+    // The cache stays empty, so a later `files` without a collection ID still
+    // fetches the files of all 40 albums.
+    it("files with a collection ID stores nothing in the cache", async () => {
+        const ctx = accountContext();
+        expect(await filesCommand(ctx, "7", {})).toBe(0);
+        expect(existsSync(join(root, "cache", "metadata.json"))).toBe(false);
+
+        stdout.text = "";
+        expect(await filesCommand(ctx, undefined, {})).toBe(0);
+        expect(stdout.text).toBe(everyPhoto);
+        expect(stderr.text).toBe(progress);
+    });
+
+    it("files counts on stderr each album whose files its refresh fetched", async () => {
+        expect(await filesCommand(accountContext(), undefined, {})).toBe(0);
+
+        expect(stdout.text).toBe(everyPhoto);
         expect(stderr.text).toBe(progress);
     });
 
     // `--json` shapes stdout only, so the refresh's progress is still counted
     // on stderr.
     it("files --json counts on stderr each album whose files its refresh fetched", async () => {
-        expect(await filesCommand(accountContext(), "7", { json: true })).toBe(
-            0,
-        );
+        const opts = { json: true };
+        expect(await filesCommand(accountContext(), undefined, opts)).toBe(0);
 
-        expect(JSON.parse(stdout.text)).toEqual([
-            {
-                id: 700,
-                title: "photo-7.jpg",
-                fileType: "image",
-                creationTime: 1000,
-                collectionID: 7,
-            },
-        ]);
-        let progress = "";
-        for (let n = 1; n <= ALBUMS; n++) {
-            progress += `Fetched the files of ${n} of ${ALBUMS} collections\n`;
-        }
+        expect(JSON.parse(stdout.text)).toHaveLength(ALBUMS);
         expect(stderr.text).toBe(progress);
     });
 });

test/cli/log.test.ts

@@ -237,10 +237,10 @@ describe("-q", () => {
     // 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({
+        expect(await quak("-q", "get", "999")).toEqual({
             code: 1,
             stdout: "",
-            stderr: "Collection 9 not found\n",
+            stderr: "File 999 not found\n",
         });
     });
 

test/cli/read.test.ts

@@ -3,20 +3,21 @@
  * issue #36, issue #52).
  *
  * The `collections`, `files`, `get`, and `get-thumb` commands must answer for
- * current server state, not the local cache. `files`, `get` and `get-thumb`
- * force a `Library.fresh()` round-trip before they read; the stand-in library
- * below serves nothing 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), and must not wait for a refresh, which would fetch the files of every
+ * 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
+ * (`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.
  *
- * `collections` and `files` also list in the library's enumeration order
- * (`listCollections`/`listFiles`) — the order the pre-library CLI printed — not
+ * `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
- * an enumeration order that a newest-first sort would rearrange, so a
- * regression to the projection order would fail here too. Field values still
- * come from the raw metadata via `cli-output.ts`.
+ * an order that a newest-first sort would rearrange, so a regression to the
+ * projection order would fail here too. Field values still come from the raw
+ * metadata via `cli-output.ts`.
  */
 
 import { describe, it, expect } from "vitest";
@@ -62,14 +63,16 @@ const file = (
     updationTime: creationTime,
 });
 
-// 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. `photos.byID`
-// returns a stand-in `Photo` carrying just the fileID the helper passes
-// through.
+// 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.
 class FakeLibrary implements FreshReadLibrary {
     freshCalls = 0;
     freshCollectionsCalls = 0;
+    // The ID of each collection `freshFiles()` was asked for.
+    freshFilesCalls: number[] = [];
     private refreshed = false;
 
     constructor(
@@ -88,9 +91,9 @@ class FakeLibrary implements FreshReadLibrary {
         return this.collections;
     }
 
-    getCollection(id: number): Collection | undefined {
-        if (!this.refreshed) return undefined;
-        return this.collections.find((c) => c.id === id);
+    async freshFiles(collection: Collection): Promise<EnteFile[]> {
+        this.freshFilesCalls.push(collection.id);
+        return this.files.filter((f) => f.collectionID === collection.id);
     }
 
     listCollections(): Collection[] {
@@ -138,18 +141,24 @@ describe("CLI read helpers (issue #36 amendment, issue #52)", () => {
         expect(rows.map((c) => c.id)).not.toEqual(newestFirst);
     });
 
-    it("freshFiles refreshes first, lists in enumeration order, keeps raw fields", async () => {
-        // Enumeration order by id 10, 11, 12; creationTimes ascending, so a
+    it("freshFiles with a collection ID reads that collection's files alone and keeps their order and raw fields", async () => {
+        // Given in the order 10, 11, 12; creationTimes ascending, so a
         // newest-first sort would reverse them.
         const files = [
             file(10, 1, 1_700_000_000_000_000),
             file(11, 1, 1_700_000_000_000_001),
             file(12, 1, 1_700_000_000_000_002),
+            file(20, 2, 1_700_000_000_000_003),
         ];
-        const lib = new FakeLibrary([collection(1, 100)], files);
+        const lib = new FakeLibrary(
+            [collection(1, 100), collection(2, 200)],
+            files,
+        );
 
         const rows = await freshFiles(lib, 1);
-        expect(lib.freshCalls).toBe(1);
+        expect(lib.freshCalls).toBe(0);
+        expect(lib.freshCollectionsCalls).toBe(1);
+        expect(lib.freshFilesCalls).toEqual([1]);
         expect(rows?.map((f) => f.id)).toEqual([10, 11, 12]);
 
         // Field values come from raw metadata: microsecond creationTime and the
@@ -206,7 +215,8 @@ describe("CLI read helpers (issue #36 amendment, issue #52)", () => {
     it("freshFiles returns undefined for an unknown collection", async () => {
         const lib = new FakeLibrary([collection(1, 100)], []);
         const rows = await freshFiles(lib, 999);
-        expect(lib.freshCalls).toBe(1);
+        expect(lib.freshCalls).toBe(0);
+        expect(lib.freshFilesCalls).toEqual([]);
         expect(rows).toBeUndefined();
     });