quak files COLLECTION fetches that collection's files alone (closes #217)
check / check (push) Waiting to run
check / check (push) Waiting to run
`quak files COLLECTION` used to wait for a full refresh, which on an empty cache fetched the files of every collection before it listed one. It now reads the collection list, then calls the new `Library.refreshCollection()`. That brings one collection up to date in the store as a refresh would, and saves it. It runs in the same slot as a refresh, so the two never run at the same time, and `close()` waits for it. It never stores a collection that a refresh has already dropped. It leaves the refresh cursor alone. `open()` therefore awaits the first refresh on a cache whose cursor is still 0. `quak files` with no argument still waits for a refresh. Model: opus-5-5
This commit is contained in:
@@ -17,6 +17,12 @@
|
||||
* 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).
|
||||
* 5. `refreshCollection()` brings one collection up to date in the store
|
||||
* (issue #217). It leaves the stored cursor where it was, so the next
|
||||
* refresh still fetches the files of every other collection that changed.
|
||||
* It never runs at the same time as a refresh, and never brings back a
|
||||
* collection that a refresh deleted. `close()` waits for it. A library
|
||||
* opened on a cache that only it wrote awaits its first refresh.
|
||||
*
|
||||
* 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
|
||||
@@ -30,6 +36,7 @@ import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { Library } from "../../src/library/index.js";
|
||||
import { MetadataStore } from "../../src/library/store.js";
|
||||
import type { CollectionsPage, FilesPage } from "../../src/client.js";
|
||||
import type { Collection, EnteFile } from "../../src/model/types.js";
|
||||
|
||||
@@ -144,6 +151,26 @@ const openSeeded = async (
|
||||
return { client, lib };
|
||||
};
|
||||
|
||||
// Hold the client's next files request until `release()` is called. `started`
|
||||
// resolves once that request has been sent.
|
||||
const holdNextFilesRequest = (
|
||||
client: MockClient,
|
||||
): { started: Promise<void>; release: () => void } => {
|
||||
let release!: () => void;
|
||||
const held = new Promise<void>((r) => (release = r));
|
||||
let sent!: () => void;
|
||||
const started = new Promise<void>((r) => (sent = r));
|
||||
const realFilesSince = client.filesSince.bind(client);
|
||||
client.filesSince = async (args) => {
|
||||
client.filesSince = realFilesSince;
|
||||
const page = await realFilesSince(args);
|
||||
sent();
|
||||
await held;
|
||||
return page;
|
||||
};
|
||||
return { started, release };
|
||||
};
|
||||
|
||||
describe("Library.fresh", () => {
|
||||
let dir: string;
|
||||
let cacheDirectory: string;
|
||||
@@ -340,3 +367,216 @@ describe("Library.freshCollections", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("Library.refreshCollection", () => {
|
||||
let dir: string;
|
||||
let cacheDirectory: string;
|
||||
|
||||
beforeEach(() => {
|
||||
dir = mkdtempSync(join(tmpdir(), "quak-refresh-collection-"));
|
||||
cacheDirectory = join(dir, "cache");
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// Collections 1 and 2 are stored at updationTime 100, with a stored cursor
|
||||
// of 100, and then both change on the server.
|
||||
it("fetches the files of one changed collection and leaves the others to the next refresh", async () => {
|
||||
const client = new MockClient();
|
||||
client.collectionsQueue.push({
|
||||
collections: [collection(1, 100), collection(2, 100)],
|
||||
deleted: [],
|
||||
cursor: 100,
|
||||
});
|
||||
client.filesFor(1, {
|
||||
files: [file(1001, 1, 90)],
|
||||
deleted: [],
|
||||
cursor: 90,
|
||||
});
|
||||
client.filesFor(2, {
|
||||
files: [file(2001, 2, 90)],
|
||||
deleted: [],
|
||||
cursor: 90,
|
||||
});
|
||||
const lib = await Library.open({
|
||||
client,
|
||||
cacheDirectory,
|
||||
refreshIntervalSeconds: SLOW_INTERVAL,
|
||||
});
|
||||
try {
|
||||
client.filesCalls.length = 0;
|
||||
client.filesFor(1, {
|
||||
files: [file(1002, 1, 110)],
|
||||
deleted: [],
|
||||
cursor: 110,
|
||||
});
|
||||
client.filesFor(2, {
|
||||
files: [file(2002, 2, 120)],
|
||||
deleted: [],
|
||||
cursor: 120,
|
||||
});
|
||||
|
||||
// Collection 1's files alone, since the stored copy.
|
||||
await lib.refreshCollection(collection(1, 110));
|
||||
expect(client.filesCalls).toEqual([
|
||||
{ collectionID: 1, sinceTime: 100 },
|
||||
]);
|
||||
expect(lib.listFiles(1).map((f) => f.id)).toEqual([1001, 1002]);
|
||||
expect(lib.listFiles(2).map((f) => f.id)).toEqual([2001]);
|
||||
|
||||
// The stored copy is now current, so no request.
|
||||
await lib.refreshCollection(collection(1, 110));
|
||||
expect(client.filesCalls).toHaveLength(1);
|
||||
|
||||
// The next refresh asks from the stored cursor, 100, so it still
|
||||
// sees collection 2 changed and fetches its files.
|
||||
client.collectionsQueue.push({
|
||||
collections: [collection(1, 110), collection(2, 120)],
|
||||
deleted: [],
|
||||
cursor: 120,
|
||||
});
|
||||
await lib.fresh();
|
||||
expect(client.collectionsSinceTimes).toEqual([0, 100]);
|
||||
expect(client.filesCalls).toEqual([
|
||||
{ collectionID: 1, sinceTime: 100 },
|
||||
{ collectionID: 2, sinceTime: 100 },
|
||||
]);
|
||||
expect(lib.listFiles(2).map((f) => f.id)).toEqual([2001, 2002]);
|
||||
} finally {
|
||||
await lib.close();
|
||||
}
|
||||
});
|
||||
|
||||
// Collection 1 is stored at 100. `refreshCollection()` asks for it at 110,
|
||||
// which adds file 1002. While that files request waits, a fresh read
|
||||
// starts a refresh. The server tells that refresh that collection 1 was
|
||||
// deleted at 130.
|
||||
it("holds off a refresh that starts during the files request, which then deletes the collection", async () => {
|
||||
const { client, lib } = await openSeeded(cacheDirectory);
|
||||
const files = holdNextFilesRequest(client);
|
||||
try {
|
||||
client.filesFor(1, {
|
||||
files: [file(1002, 1, 110)],
|
||||
deleted: [],
|
||||
cursor: 110,
|
||||
});
|
||||
client.collectionsQueue.push({
|
||||
collections: [],
|
||||
deleted: [1],
|
||||
cursor: 130,
|
||||
});
|
||||
|
||||
const refreshing = lib.refreshCollection(collection(1, 110));
|
||||
await files.started;
|
||||
const reading = lib.fresh();
|
||||
// The refresh has not sent its collections request yet.
|
||||
await new Promise((r) => setImmediate(r));
|
||||
expect(client.collectionsSinceTimes).toEqual([0]);
|
||||
|
||||
files.release();
|
||||
await refreshing;
|
||||
await reading;
|
||||
expect(client.collectionsSinceTimes).toEqual([0, 100]);
|
||||
expect(lib.getCollection(1)).toBeUndefined();
|
||||
expect(lib.listFiles(1)).toEqual([]);
|
||||
} finally {
|
||||
files.release();
|
||||
await lib.close();
|
||||
}
|
||||
});
|
||||
|
||||
// `freshCollections()` lists collection 1 at 110. Before
|
||||
// `refreshCollection()` is called, a refresh learns that it was deleted at
|
||||
// 130.
|
||||
it("stores nothing for a collection that a refresh deleted before the call", async () => {
|
||||
const { client, lib } = await openSeeded(cacheDirectory);
|
||||
try {
|
||||
client.collectionsQueue.push(
|
||||
{ collections: [collection(1, 110)], deleted: [], cursor: 110 },
|
||||
{ collections: [], deleted: [1], cursor: 130 },
|
||||
);
|
||||
const [listed] = await lib.freshCollections();
|
||||
await lib.fresh();
|
||||
client.filesCalls.length = 0;
|
||||
|
||||
await lib.refreshCollection(listed);
|
||||
expect(client.filesCalls).toEqual([]);
|
||||
expect(lib.getCollection(1)).toBeUndefined();
|
||||
} finally {
|
||||
await lib.close();
|
||||
}
|
||||
});
|
||||
|
||||
// The caller may remove the cache directory once `close()` resolves, so
|
||||
// the cache write must have landed by then.
|
||||
it("close() waits for a refreshCollection() in flight", async () => {
|
||||
const { client, lib } = await openSeeded(cacheDirectory);
|
||||
const files = holdNextFilesRequest(client);
|
||||
client.filesFor(1, {
|
||||
files: [file(1002, 1, 110)],
|
||||
deleted: [],
|
||||
cursor: 110,
|
||||
});
|
||||
|
||||
const refreshing = lib.refreshCollection(collection(1, 110));
|
||||
await files.started;
|
||||
let closed = false;
|
||||
const closing = lib.close().then(() => {
|
||||
closed = true;
|
||||
});
|
||||
await new Promise((r) => setImmediate(r));
|
||||
expect(closed).toBe(false);
|
||||
|
||||
files.release();
|
||||
await closing;
|
||||
const saved = await MetadataStore.load(
|
||||
join(cacheDirectory, "metadata.json"),
|
||||
);
|
||||
expect(saved.listFiles(1).map((f) => f.id)).toEqual([1001, 1002]);
|
||||
await refreshing;
|
||||
});
|
||||
|
||||
// `quak files 1` on an empty cache stores collection 1 alone. A library
|
||||
// opened later on that cache must not serve it as the whole account.
|
||||
it("open() awaits the first refresh on a cache that only refreshCollection() wrote", async () => {
|
||||
const client = new MockClient();
|
||||
client.filesFor(1, {
|
||||
files: [file(1001, 1, 90)],
|
||||
deleted: [],
|
||||
cursor: 90,
|
||||
});
|
||||
const first = await Library.open({
|
||||
client,
|
||||
cacheDirectory,
|
||||
refreshOnOpen: false,
|
||||
refreshIntervalSeconds: SLOW_INTERVAL,
|
||||
});
|
||||
await first.refreshCollection(collection(1, 100));
|
||||
await first.close();
|
||||
|
||||
client.collectionsQueue.push({
|
||||
collections: [collection(1, 100), collection(2, 100)],
|
||||
deleted: [],
|
||||
cursor: 100,
|
||||
});
|
||||
client.filesFor(2, {
|
||||
files: [file(2001, 2, 90)],
|
||||
deleted: [],
|
||||
cursor: 90,
|
||||
});
|
||||
const lib = await Library.open({
|
||||
client,
|
||||
cacheDirectory,
|
||||
refreshIntervalSeconds: SLOW_INTERVAL,
|
||||
});
|
||||
try {
|
||||
expect(lib.status().lastRefreshAt).toBeDefined();
|
||||
expect(lib.listCollections().map((c) => c.id)).toEqual([1, 2]);
|
||||
expect(lib.listFiles(2).map((f) => f.id)).toEqual([2001]);
|
||||
} finally {
|
||||
await lib.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user