diff --git a/src/trx-config/src/file.rs b/src/trx-config/src/file.rs index d0b1ea7a..beeaea14 100644 --- a/src/trx-config/src/file.rs +++ b/src/trx-config/src/file.rs @@ -2,10 +2,28 @@ // // SPDX-License-Identifier: GPL-2.0-or-later +//! Loading a config section out of a TOML file. +//! +//! Two file shapes are accepted: +//! +//! - **Sectioned** — a combined `trx-rs.toml` with `[trx-server]` and/or +//! `[trx-client]` tables. This is what `--print-config` and +//! `trx-configurator` emit. +//! - **Bare** — a standalone file whose root *is* the section, i.e. `[general]` +//! and `[rig]` at the top level. Hand-written per-binary configs use this. +//! +//! A file that carries some other component's section but not ours is treated +//! as "section absent" rather than as a bare file, so loading a client-only +//! config with the server reports the missing section instead of silently +//! falling back to defaults. + use serde::de::DeserializeOwned; use std::path::{Path, PathBuf}; use thiserror::Error; +/// Every section key that may appear at the root of a combined config file. +pub const SECTION_KEYS: &[&str] = &["trx-server", "trx-client"]; + #[derive(Debug, Error)] pub enum ConfigError { #[error("Failed to read config file {0}: {1}")] @@ -26,6 +44,22 @@ fn config_search_paths() -> Vec { paths } +/// Pick the table holding `key`'s settings out of a parsed document. +/// +/// Returns the named section when present, the whole document when it carries +/// no section headers at all (a bare standalone file), or `None` when the file +/// is sectioned but has no section for `key`. +fn select_section(table: &toml::Table, key: &str) -> Option { + if let Some(section) = table.get(key) { + return Some(section.clone()); + } + let is_sectioned = SECTION_KEYS.iter().any(|k| table.contains_key(*k)); + if is_sectioned { + return None; + } + Some(toml::Value::Table(table.clone())) +} + /// Extract and deserialize a named section from a TOML file. /// /// Returns `Ok(Some(cfg))` when the section is present and parses cleanly, @@ -40,12 +74,12 @@ fn load_section_from_file( let table: toml::Table = toml::from_str(&content) .map_err(|e| ConfigError::ParseError(path.to_path_buf(), e.to_string()))?; - let Some(section) = table.get(key) else { + let Some(section) = select_section(&table, key) else { return Ok(None); }; // Re-serialize the section then parse as T so all serde defaults apply. - let section_toml = toml::to_string(section) + let section_toml = toml::to_string(§ion) .map_err(|e| ConfigError::ParseError(path.to_path_buf(), e.to_string()))?; let cfg = toml::from_str::(§ion_toml) .map_err(|e| ConfigError::ParseError(path.to_path_buf(), e.to_string()))?; @@ -59,8 +93,10 @@ pub trait ConfigFile: Sized + Default + DeserializeOwned { /// Load the section from a specific file path. /// - /// Returns an error if the file cannot be read, is not valid TOML, or - /// does not contain the expected `[]` header. + /// Accepts both a sectioned file (`[]` at the root) and a bare + /// file whose root is the section itself. Returns an error if the file + /// cannot be read, is not valid TOML, or is sectioned for some other + /// component only. fn load_from_file(path: &Path) -> Result { load_section_from_file::(path, Self::section_key())?.ok_or_else(|| { ConfigError::ParseError( @@ -86,3 +122,38 @@ pub trait ConfigFile: Sized + Default + DeserializeOwned { Ok((Self::default(), None)) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn table(s: &str) -> toml::Table { + toml::from_str(s).unwrap() + } + + #[test] + fn test_select_section_prefers_named_section() { + let t = table("[trx-server]\n[trx-server.general]\ncallsign = \"W1AW\"\n"); + let section = select_section(&t, "trx-server").unwrap(); + assert!(section.get("general").is_some()); + } + + #[test] + fn test_select_section_falls_back_to_root_for_bare_file() { + let t = table("[general]\ncallsign = \"W1AW\"\n"); + let section = select_section(&t, "trx-server").unwrap(); + assert!(section.get("general").is_some()); + } + + #[test] + fn test_select_section_absent_when_other_section_present() { + let t = table("[trx-client]\n[trx-client.general]\ncallsign = \"W1AW\"\n"); + assert!(select_section(&t, "trx-server").is_none()); + } + + #[test] + fn test_select_section_empty_file_is_bare() { + let t = table(""); + assert!(select_section(&t, "trx-server").is_some()); + } +} diff --git a/src/trx-configurator/src/writer.rs b/src/trx-configurator/src/writer.rs index e5837a1a..fc1de6f7 100644 --- a/src/trx-configurator/src/writer.rs +++ b/src/trx-configurator/src/writer.rs @@ -211,10 +211,13 @@ pub fn build_server(general: ServerGeneral, rig: RigSetup, listen: ListenSetup) let mut doc = DocumentMut::new(); doc.decor_mut() .set_prefix("# trx-server configuration\n# Generated by trx-configurator\n"); - let tables = build_server_tables(general, rig, listen); - for (key, item) in tables.iter() { - doc.insert(key, item.clone()); - } + // Emit the sectioned shape (`[trx-server]`) that trx-server writes with + // --print-config, so a generated file can be dropped into a combined + // trx-rs.toml unchanged. + doc.insert( + "trx-server", + Item::Table(build_server_tables(general, rig, listen)), + ); doc } @@ -350,10 +353,10 @@ pub fn build_client( let mut doc = DocumentMut::new(); doc.decor_mut() .set_prefix("# trx-client configuration\n# Generated by trx-configurator\n"); - let tables = build_client_tables(general, remote, frontends); - for (key, item) in tables.iter() { - doc.insert(key, item.clone()); - } + doc.insert( + "trx-client", + Item::Table(build_client_tables(general, remote, frontends)), + ); doc } @@ -453,3 +456,52 @@ pub fn write_file(doc: &DocumentMut, path: &Path) -> Result<(), String> { println!("Wrote {}", path.display()); Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use trx_config::{ClientConfig, ServerConfig}; + + fn write_temp(doc: &DocumentMut) -> tempfile::NamedTempFile { + let file = tempfile::Builder::new().suffix(".toml").tempfile().unwrap(); + std::fs::write(file.path(), doc.to_string()).unwrap(); + file + } + + /// The wizard used to emit root-level `[general]` / `[rig]` tables while the + /// loader demanded a `[trx-server]` section, so every generated standalone + /// config was rejected by the binary it was generated for. + #[test] + fn test_generated_server_config_loads_and_validates() { + let file = write_temp(&build_default(ConfigType::Server)); + let cfg = ServerConfig::load_from_file(file.path()).expect("generated config must load"); + cfg.validate().expect("generated config must validate"); + assert_eq!(cfg.rig.model.as_deref(), Some("ft817")); + assert_eq!(cfg.listen.port, 4530); + } + + #[test] + fn test_generated_client_config_loads_and_validates() { + let file = write_temp(&build_default(ConfigType::Client)); + let cfg = ClientConfig::load_from_file(file.path()).expect("generated config must load"); + cfg.validate().expect("generated config must validate"); + assert_eq!(cfg.remote.url.as_deref(), Some("localhost:4530")); + } + + #[test] + fn test_generated_combined_config_loads_both_sections() { + let file = write_temp(&build_default(ConfigType::Combined)); + let server = ServerConfig::load_from_file(file.path()).expect("server section must load"); + server.validate().expect("server section must validate"); + let client = ClientConfig::load_from_file(file.path()).expect("client section must load"); + client.validate().expect("client section must validate"); + } + + #[test] + fn test_generated_docs_are_sectioned() { + let doc = build_default(ConfigType::Server); + assert!(doc.as_table().contains_key("trx-server")); + let doc = build_default(ConfigType::Client); + assert!(doc.as_table().contains_key("trx-client")); + } +}