quak get and get-thumb on an empty cache stop fetching collections once they find the file (closes #222)
check / check (push) Waiting to run
check / check (push) Waiting to run
`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
This commit was merged in pull request #226.
This commit is contained in:
+82
-39
@@ -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<number, Uint8Array>();
|
||||
|
||||
// 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<string, unknown> => {
|
||||
}): { row: Record<string, unknown>; 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<string, unknown>[] = [];
|
||||
const filesByCollection: Record<number, Record<string, unknown>[]> = {};
|
||||
const fileCiphertexts: Record<number, Uint8Array> = {};
|
||||
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);
|
||||
|
||||
|
||||
@@ -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`,
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+41
-27
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in New Issue
Block a user