improved settings cli flags > env vars > defaults

6 files changed, 215 insertions, 65 deletions

diff --git a/src/config.rs b/src/config.rs
index 9b0d0b8..d095178 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -1,74 +1,205 @@
-use anyhow::{Context, Result};
+use anyhow::Result;
+use clap::{Parser, ValueEnum};
 use serde::{Deserialize, Serialize};
-use std::env;
 
 /// Database backend type for the relay
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default, ValueEnum)]
 #[serde(rename_all = "lowercase")]
-#[derive(Default)]
 pub enum DatabaseBackend {
-    /// In-memory database (default, fastest, no persistence)
+    /// LMDB backend (persistent, general purpose)
     #[default]
-    Memory,
+    Lmdb,
     /// NostrDB backend (persistent, optimized for Nostr)
     NostrDb,
-    /// LMDB backend (persistent, general purpose)
-    Lmdb,
+    /// In-memory database (fastest, no persistence - uses temp directory for git data)
+    Memory,
 }
 
-impl std::str::FromStr for DatabaseBackend {
-    type Err = anyhow::Error;
-
-    fn from_str(s: &str) -> Result<Self> {
-        match s.to_lowercase().as_str() {
-            "memory" => Ok(Self::Memory),
-            "nostrdb" => Ok(Self::NostrDb),
-            "lmdb" => Ok(Self::Lmdb),
-            _ => Err(anyhow::anyhow!(
-                "Invalid database backend: {}. Valid options: memory, nostrdb, lmdb",
-                s
-            )),
+impl std::fmt::Display for DatabaseBackend {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Memory => write!(f, "memory"),
+            Self::NostrDb => write!(f, "nostrdb"),
+            Self::Lmdb => write!(f, "lmdb"),
         }
     }
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
+/// ngit-grasp - A GRASP (Git Relays Authorized via Signed-Nostr Proofs) implementation
+///
+/// Configuration is loaded with the following priority (highest to lowest):
+/// 1. CLI flags (e.g., --domain example.com)
+/// 2. Environment variables (e.g., NGIT_DOMAIN=example.com)
+/// 3. .env file (loaded automatically if present)
+/// 4. Built-in defaults
+#[derive(Debug, Clone, Serialize, Deserialize, Parser)]
+#[command(author, version, about, long_about = None)]
+#[command(propagate_version = true)]
 pub struct Config {
+    /// Domain where this instance is hosted (required, used in GRASP validation)
+    #[arg(long, env = "NGIT_DOMAIN")]
     pub domain: String,
-    pub owner_npub: String,
-    pub relay_name: String,
+
+    /// Owner's npub (optional, for relay info in NIP-11)
+    #[arg(long, env = "NGIT_OWNER_NPUB")]
+    pub owner_npub: Option<String>,
+
+    /// Relay name for NIP-11 information document (defaults to "${domain} grasp relay")
+    #[arg(long = "relay-name", env = "NGIT_RELAY_NAME")]
+    pub relay_name_override: Option<String>,
+
+    /// Relay description for NIP-11 information document
+    #[arg(
+        long,
+        env = "NGIT_RELAY_DESCRIPTION",
+        default_value = "Git Nostr Relay - a grasp implementation"
+    )]
     pub relay_description: String,
+
+    /// Path to store Git repositories
+    #[arg(long, env = "NGIT_GIT_DATA_PATH", default_value = "./data/git")]
     pub git_data_path: String,
+
+    /// Path to store Nostr relay data
+    #[arg(long, env = "NGIT_RELAY_DATA_PATH", default_value = "./data/relay")]
     pub relay_data_path: String,
+
+    /// Server bind address (IP:PORT)
+    #[arg(long, env = "NGIT_BIND_ADDRESS", default_value = "127.0.0.1:8080")]
     pub bind_address: String,
+
+    /// Database backend type
+    #[arg(long, env = "NGIT_DATABASE_BACKEND", value_enum, default_value_t = DatabaseBackend::Lmdb)]
     pub database_backend: DatabaseBackend,
 }
 
 impl Config {
-    pub fn from_env() -> Result<Self> {
-        // Load .env file if present
+    /// Load configuration from CLI args, environment variables, and defaults.
+    ///
+    /// Priority (highest to lowest):
+    /// 1. CLI flags
+    /// 2. Environment variables
+    /// 3. .env file
+    /// 4. Built-in defaults
+    pub fn load() -> Result<Self> {
+        // Load .env file if present (before clap parses, so env vars are available)
         dotenvy::dotenv().ok();
 
-        // Parse database backend from environment
-        let database_backend = env::var("NGIT_DATABASE_BACKEND")
-            .ok()
-            .and_then(|s| s.parse().ok())
-            .unwrap_or_default();
-
-        Ok(Config {
-            domain: env::var("NGIT_DOMAIN").unwrap_or_else(|_| "localhost:8080".to_string()),
-            owner_npub: env::var("NGIT_OWNER_NPUB").context("NGIT_OWNER_NPUB must be set")?,
-            relay_name: env::var("NGIT_RELAY_NAME")
-                .unwrap_or_else(|_| "ngit-grasp relay".to_string()),
-            relay_description: env::var("NGIT_RELAY_DESCRIPTION")
-                .unwrap_or_else(|_| "A GRASP-compliant Nostr relay for Git".to_string()),
-            git_data_path: env::var("NGIT_GIT_DATA_PATH")
-                .unwrap_or_else(|_| "./data/git".to_string()),
-            relay_data_path: env::var("NGIT_RELAY_DATA_PATH")
-                .unwrap_or_else(|_| "./data/relay".to_string()),
-            bind_address: env::var("NGIT_BIND_ADDRESS")
-                .unwrap_or_else(|_| "127.0.0.1:8080".to_string()),
-            database_backend,
-        })
+        // Parse CLI args (clap automatically handles env var fallback)
+        let config = Self::parse();
+
+        Ok(config)
+    }
+
+    /// Get relay name (defaults to "${domain} grasp relay" if not set)
+    pub fn relay_name(&self) -> String {
+        self.relay_name_override
+            .clone()
+            .unwrap_or_else(|| format!("{} grasp relay", self.domain))
+    }
+
+    /// Get effective git data path
+    /// Returns a temp directory when using memory backend, otherwise the configured path
+    pub fn effective_git_data_path(&self) -> String {
+        if self.database_backend == DatabaseBackend::Memory {
+            std::env::temp_dir()
+                .join("ngit-grasp-git")
+                .to_string_lossy()
+                .into_owned()
+        } else {
+            self.git_data_path.clone()
+        }
+    }
+
+    /// Create config for testing
+    #[cfg(test)]
+    pub fn for_testing() -> Self {
+        Self {
+            domain: "localhost:8080".to_string(),
+            owner_npub: Some("npub1test".to_string()),
+            relay_name_override: Some("test relay".to_string()),
+            relay_description: "test description".to_string(),
+            git_data_path: "./test_data/git".to_string(),
+            relay_data_path: "./test_data/relay".to_string(),
+            bind_address: "127.0.0.1:8080".to_string(),
+            database_backend: DatabaseBackend::Memory,
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_default_values() {
+        let config = Config::for_testing();
+        assert_eq!(config.domain, "localhost:8080");
+        assert_eq!(config.bind_address, "127.0.0.1:8080");
+        // for_testing() uses Memory, but the actual default is Lmdb
+        assert_eq!(config.database_backend, DatabaseBackend::Memory);
+    }
+
+    #[test]
+    fn test_lmdb_is_default() {
+        // Verify the actual default via the enum's Default trait
+        assert_eq!(DatabaseBackend::default(), DatabaseBackend::Lmdb);
+    }
+
+    #[test]
+    fn test_memory_backend_uses_temp_dir() {
+        let config = Config {
+            database_backend: DatabaseBackend::Memory,
+            ..Config::for_testing()
+        };
+        let git_path = config.effective_git_data_path();
+        assert!(git_path.contains("ngit-grasp-git"));
+    }
+
+    #[test]
+    fn test_lmdb_backend_uses_configured_path() {
+        let config = Config {
+            database_backend: DatabaseBackend::Lmdb,
+            git_data_path: "./my/git/path".to_string(),
+            relay_data_path: "./my/relay/path".to_string(),
+            ..Config::for_testing()
+        };
+        assert_eq!(config.effective_git_data_path(), "./my/git/path");
+    }
+
+    #[test]
+    fn test_database_backend_display() {
+        assert_eq!(DatabaseBackend::Memory.to_string(), "memory");
+        assert_eq!(DatabaseBackend::NostrDb.to_string(), "nostrdb");
+        assert_eq!(DatabaseBackend::Lmdb.to_string(), "lmdb");
+    }
+
+    #[test]
+    fn test_relay_name_default() {
+        let config = Config {
+            domain: "example.com".to_string(),
+            relay_name_override: None,
+            ..Config::for_testing()
+        };
+        assert_eq!(config.relay_name(), "example.com grasp relay");
+    }
+
+    #[test]
+    fn test_relay_name_override() {
+        let config = Config {
+            domain: "example.com".to_string(),
+            relay_name_override: Some("My Custom Relay".to_string()),
+            ..Config::for_testing()
+        };
+        assert_eq!(config.relay_name(), "My Custom Relay");
+    }
+
+    #[test]
+    fn test_owner_npub_optional() {
+        let config = Config {
+            owner_npub: None,
+            ..Config::for_testing()
+        };
+        assert!(config.owner_npub.is_none());
     }
 }
diff --git a/src/http/landing.rs b/src/http/landing.rs
index b978851..f9fca5b 100644
--- a/src/http/landing.rs
+++ b/src/http/landing.rs
@@ -282,7 +282,7 @@ pub fn get_html(config: &Config) -> String {
     format!(
         include_str!("../../templates/landing.html"),
         base_css = get_base_css(),
-        relay_name = config.relay_name,
+        relay_name = config.relay_name(),
         relay_description = config.relay_description,
         version = get_version(),
         curation = curation,
@@ -357,7 +357,7 @@ pub fn get_generic_404_html(config: &Config, path: &str) -> String {
 </body>
 </html>"##,
         base_css = get_base_css(),
-        relay_name = config.relay_name,
+        relay_name = config.relay_name(),
         path = path,
         version = get_version(),
         footer_script = get_footer_script(),
@@ -456,7 +456,7 @@ pub fn get_404_html(config: &Config, npub: &str, identifier: &str) -> String {
 </body>
 </html>"##,
         base_css = get_base_css(),
-        relay_name = config.relay_name,
+        relay_name = config.relay_name(),
         npub = npub,
         identifier = identifier,
         version = get_version(),
@@ -598,7 +598,7 @@ pub fn get_repo_html(config: &Config, npub: &str, identifier: &str) -> String {
 </body>
 </html>"##,
         base_css = get_base_css(),
-        relay_name = config.relay_name,
+        relay_name = config.relay_name(),
         npub = npub,
         identifier = identifier,
         version = get_version(),
diff --git a/src/http/mod.rs b/src/http/mod.rs
index 4665281..8b1f687 100644
--- a/src/http/mod.rs
+++ b/src/http/mod.rs
@@ -118,7 +118,7 @@ impl Service<Request<Incoming>> for HttpService {
         let path = req.uri().path().to_string();
         let query = req.uri().query().map(|s| s.to_string());
         let method = req.method().clone();
-        let git_data_path = self.config.git_data_path.clone();
+        let git_data_path = self.config.effective_git_data_path();
         let database = self.database.clone();
 
         // Handle OPTIONS preflight requests (CORS)
@@ -427,7 +427,7 @@ pub async fn run_server(
     let bind_addr: SocketAddr = config.bind_address.parse()?;
 
     tracing::info!("Starting HTTP server on {}", bind_addr);
-    tracing::info!("Relay name: {}", config.relay_name);
+    tracing::info!("Relay name: {}", config.relay_name());
     tracing::info!("Domain: {}", config.domain);
 
     let listener = TcpListener::bind(&bind_addr).await?;
diff --git a/src/http/nip11.rs b/src/http/nip11.rs
index ecb9769..e6a1e46 100644
--- a/src/http/nip11.rs
+++ b/src/http/nip11.rs
@@ -57,9 +57,9 @@ impl RelayInformationDocument {
     /// Create NIP-11 relay information document from configuration
     pub fn from_config(config: &Config) -> Self {
         Self {
-            name: config.relay_name.clone(),
+            name: config.relay_name(),
             description: config.relay_description.clone(),
-            pubkey: Some(config.owner_npub.clone()),
+            pubkey: config.owner_npub.clone(),
             contact: None, // Could be added to config if needed
             supported_nips: vec![
                 1,  // NIP-01: Basic protocol flow
@@ -98,8 +98,8 @@ mod tests {
     fn test_relay_information_document_structure() {
         let config = Config {
             domain: "relay.example.com".to_string(),
-            owner_npub: "npub1test".to_string(),
-            relay_name: "Test Relay".to_string(),
+            owner_npub: Some("npub1test".to_string()),
+            relay_name_override: Some("Test Relay".to_string()),
             relay_description: "A test relay".to_string(),
             git_data_path: "./data/git".to_string(),
             relay_data_path: "./data/relay".to_string(),
@@ -128,8 +128,8 @@ mod tests {
     fn test_relay_information_document_json() {
         let config = Config {
             domain: "relay.example.com".to_string(),
-            owner_npub: "npub1test".to_string(),
-            relay_name: "Test Relay".to_string(),
+            owner_npub: Some("npub1test".to_string()),
+            relay_name_override: Some("Test Relay".to_string()),
             relay_description: "A test relay".to_string(),
             git_data_path: "./data/git".to_string(),
             relay_data_path: "./data/relay".to_string(),
diff --git a/src/main.rs b/src/main.rs
index 1f18ab2..f80e920 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -2,7 +2,10 @@ use anyhow::Result;
 use tracing::{info, Level};
 use tracing_subscriber::FmtSubscriber;
 
-use ngit_grasp::{config::Config, http, nostr};
+use ngit_grasp::{
+    config::{Config, DatabaseBackend},
+    http, nostr,
+};
 
 #[tokio::main]
 async fn main() -> Result<()> {
@@ -14,10 +17,17 @@ async fn main() -> Result<()> {
 
     info!("Starting ngit-grasp with nostr-relay-builder...");
 
-    // Load configuration
-    let config = Config::from_env()?;
+    // Load configuration (priority: CLI flags > env vars > .env file > defaults)
+    let config = Config::load()?;
+
     info!("Configuration loaded: {}", config.bind_address);
-    info!("Git data directory: {}", config.git_data_path);
+    info!("Domain: {}", config.domain);
+    info!("Relay name: {}", config.relay_name());
+    info!("Git data directory: {}", config.effective_git_data_path());
+    if config.database_backend != DatabaseBackend::Memory {
+        info!("Relay data directory: {}", config.relay_data_path);
+    }
+    info!("Database backend: {}", config.database_backend);
 
     // Create Nostr relay with NIP-34 validation
     // Returns both the relay and database for direct queries in handlers
diff --git a/src/nostr/builder.rs b/src/nostr/builder.rs
index eabb38f..904cba4 100644
--- a/src/nostr/builder.rs
+++ b/src/nostr/builder.rs
@@ -1203,22 +1203,31 @@ pub fn create_relay(config: &Config) -> Result<RelayWithDatabase> {
             tracing::info!("Using LMDB backend at: {}", db_path.display());
             // Ensure the database directory exists
             std::fs::create_dir_all(db_path).map_err(|e| {
-                anyhow::anyhow!("Failed to create LMDB directory {}: {}", db_path.display(), e)
+                anyhow::anyhow!(
+                    "Failed to create LMDB directory {}: {}",
+                    db_path.display(),
+                    e
+                )
             })?;
             Arc::new(NostrLMDB::open(db_path).map_err(|e| {
-                anyhow::anyhow!("Failed to open LMDB database at {}: {}", db_path.display(), e)
+                anyhow::anyhow!(
+                    "Failed to open LMDB database at {}: {}",
+                    db_path.display(),
+                    e
+                )
             })?)
         }
     };
 
     // Build relay with GRASP-01 validation
     // Clone Arc for the write policy so both relay and policy can access the database
+    let git_data_path = config.effective_git_data_path();
     let builder = RelayBuilder::default()
         .database(database.clone())
         .write_policy(Nip34WritePolicy::new(
             &config.domain,
             database.clone(),
-            &config.git_data_path,
+            &git_data_path,
         ));
 
     tracing::info!(