diff --git a/crates/adapters/image-converter/src/config.rs b/crates/adapters/image-converter/src/config.rs index 4745e970..2bf3934a 100644 --- a/crates/adapters/image-converter/src/config.rs +++ b/crates/adapters/image-converter/src/config.rs @@ -19,24 +19,19 @@ pub struct ConversionConfig { impl ConversionConfig { pub fn from_config(config: &::config::ImageConversionConfig) -> anyhow::Result> { - Self::from_vars(config.enabled.then_some("true"), config.format.as_deref()) - } - - fn from_vars(enabled: Option<&str>, format: Option<&str>) -> anyhow::Result> { - if enabled != Some("true") { + if !config.enabled { return Ok(None); } - let format_str = format.ok_or_else(|| { - anyhow::anyhow!("IMAGE_CONVERSION_FORMAT required when IMAGE_CONVERSION_ENABLED=true") - })?; - - let format = match format_str { - "avif" => Format::Avif, - "webp" => Format::Webp, - other => anyhow::bail!( + let format = match config.format.as_deref() { + Some("avif") => Format::Avif, + Some("webp") => Format::Webp, + Some(other) => anyhow::bail!( "Unknown IMAGE_CONVERSION_FORMAT: {other:?}. Valid values: avif, webp" ), + None => { + anyhow::bail!("IMAGE_CONVERSION_FORMAT required when IMAGE_CONVERSION_ENABLED=true") + } }; Ok(Some(Self { format })) diff --git a/crates/adapters/image-converter/src/tests/config.rs b/crates/adapters/image-converter/src/tests/config.rs index 9747b481..63a302c3 100644 --- a/crates/adapters/image-converter/src/tests/config.rs +++ b/crates/adapters/image-converter/src/tests/config.rs @@ -1,37 +1,46 @@ use super::*; +fn section(enabled: bool, format: Option<&str>) -> ::config::ImageConversionConfig { + ::config::ImageConversionConfig { + enabled, + format: format.map(str::to_owned), + } +} + +fn convert(enabled: bool, format: Option<&str>) -> anyhow::Result> { + ConversionConfig::from_config(§ion(enabled, format)) +} + #[test] fn disabled_by_default() { - assert!(ConversionConfig::from_vars(None, None).unwrap().is_none()); - assert!(ConversionConfig::from_vars(Some("false"), None) - .unwrap() - .is_none()); + assert!( + ConversionConfig::from_config(&::config::ImageConversionConfig::default()) + .unwrap() + .is_none() + ); + assert!(convert(false, Some("webp")).unwrap().is_none()); } #[test] fn enabled_avif() { - let cfg = ConversionConfig::from_vars(Some("true"), Some("avif")) - .unwrap() - .unwrap(); + let cfg = convert(true, Some("avif")).unwrap().unwrap(); assert_eq!(cfg.format, Format::Avif); } #[test] fn enabled_webp() { - let cfg = ConversionConfig::from_vars(Some("true"), Some("webp")) - .unwrap() - .unwrap(); + let cfg = convert(true, Some("webp")).unwrap().unwrap(); assert_eq!(cfg.format, Format::Webp); } #[test] fn unknown_format_is_error() { - assert!(ConversionConfig::from_vars(Some("true"), Some("gif")).is_err()); + assert!(convert(true, Some("gif")).is_err()); } #[test] fn missing_format_when_enabled_is_error() { - assert!(ConversionConfig::from_vars(Some("true"), None).is_err()); + assert!(convert(true, None).is_err()); } #[test] @@ -43,15 +52,3 @@ fn avif_extension() { fn webp_extension() { assert_eq!(Format::Webp.extension(), ".webp"); } - -#[test] -fn from_config_follows_section() { - let disabled = ::config::ImageConversionConfig::default(); - assert!(ConversionConfig::from_config(&disabled).unwrap().is_none()); - let enabled = ::config::ImageConversionConfig { - enabled: true, - format: Some("webp".into()), - }; - let cfg = ConversionConfig::from_config(&enabled).unwrap().unwrap(); - assert_eq!(cfg.format, Format::Webp); -} diff --git a/crates/adapters/nats/src/config.rs b/crates/adapters/nats/src/config.rs index 31d5b393..d7ed4381 100644 --- a/crates/adapters/nats/src/config.rs +++ b/crates/adapters/nats/src/config.rs @@ -15,40 +15,23 @@ pub struct NatsConfig { impl NatsConfig { pub fn from_config(config: &::config::NatsConfig) -> anyhow::Result { - Self::from_vars( - config.url.as_deref(), - Some(&config.mode), - Some(&config.subject_prefix), - Some(&config.stream_name), - Some(&config.consumer_name), - ) - } + let url = config + .url + .clone() + .ok_or_else(|| anyhow::anyhow!("NATS_URL is not set"))?; - pub(crate) fn from_vars( - url: Option<&str>, - mode: Option<&str>, - subject_prefix: Option<&str>, - stream_name: Option<&str>, - consumer_name: Option<&str>, - ) -> anyhow::Result { - let url = url.ok_or_else(|| anyhow::anyhow!("NATS_URL is not set"))?; - - let mode = match mode.unwrap_or("jetstream") { + let mode = match config.mode.as_str() { "core" => NatsMode::Core, "jetstream" => NatsMode::JetStream, other => anyhow::bail!("unknown NATS_MODE: {other}"), }; - let subject_prefix = subject_prefix.unwrap_or("movies-diary.events").to_string(); - let stream_name = stream_name.unwrap_or("MOVIES_DIARY_EVENTS").to_string(); - let consumer_name = consumer_name.unwrap_or("worker").to_string(); - Ok(Self { - url: url.to_string(), + url, mode, - subject_prefix, - stream_name, - consumer_name, + subject_prefix: config.subject_prefix.clone(), + stream_name: config.stream_name.clone(), + consumer_name: config.consumer_name.clone(), }) } } diff --git a/crates/adapters/nats/src/tests/config.rs b/crates/adapters/nats/src/tests/config.rs index d81453e8..8d749d06 100644 --- a/crates/adapters/nats/src/tests/config.rs +++ b/crates/adapters/nats/src/tests/config.rs @@ -1,13 +1,25 @@ use super::*; +fn section(url: Option<&str>, mode: &str) -> ::config::NatsConfig { + ::config::NatsConfig { + url: url.map(str::to_owned), + mode: mode.into(), + ..Default::default() + } +} + #[test] fn errors_without_nats_url() { - assert!(NatsConfig::from_vars(None, None, None, None, None).is_err()); + assert!(NatsConfig::from_config(&::config::NatsConfig::default()).is_err()); } #[test] fn defaults_with_only_url() { - let cfg = NatsConfig::from_vars(Some("nats://localhost:4222"), None, None, None, None).unwrap(); + let cfg = NatsConfig::from_config(&::config::NatsConfig { + url: Some("nats://localhost:4222".into()), + ..Default::default() + }) + .unwrap(); assert_eq!(cfg.url, "nats://localhost:4222"); assert_eq!(cfg.mode, NatsMode::JetStream); assert_eq!(cfg.subject_prefix, "movies-diary.events"); @@ -17,31 +29,12 @@ fn defaults_with_only_url() { #[test] fn core_mode_parsed() { - let cfg = - NatsConfig::from_vars(Some("nats://test:4222"), Some("core"), None, None, None).unwrap(); + let cfg = NatsConfig::from_config(§ion(Some("nats://test:4222"), "core")).unwrap(); + assert_eq!(cfg.url, "nats://test:4222"); assert_eq!(cfg.mode, NatsMode::Core); } #[test] fn invalid_mode_errors() { - assert!( - NatsConfig::from_vars(Some("nats://test:4222"), Some("kafka"), None, None, None).is_err() - ); -} - -#[test] -fn from_config_requires_url() { - assert!(NatsConfig::from_config(&::config::NatsConfig::default()).is_err()); -} - -#[test] -fn from_config_uses_section_values() { - let section = ::config::NatsConfig { - url: Some("nats://test:4222".into()), - mode: "core".into(), - ..Default::default() - }; - let cfg = NatsConfig::from_config(§ion).unwrap(); - assert_eq!(cfg.url, "nats://test:4222"); - assert_eq!(cfg.mode, NatsMode::Core); + assert!(NatsConfig::from_config(§ion(Some("nats://test:4222"), "kafka")).is_err()); } diff --git a/crates/config/src/loader.rs b/crates/config/src/loader.rs index 250404f6..5d5e6b83 100644 --- a/crates/config/src/loader.rs +++ b/crates/config/src/loader.rs @@ -59,7 +59,8 @@ const LEGACY_ENV: &[(&str, &str)] = &[ ]; /// Reads the legacy variables as plain strings; typing is left to the config -/// structs so that e.g. `JWT_SECRET=12345` stays a string. +/// structs so that e.g. `JWT_SECRET=12345` stays a string. Empty or +/// whitespace-only values are skipped as unset. struct LegacyEnv; impl Provider for LegacyEnv { @@ -76,6 +77,11 @@ impl Provider for LegacyEnv { let value = raw .into_string() .map_err(|_| figment::Error::from(format!("{variable} is not valid unicode")))?; + // An empty value (`PORT=` in a .env template) means "not set": the + // default applies, or the optional field stays `None`. + if value.trim().is_empty() { + continue; + } let (section, field) = key.split_once('.').expect("legacy keys are section.field"); let entry = sections .entry(section.to_owned()) diff --git a/crates/config/tests/loader_test.rs b/crates/config/tests/loader_test.rs index 58f8d11f..77e80b54 100644 --- a/crates/config/tests/loader_test.rs +++ b/crates/config/tests/loader_test.rs @@ -345,6 +345,27 @@ fn malformed_numeric_env_value_fails_naming_the_variable() { }); } +#[test] +fn empty_env_value_falls_back_to_the_default() { + Jail::expect_with(|jail| { + jail_with( + jail, + &[ + ("RATE_LIMIT", ""), + ("PORT", " "), + ("TMDB_API_KEY", ""), + ("NATS_URL", ""), + ], + ); + let c = load().expect("empty values are unset, not errors"); + assert_eq!(c.server.rate_limit, 60); + assert_eq!(c.server.port, 3000); + assert!(c.metadata.tmdb_api_key.is_none()); + assert!(c.nats.url.is_none()); + Ok(()) + }); +} + #[test] fn allow_registration_accepts_true_and_1_only() { for (value, expected) in [