From 35bd45a5756d96f65a0db8da420f4969bc227a62 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 18 Oct 2026 17:40:03 +0000 Subject: [PATCH] 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 --- README.md | 16 +++- TODO.md | 6 ++ bin/quak.ts | 13 +-- src/cli-commands.ts | 12 ++- src/cli-read.ts | 31 ++++++-- test/cli/backup-undecryptable.test.ts | 2 +- test/cli/bin.test.ts | 30 +++++-- test/cli/commands.test.ts | 109 ++++++++++++++++---------- test/cli/read.test.ts | 28 +++++++ 9 files changed, 180 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index 5a53ae7..95cd6af 100644 --- a/README.md +++ b/README.md @@ -35,8 +35,11 @@ yarn quak login # List the user's collections (albums). yarn quak collections -# List files in a collection. -yarn quak files --collection 12345 +# List every file in the account. +yarn quak files + +# List the files in a collection. +yarn quak files 12345 # Download and decrypt a single file. yarn quak get 67890 --out ./photo.jpg @@ -647,7 +650,7 @@ quak login interactive or QUAK_EM quak whoami print logged-in account as JSON quak logout end the session, delete it quak collections [--json] list all collections -quak files --collection [--json] list files in a collection +quak files [collectionID] [--json] list files in a collection, or every file quak get [--out path] [--collection] download and decrypt a file quak get-thumb [--out] [--collection] download and decrypt a thumbnail quak backup [--json] [--verify] full incremental backup @@ -676,7 +679,12 @@ answers from what did decrypt (see "Default reads vs. fresh reads"). `--cache-dir` overrides where the cache lives; without it each account gets its own directory under the per-user cache path. -An ID given to `files --collection`, `get`, `get-thumb` or +`files ` lists the files in that collection. `files` with no +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 than 9007199254740991. Anything else, such as `0x10`, `1e3` or `-1`, is refused before any request with one line, `Invalid file ID: ` or diff --git a/TODO.md b/TODO.md index 02a97ff..6fc85d6 100644 --- a/TODO.md +++ b/TODO.md @@ -26,6 +26,12 @@ declares one. # Completed Steps +- 2026-10-18: `quak files` takes the collection ID as an optional argument + instead of the `--collection` option, which is gone (issue 211). With no + argument it lists every file in the account once, collection by collection; a + file in several collections is listed with the first of them, which is its + `collectionID` in `--json`. The fresh read in `src/cli-read.ts` does this. + - 2026-10-18: `quak collections` no longer waits for the files of every collection (issue 210). On an empty cache it opened its library with a refresh that fetched and decrypted the files of each collection in turn, printing diff --git a/bin/quak.ts b/bin/quak.ts index 77a1a95..9c31fb5 100644 --- a/bin/quak.ts +++ b/bin/quak.ts @@ -78,14 +78,15 @@ program program .command("files") - .description("List files in a collection") - .requiredOption( - "--collection ", - "Collection ID (from `quak collections`)", + .description("List the files in a collection, or every file in the account") + .argument( + "[collectionID]", + "Collection ID (from `quak collections`); without it, every file " + + "is listed once", ) .option("--json", "Output as JSON array") - .action((opts: { collection: string; json?: boolean }) => - run(filesCommand(context(), opts)), + .action((collectionID: string | undefined, opts: { json?: boolean }) => + run(filesCommand(context(), collectionID, opts)), ); program diff --git a/src/cli-commands.ts b/src/cli-commands.ts index 6e71734..69993e0 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -269,15 +269,21 @@ export const collectionsCommand = async ( } }; +// Lists the files of the collection `collectionIDStr` names, or without it every +// file in the account, each once (issue #211). export const filesCommand = async ( ctx: CliContext, - opts: { collection: string; json?: boolean }, + collectionIDStr: string | undefined, + opts: { json?: boolean }, ): Promise => { await init(); const client = requireSession(ctx); if (!client) return 3; - const collectionID = parseID(ctx, "collection", opts.collection); - if (collectionID === undefined) return 1; + const collectionID = + collectionIDStr === undefined + ? undefined + : parseID(ctx, "collection", collectionIDStr); + if (collectionIDStr !== undefined && collectionID === undefined) return 1; const lib = await openReadLibrary(ctx, client, opts); try { diff --git a/src/cli-read.ts b/src/cli-read.ts index e009aa3..70b1f21 100644 --- a/src/cli-read.ts +++ b/src/cli-read.ts @@ -1,6 +1,6 @@ // How the CLI's read commands obtain current data. // -// `collections`, `files --collection`, `get`, and `get-thumb` must answer for +// `collections`, `files`, `get`, and `get-thumb` must answer for // the account's state at the moment the command runs, not for whatever the // local cache last happened to hold (owner amendment, issue #36). `files`, // `get` and `get-thumb` therefore force a server round-trip through @@ -14,7 +14,10 @@ // 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`. +// 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"; @@ -27,6 +30,7 @@ export interface FreshReadLibrary { fresh(): Promise; freshCollections(): Promise; getCollection(id: number): Collection | undefined; + listCollections(): Collection[]; listFiles(collectionID: number): EnteFile[]; getFileByID(fileID: number): EnteFile | undefined; photos: Pick; @@ -40,14 +44,29 @@ export const freshCollections = ( // The files of one collection, current as of a forced refresh, 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". +// the caller can tell "no such collection" from "an empty collection". Without +// a collection ID, every file in the account, each once, as held by the first +// collection that holds it. export const freshFiles = async ( lib: FreshReadLibrary, - collectionID: number, + collectionID?: number, ): Promise => { await lib.fresh(); - if (!lib.getCollection(collectionID)) return undefined; - return lib.listFiles(collectionID); + if (collectionID !== undefined) { + return lib.getCollection(collectionID) + ? lib.listFiles(collectionID) + : undefined; + } + const listed = new Set(); + const files: EnteFile[] = []; + for (const collection of lib.listCollections()) { + for (const file of lib.listFiles(collection.id)) { + if (listed.has(file.id)) continue; + listed.add(file.id); + files.push(file); + } + } + return files; }; // One file, current as of a forced refresh, resolved to both its content diff --git a/test/cli/backup-undecryptable.test.ts b/test/cli/backup-undecryptable.test.ts index 7344ba1..bcd507c 100644 --- a/test/cli/backup-undecryptable.test.ts +++ b/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"); }); }); diff --git a/test/cli/bin.test.ts b/test/cli/bin.test.ts index bbcbe79..18b55ab 100644 --- a/test/cli/bin.test.ts +++ b/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 ` to completion. The "Not logged in" message and the exit -// are swallowed. -const quak = async (...args: string[]): Promise => { - vi.spyOn(process.stderr, "write").mockImplementation(() => true); +// Run `quak ` 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 => { + 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/); + }); +}); diff --git a/test/cli/commands.test.ts b/test/cli/commands.test.ts index 6da51e1..3b03443 100644 --- a/test/cli/commands.test.ts +++ b/test/cli/commands.test.ts @@ -121,25 +121,31 @@ const FILES: Record = { // 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"); @@ -254,7 +260,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); @@ -501,17 +507,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([ { @@ -525,11 +528,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", () => { @@ -1132,7 +1167,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 = ""; }); @@ -1191,10 +1226,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. // @@ -1248,21 +1283,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); @@ -1444,14 +1476,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 = ""; @@ -1464,12 +1494,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([ { diff --git a/test/cli/read.test.ts b/test/cli/read.test.ts index 08821a1..66ff369 100644 --- a/test/cli/read.test.ts +++ b/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);