[fix](trx-config): accept both sectioned and bare config files
trx-configurator wrote standalone configs with [general]/[rig] at the root
while the loader required a [trx-server] section header, so every config the
wizard generated with --type server or --type client was rejected by the
binary it was generated for:
$ trx-server --config trx-server.toml
Error: ParseError("trx-server.toml", "missing [trx-server] section")
Teach the loader to fall back to the document root when no section header is
present, so hand-written standalone files keep working, and have the wizard
emit the same sectioned shape --print-config does. A file carrying only the
*other* component's section still reports the missing section rather than
silently loading defaults.
Round-trip tests now load every document the wizard can generate through the
real loader and validator.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SyX26FCpMQxiBoC7r5K1A7
Signed-off-by: Stan Grams <sjg@haxx.space>
This commit is contained in:
@@ -2,10 +2,28 @@
|
|||||||
//
|
//
|
||||||
// SPDX-License-Identifier: GPL-2.0-or-later
|
// 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 serde::de::DeserializeOwned;
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
use thiserror::Error;
|
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)]
|
#[derive(Debug, Error)]
|
||||||
pub enum ConfigError {
|
pub enum ConfigError {
|
||||||
#[error("Failed to read config file {0}: {1}")]
|
#[error("Failed to read config file {0}: {1}")]
|
||||||
@@ -26,6 +44,22 @@ fn config_search_paths() -> Vec<PathBuf> {
|
|||||||
paths
|
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<toml::Value> {
|
||||||
|
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.
|
/// Extract and deserialize a named section from a TOML file.
|
||||||
///
|
///
|
||||||
/// Returns `Ok(Some(cfg))` when the section is present and parses cleanly,
|
/// Returns `Ok(Some(cfg))` when the section is present and parses cleanly,
|
||||||
@@ -40,12 +74,12 @@ fn load_section_from_file<T: DeserializeOwned>(
|
|||||||
let table: toml::Table = toml::from_str(&content)
|
let table: toml::Table = toml::from_str(&content)
|
||||||
.map_err(|e| ConfigError::ParseError(path.to_path_buf(), e.to_string()))?;
|
.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);
|
return Ok(None);
|
||||||
};
|
};
|
||||||
|
|
||||||
// Re-serialize the section then parse as T so all serde defaults apply.
|
// 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()))?;
|
.map_err(|e| ConfigError::ParseError(path.to_path_buf(), e.to_string()))?;
|
||||||
let cfg = toml::from_str::<T>(§ion_toml)
|
let cfg = toml::from_str::<T>(§ion_toml)
|
||||||
.map_err(|e| ConfigError::ParseError(path.to_path_buf(), e.to_string()))?;
|
.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.
|
/// Load the section from a specific file path.
|
||||||
///
|
///
|
||||||
/// Returns an error if the file cannot be read, is not valid TOML, or
|
/// Accepts both a sectioned file (`[<section_key>]` at the root) and a bare
|
||||||
/// does not contain the expected `[<section_key>]` header.
|
/// 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<Self, ConfigError> {
|
fn load_from_file(path: &Path) -> Result<Self, ConfigError> {
|
||||||
load_section_from_file::<Self>(path, Self::section_key())?.ok_or_else(|| {
|
load_section_from_file::<Self>(path, Self::section_key())?.ok_or_else(|| {
|
||||||
ConfigError::ParseError(
|
ConfigError::ParseError(
|
||||||
@@ -86,3 +122,38 @@ pub trait ConfigFile: Sized + Default + DeserializeOwned {
|
|||||||
Ok((Self::default(), None))
|
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());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -211,10 +211,13 @@ pub fn build_server(general: ServerGeneral, rig: RigSetup, listen: ListenSetup)
|
|||||||
let mut doc = DocumentMut::new();
|
let mut doc = DocumentMut::new();
|
||||||
doc.decor_mut()
|
doc.decor_mut()
|
||||||
.set_prefix("# trx-server configuration\n# Generated by trx-configurator\n");
|
.set_prefix("# trx-server configuration\n# Generated by trx-configurator\n");
|
||||||
let tables = build_server_tables(general, rig, listen);
|
// Emit the sectioned shape (`[trx-server]`) that trx-server writes with
|
||||||
for (key, item) in tables.iter() {
|
// --print-config, so a generated file can be dropped into a combined
|
||||||
doc.insert(key, item.clone());
|
// trx-rs.toml unchanged.
|
||||||
}
|
doc.insert(
|
||||||
|
"trx-server",
|
||||||
|
Item::Table(build_server_tables(general, rig, listen)),
|
||||||
|
);
|
||||||
doc
|
doc
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -350,10 +353,10 @@ pub fn build_client(
|
|||||||
let mut doc = DocumentMut::new();
|
let mut doc = DocumentMut::new();
|
||||||
doc.decor_mut()
|
doc.decor_mut()
|
||||||
.set_prefix("# trx-client configuration\n# Generated by trx-configurator\n");
|
.set_prefix("# trx-client configuration\n# Generated by trx-configurator\n");
|
||||||
let tables = build_client_tables(general, remote, frontends);
|
doc.insert(
|
||||||
for (key, item) in tables.iter() {
|
"trx-client",
|
||||||
doc.insert(key, item.clone());
|
Item::Table(build_client_tables(general, remote, frontends)),
|
||||||
}
|
);
|
||||||
doc
|
doc
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -453,3 +456,52 @@ pub fn write_file(doc: &DocumentMut, path: &Path) -> Result<(), String> {
|
|||||||
println!("Wrote {}", path.display());
|
println!("Wrote {}", path.display());
|
||||||
Ok(())
|
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"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user