max / makenotwork
11 files changed,
+275 insertions,
-35 deletions
| @@ -46,6 +46,26 @@ | |||
| 46 | 46 | **The unit file.** Same reasoning. A hardened unit that has drifted from the | |
| 47 | 47 | repo is a question for a human, not something a binary deploy overwrites. | |
| 48 | 48 | ||
| 49 | + | ## The database path is config, not environment | |
| 50 | + | ||
| 51 | + | Both instance configs here set `storage.db_path = "/var/lib/pom/pom.db"`, and | |
| 52 | + | that is now the only thing deciding where the database is. It has to be stated, | |
| 53 | + | because the unit sets `XDG_DATA_HOME=/var/lib` and an interactive login does | |
| 54 | + | not: before it was configured, `pom serve` under systemd and `pom test` run by | |
| 55 | + | hand as the `pom` user opened two different files on the same host. The suites | |
| 56 | + | really ran and really passed, into a database nothing serves, while | |
| 57 | + | `/status.json` said no tests had ever run. | |
| 58 | + | ||
| 59 | + | The unit still carries the `XDG_DATA_HOME` line so a rolled-back older binary | |
| 60 | + | finds the same file. A current binary ignores it. | |
| 61 | + | ||
| 62 | + | A first install has to create the database once, since opening a missing one is | |
| 63 | + | now an error rather than a silent create: | |
| 64 | + | ||
| 65 | + | ``` | |
| 66 | + | sudo -u pom pom --init --config /etc/pom/pom.toml status | |
| 67 | + | ``` | |
| 68 | + | ||
| 49 | 69 | ## One-time host setup | |
| 50 | 70 | ||
| 51 | 71 | Each host that runs pom needs the installer and its scoped sudo grant: |
| @@ -7,6 +7,14 @@ | |||
| 7 | 7 | dashboard = true | |
| 8 | 8 | # api_token loaded from POM_API_TOKEN env var | |
| 9 | 9 | ||
| 10 | + | # The database is at a fixed absolute path, not wherever XDG_DATA_HOME happens | |
| 11 | + | # to point. The unit sets XDG_DATA_HOME=/var/lib and an interactive shell does | |
| 12 | + | # not, so before this was configured `pom serve` and a hand-run `pom test` on | |
| 13 | + | # this host opened two different files and neither said so: the suites ran, | |
| 14 | + | # passed, and reported into a database nothing served. | |
| 15 | + | [storage] | |
| 16 | + | db_path = "/var/lib/pom/pom.db" | |
| 17 | + | ||
| 10 | 18 | [instance] | |
| 11 | 19 | name = "astra" | |
| 12 | 20 |
| @@ -7,6 +7,14 @@ | |||
| 7 | 7 | dashboard = false | |
| 8 | 8 | # api_token loaded from POM_API_TOKEN env var | |
| 9 | 9 | ||
| 10 | + | # The database is at a fixed absolute path, not wherever XDG_DATA_HOME happens | |
| 11 | + | # to point. The unit sets XDG_DATA_HOME=/var/lib and an interactive shell does | |
| 12 | + | # not, so before this was configured `pom serve` and a hand-run `pom test` on | |
| 13 | + | # this host opened two different files and neither said so: the suites ran, | |
| 14 | + | # passed, and reported into a database nothing served. | |
| 15 | + | [storage] | |
| 16 | + | db_path = "/var/lib/pom/pom.db" | |
| 17 | + | ||
| 10 | 18 | [instance] | |
| 11 | 19 | name = "hetzner" | |
| 12 | 20 |
| @@ -8,6 +8,9 @@ | |||
| 8 | 8 | User=pom | |
| 9 | 9 | Group=pom | |
| 10 | 10 | EnvironmentFile=-/etc/pom/env | |
| 11 | + | # Kept only so an older binary rolled back under this unit still finds | |
| 12 | + | # /var/lib/pom/pom.db. storage.db_path in pom.toml is the authority now, and a | |
| 13 | + | # current binary ignores this. | |
| 11 | 14 | Environment=XDG_DATA_HOME=/var/lib | |
| 12 | 15 | ExecStart=/usr/local/bin/pom serve --config /etc/pom/pom.toml | |
| 13 | 16 | Restart=on-failure |
| @@ -117,6 +117,14 @@ | |||
| 117 | 117 | ||
| 118 | 118 | SQLite with WAL journal mode. Schema is managed through numbered migrations (currently v1-v13). | |
| 119 | 119 | ||
| 120 | + | ### Where the database lives | |
| 121 | + | ||
| 122 | + | `storage.db_path` in the config file, when set, and it must be absolute. Unset, the path is read out of the XDG data directory, which is where the daemon and the CLI on the same host once disagreed: `pom.service` sets `XDG_DATA_HOME=/var/lib` and an interactive login does not, so `pom serve` wrote `/var/lib/pom/pom.db` while `pom test` run by hand wrote `/var/lib/pom/.local/share/pom/pom.db`. The suites ran, passed, and printed green into a database nothing served. Any instance whose database is not in the invoking user's own `~/.local/share` should set the path; running unconfigured warns. | |
| 123 | + | ||
| 124 | + | Opening a database that does not exist is an error naming the path. `--init` creates it, and is what a first install runs. Auto-creating on open is what turned one wrong path into a silent second store rather than a complaint. | |
| 125 | + | ||
| 126 | + | The instance ID sits beside the database, in whichever directory that resolves to. | |
| 127 | + | ||
| 120 | 128 | ### Tables | |
| 121 | 129 | ||
| 122 | 130 | | Table | Purpose | Key Columns | |
| @@ -1111,6 +1111,7 @@ | |||
| 1111 | 1111 | instance: crate::config::InstanceConfig::default(), | |
| 1112 | 1112 | targets: HashMap::new(), | |
| 1113 | 1113 | peers: HashMap::new(), | |
| 1114 | + | storage: crate::config::StorageConfig::default(), | |
| 1114 | 1115 | alerts: None, | |
| 1115 | 1116 | }; | |
| 1116 | 1117 | config.serve.api_token = api_token.map(std::string::ToString::to_string); |
| @@ -22,10 +22,22 @@ | |||
| 22 | 22 | /// Peer PoM instances for mesh monitoring, keyed by peer name. | |
| 23 | 23 | #[serde(default)] | |
| 24 | 24 | pub peers: HashMap<String, PeerConfig>, | |
| 25 | + | /// Where this instance keeps its database and instance ID. | |
| 26 | + | #[serde(default)] | |
| 27 | + | pub storage: StorageConfig, | |
| 25 | 28 | /// Email alert configuration via Postmark. `None` disables alerting. | |
| 26 | 29 | pub alerts: Option<AlertConfig>, | |
| 27 | 30 | } | |
| 28 | 31 | ||
| 32 | + | #[derive(Debug, Clone, Default, Deserialize)] | |
| 33 | + | pub struct StorageConfig { | |
| 34 | + | /// Absolute path to `pom.db`. Unset falls back to the XDG data directory, | |
| 35 | + | /// which is what the service unit and a hand-run CLI disagree about: set | |
| 36 | + | /// this on any instance whose database is not in the invoking user's own | |
| 37 | + | /// `~/.local/share`, and the two agree by construction. | |
| 38 | + | pub db_path: Option<PathBuf>, | |
| 39 | + | } | |
| 40 | + | ||
| 29 | 41 | // Manual Debug (below) redacts the token, keep field lists in sync when adding | |
| 30 | 42 | // fields. Secrets in a derived Debug are a latent leak on any future log/panic. | |
| 31 | 43 | #[derive(Clone, Deserialize)] | |
| @@ -755,9 +767,40 @@ | |||
| 755 | 767 | } | |
| 756 | 768 | } | |
| 757 | 769 | ||
| 770 | + | // Same rationale as repo.path: a relative db_path would resolve against | |
| 771 | + | // the launch directory, so the service and a shell would open different | |
| 772 | + | // files from the same config. | |
| 773 | + | if let Some(db) = &config.storage.db_path | |
| 774 | + | && !db.is_absolute() | |
| 775 | + | { | |
| 776 | + | return Err(PomError::Config(format!( | |
| 777 | + | "storage.db_path \"{}\" must be absolute", | |
| 778 | + | db.display() | |
| 779 | + | ))); | |
| 780 | + | } | |
| 781 | + | ||
| 758 | 782 | Ok(config) | |
| 759 | 783 | } | |
| 760 | 784 | ||
| 785 | + | /// Resolve where this instance's database lives, and say how it was decided. | |
| 786 | + | /// | |
| 787 | + | /// Configured wins over the environment. The unconfigured path reads | |
| 788 | + | /// `XDG_DATA_HOME`, which is how `pom serve` under systemd and `pom test` in | |
| 789 | + | /// a shell came to open two different databases on the same host: the unit | |
| 790 | + | /// sets the variable and an interactive login does not. | |
| 791 | + | pub fn db_path(&self) -> Result<DbLocation> { | |
| 792 | + | match &self.storage.db_path { | |
| 793 | + | Some(path) => Ok(DbLocation { | |
| 794 | + | path: path.clone(), | |
| 795 | + | source: DbPathSource::Config, | |
| 796 | + | }), | |
| 797 | + | None => Ok(DbLocation { | |
| 798 | + | path: xdg_db_path()?, | |
| 799 | + | source: DbPathSource::XdgDataHome, | |
| 800 | + | }), | |
| 801 | + | } | |
| 802 | + | } | |
| 803 | + | ||
| 761 | 804 | pub fn get_target(&self, name: &str) -> Option<&TargetConfig> { | |
| 762 | 805 | self.targets.get(name) | |
| 763 | 806 | } | |
| @@ -784,12 +827,50 @@ | |||
| 784 | 827 | Ok(config_dir?.join("pom").join("pom.toml")) | |
| 785 | 828 | } | |
| 786 | 829 | ||
| 787 | - | pub fn db_path() -> Result<PathBuf> { | |
| 830 | + | /// A resolved database location, carrying how it was resolved so an error can | |
| 831 | + | /// name the thing the operator has to change. | |
| 832 | + | #[derive(Debug, Clone, PartialEq, Eq)] | |
| 833 | + | pub struct DbLocation { | |
| 834 | + | pub path: PathBuf, | |
| 835 | + | pub source: DbPathSource, | |
| 836 | + | } | |
| 837 | + | ||
| 838 | + | #[derive(Debug, Clone, Copy, PartialEq, Eq)] | |
| 839 | + | pub enum DbPathSource { | |
| 840 | + | /// `storage.db_path` in the config file. | |
| 841 | + | Config, | |
| 842 | + | /// The XDG data directory, i.e. `XDG_DATA_HOME` or its per-platform default. | |
| 843 | + | XdgDataHome, | |
| 844 | + | } | |
| 845 | + | ||
| 846 | + | impl DbLocation { | |
| 847 | + | /// The directory the database and the instance ID share. | |
| 848 | + | pub fn dir(&self) -> &Path { | |
| 849 | + | self.path.parent().unwrap_or(Path::new(".")) | |
| 850 | + | } | |
| 851 | + | } | |
| 852 | + | ||
| 853 | + | impl std::fmt::Display for DbLocation { | |
| 854 | + | fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | |
| 855 | + | write!(f, "{} (from {})", self.path.display(), self.source) | |
| 856 | + | } | |
| 857 | + | } | |
| 858 | + | ||
| 859 | + | impl std::fmt::Display for DbPathSource { | |
| 860 | + | fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | |
| 861 | + | match self { | |
| 862 | + | Self::Config => f.write_str("storage.db_path"), | |
| 863 | + | Self::XdgDataHome => f.write_str("the XDG data directory"), | |
| 864 | + | } | |
| 865 | + | } | |
| 866 | + | } | |
| 867 | + | ||
| 868 | + | /// The unconfigured database path: `$XDG_DATA_HOME/pom/pom.db` or the | |
| 869 | + | /// per-platform equivalent. Resolution only — it creates nothing. | |
| 870 | + | pub fn xdg_db_path() -> Result<PathBuf> { | |
| 788 | 871 | let data_dir = dirs::data_local_dir() | |
| 789 | - | .ok_or_else(|| PomError::Config("Could not determine data directory".into())); | |
| 790 | - | let pom_dir = data_dir?.join("pom"); | |
| 791 | - | std::fs::create_dir_all(&pom_dir)?; | |
| 792 | - | Ok(pom_dir.join("pom.db")) | |
| 872 | + | .ok_or_else(|| PomError::Config("Could not determine data directory".into()))?; | |
| 873 | + | Ok(data_dir.join("pom").join("pom.db")) | |
| 793 | 874 | } | |
| 794 | 875 | ||
| 795 | 876 | #[cfg(test)] | |
| @@ -1570,17 +1651,60 @@ | |||
| 1570 | 1651 | } | |
| 1571 | 1652 | ||
| 1572 | 1653 | #[test] | |
| 1573 | - | fn db_path_ends_in_pom_db() { | |
| 1574 | - | // Same rationale as default_config_path. db_path also has a side | |
| 1575 | - | // effect (creates the parent dir) so we can't easily mock it; the | |
| 1576 | - | // suffix check is the cleanest pin. | |
| 1577 | - | let path = db_path().unwrap(); | |
| 1654 | + | fn xdg_db_path_ends_in_pom_db() { | |
| 1655 | + | // Same rationale as default_config_path: the exact dir varies per OS, | |
| 1656 | + | // the suffix is stable. | |
| 1657 | + | let path = xdg_db_path().unwrap(); | |
| 1578 | 1658 | assert!( | |
| 1579 | 1659 | path.ends_with("pom/pom.db") || path.ends_with("pom\\pom.db"), | |
| 1580 | 1660 | "expected .../pom/pom.db, got {path:?}" | |
| 1581 | 1661 | ); | |
| 1582 | 1662 | } | |
| 1583 | 1663 | ||
| 1664 | + | #[test] | |
| 1665 | + | fn configured_db_path_wins_over_the_environment() { | |
| 1666 | + | // The whole point of storage.db_path: the service unit and a hand-run | |
| 1667 | + | // CLI must resolve to the same file whatever XDG_DATA_HOME says. | |
| 1668 | + | let toml = r#" | |
| 1669 | + | [serve] | |
| 1670 | + | [storage] | |
| 1671 | + | db_path = "/var/lib/pom/pom.db" | |
| 1672 | + | "#; | |
| 1673 | + | let config: Config = toml::from_str(toml).unwrap(); | |
| 1674 | + | let loc = config.db_path().unwrap(); | |
| 1675 | + | assert_eq!(loc.path, PathBuf::from("/var/lib/pom/pom.db")); | |
| 1676 | + | assert_eq!(loc.source, DbPathSource::Config); | |
| 1677 | + | assert_eq!(loc.dir(), Path::new("/var/lib/pom")); | |
| 1678 | + | } | |
| 1679 | + | ||
| 1680 | + | #[test] | |
| 1681 | + | fn unconfigured_db_path_falls_back_to_xdg() { | |
| 1682 | + | let toml = "[serve]\n"; | |
| 1683 | + | let config: Config = toml::from_str(toml).unwrap(); | |
| 1684 | + | let loc = config.db_path().unwrap(); | |
| 1685 | + | assert_eq!(loc.path, xdg_db_path().unwrap()); | |
| 1686 | + | assert_eq!(loc.source, DbPathSource::XdgDataHome); | |
| 1687 | + | } | |
| 1688 | + | ||
| 1689 | + | #[test] | |
| 1690 | + | fn config_load_rejects_relative_db_path() { | |
| 1691 | + | let toml = r#" | |
| 1692 | + | [serve] | |
| 1693 | + | [storage] | |
| 1694 | + | db_path = "pom.db" | |
| 1695 | + | "#; | |
| 1696 | + | let dir = std::env::temp_dir().join(format!("pom-cfg-{}", std::process::id())); | |
| 1697 | + | std::fs::create_dir_all(&dir).unwrap(); | |
| 1698 | + | let path = dir.join("relative-db.toml"); | |
| 1699 | + | std::fs::write(&path, toml).unwrap(); | |
| 1700 | + | let err = Config::load(Some(&path)).unwrap_err(); | |
| 1701 | + | assert!( | |
| 1702 | + | err.to_string().contains("must be absolute"), | |
| 1703 | + | "expected an absolute-path complaint, got {err}" | |
| 1704 | + | ); | |
| 1705 | + | std::fs::remove_file(&path).ok(); | |
| 1706 | + | } | |
| 1707 | + | ||
| 1584 | 1708 | #[test] | |
| 1585 | 1709 | fn config_load_rejects_route_without_leading_slash() { | |
| 1586 | 1710 | // Catches `delete ! in Config::load` (L431): without the `!`, the |
| @@ -3,7 +3,7 @@ | |||
| 3 | 3 | use clap::{Parser, Subcommand}; | |
| 4 | 4 | use rmcp::ServiceExt; | |
| 5 | 5 | use tokio::io::{stdin, stdout}; | |
| 6 | - | use tracing::info; | |
| 6 | + | use tracing::{info, warn}; | |
| 7 | 7 | use tracing_subscriber::{EnvFilter, fmt, prelude::*}; | |
| 8 | 8 | ||
| 9 | 9 | use pom::config::{self, Config}; | |
| @@ -24,6 +24,12 @@ | |||
| 24 | 24 | #[arg(long, global = true)] | |
| 25 | 25 | config: Option<std::path::PathBuf>, | |
| 26 | 26 | ||
| 27 | + | /// Create the database if it does not exist yet. Without this, opening a | |
| 28 | + | /// missing database is an error: auto-creating is what let a wrong path | |
| 29 | + | /// become a silent second store rather than a complaint. | |
| 30 | + | #[arg(long, global = true)] | |
| 31 | + | init: bool, | |
| 32 | + | ||
| 27 | 33 | #[command(subcommand)] | |
| 28 | 34 | command: Option<Commands>, | |
| 29 | 35 | } | |
| @@ -105,13 +111,19 @@ | |||
| 105 | 111 | let config_path = cli.config.as_deref(); | |
| 106 | 112 | let config = Config::load(config_path)?; | |
| 107 | 113 | ||
| 114 | + | let on_missing = if cli.init { | |
| 115 | + | db::OnMissingDb::Create | |
| 116 | + | } else { | |
| 117 | + | db::OnMissingDb::Fail | |
| 118 | + | }; | |
| 119 | + | ||
| 108 | 120 | match cli.command { | |
| 109 | - | None => run_mcp_server(config).await, | |
| 110 | - | Some(cmd) => run_cli(cmd, config).await, | |
| 121 | + | None => run_mcp_server(config, on_missing).await, | |
| 122 | + | Some(cmd) => run_cli(cmd, config, on_missing).await, | |
| 111 | 123 | } | |
| 112 | 124 | } | |
| 113 | 125 | ||
| 114 | - | async fn run_mcp_server(config: Config) -> Result<()> { | |
| 126 | + | async fn run_mcp_server(config: Config, on_missing: db::OnMissingDb) -> Result<()> { | |
| 115 | 127 | tracing_subscriber::registry() | |
| 116 | 128 | .with(fmt::layer().with_writer(std::io::stderr)) | |
| 117 | 129 | .with(EnvFilter::from_default_env().add_directive("pom=info".parse()?)) | |
| @@ -119,9 +131,9 @@ | |||
| 119 | 131 | ||
| 120 | 132 | info!("Starting PoM MCP server"); | |
| 121 | 133 | ||
| 122 | - | let db_path = config::db_path()?; | |
| 123 | - | let pool = db::connect(&db_path).await?; | |
| 124 | - | info!("Database ready at {}", db_path.display()); | |
| 134 | + | let db = config.db_path()?; | |
| 135 | + | let pool = db::connect(&db.path, on_missing).await?; | |
| 136 | + | info!("Database ready at {db}"); | |
| 125 | 137 | ||
| 126 | 138 | let server = PomServer::new(pool, config); | |
| 127 | 139 | let transport = (stdin(), stdout()); | |
| @@ -134,7 +146,7 @@ | |||
| 134 | 146 | Ok(()) | |
| 135 | 147 | } | |
| 136 | 148 | ||
| 137 | - | async fn run_cli(cmd: Commands, config: Config) -> Result<()> { | |
| 149 | + | async fn run_cli(cmd: Commands, config: Config, on_missing: db::OnMissingDb) -> Result<()> { | |
| 138 | 150 | let log_level = if matches!(cmd, Commands::Serve) { | |
| 139 | 151 | "pom=info" | |
| 140 | 152 | } else { | |
| @@ -145,8 +157,20 @@ | |||
| 145 | 157 | .with(EnvFilter::from_default_env().add_directive(log_level.parse()?)) | |
| 146 | 158 | .init(); | |
| 147 | 159 | ||
| 148 | - | let db_path = config::db_path()?; | |
| 149 | - | let pool = db::connect(&db_path).await?; | |
| 160 | + | let db = config.db_path()?; | |
| 161 | + | info!("Database at {db}"); | |
| 162 | + | // An unconfigured path is read out of XDG_DATA_HOME, which the service unit | |
| 163 | + | // sets and an interactive shell does not — the two then open different | |
| 164 | + | // files on the same host and neither says so. Warn rather than info, so it | |
| 165 | + | // clears the CLI's `pom=warn` filter and is visible without RUST_LOG. | |
| 166 | + | if db.source == config::DbPathSource::XdgDataHome { | |
| 167 | + | warn!( | |
| 168 | + | "database path came from the environment, not the config. Set \ | |
| 169 | + | storage.db_path in pom.toml to pin it: {}", | |
| 170 | + | db.path.display() | |
| 171 | + | ); | |
| 172 | + | } | |
| 173 | + | let pool = db::connect(&db.path, on_missing).await?; | |
| 150 | 174 | ||
| 151 | 175 | match cmd { | |
| 152 | 176 | Commands::Health { target, json } => { |
| @@ -2,6 +2,7 @@ | |||
| 2 | 2 | ||
| 3 | 3 | use serde::Serialize; | |
| 4 | 4 | use std::collections::HashMap; | |
| 5 | + | use std::path::Path; | |
| 5 | 6 | use std::sync::Arc; | |
| 6 | 7 | use tokio::sync::RwLock; | |
| 7 | 8 | ||
| @@ -9,7 +10,7 @@ | |||
| 9 | 10 | ||
| 10 | 11 | use crate::alerts::Alerter; | |
| 11 | 12 | use crate::config::PeerConfig; | |
| 12 | - | use crate::error::{PomError, Result}; | |
| 13 | + | use crate::error::Result; | |
| 13 | 14 | ||
| 14 | 15 | /// Cap on a peer response body (matches the health probe's `MAX_RESPONSE_BYTES`). | |
| 15 | 16 | /// A hostile or MITM'd peer could otherwise return a multi-GB body that | |
| @@ -58,17 +59,17 @@ | |||
| 58 | 59 | // Identity | |
| 59 | 60 | ||
| 60 | 61 | /// Load or create a persistent instance ID (UUID v4). | |
| 61 | - | /// Stored at `~/.local/share/pom/instance_id`, same directory as `pom.db`. | |
| 62 | - | pub fn load_or_create_instance_id(override_id: Option<&str>) -> Result<String> { | |
| 62 | + | /// | |
| 63 | + | /// Stored in `data_dir`, which is the directory holding `pom.db` — passed in | |
| 64 | + | /// rather than re-derived, so the ID cannot land beside a different database | |
| 65 | + | /// than the one this process opened. | |
| 66 | + | pub fn load_or_create_instance_id(override_id: Option<&str>, data_dir: &Path) -> Result<String> { | |
| 63 | 67 | if let Some(id) = override_id { | |
| 64 | 68 | return Ok(id.to_string()); | |
| 65 | 69 | } | |
| 66 | 70 | ||
| 67 | - | let data_dir = dirs::data_local_dir() | |
| 68 | - | .ok_or_else(|| PomError::Config("Could not determine data directory".into()))?; | |
| 69 | - | let pom_dir = data_dir.join("pom"); | |
| 70 | - | std::fs::create_dir_all(&pom_dir)?; | |
| 71 | - | let id_path = pom_dir.join("instance_id"); | |
| 71 | + | std::fs::create_dir_all(data_dir)?; | |
| 72 | + | let id_path = data_dir.join("instance_id"); | |
| 72 | 73 | ||
| 73 | 74 | if id_path.exists() { | |
| 74 | 75 | let id = std::fs::read_to_string(&id_path)?.trim().to_string(); | |
| @@ -477,14 +478,27 @@ | |||
| 477 | 478 | ||
| 478 | 479 | #[test] | |
| 479 | 480 | fn override_id_takes_precedence() { | |
| 480 | - | let id = load_or_create_instance_id(Some("override-id")).unwrap(); | |
| 481 | + | let dir = std::env::temp_dir().join(format!("pom-id-override-{}", std::process::id())); | |
| 482 | + | let id = load_or_create_instance_id(Some("override-id"), &dir).unwrap(); | |
| 481 | 483 | assert_eq!(id, "override-id"); | |
| 484 | + | // An override answers without touching the disk at all. | |
| 485 | + | assert!(!dir.exists()); | |
| 482 | 486 | } | |
| 483 | 487 | ||
| 484 | 488 | #[test] | |
| 485 | - | fn auto_id_is_valid_uuid() { | |
| 486 | - | let id = load_or_create_instance_id(None).unwrap(); | |
| 489 | + | fn auto_id_is_valid_uuid_and_persists_beside_the_db() { | |
| 490 | + | let dir = std::env::temp_dir().join(format!("pom-id-auto-{}", std::process::id())); | |
| 491 | + | std::fs::remove_dir_all(&dir).ok(); | |
| 492 | + | let id = load_or_create_instance_id(None, &dir).unwrap(); | |
| 487 | 493 | assert!(uuid::Uuid::parse_str(&id).is_ok()); | |
| 494 | + | // Written where the caller said, not where XDG_DATA_HOME points, and | |
| 495 | + | // stable across calls. | |
| 496 | + | assert_eq!( | |
| 497 | + | std::fs::read_to_string(dir.join("instance_id")).unwrap(), | |
| 498 | + | id | |
| 499 | + | ); | |
| 500 | + | assert_eq!(load_or_create_instance_id(None, &dir).unwrap(), id); | |
| 501 | + | std::fs::remove_dir_all(&dir).ok(); | |
| 488 | 502 | } | |
| 489 | 503 | ||
| 490 | 504 | #[test] |
| @@ -20,7 +20,8 @@ | |||
| 20 | 20 | // Cancellation token for graceful shutdown | |
| 21 | 21 | let token = tokio_util::sync::CancellationToken::new(); | |
| 22 | 22 | ||
| 23 | - | let instance_id = peer::load_or_create_instance_id(config.instance.id.as_deref())?; | |
| 23 | + | let instance_id = | |
| 24 | + | peer::load_or_create_instance_id(config.instance.id.as_deref(), config.db_path()?.dir())?; | |
| 24 | 25 | let instance_name = config.instance_name(); | |
| 25 | 26 | let instance_info = peer::InstanceInfo { | |
| 26 | 27 | id: instance_id.clone(), |
| @@ -3,6 +3,7 @@ | |||
| 3 | 3 | //! databases are detected and stamped as version 1. | |
| 4 | 4 | ||
| 5 | 5 | use super::{FromStr, Path, Result, SqliteConnectOptions, SqlitePool, SqlitePoolOptions}; | |
| 6 | + | use crate::error::PomError; | |
| 6 | 7 | use tracing::{info, instrument}; | |
| 7 | 8 | ||
| 8 | 9 | /// Each migration is a (version, description, SQL) tuple. Versions start at 1. | |
| @@ -319,10 +320,38 @@ | |||
| 319 | 320 | ), | |
| 320 | 321 | ]; | |
| 321 | 322 | ||
| 322 | - | #[instrument(skip_all)] | |
| 323 | - | pub async fn connect(path: &Path) -> Result<SqlitePool> { | |
| 323 | + | /// What to do when the database file is not there yet. | |
| 324 | + | #[derive(Debug, Clone, Copy, PartialEq, Eq)] | |
| 325 | + | pub enum OnMissingDb { | |
| 326 | + | /// Create the file (and its parent directory) — `pom --init`. | |
| 327 | + | Create, | |
| 328 | + | /// Fail, naming the path. Creating on open is what turned one wrong path | |
| 329 | + | /// into a silent second database: five suites ran, passed, and reported | |
| 330 | + | /// into a file nothing served. | |
| 331 | + | Fail, | |
| 332 | + | } | |
| 333 | + | ||
| 334 | + | #[instrument(skip(on_missing))] | |
| 335 | + | pub async fn connect(path: &Path, on_missing: OnMissingDb) -> Result<SqlitePool> { | |
| 336 | + | if !path.exists() { | |
| 337 | + | match on_missing { | |
| 338 | + | OnMissingDb::Fail => { | |
| 339 | + | return Err(PomError::Config(format!( | |
| 340 | + | "no database at {}. Run `pom --init` to create it, or point \ | |
| 341 | + | storage.db_path at the existing one", | |
| 342 | + | path.display() | |
| 343 | + | ))); | |
| 344 | + | } | |
| 345 | + | OnMissingDb::Create => { | |
| 346 | + | if let Some(dir) = path.parent() { | |
| 347 | + | std::fs::create_dir_all(dir)?; | |
| 348 | + | } | |
| 349 | + | } | |
| 350 | + | } | |
| 351 | + | } | |
| 352 | + | ||
| 324 | 353 | let opts = SqliteConnectOptions::from_str(&format!("sqlite:{}", path.display()))? | |
| 325 | - | .create_if_missing(true) | |
| 354 | + | .create_if_missing(on_missing == OnMissingDb::Create) | |
| 326 | 355 | .foreign_keys(true) | |
| 327 | 356 | .journal_mode(sqlx::sqlite::SqliteJournalMode::Wal); | |
| 328 | 357 |