quak collections reads the collection list alone (closes #210)
check / check (push) Waiting to run
check / check (push) Waiting to run
On an empty cache the read commands opened their library with a refresh that fetched and decrypted the files of every collection in turn, printing nothing and storing nothing until it had them all, so `quak collections` seemed to hang on a large account. The new `lib.freshCollections()` is the fresh read of the collection list alone: one request, and nothing stored, so a later refresh still fetches every collection's files. The read commands open their library with the new `refreshOnOpen: false` and make only the refresh they ask for. A refresh sends a "progress" event after each collection whose files it fetched; the commands that wait on one print it on stderr, except with `--json`. Model: opus-5-5
This commit was merged in pull request #214.
This commit is contained in:
@@ -14,6 +14,9 @@
|
||||
* three.
|
||||
* 3. A refresh that fails rejects the fresh read (currency was unavailable),
|
||||
* while the default reads stay silent and keep serving the last good copy.
|
||||
* 4. `freshCollections()` reads the collection list alone, with one request
|
||||
* and no collection's files, and stores nothing, so the next refresh still
|
||||
* fetches the files of every collection (issue #210).
|
||||
*
|
||||
* The client is the same metadata-only mock the background-refresh tests use:
|
||||
* no crypto, no network, scripted pages, and a record of each call. A long
|
||||
@@ -270,3 +273,70 @@ describe("Library.fresh", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("Library.freshCollections", () => {
|
||||
let dir: string;
|
||||
let cacheDirectory: string;
|
||||
|
||||
beforeEach(() => {
|
||||
dir = mkdtempSync(join(tmpdir(), "quak-fresh-collections-"));
|
||||
cacheDirectory = join(dir, "cache");
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// A refresh fetches a collection's files only when the collection is new
|
||||
// or its updationTime has advanced past the stored copy's. Had the
|
||||
// collection list been stored, the next refresh would take every
|
||||
// collection for one it already holds the files of, and fetch none.
|
||||
it("asks for the collection list alone and stores nothing", async () => {
|
||||
const client = new MockClient();
|
||||
const page = {
|
||||
collections: [collection(2, 100), collection(1, 110)],
|
||||
deleted: [],
|
||||
cursor: 110,
|
||||
};
|
||||
client.collectionsQueue.push(page, page);
|
||||
client.filesFor(1, {
|
||||
files: [file(1001, 1, 90)],
|
||||
deleted: [],
|
||||
cursor: 90,
|
||||
});
|
||||
client.filesFor(2, {
|
||||
files: [file(2001, 2, 95)],
|
||||
deleted: [],
|
||||
cursor: 95,
|
||||
});
|
||||
|
||||
const lib = await Library.open({
|
||||
client,
|
||||
cacheDirectory,
|
||||
refreshOnOpen: false,
|
||||
refreshIntervalSeconds: SLOW_INTERVAL,
|
||||
});
|
||||
try {
|
||||
// One request, the whole list in the server's order, and no
|
||||
// collection's files.
|
||||
const collections = await lib.freshCollections();
|
||||
expect(collections.map((c) => c.id)).toEqual([2, 1]);
|
||||
expect(collections.map((c) => c.name)).toEqual([
|
||||
"album-2",
|
||||
"album-1",
|
||||
]);
|
||||
expect(client.collectionsSinceTimes).toEqual([0]);
|
||||
expect(client.filesCalls).toEqual([]);
|
||||
expect(lib.listCollections()).toEqual([]);
|
||||
|
||||
// The next refresh still asks from the beginning and fetches the
|
||||
// files of both collections.
|
||||
const reads = await lib.fresh();
|
||||
expect(client.collectionsSinceTimes).toEqual([0, 0]);
|
||||
expect(reads.photos.byID({ fileID: 1001 })?.fileID).toBe(1001);
|
||||
expect(reads.photos.byID({ fileID: 2001 })?.fileID).toBe(2001);
|
||||
} finally {
|
||||
await lib.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -26,6 +26,9 @@
|
||||
* opening.
|
||||
* 8. A save failure that leaves RAM ahead of disk keeps `status().lastError`
|
||||
* set and keeps retrying the write; a later empty refresh does not clear it.
|
||||
* 9. With `refreshOnOpen: false`, `open()` starts no refresh at all, and a
|
||||
* refresh reports through `onProgress` each collection whose files it
|
||||
* fetched.
|
||||
*
|
||||
* The client is a mock: no crypto, no network. It serves scripted pages and
|
||||
* records the `sinceTime` each call carried so cursor threading is provable.
|
||||
@@ -581,6 +584,107 @@ describe("Library.open and background refresh", () => {
|
||||
}
|
||||
});
|
||||
|
||||
// A caller that asks for each refresh it needs, as the CLI's read commands
|
||||
// do, opens without one: on an empty cache the refresh `open()` would
|
||||
// await fetches the files of every collection, which `quak collections`
|
||||
// never needs (issue #210).
|
||||
it("starts no refresh as it opens when refreshOnOpen is false", async () => {
|
||||
const client = new MockClient();
|
||||
client.collectionsQueue.push({
|
||||
collections: [collection(1, 100)],
|
||||
deleted: [],
|
||||
cursor: 100,
|
||||
});
|
||||
|
||||
const lib = await Library.open({
|
||||
client,
|
||||
cacheDirectory,
|
||||
refreshOnOpen: false,
|
||||
refreshIntervalSeconds: Infinity,
|
||||
});
|
||||
try {
|
||||
expect(client.collectionsSinceTimes).toEqual([]);
|
||||
expect(lib.listCollections()).toEqual([]);
|
||||
expect(lib.status().lastRefreshAt).toBeUndefined();
|
||||
|
||||
// The refresh comes when the caller asks for it.
|
||||
await lib.fresh();
|
||||
expect(client.collectionsSinceTimes).toEqual([0]);
|
||||
expect(lib.listCollections().map((c) => c.id)).toEqual([1]);
|
||||
} finally {
|
||||
await lib.close();
|
||||
}
|
||||
});
|
||||
|
||||
// A refresh fetches the files of each collection that changed, one
|
||||
// collection after another, and stores nothing until it has them all. On
|
||||
// a large account that takes a while, so it reports each collection whose
|
||||
// files it has fetched, and how many it will fetch in all (issue #210).
|
||||
it("reports each collection whose files a refresh fetched", async () => {
|
||||
const events: RefreshEvent[] = [];
|
||||
const client = new MockClient();
|
||||
client.collectionsQueue.push({
|
||||
collections: [
|
||||
collection(1, 100),
|
||||
collection(2, 100),
|
||||
collection(3, 100),
|
||||
],
|
||||
deleted: [],
|
||||
cursor: 100,
|
||||
});
|
||||
|
||||
const lib = await Library.open({
|
||||
client,
|
||||
cacheDirectory,
|
||||
refreshIntervalSeconds: Infinity,
|
||||
onProgress: (e) => events.push(e),
|
||||
});
|
||||
try {
|
||||
expect(events).toEqual([
|
||||
{ operation: "refresh", status: "started" },
|
||||
{
|
||||
operation: "refresh",
|
||||
status: "progress",
|
||||
fetched: 1,
|
||||
total: 3,
|
||||
},
|
||||
{
|
||||
operation: "refresh",
|
||||
status: "progress",
|
||||
fetched: 2,
|
||||
total: 3,
|
||||
},
|
||||
{
|
||||
operation: "refresh",
|
||||
status: "progress",
|
||||
fetched: 3,
|
||||
total: 3,
|
||||
},
|
||||
{ operation: "refresh", status: "done" },
|
||||
]);
|
||||
|
||||
// Only collection 2 changed since, so the next refresh fetches
|
||||
// its files alone and counts one.
|
||||
events.length = 0;
|
||||
client.collectionsQueue.push({
|
||||
collections: [collection(2, 200)],
|
||||
deleted: [],
|
||||
cursor: 200,
|
||||
});
|
||||
await lib.fresh();
|
||||
expect(events.filter((e) => e.status === "progress")).toEqual([
|
||||
{
|
||||
operation: "refresh",
|
||||
status: "progress",
|
||||
fetched: 1,
|
||||
total: 1,
|
||||
},
|
||||
]);
|
||||
} finally {
|
||||
await lib.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps a save failure visible until a save actually succeeds", async () => {
|
||||
const saveSpy = vi
|
||||
.spyOn(MetadataStore.prototype, "save")
|
||||
|
||||
Reference in New Issue
Block a user