diff --git a/apps/darkroom-android/Cargo.toml b/apps/darkroom-android/Cargo.toml index 5699149..c51e112 100644 --- a/apps/darkroom-android/Cargo.toml +++ b/apps/darkroom-android/Cargo.toml @@ -19,6 +19,10 @@ dr-ui.workspace = true # For `account::set_data_dir`: only the platform entry point knows where Android # lets this app keep files, and it must be set before any store is opened. dr-sync.workspace = true +# For the panic hook, and for `crash::set_state_dir` — Android has no XDG +# directories, so the entry point is the only place that knows where a crash +# record may be written. +dr-plat.workspace = true # Directly, not just through dr-ui: `android_main` takes an `AndroidApp` and # calls `slint::android::init`, both of which come from this crate. The backend # feature comes from dr-ui's target-specific dependency. diff --git a/apps/darkroom-android/src/lib.rs b/apps/darkroom-android/src/lib.rs index ac0b0ec..3e73805 100644 --- a/apps/darkroom-android/src/lib.rs +++ b/apps/darkroom-android/src/lib.rs @@ -39,9 +39,18 @@ fn android_main(app: slint::android::AndroidApp) { // worker thread that panics is invisible: the process survives, the // channel it was writing to closes, and the UI reports only that // something "failed unexpectedly" with no way to find out what. - std::panic::set_hook(Box::new(|info| { - log::error!("panic: {info}"); - })); + // + // This used to be one `log::error!` of the raw panic, which had two + // problems: logcat is a ring buffer that is gone by the time a user + // reports anything, and the raw message can carry a document URI naming + // their library or a credential a library interpolated into an error + // (NFR-SEC-2). `dr_plat::crash` writes a redacted record to disk and logs + // the redacted form. Nothing uploads it. + // + // Before `set_state_dir` on purpose: the hook resolves the directory when + // it fires, so installing it first covers the startup below rather than + // leaving it uncovered. + dr_plat::crash::install(env!("CARGO_PKG_VERSION")); log::info!("DarkRoom v{}", env!("CARGO_PKG_VERSION")); @@ -54,6 +63,10 @@ fn android_main(app: slint::android::AndroidApp) { match app.internal_data_path() { Some(dir) => { log::info!("data dir: {}", dir.display()); + // Crash records go beside the account data rather than under it: + // both are app-private and neither is a cache, which is the whole + // distinction that matters here (see `dr_plat::crash::state_dir`). + dr_plat::crash::set_state_dir(dir.join("state")); dr_sync::account::set_data_dir(dir); } None => log::error!("no internal data path; settings will not persist"), diff --git a/apps/darkroom-desktop/Cargo.toml b/apps/darkroom-desktop/Cargo.toml index 238b7f6..630f2f2 100644 --- a/apps/darkroom-desktop/Cargo.toml +++ b/apps/darkroom-desktop/Cargo.toml @@ -7,6 +7,10 @@ license.workspace = true [dependencies] dr-ui.workspace = true +# For the panic hook alone. Directly rather than through dr-ui, because it has +# to be installed before `dr_ui::run` — a panic during startup is exactly the +# one this exists to catch. +dr-plat.workspace = true anyhow.workspace = true env_logger.workspace = true log.workspace = true diff --git a/apps/darkroom-desktop/src/main.rs b/apps/darkroom-desktop/src/main.rs index 05713c8..149a5ff 100644 --- a/apps/darkroom-desktop/src/main.rs +++ b/apps/darkroom-desktop/src/main.rs @@ -10,6 +10,14 @@ fn main() -> anyhow::Result<()> { )) .init(); + // Immediately after the logger and before anything that could fail. Until + // now a panic on desktop went to stderr and died with the terminal, which + // means every panic a user has ever hit was unreportable: the process + // survives (the panicking worker does not), a control goes dead, and there + // is nothing on disk to say why. The record is local and stays local — + // there is no upload path, by design; see `dr_plat::crash`. + dr_plat::crash::install(env!("CARGO_PKG_VERSION")); + log::info!("DarkRoom v{}", env!("CARGO_PKG_VERSION")); let paths: Vec = std::env::args().skip(1).map(PathBuf::from).collect(); diff --git a/core/dr-catalog/src/error.rs b/core/dr-catalog/src/error.rs index 0a8f0da..e88ec7f 100644 --- a/core/dr-catalog/src/error.rs +++ b/core/dr-catalog/src/error.rs @@ -1,14 +1,37 @@ -//! TRACES: NFR-ARCH-4 | NFR-R5 +//! TRACES: NFR-ARCH-4 | NFR-R5 | NFR-R6 //! Catalog errors. //! //! Typed and attached to the affected subject rather than panicking — a //! corrupt row or a failed job marks one image and lets the batch continue. +//! +//! # Why `From` is written by hand +//! +//! One class of SQLite failure is not about the statement that hit it: when +//! the file itself is damaged, *every* query fails, and which one the user +//! happened to trigger first says nothing. Before this, corruption reached the +//! interface as whatever `Sqlite(...)` the first failing query produced — +//! "database disk image is malformed" attached to a thumbnail refresh — and +//! there was nowhere to hang a recovery offer. +//! +//! So the conversion classifies rather than wraps: `SQLITE_CORRUPT` and +//! `SQLITE_NOTADB` become [`CatalogError::Corrupt`] wherever they arise, which +//! means a background job that trips over the damage reports the same thing +//! the startup check does (see [`crate::recovery`]). /// Something went wrong talking to the catalog. #[derive(Debug, thiserror::Error)] pub enum CatalogError { #[error("sqlite: {0}")] - Sqlite(#[from] rusqlite::Error), + Sqlite(#[source] rusqlite::Error), + + /// The catalog file is damaged. + /// + /// Its own variant because it is the one error with a *user-facing + /// remedy*: restore the NFR-R2 backup, or discard the index and rebuild it + /// from sources plus sidecars (NFR-R6, invariant §5.2.4). Every other + /// variant here is either a caller's mistake or a fact about one row. + #[error("the catalog file is damaged: {detail}")] + Corrupt { detail: String }, /// The catalog was written by a newer build. /// @@ -74,3 +97,38 @@ pub enum CatalogError { #[error("io: {0}")] Io(String), } + +impl From for CatalogError { + fn from(e: rusqlite::Error) -> Self { + if is_corruption(&e) { + // `to_string` rather than keeping the error: the detail is going + // into a dialog and into a log line, and the recovery path has no + // use for the rusqlite type once it knows the file is damaged. + CatalogError::Corrupt { + detail: e.to_string(), + } + } else { + CatalogError::Sqlite(e) + } + } +} + +/// Whether a SQLite failure means the *file* is damaged rather than the +/// statement wrong. +/// +/// `SQLITE_NOTADB` is included because it is what a truncated or overwritten +/// catalog produces — SQLite cannot read the header, so it declines to call it +/// a database at all. To a user those are the same accident, and the same two +/// offers answer both. +/// +/// Deliberately *not* included: `SQLITE_CANTOPEN` (a missing file, which +/// `Connection::open` fixes by creating one), `SQLITE_BUSY`, and +/// `SQLITE_IOERR` — a failing disk or a dropped network mount is a different +/// problem, and telling the user to rebuild their index would be a wrong +/// answer delivered confidently. +fn is_corruption(e: &rusqlite::Error) -> bool { + matches!( + e.sqlite_error_code(), + Some(rusqlite::ErrorCode::DatabaseCorrupt) | Some(rusqlite::ErrorCode::NotADatabase) + ) +} diff --git a/core/dr-catalog/src/lib.rs b/core/dr-catalog/src/lib.rs index 0ca953f..2cc9a86 100644 --- a/core/dr-catalog/src/lib.rs +++ b/core/dr-catalog/src/lib.rs @@ -21,6 +21,7 @@ //! - [`runner`] — the thing that drains it, driven by whoever owns the thread //! - [`trash`] — soft delete to a folder, then permanent delete //! - [`merge`] / [`sync`] — cross-device merging of collections and keywords +//! - [`recovery`] — backups, and the two offers made when this file is damaged //! //! # The one thing everything is designed around //! @@ -47,6 +48,7 @@ pub mod keywords; pub mod merge; pub mod query; pub mod rating; +pub mod recovery; pub mod runner; pub mod scan; pub mod schema; @@ -65,6 +67,7 @@ pub use keywords::{Coverage, Keyword, KeywordId, SelectionKeyword}; pub use merge::MergeReport; pub use query::{Query, Sort}; pub use rating::{Judgement, MAX_RATING}; +pub use recovery::Backup; // Not `runner::Budget`: `cache::Budget` already owns that name here and // means something else entirely (bytes on disk, not jobs in a slot). // Callers spell the work budget `runner::Budget`, where it is unambiguous. @@ -222,9 +225,23 @@ pub struct Catalog { impl Catalog { /// Open or create a catalog, migrating it forward if needed. + /// + /// Does **not** verify the file — see [`Self::open_verified`], and + /// [`recovery`] for why the check is bound to startup rather than to every + /// open. Damage this trips over on the way past is still reported as + /// [`CatalogError::Corrupt`] rather than as a stray SQLite error. pub fn open(path: &Path) -> Result { let conn = Connection::open(path)?; schema::configure(&conn)?; + // NFR-R2, and the reason it is *here*: a migration is the one routine + // operation that rewrites table structure, so it is the likeliest way + // this file becomes unreadable — and afterwards there is no + // pre-migration state left to copy. A failure to take the copy is + // logged rather than raised: a full disk must not be the thing that + // makes a library unopenable. + if let Err(e) = recovery::backup_before_migration(&conn, path) { + log::warn!("could not back up before migrating: {e}"); + } let from = schema::migrate(&conn)?; // A migration adds a column; it cannot know what the value should be // for rows that already existed. Backfilling on open is what stops @@ -235,6 +252,26 @@ impl Catalog { Ok(Catalog { conn }) } + /// TRACES: NFR-R6 + /// Open a catalog, checking the file first. + /// + /// What startup calls. On [`CatalogError::Corrupt`] the caller has a user + /// in front of it and must make the two offers [`recovery`] describes, + /// rather than reporting a SQLite message on a banner and carrying on into + /// a scan that would write into the damage. + /// + /// Checked *before* opening rather than after, because opening runs + /// migrations: a damaged catalog that happens to have an intact header + /// would otherwise be migrated — rewriting structure on top of structure + /// that is already wrong — before anybody asked whether it was sound. + pub fn open_verified(path: &Path) -> Result { + // A catalog that is not there yet is not damaged; `open` creates it. + if path.is_file() { + recovery::check_file(path)?; + } + Self::open(path) + } + /// An in-memory catalog, for tests and for a throwaway import preview. pub fn in_memory() -> Result { let conn = Connection::open_in_memory()?; diff --git a/core/dr-catalog/src/recovery.rs b/core/dr-catalog/src/recovery.rs new file mode 100644 index 0000000..cb1b185 --- /dev/null +++ b/core/dr-catalog/src/recovery.rs @@ -0,0 +1,662 @@ +//! TRACES: NFR-R2 | NFR-R6 +//! What to do once the index is already damaged. +//! +//! # Why this can be a small module +//! +//! Because of a property the rest of the catalog was built to keep: the +//! catalog is an *index*, not a source of truth (ARCH §6.12, invariant +//! §5.2.4). Sidecars beside the images hold the authoritative ratings, +//! keywords and edit graphs, for every catalogued image and whether or not a +//! remote account exists (FR-CAT-8). So the worst outcome available here is a +//! rescan — expensive, but not a loss. +//! +//! That is the second offer. The first is cheaper and loses nothing at all: a +//! backup, restored. +//! +//! # The one thing a rebuild does not recover +//! +//! **Collections.** A manual collection is a set of images the user assembled +//! by hand and nothing in the filesystem records it (`docs/catalog.md` §8.1) — +//! which is the whole reason the catalog file itself syncs. So the two offers +//! are not interchangeable, and the interface must not present them as if they +//! were: a restore keeps the user's collections, a rebuild does not. +//! +//! # When the check runs, and when it does not +//! +//! [`integrity_check`] reads every page of the database. That is affordable +//! once, at startup, where a failure has a user in front of it who can answer +//! a question — and it is *not* affordable on every [`Catalog::open`], which +//! this application does per background task, dozens of times a session. So +//! the check is bound to [`Catalog::open_verified`] rather than to `open`, +//! and the cheap half of the story — classifying `SQLITE_CORRUPT` and +//! `SQLITE_NOTADB` as [`CatalogError::Corrupt`] — happens for free on every +//! query through [`crate::error`]'s conversion. A background job that trips +//! over the damage first therefore reports the same thing the startup check +//! would have. +//! +//! [`Catalog::open`]: crate::Catalog::open +//! [`Catalog::open_verified`]: crate::Catalog::open_verified + +use std::path::{Path, PathBuf}; +use std::time::{SystemTime, UNIX_EPOCH}; + +use rusqlite::Connection; + +use crate::error::CatalogError; +use crate::schema; + +/// Directory backups live in, relative to the catalog file. +/// +/// Beside the catalog rather than in the cache directory, and that is the +/// point of the choice: this is the copy the user falls back on, and a cache +/// is a place the operating system is entitled to empty without asking +/// (see `library::data_root` for the same reasoning about sidecars). +const BACKUP_DIR: &str = "backups"; + +/// How many backups are kept. +/// +/// Small on purpose. A backup is a full copy of a catalog that is tens of +/// megabytes at 50k images, and the value of the third-oldest one is close to +/// zero: corruption is noticed at the next launch, not months later. What the +/// depth buys is protection against backing *up* the damage — if a corrupt +/// catalog is copied before anyone notices, the generation behind it is still +/// clean. +pub const KEEP_BACKUPS: usize = 3; + +/// Suffix given to a catalog that has been set aside as damaged. +/// +/// Kept rather than deleted. It costs disk this application would rather not +/// spend, and it is still the right call: `.sqlite` files have been recovered +/// by hand before, the user has not consented to a deletion, and NFR-R4's +/// instinct — never destroy what the user did not ask you to destroy — does +/// not stop applying at the catalog's edge. +const DAMAGED_SUFFIX: &str = "damaged"; + +/// One kept backup. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Backup { + pub path: PathBuf, + /// UTC seconds at which it was taken, read from the filename rather than + /// from the filesystem: a copy, a restore or a sync can rewrite an mtime, + /// and then the newest backup is not the one that looks newest. + pub taken_at: i64, + pub bytes: u64, +} + +/// Where backups for `catalog` are kept. +pub fn backup_dir(catalog: &Path) -> PathBuf { + catalog + .parent() + .unwrap_or_else(|| Path::new(".")) + .join(BACKUP_DIR) +} + +/// Check the database this connection is attached to. +/// +/// `quick_check` rather than `integrity_check`: the difference is that +/// `quick_check` skips verifying that every index agrees with its table, which +/// is the expensive half and the half this application least needs — every +/// index here is derivable, and `REINDEX` fixes one without anybody being +/// asked a question. What is left still reads every page, and catches the +/// damage that matters: torn b-trees, a bad freelist, a truncated file. +/// +/// Returns [`CatalogError::Corrupt`] carrying what SQLite said, so the message +/// the user sees is the diagnosis rather than a paraphrase of it. +pub fn integrity_check(conn: &Connection) -> Result<(), CatalogError> { + // The argument caps how many problems are reported. One is enough: the + // answer is the same whether the file has one damaged page or nine + // hundred, and an unbounded check on a badly damaged file can run for a + // very long time producing a list nobody will read. + let mut stmt = conn.prepare("PRAGMA quick_check(1)")?; + let rows: Vec = stmt + .query_map([], |r| r.get(0))? + .collect::, _>>()?; + + // A healthy database answers with the single row "ok". + if rows.len() == 1 && rows[0] == "ok" { + return Ok(()); + } + Err(CatalogError::Corrupt { + detail: rows.join("; "), + }) +} + +/// Check a catalog file that is not currently open. +/// +/// Used before a restore: a backup is only worth swapping in if it is sound, +/// and swapping in a second damaged file — leaving the user with no catalog +/// and no offer left — is the failure this exists to prevent. +pub fn check_file(path: &Path) -> Result<(), CatalogError> { + if !path.is_file() { + return Err(CatalogError::Io(format!("{} is missing", path.display()))); + } + // Read-write rather than read-only, which reads oddly for a check. A + // backup carries the WAL journal mode in its header because it was copied + // page-for-page from a WAL database, and SQLite cannot open one read-only + // without a shared-memory file it is then not allowed to create. Nothing + // here writes; the connection is opened, read, and dropped. + let conn = Connection::open(path)?; + integrity_check(&conn) +} + +/// Take a backup of the open catalog. +/// +/// Returns the file written. Older generations beyond [`KEEP_BACKUPS`] are +/// pruned, newest kept. +/// +/// Goes through `crate::sync::copy_to` — SQLite's own backup API after a +/// TRUNCATE checkpoint — rather than copying the file. A WAL database is not +/// one file, and `fs::copy` of the main file alone would silently back up a +/// state that is older than the catalog and possibly torn, which is the one +/// failure mode a backup cannot afford. +pub fn backup(conn: &Connection, catalog: &Path) -> Result { + let dir = backup_dir(catalog); + std::fs::create_dir_all(&dir) + .map_err(|e| CatalogError::Io(format!("creating {}: {e}", dir.display())))?; + + let dest = dir.join(format!("catalog-{}.sqlite", now())); + // A second backup within the same second would otherwise land on the first + // one's name. Rare, and only reachable from tests and a retry, but the + // result would be a half-overwritten backup rather than two. + if dest.exists() { + std::fs::remove_file(&dest) + .map_err(|e| CatalogError::Io(format!("replacing {}: {e}", dest.display())))?; + } + + // Dropped immediately: the copy is complete when `copy_to` returns, and + // holding the connection open would leave a `-wal` beside a file whose + // whole purpose is to be a single self-contained artefact. + drop(crate::sync::copy_to(conn, &dest)?); + + prune(catalog); + Ok(dest) +} + +/// Back up before a migration, if there is anything to back up. +/// +/// Called from [`Catalog::open`](crate::Catalog::open) between `configure` and +/// `migrate`. NFR-R2 asks for this and the reasoning is narrower than "backups +/// are prudent": a migration is the one routine operation that rewrites table +/// structure, so it is the likeliest way a catalog becomes unreadable, and it +/// is the one moment where the pre-change state is still on disk to be copied. +/// Afterwards there is nothing left to take a copy *of*. +/// +/// A no-op in the two cases where it would cost without buying anything: a +/// catalog already at [`schema::SCHEMA_VERSION`], and a brand-new file at +/// version 0 with no tables in it yet. +pub fn backup_before_migration(conn: &Connection, catalog: &Path) -> Result<(), CatalogError> { + let from: i64 = conn.query_row("PRAGMA user_version", [], |r| r.get(0))?; + if from == 0 || from >= schema::SCHEMA_VERSION { + return Ok(()); + } + let path = backup(conn, catalog)?; + log::info!( + "backed up catalog at v{from} to {} before migrating to v{}", + path.display(), + schema::SCHEMA_VERSION + ); + Ok(()) +} + +/// The backups available for `catalog`, newest first. +/// +/// Never fails: an unreadable or absent backup directory means there are no +/// backups, which is a fact about the offer to make rather than an error to +/// report on top of the corruption the user is already looking at. +pub fn backups(catalog: &Path) -> Vec { + let dir = backup_dir(catalog); + let Ok(entries) = std::fs::read_dir(&dir) else { + return Vec::new(); + }; + + let mut out: Vec = entries + .flatten() + .filter_map(|e| { + let path = e.path(); + let taken_at = timestamp_of(&path)?; + let bytes = e.metadata().ok()?.len(); + Some(Backup { + path, + taken_at, + bytes, + }) + }) + .collect(); + out.sort_by_key(|b| std::cmp::Reverse(b.taken_at)); + out +} + +/// Put a backup back in place of the damaged catalog. +/// +/// **Every connection to `catalog` must be closed first.** This replaces the +/// file underneath anything still holding it open, which on a live connection +/// is how a *second* corrupt catalog gets made. +/// +/// The order is deliberate: +/// +/// 1. The backup is checked. A restore that installs a second damaged file +/// leaves the user with nothing to try next. +/// 2. The damaged catalog is renamed aside, and its `-wal` and `-shm` are +/// **deleted**. This is the step that is easy to leave out and fatal to +/// leave out: a journal belonging to the old file, sitting beside the new +/// one under the same name, is replayed into it on the next open. That is +/// not a restore, it is a fresh corruption with the evidence gone. +/// 3. The backup is *copied* into place, not moved, so a failure here can be +/// retried against the same backup. +pub fn restore(catalog: &Path, backup: &Path) -> Result<(), CatalogError> { + check_file(backup)?; + set_aside(catalog)?; + std::fs::copy(backup, catalog).map_err(|e| { + CatalogError::Io(format!( + "restoring {} from {}: {e}", + catalog.display(), + backup.display() + )) + })?; + log::info!( + "restored {} from backup {}", + catalog.display(), + backup.display() + ); + Ok(()) +} + +/// Move a damaged catalog out of the way so the next open builds a fresh one. +/// +/// This is the rebuild path (NFR-R6's second offer) and also the first half of +/// a [`restore`]. Nothing else is needed to rebuild: the next +/// [`Catalog::open`](crate::Catalog::open) creates an empty catalog at the +/// current schema, and the ordinary scan repopulates it from sources and +/// sidecars — which is precisely invariant §5.2.4 being spent rather than +/// merely asserted. +/// +/// Returns where the damaged file was put, or `None` if there was no catalog +/// to move — a caller may be recovering from a file SQLite could not open +/// because it was never created. +pub fn set_aside(catalog: &Path) -> Result, CatalogError> { + let moved = if catalog.exists() { + let dest = with_suffix(catalog, DAMAGED_SUFFIX); + // An earlier damaged copy is replaced rather than accumulating: two of + // these is two full-size catalogs on the user's disk, and the older + // one has already been superseded by a recovery the user completed. + let _ = std::fs::remove_file(&dest); + // The rename first, so that a failure here leaves the journals with + // the file they belong to rather than orphaned beside a catalog that + // is still in use. + std::fs::rename(catalog, &dest).map_err(|e| { + CatalogError::Io(format!( + "setting aside {} as {}: {e}", + catalog.display(), + dest.display() + )) + })?; + log::warn!( + "catalog {} was damaged; kept as {}", + catalog.display(), + dest.display() + ); + Some(dest) + } else { + None + }; + + // Then the journals, whether or not there was a catalog to move: a `-wal` + // orphaned beside a missing database is replayed into whatever takes that + // name next, which would not be a restore but a fresh corruption with the + // evidence gone. + for sidecar in journals(catalog) { + if let Err(e) = std::fs::remove_file(&sidecar) { + if e.kind() != std::io::ErrorKind::NotFound { + return Err(CatalogError::Io(format!( + "removing stale journal {}: {e}", + sidecar.display() + ))); + } + } + } + + Ok(moved) +} + +/// Delete backups beyond [`KEEP_BACKUPS`]. +/// +/// Best-effort and silent about individual failures: failing to delete an old +/// backup is not a reason to fail the new one, which is already written. +fn prune(catalog: &Path) { + for old in backups(catalog).into_iter().skip(KEEP_BACKUPS) { + if let Err(e) = std::fs::remove_file(&old.path) { + log::warn!("could not prune backup {}: {e}", old.path.display()); + } + } +} + +/// The WAL and shared-memory files SQLite keeps beside a database. +fn journals(catalog: &Path) -> [PathBuf; 2] { + [with_suffix(catalog, "wal"), with_suffix(catalog, "shm")] +} + +/// `catalog.sqlite` plus `-suffix`, the way SQLite names its own sidecars. +/// +/// Appended to the whole filename rather than replacing the extension, so +/// `catalog.sqlite-wal` is what SQLite would look for and `catalog.sqlite- +/// damaged` sorts next to the catalog it came from. +fn with_suffix(catalog: &Path, suffix: &str) -> PathBuf { + let mut s = catalog.as_os_str().to_os_string(); + s.push("-"); + s.push(suffix); + PathBuf::from(s) +} + +/// Read the timestamp out of a backup's filename, or `None` if this is not one. +/// +/// Doubles as the filter that keeps [`backups`] from offering the user +/// something that is not a catalog — a stray file in the directory, or a `-wal` +/// left by a crash mid-backup. +fn timestamp_of(path: &Path) -> Option { + let name = path.file_name()?.to_str()?; + name.strip_prefix("catalog-")? + .strip_suffix(".sqlite")? + .parse() + .ok() +} + +/// Seconds since the epoch, or 0 if the clock is before it. +fn now() -> i64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::Catalog; + use std::io::{Seek, SeekFrom, Write}; + + /// A scratch directory that cleans up with the test. + fn tempdir(tag: &str) -> PathBuf { + let base = std::env::temp_dir().join(format!( + "dr-recovery-{tag}-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&base); + std::fs::create_dir_all(&base).unwrap(); + base + } + + /// A catalog on disk with enough rows to span several pages, closed. + /// + /// Closed matters: WAL means the rows are in `catalog.sqlite-wal` until + /// something checkpoints, and a test that corrupted the main file while + /// the data was still in the journal would be corrupting empty space. + fn fixture(path: &Path, images: i64) { + let cat = Catalog::open(path).unwrap(); + let c = cat.connection(); + c.execute( + "INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'lib')", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO collections(uuid, name, kind, created, revision, modified) + VALUES ('u1', 'Iceland', 0, 0, 1, 1)", + [], + ) + .unwrap(); + for i in 1..=images { + c.execute( + "INSERT INTO images(id, root_id, source_ref, added_at) + VALUES (?1, 1, ?2, 0)", + rusqlite::params![i, format!("DCIM/IMG_{i:05}.CR3")], + ) + .unwrap(); + } + crate::sync::checkpoint(c).unwrap(); + drop(cat); + } + + /// Scribble over everything past the first two pages. + /// + /// Past them rather than over them so that page 1 — the header and the + /// schema — survives: this produces a file SQLite is willing to open and + /// then finds damaged, which is the case `quick_check` exists for. Wiping + /// the header instead produces `SQLITE_NOTADB` at the first pragma, which + /// is a different branch and has its own test. + fn corrupt(path: &Path) { + let mut f = std::fs::OpenOptions::new().write(true).open(path).unwrap(); + let len = f.metadata().unwrap().len(); + assert!( + len > 8192, + "fixture is only {len} bytes; corrupting past page 2 would be a no-op" + ); + let junk = vec![0x5a_u8; (len - 8192) as usize]; + f.seek(SeekFrom::Start(8192)).unwrap(); + f.write_all(&junk).unwrap(); + f.sync_all().unwrap(); + } + + #[test] + fn a_healthy_catalog_passes() { + let cat = Catalog::in_memory().unwrap(); + integrity_check(cat.connection()).unwrap(); + } + + #[test] + fn a_corrupt_catalog_is_reported_as_corrupt_not_as_sqlite() { + // The whole point of the variant: this used to arrive as whatever + // rusqlite error the first failing query produced, with nowhere to + // hang a recovery offer. + let dir = tempdir("detect"); + let path = dir.join("catalog.sqlite"); + fixture(&path, 500); + corrupt(&path); + + assert!(matches!( + Catalog::open_verified(&path), + Err(CatalogError::Corrupt { .. }) + )); + } + + #[test] + fn a_file_that_is_not_a_database_is_also_corrupt() { + // A truncated or overwritten catalog never reaches `quick_check`: the + // first pragma fails with SQLITE_NOTADB. Same accident to the user, + // same two offers, so it must classify the same way. + let dir = tempdir("notadb"); + let path = dir.join("catalog.sqlite"); + std::fs::write(&path, b"this is not a catalog, it is a text file\n").unwrap(); + + assert!(matches!( + Catalog::open(&path), + Err(CatalogError::Corrupt { .. }) + )); + } + + #[test] + fn restoring_a_backup_recovers_the_collections_a_rebuild_would_lose() { + // The first NFR-R6 branch, asserted on the thing that distinguishes it + // from the second: a collection exists nowhere but the catalog, so it + // is the evidence that the *contents* came back and not merely a + // readable file (docs/catalog.md §8.1). + let dir = tempdir("restore"); + let path = dir.join("catalog.sqlite"); + fixture(&path, 500); + + { + let cat = Catalog::open(&path).unwrap(); + backup(cat.connection(), &path).unwrap(); + } + corrupt(&path); + assert!(matches!( + Catalog::open_verified(&path), + Err(CatalogError::Corrupt { .. }) + )); + + let newest = backups(&path).into_iter().next().expect("a backup exists"); + restore(&path, &newest.path).unwrap(); + + let cat = Catalog::open_verified(&path).unwrap(); + let name: String = cat + .connection() + .query_row("SELECT name FROM collections", [], |r| r.get(0)) + .unwrap(); + assert_eq!(name, "Iceland"); + let images: i64 = cat + .connection() + .query_row("SELECT count(*) FROM images", [], |r| r.get(0)) + .unwrap(); + assert_eq!(images, 500); + } + + #[test] + fn a_damaged_backup_is_refused_rather_than_installed() { + let dir = tempdir("badbackup"); + let path = dir.join("catalog.sqlite"); + fixture(&path, 500); + { + let cat = Catalog::open(&path).unwrap(); + backup(cat.connection(), &path).unwrap(); + } + let newest = backups(&path).into_iter().next().unwrap(); + corrupt(&newest.path); + corrupt(&path); + + assert!(matches!( + restore(&path, &newest.path), + Err(CatalogError::Corrupt { .. }) + )); + // And the damaged catalog is still where it was, so the second offer + // is still available. + assert!(path.exists()); + } + + #[test] + fn setting_aside_leaves_a_fresh_catalog_to_rebuild_into() { + // The second NFR-R6 branch. What makes it a rebuild rather than a data + // loss is invariant §5.2.4, which lives outside this crate — what is + // testable here is that the damaged file is out of the way, kept, and + // that the next open succeeds on an empty catalog at the current + // schema, which is what a scan then fills. + let dir = tempdir("rebuild"); + let path = dir.join("catalog.sqlite"); + fixture(&path, 500); + corrupt(&path); + + let kept = set_aside(&path).unwrap().expect("the catalog was there"); + assert!(kept.exists(), "the damaged catalog was deleted, not kept"); + assert!(!path.exists()); + + let cat = Catalog::open_verified(&path).unwrap(); + let images: i64 = cat + .connection() + .query_row("SELECT count(*) FROM images", [], |r| r.get(0)) + .unwrap(); + assert_eq!(images, 0); + let v: i64 = cat + .connection() + .query_row("PRAGMA user_version", [], |r| r.get(0)) + .unwrap(); + assert_eq!(v, schema::SCHEMA_VERSION); + } + + #[test] + fn a_stale_journal_does_not_follow_the_catalog_into_recovery() { + // The step that is easy to omit: a `-wal` belonging to the damaged + // file is replayed into whatever takes its name next. + let dir = tempdir("journal"); + let path = dir.join("catalog.sqlite"); + fixture(&path, 500); + std::fs::write(with_suffix(&path, "wal"), b"stale").unwrap(); + + set_aside(&path).unwrap(); + assert!(!with_suffix(&path, "wal").exists()); + } + + #[test] + fn a_migration_is_backed_up_before_it_runs() { + // NFR-R2's second clause, against a real v1 catalog rather than a + // faked version number: the point is not that *a* file appears but + // that it holds the state from before the migration, which is the only + // state that is any use if the migration is what breaks it. + let dir = tempdir("premigrate"); + let path = dir.join("catalog.sqlite"); + { + let c = Connection::open(&path).unwrap(); + schema::configure(&c).unwrap(); + // `v1_for_attached` names the schema it targets, and "main" is a + // schema like any other — so this is the real v1, without needing + // `V1` itself to become visible outside its module. + c.execute_batch(&schema::v1_for_attached("main")).unwrap(); + c.pragma_update(None, "user_version", 1).unwrap(); + c.execute( + "INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'lib')", + [], + ) + .unwrap(); + crate::sync::checkpoint(&c).unwrap(); + } + assert!(backups(&path).is_empty()); + + Catalog::open(&path).unwrap(); + + let taken = backups(&path); + assert_eq!(taken.len(), 1, "no backup was taken before the migration"); + check_file(&taken[0].path).unwrap(); + let kept = Connection::open(&taken[0].path).unwrap(); + let v: i64 = kept + .query_row("PRAGMA user_version", [], |r| r.get(0)) + .unwrap(); + assert_eq!(v, 1, "the backup was taken after the migration, not before"); + } + + #[test] + fn opening_an_up_to_date_catalog_takes_no_backup() { + // Or every background task that opens the catalog would copy it. + let dir = tempdir("nobackup"); + let path = dir.join("catalog.sqlite"); + fixture(&path, 10); + Catalog::open(&path).unwrap(); + assert!(backups(&path).is_empty()); + } + + #[test] + fn only_the_newest_generations_are_kept() { + let dir = tempdir("prune"); + let path = dir.join("catalog.sqlite"); + fixture(&path, 10); + let cat = Catalog::open(&path).unwrap(); + + // Written by hand rather than by calling `backup` in a loop: the + // filename carries whole seconds, so real calls would collide. + std::fs::create_dir_all(backup_dir(&path)).unwrap(); + for t in 1..=KEEP_BACKUPS as i64 + 2 { + drop( + crate::sync::copy_to( + cat.connection(), + &backup_dir(&path).join(format!("catalog-{t}.sqlite")), + ) + .unwrap(), + ); + } + prune(&path); + + let kept = backups(&path); + assert_eq!(kept.len(), KEEP_BACKUPS); + // Newest first, and the newest is the highest timestamp. + assert_eq!(kept[0].taken_at, KEEP_BACKUPS as i64 + 2); + } + + #[test] + fn a_stray_file_in_the_backup_directory_is_not_offered_as_one() { + let dir = tempdir("stray"); + let path = dir.join("catalog.sqlite"); + fixture(&path, 10); + std::fs::create_dir_all(backup_dir(&path)).unwrap(); + std::fs::write(backup_dir(&path).join("notes.txt"), b"hello").unwrap(); + std::fs::write(backup_dir(&path).join("catalog-7.sqlite-wal"), b"x").unwrap(); + + assert!(backups(&path).is_empty()); + } +} diff --git a/core/dr-catalog/src/sync.rs b/core/dr-catalog/src/sync.rs index 975d82a..67022b6 100644 --- a/core/dr-catalog/src/sync.rs +++ b/core/dr-catalog/src/sync.rs @@ -52,6 +52,21 @@ pub fn checkpoint(conn: &Connection) -> Result<(), CatalogError> { /// coherent even with writers active. Callers should still prefer a quiet /// moment — this competes with background jobs for the write lock. pub fn snapshot_for_upload(conn: &Connection, dest: &Path) -> Result<(), CatalogError> { + let out = copy_to(conn, dest)?; + strip_face_crops(&out)?; + Ok(()) +} + +/// Checkpoint, then copy the whole database to `dest`, and hand back the +/// connection to the copy. +/// +/// Split out from [`snapshot_for_upload`] because [`crate::recovery`] wants +/// exactly this and none of what follows it there: an NFR-R2 backup is the +/// file the user may have to *live on*, so it keeps the face crops that an +/// upload strips. Sharing the copy rather than reimplementing it is what keeps +/// the WAL discipline in one place — a backup taken with `fs::copy` would be +/// the torn snapshot this module's header exists to warn about. +pub(crate) fn copy_to(conn: &Connection, dest: &Path) -> Result { checkpoint(conn)?; let mut out = Connection::open(dest)?; @@ -63,8 +78,7 @@ pub fn snapshot_for_upload(conn: &Connection, dest: &Path) -> Result<(), Catalog backup.run_to_completion(i32::MAX, std::time::Duration::ZERO, None)?; drop(backup); - strip_face_crops(&out)?; - Ok(()) + Ok(out) } /// Drop the stored face crops from a snapshot before it is uploaded. diff --git a/platform/dr-plat/src/crash.rs b/platform/dr-plat/src/crash.rs new file mode 100644 index 0000000..654ab3c --- /dev/null +++ b/platform/dr-plat/src/crash.rs @@ -0,0 +1,620 @@ +//! TRACES: NFR-OPS-2 | NFR-SEC-2 +//! Local crash capture. +//! +//! # What this is, and the half it deliberately is not +//! +//! NFR-OPS-2 is two sentences: *local crash capture always; upload only on +//! explicit opt-in.* Only the first is built here, and the second is not +//! half-built either — there is no upload path, no endpoint, no queue and no +//! "send this later" flag. Opt-in upload needs a server to receive it and a +//! consent flow that states what leaves the device (NFR-SEC-4, and the same +//! preview-and-consent step NFR-OPS-1 requires of the diagnostics bundle); +//! neither exists, and a transport built ahead of the consent is exactly the +//! shape of thing that later gets switched on by default. +//! +//! So a crash record is a file on the user's own disk. Nothing reads it but a +//! person. +//! +//! # Why a panic is worth writing down at all +//! +//! NFR-ARCH-4 says no worker error may panic the process, and the application +//! is built that way — errors are typed and attached to the image or job they +//! belong to. A panic is therefore, by construction, a *bug*: an invariant +//! this codebase believed and got wrong. Before this, one of those was +//! invisible on desktop (stderr, discarded with the terminal) and one line of +//! `log::error!` on Android. What the user saw was a job that stopped, or a +//! channel that closed and a control that went dead, with nothing to report. +//! +//! # What goes in a record, and what may never +//! +//! The rule the content is chosen under is NFR-SEC-2 — credentials never reach +//! logs or plain files — extended to the thing this application is actually +//! about: **a user's library is private, and its shape is private too.** A +//! path is not a neutral technical detail here. `/home/anna/Photos/2019 +//! Divorce/` names something about a person, and a crash record is a file that +//! gets attached to a bug report by someone trying to be helpful. +//! +//! Hence [`redact`], which is applied to the panic message *and* to the +//! backtrace before either is written, and which is deliberately blunt: it +//! removes anything that looks like a path or a URL, keeping only the basename +//! of `.rs` files so a backtrace is still readable. Over-redaction costs +//! legibility; under-redaction costs a user something they cannot take back. +//! +//! NFR-SEC-5 — face data never enters a diagnostics bundle or crash report +//! "under any configuration" — is met structurally rather than by filtering: +//! this module reads no catalog, opens no image, and touches no account. A +//! record is assembled from the panic hook's own arguments and from +//! [`std::env::consts`], and there is no code path from here to an embedding, +//! a crop, or a cluster. The message length cap is the backstop for the +//! remaining case — a panic payload that some *other* module formatted a large +//! value into. +//! +//! # What this leaves cheaper for NFR-OPS-1 +//! +//! The rotating on-disk log is unbuilt, and it wants three things that are +//! here: [`state_dir`] (the XDG state directory, resolved once, overridable +//! for Android where XDG does not exist), [`redact`] (NFR-OPS-1's "automatic +//! redaction of credentials and tokens" is the same function), and `prune` +//! (size-capped rotation is this counting files instead of bytes). A log +//! belongs at `state_dir().join("log")` beside `crash/`, and the diagnostics +//! bundle then has one directory to collect. + +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; +use std::time::{SystemTime, UNIX_EPOCH}; + +/// How many crash records are kept, newest first. +/// +/// Ten because the useful pattern in a crash record is usually a *repeat* — +/// the same panic three launches running is a far stronger report than one — +/// and because these are a few kilobytes each, so the cap exists to stop a +/// crash loop filling a disk rather than to save space. +pub const KEEP_RECORDS: usize = 10; + +/// The longest panic message written to a record. +/// +/// A backstop, not a redaction: [`redact`] handles what must not be written at +/// all. This bounds what a panic that formatted something enormous into its +/// message — a decoded buffer, a `Vec` of embeddings — can put on disk. +const MAX_MESSAGE: usize = 2000; + +/// Android's per-app directory, once the entry point has said what it is. +static STATE_DIR: OnceLock = OnceLock::new(); + +/// Declare the directory this application may keep state in. +/// +/// Only Android needs to call this, and it must call it before a crash rather +/// than before the hook is installed — [`install`] resolves the directory at +/// crash time precisely so that the hook can go in first, covering the startup +/// it would otherwise miss. Neither `XDG_STATE_HOME` nor `HOME` is set there, +/// and the fallback would resolve to a path the app cannot write. +/// +/// Later calls are ignored rather than racing, matching +/// `dr_sync::account::set_data_dir`, which the same entry point calls for the +/// same reason. +pub fn set_state_dir(dir: PathBuf) { + let _ = STATE_DIR.set(dir); +} + +/// Where this application keeps state that is neither configuration nor cache. +/// +/// `$XDG_STATE_HOME/darkroom`, falling back to `~/.local/state/darkroom`. +/// State rather than cache because a crash record must survive the sweep that +/// a cache directory exists to permit, and rather than config because it is +/// not something the user edits. +pub fn state_dir() -> PathBuf { + if let Some(d) = STATE_DIR.get() { + return d.clone(); + } + std::env::var_os("XDG_STATE_HOME") + .map(PathBuf::from) + .unwrap_or_else(|| { + PathBuf::from(std::env::var("HOME").unwrap_or_default()).join(".local/state") + }) + .join("darkroom") +} + +/// Where crash records are written. +pub fn crash_dir() -> PathBuf { + state_dir().join("crash") +} + +/// Install the panic hook. +/// +/// Call once, as early as the entry point can — before the window, before any +/// store, before anything that could itself panic. The directory is resolved +/// lazily inside the hook, so installing this before +/// [`set_state_dir`] is correct rather than merely tolerated. +/// +/// The previously installed hook still runs afterwards. On desktop that is the +/// standard library's, which prints the panic to stderr, and a developer +/// watching a terminal should not lose that because the application started +/// writing files. stderr is the one surface that sees the message unredacted, +/// which is a considered exception: it is ephemeral, local, and never attached +/// to a bug report. +pub fn install(app_version: &str) { + let version = app_version.to_string(); + let previous = std::panic::take_hook(); + + std::panic::set_hook(Box::new(move |info| { + // A panic inside a panic hook aborts the process, which would replace + // a diagnosable crash with an undiagnosable one. Everything below is + // written to be infallible, and this is the admission that "written to + // be" is not the same as "is". + let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let record = compose(&version, info); + // Redacted, because this is a log file and NFR-SEC-2 governs it. + // The unredacted form goes to stderr below, through the hook this + // one chained onto. + log::error!("panic: {}", record.summary); + match write_record(&crash_dir(), &record) { + // The name, not the path: the log this line lands in is a + // sibling of the record, and printing the directory would put + // the user's home in a file they may hand to someone. + Ok(path) => log::error!( + "crash record written: {}", + path.file_name().unwrap_or_default().to_string_lossy() + ), + Err(e) => log::error!("could not write a crash record: {e}"), + } + })); + + previous(info); + })); +} + +/// The crash records on disk, newest first. +/// +/// For a future diagnostics bundle, and for a person looking for the file to +/// attach to a report. +pub fn records() -> Vec { + let Ok(entries) = std::fs::read_dir(crash_dir()) else { + return Vec::new(); + }; + let mut out: Vec<(i64, PathBuf)> = entries + .flatten() + .filter_map(|e| { + let path = e.path(); + Some((timestamp_of(&path)?, path)) + }) + .collect(); + out.sort_by_key(|(when, _)| std::cmp::Reverse(*when)); + out.into_iter().map(|(_, p)| p).collect() +} + +/// One crash, formatted. +struct Record { + /// The whole file. + body: String, + /// One redacted line, for the log. + summary: String, + when: i64, +} + +/// Build a record from what the panic hook was handed. +/// +/// Split from the writing so the *content* rules — what is included, what is +/// redacted, what is capped — are testable without a filesystem, and so a +/// future diagnostics bundle can reuse the same composition. +fn compose(version: &str, info: &std::panic::PanicHookInfo<'_>) -> Record { + let payload = info.payload(); + let message = payload + .downcast_ref::<&str>() + .copied() + .or_else(|| payload.downcast_ref::().map(|s| s.as_str())) + // A panic can carry any `Any`, and `panic_any` is used by some + // libraries. There is nothing to print, and saying so is better than + // an empty field that reads like a bug in this code. + .unwrap_or("(panic payload was not a string)"); + + let message = redact(&truncate(message, MAX_MESSAGE)); + let at = info + .location() + .map(|l| format!("{}:{}:{}", l.file(), l.line(), l.column())) + // `location()` is a compile-time source path, not one of the user's, + // but it goes through the same redaction: a dependency built from a + // registry checkout carries the *builder's* home directory in it. + .map(|s| redact(&s)) + .unwrap_or_else(|| "unknown".to_string()); + let thread = std::thread::current() + .name() + .unwrap_or("unnamed") + .to_string(); + + let when = now(); + let summary = format!("{message} (at {at}, thread {thread})"); + + let body = format!( + "darkroom-crash 1\n\ + version: {version}\n\ + when: {when}\n\ + os: {}\n\ + arch: {}\n\ + thread: {thread}\n\ + at: {at}\n\ + message: {message}\n\ + backtrace:\n{}\n", + std::env::consts::OS, + std::env::consts::ARCH, + redact(&std::backtrace::Backtrace::force_capture().to_string()), + ); + + Record { + body, + summary, + when, + } +} + +/// Write one record into `dir`, then prune. +/// +/// Takes the directory rather than calling [`crash_dir`] so a test can drive +/// the real writing path without an environment variable and without a +/// `OnceLock` it cannot reset. +fn write_record(dir: &Path, record: &Record) -> std::io::Result { + std::fs::create_dir_all(dir)?; + // The pid distinguishes two processes crashing in the same second, which + // is not hypothetical: a background export and the app can be separate + // processes on desktop, and a crash loop retries fast. + let path = dir.join(format!("crash-{}-{}.txt", record.when, std::process::id())); + std::fs::write(&path, &record.body)?; + prune(dir, KEEP_RECORDS); + Ok(path) +} + +/// Delete all but the `keep` newest records in `dir`. +/// +/// Best-effort: failing to delete an old record is no reason to lose the new +/// one, which is already on disk. +fn prune(dir: &Path, keep: usize) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + let mut found: Vec<(i64, PathBuf)> = entries + .flatten() + .filter_map(|e| { + let path = e.path(); + Some((timestamp_of(&path)?, path)) + }) + .collect(); + found.sort_by_key(|(when, _)| std::cmp::Reverse(*when)); + for (_, path) in found.into_iter().skip(keep) { + let _ = std::fs::remove_file(path); + } +} + +/// Pull the timestamp back out of a record's filename. +/// +/// Doubles as the filter that keeps anything else in the directory — an +/// editor's backup file, a log — from being counted as a crash record or +/// pruned as one. +fn timestamp_of(path: &Path) -> Option { + let name = path.file_name()?.to_str()?; + let rest = name.strip_prefix("crash-")?.strip_suffix(".txt")?; + rest.split('-').next()?.parse().ok() +} + +/// Remove from `text` everything that would say something about the user. +/// +/// # The rule, and why it is this blunt one +/// +/// A token containing `/` is treated as a path or a URL and removed. Rust +/// source files are the exception and keep their basename, because a backtrace +/// with no filenames is close to useless and `library.rs:1270` says nothing +/// about anybody. A token that reads like a secret — a long opaque run, or the +/// value beside a word like `password` — is removed regardless of shape. +/// +/// Blunter than a list of known-sensitive shapes on purpose. The cost of +/// over-redaction is a diagnostic that is harder to read; the cost of +/// under-redaction is a directory listing of somebody's photographs in a file +/// they may attach to a public bug report. Those are not comparable, and this +/// is not the place to be clever about the boundary. +/// +/// Known limits, stated rather than hidden: a path with no `/` in it (a bare +/// filename) survives, and a person's name that some *other* module formatted +/// into a panic message survives. Both are bounded by the fact that nothing +/// here reads the catalog — see the module documentation — and the second is +/// why `MAX_MESSAGE` exists. +pub fn redact(text: &str) -> String { + let mut out: Vec = Vec::new(); + let mut redact_next = false; + + for token in text.split_inclusive(char::is_whitespace) { + // Whitespace is preserved exactly — a backtrace is read as a shape as + // much as as text — so the classification runs on the token without + // its trailing space and the space is put back. + let trailing: String = token + .chars() + .skip_while(|c| !c.is_whitespace()) + .collect::(); + let word = &token[..token.len() - trailing.len()]; + + if word.is_empty() { + out.push(token.to_string()); + continue; + } + + let replacement = if redact_next { + Some("".to_string()) + } else { + classify(word) + }; + redact_next = names_a_secret(word); + + out.push(match replacement { + Some(r) => format!("{r}{trailing}"), + None => token.to_string(), + }); + } + + out.join("") +} + +/// What one token should be replaced with, or `None` to keep it. +fn classify(word: &str) -> Option { + // `key=value` and `key: value` written as one token. Split at the first + // separator so `Authorization:Bearer` loses the half that matters. + if let Some((head, tail)) = word.split_once(['=', ':']) { + if names_a_secret(head) && !tail.is_empty() { + return Some(format!("{head}=")); + } + } + + // Strip the punctuation a sentence puts around a path — "opening + // /home/x/y.CR3:" — so the classification sees the path itself, then put + // nothing back: the punctuation is not worth the complexity of restoring + // it around a placeholder. + let bare = word.trim_matches(|c: char| matches!(c, '"' | '\'' | '(' | ')' | ',' | ';' | ':')); + + if bare.contains('/') { + // A Rust source path keeps its basename. Line and column are part of + // the same token in a backtrace ("src/library.rs:1270:9"), and they + // are kept: they are facts about this codebase. + if let Some(rs) = rust_source_tail(bare) { + return Some(format!("…/{rs}")); + } + return Some("".to_string()); + } + if bare.starts_with('~') { + return Some("".to_string()); + } + if looks_opaque(bare) { + return Some("".to_string()); + } + None +} + +/// The `library.rs:1270:9` tail of a path naming a Rust source file. +fn rust_source_tail(word: &str) -> Option<&str> { + let tail = word.rsplit('/').next()?; + // The extension is followed by `:line:col` in a backtrace and by nothing + // in a `Location`, so match on the extension rather than on the end. + if tail.contains(".rs") { + Some(tail) + } else { + None + } +} + +/// Whether this word introduces a value that must not be written down. +fn names_a_secret(word: &str) -> bool { + let w = word + .trim_matches(|c: char| !c.is_alphanumeric() && c != '_' && c != '-') + .to_ascii_lowercase(); + matches!( + w.as_str(), + "password" + | "passwd" + | "app-password" + | "app_password" + | "token" + | "secret" + | "bearer" + | "authorization" + | "apikey" + | "api-key" + | "api_key" + | "credential" + | "credentials" + | "cookie" + ) +} + +/// Whether a word looks like a key rather than like prose. +/// +/// A long run of the characters secrets are encoded in, containing both a +/// letter and a digit — which is what an app password, a bearer token or a +/// base64 blob looks like, and what an English word or a Rust identifier does +/// not. +fn looks_opaque(word: &str) -> bool { + word.len() >= 20 + && word + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '_' | '-' | '=')) + && word.chars().any(|c| c.is_ascii_digit()) + && word.chars().any(|c| c.is_ascii_alphabetic()) +} + +/// Cut `s` to `max` bytes on a character boundary, saying that it was cut. +fn truncate(s: &str, max: usize) -> String { + if s.len() <= max { + return s.to_string(); + } + let mut end = max; + while end > 0 && !s.is_char_boundary(end) { + end -= 1; + } + format!("{}… ({} bytes truncated)", &s[..end], s.len() - end) +} + +/// Seconds since the epoch, or 0 if the clock is before it. +fn now() -> i64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_users_photograph_never_reaches_a_record() { + let r = redact("failed to open /home/anna/Photos/2019 Divorce/IMG_0042.CR3"); + assert!(!r.contains("anna"), "{r}"); + assert!(!r.contains("IMG_0042"), "{r}"); + assert!(!r.contains("Divorce"), "{r}"); + assert!(r.contains("failed to open"), "the diagnosis was lost: {r}"); + } + + #[test] + fn an_android_document_uri_is_a_path_too() { + // SAF hands out `content://` URIs and `primary:DCIM/...` refs rather + // than paths, and both name the user's library just as precisely. + let r = redact("no such image: content://com.android.providers/tree/primary%3ADCIM"); + assert!(!r.contains("DCIM"), "{r}"); + let r = redact("source_ref primary:DCIM/Camera/IMG_1.CR3 missing"); + assert!(!r.contains("IMG_1"), "{r}"); + } + + #[test] + fn a_server_url_is_removed_because_it_names_the_user() { + // A Nextcloud URL is the user's own server, often with their login in + // the path. NFR-SEC-3 is about the wire; this is about the disk. + let r = redact("PROPFIND https://cloud.example.org/remote.php/dav/files/anna/ failed"); + assert!(!r.contains("cloud.example.org"), "{r}"); + assert!(!r.contains("anna"), "{r}"); + assert!(r.contains("PROPFIND"), "{r}"); + } + + #[test] + fn a_credential_never_reaches_a_record() { + // NFR-SEC-2, which forbids credentials in logs and plain files. Both + // spellings: the value beside a naming word, and the value alone. + let r = redact("auth failed: password hunter2correcthorse"); + assert!(!r.contains("hunter2correcthorse"), "{r}"); + + let r = redact("Authorization: Bearer aGVsbG90aGVyZTEyMzQ1Njc4OTA="); + assert!(!r.contains("aGVsbG90aGVyZTEyMzQ1Njc4OTA"), "{r}"); + + let r = redact("rejected app-password=abcde-fghij-12345-klmno-pqrst"); + assert!(!r.contains("abcde-fghij"), "{r}"); + } + + #[test] + fn a_bare_secret_is_caught_by_its_shape() { + // Nextcloud app passwords arrive with no label at all when they are + // interpolated into a message by a library this codebase does not own. + let r = redact("login failed for aBcDe1FgHiJ2kLmNo3PqRsT4uV"); + assert!(!r.contains("aBcDe1FgHiJ"), "{r}"); + } + + #[test] + fn a_backtrace_keeps_the_frames_that_make_it_readable() { + // The whole reason the `.rs` exception exists: redacting these to + // `` leaves a backtrace of nothing but symbol names, and the + // line number is a fact about this codebase rather than about anyone. + let r = redact(" 3: dr_ui::library::run_scan\n at ./ui/dr-ui/src/library.rs:1270:9\n"); + assert!(r.contains("library.rs:1270:9"), "{r}"); + assert!(r.contains("dr_ui::library::run_scan"), "{r}"); + assert!(!r.contains("ui/dr-ui/src"), "{r}"); + // And the shape survives, because a backtrace is read as a shape. + assert!(r.contains('\n'), "{r}"); + assert!(r.starts_with(" 3:"), "{r}"); + } + + #[test] + fn ordinary_words_are_left_alone() { + // Over-redaction has a cost too: a record that says nothing is not + // safer, it is just useless. + let msg = "assertion failed: tier_desired was 3, expected 2"; + assert_eq!(redact(msg), msg); + } + + #[test] + fn an_enormous_payload_is_capped() { + // A panic that formatted a decoded buffer — or, the case NFR-SEC-5 + // cares about, an embedding — into its message. + let huge = "9".repeat(MAX_MESSAGE * 3); + let cut = truncate(&huge, MAX_MESSAGE); + assert!(cut.len() < huge.len()); + assert!(cut.contains("truncated"), "{cut}"); + } + + #[test] + fn truncation_does_not_split_a_character() { + let s = "é".repeat(100); + // 3 is mid-character for a 2-byte encoding. + let cut = truncate(&s, 3); + assert!(cut.starts_with('é')); + } + + #[test] + fn records_are_written_and_rotated() { + let dir = std::env::temp_dir().join(format!("dr-crash-test-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + + for i in 0..(KEEP_RECORDS as i64 + 5) { + write_record( + &dir, + &Record { + body: format!("darkroom-crash 1\nmessage: {i}\n"), + summary: String::new(), + // Distinct seconds, or the pid-suffixed names would + // collide and the rotation would have nothing to count. + when: 1_000 + i, + }, + ) + .unwrap(); + } + + let kept: Vec = std::fs::read_dir(&dir) + .unwrap() + .flatten() + .map(|e| e.path()) + .collect(); + assert_eq!(kept.len(), KEEP_RECORDS); + // The newest survive, not the oldest. + let newest = kept.iter().filter_map(|p| timestamp_of(p)).max(); + let oldest = kept.iter().filter_map(|p| timestamp_of(p)).min(); + assert_eq!(newest, Some(1_000 + KEEP_RECORDS as i64 + 4)); + assert_eq!(oldest, Some(1_000 + 5)); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_stray_file_is_neither_listed_nor_pruned() { + let dir = std::env::temp_dir().join(format!("dr-crash-stray-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("darkroom.log"), b"not a crash").unwrap(); + + for i in 0..(KEEP_RECORDS as i64 + 5) { + write_record( + &dir, + &Record { + body: String::new(), + summary: String::new(), + when: 2_000 + i, + }, + ) + .unwrap(); + } + + assert!(dir.join("darkroom.log").is_file(), "the log was pruned"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn the_state_directory_is_neither_config_nor_cache() { + // NFR-OPS-1 puts the log here too, and NFR-OPS-3 keeps preferences + // separate from both. The distinction is what stops a crash record + // being swept away by the thing that is allowed to sweep caches. + let dir = state_dir(); + let s = dir.to_string_lossy(); + assert!(s.ends_with("darkroom"), "{s}"); + assert!(!s.contains("/cache"), "{s}"); + } +} diff --git a/platform/dr-plat/src/lib.rs b/platform/dr-plat/src/lib.rs index 808ed80..77f0b68 100644 --- a/platform/dr-plat/src/lib.rs +++ b/platform/dr-plat/src/lib.rs @@ -4,6 +4,13 @@ //! construction, so `core/` contains no `#[cfg(target_os)]` (NFR-PORT-1, //! ARCH §10). +// Not a trait, and the one module here that is not. It belongs in this crate +// for the same reason the traits do: "where does this platform let an +// application keep state" is a platform question, and both entry points need +// the answer before either has a window. Resolving it in `apps/` would mean +// writing it twice, once per platform, which is the arrangement this crate +// exists to prevent. +pub mod crash; pub mod display; pub mod secrets; pub mod storage; diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index 23a980b..483976a 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -49,6 +49,7 @@ mod net_runtime; mod peaking; mod preset_store; mod presets; +mod recovery_ui; mod remote; mod segmentation; mod settings_store; diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index 1112280..70a2b3a 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -862,7 +862,13 @@ pub fn open( // Before the scan, not after it: the grid can be filled from disk now and // the scan is only ever going to add to it. - show_catalog_now(window, &ctl, &path, &coll_ctl); + // + // And it is the gate on the scan, not merely a prelude to it — a damaged + // catalog has a question on screen, and a scan writing into it while that + // question is unanswered is how the last good copy gets destroyed. + if !show_catalog_now(window, &ctl, &path, &coll_ctl) { + return; + } let rx = library::spawn_scan( conn.clone(), @@ -1095,7 +1101,7 @@ fn drain_scan( /// the same operation: a scan is the only request that both proves the server /// is reachable and brings the catalog up to date. Keeping them one function /// is what stops "retry" from quietly becoming a weaker probe than "rescan". -fn start_rescan( +pub(crate) fn start_rescan( window: &AppWindow, ctl: &Rc, coll_ctl: &Rc, @@ -1916,23 +1922,38 @@ fn schedule_reload(window: &AppWindow, ctl: &Rc) { /// state already says "Scanning…", and an error here would contradict a scan /// that is working perfectly. `Catalog::open` creates the file in that case, so /// what the grid reads is an empty catalog rather than a failure. -fn show_catalog_now( +/// Returns whether it is safe to go on and scan. +/// +/// `false` means the catalog is damaged and the recovery question is up. The +/// caller must not start a scan on that answer: `Catalog::open` succeeds on a +/// file whose header survived, so the scan would write ETags and image rows +/// into damaged pages while the user is still reading the question — turning a +/// file that had a backup into one where the backup is the only copy left. +pub(crate) fn show_catalog_now( window: &AppWindow, ctl: &Rc, catalog_path: &std::path::Path, coll_ctl: &Rc, -) { +) -> bool { if ctl.catalog.borrow().is_some() { - return; + return true; } - let cat = match Catalog::open(catalog_path) { + // Verified rather than plain: this is the once-per-launch moment where a + // full check is affordable and there is a user in front of it who can + // answer the question. See `dr_catalog::recovery` for why it is not on + // every open. + let cat = match Catalog::open_verified(catalog_path) { Ok(cat) => cat, + Err(dr_catalog::CatalogError::Corrupt { detail }) => { + crate::recovery_ui::offer(window, catalog_path, &detail); + return false; + } Err(e) => { // Not surfaced: the scan is the thing that has to work, and it is // still running. If it fails too, it reports for both of them. log::info!("no catalog to show before the scan: {e}"); - return; + return true; } }; @@ -1964,6 +1985,19 @@ fn show_catalog_now( crate::collections_ui::refresh_tree(window, coll_ctl, &cat); *ctl.catalog.borrow_mut() = Some(cat); load_window(window, ctl); + true +} + +/// Drop the open catalog, so the next `show_catalog_now` opens the file +/// again rather than returning early. +/// +/// Only recovery needs this, and it needs it for a specific reason: the file +/// under that connection has been replaced. A handle to the catalog that was +/// there before is a handle to a file that no longer has a name, and every +/// read through it would return the damaged pages the recovery just moved out +/// of the way. +pub(crate) fn forget_catalog(ctl: &Rc) { + *ctl.catalog.borrow_mut() = None; } /// What one cell of the outgoing model is worth keeping. @@ -5644,6 +5678,11 @@ pub fn wire( start_rescan(&w, &ctl, &coll_ctl); }); } + + // Last, and in its own module: the answers to a damaged catalog have + // nothing to do with the library view except that they run before it + // exists. + crate::recovery_ui::wire(window, &ctl, &coll_ctl); } /// Reload the grid after the filter changed. diff --git a/ui/dr-ui/src/recovery_ui.rs b/ui/dr-ui/src/recovery_ui.rs new file mode 100644 index 0000000..f8037cc --- /dev/null +++ b/ui/dr-ui/src/recovery_ui.rs @@ -0,0 +1,285 @@ +//! The two offers made when the catalog turns out to be damaged. +//! +//! `dr_catalog::recovery` owns the mechanism — the integrity check, the +//! backups, the restore, setting the damaged file aside. This module owns the +//! *conversation*: what a user is told has happened, which of the two answers +//! are available, and what runs afterwards. +//! +//! # The thing that has to be said first +//! +//! **The photographs are fine, and so are the edits.** A user told that their +//! library database is corrupt will assume they have lost their work, because +//! in every other photo application they would have. Here they have not: +//! sources are read-only to this application (NFR-R4), and ratings, keywords +//! and edit graphs live in sidecars beside the images for every catalogued +//! photograph, whether or not an account exists (FR-CAT-8, invariant §5.2.4). +//! That sentence is the first line of the dialogue, before the diagnosis, +//! because it is the answer to the question the user is actually asking. +//! +//! # Why the two answers are not interchangeable +//! +//! A restore brings back **collections**; a rebuild cannot. Every other thing +//! the catalog holds has authoritative backing outside it, which is what makes +//! a rebuild survivable — but a manual collection is a set of images the user +//! assembled by hand and nothing in the filesystem records it +//! (`docs/catalog.md` §8.1). So the labels say which one loses them, and the +//! rebuild is not given the affirmative styling while a restore is on offer. +//! +//! # Why the scan is held back +//! +//! `library_ui::open` shows the catalog and then starts a scan. On a damaged +//! catalog the scan is actively harmful: a plain `Catalog::open` on a file +//! whose header is intact succeeds, and the scan would then write folder +//! ETags and image rows into damaged pages — turning a recoverable file into +//! one whose backup is the only copy left, and doing it in the seconds while +//! the user is still reading the question. So `show_catalog_now` reports +//! whether it is safe to continue, and this module restarts the scan itself +//! once the file underneath has been replaced. + +use std::cell::RefCell; +use std::path::{Path, PathBuf}; +use std::rc::Rc; + +use dr_catalog::recovery; +use slint::ComponentHandle; + +use crate::library_ui::LibraryController; +use crate::AppWindow; + +/// What the open question is about. +/// +/// A thread-local rather than a field on `LibraryController`, because the +/// question is asked *before* that controller has a catalog and is answered by +/// callbacks wired at startup. Thread-local is sound here for the same reason +/// [`crate::memory`]'s registry is: everything below runs on the Slint event +/// loop thread, which is the only thread that has an `AppWindow` to show it +/// on. +thread_local! { + static PENDING: RefCell> = const { RefCell::new(None) }; +} + +/// The damaged catalog and what can be done about it. +struct Pending { + catalog: PathBuf, + /// Newest first. Empty is the ordinary case on a young install and is not + /// an error — it removes one offer, not both. + backups: Vec, +} + +/// Ask what should happen to a damaged catalog. +/// +/// Called from `library_ui::show_catalog_now` when the startup integrity check +/// fails. `detail` is what SQLite said, carried through verbatim: a diagnosis +/// the user can quote into a bug report is worth more than a reassurance they +/// cannot check. +pub(crate) fn offer(window: &AppWindow, catalog: &Path, detail: &str) { + let backups = recovery::backups(catalog); + log::error!( + "catalog {} failed its integrity check: {detail} ({} backup(s) available)", + catalog.display(), + backups.len() + ); + + window.set_recovery_title("This library's index is damaged".into()); + window.set_recovery_detail( + // Two facts and their order matters: what is safe, then what is lost. + "Your photographs and your edits are safe — they are in the files \ + themselves and in the sidecars beside them. What is damaged is only \ + DarkRoom's index of them, which can be rebuilt." + .into(), + ); + window.set_recovery_diagnosis(detail.into()); + + match backups.first() { + Some(newest) => { + window.set_recovery_can_restore(true); + window.set_recovery_restore_label( + format!( + "Restore the backup from {} · keeps your collections", + describe_age(newest.taken_at) + ) + .into(), + ); + } + None => { + window.set_recovery_can_restore(false); + window.set_recovery_restore_label(slint::SharedString::new()); + } + } + + window.set_recovery_rebuild_label( + if backups.is_empty() { + // Nothing to compare it against, so the label states the cost + // rather than the difference. + "Rebuild from your photographs · rescans the library" + } else { + "Rebuild from your photographs · loses your collections" + } + .into(), + ); + window.set_recovery_busy(false); + // The scan was held back, so the "Scanning…" the grid is showing behind + // this would be a lie the moment the question is dismissed. + window.set_library_scanning(false); + + PENDING.with(|p| { + *p.borrow_mut() = Some(Pending { + catalog: catalog.to_path_buf(), + backups, + }) + }); +} + +/// Close the question without answering it. +/// +/// Leaves the banner set, because the library genuinely does not work and a +/// dialogue that vanishes leaving no trace of why nothing loads is worse than +/// no dialogue at all. +fn dismiss(window: &AppWindow) { + PENDING.with(|p| *p.borrow_mut() = None); + window.set_recovery_title(slint::SharedString::new()); + window.set_library_scanning(false); + window.set_library_error( + "The library index is damaged. Rescan to rebuild it, or restore a backup.".into(), + ); +} + +/// Install the three answers. +/// +/// Called at the end of `library_ui::wire`, which is where every other +/// window-level callback in this area is installed. +pub(crate) fn wire( + window: &AppWindow, + ctl: &Rc, + coll_ctl: &Rc, +) { + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + let coll = coll_ctl.clone(); + window.on_recovery_restore(move || { + let Some(w) = weak.upgrade() else { return }; + answer(&w, &ctl, &coll, Answer::Restore); + }); + } + + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + let coll = coll_ctl.clone(); + window.on_recovery_rebuild(move || { + let Some(w) = weak.upgrade() else { return }; + answer(&w, &ctl, &coll, Answer::Rebuild); + }); + } + + { + let weak = window.as_weak(); + window.on_recovery_dismiss(move || { + let Some(w) = weak.upgrade() else { return }; + dismiss(&w); + }); + } +} + +/// Which of the two the user chose. +#[derive(Clone, Copy, PartialEq, Eq)] +enum Answer { + Restore, + Rebuild, +} + +/// Carry out an answer, then get the library going again. +/// +/// Both answers end the same way — the file under `catalog_path` is one this +/// build can open — so both continue into the same two steps: open the catalog +/// for the grid, and start a scan. A rebuild needs the scan to have anything +/// at all; a restore needs it because the backup is by definition older than +/// the library. +fn answer( + window: &AppWindow, + ctl: &Rc, + coll_ctl: &Rc, + which: Answer, +) { + let Some(pending) = PENDING.with(|p| p.borrow_mut().take()) else { + return; + }; + window.set_recovery_busy(true); + + // Before the file moves, not after. There is normally no open catalog here + // — `show_catalog_now` returned before storing one — but "normally" is not + // a guarantee worth resting a file rename on, and a connection to a file + // that has just been renamed out from under it reads the damaged pages + // forever. + crate::library_ui::forget_catalog(ctl); + + // Synchronous, on the UI thread, and that is a considered choice rather + // than an oversight: this is a file copy of a catalog — tens of megabytes + // at 50k images — at a moment when there is nothing else on screen to + // block, no scan running, and no frame worth keeping smooth. Moving it to + // a worker would buy a spinner and cost the guarantee that nothing else + // touches the file while it is being replaced. + let outcome = match which { + Answer::Restore => match pending.backups.first() { + Some(b) => recovery::restore(&pending.catalog, &b.path), + None => Ok(()), + }, + Answer::Rebuild => recovery::set_aside(&pending.catalog).map(|_| ()), + }; + + if let Err(e) = outcome { + // The question stays up: the *other* answer may still work, and a + // failed restore in particular leaves the rebuild untouched. + log::error!("recovery failed: {e}"); + window.set_recovery_busy(false); + window.set_recovery_diagnosis(format!("That did not work: {e}").into()); + PENDING.with(|p| *p.borrow_mut() = Some(pending)); + return; + } + + window.set_recovery_busy(false); + window.set_recovery_title(slint::SharedString::new()); + window.set_library_error(slint::SharedString::new()); + + if crate::library_ui::show_catalog_now(window, ctl, &pending.catalog, coll_ctl) { + crate::library_ui::start_rescan(window, ctl, coll_ctl); + } +} + +/// "today", "3 days ago" — enough to choose by, without a date library. +/// +/// The user is deciding how much work a restore costs them, and the answer to +/// that is an *age*, not a timestamp: "yesterday" is immediately actionable +/// and "1756512000" is not. Whole days, because an hour's precision would +/// invite a confidence the backup schedule does not earn. +fn describe_age(taken_at: i64) -> String { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0); + let days = (now - taken_at).max(0) / 86_400; + match days { + 0 => "today".to_string(), + 1 => "yesterday".to_string(), + d => format!("{d} days ago"), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn an_age_reads_as_an_age() { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() as i64; + assert_eq!(describe_age(now), "today"); + assert_eq!(describe_age(now - 86_400), "yesterday"); + assert_eq!(describe_age(now - 5 * 86_400), "5 days ago"); + // A clock that has gone backwards must not produce "-2 days ago". + assert_eq!(describe_age(now + 86_400), "today"); + } +} diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index b54a475..abf771d 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -11,6 +11,7 @@ import { GestureRow } from "gestures.slint"; import { Button, PanelHeading, Label, Value, Caption, Panel, EmptyState, ProgressBar, ActivityRow } from "widgets.slint"; import { CollectionsPanel, CollectionRow, OfflinePrompt } from "collections.slint"; import { HistogramPanel, HistogramView } from "histogram.slint"; +import { RecoveryPrompt } from "recovery.slint"; import { PresetSheet, ScopeChips, ScopeKind } from "presets.slint"; import { FocusMarks, FocusPanel } from "peaking.slint"; import { SettingsPage } from "settings.slint"; @@ -365,6 +366,21 @@ export component AppWindow inherits Window { callback offline-prompt-release(); callback offline-prompt-dismiss(); + // The question a damaged catalog asks. Same shape as the prompt above and + // for the same reason: an empty title is what closes it, and every word in + // it is composed in Rust, which is the only side that knows what SQLite + // said and which backups exist. + in property recovery-title: ""; + in property recovery-detail: ""; + in property recovery-diagnosis: ""; + in property recovery-restore-label: ""; + in property recovery-can-restore: false; + in property recovery-rebuild-label: ""; + in property recovery-busy: false; + callback recovery-restore(); + callback recovery-rebuild(); + callback recovery-dismiss(); + in property library-root-label: ""; in-out property <[TimelineBar]> library-timeline; in property library-timeline-label: ""; @@ -1152,6 +1168,14 @@ in property panel-visible: true; // would leave the library from behind an open question — the // view changing underneath a modal, which reads as the app // having lost its place. + // + // The recovery question is asked first because it is drawn + // over everything, the offline prompt included: Back must + // reach the thing the user can actually see. + if (root.recovery-title != "") { + root.recovery-dismiss(); + return accept; + } if (root.offline-prompt-title != "") { root.offline-prompt-dismiss(); return accept; @@ -2672,5 +2696,24 @@ in property panel-visible: true; release() => { root.offline-prompt-release(); } dismiss() => { root.offline-prompt-dismiss(); } } + + // Last, and therefore over everything including the settings page and + // the offline prompt. Not a preference about layering: this is asked + // before the grid exists, and nothing else in the window is about a + // library that can be read. + RecoveryPrompt { + width: 100%; + height: 100%; + title: root.recovery-title; + detail: root.recovery-detail; + diagnosis: root.recovery-diagnosis; + restore-label: root.recovery-restore-label; + can-restore: root.recovery-can-restore; + rebuild-label: root.recovery-rebuild-label; + busy: root.recovery-busy; + restore() => { root.recovery-restore(); } + rebuild() => { root.recovery-rebuild(); } + dismiss() => { root.recovery-dismiss(); } + } } } diff --git a/ui/dr-ui/ui/recovery.slint b/ui/dr-ui/ui/recovery.slint new file mode 100644 index 0000000..cc5372b --- /dev/null +++ b/ui/dr-ui/ui/recovery.slint @@ -0,0 +1,145 @@ +// The question asked when the catalog turns out to be damaged. +// +// # Why this is a modal, when almost nothing else here is +// +// The house rule in this interface is to put the consequence in the button's +// label rather than to raise a dialogue — "Export 40", "Empty trash · 128" — +// and a genuine modal is kept for the two cases where the answer commits +// gigabytes. This is the third case, and it earns it for a different reason: +// there is nothing behind it to interact with. The grid cannot be drawn, the +// scan must not run (it would write into the damage), and every control in the +// window is about a library that cannot be read. A banner over an empty grid +// would be a question the user could scroll away from and then wonder why +// nothing worked. +// +// # Why the backdrop does not dismiss it +// +// Every other overlay here closes on a tap outside, and this one deliberately +// does not. A stray tap that loses the two offers leaves the application in a +// state with no way forward and no obvious way back to the question. There is +// a "Leave it for now" button instead, which says what it does. +// +// # Why the destructive answer is not the primary one +// +// A restore keeps the user's collections; a rebuild cannot, because a manual +// collection is a set of images the user assembled by hand and nothing in the +// filesystem records it (docs/catalog.md §8.1). So the two answers are not +// interchangeable, the difference is stated in the button rather than in a +// second dialogue after it, and the rebuild is the plain button even when it +// is the only one available. + +import { Theme } from "theme.slint"; +import { Button } from "widgets.slint"; + +export component RecoveryPrompt inherits Rectangle { + /// What went wrong, in the user's terms. Empty closes the prompt — one + /// source for "is this open", rather than a bool that can disagree with + /// the words beside it. + in property title; + /// What is safe and what is not, which is the part that determines whether + /// the next minute is frightening. + in property detail; + /// What SQLite actually said, kept because a bug report needs it and + /// because a diagnosis the user can read is worth more than a reassurance + /// they cannot check. + in property diagnosis; + /// The restore offer, naming the backup's date. Empty when there is no + /// backup to restore from, which is the case a fresh install is in. + in property restore-label; + in property can-restore: false; + /// The rebuild offer, naming what it costs — a full rescan, and the + /// collections it cannot bring back. + in property rebuild-label; + /// Set while a restore or rebuild is running, so neither can be started + /// twice against the same file. + in property busy: false; + + callback restore(); + callback rebuild(); + callback dismiss(); + + visible: root.title != ""; + background: #000000E0; + + // Swallows everything that misses the card, and answers nothing. See the + // header: losing this by a stray tap leaves nowhere to go. + TouchArea { } + + Rectangle { + width: min(460px, parent.width - 2 * Theme.gap-lg); + height: min(card.preferred-height, parent.height - 2 * Theme.gap-lg); + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; + background: Theme.surface; + border-radius: Theme.radius; + border-width: 1px; + border-color: Theme.rule; + + TouchArea { } + + card := VerticalLayout { + padding: Theme.gap-lg; + spacing: Theme.gap; + + Text { + text: "Recover library"; + color: Theme.ink-faint; + font-size: Theme.text-sm; + font-weight: 700; + letter-spacing: 1.2px; + } + + Text { + text: root.title; + color: Theme.ink; + font-size: Theme.text-lg; + font-weight: 600; + wrap: word-wrap; + } + + Text { + text: root.detail; + color: Theme.ink-dim; + font-size: Theme.text; + wrap: word-wrap; + } + + // Wrapped rather than elided: this is the one line a bug report + // needs verbatim, and a truncated SQLite message is no message. + Text { + text: root.diagnosis; + color: Theme.ink-faint; + font-size: Theme.text-sm; + wrap: word-wrap; + } + + Rectangle { height: 1px; background: Theme.rule; } + + // Stacked, not a row: each label carries what its answer costs — + // a date, a count of photographs — and three of those side by side + // elide away exactly the part that lets the user choose. + if root.can-restore: Button { + text: root.busy ? "Working…" : root.restore-label; + primary: true; + enabled: !root.busy; + clicked => { root.restore(); } + } + + Button { + text: root.busy ? "Working…" : root.rebuild-label; + // Primary only when it is the only answer there is. A rebuild + // discards collections, so it does not get the emphasis while + // a restore that keeps them is on the table. + primary: !root.can-restore; + enabled: !root.busy; + clicked => { root.rebuild(); } + } + + Button { + text: "Leave it for now"; + enabled: !root.busy; + clicked => { root.dismiss(); } + } + } + } +}