Merge branch 'm' into 'dir-filter'

Simplify DirFilter API

See merge request nickm/arti!1

crates/arti-client/src/builder.rs

@@ -60,7 +60,7 @@ pub struct TorClientBuilder<R: Runtime> {
     ///
     /// Only available when `arti-client` is built with the `dirfilter` and `experimental-api` features.
     #[cfg(feature = "dirfilter")]
-    dirfilter: Option<tor_dirmgr::filter::DynFilter>,
+    dirfilter: tor_dirmgr::filter::FilterConfig,
 }
 
 impl<R: Runtime> TorClientBuilder<R> {
@@ -113,9 +113,9 @@ impl<R: Runtime> TorClientBuilder<R> {
     #[cfg(feature = "dirfilter")]
     pub fn dirfilter<F>(mut self, filter: F) -> Self
     where
-        F: tor_dirmgr::filter::DirFilter + Send + Sync + 'static,
+        F: Into<Arc<dyn tor_dirmgr::filter::DirFilter + 'static>>,
     {
-        self.dirfilter = Some(tor_dirmgr::filter::DynFilter::new(filter));
+        self.dirfilter = Some(filter.into());
         self
     }
 

crates/tor-dirmgr/src/config.rs

@@ -278,7 +278,7 @@ impl DirMgrConfig {
 pub struct DirMgrExtensions {
     /// A filter to be used when installing new directory objects.
     #[cfg(feature = "dirfilter")]
-    pub filter: Option<crate::filter::DynFilter>,
+    pub filter: crate::filter::FilterConfig,
 }
 
 impl DownloadScheduleConfig {

crates/tor-dirmgr/src/filter.rs

@@ -7,77 +7,33 @@
 //! future versions, or its API might change completely. There are no semver
 //! guarantees.
 
+use std::fmt::Debug;
 use std::sync::Arc;
 
 use crate::Result;
 use tor_netdoc::doc::{microdesc::Microdesc, netstatus::UncheckedMdConsensus};
 
+/// Filtering configuration, as provided to the directory code
+pub type FilterConfig = Option<Arc<dyn DirFilter>>;
+
 /// An object that can filter directory documents before they're handled.
 ///
 /// Instances of DirFilter can be used for testing, to modify directory data
 /// on-the-fly.
-pub trait DirFilter {
+pub trait DirFilter: Debug + Send + Sync {
     /// Modify `consensus` in an unspecified way.
-    fn filter_consensus(&self, consensus: UncheckedMdConsensus) -> Result<UncheckedMdConsensus>;
-    /// Modify `md` in an unspecified way.
-    fn filter_md(&self, md: Microdesc) -> Result<Microdesc>;
-}
-
-/// A dynamic [`DirFilter`] instance.
-#[derive(Clone)]
-pub struct DynFilter {
-    /// A reference to the DirFilter object
-    filter: Arc<dyn DirFilter + Send + Sync>,
-}
-
-impl From<&Option<DynFilter>> for DynFilter {
-    fn from(option: &Option<DynFilter>) -> Self {
-        option.as_ref().map(Clone::clone).unwrap_or_default()
-    }
-}
-
-impl Default for DynFilter {
-    fn default() -> Self {
-        DynFilter::new(NilFilter)
-    }
-}
-
-impl DynFilter {
-    /// Wrap `filter` as a [`DynFilter`]
-    pub fn new<T>(filter: T) -> Self
-    where
-        T: DirFilter + Send + Sync + 'static,
-    {
-        DynFilter {
-            filter: Arc::new(filter),
-        }
-    }
-}
-
-impl DirFilter for DynFilter {
-    fn filter_consensus(&self, consensus: UncheckedMdConsensus) -> Result<UncheckedMdConsensus> {
-        self.filter.filter_consensus(consensus)
-    }
-
-    fn filter_md(&self, md: Microdesc) -> Result<Microdesc> {
-        self.filter.filter_md(md)
-    }
-}
-
-impl std::fmt::Debug for DynFilter {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("DynFilter").finish_non_exhaustive()
-    }
-}
-
-/// A [`DirFilter`] that does nothing.
-struct NilFilter;
-
-impl DirFilter for NilFilter {
     fn filter_consensus(&self, consensus: UncheckedMdConsensus) -> Result<UncheckedMdConsensus> {
         Ok(consensus)
     }
+    /// Modify `md` in an unspecified way.
     fn filter_md(&self, md: Microdesc) -> Result<Microdesc> {
         Ok(md)
     }
 }
+
+/// A [`DirFilter`] that does nothing.
+#[derive(Debug)]
+#[allow(clippy::exhaustive_structs)]
+pub struct NilFilter;
+
+impl DirFilter for NilFilter {}

crates/tor-dirmgr/src/lib.rs

@@ -228,7 +228,7 @@ pub struct DirMgr<R: Runtime> {
 
     /// A filter that gets applied to directory objects before we use them.
     #[cfg(feature = "dirfilter")]
-    filter: filter::DynFilter,
+    filter: crate::filter::FilterConfig,
 }
 
 /// RAII guard to reset an AtomicBool on drop.
@@ -710,7 +710,7 @@ impl<R: Runtime> DirMgr<R> {
             inner: receive_status,
         };
         #[cfg(feature = "dirfilter")]
-        let filter = (&config.extensions.filter).into();
+        let filter = config.extensions.filter.clone();
 
         Ok(DirMgr {
             config: config.into(),

crates/tor-dirmgr/src/state.rs

@@ -23,8 +23,6 @@ use tracing::{info, warn};
 
 use crate::event::{DirStatus, DirStatusInner};
 
-#[cfg(feature = "dirfilter")]
-use crate::filter::DirFilter;
 use crate::storage::{DynStore, EXPIRATION_DEFAULTS};
 use crate::{
     docmeta::{AuthCertMeta, ConsensusMeta},
@@ -89,7 +87,7 @@ pub(crate) trait WriteNetDir: 'static + Sync + Send {
 
     /// Return the currently configured DynFilter for this state.
     #[cfg(feature = "dirfilter")]
-    fn filter(&self) -> crate::filter::DynFilter;
+    fn filter(&self) -> &dyn crate::filter::DirFilter;
 }
 
 impl<R: Runtime> WriteNetDir for crate::DirMgr<R> {
@@ -116,8 +114,8 @@ impl<R: Runtime> WriteNetDir for crate::DirMgr<R> {
     }
 
     #[cfg(feature = "dirfilter")]
-    fn filter(&self) -> crate::filter::DynFilter {
-        self.filter.clone()
+    fn filter(&self) -> &dyn crate::filter::DirFilter {
+        self.filter.as_deref().unwrap_or(&crate::filter::NilFilter)
     }
 }
 
@@ -1059,8 +1057,8 @@ mod test {
             self.now
         }
         #[cfg(feature = "dirfilter")]
-        fn filter(&self) -> crate::filter::DynFilter {
-            Default::default()
+        fn filter(&self) -> &dyn crate::filter::DirFilter {
+            &crate::filter::NilFilter
         }
     }