From bd96581c6adce7571313dd73f67210e5c6faa4e2 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Thu, 29 Jun 2023 16:54:55 +0200 Subject: [PATCH] Don't return Box from constructors --- src/domain/acme/manager.rs | 87 ++++++++++++++++++++------------------ src/domain/acme/store.rs | 37 ++++++++-------- src/domain/checker.rs | 40 +++++++++--------- src/domain/config.rs | 16 +++---- src/domain/manager.rs | 20 +++++---- src/main.rs | 23 ++++++---- src/origin/store/git.rs | 28 ++++++------ src/origin/store/mux.rs | 14 +++--- 8 files changed, 142 insertions(+), 123 deletions(-) diff --git a/src/domain/acme/manager.rs b/src/domain/acme/manager.rs index 3d57797..729d3e6 100644 --- a/src/domain/acme/manager.rs +++ b/src/domain/acme/manager.rs @@ -24,55 +24,62 @@ pub trait Manager: Sync + Send { ) -> Result<(PrivateKey, Vec), GetCertificateError>; } -struct ManagerImpl { +pub struct ManagerImpl { store: Box, account: sync::Arc, } -pub async fn new( - store: Box, - contact_email: &str, -) -> Result, unexpected::Error> { - let dir = acme2::DirectoryBuilder::new(LETS_ENCRYPT_URL.to_string()) - .build() - .await - .or_unexpected_while("creating acme2 directory builder")?; +impl ManagerImpl { + pub async fn new( + store: Store, + contact_email: &str, + ) -> Result { + let dir = acme2::DirectoryBuilder::new(LETS_ENCRYPT_URL.to_string()) + .build() + .await + .or_unexpected_while("creating acme2 directory builder")?; - let mut contact = String::from("mailto:"); - contact.push_str(contact_email); + let mut contact = String::from("mailto:"); + contact.push_str(contact_email); - let mut builder = acme2::AccountBuilder::new(dir); - builder.contact(vec![contact]); - builder.terms_of_service_agreed(true); + let mut builder = acme2::AccountBuilder::new(dir); + builder.contact(vec![contact]); + builder.terms_of_service_agreed(true); - match store.get_account_key() { - Ok(account_key) => { - builder.private_key( - (&account_key) - .try_into() - .or_unexpected_while("parsing private key")?, - ); + match store.get_account_key() { + Ok(account_key) => { + builder.private_key( + (&account_key) + .try_into() + .or_unexpected_while("parsing private key")?, + ); + } + Err(acme::store::GetAccountKeyError::NotFound) => (), + Err(acme::store::GetAccountKeyError::Unexpected(err)) => { + return Err(err.into_unexpected()) + } } - Err(acme::store::GetAccountKeyError::NotFound) => (), - Err(acme::store::GetAccountKeyError::Unexpected(err)) => return Err(err.into_unexpected()), + + let account = builder + .build() + .await + .or_unexpected_while("building account")?; + + let account_key: acme::PrivateKey = account + .private_key() + .as_ref() + .try_into() + .or_unexpected_while("parsing private key back out")?; + + store + .set_account_key(&account_key) + .or_unexpected_while("storing account key")?; + + Ok(Self { + store: Box::from(store), + account, + }) } - - let account = builder - .build() - .await - .or_unexpected_while("building account")?; - - let account_key: acme::PrivateKey = account - .private_key() - .as_ref() - .try_into() - .or_unexpected_while("parsing private key back out")?; - - store - .set_account_key(&account_key) - .or_unexpected_while("storing account key")?; - - Ok(Box::new(ManagerImpl { store, account })) } impl Manager for ManagerImpl { diff --git a/src/domain/acme/store.rs b/src/domain/acme/store.rs index 2e41217..585c588 100644 --- a/src/domain/acme/store.rs +++ b/src/domain/acme/store.rs @@ -66,28 +66,29 @@ struct StoredPKeyCert { cert: Vec, } -struct FSStore { +pub struct FSStore { dir_path: path::PathBuf, } -pub fn new(dir_path: &path::Path) -> Result, unexpected::Error> { - vec![ - dir_path, - dir_path.join("http01_challenge_keys").as_ref(), - dir_path.join("certificates").as_ref(), - ] - .iter() - .map(|dir| { - fs::create_dir_all(dir).map_unexpected_while(|| format!("creating dir {}", dir.display())) - }) - .try_collect()?; - - Ok(Box::new(FSStore { - dir_path: dir_path.into(), - })) -} - impl FSStore { + pub fn new(dir_path: &path::Path) -> Result { + vec![ + dir_path, + dir_path.join("http01_challenge_keys").as_ref(), + dir_path.join("certificates").as_ref(), + ] + .iter() + .map(|dir| { + fs::create_dir_all(dir) + .map_unexpected_while(|| format!("creating dir {}", dir.display())) + }) + .try_collect()?; + + Ok(Self { + dir_path: dir_path.into(), + }) + } + fn account_key_path(&self) -> path::PathBuf { self.dir_path.join("account.key") } diff --git a/src/domain/checker.rs b/src/domain/checker.rs index f4e0032..bc33a7f 100644 --- a/src/domain/checker.rs +++ b/src/domain/checker.rs @@ -36,27 +36,27 @@ pub struct DNSChecker { client: tokio::sync::Mutex, } -pub async fn new( - target_a: net::Ipv4Addr, - resolver_addr: &str, -) -> Result { - let resolver_addr = resolver_addr - .parse() - .map_err(|_| NewDNSCheckerError::InvalidResolverAddress)?; - - let stream = udp::UdpClientStream::::new(resolver_addr); - - let (client, bg) = AsyncClient::connect(stream).await.or_unexpected()?; - - tokio::spawn(bg); - - Ok(DNSChecker { - target_a, - client: tokio::sync::Mutex::new(client), - }) -} - impl DNSChecker { + pub async fn new( + target_a: net::Ipv4Addr, + resolver_addr: &str, + ) -> Result { + let resolver_addr = resolver_addr + .parse() + .map_err(|_| NewDNSCheckerError::InvalidResolverAddress)?; + + let stream = udp::UdpClientStream::::new(resolver_addr); + + let (client, bg) = AsyncClient::connect(stream).await.or_unexpected()?; + + tokio::spawn(bg); + + Ok(Self { + target_a, + client: tokio::sync::Mutex::new(client), + }) + } + pub async fn check_domain( &self, domain: &domain::Name, diff --git a/src/domain/config.rs b/src/domain/config.rs index 4315a7d..0a60348 100644 --- a/src/domain/config.rs +++ b/src/domain/config.rs @@ -45,18 +45,18 @@ pub trait Store: Sync + Send { fn all_domains(&self) -> Result, unexpected::Error>; } -struct FSStore { +pub struct FSStore { dir_path: PathBuf, } -pub fn new(dir_path: &Path) -> io::Result> { - fs::create_dir_all(dir_path)?; - Ok(Box::new(FSStore { - dir_path: dir_path.into(), - })) -} - impl FSStore { + pub fn new(dir_path: &Path) -> io::Result { + fs::create_dir_all(dir_path)?; + Ok(Self { + dir_path: dir_path.into(), + }) + } + fn config_dir_path(&self, domain: &domain::Name) -> PathBuf { self.dir_path.join(domain.as_str()) } diff --git a/src/domain/manager.rs b/src/domain/manager.rs index 277dcf6..c8e6a85 100644 --- a/src/domain/manager.rs +++ b/src/domain/manager.rs @@ -176,18 +176,22 @@ async fn sync_origins(origin_store: &dyn origin::store::Store, canceller: Cancel } } -pub fn new( +pub fn new< + OriginStore: origin::store::Store + 'static, + DomainConfigStore: config::Store + 'static, + AcmeManager: acme::manager::Manager + 'static, +>( task_stack: &mut util::TaskStack, - origin_store: Box, - domain_config_store: Box, + origin_store: OriginStore, + domain_config_store: DomainConfigStore, domain_checker: checker::DNSChecker, - acme_manager: Option>, + acme_manager: Option, ) -> sync::Arc { let manager = sync::Arc::new(ManagerImpl { - origin_store, - domain_config_store, - domain_checker, - acme_manager, + origin_store: Box::from(origin_store), + domain_config_store: Box::from(domain_config_store), + domain_checker: domain_checker, + acme_manager: acme_manager.map(|m| Box::new(m) as Box), }); task_stack.push_spawn(|canceller| { diff --git a/src/main.rs b/src/main.rs index 936fc1d..e9e78bb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -73,33 +73,38 @@ async fn main() { ) .init(); - let origin_store = domani::origin::store::git::new(config.origin_store_git_dir_path) + let origin_store = domani::origin::store::git::FSStore::new(config.origin_store_git_dir_path) .expect("git origin store initialization failed"); - let domain_checker = domani::domain::checker::new( + let domain_checker = domani::domain::checker::DNSChecker::new( config.domain_checker_target_a, &config.domain_checker_resolver_addr, ) .await .expect("domain checker initialization failed"); - let domain_config_store = domani::domain::config::new(&config.domain_config_store_dir_path) - .expect("domain config store initialization failed"); + let domain_config_store = + domani::domain::config::FSStore::new(&config.domain_config_store_dir_path) + .expect("domain config store initialization failed"); let domain_acme_manager = if config.https_listen_addr.is_some() { let domain_acme_store_dir_path = config.domain_acme_store_dir_path.unwrap(); - let domain_acme_store = domani::domain::acme::store::new(&domain_acme_store_dir_path) - .expect("domain acme store initialization failed"); + let domain_acme_store = + domani::domain::acme::store::FSStore::new(&domain_acme_store_dir_path) + .expect("domain acme store initialization failed"); // if https_listen_addr is set then domain_acme_contact_email is required, see the Cli/clap // settings. let domain_acme_contact_email = config.domain_acme_contact_email.unwrap(); Some( - domani::domain::acme::manager::new(domain_acme_store, &domain_acme_contact_email) - .await - .expect("domain acme manager initialization failed"), + domani::domain::acme::manager::ManagerImpl::new( + domain_acme_store, + &domain_acme_contact_email, + ) + .await + .expect("domain acme manager initialization failed"), ) } else { None diff --git a/src/origin/store/git.rs b/src/origin/store/git.rs index 2ecc90c..2fae03b 100644 --- a/src/origin/store/git.rs +++ b/src/origin/store/git.rs @@ -59,9 +59,9 @@ enum GetOriginError { Unexpected(#[from] unexpected::Error), } -/// git::Store implements the Store trait for any Descr::Git based Origins. If any non-git -/// Descrs are used then this implementation will panic. -struct Store { +/// Implements the Store trait for any Descr::Git based Origins, storing the git repos on disk. If +/// any non-git Descrs are used then this implementation will panic. +pub struct FSStore { dir_path: PathBuf, // to prevent against syncing the same origin more than once at a time, but still allowing @@ -71,16 +71,16 @@ struct Store { origins: sync::RwLock>>, } -pub fn new(dir_path: PathBuf) -> io::Result> { - fs::create_dir_all(&dir_path)?; - Ok(Box::new(Store { - dir_path, - sync_guard: sync::Mutex::new(collections::HashMap::new()), - origins: sync::RwLock::new(collections::HashMap::new()), - })) -} +impl FSStore { + pub fn new(dir_path: PathBuf) -> io::Result { + fs::create_dir_all(&dir_path)?; + Ok(Self { + dir_path, + sync_guard: sync::Mutex::new(collections::HashMap::new()), + origins: sync::RwLock::new(collections::HashMap::new()), + }) + } -impl Store { fn repo_path(&self, descr: &origin::Descr) -> PathBuf { self.dir_path.join(descr.id()) } @@ -208,7 +208,7 @@ impl Store { } } -impl super::Store for Store { +impl super::Store for FSStore { fn sync(&self, descr: origin::Descr, limits: store::Limits) -> Result<(), store::SyncError> { // attempt to lock this descr for syncing, doing so within a new scope so the mutex // isn't actually being held for the whole method duration. @@ -344,7 +344,7 @@ mod tests { let limits = store::Limits {}; - let store = super::new(tmp_dir.path().to_path_buf()).expect("store created"); + let store = super::FSStore::new(tmp_dir.path().to_path_buf()).expect("store created"); store .sync(descr.clone(), limits) diff --git a/src/origin/store/mux.rs b/src/origin/store/mux.rs index 9c38e84..7afe201 100644 --- a/src/origin/store/mux.rs +++ b/src/origin/store/mux.rs @@ -2,26 +2,28 @@ use crate::error::unexpected::Mappable; use crate::origin::{self, store}; use std::sync; -struct Store +pub struct Store where - S: store::Store, + S: store::Store + 'static, F: Fn(&origin::Descr) -> Option + Sync + Send, { mapping_fn: F, stores: Vec, } -pub fn new(mapping_fn: F, stores: Vec) -> Box +impl Store where S: store::Store + 'static, - F: Fn(&origin::Descr) -> Option + Sync + Send + 'static, + F: Fn(&origin::Descr) -> Option + Sync + Send, { - Box::new(Store { mapping_fn, stores }) + pub fn new(mapping_fn: F, stores: Vec) -> Store { + Store { mapping_fn, stores } + } } impl store::Store for Store where - S: store::Store, + S: store::Store + 'static, F: Fn(&origin::Descr) -> Option + Sync + Send, { fn sync(&self, descr: origin::Descr, limits: store::Limits) -> Result<(), store::SyncError> {