Expire trash after 30 days, and make the deadline something you can see
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 6s
CI & Build / Python tests (push) Successful in 12s
CI & Build / Build & push image (push) Successful in 44s
Desktop (Tauri) / Tauri desktop (Linux) (push) Failing after 1m45s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m12s
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 6s
CI & Build / Python tests (push) Successful in 12s
CI & Build / Build & push image (push) Successful in 44s
Desktop (Tauri) / Tauri desktop (Linux) (push) Failing after 1m45s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m12s
Trash had no end. A note sat in /trash until someone emptied it by hand, and its attachment BYTES sat on disk the whole time — the pile-up the operator asked about. Nothing purged; there was no scheduler at all. Retention is server-owned: `trash_retention_days` (default 30, 0 = keep forever) in the settings registry, so it lands in admin Settings with no migration and takes effect without a restart. A background sweep started in before_serving does the work. Clients learn about a purge the way they learn about any deletion — as a tombstone on the delta feed. An auto-purge nobody can see coming is data loss on a timer, so the window is now visible: /api/config publishes it, notes carry `deleted_at`, Trash leads with the policy, and each card counts down. The countdown rounds DOWN — saying "1 day left" for a note with ten minutes on the clock is the one error here that actually costs someone a note. Three things this turned up on the way: - `DELETE /api/notes/<id>` hard-deleted the row, leaving no tombstone at all. A permanent delete in the web UI never reached a linked device, which would keep its copy forever and push it back on the next edit. It now purges through the same path as everything else. - The purge left `note_revisions` and `note_link_previews` behind. A revision holds the full body, so the text of a "permanently deleted" note was still sitting in the database. - `deleted_at` now SURVIVES a purge instead of being cleared. It's still true, and it means every query that says "not trashed" excludes tombstones for free — without it a content-less row reads as a perfectly normal active note and shows up on the board as a blank card. Desktop keeps its own clock only when there's nobody else to keep one: the sweep runs at startup on an UNLINKED device and refuses otherwise. A linked client that expired notes on its own schedule could destroy something the server was deliberately keeping, then push that delete upstream. Local policy must never outrank the server's — so it also adopts the server's window for the countdown rather than showing its offline default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi
This commit is contained in:
@@ -0,0 +1,205 @@
|
||||
//! Trash retention for a device with no server (M11.3).
|
||||
//!
|
||||
//! The server owns this policy whenever there IS one: a linked client learns about
|
||||
//! every permanent deletion from the delta feed, as a tombstone, and does exactly
|
||||
//! what it's told. This module exists for the case the server can't cover — an
|
||||
//! offline-only install, where trash would otherwise sit forever and the attachment
|
||||
//! bytes with it.
|
||||
//!
|
||||
//! Which is why the sweep refuses to run while linked. If it didn't, a device could
|
||||
//! decide on its own that a note had expired, destroy it, and then push that delete
|
||||
//! upstream — overruling a server that was deliberately keeping it (retention off, or
|
||||
//! a longer window than this constant). A client's local policy must never outrank
|
||||
//! the server's.
|
||||
|
||||
use chrono::{DateTime, Duration, Utc};
|
||||
use rusqlite::Connection;
|
||||
|
||||
use super::store;
|
||||
use crate::sync::state;
|
||||
|
||||
/// The window an unlinked device uses. Matches the server's default so a device that
|
||||
/// later links doesn't see its trash behave differently from one that always was.
|
||||
pub const LOCAL_RETENTION_DAYS: i64 = 30;
|
||||
|
||||
/// Purge trash older than `retention_days`. Returns how many notes went.
|
||||
///
|
||||
/// `now` is a parameter so the window arithmetic is testable without waiting a month.
|
||||
pub fn sweep_expired_trash(
|
||||
conn: &Connection,
|
||||
retention_days: i64,
|
||||
now: DateTime<Utc>,
|
||||
) -> rusqlite::Result<usize> {
|
||||
if retention_days <= 0 {
|
||||
return Ok(0);
|
||||
}
|
||||
let cutoff = now - Duration::days(retention_days);
|
||||
let expired: Vec<String> = {
|
||||
let mut stmt =
|
||||
conn.prepare("SELECT id, trashed_at FROM notes WHERE trashed = 1 AND trashed_at IS NOT NULL")?;
|
||||
let rows = stmt.query_map([], |r| Ok((r.get::<_, String>(0)?, r.get::<_, String>(1)?)))?;
|
||||
rows.filter_map(|row| {
|
||||
let (id, stamped) = row.ok()?;
|
||||
// PARSED, not string-compared. The server writes `+00:00` offsets and this
|
||||
// client writes `Z`, so two timestamps for the same instant don't sort
|
||||
// against each other as text — and the failure would be silent.
|
||||
let trashed_at = DateTime::parse_from_rfc3339(&stamped).ok()?;
|
||||
// An unparseable or missing timestamp means "age unknown", and the only
|
||||
// safe reading of that is to keep the note. Deleting on a guess is the one
|
||||
// outcome nobody can undo.
|
||||
(trashed_at.with_timezone(&Utc) < cutoff).then_some(id)
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
for id in &expired {
|
||||
// Through delete_forever, so a `pending_deletes` tombstone is recorded. That's
|
||||
// right even here: while unlinked this device holds the only copy, so if it
|
||||
// links later the server should learn the note was deleted, not re-send it.
|
||||
store::delete_forever(conn, id)?;
|
||||
}
|
||||
Ok(expired.len())
|
||||
}
|
||||
|
||||
/// The startup sweep: runs only on an unlinked device (see the module note).
|
||||
/// Returns `None` when it didn't run because the device is linked.
|
||||
pub fn sweep_if_unlinked(conn: &Connection) -> rusqlite::Result<Option<usize>> {
|
||||
if state::read(conn)?.server_url.is_some() {
|
||||
return Ok(None);
|
||||
}
|
||||
sweep_expired_trash(conn, LOCAL_RETENTION_DAYS, Utc::now()).map(Some)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::local::schema;
|
||||
|
||||
fn db() -> Connection {
|
||||
let conn = Connection::open_in_memory().expect("in-memory db");
|
||||
schema::migrate(&conn).expect("migrate");
|
||||
conn
|
||||
}
|
||||
|
||||
/// A trashed note stamped `trashed_at` days ago, in the format the CLIENT writes
|
||||
/// (`...Z`, millisecond precision — see `store::now`).
|
||||
fn trashed_note(conn: &Connection, id: &str, days_ago: i64) {
|
||||
let stamped = (Utc::now() - Duration::days(days_ago))
|
||||
.to_rfc3339_opts(chrono::SecondsFormat::Millis, true);
|
||||
conn.execute(
|
||||
"INSERT INTO notes (id, title, body, created_at, updated_at, trashed, trashed_at)
|
||||
VALUES (?1, 'T', 'B', ?2, ?2, 1, ?2)",
|
||||
rusqlite::params![id, stamped],
|
||||
)
|
||||
.expect("insert");
|
||||
}
|
||||
|
||||
fn note_count(conn: &Connection) -> i64 {
|
||||
conn.query_row("SELECT COUNT(*) FROM notes", [], |r| r.get(0))
|
||||
.expect("count")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn purges_trash_past_the_window_and_keeps_the_rest() {
|
||||
let conn = db();
|
||||
trashed_note(&conn, "old", 40);
|
||||
trashed_note(&conn, "fresh", 3);
|
||||
let purged = sweep_expired_trash(&conn, 30, Utc::now()).expect("sweep");
|
||||
assert_eq!(purged, 1);
|
||||
assert_eq!(note_count(&conn), 1, "only the expired note should go");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_note_exactly_at_the_boundary_survives() {
|
||||
// Strictly older than the cutoff, so the note trashed 30 days ago gets its
|
||||
// full 30 days rather than being cut a moment short.
|
||||
let conn = db();
|
||||
trashed_note(&conn, "boundary", 30);
|
||||
assert_eq!(sweep_expired_trash(&conn, 30, Utc::now()).expect("sweep"), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retention_off_purges_nothing() {
|
||||
let conn = db();
|
||||
trashed_note(&conn, "ancient", 4000);
|
||||
assert_eq!(sweep_expired_trash(&conn, 0, Utc::now()).expect("sweep"), 0);
|
||||
assert_eq!(sweep_expired_trash(&conn, -1, Utc::now()).expect("sweep"), 0);
|
||||
assert_eq!(note_count(&conn), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_untrashed_note_is_never_swept() {
|
||||
let conn = db();
|
||||
conn.execute(
|
||||
"INSERT INTO notes (id, title, body, created_at, updated_at, trashed)
|
||||
VALUES ('live', 'T', 'B', '2020-01-01T00:00:00.000Z', '2020-01-01T00:00:00.000Z', 0)",
|
||||
[],
|
||||
)
|
||||
.expect("insert");
|
||||
assert_eq!(sweep_expired_trash(&conn, 30, Utc::now()).expect("sweep"), 0);
|
||||
assert_eq!(note_count(&conn), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unparseable_timestamp_keeps_the_note() {
|
||||
// "Age unknown" must never resolve to "delete it".
|
||||
let conn = db();
|
||||
conn.execute(
|
||||
"INSERT INTO notes (id, title, body, created_at, updated_at, trashed, trashed_at)
|
||||
VALUES ('weird', 'T', 'B', '2020-01-01T00:00:00.000Z', '2020-01-01T00:00:00.000Z', 1, 'not a date')",
|
||||
[],
|
||||
)
|
||||
.expect("insert");
|
||||
assert_eq!(sweep_expired_trash(&conn, 30, Utc::now()).expect("sweep"), 0);
|
||||
assert_eq!(note_count(&conn), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_server_style_offset_timestamp_is_understood() {
|
||||
// The server serializes with a `+00:00` offset, not `Z`. Comparing those as
|
||||
// strings would quietly never match — this is the case that catches it.
|
||||
let conn = db();
|
||||
let stamped = (Utc::now() - Duration::days(40)).to_rfc3339();
|
||||
conn.execute(
|
||||
"INSERT INTO notes (id, title, body, created_at, updated_at, trashed, trashed_at)
|
||||
VALUES ('server', 'T', 'B', ?1, ?1, 1, ?1)",
|
||||
rusqlite::params![stamped],
|
||||
)
|
||||
.expect("insert");
|
||||
assert_eq!(sweep_expired_trash(&conn, 30, Utc::now()).expect("sweep"), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_purged_note_leaves_a_pending_delete_behind() {
|
||||
// Without the tombstone, linking this device later would let the server
|
||||
// re-send a note the user already destroyed here.
|
||||
let conn = db();
|
||||
trashed_note(&conn, "old", 40);
|
||||
sweep_expired_trash(&conn, 30, Utc::now()).expect("sweep");
|
||||
let pending: i64 = conn
|
||||
.query_row(
|
||||
"SELECT COUNT(*) FROM pending_deletes WHERE entity = 'note' AND id = 'old'",
|
||||
[],
|
||||
|r| r.get(0),
|
||||
)
|
||||
.expect("count");
|
||||
assert_eq!(pending, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_linked_device_does_not_sweep() {
|
||||
// The whole safety rule: with a server present, purging is the server's call.
|
||||
let conn = db();
|
||||
trashed_note(&conn, "old", 400);
|
||||
state::set_link(&conn, "https://notes.example", "token").expect("link");
|
||||
assert_eq!(sweep_if_unlinked(&conn).expect("sweep"), None);
|
||||
assert_eq!(note_count(&conn), 1, "the note must survive on a linked device");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unlinked_device_sweeps() {
|
||||
let conn = db();
|
||||
trashed_note(&conn, "old", 400);
|
||||
assert_eq!(sweep_if_unlinked(&conn).expect("sweep"), Some(1));
|
||||
assert_eq!(note_count(&conn), 0);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user