Free unused attachments, and keep versions by count rather than by age
Two rules governed version retention: the newest 50 per note, and a sweep of anything past 90 days. The second was wrong. The count already bounds what a note can accumulate, so age only ever deleted history nothing had replaced — and it bit hardest exactly where history is most wanted, a note left alone for a quarter of a year losing its whole past for having done nothing. It is off unless VERSION_RETENTION_DAYS is set. How many versions to keep is now a notebook's own setting, falling back to the server default when left empty. A reference notebook can keep fifty of everything while a scratch one keeps three. That makes freeing attachments answerable. A picture taken out of a note cannot be deleted straight away: the versions from before still show it, and restoring one would find a hole. It is released once no current note and no kept version refers to it — which, with retention by count, is when the last version holding it is pushed out. The server cannot look for itself, bodies being encrypted, so the client reports a note's attachments on every save as it already does for links, and a version inherits what the note held when it was snapshotted — by definition what that version contains. Two things make it safe rather than merely plausible. Silence is not "uses nothing": a note written before this existed has no rows, which looks identical to one with no pictures, so nothing is released for a user while any of their notes has yet to report — the prefetch that fills the search index reports for them on its first pass. And an upload has a row before the note using it does, so recent attachments are left alone. POST /api/admin/housekeeping runs the sweep on demand. Caught while testing: the same block of markup had been pasted into the create-notebook dialog as well, where the state it referenced did not exist — the dialog crashed on open. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
a32d6eba99
commit
3acb8b66a4
12 files changed
+507
-28
No files matched your search
@@ -0,0 +1,167 @@
|
||||
/* When a picture is freed, and when it must not be.
|
||||
*
|
||||
* Removing an image from a note cannot delete the file straight away: the
|
||||
* versions from before still show it, and restoring one would find a hole. It
|
||||
* goes when the last version holding it is pushed out by newer ones — which is
|
||||
* the first moment nothing can ask for it back.
|
||||
*
|
||||
* Versions expire by count, not by age, so this is testable by simply editing
|
||||
* the note enough times. */
|
||||
import {
|
||||
api,
|
||||
check,
|
||||
createNote,
|
||||
createNotebook,
|
||||
launch,
|
||||
report,
|
||||
setTitle,
|
||||
typeInEditor,
|
||||
withFreshUser,
|
||||
} from "./lib.mjs";
|
||||
|
||||
const PNG =
|
||||
"iVBORw0KGgoAAAANSUhEUgAAAAMAAAADCAYAAABWKLW/AAAAFElEQVR42mP8z8BQz0AEYBxVSF+FABJADveWkH6oAAAAAElFTkSuQmCC";
|
||||
|
||||
const browser = await launch();
|
||||
let session;
|
||||
try {
|
||||
session = await withFreshUser(browser, "attlife");
|
||||
const p = session.page;
|
||||
// Housekeeping is an administrator's to ask for, not the test user's.
|
||||
const adminApi = (method, path, body) => api(session.adminPage, method, path, body);
|
||||
await createNotebook(p, "Anhang Buch");
|
||||
|
||||
/* Two versions per note, so the picture falls out of history quickly. */
|
||||
await p.locator("button").filter({ hasText: "Anhang Buch" }).first().click({ button: "right" });
|
||||
await p.waitForTimeout(600);
|
||||
await p.getByText("Edit", { exact: true }).click();
|
||||
await p.waitForTimeout(600);
|
||||
await p.getByLabel("Versions per note").fill("2");
|
||||
await p.getByRole("button", { name: /Save|Speichern/ }).click();
|
||||
await p.waitForTimeout(1500);
|
||||
|
||||
const notebooks = await api(p, "GET", "/notebooks");
|
||||
check(notebooks.body[0].max_versions === 2, "the notebook keeps its own version limit", String(notebooks.body[0].max_versions));
|
||||
|
||||
/* ── a note with a picture ── */
|
||||
await p.locator("button").filter({ hasText: "Anhang Buch" }).first().click();
|
||||
await p.waitForTimeout(800);
|
||||
await createNote(p);
|
||||
await setTitle(p, "Mit Bild");
|
||||
// Long, so that replacing it later counts as a large deletion. Edits inside
|
||||
// five minutes are otherwise folded into one version, and the snapshot holding
|
||||
// the picture would never be taken.
|
||||
await typeInEditor(p, "Ein ausführlicher Text, der später ersetzt wird. ".repeat(8));
|
||||
await p.locator(".ProseMirror").click();
|
||||
await p.keyboard.press("Control+End");
|
||||
await p.evaluate(async (b64) => {
|
||||
const bytes = Uint8Array.from(atob(b64), (c) => c.charCodeAt(0));
|
||||
const dt = new DataTransfer();
|
||||
dt.items.add(new File([bytes], "dot.png", { type: "image/png" }));
|
||||
document
|
||||
.querySelector(".ProseMirror")
|
||||
.dispatchEvent(new ClipboardEvent("paste", { clipboardData: dt, bubbles: true, cancelable: true }));
|
||||
}, PNG);
|
||||
await p.waitForTimeout(3500);
|
||||
|
||||
const noteId = (await api(p, "GET", "/notes")).body[0].id;
|
||||
const attachmentId = await p.evaluate(() => {
|
||||
const img = document.querySelector(".ProseMirror img");
|
||||
return img?.getAttribute("data-attachment") || null;
|
||||
});
|
||||
check(!!attachmentId, "the pasted picture became an attachment", String(attachmentId));
|
||||
|
||||
/* Attachments are content-addressed and served `immutable`, so the browser
|
||||
* answers a repeat request from its own cache and would report a deleted file
|
||||
* as still present. The cache is right to do that; the question here is what
|
||||
* the server holds, so this one asks it directly. */
|
||||
const stillThere = async () =>
|
||||
(await p.evaluate(async (id) => {
|
||||
const r = await fetch(`/api/attachments/${id}`, {
|
||||
cache: "no-store",
|
||||
headers: { Authorization: "Bearer " + localStorage.getItem("inknote_token") },
|
||||
});
|
||||
return r.status;
|
||||
}, attachmentId)) !== 404;
|
||||
check(await stillThere(), "and can be fetched while the note uses it");
|
||||
|
||||
/* ── take it out again ── */
|
||||
// Shrinking a note by more than half bypasses the coalescing window, so the
|
||||
// state holding the picture is definitely kept as a version.
|
||||
await p.locator(".ProseMirror").click();
|
||||
await p.keyboard.press("Control+A");
|
||||
await p.keyboard.type("Raus.");
|
||||
await p.waitForTimeout(2800);
|
||||
|
||||
const body = (await api(p, "GET", `/notes/${noteId}`)).body.content;
|
||||
check(!body.includes(attachmentId), "the note no longer refers to it");
|
||||
check(
|
||||
await stillThere(),
|
||||
"but it is kept, because the versions from before still show it — deleting now would leave a hole in the history"
|
||||
);
|
||||
|
||||
/** Which of the kept versions still carry the picture. */
|
||||
const versionsHolding = async () => {
|
||||
const list = (await api(p, "GET", `/notes/${noteId}/versions`)).body;
|
||||
const held = await Promise.all(
|
||||
list.map(async (v) => {
|
||||
const body = (await api(p, "GET", `/notes/${noteId}/versions/${v.id}`)).body.content || "";
|
||||
return body.includes(attachmentId) ? v.id : 0;
|
||||
})
|
||||
);
|
||||
return { total: list.length, holding: held.filter(Boolean) };
|
||||
};
|
||||
|
||||
const before = await versionsHolding();
|
||||
check(
|
||||
before.holding.length > 0,
|
||||
"a kept version really does still contain the picture — which is why it may not be deleted yet",
|
||||
`${before.holding.length} of ${before.total}`
|
||||
);
|
||||
|
||||
/* ── push the versions out ── */
|
||||
// With a limit of two, a handful of well-separated edits replaces them both.
|
||||
/* The churn goes through the API rather than the editor.
|
||||
*
|
||||
* Typing in the open note races the six-second poller, which can put an
|
||||
* earlier body back on screen and produce a version holding the picture again
|
||||
* — real behaviour, but noise for this test. What is under test here is the
|
||||
* server's bookkeeping: that versions inherit a note's attachments, that the
|
||||
* notebook's limit pushes them out, and that freeing follows. Each write grows
|
||||
* then shrinks the note, since a large deletion forces a snapshot instead of
|
||||
* being folded into the five-minute window.
|
||||
*/
|
||||
const writeBody = async (text) => {
|
||||
const doc = { type: "doc", content: [{ type: "paragraph", content: [{ type: "text", text }] }] };
|
||||
await api(p, "PUT", `/notes/${noteId}`, { content: JSON.stringify({ v: 2, doc }), attachments: [] });
|
||||
await p.waitForTimeout(150);
|
||||
};
|
||||
|
||||
let state = before;
|
||||
for (let i = 0; i < 8 && state.holding.length > 0; i++) {
|
||||
await writeBody(`Fassung ${i + 2}. `.repeat(20));
|
||||
await writeBody(`Kurz ${i + 2}.`);
|
||||
state = await versionsHolding();
|
||||
}
|
||||
|
||||
check(state.total <= 2, "the notebook's limit is what bounds the history", `${state.total} versions`);
|
||||
check(
|
||||
state.holding.length === 0,
|
||||
"editing on eventually pushes every version holding the picture out",
|
||||
`${state.holding.length} still hold it`
|
||||
);
|
||||
|
||||
/* The sweep runs on a timer in production; asked for directly here, with the
|
||||
in-flight grace lowered since nothing is in flight in a finished test. */
|
||||
const swept = await adminApi("POST", "/admin/housekeeping", { graceSeconds: 0 });
|
||||
check(swept.status === 200, "the sweep can be run on demand", String(swept.status));
|
||||
await p.waitForTimeout(1200);
|
||||
check(!(await stillThere()), "and the picture is finally released");
|
||||
} catch (e) {
|
||||
check(false, "suite ran to the end", e.message);
|
||||
} finally {
|
||||
await session?.cleanup();
|
||||
const code = report();
|
||||
await browser.close();
|
||||
process.exit(code);
|
||||
}
|
||||
Reference in new issue
Block a user