From 0036b916622e9371e3788f1ac34e30efc2ba7da4 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Mon, 25 Apr 2022 11:39:24 +0100
Subject: [PATCH] Introduce define_list_config_builder macro

This replaces two almost-identical sets of structs and impls.  More
are on the way, as per
  https://gitlab.torproject.org/tpo/core/arti/-/issues/447
---
 crates/arti/src/logging.rs              |  55 ++----------
 crates/tor-config/src/lib.rs            |   1 +
 crates/tor-config/src/list_builder.rs   | 111 ++++++++++++++++++++++++
 crates/tor-guardmgr/src/fallback/set.rs |  61 +++----------
 4 files changed, 134 insertions(+), 94 deletions(-)
 create mode 100644 crates/tor-config/src/list_builder.rs

diff --git a/crates/arti/src/logging.rs b/crates/arti/src/logging.rs
index af4809dab7..bef7c80905 100644
--- a/crates/arti/src/logging.rs
+++ b/crates/arti/src/logging.rs
@@ -5,7 +5,7 @@ use derive_builder::Builder;
 use serde::Deserialize;
 use std::path::Path;
 use std::str::FromStr;
-use tor_config::{CfgPath, ConfigBuildError};
+use tor_config::{define_list_config_builder, CfgPath, ConfigBuildError};
 use tracing::Subscriber;
 use tracing_appender::non_blocking::WorkerGuard;
 use tracing_subscriber::layer::SubscriberExt;
@@ -65,52 +65,15 @@ impl LoggingConfig {
 /// Local type alias, mostly helpful for derive_builder to DTRT
 type LogfileListConfig = Vec<LogfileConfig>;
 
-#[derive(Default, Clone, Deserialize)]
-#[serde(transparent)]
-/// List of logfiles to use, being built as part of the configuration
-pub struct LogfileListConfigBuilder {
-    /// The logfiles, as overridden
-    files: Option<Vec<LogfileConfigBuilder>>,
-}
-
-impl LogfileListConfigBuilder {
-    /// Add a file logger
-    pub fn append(&mut self, file: LogfileConfigBuilder) -> &mut Self {
-        self.files
-            .get_or_insert_with(Self::default_files)
-            .push(file);
-        self
-    }
-
-    /// Set the list of file loggers to the supplied `files`
-    pub fn set(&mut self, files: impl IntoIterator<Item = LogfileConfigBuilder>) -> &mut Self {
-        self.files = Some(files.into_iter().collect());
-        self
-    }
-
-    /// Default logfiles
-    ///
-    /// (Currently) there are no defauolt logfiles.
-    pub(crate) fn default_files() -> Vec<LogfileConfigBuilder> {
-        vec![]
-    }
-
-    /// Resolve `LoggingConfigBuilder.files` to a value for `LoggingConfig.files`
-    pub(crate) fn build(&self) -> Result<Vec<LogfileConfig>, ConfigBuildError> {
-        let default_buffer;
-        let files = match &self.files {
-            Some(files) => files,
-            None => {
-                default_buffer = Self::default_files();
-                &default_buffer
-            }
-        };
-        let files = files
-            .iter()
-            .map(|item| item.build())
-            .collect::<Result<_, _>>()?;
-        Ok(files)
+define_list_config_builder! {
+    [
+        /// List of logfiles to use, being built as part of the configuration
+    ]
+    pub struct LogfileListConfigBuilder {
+        files: [LogfileConfigBuilder],
     }
+    built: LogfileListConfig = files;
+    default = vec![];
 }
 
 /// Configuration information for an (optionally rotating) logfile.
diff --git a/crates/tor-config/src/lib.rs b/crates/tor-config/src/lib.rs
index a7a756f58c..26c7b6f098 100644
--- a/crates/tor-config/src/lib.rs
+++ b/crates/tor-config/src/lib.rs
@@ -47,6 +47,7 @@
 #![deny(clippy::unwrap_used)]
 
 mod err;
+mod list_builder;
 mod mut_cfg;
 mod path;
 
diff --git a/crates/tor-config/src/list_builder.rs b/crates/tor-config/src/list_builder.rs
new file mode 100644
index 0000000000..eaac017fc0
--- /dev/null
+++ b/crates/tor-config/src/list_builder.rs
@@ -0,0 +1,111 @@
+//! Config list builder for lists
+
+/// Define a list builder struct and implement the conventional methods
+///
+/// The macro-generated builder struct contains `Option<Vec<ThingBuilder>>`, to allow it to
+/// distinguish "never set" from "has been adjusted or set, possibly to the empty list".
+///
+/// `#[derive(Default, Clone, Deserialize)]` will be applied, but you can specify other attributes
+/// too.  You should supply a doc comment for the builder struct, as shown in the example.
+///
+/// The `built` clause specifies the type of the built value, and how to construct it.
+/// In the expression part, `things` (the field name) will be the resolved `Vec<Thing>`
+/// and should be consumed by the expression.
+/// If the built value is simply a `Vec`, you can just write `built: things;`.
+///
+/// The `default` clause must provide an expression evaluating to a `Vec<ThingBuilder>`.  If it is
+/// nontrivial, you should put the actual defaulting functionality in a (probably-private)
+/// function, as the macro will expand it twice.
+///
+/// ```
+/// use derive_builder::Builder;
+/// use serde::Deserialize;
+/// use tor_config::{define_list_config_builder, ConfigBuildError};
+///
+/// #[derive(Builder, Debug, Eq, PartialEq)]
+/// #[builder(build_fn(error = "ConfigBuildError"))]
+/// #[builder(derive(Deserialize))]
+/// pub struct Thing { value: i32 }
+///
+/// #[derive(Debug)]
+/// pub struct ThingList { things: Vec<Thing> }
+///
+/// define_list_config_builder! {
+///    [
+///        /// List of things, being built as part of the configuration
+///    ]
+///    pub struct ThingListBuilder {
+///        pub(crate) things: [ThingBuilder],
+///    }
+///    built: ThingList = ThingList { things };
+///    default = vec![];
+/// }
+///
+/// let mut thinglist = ThingListBuilder::default();
+/// thinglist.append(ThingBuilder::default().value(42).clone());
+/// assert_eq!{ thinglist.build().unwrap().things, &[Thing { value: 42 }] }
+///
+/// thinglist.set(vec![ThingBuilder::default().value(38).clone()]);
+/// assert_eq!{ thinglist.build().unwrap().things, &[Thing { value: 38 }] }
+/// ```
+#[macro_export]
+macro_rules! define_list_config_builder {
+    {
+        [
+            $($docs_and_attrs:tt)*
+        ]
+        pub struct $ListBuilder:ident {
+            $field_vis:vis $things:ident : [$EntryBuilder:ty] $(,)?
+        }
+        built: $Built:ty = $built:expr;
+        default = $default:expr;
+    } => {
+        $($docs_and_attrs)*
+        #[derive(Default, Clone, Deserialize)]
+        #[serde(transparent)]
+        ///
+        /// This is a builder pattern struct which will be resolved
+        /// during configuration resolution, via the `build` method.
+        pub struct $ListBuilder {
+            /// The list, as overridden
+            $field_vis $things: Option<Vec<$EntryBuilder>>,
+        }
+        impl $ListBuilder {
+            /// Add one
+            pub fn append(&mut self, item: $EntryBuilder) -> &mut Self {
+                self.$things
+                    .get_or_insert_with(|| $default)
+                    .push(item);
+                self
+            }
+
+            /// Set the list to the supplied one, discarding any previous settings
+            pub fn set(&mut self, list: impl IntoIterator<Item = $EntryBuilder>) -> &mut Self {
+                self.$things = Some(list.into_iter().collect());
+                self
+            }
+
+            /// Checks whether any calls have been made to set or adjust the list
+            pub fn is_unmodified_default(&self) -> bool {
+                self.$things.is_none()
+            }
+
+            /// Resolve to a list of built items
+            pub fn build(&self) -> Result<$Built, ConfigBuildError> {
+                let default_buffer;
+                let $things = match &self.$things {
+                    Some($things) => $things,
+                    None => {
+                        default_buffer = $default;
+                        &default_buffer
+                    }
+                };
+                let $things = $things
+                    .iter()
+                    .map(|item| item.build())
+                    .collect::<Result<_, _>>()?;
+                Ok($built)
+            }
+        }
+    }
+}
diff --git a/crates/tor-guardmgr/src/fallback/set.rs b/crates/tor-guardmgr/src/fallback/set.rs
index c7cdb1ca11..8a97e8f521 100644
--- a/crates/tor-guardmgr/src/fallback/set.rs
+++ b/crates/tor-guardmgr/src/fallback/set.rs
@@ -8,7 +8,7 @@ use super::{DirStatus, FallbackDir, FallbackDirBuilder};
 use crate::fallback::default_fallbacks;
 use crate::{ids::FallbackId, PickGuardError};
 use serde::Deserialize;
-use tor_config::ConfigBuildError;
+use tor_config::{define_list_config_builder, ConfigBuildError};
 
 /// A list of fallback directories.
 ///
@@ -30,54 +30,19 @@ impl<T: IntoIterator<Item = FallbackDir>> From<T> for FallbackList {
     }
 }
 
-/// List of fallback directories, being built as part of the configuration
-///
-/// Fallback directories (represented by [`FallbackDir`]) are used by Tor
-/// clients when they don't already have enough other directory information to
-/// contact the network.
-///
-/// This is a builder pattern struct which will be resolved into a `FallbackList`
-/// during configuration resolution, via the [`build`](FallbackListBuilder::build) method.
-#[derive(Default, Clone, Deserialize)]
-#[serde(transparent)]
-pub struct FallbackListBuilder {
-    /// The fallbacks, as overridden
-    pub(crate) fallbacks: Option<Vec<FallbackDirBuilder>>,
-}
-
-impl FallbackListBuilder {
-    /// Append `fallback` to the list
-    pub fn append(&mut self, fallback: FallbackDirBuilder) {
-        self.fallbacks
-            .get_or_insert_with(default_fallbacks)
-            .push(fallback);
-    }
-
-    /// Replace the list with the supplied one
-    pub fn set(&mut self, fallbacks: impl IntoIterator<Item = FallbackDirBuilder>) {
-        self.fallbacks = Some(fallbacks.into_iter().collect());
-    }
-
-    /// Checks whether any calls have been made to set or adjust the set
-    pub fn is_unmodified_default(&self) -> bool {
-        self.fallbacks.is_none()
-    }
-
-    /// Resolve into a `FallbackList`
-    pub fn build(&self) -> Result<FallbackList, ConfigBuildError> {
-        let default_buffer;
-        let fallbacks = if let Some(f) = &self.fallbacks {
-            f
-        } else {
-            default_buffer = default_fallbacks();
-            &default_buffer
-        };
-        let fallbacks = fallbacks
-            .iter()
-            .map(|item| item.build())
-            .collect::<Result<_, _>>()?;
-        Ok(FallbackList { fallbacks })
+define_list_config_builder! {
+    [
+        /// List of fallback directories, being built as part of the configuration
+        ///
+        /// Fallback directories (represented by [`FallbackDir`]) are used by Tor
+        /// clients when they don't already have enough other directory information to
+        /// contact the network.
+    ]
+    pub struct FallbackListBuilder {
+        pub(crate) fallbacks: [FallbackDirBuilder],
     }
+    built: FallbackList = FallbackList { fallbacks };
+    default = default_fallbacks();
 }
 
 impl FallbackList {
-- 
GitLab