From 8bc645b006080b860e40c0ff55b485125dc6157d Mon Sep 17 00:00:00 2001 From: rtkay123 Date: Thu, 2 Apr 2026 14:27:45 +0200 Subject: test(api): schema --- lib/warden-core/src/config/cli/database.rs | 192 ++++++++++++++++++------ lib/warden-core/src/config/cli/mod.rs | 10 ++ lib/warden-core/src/config/mod.rs | 231 +++++++++++++++++++++++------ 3 files changed, 345 insertions(+), 88 deletions(-) (limited to 'lib/warden-core/src/config') diff --git a/lib/warden-core/src/config/cli/database.rs b/lib/warden-core/src/config/cli/database.rs index 70bf600..90032a2 100644 --- a/lib/warden-core/src/config/cli/database.rs +++ b/lib/warden-core/src/config/cli/database.rs @@ -22,7 +22,7 @@ pub struct Database { pub database_password: Option, /// Database host - #[arg(long, env = "DB_HOST", default_value = "localhost")] + #[arg(long, env = "DB_HOST")] #[serde(rename = "host")] pub database_host: Option, @@ -37,7 +37,7 @@ pub struct Database { pub database_name: Option, /// Database pool size - #[arg(long, env = "DATABASE_POOL_SIZE", default_value = "10")] + #[arg(long, env = "DATABASE_POOL_SIZE")] #[serde(rename = "pool-size")] pub database_pool_size: Option, } @@ -58,56 +58,67 @@ impl Default for Database { impl Database { pub fn merge(cli: &Self, file: &Self) -> Result { - let url = cli.database_url.clone().or(file.database_url.clone()); - let pool_size = cli .database_pool_size .or(file.database_pool_size) .unwrap_or(10); - let final_url = match url { - Some(u) => u, - None => { - let host = cli - .database_host - .clone() - .or(file.database_host.clone()) - .unwrap_or_else(|| "localhost".to_string()); - - let mut u = Url::parse(&format!("postgresql://{}", host))?; - - let user = cli - .database_username - .as_ref() - .or(file.database_username.as_ref()); - let pass = cli - .database_password - .as_ref() - .or(file.database_password.as_ref()); - let port = cli.database_port.or(file.database_port); - let name = cli.database_name.as_ref().or(file.database_name.as_ref()); - - if let Some(user) = user { - u.set_username(user).ok(); - } - if let Some(pass) = pass { - u.set_password(Some(pass)).ok(); - } - if let Some(port) = port { - u.set_port(Some(port)).ok(); - } - if let Some(name) = name { - u.set_path(name); - } - - u - } - }; + if let Some(url) = cli + .database_url + .clone() + .or_else(|| file.database_url.clone()) + { + return Ok(Self { + database_url: Some(url), + database_pool_size: Some(pool_size), + ..Default::default() + }); + } + + let host = cli + .database_host + .as_deref() + .or(file.database_host.as_deref()) + .unwrap_or("localhost"); + + let mut u = Url::parse(&format!("postgresql://{}", host))?; + + let user = cli + .database_username + .as_deref() + .or(file.database_username.as_deref()); + let pass = cli + .database_password + .as_deref() + .or(file.database_password.as_deref()); + let port = cli.database_port.or(file.database_port); + let name = cli + .database_name + .as_deref() + .or(file.database_name.as_deref()); + + if let Some(user) = user { + u.set_username(user).ok(); + } + if let Some(pass) = pass { + u.set_password(Some(pass)).ok(); + } + if let Some(port) = port { + u.set_port(Some(port)).ok(); + } + if let Some(name) = name { + u.set_path(name); + } Ok(Self { - database_url: Some(final_url), + database_url: Some(u), database_pool_size: Some(pool_size), - ..cli.clone() + // Carry over the other fields for record-keeping + database_host: Some(host.to_string()), + database_username: user.map(String::from), + database_password: pass.map(String::from), + database_port: port, + database_name: name.map(String::from), }) } @@ -134,3 +145,96 @@ impl Database { Ok(url) } } + +#[cfg(test)] +mod tests { + use super::*; + use url::Url; + + /// Helper to create a "naked" Database struct with all Nones + /// Useful for testing merge logic without Default values interfering + fn empty_db() -> Database { + Database { + database_url: None, + database_username: None, + database_password: None, + database_host: None, + database_port: None, + database_name: None, + database_pool_size: None, + } + } + + #[test] + fn test_get_url_from_components() { + let db = Database { + database_host: Some("127.0.0.1".to_string()), + database_username: Some("admin".to_string()), + database_password: Some("secret".to_string()), + database_port: Some(5432), + database_name: Some("testdb".to_string()), + ..empty_db() + }; + + let url = db.get_url().expect("Should parse URL"); + // Note: get_url uses "postgres://" scheme + assert_eq!( + url.as_str(), + "postgres://admin:secret@127.0.0.1:5432/testdb" + ); + } + + #[test] + fn test_merge_cli_overrides_file() { + let mut file_config = empty_db(); + file_config.database_host = Some("file-host".to_string()); + file_config.database_port = Some(1111); + + let mut cli_config = empty_db(); + cli_config.database_host = Some("cli-host".to_string()); + // database_port is None in CLI + + let merged = Database::merge(&cli_config, &file_config).expect("Merge failed"); + + let url = merged.database_url.unwrap(); + // CLI host should win + assert_eq!(url.host_str(), Some("cli-host")); + // File port should win because CLI was None + assert_eq!(url.port(), Some(1111)); + } + + #[test] + fn test_merge_url_override_wins_all() { + let mut file_config = empty_db(); + file_config.database_host = Some("local-host".to_string()); + + let mut cli_config = empty_db(); + let expected_url = "postgresql://remote-host:9999/prod"; + cli_config.database_url = Some(Url::parse(expected_url).unwrap()); + + let merged = Database::merge(&cli_config, &file_config).expect("Merge failed"); + + assert_eq!(merged.database_url.unwrap().as_str(), expected_url); + } + + #[test] + fn test_merge_pool_size_logic() { + let mut file_config = empty_db(); + file_config.database_pool_size = Some(50); + + let cli_config = empty_db(); // pool_size is None + + let merged = Database::merge(&cli_config, &file_config).expect("Merge failed"); + + // Should take file value if CLI is None + assert_eq!(merged.database_pool_size, Some(50)); + } + + #[test] + fn test_default_trait_implementation() { + let db = Database::default(); + assert_eq!(db.database_port, Some(5432)); + assert_eq!(db.database_username, Some("postgres".to_string())); + assert_eq!(db.database_host, Some("localhost".to_string())); + } +} diff --git a/lib/warden-core/src/config/cli/mod.rs b/lib/warden-core/src/config/cli/mod.rs index e0c5450..f4ffe05 100644 --- a/lib/warden-core/src/config/cli/mod.rs +++ b/lib/warden-core/src/config/cli/mod.rs @@ -57,6 +57,15 @@ pub struct Server { default_value = "5" )] pub timeout_secs: Option, + /// Pagination limit + #[arg( + long, + value_name = "PAGINATION_LIMIT", + env = "PAGINATION_LIMIT", + default_value = "50" + )] + #[arg(value_parser = clap::value_parser!(i64).range(1..))] + pub pagination_limit: Option, } impl Default for Server { @@ -70,6 +79,7 @@ impl Default for Server { )), log_dir: Some(std::env::temp_dir()), timeout_secs: Some(5), + pagination_limit: Some(50), } } } diff --git a/lib/warden-core/src/config/mod.rs b/lib/warden-core/src/config/mod.rs index 9d0c937..8be7205 100644 --- a/lib/warden-core/src/config/mod.rs +++ b/lib/warden-core/src/config/mod.rs @@ -13,6 +13,16 @@ use crate::WardenError; use crate::config::cli::CliEnvironment; use crate::config::cli::database::Database; +macro_rules! pick { + ($cli:expr, $file:expr, $name:expr, $missing:expr) => {{ + let val = $cli.clone().or($file.clone()); + if val.is_none() { + $missing.push($name); + } + val + }}; +} + #[derive(Deserialize, Default, Debug, ValueEnum, Clone, Copy)] #[serde(rename_all = "lowercase")] pub enum Environment { @@ -43,67 +53,65 @@ pub struct Server { pub log_level: EnvFilter, pub log_dir: PathBuf, pub timeout_secs: u64, + pub pagination_limit: i64, } impl Server { - fn merge(cli: &Cli, file: &Cli, missing: &mut Vec<&str>) -> Result { - let port = cli.server.port.or(file.server.port); - - if port.is_none() { - missing.push("server.port"); - } - - let timeout = cli.server.timeout_secs.or(file.server.timeout_secs); - - if timeout.is_none() { - missing.push("server.timeout"); - } - - let log_dir = cli.server.log_dir.clone().or(file.server.log_dir.clone()); - - if log_dir.is_none() { - missing.push("server.log_dir"); - } - - let log_level = cli - .server - .log_level - .as_ref() - .or(file.server.log_level.as_ref()) - .map(ToOwned::to_owned); - - if log_level.is_none() { - missing.push("server.log_level"); - } - - let environment = cli.server.environment.or(file.server.environment); - - if environment.is_none() { - missing.push("server.environment"); - } + pub fn merge(cli: &Cli, file: &Cli, missing: &mut Vec<&str>) -> Result { + let port = pick!(cli.server.port, file.server.port, "server.port", missing); + let timeout = pick!( + cli.server.timeout_secs, + file.server.timeout_secs, + "server.timeout", + missing + ); + let log_dir = pick!( + cli.server.log_dir.clone(), + file.server.log_dir.clone(), + "server.log_dir", + missing + ); + let env = pick!( + cli.server.environment, + file.server.environment, + "server.environment", + missing + ); + let raw_log_level = pick!( + cli.server.log_level.clone(), + file.server.log_level.clone(), + "server.log_level", + missing + ); + let pagination_limit = pick!( + cli.server.pagination_limit.clone(), + file.server.pagination_limit.clone(), + "server.pagination_limit", + missing + ); if !missing.is_empty() { - let err = missing + let err_msg = missing .iter() .map(|f| format!(" - {}", f)) .collect::>() .join("\n"); - return Err(WardenError::Config(err)); + return Err(WardenError::Config(format!( + "Missing required fields:\n{}", + err_msg + ))); } - let log_level = - tracing_subscriber::EnvFilter::try_from_default_env().unwrap_or_else(|_| { - // axum logs rejections from built-in extractors with the `axum::rejection` - // target, at `TRACE` level. `axum::rejection=trace` enables showing those events - log_level.unwrap().into() - }); + let log_level = tracing_subscriber::EnvFilter::try_from_default_env() + .unwrap_or_else(|_| raw_log_level.unwrap().into()); Ok(Self { port: port.unwrap(), - environment: environment.unwrap().into(), + environment: env.unwrap().into(), log_dir: log_dir.unwrap(), timeout_secs: timeout.unwrap(), log_level, + pagination_limit: pagination_limit.unwrap(), }) } } @@ -118,3 +126,138 @@ impl Configuration { Ok(Self { server, database }) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_merge_config() { + let mut cli = Cli::default(); + cli.server.port = Some(8080); + let timeout = 30; + cli.server.timeout_secs = Some(timeout); + + let file = Cli { + server: cli::Server { + environment: Some(CliEnvironment::Dev), + log_level: Some("info".into()), + log_dir: Some(PathBuf::from("/tmp")), + timeout_secs: Some(timeout), + ..Default::default() + }, + ..Default::default() + }; + + let result = Configuration::merge(&cli, &file); + assert!( + result.is_ok(), + "Merge should succeed when all fields are covered" + ); + + let server = result.unwrap(); + assert_eq!(server.server.port, 8080); + } + + #[test] + fn test_merge_all_fields_present_success() { + let mut cli = Cli::default(); + cli.server.port = Some(8080); + let timeout = 30; + cli.server.timeout_secs = Some(timeout); + + let file = Cli { + server: cli::Server { + environment: Some(CliEnvironment::Dev), + log_level: Some("info".into()), + log_dir: Some(PathBuf::from("/tmp")), + timeout_secs: Some(timeout), + ..Default::default() + }, + ..Default::default() + }; + let mut missing = vec![]; + + let result = Server::merge(&cli, &file, &mut missing); + assert!( + result.is_ok(), + "Merge should succeed when all fields are covered" + ); + + let server = result.unwrap(); + assert_eq!(server.port, 8080); + assert_eq!(server.timeout_secs, timeout); + } + + #[test] + fn test_merge_error_accumulation() { + let cli = Cli { + server: cli::Server { + port: None, + environment: None, + log_level: None, + log_dir: None, + timeout_secs: None, + pagination_limit: None, + }, + ..Default::default() + }; + + let mut missing = vec![]; + + let result = Server::merge(&cli, &cli, &mut missing); + dbg!(&result); + + match result { + Err(WardenError::Config(msg)) => { + assert!(msg.contains("server.port")); + assert!(msg.contains("server.environment")); + } + _ => panic!("Expected a Config error with multiple missing fields"), + } + } + + #[test] + fn test_cli_priority_over_file() { + let mut cli = Cli::default(); + cli.server.port = Some(9999); + + let file = Cli { + server: cli::Server { + port: Some(1111), // This should be ignored + environment: Some(CliEnvironment::Prod), + log_level: Some("error".into()), + log_dir: Some(PathBuf::from("/var/log")), + timeout_secs: Some(60), + ..Default::default() + }, + ..Default::default() + }; + + let mut missing = vec![]; + + let server = Server::merge(&cli, &file, &mut missing).expect("Merge failed"); + assert_eq!(server.port, 9999, "CLI port must override File port"); + } + + #[test] + fn test_env_filter_from_raw_string() { + let log_level = "warn"; + let cli = Cli { + server: cli::Server { + port: Some(80), + environment: Some(CliEnvironment::Production), + log_level: Some(log_level.to_string()), + log_dir: Some(PathBuf::from(".")), + timeout_secs: Some(5), + ..Default::default() + }, + ..Default::default() + }; + let file = Cli::default(); + let mut missing = vec![]; + + let server = Server::merge(&cli, &file, &mut missing).unwrap(); + assert_eq!(&server.log_level.to_string(), log_level) + } +} -- cgit v1.2.3