diff --git a/README.md b/README.md index 8e34535..640417a 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. @@ -1180,6 +1183,12 @@ since the stored copy, so a collection stored without its files would never get them. `quak collections` uses it (issue https://git.eeqj.de/sneak/quak/issues/210). +`await lib.freshFiles(collection)` is the fresh read of one collection's files. +It answers with every live file in that collection that decrypts, in the +server's order. `collection` is a record that `freshCollections()` returned. The +method fetches no other collection's files and stores nothing. `quak files` with +a collection ID uses it. + An album or file in the server's answer that quak cannot decrypt, or whose metadata is not JSON, does not fail the refresh. The refresh leaves it out, an album with all its files, and drops any older copy the cache held, so reads 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..9373717 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -128,12 +128,13 @@ const readLibraryClient = (client: Client): LibraryClient => ({ contentSource: () => client.contentSource(), }); -// 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. +// Open a library for a single point command. The aggressive background +// precache (issue #48) is off, because a one-shot `collections` or `get` must +// not start downloading the whole account. No refresh starts as the library +// opens or on a timer. Each command asks for the refresh 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..5980fbc 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,24 @@ 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. It returns each file once, as the latest copy the server sent, + // where the first copy was. + 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, + }); + const byID = new Map(); + for (const file of files) byID.set(file.id, file); + return [...byID.values()]; + } + // 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..3822da7 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,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(); + + // 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 => { + 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[]> = {}; 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); }); }); 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(); });