From 7a40326719d7dbdb76bbcf2b58132dee2bebcf62 Mon Sep 17 00:00:00 2001 From: John Doty Date: Thu, 15 Aug 2024 10:14:43 -0700 Subject: [PATCH] Re-work config code Add raw description as a possible config for a port, and update the documentation appropriately. --- doc/fwd.man.md | 13 +- src/client/config.rs | 408 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 348 insertions(+), 73 deletions(-) diff --git a/doc/fwd.man.md b/doc/fwd.man.md index 9f8f6da..b7e5df9 100644 --- a/doc/fwd.man.md +++ b/doc/fwd.man.md @@ -109,14 +109,15 @@ If **FILE** is **-**, this reads text from stdin instead. The following is an example of a *config.toml* file: ``` -auto=true # should `fwd` should enable identified ports +auto=true # should `fwd` should enable identified ports (default true) -[servers.foo] # Server-specific settings for foo -auto=true -ports=[1080, 1082] # ports that are always present +[servers.foo] # Server-specific settings for foo +auto=true # defaults to the global setting +ports=[1080, 1082] # ports that are always present -[servers.bar.ports] # `ports` can also be a table with port numbers as keys -1080=true # the values can be booleans (for enabled) +[servers.bar.ports] # `ports` can also be a table with port numbers as keys +1080=true # the values can be booleans (for enabled)... +1081="My program" # or strings (for descriptions). [servers.bar.ports.1082] # port values can also be tables enabled=true diff --git a/src/client/config.rs b/src/client/config.rs index 77d44e8..e3a3cc0 100644 --- a/src/client/config.rs +++ b/src/client/config.rs @@ -1,15 +1,17 @@ use anyhow::{bail, Result}; use std::collections::hash_map; use std::collections::HashMap; -use toml::Value; +use toml::value::{Table, Value}; #[derive(Debug, Clone)] +#[cfg_attr(test, derive(PartialEq, Eq))] pub struct PortConfig { pub enabled: bool, pub description: Option, } #[derive(Debug, Clone)] +#[cfg_attr(test, derive(PartialEq, Eq))] pub struct ServerConfig { auto: bool, ports: HashMap, @@ -45,6 +47,7 @@ impl ServerConfig { } #[derive(Debug)] +#[cfg_attr(test, derive(PartialEq, Eq))] pub struct Config { auto: bool, servers: HashMap, @@ -85,85 +88,101 @@ fn default() -> Config { Config { auto: true, servers: HashMap::new() } } -fn parse_config(value: &Value) -> Result { - match value { - Value::Table(table) => Ok({ - let auto = match table.get("auto") { - None => true, - Some(Value::Boolean(v)) => *v, - Some(v) => bail!("expected a true or false, got {:?}", v), - }; - Config { auto, servers: get_servers(table, auto)? } - }), - _ => bail!("top level must be a table"), +fn get_bool(table: &Table, key: &str, default: bool) -> Result { + match table.get(key) { + None => Ok(default), + Some(Value::Boolean(v)) => Ok(*v), + Some(v) => bail!("expected a true or false, got {v:?}"), } } -fn get_servers( - table: &toml::value::Table, +fn parse_config(value: &Value) -> Result { + let Value::Table(table) = value else { + bail!("top level must be a table") + }; + + let auto = get_bool(table, "auto", true)?; + let servers = match table.get("servers") { + None => &Table::new(), + Some(Value::Table(t)) => t, + Some(v) => bail!("Expected a table in the servers key, got {v:?}"), + }; + + Ok(Config { + auto, + servers: parse_servers(servers, auto)?, + }) +} + +fn parse_servers( + table: &Table, auto: bool, ) -> Result> { - match table.get("servers") { - None => Ok(HashMap::new()), - Some(Value::Table(table)) => Ok({ - let mut servers = HashMap::new(); - for (k, v) in table { - servers.insert(k.clone(), get_server(v, auto)?); - } - servers - }), - v => bail!("expected a table in the servers key, got {:?}", v), + let mut servers = HashMap::new(); + for (k, v) in table { + let Value::Table(table) = v else { + bail!("expected a table for server {k}, got {v:?}"); + }; + + servers.insert(k.clone(), parse_server(table, auto)?); } + Ok(servers) } -fn get_server(value: &Value, auto: bool) -> Result { +fn parse_server(table: &Table, auto: bool) -> Result { + let auto = get_bool(table, "auto", auto)?; + let ports = match table.get("ports") { + None => HashMap::new(), + Some(v) => parse_ports(v)?, + }; + + Ok(ServerConfig { auto, ports }) +} + +fn parse_ports(value: &Value) -> Result> { match value { - Value::Table(table) => Ok(ServerConfig { - auto: match table.get("auto") { - None => auto, // Default to global default - Some(Value::Boolean(v)) => *v, - Some(v) => bail!("expected true or false, got {:?}", v), - }, - ports: get_ports(table)?, - }), - value => bail!("expected a table, got {:?}", value), - } -} - -fn get_ports(table: &toml::value::Table) -> Result> { - match table.get("ports") { - None => Ok(HashMap::new()), - Some(Value::Table(table)) => Ok({ - let mut ports = HashMap::new(); - for (k,v) in table { - let port:u16 = k.parse()?; - let config = match v { - Value::Boolean(enabled) => PortConfig{enabled:*enabled, description:None}, - Value::Table(table) => PortConfig{ - enabled: match table.get("enabled") { - Some(Value::Boolean(enabled)) => *enabled, - _ => bail!("not implemented"), - }, - description: match table.get("description") { - Some(Value::String(desc)) => Some(desc.clone()), - Some(v) => bail!("expect a string description, got {:?}", v), - None => None, - }, - }, - _ => bail!("expected either a boolean (enabled) or a table for a port config, got {:?}", v), - }; - ports.insert(port, config); - } - ports - }), - Some(Value::Array(array)) => Ok({ + Value::Array(array) => { let mut ports = HashMap::new(); for v in array { - ports.insert(get_port_number(v)?, PortConfig{enabled:true, description:None}); + ports.insert( + get_port_number(v)?, + PortConfig { enabled: true, description: None }, + ); } - ports + Ok(ports) + } + + Value::Table(table) => { + let mut ports = HashMap::new(); + for (k, v) in table { + let port: u16 = k.parse()?; + let config = parse_port_config(v)?; + ports.insert(port, config); + } + Ok(ports) + } + + _ => bail!("ports must be either an array or a table, got {value:?}"), + } +} + +fn parse_port_config(value: &Value) -> Result { + match value { + Value::Boolean(enabled) => Ok(PortConfig{enabled:*enabled, description:None}), + Value::String(description) => Ok(PortConfig{ + enabled: true, + description: Some(description.clone()), }), - Some(v) => bail!("ports must be either a table of ' = ...' or an array of ports, got {:?}", v), + Value::Table(table) => { + let enabled = get_bool(table, "enabled", true)?; + let description = match table.get("description") { + Some(Value::String(desc)) => Some(desc.clone()), + Some(v) => bail!("expect a string description, got {v:?}"), + None => None, + }; + Ok(PortConfig { enabled, description }) + }, + _ => bail!("expected either a boolean (enabled), a string (description), or a table for a port config, got {value:?}"), } } @@ -174,3 +193,258 @@ fn get_port_number(v: &Value) -> Result { }; Ok(port) } + +#[cfg(test)] +mod test { + use super::*; + use pretty_assertions::assert_eq; + + fn config_test(config: &str, expected: Config) { + let config = config.parse::().expect("case not toml"); + let config = parse_config(&config).expect("unable to parse config"); + + assert_eq!(expected, config); + } + + fn config_error_test(config: &str) { + let config = config.parse::().expect("case not toml"); + assert!(parse_config(&config).is_err()); + } + + #[test] + fn empty() { + config_test("", Config { auto: true, servers: HashMap::new() }); + } + + #[test] + fn auto_false() { + config_test( + " +auto=false +", + Config { auto: false, servers: HashMap::new() }, + ); + } + + #[test] + fn auto_not_boolean() { + config_error_test( + " +auto='what is going on' +", + ); + } + + #[test] + fn servers_not_table() { + config_error_test("servers=1234"); + } + + #[test] + fn servers_default() { + config_test("servers.foo={}", { + let mut servers = HashMap::new(); + servers.insert( + "foo".to_string(), + ServerConfig { auto: true, ports: HashMap::new() }, + ); + Config { auto: true, servers } + }) + } + + #[test] + fn servers_auto_false() { + config_test( + " +[servers.foo] +auto=false +", + { + let mut servers = HashMap::new(); + servers.insert( + "foo".to_string(), + ServerConfig { auto: false, ports: HashMap::new() }, + ); + Config { auto: true, servers } + }, + ) + } + + #[test] + fn servers_auto_not_bool() { + config_error_test( + " +[servers.foo] +auto=1234 +", + ) + } + + #[test] + fn servers_ports_list() { + config_test( + " +[servers.foo] +ports=[1,2,3] +", + { + let mut servers = HashMap::new(); + servers.insert( + "foo".to_string(), + ServerConfig { + auto: true, + ports: { + let mut ports = HashMap::new(); + ports.insert( + 1, + PortConfig { enabled: true, description: None }, + ); + ports.insert( + 2, + PortConfig { enabled: true, description: None }, + ); + ports.insert( + 3, + PortConfig { enabled: true, description: None }, + ); + ports + }, + }, + ); + Config { auto: true, servers } + }, + ) + } + + #[test] + fn servers_ports_table_variations() { + config_test( + " +[servers.foo.ports] +1=true +2={enabled=false} +3=false +", + { + let mut servers = HashMap::new(); + servers.insert( + "foo".to_string(), + ServerConfig { + auto: true, + ports: { + let mut ports = HashMap::new(); + ports.insert( + 1, + PortConfig { enabled: true, description: None }, + ); + ports.insert( + 2, + PortConfig { + enabled: false, + description: None, + }, + ); + ports.insert( + 3, + PortConfig { + enabled: false, + description: None, + }, + ); + ports + }, + }, + ); + Config { auto: true, servers } + }, + ) + } + + #[test] + fn servers_ports_table_descriptions() { + config_test( + " +[servers.foo.ports] +1={enabled=false} +2={description='humble'} +", + { + let mut servers = HashMap::new(); + servers.insert( + "foo".to_string(), + ServerConfig { + auto: true, + ports: { + let mut ports = HashMap::new(); + ports.insert( + 1, + PortConfig { + enabled: false, + description: None, + }, + ); + ports.insert( + 2, + PortConfig { + enabled: true, + description: Some("humble".to_string()), + }, + ); + ports + }, + }, + ); + Config { auto: true, servers } + }, + ) + } + + #[test] + fn servers_ports_raw_desc() { + config_test( + " +[servers.foo.ports] +1='humble' +", + { + let mut servers = HashMap::new(); + servers.insert( + "foo".to_string(), + ServerConfig { + auto: true, + ports: { + let mut ports = HashMap::new(); + ports.insert( + 1, + PortConfig { + enabled: true, + description: Some("humble".to_string()), + }, + ); + ports + }, + }, + ); + Config { auto: true, servers } + }, + ) + } + + #[test] + fn servers_inherit_auto() { + config_test( + " +auto=false +servers.foo={} +", + { + let mut servers = HashMap::new(); + servers.insert( + "foo".to_string(), + ServerConfig { auto: false, ports: HashMap::new() }, + ); + Config { auto: false, servers } + }, + ) + } +}