From 3c6dfc948702ea1681ae0b1cb52b2e47ca4abb85 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 26 Oct 2026 06:37:43 +0000 Subject: [PATCH] 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. `quak files` without a collection ID still waits for a refresh. Tests that ran `quak files 1` to fill the cache, to show a refresh's progress or to show what `-q` hides now run a command that still refreshes. Model: opus-5-5 --- README.md | 25 +++++----- TODO.md | 7 +++ src/cli-commands.ts | 17 +++---- src/cli-read.ts | 53 ++++++++++----------- src/library/index.ts | 22 +++++++-- test/cli/commands.test.ts | 98 +++++++++++++++++++++++---------------- test/cli/log.test.ts | 4 +- test/cli/read.test.ts | 58 +++++++++++++---------- 8 files changed, 169 insertions(+), 115 deletions(-) diff --git a/README.md b/README.md index 8e34535..74b308e 100644 --- a/README.md +++ b/README.md @@ -685,17 +685,20 @@ library. The read commands — `collections`, `files`, `get`, `get-thumb`, `helper fix-missing-thumbnails` — force a fresh server round-trip before they answer, so they report current account state rather than whatever the cache last held. `collections` asks the server for the collection list alone, in one -request, and neither fetches any collection's files nor changes the cache. The -others wait for a refresh. With an empty cache, as on a first run, that refresh -fetches the files of every collection, one collection after another, and stores -nothing until it has them all, which on a large account takes a while. Meanwhile -the command logs a line on stderr for each collection whose files it has -fetched, such as `Fetched the files of 3 of 120 collections`, with `--json` too; -`backup` does the same while the refresh it starts with runs. stdout holds only -the command's output. If that round-trip fails, the command prints the error on -one line and exits 1, or 3 when the server no longer accepts the saved session -(see "Session handling"). An album or file quak cannot decrypt does not fail it; -the command answers from what did decrypt (see "Default reads vs. fresh reads"). +request, and neither fetches any collection's files nor changes the cache. +`files` with a collection ID asks for the same list, then for that one +collection's files, and does not change the cache either. The others wait for a +refresh, and so does `files` without a collection ID. With an empty cache, as on +a first run, that refresh fetches the files of every collection, one collection +after another, and stores nothing until it has them all, which on a large +account takes a while. Meanwhile the command logs a line on stderr for each +collection whose files it has fetched, such as +`Fetched the files of 3 of 120 collections`, with `--json` too; `backup` does +the same while the refresh it starts with runs. stdout holds only the command's +output. If that round-trip fails, the command prints the error on one line and +exits 1, or 3 when the server no longer accepts the saved session (see "Session +handling"). An album or file quak cannot decrypt does not fail it; the command +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. diff --git a/TODO.md b/TODO.md index 99ae803..cbf043d 100644 --- a/TODO.md +++ b/TODO.md @@ -26,6 +26,13 @@ declares one. # Completed Steps +- 2026-10-26: `quak files` with a collection ID fetches that collection's files + alone (https://git.eeqj.de/sneak/quak/issues/217). It used to wait for a + refresh, which on an empty cache fetched the files of every collection. It now + reads the collection list through `lib.freshCollections()`, then the one + collection's files through the new `lib.freshFiles()`. It stores nothing, and + the files print in the server's order. + - 2026-10-22: The JSON `quak backup` and `quak backup-metadata` write for a file holds `keywords` in place of the CLIP embedding in its ML data (https://git.eeqj.de/sneak/quak/issues/218). The keywords are the titles of diff --git a/src/cli-commands.ts b/src/cli-commands.ts index 52f1b05..27f2064 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -131,9 +131,10 @@ const readLibraryClient = (client: Client): LibraryClient => ({ // Open a library for a single point command: the aggressive background precache // (issue #48) is off — a one-shot `collections` or `get` must not start // downloading the whole account — and no refresh starts as it opens or on a -// timer: each command asks for the one it needs, and `collections` needs none -// (issue #210). While a refresh runs, each collection whose files it has -// fetched is logged, so a large account shows progress rather than nothing. +// timer: each command asks for the one it needs. `collections` and `files` +// with a collection ID 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.open({ client: readLibraryClient(client), @@ -282,11 +283,11 @@ export const filesCommand = async ( const lib = await openReadLibrary(ctx, client); try { - // Force a server round-trip and list in enumeration order (issue #36 - // amendment, issue #52). Each file prints from its own decrypted - // metadata (raw title, microsecond creationTime) via cli-output, and in - // the pre-library CLI's enumeration order, not the projection's - // newest-first order. + // Ask the server for current state (issue #36 amendment, issue #52). + // Each file prints from its own decrypted metadata (raw title, + // microsecond creationTime) via cli-output. The files print in the + // order `freshFiles` returns them, not the projection's newest-first + // order. const files = await freshFiles(lib, collectionID); if (!files) { ctx.log("fatal", `Collection ${collectionID} not found`); diff --git a/src/cli-read.ts b/src/cli-read.ts index 70b1f21..a9f57f1 100644 --- a/src/cli-read.ts +++ b/src/cli-read.ts @@ -2,34 +2,35 @@ // // `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 -// `Library.fresh()` and only then read — so a collection, file, or metadata -// change made elsewhere is visible. `collections` asks the library for the -// collection list alone through `Library.freshCollections()` (issue #210): a -// refresh would also fetch the files of every collection, which on a large -// account takes long and which `collections` does not print. +// local cache last happened to hold (owner amendment, issue #36). `get`, +// `get-thumb` and `files` without a collection ID therefore force a server +// round-trip through `Library.fresh()` and only then read, so a change made +// elsewhere is visible. `collections` asks the library for the collection list +// alone through `Library.freshCollections()` (issue #210): a refresh would also +// fetch the files of every collection, which on a large account takes long and +// which `collections` does not print. `files` with a collection ID reads that +// list too, then that one collection's files through `Library.freshFiles()` +// (issue #217). // -// `collections` lists in the server's order and `files` in the library's own -// 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`. `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. +// `collections` and `files` with a collection ID list in the server's order. +// `files` without one lists in the library's own enumeration order +// (`listFiles()`). Neither command uses 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`. `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"; // The slice of `Library` these helpers read. `Library` satisfies it -// structurally; a test can drive them with a stand-in that records the -// `fresh()` and `freshCollections()` calls and serves records in a known -// enumeration order. +// structurally; a test can drive them with a stand-in that records each fresh +// read and serves records in a known enumeration order. export interface FreshReadLibrary { fresh(): Promise; freshCollections(): Promise; - getCollection(id: number): Collection | undefined; + freshFiles(collection: Collection): Promise; listCollections(): Collection[]; listFiles(collectionID: number): EnteFile[]; getFileByID(fileID: number): EnteFile | undefined; @@ -42,21 +43,21 @@ export const freshCollections = ( lib: FreshReadLibrary, ): Promise => lib.freshCollections(); -// The files of one collection, current as of a forced refresh, in enumeration +// The files of one collection, as the server holds them now, in the server's // order. `undefined` (not an empty list) when the collection does not exist, so // 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. +// a collection ID, every file in the account as of a forced refresh, each once, +// as held by the first collection that holds it. export const freshFiles = async ( lib: FreshReadLibrary, collectionID?: number, ): Promise => { - await lib.fresh(); if (collectionID !== undefined) { - return lib.getCollection(collectionID) - ? lib.listFiles(collectionID) - : undefined; + const collections = await lib.freshCollections(); + const collection = collections.find((c) => c.id === collectionID); + return collection ? lib.freshFiles(collection) : undefined; } + await lib.fresh(); const listed = new Set(); const files: EnteFile[] = []; for (const collection of lib.listCollections()) { diff --git a/src/library/index.ts b/src/library/index.ts index a3bed63..172475f 100644 --- a/src/library/index.ts +++ b/src/library/index.ts @@ -20,9 +20,10 @@ // // `freshCollections()` is the fresh read of the collection list alone (issue // #210): one collections request, no collection's files, and nothing stored, -// for a caller such as `quak collections` that needs no more. A caller that -// asks for each refresh it needs can also open the library with -// `refreshOnOpen: false`, so that `open()` starts none. +// for a caller such as `quak collections` that needs no more. `freshFiles()` +// is the fresh read of one collection's files, and stores nothing either +// (issue #217). A caller that asks for each refresh it needs can also open the +// library with `refreshOnOpen: false`, so that `open()` starts none. // // A refresh reports through `onProgress` each collection whose files it has // fetched, so a large account, whose first refresh fetches the files of every @@ -625,6 +626,21 @@ export class Library { return collections; } + // The fresh read of one collection's files (issue #217): every live file + // in it that decrypts, in the server's order. `collection` is a record + // `freshCollections()` returned, whose key decrypts the files. It fetches + // no other collection's files. Like `freshCollections()`, it stores + // nothing. + async freshFiles(collection: Collection): Promise { + if (this.closed) throw new Error("the library is closed"); + const { files } = await this.client.filesSince({ + collectionID: collection.id, + collectionKey: collection.key, + sinceTime: 0, + }); + return files; + } + // Back up every in-scope file to `opts.downloadDirectory`, or else the // library's, each original at its save path, with a durable failure // ledger (issue #51). Takes the lock in that directory first, unless the diff --git a/test/cli/commands.test.ts b/test/cli/commands.test.ts index 3269fdb..d080cc5 100644 --- a/test/cli/commands.test.ts +++ b/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,30 @@ 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`; + } + // Album n is named `Album n` and holds one photo, n * 100, titled // `photo-n.jpg`. beforeAll(async () => { @@ -1664,7 +1672,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,7 +1685,7 @@ 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. @@ -1698,46 +1707,53 @@ 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", + ]); + }); + + // 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); }); }); diff --git a/test/cli/log.test.ts b/test/cli/log.test.ts index 137c983..e30970e 100644 --- a/test/cli/log.test.ts +++ b/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", }); }); diff --git a/test/cli/read.test.ts b/test/cli/read.test.ts index 66ff369..cb8dc1d 100644 --- a/test/cli/read.test.ts +++ b/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 { + 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(); });