quak files takes the collection ID as an optional argument (closes #211)

Mandatory arguments are positional, so `quak files` takes the collection
ID as an optional argument and the `--collection` option is removed.
With a collection ID it lists that collection's files. Without one it
lists every file in the account, collection by collection, each file
once: the library keeps a record of a file for each collection that
holds it, and only the first collection's record is listed, so that
collection is its `collectionID` in `--json`. The `--help` text, the
README and `TODO.md` say so.

Model: opus-5-5

test/cli/backup-undecryptable.test.ts

@@ -548,7 +548,7 @@ describe("the read commands with records quak cannot decrypt", () => {
         expect(await collectionsCommand(context(ente), {})).toBe(0);
         expect(stdout.text).toBe("1\talbum\tTrip\n");
 
-        expect(await filesCommand(context(ente), { collection: "1" })).toBe(0);
+        expect(await filesCommand(context(ente), "1", {})).toBe(0);
         expect(stdout.text).toBe("10\timage\tphoto-10.jpg\n");
     });
 });

test/cli/bin.test.ts

@@ -1,6 +1,7 @@
 /**
- * Tests for the retry options `bin/quak.ts` loads the saved session with:
- * `quak backup` gets the unattended ones, every other command the default.
+ * Tests for the retry options `bin/quak.ts` loads the saved session with
+ * (`quak backup` gets the unattended ones, every other command the default),
+ * and for `quak files` taking its collection ID as an optional argument.
  *
  * Each test runs `bin/quak.ts`, as the smoke test does, with its session loader
  * replaced by one that records the options it is given and reports no session,
@@ -26,14 +27,19 @@ vi.mock("../../src/cli-session.js", () => ({
 const argv = process.argv;
 const dir = mkdtempSync(join(tmpdir(), "quak-bin-test-"));
 
-// Run `quak <args>` to completion. The "Not logged in" message and the exit
-// are swallowed.
-const quak = async (...args: string[]): Promise<void> => {
-    vi.spyOn(process.stderr, "write").mockImplementation(() => true);
+// Run `quak <args>` to completion and return what it wrote on stderr, which is
+// kept off the terminal. The exit is swallowed.
+const quak = async (...args: string[]): Promise<string> => {
+    let written = "";
+    vi.spyOn(process.stderr, "write").mockImplementation((text) => {
+        written += String(text);
+        return true;
+    });
     vi.spyOn(process, "exit").mockImplementation(() => undefined as never);
     process.argv = ["node", "quak", ...args];
     vi.resetModules();
     await import("../../bin/quak.js");
+    return written;
 };
 
 afterEach(() => {
@@ -69,3 +75,15 @@ describe("bin/quak.ts session loading", () => {
         expect(loaded).toEqual([undefined]);
     });
 });
+
+// The command line takes `files` with or without a collection ID, and the
+// command runs as far as its session check either way (issue #211).
+describe("bin/quak.ts files", () => {
+    it("runs without a collection ID", async () => {
+        expect(await quak("files")).toMatch(/^Not logged in/);
+    });
+
+    it("runs with a collection ID", async () => {
+        expect(await quak("files", "7")).toMatch(/^Not logged in/);
+    });
+});

test/cli/commands.test.ts

@@ -121,25 +121,31 @@ const FILES: Record<number, EnteFile[]> = {
 // An original is 7 bytes and a thumbnail 3. `failID` makes that file's
 // original fail; `emptyThumbID` makes the server report that file's
 // thumbnail as empty. `withNewFile` adds new.jpg (102) to Vacation, advancing
-// the collection's updationTime as the server does, and `refreshError` makes
-// listing collections fail with that message.
+// 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.
+// `refreshError` makes listing collections fail with that message.
 const fakeClient = (
     opts: {
         failID?: number;
         emptyThumbID?: number;
         withNewFile?: boolean;
+        beachInWork?: boolean;
         refreshError?: string;
     } = {},
 ) => {
     const collections = opts.withNewFile
         ? [{ ...COLLECTIONS[0], updationTime: 2 }, COLLECTIONS[1]]
         : COLLECTIONS;
-    const files = opts.withNewFile
-        ? {
-              ...FILES,
-              1: [...FILES[1], { ...file(102, 1, "new.jpg"), updationTime: 2 }],
-          }
-        : FILES;
+    let files = FILES;
+    if (opts.withNewFile) {
+        files = {
+            ...files,
+            1: [...files[1], { ...file(102, 1, "new.jpg"), updationTime: 2 }],
+        };
+    }
+    if (opts.beachInWork) {
+        files = { ...files, 2: [...files[2], file(100, 2, "beach.jpg")] };
+    }
     const source: ContentSource = {
         original: async ({ file: f, destination }) => {
             if (f.id === opts.failID) throw new Error("HTTP 500 from server");
@@ -257,7 +263,7 @@ describe("session file", () => {
         const dir = join(root, "backup");
         expect(await whoamiCommand(ctx)).toBe(3);
         expect(await collectionsCommand(ctx, {})).toBe(3);
-        expect(await filesCommand(ctx, { collection: "1" })).toBe(3);
+        expect(await filesCommand(ctx, "1", {})).toBe(3);
         expect(await getCommand(ctx, "100", {})).toBe(3);
         expect(await getThumbCommand(ctx, "100", {})).toBe(3);
         expect(await backupMetadataCommand(ctx, dir, {})).toBe(3);
@@ -504,17 +510,14 @@ describe("collections", () => {
 
 describe("files", () => {
     it("prints one tab-separated line per file", async () => {
-        expect(await filesCommand(context(), { collection: "1" })).toBe(0);
+        expect(await filesCommand(context(), "1", {})).toBe(0);
         expect(stdout.text).toBe(
             "100\timage\tbeach.jpg\n" + "101\timage\tsunset.jpg\n",
         );
     });
 
     it("prints a JSON array with --json", async () => {
-        const code = await filesCommand(context(), {
-            collection: "2",
-            json: true,
-        });
+        const code = await filesCommand(context(), "2", { json: true });
         expect(code).toBe(0);
         expect(JSON.parse(stdout.text)).toEqual([
             {
@@ -528,11 +531,43 @@ describe("files", () => {
     });
 
     it("exits 1 for an unknown collection", async () => {
-        expect(await filesCommand(context(), { collection: "9" })).toBe(1);
+        expect(await filesCommand(context(), "9", {})).toBe(1);
         expect(stderr.text).toBe(
             FETCHED_BOTH_ALBUMS + "Collection 9 not found\n",
         );
     });
+
+    // Without a collection ID it lists every file in the account once (issue
+    // #211). beach.jpg is in Vacation and Work, and Vacation comes first, so it
+    // is listed once, as Vacation's.
+    it("without a collection ID prints every file once", async () => {
+        const ctx = context(fakeClient({ beachInWork: true }));
+        expect(await filesCommand(ctx, undefined, {})).toBe(0);
+        expect(stdout.text).toBe(
+            "100\timage\tbeach.jpg\n" +
+                "101\timage\tsunset.jpg\n" +
+                "200\timage\tdiagram.png\n",
+        );
+        expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS);
+    });
+
+    it("without a collection ID gives each file the first album holding it with --json", async () => {
+        const ctx = context(fakeClient({ beachInWork: true }));
+        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("");
+    });
 });
 
 describe("get and get-thumb", () => {
@@ -1265,7 +1300,7 @@ describe("backup-metadata --exif", () => {
 // collection list alone and stores nothing.)
 describe("backup-metadata and the thumbnail helpers refresh first", () => {
     beforeEach(async () => {
-        expect(await filesCommand(context(), { collection: "1" })).toBe(0);
+        expect(await filesCommand(context(), "1", {})).toBe(0);
         stdout.text = "";
         stderr.text = "";
     });
@@ -1324,10 +1359,10 @@ describe("backup-metadata and the thumbnail helpers refresh first", () => {
     });
 });
 
-// `files --collection`, `get`, `get-thumb` and `helper fix-missing-thumbnails
-// --file` take an ID only when it is written in decimal digits and is a safe
-// integer. `Number()` alone reads `0x10` as 16, `1e3` as 1000 and an empty
-// string as 0, so the command would act on a file the user never named.
+// `files`, `get`, `get-thumb` and `helper fix-missing-thumbnails --file` take
+// an ID only when it is written in decimal digits and is a safe integer.
+// `Number()` alone reads `0x10` as 16, `1e3` as 1000 and an empty string as 0,
+// so the command would act on a file the user never named.
 // Anything else is refused with one line naming it, and exit 1, before any
 // request.
 //
@@ -1381,21 +1416,18 @@ describe("an ID given on the command line", () => {
 
     it("files looks up a decimal collection ID", async () => {
         const ctx = emptyAccount();
-        expect(await filesCommand(ctx, { collection: "1" })).toBe(1);
+        expect(await filesCommand(ctx, "1", {})).toBe(1);
         expect(stderr.text).toBe("Collection 1 not found\n");
         expect(requests).not.toHaveLength(0);
     });
 
-    it.each(NOT_IDS)(
-        "files refuses --collection %j before any request",
-        async (arg) => {
-            const ctx = emptyAccount();
-            expect(await filesCommand(ctx, { collection: arg })).toBe(1);
-            expect(stderr.text).toBe(`Invalid collection ID: ${arg}\n`);
-            expect(stdout.text).toBe("");
-            expect(requests).toHaveLength(0);
-        },
-    );
+    it.each(NOT_IDS)("files refuses %j before any request", async (arg) => {
+        const ctx = emptyAccount();
+        expect(await filesCommand(ctx, arg, {})).toBe(1);
+        expect(stderr.text).toBe(`Invalid collection ID: ${arg}\n`);
+        expect(stdout.text).toBe("");
+        expect(requests).toHaveLength(0);
+    });
 
     it("get looks up a decimal file ID", async () => {
         expect(await getCommand(emptyAccount(), "100", {})).toBe(1);
@@ -1577,14 +1609,12 @@ describe("an account with many albums", () => {
         // 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, { collection: "7" })).toBe(0);
+        expect(await filesCommand(ctx, "7", {})).toBe(0);
         expect(stdout.text).toBe("700\timage\tphoto-7.jpg\n");
     });
 
     it("files counts on stderr each album whose files its refresh fetched", async () => {
-        expect(await filesCommand(accountContext(), { collection: "7" })).toBe(
-            0,
-        );
+        expect(await filesCommand(accountContext(), "7", {})).toBe(0);
 
         expect(stdout.text).toBe("700\timage\tphoto-7.jpg\n");
         let progress = "";
@@ -1597,12 +1627,9 @@ describe("an account with many albums", () => {
     // With `--json` the refresh's progress is not printed, as `backup` and
     // the thumbnail helpers print none of theirs.
     it("files --json prints no progress", async () => {
-        expect(
-            await filesCommand(accountContext(), {
-                collection: "7",
-                json: true,
-            }),
-        ).toBe(0);
+        expect(await filesCommand(accountContext(), "7", { json: true })).toBe(
+            0,
+        );
 
         expect(JSON.parse(stdout.text)).toEqual([
             {

test/cli/read.test.ts

@@ -93,6 +93,10 @@ class FakeLibrary implements FreshReadLibrary {
         return this.collections.find((c) => c.id === id);
     }
 
+    listCollections(): Collection[] {
+        return this.refreshed ? this.collections : [];
+    }
+
     listFiles(collectionID: number): EnteFile[] {
         return this.refreshed
             ? this.files.filter((f) => f.collectionID === collectionID)
@@ -175,6 +179,30 @@ describe("CLI read helpers (issue #36 amendment, issue #52)", () => {
         ]);
     });
 
+    // Without a collection ID, `files` lists every file in the account once
+    // (issue #211). File 11 is in collections 1 and 2; collection 2 is
+    // enumerated first, so 11 is listed once, with collection 2's record.
+    it("freshFiles without a collection ID lists every file once, from the first collection holding it", async () => {
+        const files = [
+            file(10, 1, 1_700_000_000_000_000),
+            file(11, 1, 1_700_000_000_000_001),
+            file(20, 2, 1_700_000_000_000_002),
+            file(11, 2, 1_700_000_000_000_001),
+        ];
+        const lib = new FakeLibrary(
+            [collection(2, 100), collection(1, 200)],
+            files,
+        );
+
+        const rows = await freshFiles(lib);
+        expect(lib.freshCalls).toBe(1);
+        expect(rows?.map((f) => [f.id, f.collectionID])).toEqual([
+            [20, 2],
+            [11, 2],
+            [10, 1],
+        ]);
+    });
+
     it("freshFiles returns undefined for an unknown collection", async () => {
         const lib = new FakeLibrary([collection(1, 100)], []);
         const rows = await freshFiles(lib, 999);