Commit 678504d9 authored by Ian Jackson's avatar Ian Jackson
Browse files

Abolish filter::DynFilter in favour of transparent DirFilter

There are two reasons why the DynFilter newtype might be needed:

1. To impl Default.  But we don't need it to impl Default since we can
have an accessor which does the defaulting.

2. To hide the API.  But this is usrely an unstable API.

Just writing Arc<dyn> gets rid of a lot of unnecessary boilerplate and
conversion code.
parent 0846ebd6
Loading
Loading
Loading
Loading
+3 −3
Original line number Diff line number Diff line
@@ -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 + '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
    }

+1 −1
Original line number Diff line number Diff line
@@ -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 {
+5 −42
Original line number Diff line number Diff line
@@ -13,6 +13,9 @@ 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
@@ -24,50 +27,10 @@ pub trait DirFilter: Debug + Send + Sync {
    fn filter_md(&self, md: Microdesc) -> Result<Microdesc>;
}

/// A dynamic [`DirFilter`] instance.
#[derive(Clone, Debug)]
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)
    }
}

/// A [`DirFilter`] that does nothing.
#[derive(Debug)]
struct NilFilter;
#[allow(clippy::exhaustive_structs)]
pub struct NilFilter;

impl DirFilter for NilFilter {
    fn filter_consensus(&self, consensus: UncheckedMdConsensus) -> Result<UncheckedMdConsensus> {
+2 −2
Original line number Diff line number Diff line
@@ -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(),
+5 −7
Original line number Diff line number Diff line
@@ -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
        }
    }