fix: config empty values unset, drop adapter from_vars

This commit is contained in:
2026-10-25 21:18:16 +01:00
parent 9bf9342fc0
commit a00a5dc655
6 changed files with 83 additions and 88 deletions

View File

@@ -19,24 +19,19 @@ pub struct ConversionConfig {
impl ConversionConfig { impl ConversionConfig {
pub fn from_config(config: &::config::ImageConversionConfig) -> anyhow::Result<Option<Self>> { pub fn from_config(config: &::config::ImageConversionConfig) -> anyhow::Result<Option<Self>> {
Self::from_vars(config.enabled.then_some("true"), config.format.as_deref()) if !config.enabled {
}
fn from_vars(enabled: Option<&str>, format: Option<&str>) -> anyhow::Result<Option<Self>> {
if enabled != Some("true") {
return Ok(None); return Ok(None);
} }
let format_str = format.ok_or_else(|| { let format = match config.format.as_deref() {
anyhow::anyhow!("IMAGE_CONVERSION_FORMAT required when IMAGE_CONVERSION_ENABLED=true") Some("avif") => Format::Avif,
})?; Some("webp") => Format::Webp,
Some(other) => anyhow::bail!(
let format = match format_str {
"avif" => Format::Avif,
"webp" => Format::Webp,
other => anyhow::bail!(
"Unknown IMAGE_CONVERSION_FORMAT: {other:?}. Valid values: avif, webp" "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 })) Ok(Some(Self { format }))

View File

@@ -1,37 +1,46 @@
use super::*; 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<Option<ConversionConfig>> {
ConversionConfig::from_config(&section(enabled, format))
}
#[test] #[test]
fn disabled_by_default() { fn disabled_by_default() {
assert!(ConversionConfig::from_vars(None, None).unwrap().is_none()); assert!(
assert!(ConversionConfig::from_vars(Some("false"), None) ConversionConfig::from_config(&::config::ImageConversionConfig::default())
.unwrap() .unwrap()
.is_none()); .is_none()
);
assert!(convert(false, Some("webp")).unwrap().is_none());
} }
#[test] #[test]
fn enabled_avif() { fn enabled_avif() {
let cfg = ConversionConfig::from_vars(Some("true"), Some("avif")) let cfg = convert(true, Some("avif")).unwrap().unwrap();
.unwrap()
.unwrap();
assert_eq!(cfg.format, Format::Avif); assert_eq!(cfg.format, Format::Avif);
} }
#[test] #[test]
fn enabled_webp() { fn enabled_webp() {
let cfg = ConversionConfig::from_vars(Some("true"), Some("webp")) let cfg = convert(true, Some("webp")).unwrap().unwrap();
.unwrap()
.unwrap();
assert_eq!(cfg.format, Format::Webp); assert_eq!(cfg.format, Format::Webp);
} }
#[test] #[test]
fn unknown_format_is_error() { fn unknown_format_is_error() {
assert!(ConversionConfig::from_vars(Some("true"), Some("gif")).is_err()); assert!(convert(true, Some("gif")).is_err());
} }
#[test] #[test]
fn missing_format_when_enabled_is_error() { fn missing_format_when_enabled_is_error() {
assert!(ConversionConfig::from_vars(Some("true"), None).is_err()); assert!(convert(true, None).is_err());
} }
#[test] #[test]
@@ -43,15 +52,3 @@ fn avif_extension() {
fn webp_extension() { fn webp_extension() {
assert_eq!(Format::Webp.extension(), ".webp"); 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);
}

View File

@@ -15,40 +15,23 @@ pub struct NatsConfig {
impl NatsConfig { impl NatsConfig {
pub fn from_config(config: &::config::NatsConfig) -> anyhow::Result<Self> { pub fn from_config(config: &::config::NatsConfig) -> anyhow::Result<Self> {
Self::from_vars( let url = config
config.url.as_deref(), .url
Some(&config.mode), .clone()
Some(&config.subject_prefix), .ok_or_else(|| anyhow::anyhow!("NATS_URL is not set"))?;
Some(&config.stream_name),
Some(&config.consumer_name),
)
}
pub(crate) fn from_vars( let mode = match config.mode.as_str() {
url: Option<&str>,
mode: Option<&str>,
subject_prefix: Option<&str>,
stream_name: Option<&str>,
consumer_name: Option<&str>,
) -> anyhow::Result<Self> {
let url = url.ok_or_else(|| anyhow::anyhow!("NATS_URL is not set"))?;
let mode = match mode.unwrap_or("jetstream") {
"core" => NatsMode::Core, "core" => NatsMode::Core,
"jetstream" => NatsMode::JetStream, "jetstream" => NatsMode::JetStream,
other => anyhow::bail!("unknown NATS_MODE: {other}"), 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 { Ok(Self {
url: url.to_string(), url,
mode, mode,
subject_prefix, subject_prefix: config.subject_prefix.clone(),
stream_name, stream_name: config.stream_name.clone(),
consumer_name, consumer_name: config.consumer_name.clone(),
}) })
} }
} }

View File

@@ -1,13 +1,25 @@
use super::*; 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] #[test]
fn errors_without_nats_url() { 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] #[test]
fn defaults_with_only_url() { 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.url, "nats://localhost:4222");
assert_eq!(cfg.mode, NatsMode::JetStream); assert_eq!(cfg.mode, NatsMode::JetStream);
assert_eq!(cfg.subject_prefix, "movies-diary.events"); assert_eq!(cfg.subject_prefix, "movies-diary.events");
@@ -17,31 +29,12 @@ fn defaults_with_only_url() {
#[test] #[test]
fn core_mode_parsed() { fn core_mode_parsed() {
let cfg = let cfg = NatsConfig::from_config(&section(Some("nats://test:4222"), "core")).unwrap();
NatsConfig::from_vars(Some("nats://test:4222"), Some("core"), None, None, None).unwrap(); assert_eq!(cfg.url, "nats://test:4222");
assert_eq!(cfg.mode, NatsMode::Core); assert_eq!(cfg.mode, NatsMode::Core);
} }
#[test] #[test]
fn invalid_mode_errors() { fn invalid_mode_errors() {
assert!( assert!(NatsConfig::from_config(&section(Some("nats://test:4222"), "kafka")).is_err());
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(&section).unwrap();
assert_eq!(cfg.url, "nats://test:4222");
assert_eq!(cfg.mode, NatsMode::Core);
} }

View File

@@ -59,7 +59,8 @@ const LEGACY_ENV: &[(&str, &str)] = &[
]; ];
/// Reads the legacy variables as plain strings; typing is left to the config /// 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; struct LegacyEnv;
impl Provider for LegacyEnv { impl Provider for LegacyEnv {
@@ -76,6 +77,11 @@ impl Provider for LegacyEnv {
let value = raw let value = raw
.into_string() .into_string()
.map_err(|_| figment::Error::from(format!("{variable} is not valid unicode")))?; .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 (section, field) = key.split_once('.').expect("legacy keys are section.field");
let entry = sections let entry = sections
.entry(section.to_owned()) .entry(section.to_owned())

View File

@@ -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] #[test]
fn allow_registration_accepts_true_and_1_only() { fn allow_registration_accepts_true_and_1_only() {
for (value, expected) in [ for (value, expected) in [