From fde6f19c869bd756e952836f3af21eec9a817f74 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 26 Oct 2026 08:20:51 +0000 Subject: [PATCH] quak get and get-thumb on an empty cache stop fetching collections once they find the file (closes #222) `quak get` and `quak get-thumb` used to wait for a refresh, which on an empty cache fetched the files of every collection. Now, on an empty cache, `freshFile` reads the collection list, then one collection's files at a time, and stops at the first that holds the file. That search stores nothing. Once the cache holds the account's collections, `freshFile` refreshes as before and looks the file up in the cache. The commands fetch the content through the new `Library.originalOf()` and `Library.thumbnailOf()`, which take the file record itself, since the store may not hold it. On an empty cache they no longer print a refresh's progress lines. The `-q` test that relied on `get` refreshing now runs `helper fix-missing-thumbnails`. Model: opus-5-5 --- README.md | 25 ++++-- TODO.md | 9 ++ src/cli-commands.ts | 19 ++--- src/cli-read.ts | 31 ++++--- src/library/content.ts | 49 ++++++++--- src/library/index.ts | 32 +++++++ test/cli/commands.test.ts | 121 ++++++++++++++++++--------- test/cli/log.test.ts | 8 +- test/cli/read.test.ts | 68 +++++++++------ test/library/content-library.test.ts | 6 ++ test/library/content.test.ts | 19 +++++ 11 files changed, 278 insertions(+), 109 deletions(-) diff --git a/README.md b/README.md index 640417a..97d8e56 100644 --- a/README.md +++ b/README.md @@ -687,12 +687,16 @@ 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. `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 +collection's files, and does not change the cache either. On an empty cache, +`get` and `get-thumb` ask for the same list, then for one collection's files at +a time, until they find the file. They store none of the collections or file +records they read. The original or thumbnail they fetch is kept in the cache. +Once the cache holds the account's collections, they wait for a refresh instead. +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 @@ -1187,7 +1191,8 @@ https://git.eeqj.de/sneak/quak/issues/210). 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. +a collection ID uses it, and so do `quak get` and `quak get-thumb` on an empty +cache. 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 @@ -1270,6 +1275,12 @@ source. An original that `original()`, `content()` or `exif()` downloads lands in the cache, which does not make `isLocal` true; only `download()` and `lib.backup()` do. +`await lib.originalOf(file, opts?)` and `await lib.thumbnailOf(file, opts?)` +fetch and cache as `photo.original()` and `photo.thumbnail()` do. `file` is an +`EnteFile` the caller holds, and the library need not hold it. On an empty +cache, `quak get` and `quak get-thumb` pass one that `lib.freshFiles()` +returned. + Lower-level accessors that return decrypted model objects (which hold key material) are also available: `listCollections()`, `getCollection(id)`, `listFiles(collectionID)`, `getFile(collectionID, fileID)`, and diff --git a/TODO.md b/TODO.md index cbf043d..197ab3d 100644 --- a/TODO.md +++ b/TODO.md @@ -26,6 +26,15 @@ declares one. # Completed Steps +- 2026-10-26: On an empty cache, `quak get` and `quak get-thumb` stop fetching + collections' files once they find the file + (https://git.eeqj.de/sneak/quak/issues/222). They used to wait for a refresh, + which on an empty cache fetched the files of every collection. Now they read + the collection list, then one collection's files at a time. They store none of + the collections or file records they read, and fetch the original or thumbnail + through the new `lib.originalOf()` and `lib.thumbnailOf()`. Once the cache + holds the account's collections, they still wait for a refresh. + - 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 diff --git a/src/cli-commands.ts b/src/cli-commands.ts index 9373717..1bb7d13 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -133,8 +133,9 @@ const readLibraryClient = (client: Client): LibraryClient => ({ // 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. +// #217). Nor do `get` and `get-thumb` on an empty cache (issue #222). 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), @@ -324,14 +325,13 @@ export const getCommand = async ( try { // Force a server round-trip so the file resolves against current state // (issue #36 amendment, issue #52). - const resolved = await freshFile(lib, fileID); - if (!resolved) { + const file = await freshFile(lib, fileID); + if (!file) { ctx.log("fatal", `File ${fileID} not found`); return 1; } - const { photo, file } = resolved; - const result = await photo.original(); + const result = await lib.originalOf(file); // Default name is the file's own title, as the pre-library CLI used // (not the editedName-preferring projection title) (issue #52). if (result.videoPath === undefined) { @@ -382,14 +382,13 @@ export const getThumbCommand = async ( try { // Force a server round-trip so the file resolves against current state // (issue #36 amendment, issue #52). - const resolved = await freshFile(lib, fileID); - if (!resolved) { + const file = await freshFile(lib, fileID); + if (!file) { ctx.log("fatal", `File ${fileID} not found`); return 1; } - const { photo, file } = resolved; - const result = await photo.thumbnail(); + const result = await lib.thumbnailOf(file); // Default name is thumb_, as the pre-library CLI // used (not the projection title) (issue #52). const outPath = opts.out ?? thumbnailName(file); diff --git a/src/cli-read.ts b/src/cli-read.ts index a9f57f1..e5234fa 100644 --- a/src/cli-read.ts +++ b/src/cli-read.ts @@ -10,7 +10,8 @@ // 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). +// (issue #217). On an empty cache, `get` and `get-thumb` read that list, then +// one collection's files at a time, until they find the file (issue #222). // // `collections` and `files` with a collection ID list in the server's order. // `files` without one lists in the library's own enumeration order @@ -22,7 +23,6 @@ // 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 each fresh @@ -34,7 +34,6 @@ export interface FreshReadLibrary { listCollections(): Collection[]; listFiles(collectionID: number): EnteFile[]; getFileByID(fileID: number): EnteFile | undefined; - photos: Pick; } // Every live collection, current as of one collections request, in the @@ -70,16 +69,24 @@ export const freshFiles = async ( return files; }; -// One file, current as of a forced refresh, resolved to both its content -// handle (`Photo`, for fetching bytes) and its raw record (`EnteFile`, for the -// default output name and field values). `undefined` when the file is unknown. +// One file as the server holds it now, or `undefined` when no collection holds +// it. When the cache holds the account's collections, it refreshes and looks +// the file up in the cache. On an empty cache, it reads the collection list, +// then one collection's files at a time, and stops at the first collection +// that holds the file. That search stores nothing. export const freshFile = async ( lib: FreshReadLibrary, fileID: number, -): Promise<{ photo: Photo; file: EnteFile } | undefined> => { - await lib.fresh(); - const photo = lib.photos.byID({ fileID }); - const file = lib.getFileByID(fileID); - if (!photo || !file) return undefined; - return { photo, file }; +): Promise => { + // Only a refresh stores collections, so a cache holding any holds them all. + if (lib.listCollections().length > 0) { + await lib.fresh(); + return lib.getFileByID(fileID); + } + for (const collection of await lib.freshCollections()) { + const files = await lib.freshFiles(collection); + const file = files.find((f) => f.id === fileID); + if (file) return file; + } + return undefined; }; diff --git a/src/library/content.ts b/src/library/content.ts index 9f2d6e6..906c0d6 100644 --- a/src/library/content.ts +++ b/src/library/content.ts @@ -581,14 +581,34 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { fileID: number, opts?: ContentOptions, ): Promise { - return this.get(fileID, "original", "on-demand", opts?.onProgress); + const file = this.fileOrThrow(fileID); + return this.get(file, "original", "on-demand", opts?.onProgress); } async thumbnail( fileID: number, opts?: ContentOptions, ): Promise { - return this.get(fileID, "thumbnail", "on-demand", opts?.onProgress); + const file = this.fileOrThrow(fileID); + return this.get(file, "thumbnail", "on-demand", opts?.onProgress); + } + + // The original of `file`, fetched and cached as `original` does. `file` is + // used as given, so `getFile` need not know it. + async originalOf( + file: EnteFile, + opts?: ContentOptions, + ): Promise { + return this.get(file, "original", "on-demand", opts?.onProgress); + } + + // The thumbnail of `file`, fetched and cached as `thumbnail` does. `file` + // is used as given, so `getFile` need not know it. + async thumbnailOf( + file: EnteFile, + opts?: ContentOptions, + ): Promise { + return this.get(file, "thumbnail", "on-demand", opts?.onProgress); } // Put the original at the save path of `file` under the download directory @@ -615,7 +635,7 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { destination: string, ): Promise { const result = await this.acquire( - fileID, + this.fileOrThrow(fileID), "original", "on-demand", undefined, @@ -638,8 +658,7 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { fileID: number, destination: string, ): Promise<{ path: string; videoPath?: string }> { - const file = this.getFile(fileID); - if (!file) throw new Error(`content cache: unknown file ${fileID}`); + const file = this.fileOrThrow(fileID); return this.pools.content.run( async () => { const fetched = await this.fetchInto( @@ -721,7 +740,8 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { onProgress: ((event: EnsureEvent) => void) | undefined, ): Promise { try { - const result = await this.acquire(fileID, kind, priority, signal); + const file = this.fileOrThrow(fileID); + const result = await this.acquire(file, kind, priority, signal); const status = result.cached ? "skipped" : "done"; onProgress?.({ fileID, status, path: result.path }); return { fileID, path: result.path }; @@ -736,8 +756,16 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { } } + // The record `getFile` gives for `fileID`. It throws for a file `getFile` + // does not know. + private fileOrThrow(fileID: number): EnteFile { + const file = this.getFile(fileID); + if (!file) throw new Error(`content cache: unknown file ${fileID}`); + return file; + } + private async get( - fileID: number, + file: EnteFile, kind: Kind, priority: Priority, onProgress: ((event: ContentEvent) => void) | undefined, @@ -745,7 +773,7 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { const onByte: ProgressCallback | undefined = onProgress ? (bytesDone) => onProgress({ status: "downloading", bytesDone }) : undefined; - const result = await this.acquire(fileID, kind, priority, undefined, { + const result = await this.acquire(file, kind, priority, undefined, { onByte, }); onProgress?.( @@ -767,7 +795,7 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { // photo's original is present only with its video, and is returned with // it. private async acquire( - fileID: number, + file: EnteFile, kind: Kind, priority: Priority, signal: AbortSignal | undefined, @@ -778,8 +806,7 @@ export class ContentCache implements PhotoContent, ThumbnailsAPI { videoPath?: string; cached: boolean; }> { - const file = this.getFile(fileID); - if (!file) throw new Error(`content cache: unknown file ${fileID}`); + const fileID = file.id; const isLivePhoto = kind === "original" && file.metadata.fileType === "livePhoto"; diff --git a/src/library/index.ts b/src/library/index.ts index 5980fbc..23239e9 100644 --- a/src/library/index.ts +++ b/src/library/index.ts @@ -66,6 +66,8 @@ import { ContentCache, savePath, storedAtSavePath, + type ContentOptions, + type ContentResult, type ContentSource, type ThumbnailsAPI, type EnsureOptions, @@ -644,6 +646,36 @@ export class Library { return [...byID.values()]; } + // The original of `file`, fetched and cached as `Photo.original()` does. + // `file` is used as given, so the store need not hold it. On an empty + // cache, `quak get` passes a file that `freshFiles()` returned. + async originalOf( + file: EnteFile, + opts?: ContentOptions, + ): Promise { + if (!this.cache) { + throw new Error( + "originalOf requires a library opened with a content cache", + ); + } + return this.cache.originalOf(file, opts); + } + + // The thumbnail of `file`, fetched and cached as `Photo.thumbnail()` does. + // `file` is used as given, so the store need not hold it. On an empty + // cache, `quak get-thumb` passes a file that `freshFiles()` returned. + async thumbnailOf( + file: EnteFile, + opts?: ContentOptions, + ): Promise { + if (!this.cache) { + throw new Error( + "thumbnailOf requires a library opened with a content cache", + ); + } + return this.cache.thumbnailOf(file, opts); + } + // 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 3822da7..78f99fa 100644 --- a/test/cli/commands.test.ts +++ b/test/cli/commands.test.ts @@ -194,9 +194,6 @@ const FETCHED_BOTH_ALBUMS = "Fetched the files of 1 of 2 collections\n" + "Fetched the files of 2 of 2 collections\n"; -// The same for an account with one album. -const FETCHED_ONE_ALBUM = "Fetched the files of 1 of 1 collections\n"; - // Collects everything written to it. class Output { text = ""; @@ -576,27 +573,27 @@ describe("get and get-thumb", () => { const out = join(root, "diagram.png"); expect(await getCommand(context(), "200", { out })).toBe(0); expect(readFileSync(out)).toEqual(Buffer.alloc(7, 200)); - expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS + `7 bytes -> ${out}\n`); + expect(stderr.text).toBe(`7 bytes -> ${out}\n`); }); it("get-thumb finds a file in any album without --collection", async () => { const out = join(root, "thumb.jpg"); expect(await getThumbCommand(context(), "200", { out })).toBe(0); expect(readFileSync(out)).toEqual(Buffer.alloc(3, 200)); - expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS + `3 bytes -> ${out}\n`); + expect(stderr.text).toBe(`3 bytes -> ${out}\n`); }); it("get exits 1 when no album has the file", async () => { const out = join(root, "x"); expect(await getCommand(context(), "999", { out })).toBe(1); - expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS + "File 999 not found\n"); + expect(stderr.text).toBe("File 999 not found\n"); expect(existsSync(out)).toBe(false); }); it("get-thumb exits 1 when no album has the file", async () => { const out = join(root, "x"); expect(await getThumbCommand(context(), "999", { out })).toBe(1); - expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS + "File 999 not found\n"); + expect(stderr.text).toBe("File 999 not found\n"); expect(existsSync(out)).toBe(false); }); }); @@ -644,7 +641,7 @@ describe("names from an album name or a title over 255 bytes", () => { expect(Buffer.byteLength(name)).toBe(255); expect(code).toBe(0); expect(readdirSync(dir)).toEqual([name]); - expect(stderr.text).toBe(FETCHED_ONE_ALBUM + `7 bytes -> ${name}\n`); + expect(stderr.text).toBe(`7 bytes -> ${name}\n`); }); it("get-thumb cuts the title short so that thumb_ and it fit in 255 bytes, and exits 0", async () => { @@ -656,7 +653,7 @@ describe("names from an album name or a title over 255 bytes", () => { expect(Buffer.byteLength(name)).toBe(255); expect(code).toBe(0); expect(readdirSync(dir)).toEqual([name]); - expect(stderr.text).toBe(FETCHED_ONE_ALBUM + `3 bytes -> ${name}\n`); + expect(stderr.text).toBe(`3 bytes -> ${name}\n`); }); it("backup-metadata cuts the album name short so that its folder's name fits in 255 bytes, and exits 0", async () => { @@ -745,8 +742,7 @@ describe("a live photo", () => { Buffer.from(VIDEO), ); expect(stderr.text).toBe( - FETCHED_ONE_ALBUM + - `${IMAGE.length} bytes -> IMG_0300.heic\n` + + `${IMAGE.length} bytes -> IMG_0300.heic\n` + `${VIDEO.length} bytes -> IMG_0300.mov\n`, ); }); @@ -800,8 +796,7 @@ describe("a live photo", () => { expect(readFileSync(out)).toEqual(Buffer.from(IMAGE)); expect(readFileSync(video)).toEqual(Buffer.from(VIDEO)); expect(stderr.text).toBe( - FETCHED_ONE_ALBUM + - `${IMAGE.length} bytes -> ${out}\n${VIDEO.length} bytes -> ${video}\n`, + `${IMAGE.length} bytes -> ${out}\n${VIDEO.length} bytes -> ${video}\n`, ); }); @@ -815,8 +810,7 @@ describe("a live photo", () => { expect(existsSync(out)).toBe(false); expect(existsSync(join(root, "photo.mov"))).toBe(false); expect(stderr.text).toBe( - FETCHED_ONE_ALBUM + - `File 300 is a live photo, and its video would also be written to ${out}\n`, + `File 300 is a live photo, and its video would also be written to ${out}\n`, ); }); @@ -1575,13 +1569,14 @@ describe("an ID given on the command line", () => { ); }); -// 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. +// An account with many albums (issues #210, #217 and #222). On an empty cache +// `collections`, `files` with a collection ID and `get` 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"; @@ -1602,14 +1597,15 @@ describe("an account with many albums", () => { // 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. + // A photo in album `collectionID` as the server sends it, and its original + // encrypted as the server stores it. The original's bytes are the title. + // These tests fetch no thumbnail. const photoRow = (photo: { id: number; collectionID: number; title: string; updationTime: number; - }): Record => { + }): { row: Record; original: Uint8Array } => { const photoKey = sodium.crypto_secretstream_xchacha20poly1305_keygen(); const photoKeyBox = encryptSecretbox( photoKey, @@ -1626,7 +1622,11 @@ describe("an account with many albums", () => { ), photoKey, ); - return { + const original = encryptStreamBlob( + new TextEncoder().encode(photo.title), + photoKey, + ); + const row = { id: photo.id, collectionID: photo.collectionID, ownerID: USER_ID, @@ -1636,10 +1636,11 @@ describe("an account with many albums", () => { encryptedData: toBase64(metadata.ciphertext), decryptionHeader: toBase64(metadata.header), }, - file: { decryptionHeader: "" }, + file: { decryptionHeader: toBase64(original.header) }, thumbnail: { decryptionHeader: "" }, updationTime: photo.updationTime, }; + return { row, original: original.ciphertext }; }; // Album n is named `Album n` and holds one photo, n * 100, titled @@ -1649,6 +1650,7 @@ describe("an account with many albums", () => { const account = await buildAccount(PASSWORD); const collections: Record[] = []; const filesByCollection: Record[]> = {}; + const fileCiphertexts: Record = {}; for (let id = 1; id <= ALBUMS; id++) { const albumKey = sodium.crypto_secretbox_keygen(); albumKeys.set(id, albumKey); @@ -1667,14 +1669,14 @@ describe("an account with many albums", () => { type: "album", updationTime: 1, }); - filesByCollection[id] = [ - photoRow({ - id: id * 100, - collectionID: id, - title: `photo-${id}.jpg`, - updationTime: 1, - }), - ]; + const { row, original } = photoRow({ + id: id * 100, + collectionID: id, + title: `photo-${id}.jpg`, + updationTime: 1, + }); + filesByCollection[id] = [row]; + fileCiphertexts[id * 100] = original; } const ente = buildMetaFetch({ verifier: account.verifier, @@ -1684,7 +1686,7 @@ describe("an account with many albums", () => { collections, filesByCollection, encryptedMLData: {}, - fileCiphertexts: {}, + fileCiphertexts, }); server = (async (input: RequestInfo | URL, init?: RequestInit) => { const url = @@ -1758,13 +1760,13 @@ describe("an account with many albums", () => { collectionID: 7, title: "photo-7.jpg", updationTime: 1, - }), + }).row, photoRow({ id: 701, collectionID: 7, title: "photo-701.jpg", updationTime: 1, - }), + }).row, ]; const secondPage = [ photoRow({ @@ -1772,7 +1774,7 @@ describe("an account with many albums", () => { collectionID: 7, title: "photo-7-edited.jpg", updationTime: 2, - }), + }).row, ]; const paged = (async (input: RequestInfo | URL, init?: RequestInit) => { const url = new URL(input instanceof Request ? input.url : input); @@ -1812,6 +1814,47 @@ describe("an account with many albums", () => { expect(stderr.text).toBe(progress); }); + // Photo 200 is in album 2, so `get` asks for no file of album 3 or later. + // The last request downloads the photo's original. + it("get on an empty cache asks for the albums' files only until one holds the photo", async () => { + const out = join(root, "photo.jpg"); + expect(await getCommand(accountContext(), "200", { out })).toBe(0); + + expect(readFileSync(out, "utf-8")).toBe("photo-2.jpg"); + expect(stderr.text).toBe(`11 bytes -> ${out}\n`); + expect(requests).toEqual([ + "/collections/v2?sinceTime=0", + "/collections/v2/diff?collectionID=1&sinceTime=0", + "/collections/v2/diff?collectionID=2&sinceTime=0", + "/?fileID=200", + ]); + expect(existsSync(join(root, "cache", "metadata.json"))).toBe(false); + }); + + // `files` first fills the cache, so `get` waits for a refresh. No album has + // changed since, so the refresh asks for no album's files. + it("get on a cache that holds the account asks for no unchanged album's files, also for a photo in no album", async () => { + const ctx = accountContext(); + expect(await filesCommand(ctx, undefined, {})).toBe(0); + requests.length = 0; + stderr.text = ""; + + const out = join(root, "photo.jpg"); + expect(await getCommand(ctx, "4000", { out })).toBe(0); + expect(readFileSync(out, "utf-8")).toBe("photo-40.jpg"); + expect(stderr.text).toBe(`12 bytes -> ${out}\n`); + expect(requests).toEqual([ + "/collections/v2?sinceTime=1", + "/?fileID=4000", + ]); + + requests.length = 0; + stderr.text = ""; + expect(await getCommand(ctx, "999", { out })).toBe(1); + expect(stderr.text).toBe("File 999 not found\n"); + expect(requests).toEqual(["/collections/v2?sinceTime=1"]); + }); + it("files counts on stderr each album whose files its refresh fetched", async () => { expect(await filesCommand(accountContext(), undefined, {})).toBe(0); diff --git a/test/cli/log.test.ts b/test/cli/log.test.ts index e30970e..1bc1fa6 100644 --- a/test/cli/log.test.ts +++ b/test/cli/log.test.ts @@ -234,13 +234,15 @@ describe("-q", () => { }); }); - // The refresh's progress is left out; the line saying why the command + // No album holds file 999, so fixing it fails. The refresh's progress and + // the list of failed files are 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", "get", "999")).toEqual({ + const fix = ["helper", "fix-missing-thumbnails", "--file", "999"]; + expect(await quak("-q", ...fix)).toEqual({ code: 1, stdout: "", - stderr: "File 999 not found\n", + stderr: `quak: 1 file(s) failed; see the "Failed files:" list, or the JSON with --json\n`, }); }); diff --git a/test/cli/read.test.ts b/test/cli/read.test.ts index cb8dc1d..b60b068 100644 --- a/test/cli/read.test.ts +++ b/test/cli/read.test.ts @@ -11,7 +11,8 @@ * (`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. + * collection. On an empty cache, `get` and `get-thumb` ask for that list, then + * for one collection's files at a time, until they find the file (issue #222). * * `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 @@ -29,7 +30,6 @@ import { type FreshReadLibrary, } from "../../src/cli-read.js"; import { fileListRow } from "../../src/cli-output.js"; -import type { Photo } from "../../src/library/index.js"; import type { Collection, EnteFile } from "../../src/model/types.js"; const collection = (id: number, updationTime: number): Collection => ({ @@ -66,8 +66,7 @@ const file = ( // 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. +// refresh. class FakeLibrary implements FreshReadLibrary { freshCalls = 0; freshCollectionsCalls = 0; @@ -110,14 +109,6 @@ class FakeLibrary implements FreshReadLibrary { if (!this.refreshed) return undefined; return this.files.find((f) => f.id === fileID); } - - photos = { - byID: ({ fileID }: { fileID: number }): Photo | undefined => { - if (!this.refreshed) return undefined; - if (!this.files.some((f) => f.id === fileID)) return undefined; - return { fileID } as unknown as Photo; - }, - }; } describe("CLI read helpers (issue #36 amendment, issue #52)", () => { @@ -220,23 +211,46 @@ describe("CLI read helpers (issue #36 amendment, issue #52)", () => { expect(rows).toBeUndefined(); }); - it("freshFile refreshes first, then resolves the photo and its raw record", async () => { - const f = file(10, 1, 1_700_000_000_000_000); - const lib = new FakeLibrary([collection(1, 100)], [f]); - - const resolved = await freshFile(lib, 10); - expect(lib.freshCalls).toBe(1); - expect(resolved?.photo.fileID).toBe(10); - expect(resolved?.file.metadata.title).toBe("file-10.jpg"); - expect(resolved?.file.metadata.creationTime).toBe( - 1_700_000_000_000_000, + // Three collections, each holding one file. + const threeCollections = (): FakeLibrary => + new FakeLibrary( + [collection(1, 100), collection(2, 200), collection(3, 300)], + [ + file(10, 1, 1_700_000_000_000_000), + file(20, 2, 1_700_000_000_000_001), + file(30, 3, 1_700_000_000_000_002), + ], ); + + it("freshFile on an empty cache reads each collection's files in the server's order until one holds the file", async () => { + const lib = threeCollections(); + + const found = await freshFile(lib, 20); + expect(lib.freshCalls).toBe(0); + expect(lib.freshCollectionsCalls).toBe(1); + expect(lib.freshFilesCalls).toEqual([1, 2]); + expect(found?.metadata.title).toBe("file-20.jpg"); + expect(found?.metadata.creationTime).toBe(1_700_000_000_000_001); }); - it("freshFile returns undefined for an unknown file", async () => { - const lib = new FakeLibrary([collection(1, 100)], []); - const resolved = await freshFile(lib, 404); - expect(lib.freshCalls).toBe(1); - expect(resolved).toBeUndefined(); + // The stand-in's cache holds what the server holds once `fresh()` has run. + it("freshFile on a cache that holds the collections refreshes, then looks the file up in the cache", async () => { + const lib = threeCollections(); + await lib.fresh(); + + const found = await freshFile(lib, 30); + expect(lib.freshCalls).toBe(2); + expect(lib.freshCollectionsCalls).toBe(0); + expect(lib.freshFilesCalls).toEqual([]); + expect(found?.collectionID).toBe(3); + }); + + it("freshFile on an empty cache returns undefined for an unknown file, after reading every collection's files", async () => { + const lib = threeCollections(); + + const found = await freshFile(lib, 404); + expect(lib.freshCalls).toBe(0); + expect(lib.freshFilesCalls).toEqual([1, 2, 3]); + expect(found).toBeUndefined(); }); }); diff --git a/test/library/content-library.test.ts b/test/library/content-library.test.ts index 48a1094..92e0985 100644 --- a/test/library/content-library.test.ts +++ b/test/library/content-library.test.ts @@ -253,6 +253,12 @@ describe("Library content wiring", () => { await expect( lib.thumbnails.ensure({ fileIDs: [1], priority: "visible" }), ).rejects.toThrow(/content cache/i); + await expect(lib.originalOf(file(1, 1))).rejects.toThrow( + /content cache/i, + ); + await expect(lib.thumbnailOf(file(1, 1))).rejects.toThrow( + /content cache/i, + ); await lib.close(); }); }); diff --git a/test/library/content.test.ts b/test/library/content.test.ts index b461742..1809116 100644 --- a/test/library/content.test.ts +++ b/test/library/content.test.ts @@ -375,6 +375,25 @@ describe("ContentCache.original / thumbnail", () => { await cache.open(); await expect(cache.original(999)).rejects.toThrow(/unknown file/i); }); + + // `getFile` knows no file here, so the cache can fetch only the file it is + // given. + it("originalOf and thumbnailOf fetch and cache the file they are given", async () => { + const { cache, source } = buildCache({ files: [] }); + await cache.open(); + + const original = await cache.originalOf(file(5)); + const thumbnail = await cache.thumbnailOf(file(5)); + + expect(source.originalCalls).toEqual([5]); + expect(source.thumbnailCalls).toEqual([5]); + expect(original.path).toBe(join(cacheDir, "originals", "5.jpg")); + expect(thumbnail.path).toBe(join(cacheDir, "thumbnails", "5.jpg")); + expect(cache.pathsFor(5)).toEqual({ + originalPath: original.path, + thumbnailPath: thumbnail.path, + }); + }); }); describe("ContentCache.ensureThumbnails", () => { -- 2.54.0