diff --git a/Cargo.lock b/Cargo.lock index c5bb67e..16ffffb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3329,6 +3329,7 @@ dependencies = [ "lumbridge-ui-fixture", "serde", "serde_json", + "tempfile", ] [[package]] diff --git a/apps/lumbridge/Cargo.toml b/apps/lumbridge/Cargo.toml index dee9ad3..4ba36ee 100644 --- a/apps/lumbridge/Cargo.toml +++ b/apps/lumbridge/Cargo.toml @@ -20,5 +20,8 @@ lumbridge-ui-fixture = { path = "../../crates/lumbridge-ui-fixture" } serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.149" +[dev-dependencies] +tempfile = "3.27.0" + [lints] workspace = true diff --git a/apps/lumbridge/src/main.rs b/apps/lumbridge/src/main.rs index 1c1f027..f2100a3 100644 --- a/apps/lumbridge/src/main.rs +++ b/apps/lumbridge/src/main.rs @@ -3,6 +3,7 @@ mod geometry; mod input; mod keymap; mod panel_registry; +mod persistence; mod settings_view; mod sidebar; mod theme; @@ -10,7 +11,6 @@ mod timing; mod usage_feed; use std::collections::BTreeMap; -use std::path::PathBuf; use std::time::Duration; use gpui::{ @@ -52,7 +52,6 @@ use usage_feed::{UsageFeed, UsageSegment}; const RUNTIME_POLL_INTERVAL: Duration = Duration::from_millis(16); const RUNTIME_DRAIN_LIMIT: usize = 64; -const WORKSPACE_SNAPSHOT_ID: &str = "lumbridge-code-gpui-spike"; /// The point size the terminal is painted at. const TERMINAL_FONT_SIZE: f32 = 13.0; /// Named rather than "monospace" so the measurement and the painting agree on a @@ -499,67 +498,6 @@ fn dim_color(color: Rgba) -> Rgba { } } -fn workspace_database_path() -> Option { - if let Some(path) = std::env::var_os("LUMBRIDGE_SPIKE_DB") { - return Some(PathBuf::from(path)); - } - if let Some(root) = std::env::var_os("XDG_DATA_HOME") { - return Some(PathBuf::from(root).join("lumbridge/lumbridge.db")); - } - std::env::var_os("HOME") - .map(PathBuf::from) - .map(|home| home.join(".local/share/lumbridge/lumbridge.db")) -} - -fn load_panel_registry() -> (Option, PanelRegistry, String) { - let Some(path) = workspace_database_path() else { - return ( - None, - PanelRegistry::first_run(), - "layout memory-only · no data directory".to_owned(), - ); - }; - if let Some(parent) = path.parent() - && let Err(error) = std::fs::create_dir_all(parent) - { - return ( - None, - PanelRegistry::first_run(), - format!("layout memory-only · {error}"), - ); - } - let store = match Store::open(&path) { - Ok(store) => store, - Err(error) => { - return ( - None, - PanelRegistry::first_run(), - format!("layout memory-only · {error}"), - ); - } - }; - match store.workspace_snapshot(WORKSPACE_SNAPSHOT_ID) { - Ok(Some(json)) => match PanelRegistry::from_json(&json) { - Ok(panels) => (Some(store), panels, "layout restored · SQLite".to_owned()), - Err(error) => ( - Some(store), - PanelRegistry::first_run(), - format!("invalid layout ignored · {error}"), - ), - }, - Ok(None) => ( - Some(store), - PanelRegistry::first_run(), - "layout ready · SQLite".to_owned(), - ), - Err(error) => ( - Some(store), - PanelRegistry::first_run(), - format!("layout read failed · {error}"), - ), - } -} - impl LumbridgeShell { fn new(window: &mut Window, cx: &mut Context) -> Self { let root_focus = cx.focus_handle(); @@ -602,7 +540,7 @@ impl LumbridgeShell { }) .detach(); - let (store, panels, persistence_status) = load_panel_registry(); + let (store, panels, persistence_status) = persistence::load_panel_registry(&ProcessEnv); let sidebar = SidebarState::default(); let cell = CellMetrics::measure(cx); let terminal_dimensions = terminal_dimensions_for_window( @@ -680,23 +618,19 @@ impl LumbridgeShell { } } + /// Writes the layout, or records why it was not written. + /// + /// The whole decision lives in [`persistence::persist_panels`]; what stays + /// here is the one thing the shell owns, which is that a workspace with no + /// store is not a failure to report. It is memory-only, the footer already + /// says so from startup, and overwriting that with a save error every time + /// a pane moved would replace an accurate standing message with a noisier + /// one that says less. fn persist_panels(&mut self) { let Some(store) = &self.store else { return; }; - let result = self - .panels - .to_json() - .map_err(|error| error.clone()) - .and_then(|json| { - store - .save_workspace_snapshot(WORKSPACE_SNAPSHOT_ID, &json) - .map_err(|error| error.to_string()) - }); - self.persistence_status = match result { - Ok(()) => "layout saved · SQLite".to_owned(), - Err(error) => format!("layout save failed · {error}"), - }; + self.persistence_status = persistence::persist_panels(store, &self.panels); } fn dispatch(&mut self, action: ShellAction) -> ActionOutcome { diff --git a/apps/lumbridge/src/persistence.rs b/apps/lumbridge/src/persistence.rs new file mode 100644 index 0000000..2ca7427 --- /dev/null +++ b/apps/lumbridge/src/persistence.rs @@ -0,0 +1,317 @@ +//! Where the workspace layout is kept, and what the shell says when keeping it +//! did not work. +//! +//! Two rules run through everything here. The first is that losing the layout +//! must never lose the session: every failure below falls back to +//! [`PanelRegistry::first_run`] and carries on, because a shell that refuses to +//! open because a database file is unreadable has turned a cosmetic problem +//! into a total one. The second is that the fallback must be *visible*. Each +//! path returns a status string the footer shows, so `layout memory-only` and +//! `layout saved · SQLite` are different words on screen; silently degrading to +//! memory-only would leave a user rearranging panes every morning with no idea +//! why they never stick. +//! +//! Those strings are the reason this file has tests at all. They are the entire +//! user-visible contract of the persistence layer, and until now not one of +//! them was asserted anywhere — the only way to find out that a broken database +//! reports "layout read failed" rather than nothing was to break one. +//! +//! This module does IO and still has no renderer in it, which is what makes the +//! tests possible: a `tempfile::TempDir` and a [`MapEnv`] reproduce every +//! branch, including the ones that only happen on a machine with no writable +//! data directory. +//! +//! The environment is read through [`EnvSource`] rather than `std::env` +//! directly, for the reason that trait already documents in +//! `lumbridge-settings`: the workspace forbids `unsafe`, `set_var` is unsafe in +//! Rust 2024, and a precedence rule that cannot be tested without mutating the +//! process running the test is a precedence rule nobody will test. + +use std::path::PathBuf; + +use lumbridge_settings::EnvSource; +use lumbridge_storage::Store; + +use crate::panel_registry::PanelRegistry; + +/// The workspace this build reads and writes. +/// +/// One workspace, named after the spike the shell grew out of. Changing the +/// string orphans every layout already on disk, which is why it is a constant +/// and not a format string over anything. +const WORKSPACE_SNAPSHOT_ID: &str = "lumbridge-code-gpui-spike"; + +/// Where the layout database lives, in order of who gets to decide. +/// +/// `LUMBRIDGE_SPIKE_DB` first so a test or a second instance can be pointed at +/// its own file without touching the real one; then `XDG_DATA_HOME`, which is +/// the answer on a machine that has one; then `HOME` with the path XDG +/// specifies as the default. `None` means there is nowhere to write, not that +/// writing failed — the caller distinguishes the two in what it tells the user. +pub(crate) fn workspace_database_path(env: &impl EnvSource) -> Option { + if let Some(path) = env.get("LUMBRIDGE_SPIKE_DB") { + return Some(PathBuf::from(path)); + } + if let Some(root) = env.get("XDG_DATA_HOME") { + return Some(PathBuf::from(root).join("lumbridge/lumbridge.db")); + } + env.get("HOME") + .map(PathBuf::from) + .map(|home| home.join(".local/share/lumbridge/lumbridge.db")) +} + +pub(crate) fn load_panel_registry(env: &impl EnvSource) -> (Option, PanelRegistry, String) { + let Some(path) = workspace_database_path(env) else { + return ( + None, + PanelRegistry::first_run(), + "layout memory-only · no data directory".to_owned(), + ); + }; + if let Some(parent) = path.parent() + && let Err(error) = std::fs::create_dir_all(parent) + { + return ( + None, + PanelRegistry::first_run(), + format!("layout memory-only · {error}"), + ); + } + let store = match Store::open(&path) { + Ok(store) => store, + Err(error) => { + return ( + None, + PanelRegistry::first_run(), + format!("layout memory-only · {error}"), + ); + } + }; + match store.workspace_snapshot(WORKSPACE_SNAPSHOT_ID) { + Ok(Some(json)) => match PanelRegistry::from_json(&json) { + Ok(panels) => (Some(store), panels, "layout restored · SQLite".to_owned()), + Err(error) => ( + Some(store), + PanelRegistry::first_run(), + format!("invalid layout ignored · {error}"), + ), + }, + Ok(None) => ( + Some(store), + PanelRegistry::first_run(), + "layout ready · SQLite".to_owned(), + ), + Err(error) => ( + Some(store), + PanelRegistry::first_run(), + format!("layout read failed · {error}"), + ), + } +} + +/// Writes the layout and reports what happened, in the footer's own words. +pub(crate) fn persist_panels(store: &Store, panels: &PanelRegistry) -> String { + let result = panels + .to_json() + .map_err(|error| error.clone()) + .and_then(|json| { + store + .save_workspace_snapshot(WORKSPACE_SNAPSHOT_ID, &json) + .map_err(|error| error.to_string()) + }); + match result { + Ok(()) => "layout saved · SQLite".to_owned(), + Err(error) => format!("layout save failed · {error}"), + } +} + +#[cfg(test)] +mod tests { + use super::{ + WORKSPACE_SNAPSHOT_ID, load_panel_registry, persist_panels, workspace_database_path, + }; + use crate::panel_registry::PanelRegistry; + use lumbridge_settings::MapEnv; + use lumbridge_storage::Store; + use std::path::PathBuf; + + /// An environment pointing the shell at a database inside `dir`. + fn spike_db(dir: &tempfile::TempDir) -> MapEnv { + let path = dir.path().join("workspace.db"); + MapEnv::new([( + "LUMBRIDGE_SPIKE_DB", + path.to_str().expect("the tempdir path is UTF-8"), + )]) + } + + #[test] + fn the_override_outranks_xdg_and_xdg_outranks_home() { + // The order matters more than it looks: an instance told to use its own + // database must not quietly write to the real one, and a machine with + // XDG_DATA_HOME set has already answered the question HOME would only + // be guessing at. + assert_eq!( + workspace_database_path(&MapEnv::new([ + ("LUMBRIDGE_SPIKE_DB", "/tmp/explicit.db"), + ("XDG_DATA_HOME", "/tmp/xdg"), + ("HOME", "/home/someone"), + ])), + Some(PathBuf::from("/tmp/explicit.db")) + ); + assert_eq!( + workspace_database_path(&MapEnv::new([ + ("XDG_DATA_HOME", "/tmp/xdg"), + ("HOME", "/home/someone"), + ])), + Some(PathBuf::from("/tmp/xdg/lumbridge/lumbridge.db")) + ); + assert_eq!( + workspace_database_path(&MapEnv::new([("HOME", "/home/someone")])), + Some(PathBuf::from( + "/home/someone/.local/share/lumbridge/lumbridge.db" + )) + ); + } + + #[test] + fn no_data_directory_at_all_is_none_rather_than_a_relative_path() { + // Falling back to a bare "lumbridge.db" here would write into whatever + // directory the process happened to start in, which is a file nobody + // asked for in a place nobody will look. + assert_eq!(workspace_database_path(&MapEnv::default()), None); + } + + #[test] + fn with_nowhere_to_write_the_shell_still_opens_and_says_it_is_memory_only() { + let (store, panels, status) = load_panel_registry(&MapEnv::default()); + assert!(store.is_none()); + assert_eq!(panels.attached_count(), 3, "the first-run workspace opens"); + assert_eq!(status, "layout memory-only · no data directory"); + } + + #[test] + fn a_directory_that_cannot_be_created_degrades_to_memory_only() { + let dir = tempfile::tempdir().expect("tempdir"); + let blocker = dir.path().join("not-a-directory"); + std::fs::write(&blocker, b"").expect("write the blocking file"); + let inside = blocker.join("workspace.db"); + let env = MapEnv::new([( + "LUMBRIDGE_SPIKE_DB", + inside.to_str().expect("the tempdir path is UTF-8"), + )]); + + let (store, _, status) = load_panel_registry(&env); + assert!(store.is_none()); + assert!( + status.starts_with("layout memory-only · "), + "an unusable data directory must be named, not swallowed: {status}" + ); + } + + #[test] + fn a_database_that_will_not_open_degrades_to_memory_only() { + // The path exists and is a directory, so create_dir_all succeeds and + // SQLite is the one that refuses. This is the branch that separates + // "nowhere to write" from "somewhere that does not work". + let dir = tempfile::tempdir().expect("tempdir"); + let occupied = dir.path().join("occupied"); + std::fs::create_dir(&occupied).expect("create the occupying directory"); + let env = MapEnv::new([( + "LUMBRIDGE_SPIKE_DB", + occupied.to_str().expect("the tempdir path is UTF-8"), + )]); + + let (store, panels, status) = load_panel_registry(&env); + assert!(store.is_none()); + assert_eq!(panels.attached_count(), 3); + assert!( + status.starts_with("layout memory-only · "), + "a database that will not open must be named: {status}" + ); + assert_ne!(status, "layout memory-only · no data directory"); + } + + #[test] + fn a_fresh_database_is_ready_rather_than_restored() { + // "restored" is a claim about the user's own layout. Saying it over a + // database with nothing in it would be a lie the very first time the + // app is opened. + let dir = tempfile::tempdir().expect("tempdir"); + let (store, _, status) = load_panel_registry(&spike_db(&dir)); + assert!(store.is_some()); + assert_eq!(status, "layout ready · SQLite"); + } + + #[test] + fn a_saved_layout_comes_back_and_says_it_was_restored() { + let dir = tempfile::tempdir().expect("tempdir"); + let env = spike_db(&dir); + + let (store, mut panels, _) = load_panel_registry(&env); + let store = store.expect("the store opened"); + let created = panels.create(crate::panel_registry::PanelKind::Terminal); + assert_eq!(persist_panels(&store, &panels), "layout saved · SQLite"); + drop(store); + + let (_, restored, status) = load_panel_registry(&env); + assert_eq!(status, "layout restored · SQLite"); + assert!( + restored.panel(created).is_some(), + "the panel created before the save must survive it" + ); + } + + #[test] + fn an_unreadable_layout_is_ignored_by_name_and_the_workspace_still_opens() { + // The failure this guards is the worst one available: a snapshot the + // shell cannot parse must not stop the shell starting, and it must not + // be discarded quietly either, because the status line is the only + // notice a user gets that their arrangement was dropped. + let dir = tempfile::tempdir().expect("tempdir"); + let env = spike_db(&dir); + let path = workspace_database_path(&env).expect("the override names a path"); + let store = Store::open(&path).expect("the store opened"); + store + .save_workspace_snapshot(WORKSPACE_SNAPSHOT_ID, "{ not json at all") + .expect("the snapshot row was written"); + drop(store); + + let (store, panels, status) = load_panel_registry(&env); + assert!( + store.is_some(), + "a corrupt snapshot is not a reason to stop persisting" + ); + assert!( + status.starts_with("invalid layout ignored · "), + "the reason must reach the footer: {status}" + ); + assert_eq!( + panels.attached_count(), + PanelRegistry::first_run().attached_count() + ); + } + + #[test] + fn a_structurally_valid_but_impossible_layout_is_refused_the_same_way() { + // Parsing is not validation. A snapshot whose selected panel is not + // attached is well-formed JSON and an unopenable workspace, and it must + // land in the same fallback rather than reaching the shell. + let dir = tempfile::tempdir().expect("tempdir"); + let env = spike_db(&dir); + let path = workspace_database_path(&env).expect("the override names a path"); + let store = Store::open(&path).expect("the store opened"); + store + .save_workspace_snapshot( + WORKSPACE_SNAPSHOT_ID, + r#"{"schema_version":1,"panels":[],"selected":1,"next_id":2}"#, + ) + .expect("the snapshot row was written"); + drop(store); + + let (_, _, status) = load_panel_registry(&env); + assert!( + status.starts_with("invalid layout ignored · "), + "validation failures share the fallback with parse failures: {status}" + ); + } +}