Add optional builder pattern for BuilderConfig (non-breaking)

Addresses @jstuczyn's feedback about too many arguments by adding
BuilderConfigBuilder as an alternative to the existing new() method.
This commit is contained in:
Tommy Verrall
2025-10-20 11:39:50 +02:00
committed by Jędrzej Stuczyński
parent b5b8b8f224
commit e1b06f02f3
@@ -56,6 +56,9 @@ pub struct MixnetClientConfig {
}
impl BuilderConfig {
/// Creates a new BuilderConfig with all required parameters.
///
/// However, consider using `BuilderConfig::builder()` instead.
#[allow(clippy::too_many_arguments)]
pub fn new(
entry_node: NymNodeWithKeys,
@@ -84,6 +87,22 @@ impl BuilderConfig {
}
}
/// Creates a builder for BuilderConfig
///
/// This is the preferred way to construct a BuilderConfig.
///
/// # Example
/// ```ignore
/// let config = BuilderConfig::builder()
/// .entry_node(entry)
/// .exit_node(exit)
/// .user_agent(agent)
/// .build()?;
/// ```
pub fn builder() -> BuilderConfigBuilder {
BuilderConfigBuilder::default()
}
pub fn mixnet_client_debug_config(&self) -> DebugConfig {
if self.two_hops {
two_hop_debug_config(&self.mixnet_client_config)
@@ -235,6 +254,113 @@ fn true_to_disabled(val: bool) -> &'static str {
if val { "disabled" } else { "enabled" }
}
/// Builder for `BuilderConfig`
///
/// This provides a more convinient way to construct a `BuilderConfig` compared to the
/// `new()` constructor with many arguments.
#[derive(Default)]
pub struct BuilderConfigBuilder {
entry_node: Option<NymNodeWithKeys>,
exit_node: Option<NymNodeWithKeys>,
data_path: Option<PathBuf>,
mixnet_client_config: Option<MixnetClientConfig>,
two_hops: bool,
user_agent: Option<UserAgent>,
custom_topology_provider: Option<Box<dyn TopologyProvider + Send + Sync>>,
network_env: Option<NymNetworkDetails>,
cancel_token: Option<CancellationToken>,
#[cfg(unix)]
connection_fd_callback: Option<Arc<dyn Fn(RawFd) + Send + Sync>>,
}
impl BuilderConfigBuilder {
pub fn new() -> Self {
Self::default()
}
pub fn entry_node(mut self, entry_node: NymNodeWithKeys) -> Self {
self.entry_node = Some(entry_node);
self
}
pub fn exit_node(mut self, exit_node: NymNodeWithKeys) -> Self {
self.exit_node = Some(exit_node);
self
}
pub fn data_path(mut self, data_path: Option<PathBuf>) -> Self {
self.data_path = data_path;
self
}
pub fn mixnet_client_config(mut self, mixnet_client_config: MixnetClientConfig) -> Self {
self.mixnet_client_config = Some(mixnet_client_config);
self
}
pub fn two_hops(mut self, two_hops: bool) -> Self {
self.two_hops = two_hops;
self
}
pub fn user_agent(mut self, user_agent: UserAgent) -> Self {
self.user_agent = Some(user_agent);
self
}
pub fn custom_topology_provider(
mut self,
custom_topology_provider: Box<dyn TopologyProvider + Send + Sync>,
) -> Self {
self.custom_topology_provider = Some(custom_topology_provider);
self
}
pub fn network_env(mut self, network_env: NymNetworkDetails) -> Self {
self.network_env = Some(network_env);
self
}
pub fn cancel_token(mut self, cancel_token: CancellationToken) -> Self {
self.cancel_token = Some(cancel_token);
self
}
#[cfg(unix)]
pub fn connection_fd_callback(
mut self,
connection_fd_callback: Arc<dyn Fn(RawFd) + Send + Sync>,
) -> Self {
self.connection_fd_callback = Some(connection_fd_callback);
self
}
/// Builds the `BuilderConfig`.
///
/// Returns an error if any required field is missing.
pub fn build(self) -> Result<BuilderConfig, String> {
Ok(BuilderConfig {
entry_node: self.entry_node.ok_or("entry_node is required")?,
exit_node: self.exit_node.ok_or("exit_node is required")?,
data_path: self.data_path,
mixnet_client_config: self
.mixnet_client_config
.ok_or("mixnet_client_config is required")?,
two_hops: self.two_hops,
user_agent: self.user_agent.ok_or("user_agent is required")?,
custom_topology_provider: self
.custom_topology_provider
.ok_or("custom_topology_provider is required")?,
network_env: self.network_env.ok_or("network_env is required")?,
cancel_token: self.cancel_token.ok_or("cancel_token is required")?,
#[cfg(unix)]
connection_fd_callback: self
.connection_fd_callback
.ok_or("connection_fd_callback is required")?,
})
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -247,4 +373,49 @@ mod tests {
assert_eq!(config.min_mixnode_performance, None);
assert_eq!(config.min_gateway_performance, None);
}
#[test]
fn test_builder_config_builder_fails_without_required_fields() {
// Building without any fields should fail
let result = BuilderConfig::builder().build();
assert!(result.is_err());
if let Err(err) = result {
assert!(err.contains("entry_node is required"));
}
}
#[test]
fn test_builder_config_builder_fails_missing_individual_fields() {
// Test that each required field is validated
let result = BuilderConfig::builder().build();
assert!(result.is_err());
// We can only test the first error message since build() short-circuits
if let Err(err) = result {
assert!(
err.contains("entry_node")
|| err.contains("exit_node")
|| err.contains("mixnet_client_config")
|| err.contains("user_agent")
|| err.contains("custom_topology_provider")
|| err.contains("network_env")
|| err.contains("cancel_token")
);
}
}
#[test]
fn test_builder_config_builder_method_chaining() {
// Test that builder methods chain properly and return Self
let builder = BuilderConfig::builder();
// Verify the builder returns itself for chaining
let builder = builder.two_hops(true);
let builder = builder.two_hops(false);
let builder = builder.data_path(None);
// Builder should still fail because required fields are missing
let result = builder.build();
assert!(result.is_err());
}
}