From 3acb8b66a42252e7cf38fadef8987c3269b1b147 Mon Sep 17 00:00:00 2001 From: marc Date: Sat, 8 Aug 2026 22:22:46 +0200 Subject: [PATCH] Free unused attachments, and keep versions by count rather than by age MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .env.example | 4 +- backend/server.js | 187 ++++++++++++++++++++++++-- e2e/attachments-lifecycle.mjs | 167 +++++++++++++++++++++++ e2e/lib.mjs | 2 + e2e/run.sh | 2 +- frontend/src/editor/content.ts | 16 +++ frontend/src/editor/useNoteEditor.jsx | 5 +- frontend/src/hooks/useNoteData.ts | 4 +- frontend/src/syncEngine.ts | 55 +++++++- frontend/src/types.ts | 2 + frontend/src/ui/NotebookModals.jsx | 44 +++++- readme.md | 47 ++++++- 12 files changed, 507 insertions(+), 28 deletions(-) create mode 100644 e2e/attachments-lifecycle.mjs diff --git a/.env.example b/.env.example index 9e0d731..02f4772 100644 --- a/.env.example +++ b/.env.example @@ -27,7 +27,7 @@ INKNOTE_ADMIN_PASSWORD= # INKNOTE_MAX_PDF_MB=200 # imported PDFs, limited separately # INKNOTE_UID=1000 # must own INKNOTE_DATA_DIR # INKNOTE_GID=1000 -# MAX_VERSIONS_PER_NOTE=50 -# VERSION_RETENTION_DAYS=90 +# MAX_VERSIONS_PER_NOTE=50 # default; each notebook can set its own +# VERSION_RETENTION_DAYS= # off: versions expire by count, not by age # VERSION_COALESCE_MINUTES=5 # ALLOWED_ORIGINS= # only if the UI is on another host diff --git a/backend/server.js b/backend/server.js index 6b26da7..c138382 100644 --- a/backend/server.js +++ b/backend/server.js @@ -201,6 +201,27 @@ db.exec(` -- Past states of a note, so an accidental overwrite can be undone after the -- fact. Content is encrypted here exactly as it is in the notes table. + /* Which attachments a note's current body uses, and which each kept version + * used. The server cannot read either — both are encrypted — so the client + * reports the note's set on every save, and a version inherits whatever the + * note held at the moment it was snapshotted, which is by definition what + * that version contains. + * + * Together these are what make freeing an attachment safe: a picture removed + * from a note is still in the versions that came before, and deleting the file + * would leave a hole in history that restoring cannot fill. */ + CREATE TABLE IF NOT EXISTS note_attachments ( + note_id TEXT NOT NULL REFERENCES notes(id) ON DELETE CASCADE, + attachment_id TEXT NOT NULL, + PRIMARY KEY (note_id, attachment_id) + ); + + CREATE TABLE IF NOT EXISTS version_attachments ( + version_id INTEGER NOT NULL REFERENCES note_versions(id) ON DELETE CASCADE, + attachment_id TEXT NOT NULL, + PRIMARY KEY (version_id, attachment_id) + ); + CREATE TABLE IF NOT EXISTS note_versions ( id INTEGER PRIMARY KEY AUTOINCREMENT, note_id TEXT NOT NULL REFERENCES notes(id) ON DELETE CASCADE, @@ -225,6 +246,8 @@ db.exec(` created_at TEXT DEFAULT (datetime('now')) ); + CREATE INDEX IF NOT EXISTS idx_note_attachments_att ON note_attachments(attachment_id); + CREATE INDEX IF NOT EXISTS idx_version_attachments_att ON version_attachments(attachment_id); CREATE INDEX IF NOT EXISTS idx_notes_user ON notes(user_id); CREATE UNIQUE INDEX IF NOT EXISTS idx_attachments_owner_hash ON attachments(user_id, sha256); CREATE INDEX IF NOT EXISTS idx_note_versions ON note_versions(note_id, id DESC); @@ -299,6 +322,23 @@ if (!tableColumns("notes").includes("drawing_strokes")) { // A PDF note points at the attachment holding its file. The old pdf_path column // stored a filesystem path instead, which is why it never survived a container // rebuild; attachments are content-addressed, encrypted and deduplicated. +// How many versions a notebook keeps per note. Null falls back to the global +// default, so a notebook only carries a number once someone has chosen one. +/* Whether this note's client has told us which attachments it uses. + * + * Notes written before that existed have no reference rows, which is + * indistinguishable from a note using no pictures at all — and freeing on that + * basis would delete images that are still on screen. Nothing is released for a + * user until every one of their notes has reported, which the background + * prefetch takes care of on the first pass after an upgrade. */ +if (!tableColumns("notes").includes("attachments_reported")) { + db.exec("ALTER TABLE notes ADD COLUMN attachments_reported INTEGER NOT NULL DEFAULT 0"); +} + +if (!tableColumns("notebooks").includes("max_versions")) { + db.exec("ALTER TABLE notebooks ADD COLUMN max_versions INTEGER"); +} + if (!tableColumns("notes").includes("pdf_attachment_id")) { db.exec("ALTER TABLE notes ADD COLUMN pdf_attachment_id TEXT"); } @@ -673,6 +713,26 @@ app.put("/api/admin/users/:id/password", authMiddleware, requireAdmin, async (re } }); +/** + * Runs the housekeeping sweep now instead of waiting for its timer. + * + * `graceSeconds` lowers the delay that protects an upload still in flight — its + * row exists before the client has had a chance to report the note that uses it. + * Only an administrator can ask, and only on their own instance; the default + * leaves the protection alone. + */ +app.post("/api/admin/housekeeping", authMiddleware, requireAdmin, (req, res) => { + const grace = + req.body?.graceSeconds === undefined + ? ORPHAN_GRACE_MS + : Math.max(0, Number(req.body.graceSeconds) || 0) * 1000; + pruneSyncLog(); + pruneOldVersions(); + pruneUnreferencedAttachments(grace); + pruneOrphanAttachments(grace); + res.json({ success: true }); +}); + app.delete("/api/admin/users/:id", authMiddleware, requireAdmin, (req, res) => { const target = db.prepare("SELECT * FROM users WHERE id = ?").get(req.params.id); if (!target) return res.status(404).json({ error: "User not found" }); @@ -710,15 +770,28 @@ app.post("/api/notebooks", authMiddleware, (req, res) => { }); app.put("/api/notebooks/:id", authMiddleware, (req, res) => { - const { name, coverIndex, sortOrder } = req.body; + const { name, coverIndex, sortOrder, maxVersions } = req.body; const nb = db.prepare("SELECT * FROM notebooks WHERE id = ? AND user_id = ?").get(req.params.id, req.userId); if (!nb) return res.status(404).json({ error: "Not found" }); + // null clears the notebook's own setting and puts it back on the default; + // undefined leaves whatever it had. + let versions; + if (maxVersions === null) versions = null; + else if (maxVersions !== undefined) { + const n = Number(maxVersions); + if (!Number.isFinite(n) || n < 1) return res.status(400).json({ error: "maxVersions must be at least 1" }); + versions = Math.min(Math.round(n), MAX_VERSIONS_CEILING); + } + db.prepare("UPDATE notebooks SET name = COALESCE(?, name), cover_index = COALESCE(?, cover_index), sort_order = COALESCE(?, sort_order), updated_at = datetime('now'), version = version + 1 WHERE id = ?") .run(name, coverIndex, sortOrder, req.params.id); + if (maxVersions !== undefined) { + db.prepare("UPDATE notebooks SET max_versions = ? WHERE id = ?").run(versions, req.params.id); + } broadcastToUser(req.userId, { type: "notebook_updated", data: { id: req.params.id, name, coverIndex } }, req.deviceId); - res.json({ success: true }); + res.json({ success: true, defaultMaxVersions: MAX_VERSIONS_PER_NOTE }); }); app.delete("/api/notebooks/:id", authMiddleware, (req, res) => { @@ -743,16 +816,37 @@ const num = (name, fallback) => { }; const VERSION_COALESCE_MS = num("VERSION_COALESCE_MINUTES", 5) * 60 * 1000; const MAX_VERSIONS_PER_NOTE = num("MAX_VERSIONS_PER_NOTE", 50); -const VERSION_RETENTION_DAYS = num("VERSION_RETENTION_DAYS", 90); +/* A per-notebook setting is a number a user types; it needs an upper bound. */ +const MAX_VERSIONS_CEILING = 500; +/* Zero means "never expire by age", and that is the default. + * + * Versions used to be dropped after 90 days as well as being capped per note. + * The cap already bounds how much a note can accumulate, so the age rule only + * ever deleted history that nothing had replaced — and it hit hardest exactly + * where history is most wanted: a note untouched for a quarter of a year lost + * its whole past, having done nothing but sit there. Set the variable to a + * number of days to bring it back. */ +const VERSION_RETENTION_DAYS = num("VERSION_RETENTION_DAYS", 0); const sqliteTimeToMs = (t) => (t ? Date.parse(`${String(t).replace(" ", "T")}Z`) : 0); +/** How many versions this note's notebook keeps — its own setting, or the default. */ +function versionLimitFor(noteId) { + const row = db + .prepare( + "SELECT b.max_versions AS limit_ FROM notes n LEFT JOIN notebooks b ON b.id = n.notebook_id WHERE n.id = ?" + ) + .get(noteId); + const own = Number(row?.limit_); + return Number.isFinite(own) && own > 0 ? Math.min(own, MAX_VERSIONS_CEILING) : MAX_VERSIONS_PER_NOTE; +} + function pruneNoteVersions(noteId) { db.prepare(` DELETE FROM note_versions WHERE note_id = ? AND id NOT IN ( SELECT id FROM note_versions WHERE note_id = ? ORDER BY id DESC LIMIT ? - )`).run(noteId, noteId, MAX_VERSIONS_PER_NOTE); + )`).run(noteId, noteId, versionLimitFor(noteId)); } function snapshotNote(userId, noteId, deviceId, { force = false, reason = "edit" } = {}) { @@ -768,9 +862,18 @@ function snapshotNote(userId, noteId, deviceId, { force = false, reason = "edit" if (last && Date.now() - sqliteTimeToMs(last.created_at) < VERSION_COALESCE_MS) return; } - db.prepare(`INSERT INTO note_versions (note_id, user_id, title, content_encrypted, drawing_strokes, device_id, reason) + const inserted = db.prepare(`INSERT INTO note_versions (note_id, user_id, title, content_encrypted, drawing_strokes, device_id, reason) VALUES (?, ?, ?, ?, ?, ?, ?)`) .run(noteId, userId, note.title, note.content_encrypted, note.drawing_strokes, deviceId, reason); + + /* The version inherits the note's attachments as they stand right now, which + * is exactly what the body being snapshotted refers to — the snapshot is taken + * before the incoming write is applied, and so is this. Without it, removing a + * picture from a note would free the file while older versions still showed it. */ + db.prepare( + "INSERT OR IGNORE INTO version_attachments (version_id, attachment_id) SELECT ?, attachment_id FROM note_attachments WHERE note_id = ?" + ).run(inserted.lastInsertRowid, noteId); + pruneNoteVersions(noteId); } @@ -871,8 +974,24 @@ const replaceNoteLinks = db.transaction((userId, sourceId, links) => { } }); +/* An upper bound on what one note may claim, so a malformed client cannot + * write an unbounded number of rows. */ +const MAX_ATTACHMENTS_PER_NOTE = 500; + +/** Records which attachments a note's body currently uses. */ +const replaceNoteAttachments = db.transaction((noteId, ids) => { + db.prepare("DELETE FROM note_attachments WHERE note_id = ?").run(noteId); + db.prepare("UPDATE notes SET attachments_reported = 1 WHERE id = ?").run(noteId); + const insert = db.prepare( + "INSERT OR IGNORE INTO note_attachments (note_id, attachment_id) VALUES (?, ?)" + ); + for (const id of ids.slice(0, MAX_ATTACHMENTS_PER_NOTE)) { + if (typeof id === "string" && id) insert.run(noteId, id); + } +}); + app.put("/api/notes/:id", authMiddleware, (req, res) => { - const { title, content, notebookId, starred, baseUpdatedAt, links } = req.body; + const { title, content, notebookId, starred, baseUpdatedAt, links, attachments } = req.body; const note = db.prepare("SELECT * FROM notes WHERE id = ? AND user_id = ?").get(req.params.id, req.userId); if (!note) return res.status(404).json({ error: "Not found" }); @@ -905,6 +1024,7 @@ app.put("/api/notes/:id", authMiddleware, (req, res) => { .run(title, encContent, notebookId, starred !== undefined ? (starred ? 1 : 0) : null, req.deviceId, req.params.id); if (Array.isArray(links)) replaceNoteLinks(req.userId, req.params.id, links); + if (Array.isArray(attachments)) replaceNoteAttachments(req.params.id, attachments); const fresh = db.prepare("SELECT updated_at FROM notes WHERE id = ?").get(req.params.id); broadcastToUser(req.userId, { type: "note_updated", data: { id: req.params.id, title } }, req.deviceId); @@ -1376,9 +1496,10 @@ function pruneSyncLog() { } catch (err) { console.error("[prune] failed:", err.message); } } -/* Per-note count is capped on write; this only clears out versions of notes - * that stopped being edited long ago. */ +/* Off unless VERSION_RETENTION_DAYS is set — the per-note count is what bounds + * growth, and see the note on that constant for why age is a poor second rule. */ function pruneOldVersions() { + if (VERSION_RETENTION_DAYS <= 0) return; try { const r = db.prepare(`DELETE FROM note_versions WHERE created_at < datetime('now', '-${VERSION_RETENTION_DAYS} days')`).run(); if (r.changes) console.log(` 🧹 pruned ${r.changes} note versions older than ${VERSION_RETENTION_DAYS} days`); @@ -1391,7 +1512,51 @@ function pruneOldVersions() { * Only files that have sat untouched for an hour are considered, so an upload * in flight — written to disk just before its row is inserted — is never hit. */ const ORPHAN_GRACE_MS = 60 * 60 * 1000; -function pruneOrphanAttachments() { +/** + * Deletes attachments nothing refers to any more. + * + * An attachment is free once no current note body and no kept version mentions + * it. Both halves matter: a picture taken out of a note is still in the versions + * from before it was removed, and freeing it then would leave a hole that + * restoring cannot fill. + * + * So a deleted image does not vanish at once. It goes when the last version + * holding it is pushed out by newer ones — which is the point at which nothing + * can ask for it back. + * + * Rows are only considered once the note has actually reported its attachments; + * a client that has never sent the list would otherwise look like a note using + * nothing at all. + */ +function pruneUnreferencedAttachments(graceMs = ORPHAN_GRACE_MS) { + try { + const rows = db + .prepare( + `SELECT a.id, a.sha256 FROM attachments a + WHERE a.created_at < datetime('now', ?) + AND NOT EXISTS (SELECT 1 FROM note_attachments na WHERE na.attachment_id = a.id) + AND NOT EXISTS (SELECT 1 FROM version_attachments va WHERE va.attachment_id = a.id) + -- Nothing is released while any of this user's notes has yet to say + -- what it uses: silence there is not the same as "uses nothing". + AND NOT EXISTS ( + SELECT 1 FROM notes n + WHERE n.user_id = a.user_id AND n.deleted_at IS NULL AND n.attachments_reported = 0 + )` + ) + .all(`-${Math.round(graceMs / 1000)} seconds`); + + if (rows.length === 0) return; + const del = db.prepare("DELETE FROM attachments WHERE id = ?"); + for (const row of rows) del.run(row.id); + console.log(` 🧹 released ${rows.length} attachment(s) no note or version refers to`); + // The files behind them go in the sweep below, which only removes blobs no + // row points at — including copies still shared with another user's row. + } catch (err) { + console.error("[prune] attachment references failed:", err.message); + } +} + +function pruneOrphanAttachments(graceMs = ORPHAN_GRACE_MS) { try { if (!fs.existsSync(ATTACHMENT_DIR)) return; const referenced = new Set(db.prepare("SELECT sha256 FROM attachments").all().map((r) => r.sha256)); @@ -1400,7 +1565,7 @@ function pruneOrphanAttachments() { if (referenced.has(name)) continue; const full = path.join(ATTACHMENT_DIR, name); try { - if (Date.now() - fs.statSync(full).mtimeMs < ORPHAN_GRACE_MS) continue; + if (Date.now() - fs.statSync(full).mtimeMs < graceMs) continue; fs.unlinkSync(full); removed++; } catch { /* leave anything we cannot inspect alone */ } @@ -1409,7 +1574,7 @@ function pruneOrphanAttachments() { } catch (err) { console.error("[prune] attachments failed:", err.message); } } -const housekeeping = () => { pruneSyncLog(); pruneOldVersions(); pruneOrphanAttachments(); }; +const housekeeping = () => { pruneSyncLog(); pruneOldVersions(); pruneUnreferencedAttachments(); pruneOrphanAttachments(); }; housekeeping(); setInterval(housekeeping, 24 * 60 * 60 * 1000).unref(); diff --git a/e2e/attachments-lifecycle.mjs b/e2e/attachments-lifecycle.mjs new file mode 100644 index 0000000..49ea1c9 --- /dev/null +++ b/e2e/attachments-lifecycle.mjs @@ -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); +} diff --git a/e2e/lib.mjs b/e2e/lib.mjs index 4432807..d20a9fe 100644 --- a/e2e/lib.mjs +++ b/e2e/lib.mjs @@ -118,6 +118,8 @@ export async function withFreshUser(browser, prefix = "e2e") { page, user, pass, + /** Signed in as the administrator — for anything the test user may not do. */ + adminPage, async cleanup() { await api(adminPage, "DELETE", `/admin/users/${created.body.id}`); await adminPage.context().close(); diff --git a/e2e/run.sh b/e2e/run.sh index 6db33ce..4ea47f7 100755 --- a/e2e/run.sh +++ b/e2e/run.sh @@ -13,7 +13,7 @@ cd "$(dirname "$0")" IMAGE="mcr.microsoft.com/playwright:v1.49.0-noble" URL="${INKNOTE_URL:-http://localhost:3000}" SUITES=("$@") -[ ${#SUITES[@]} -eq 0 ] && SUITES=(admin editor search-attachments history drawing export conflict pdf) +[ ${#SUITES[@]} -eq 0 ] && SUITES=(admin editor search-attachments history drawing export conflict pdf attachments-lifecycle) mkdir -p shots if [ ! -d node_modules ]; then diff --git a/frontend/src/editor/content.ts b/frontend/src/editor/content.ts index 1c85095..1df945c 100644 --- a/frontend/src/editor/content.ts +++ b/frontend/src/editor/content.ts @@ -122,6 +122,22 @@ export function extractNoteLinks(doc: DocNode, selfId?: string): string[] { return [...ids]; } +/** + * Ids of every attachment this document embeds. + * + * The server stores content encrypted and cannot look inside, so this is the + * only account of what a note still uses. It is what lets a picture removed from + * a note eventually be freed — and, just as important, what stops one still in + * use from being deleted. + */ +export function extractAttachmentIds(doc: DocNode): string[] { + const ids = new Set(); + walk(doc, (n) => { + if (n.type === "image" && n.attrs?.attachmentId) ids.add(n.attrs.attachmentId); + }); + return [...ids]; +} + /** Plain text of a document — used for list previews and local search. */ export function docToText(doc: unknown, limit = 400): string { const parts: string[] = []; diff --git a/frontend/src/editor/useNoteEditor.jsx b/frontend/src/editor/useNoteEditor.jsx index a509cea..fcb9159 100644 --- a/frontend/src/editor/useNoteEditor.jsx +++ b/frontend/src/editor/useNoteEditor.jsx @@ -2,7 +2,7 @@ import { useEffect, useMemo, useRef, useCallback } from "react"; import { useEditor, EditorContent } from "@tiptap/react"; import { buildExtensions } from "./extensions.js"; -import { parseNoteContent, serializeNoteContent, extractNoteLinks } from "./content"; +import { parseNoteContent, serializeNoteContent, extractNoteLinks, extractAttachmentIds } from "./content"; import { uploadImage, ACCEPTED_TYPES } from "./attachments.js"; import "./editor.css"; @@ -78,6 +78,9 @@ export function useNoteEditor({ noteId, content, editable = true, onChange, sear onChangeRef.current?.({ content: serializeNoteContent(doc), links: extractNoteLinks(doc, noteId), + // Travels with every save: the server cannot read the body, so this is + // its only account of which pictures the note still uses. + attachments: extractAttachmentIds(doc), }); }, // Rebuilt per note so each one starts with its own document and history. diff --git a/frontend/src/hooks/useNoteData.ts b/frontend/src/hooks/useNoteData.ts index 8140488..54d40e9 100644 --- a/frontend/src/hooks/useNoteData.ts +++ b/frontend/src/hooks/useNoteData.ts @@ -19,6 +19,8 @@ export interface NoteEdit { content?: string; /** Ids of notes and notebooks the body links to; stored server-side for backlinks. */ links?: string[]; + /** Ids of attachments the body embeds, so unused ones can eventually be freed. */ + attachments?: string[]; drawingStrokes?: StrokeDoc | null; } @@ -212,7 +214,7 @@ export function useNoteData(user: User | null, activeNote: string | null): NoteD if (edit.content !== undefined) { const content = edit.content; setNoteContents((p) => ({ ...p, [id]: { ...(p[id] ?? EMPTY_CONTENT), content } })); - void sync.updateNoteContent(id, content, edit.links); + void sync.updateNoteContent(id, content, edit.links, edit.attachments); searchIndex.setEntry(id, notesRef.current.find((n) => n.id === id)?.title || "", content); } if (edit.drawingStrokes !== undefined) { diff --git a/frontend/src/syncEngine.ts b/frontend/src/syncEngine.ts index 171f4e3..f6df26f 100644 --- a/frontend/src/syncEngine.ts +++ b/frontend/src/syncEngine.ts @@ -71,6 +71,7 @@ interface ServerNotebook { name: string; cover_index?: number; sort_order?: number; + max_versions?: number | null; } interface ServerNote { @@ -104,6 +105,8 @@ export interface NoteUpdate { notebookId?: string | null; starred?: boolean; links?: string[]; + /** Attachments the body embeds, so ones nothing uses can be released. */ + attachments?: string[]; /** * The server revision this edit was written on top of. * @@ -399,6 +402,7 @@ export async function fullSync(): Promise { name: nb.name, cover: nb.cover_index || 0, sortOrder: nb.sort_order || 0, + maxVersions: nb.max_versions ?? null, })); const serverNbIds = new Set(nbs.map((nb) => nb.id)); const unsyncedNbs = localNbs.filter((nb) => nb._pendingCreate && !serverNbIds.has(nb.id)); @@ -459,7 +463,10 @@ function toNoteMeta(n: ServerNote, trashed: boolean): NoteMeta { /** Every kind of change that can wait offline. */ type QueuedAction = | { action: "createNotebook"; data: { localId: string; name: string; cover: number; sortOrder: number } } - | { action: "updateNotebook"; data: { id: string; name?: string; cover?: number; sortOrder?: number } } + | { + action: "updateNotebook"; + data: { id: string; name?: string; cover?: number; sortOrder?: number; maxVersions?: number | null }; + } | { action: "deleteNotebook"; data: { id: string } } | { action: "createNote"; data: { localId: string; title: string; folderId: string | null } } | { action: "updateNote"; data: { id: string; updates: NoteUpdate } } @@ -542,6 +549,7 @@ async function processEntry(entry: QueueEntry): Promise { name: a.data.name, coverIndex: a.data.cover, sortOrder: a.data.sortOrder, + ...(a.data.maxVersions !== undefined ? { maxVersions: a.data.maxVersions } : {}), }); break; case "deleteNotebook": @@ -695,6 +703,9 @@ export async function updateNotebook(id: string, updates: Partial): Pr name: updates.name, coverIndex: updates.cover, sortOrder: updates.sortOrder, + // null is meaningful here — it clears the notebook's own setting — so it + // has to travel, while undefined must stay away and change nothing. + ...(updates.maxVersions !== undefined ? { maxVersions: updates.maxVersions } : {}), }); return; } catch (e) { @@ -703,7 +714,13 @@ export async function updateNotebook(id: string, updates: Partial): Pr } await enqueue({ action: "updateNotebook", - data: { id, name: updates.name, cover: updates.cover, sortOrder: updates.sortOrder }, + data: { + id, + name: updates.name, + cover: updates.cover, + sortOrder: updates.sortOrder, + maxVersions: updates.maxVersions, + }, }); notifyStatus("pending"); } @@ -851,7 +868,12 @@ export async function updateNoteTitle(id: string, title: string): Promise }); } -export async function updateNoteContent(id: string, content: string, links?: string[]): Promise { +export async function updateNoteContent( + id: string, + content: string, + links?: string[], + attachments?: string[] +): Promise { const ex = await db.getNoteContent(id); await db.putNoteContent(id, content, ex?.drawingData || null, ex?.drawingStrokes || null); const note = await db.getNote(id); @@ -860,9 +882,9 @@ export async function updateNoteContent(id: string, content: string, links?: str // The server cannot read the encrypted body, so outgoing links travel with it. // baseUpdatedAt is the revision this copy came from — see NoteUpdate. - const updates: NoteUpdate = Array.isArray(links) - ? { content, links, baseUpdatedAt: ex?.updatedAt ?? null } - : { content, baseUpdatedAt: ex?.updatedAt ?? null }; + const updates: NoteUpdate = { content, baseUpdatedAt: ex?.updatedAt ?? null }; + if (Array.isArray(links)) updates.links = links; + if (Array.isArray(attachments)) updates.attachments = attachments; debounce(`content_${id}`, async () => { if (_online && api.token && !note?._pendingCreate) { @@ -1081,6 +1103,26 @@ export async function createPdfNote( * search with almost nothing to look at on a fresh device. This walks the notes * with no cached body (or a stale one) and fills them in, deliberately slowly so * it never competes with what the user is doing. */ +/** + * Tells the server which attachments a note body uses. + * + * Notes written before attachment references existed have never said, and the + * server cannot look inside an encrypted body to find out. Until every note has + * reported, nothing is ever released — so this pass, riding along with the + * prefetch that fills the search index, is what eventually lets unused pictures + * be freed. Sent even when empty: "uses nothing" is the answer that matters. + */ +async function reportAttachments(noteId: string, content: string): Promise { + try { + const { parseNoteContent, extractAttachmentIds } = await import("./editor/content"); + const { parsingExtensions } = await import("./editor/extensions.js"); + const ids = extractAttachmentIds(parseNoteContent(content, parsingExtensions)); + await api.put(`/notes/${noteId}`, { attachments: ids }); + } catch { + /* one note that will not parse must not stop the pass */ + } +} + export interface PrefetchProgress { id: string; content: string; @@ -1114,6 +1156,7 @@ export async function prefetchAllContent({ const d = await api.get(`/notes/${n.id}`); const c = shapeContent(d); await db.putNoteContent(n.id, c.content, c.drawingData, c.drawingStrokes, c.updatedAt); + await reportAttachments(n.id, c.content); fetched++; onProgress?.({ id: n.id, content: c.content, fetched, total: pending.length }); } catch { diff --git a/frontend/src/types.ts b/frontend/src/types.ts index f04460f..e3eae34 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -69,6 +69,8 @@ export interface Notebook { name: string; cover: number; sortOrder: number; + /** Versions kept per note here; null means the server's default. */ + maxVersions?: number | null; _pendingCreate?: boolean; } diff --git a/frontend/src/ui/NotebookModals.jsx b/frontend/src/ui/NotebookModals.jsx index 2704eb1..e96b00f 100644 --- a/frontend/src/ui/NotebookModals.jsx +++ b/frontend/src/ui/NotebookModals.jsx @@ -4,6 +4,15 @@ import { COVERS } from "./constants.jsx"; export function EditNotebookModal({ notebook, onClose, onSave, dark }) { const [name, setName] = useState(notebook.name); const [sel, setSel] = useState(notebook.cover); + // Empty means "use the server's default", which is why it is a string here and + // not a number with a stand-in value. + const [versions, setVersions] = useState(notebook.maxVersions == null ? "" : String(notebook.maxVersions)); + + const save = () => { + if (!name.trim()) return; + const n = versions.trim() === "" ? null : Math.max(1, Math.round(Number(versions))); + onSave({ name: name.trim(), cover: sel, maxVersions: Number.isFinite(n) ? n : null }); + }; const T = useMemo( () => ({ bg: dark ? "#161b22" : "#fff", @@ -69,9 +78,40 @@ export function EditNotebookModal({ notebook, onClose, onSave, dark }) { boxSizing: "border-box", }} onKeyDown={(e) => { - if (e.key === "Enter" && name.trim()) onSave({ name: name.trim(), cover: sel }); + if (e.key === "Enter") save(); }} /> +
+ + setVersions(e.target.value)} + placeholder="Vorgabe" + aria-label="Versions per note" + style={{ + width: 110, + padding: "8px 12px", + borderRadius: 10, + border: `1px solid ${T.border}`, + background: dark ? "#0d1117" : "#f6f8fa", + color: T.text, + fontSize: 14, + outline: "none", + boxSizing: "border-box", + }} + onKeyDown={(e) => { + if (e.key === "Enter") save(); + }} + /> +
+ Ältere Fassungen werden erst gelöscht, wenn neue sie verdrängen — nicht nach Zeit. Leer lassen für + die Vorgabe des Servers. +
+
Cover style
@@ -115,7 +155,7 @@ export function EditNotebookModal({ notebook, onClose, onSave, dark }) {