Fix config regressions and ownership import errors

This commit is contained in:
Rasyidan Akbar F. 2026-03-17 21:23:33 +07:00
commit 65b95aa7b8
3 changed files with 124 additions and 29 deletions

View file

@ -657,6 +657,10 @@ fn handle_import(args: &ImportArgs, config: &IdxConfig) -> Result<(), IdxError>
for symbol in &clean {
eprintln!(" - {symbol}");
}
return Err(IdxError::Unsupported(
"--fetch-bing import is not implemented yet".to_string(),
));
}
}

View file

@ -47,6 +47,14 @@ impl ProviderKind {
}
impl HistoryProviderKind {
pub fn as_str(self) -> &'static str {
match self {
Self::Auto => "auto",
Self::Yahoo => "yahoo",
Self::Msn => "msn",
}
}
fn parse(value: &str) -> Result<Self, IdxError> {
if value.eq_ignore_ascii_case("auto") {
Ok(Self::Auto)
@ -250,17 +258,22 @@ const KNOWN_CONFIG_KEYS: &[&str] = &[
"general.color",
"cache.quote_ttl",
"cache.fundamental_ttl",
"ownership.db_path",
];
/// Validates a config key and value before writing
fn validate_config_key_value(key: &str, value: &str) -> Result<(), IdxError> {
normalize_config_value(key, value).map(|_| ())
}
fn normalize_config_value(key: &str, value: &str) -> Result<toml::Value, IdxError> {
match key {
"general.provider" => {
ProviderKind::parse(value)?;
}
"general.history_provider" => {
HistoryProviderKind::parse(value)?;
}
"general.provider" => Ok(toml::Value::String(
ProviderKind::parse(value)?.as_str().to_string(),
)),
"general.history_provider" => Ok(toml::Value::String(
HistoryProviderKind::parse(value)?.as_str().to_string(),
)),
"general.output" => {
if !value.eq_ignore_ascii_case("table") && !value.eq_ignore_ascii_case("json") {
return Err(IdxError::InvalidInput(format!(
@ -268,6 +281,7 @@ fn validate_config_key_value(key: &str, value: &str) -> Result<(), IdxError> {
value
)));
}
Ok(toml::Value::String(value.to_ascii_lowercase()))
}
"cache.quote_ttl" | "cache.fundamental_ttl" => {
let parsed: i64 = value.parse().map_err(|_| {
@ -282,9 +296,15 @@ fn validate_config_key_value(key: &str, value: &str) -> Result<(), IdxError> {
value
)));
}
Ok(toml::Value::Integer(parsed))
}
"general.exchange" => {
// Exchange is free-form (e.g. JK, US, etc.)
Ok(toml::Value::String(value.to_string()))
}
"ownership.db_path" => {
// Ownership DB path is free-form and may be absolute or relative.
Ok(toml::Value::String(value.to_string()))
}
"general.color" => {
if !value.eq_ignore_ascii_case("true") && !value.eq_ignore_ascii_case("false") {
@ -293,21 +313,19 @@ fn validate_config_key_value(key: &str, value: &str) -> Result<(), IdxError> {
value
)));
}
Ok(toml::Value::Boolean(value.eq_ignore_ascii_case("true")))
}
_ => {
return Err(IdxError::InvalidInput(format!(
_ => Err(IdxError::InvalidInput(format!(
"unknown config key '{}'. Valid keys: {}",
key,
KNOWN_CONFIG_KEYS.join(", ")
)));
))),
}
}
Ok(())
}
pub fn set_config_value(key: &str, value: &str) -> Result<(), IdxError> {
// Validate the key and value before writing
validate_config_key_value(key, value)?;
let normalized = normalize_config_value(key, value)?;
let path = ensure_default_config()?;
let raw = fs::read_to_string(&path).map_err(|e| IdxError::Io(e.to_string()))?;
@ -321,7 +339,7 @@ pub fn set_config_value(key: &str, value: &str) -> Result<(), IdxError> {
while let Some(part) = parts.next() {
if parts.peek().is_none() {
current.insert(part.to_string(), parse_toml_value(value));
current.insert(part.to_string(), normalized.clone());
} else {
let entry = current
.entry(part.to_string())
@ -342,19 +360,6 @@ pub fn set_config_value(key: &str, value: &str) -> Result<(), IdxError> {
.map_err(|e| IdxError::Io(e.to_string()))
}
fn parse_toml_value(value: &str) -> toml::Value {
if let Ok(v) = value.parse::<i64>() {
return toml::Value::Integer(v);
}
if let Ok(v) = value.parse::<f64>() {
return toml::Value::Float(v);
}
if let Ok(v) = value.parse::<bool>() {
return toml::Value::Boolean(v);
}
toml::Value::String(value.to_string())
}
#[cfg(test)]
mod tests {
use super::IdxConfig;
@ -430,6 +435,14 @@ mod tests {
assert!(super::validate_config_key_value("general.provider", "Msn").is_ok());
}
#[test]
fn normalize_provider_lowercases_value() {
assert_eq!(
super::normalize_config_value("general.provider", "Msn").unwrap(),
toml::Value::String("msn".to_string())
);
}
#[test]
fn validate_rejects_invalid_history_provider() {
let result = super::validate_config_key_value("general.history_provider", "bogus");
@ -457,6 +470,14 @@ mod tests {
assert!(super::validate_config_key_value("general.history_provider", "msn").is_ok());
}
#[test]
fn normalize_history_provider_lowercases_value() {
assert_eq!(
super::normalize_config_value("general.history_provider", "Auto").unwrap(),
toml::Value::String("auto".to_string())
);
}
#[test]
fn validate_rejects_invalid_output() {
let result = super::validate_config_key_value("general.output", "bogus");
@ -485,6 +506,14 @@ mod tests {
assert!(super::validate_config_key_value("general.output", "Json").is_ok());
}
#[test]
fn normalize_output_lowercases_value() {
assert_eq!(
super::normalize_config_value("general.output", "Json").unwrap(),
toml::Value::String("json".to_string())
);
}
#[test]
fn validate_rejects_negative_ttl() {
let result = super::validate_config_key_value("cache.quote_ttl", "-1");
@ -526,6 +555,12 @@ mod tests {
assert!(super::validate_config_key_value("general.exchange", "US").is_ok());
}
#[test]
fn validate_accepts_ownership_db_path() {
assert!(super::validate_config_key_value("ownership.db_path", "/tmp/ownership.db").is_ok());
assert!(super::validate_config_key_value("ownership.db_path", "data/ownership.db").is_ok());
}
#[test]
fn validate_accepts_valid_color() {
assert!(super::validate_config_key_value("general.color", "true").is_ok());
@ -534,6 +569,14 @@ mod tests {
assert!(super::validate_config_key_value("general.color", "False").is_ok());
}
#[test]
fn normalize_color_writes_boolean() {
assert_eq!(
super::normalize_config_value("general.color", "True").unwrap(),
toml::Value::Boolean(true)
);
}
#[test]
fn validate_rejects_invalid_color() {
let result = super::validate_config_key_value("general.color", "foo");

View file

@ -301,6 +301,54 @@ fn config_set_and_get_provider_round_trip() {
.stdout(predicate::str::contains("msn"));
}
#[test]
fn config_set_and_get_ownership_db_path_round_trip() {
let root = test_env_dir("config-ownership-db-path");
bin_with_root(&root)
.args(["config", "set", "ownership.db_path", "/tmp/ownership.db"])
.assert()
.success();
bin_with_root(&root)
.args(["config", "get", "ownership.db_path"])
.assert()
.success()
.stdout(predicate::str::contains("/tmp/ownership.db"));
}
#[test]
fn config_set_mixed_case_provider_does_not_break_future_loads() {
let root = test_env_dir("config-mixed-case-provider");
bin_with_root(&root)
.args(["config", "init"])
.assert()
.success();
bin_with_root(&root)
.args(["config", "set", "general.provider", "Msn"])
.assert()
.success();
bin_with_root(&root)
.args(["version"])
.assert()
.success()
.stdout(predicate::str::contains(env!("CARGO_PKG_VERSION")));
}
#[test]
fn ownership_import_fetch_bing_reports_unsupported() {
test_bin("ownership-fetch-bing-unsupported")
.args(["ownership", "import", "--fetch-bing", "BBCA"])
.assert()
.failure()
.stderr(predicate::str::contains(
"--fetch-bing import is not implemented yet",
));
}
#[test]
fn technical_serves_stale_cache_on_provider_failure_with_warning() {
let root = test_env_dir("technical-stale");