From 8ab670d9c82df5db5802788c435b4ddb5abfc2f4 Mon Sep 17 00:00:00 2001 From: Carine Dengler Date: Mon, 6 Nov 2023 16:05:42 +0100 Subject: [PATCH 1/8] feat: use variables for file names and improve debug messages by including full path --- src/client.rs | 5 +++-- src/flatfiledirmgr.rs | 29 +++++++++++++++++------------ 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/client.rs b/src/client.rs index c450e04..fbf1ca0 100644 --- a/src/client.rs +++ b/src/client.rs @@ -22,6 +22,7 @@ use crate::{ pub struct Client(TorClient); impl Client { + const AUTHORITY_FILENAME: &'static str = "authority.json"; /// Create a new client with the given cache directory pub async fn new(cache: &Path) -> Result { let runtime = Runtime::current().context("get runtime")?; @@ -43,8 +44,8 @@ impl Client { .cache_dir(CfgPath::new_literal(cache_path)) .state_dir(CfgPath::new_literal(cache_path)); - let auth_path = cache_path.join("authority.json"); - let auth_raw = fs::read_to_string(auth_path).context("Failed to read authority")?; + let auth_path = cache_path.join(Self::AUTHORITY_FILENAME); + let auth_raw = fs::read_to_string(auth_path.clone()).context(format!("Failed to read {}", auth_path.to_string_lossy()))?; let auth = serde_json::from_str(auth_raw.as_str())?; cfg_builder.tor_network().set_authorities(vec![auth]); diff --git a/src/flatfiledirmgr.rs b/src/flatfiledirmgr.rs index 27e0195..2a3fddd 100644 --- a/src/flatfiledirmgr.rs +++ b/src/flatfiledirmgr.rs @@ -54,6 +54,11 @@ pub struct FlatFileDirMgr { } impl FlatFileDirMgr { + /// Contents of the directory cache. + const CONSENSUS_FILENAME: &'static str = "consensus.txt"; + const MICRODESCRIPTORS_FILENAME: &'static str = "microdescriptors.txt"; + const CERTIFICATE_FILENAME: &'static str = "certificate.txt"; + const CHURN_FILENAME: &'static str = "churn.txt"; /// Create a new FlatFileDirMgr from a given configuration. pub fn from_config(config: DirMgrConfig, circmgr: Arc>) -> Result> { let netdir = SharedMutArc::new(); @@ -147,14 +152,14 @@ impl FlatFileDirMgr { &self, cache_path: &Path, ) -> Result> { - let path = cache_path.join("consensus.txt"); + let path = cache_path.join(Self::CONSENSUS_FILENAME); let consensus_text = - fs::read_to_string(path).map_err(|_| Error::UnrecognizedAuthorities)?; - debug!("consensus.txt loaded"); + fs::read_to_string(path.clone()).map_err(|_| Error::UnrecognizedAuthorities)?; + debug!("{} loaded", path.to_string_lossy()); - let path = cache_path.join("churn.txt"); - let churn_text = fs::read_to_string(path).unwrap_or_else(|_| "".to_string()); - debug!("churn.txt loaded"); + let path = cache_path.join(Self::CHURN_FILENAME); + let churn_text = fs::read_to_string(path.clone()).unwrap_or_else(|_| "".to_string()); + debug!("{} loaded", path.to_string_lossy()); let (_, _, parsed) = MdConsensus::parse(&consensus_text) .map_err(|_| Error::CacheCorruption("Failed to parse consensus"))?; @@ -195,9 +200,9 @@ impl FlatFileDirMgr { /// Load the certificate from a flat file. fn load_certificate(&self, cache_path: &Path) -> Result { - let path = cache_path.join("certificate.txt"); - let certificate = fs::read_to_string(path).map_err(|_| Error::UnrecognizedAuthorities)?; - debug!("certificate.txt loaded"); + let path = cache_path.join(Self::CERTIFICATE_FILENAME); + let certificate = fs::read_to_string(path.clone()).map_err(|_| Error::UnrecognizedAuthorities)?; + debug!("{} loaded", path.to_string_lossy()); let parsed = AuthCert::parse(certificate.as_str()) .map_err(|_| Error::CacheCorruption("Failed to parse certificate"))? @@ -211,9 +216,9 @@ impl FlatFileDirMgr { /// Load the list of microdescriptors from a flat file. fn load_microdesc(&self, cache_path: &Path) -> Result> { - let path = cache_path.join("microdescriptors.txt"); - let udesc_text = fs::read_to_string(path).map_err(|_| Error::UnrecognizedAuthorities)?; - debug!("microdescriptors.txt loaded"); + let path = cache_path.join(Self::MICRODESCRIPTORS_FILENAME); + let udesc_text = fs::read_to_string(path.clone()).map_err(|_| Error::UnrecognizedAuthorities)?; + debug!("{} loaded", path.to_string_lossy()); let udesc = MicrodescReader::new( udesc_text.as_str(), From 2fd4a9f03b71e83c974b8456902cd6758d04131d Mon Sep 17 00:00:00 2001 From: Carine Dengler Date: Mon, 6 Nov 2023 17:45:02 +0100 Subject: [PATCH 2/8] feat: add method to check directory integrity --- src/flatfiledirmgr.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/flatfiledirmgr.rs b/src/flatfiledirmgr.rs index 2a3fddd..d8176d1 100644 --- a/src/flatfiledirmgr.rs +++ b/src/flatfiledirmgr.rs @@ -59,6 +59,7 @@ impl FlatFileDirMgr { const MICRODESCRIPTORS_FILENAME: &'static str = "microdescriptors.txt"; const CERTIFICATE_FILENAME: &'static str = "certificate.txt"; const CHURN_FILENAME: &'static str = "churn.txt"; + const AUTHORITY_FILENAME: &'static str = "authority.json"; /// Create a new FlatFileDirMgr from a given configuration. pub fn from_config(config: DirMgrConfig, circmgr: Arc>) -> Result> { let netdir = SharedMutArc::new(); @@ -75,6 +76,21 @@ impl FlatFileDirMgr { })) } + /// Check cache directory content. + pub fn check_directory(&self, cache_path: &Path) -> Result<()> { + let mut any_missing = false; + for filename in vec![Self::CONSENSUS_FILENAME, Self::MICRODESCRIPTORS_FILENAME, Self::CERTIFICATE_FILENAME, Self::CHURN_FILENAME, Self::AUTHORITY_FILENAME].iter() { + if !cache_path.join(filename).exists(){ + any_missing = true; + debug!("required file missing: {filename}"); + } + } + if any_missing { + return Err(Error::CacheCorruption("required files missing in cache")) + } + Ok(()) + } + /// Try to load the directory from flat files. /// /// This is strongly inspired by the add_from_cache() methods from the various states in @@ -82,6 +98,7 @@ impl FlatFileDirMgr { pub async fn load_directory(&self) -> Result { let config = self.config.get(); let cache_path = &config.cache_path; + self.check_directory(cache_path)?; // Consensus let unvalidated = self.load_consensus(cache_path)?; From 6220b9cc04c8614ecb158c267ca115cf60a943b9 Mon Sep 17 00:00:00 2001 From: Carine Dengler Date: Mon, 6 Nov 2023 17:52:27 +0100 Subject: [PATCH 3/8] style: apply linting --- src/client.rs | 3 ++- src/flatfiledirmgr.rs | 20 +++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/client.rs b/src/client.rs index fbf1ca0..4ee859a 100644 --- a/src/client.rs +++ b/src/client.rs @@ -45,7 +45,8 @@ impl Client { .state_dir(CfgPath::new_literal(cache_path)); let auth_path = cache_path.join(Self::AUTHORITY_FILENAME); - let auth_raw = fs::read_to_string(auth_path.clone()).context(format!("Failed to read {}", auth_path.to_string_lossy()))?; + let auth_raw = fs::read_to_string(auth_path.clone()) + .context(format!("Failed to read {}", auth_path.to_string_lossy()))?; let auth = serde_json::from_str(auth_raw.as_str())?; cfg_builder.tor_network().set_authorities(vec![auth]); diff --git a/src/flatfiledirmgr.rs b/src/flatfiledirmgr.rs index d8176d1..0f08e15 100644 --- a/src/flatfiledirmgr.rs +++ b/src/flatfiledirmgr.rs @@ -79,14 +79,22 @@ impl FlatFileDirMgr { /// Check cache directory content. pub fn check_directory(&self, cache_path: &Path) -> Result<()> { let mut any_missing = false; - for filename in vec![Self::CONSENSUS_FILENAME, Self::MICRODESCRIPTORS_FILENAME, Self::CERTIFICATE_FILENAME, Self::CHURN_FILENAME, Self::AUTHORITY_FILENAME].iter() { - if !cache_path.join(filename).exists(){ + for filename in [ + Self::CONSENSUS_FILENAME, + Self::MICRODESCRIPTORS_FILENAME, + Self::CERTIFICATE_FILENAME, + Self::CHURN_FILENAME, + Self::AUTHORITY_FILENAME, + ] + .iter() + { + if !cache_path.join(filename).exists() { any_missing = true; debug!("required file missing: {filename}"); } } if any_missing { - return Err(Error::CacheCorruption("required files missing in cache")) + return Err(Error::CacheCorruption("required files missing in cache")); } Ok(()) } @@ -218,7 +226,8 @@ impl FlatFileDirMgr { /// Load the certificate from a flat file. fn load_certificate(&self, cache_path: &Path) -> Result { let path = cache_path.join(Self::CERTIFICATE_FILENAME); - let certificate = fs::read_to_string(path.clone()).map_err(|_| Error::UnrecognizedAuthorities)?; + let certificate = + fs::read_to_string(path.clone()).map_err(|_| Error::UnrecognizedAuthorities)?; debug!("{} loaded", path.to_string_lossy()); let parsed = AuthCert::parse(certificate.as_str()) @@ -234,7 +243,8 @@ impl FlatFileDirMgr { /// Load the list of microdescriptors from a flat file. fn load_microdesc(&self, cache_path: &Path) -> Result> { let path = cache_path.join(Self::MICRODESCRIPTORS_FILENAME); - let udesc_text = fs::read_to_string(path.clone()).map_err(|_| Error::UnrecognizedAuthorities)?; + let udesc_text = + fs::read_to_string(path.clone()).map_err(|_| Error::UnrecognizedAuthorities)?; debug!("{} loaded", path.to_string_lossy()); let udesc = MicrodescReader::new( From e4f4cb4d525db90227bf4b7f2b43917eeab50a4e Mon Sep 17 00:00:00 2001 From: Carine Dengler Date: Tue, 7 Nov 2023 13:55:25 +0100 Subject: [PATCH 4/8] fix: no need to pass self --- src/flatfiledirmgr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/flatfiledirmgr.rs b/src/flatfiledirmgr.rs index 0f08e15..09dd903 100644 --- a/src/flatfiledirmgr.rs +++ b/src/flatfiledirmgr.rs @@ -77,7 +77,7 @@ impl FlatFileDirMgr { } /// Check cache directory content. - pub fn check_directory(&self, cache_path: &Path) -> Result<()> { + fn check_directory(cache_path: &Path) -> Result<()> { let mut any_missing = false; for filename in [ Self::CONSENSUS_FILENAME, @@ -106,7 +106,7 @@ impl FlatFileDirMgr { pub async fn load_directory(&self) -> Result { let config = self.config.get(); let cache_path = &config.cache_path; - self.check_directory(cache_path)?; + Self::check_directory(cache_path)?; // Consensus let unvalidated = self.load_consensus(cache_path)?; From 37b21d9a1ac809a216af6d279c3a2457a9309618 Mon Sep 17 00:00:00 2001 From: Carine Dengler Date: Tue, 7 Nov 2023 16:37:12 +0100 Subject: [PATCH 5/8] feat: export filenames --- src/flatfiledirmgr.rs | 33 ++++++++++++++++++--------------- src/lib.rs | 4 ++++ 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/flatfiledirmgr.rs b/src/flatfiledirmgr.rs index 09dd903..1363da3 100644 --- a/src/flatfiledirmgr.rs +++ b/src/flatfiledirmgr.rs @@ -31,6 +31,16 @@ use tor_netdir::params::NetParameters; /// 1/CHURN_FRACTION is the threshold of the consensus relays that we can remove with the churn const CHURN_FRACTION: usize = 6; +/// Contents of the directory cache. +/// CONSENSUS_FILENAME is the name of the file containing the consensus. +pub const CONSENSUS_FILENAME: &'static str = "consensus.txt"; +/// MICRODESCRIPTORS_FILENAME is the name of the file containing the microdescriptors. +pub const MICRODESCRIPTORS_FILENAME: &'static str = "microdescriptors.txt"; +/// CERTIFICATE_FILENAME is the name of the certificate. +pub const CERTIFICATE_FILENAME: &'static str = "certificate.txt"; +/// CHURN_FILENAME is the name of the churn info file. +pub const CHURN_FILENAME: &'static str = "churn.txt"; + /// A directory manager that loads the directory information from flat files read from the cache /// directory. pub struct FlatFileDirMgr { @@ -54,12 +64,6 @@ pub struct FlatFileDirMgr { } impl FlatFileDirMgr { - /// Contents of the directory cache. - const CONSENSUS_FILENAME: &'static str = "consensus.txt"; - const MICRODESCRIPTORS_FILENAME: &'static str = "microdescriptors.txt"; - const CERTIFICATE_FILENAME: &'static str = "certificate.txt"; - const CHURN_FILENAME: &'static str = "churn.txt"; - const AUTHORITY_FILENAME: &'static str = "authority.json"; /// Create a new FlatFileDirMgr from a given configuration. pub fn from_config(config: DirMgrConfig, circmgr: Arc>) -> Result> { let netdir = SharedMutArc::new(); @@ -80,11 +84,10 @@ impl FlatFileDirMgr { fn check_directory(cache_path: &Path) -> Result<()> { let mut any_missing = false; for filename in [ - Self::CONSENSUS_FILENAME, - Self::MICRODESCRIPTORS_FILENAME, - Self::CERTIFICATE_FILENAME, - Self::CHURN_FILENAME, - Self::AUTHORITY_FILENAME, + CONSENSUS_FILENAME, + MICRODESCRIPTORS_FILENAME, + CERTIFICATE_FILENAME, + CHURN_FILENAME, ] .iter() { @@ -177,12 +180,12 @@ impl FlatFileDirMgr { &self, cache_path: &Path, ) -> Result> { - let path = cache_path.join(Self::CONSENSUS_FILENAME); + let path = cache_path.join(CONSENSUS_FILENAME); let consensus_text = fs::read_to_string(path.clone()).map_err(|_| Error::UnrecognizedAuthorities)?; debug!("{} loaded", path.to_string_lossy()); - let path = cache_path.join(Self::CHURN_FILENAME); + let path = cache_path.join(CHURN_FILENAME); let churn_text = fs::read_to_string(path.clone()).unwrap_or_else(|_| "".to_string()); debug!("{} loaded", path.to_string_lossy()); @@ -225,7 +228,7 @@ impl FlatFileDirMgr { /// Load the certificate from a flat file. fn load_certificate(&self, cache_path: &Path) -> Result { - let path = cache_path.join(Self::CERTIFICATE_FILENAME); + let path = cache_path.join(CERTIFICATE_FILENAME); let certificate = fs::read_to_string(path.clone()).map_err(|_| Error::UnrecognizedAuthorities)?; debug!("{} loaded", path.to_string_lossy()); @@ -242,7 +245,7 @@ impl FlatFileDirMgr { /// Load the list of microdescriptors from a flat file. fn load_microdesc(&self, cache_path: &Path) -> Result> { - let path = cache_path.join(Self::MICRODESCRIPTORS_FILENAME); + let path = cache_path.join(MICRODESCRIPTORS_FILENAME); let udesc_text = fs::read_to_string(path.clone()).map_err(|_| Error::UnrecognizedAuthorities)?; debug!("{} loaded", path.to_string_lossy()); diff --git a/src/lib.rs b/src/lib.rs index f123838..2e93f35 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,3 +8,7 @@ mod flatfiledirmgr; mod http; pub use client::Client; +pub use flatfiledirmgr::CERTIFICATE_FILENAME; +pub use flatfiledirmgr::CHURN_FILENAME; +pub use flatfiledirmgr::CONSENSUS_FILENAME; +pub use flatfiledirmgr::MICRODESCRIPTORS_FILENAME; From 15628dbc6c5c00ae6f7183d54feda0986100cbd2 Mon Sep 17 00:00:00 2001 From: Carine Dengler Date: Tue, 7 Nov 2023 16:38:09 +0100 Subject: [PATCH 6/8] test: add tests for missing required files in directory --- src/flatfiledirmgr.rs | 8 ++++---- tests/client.rs | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/flatfiledirmgr.rs b/src/flatfiledirmgr.rs index 1363da3..144f024 100644 --- a/src/flatfiledirmgr.rs +++ b/src/flatfiledirmgr.rs @@ -33,13 +33,13 @@ const CHURN_FRACTION: usize = 6; /// Contents of the directory cache. /// CONSENSUS_FILENAME is the name of the file containing the consensus. -pub const CONSENSUS_FILENAME: &'static str = "consensus.txt"; +pub const CONSENSUS_FILENAME: &str = "consensus.txt"; /// MICRODESCRIPTORS_FILENAME is the name of the file containing the microdescriptors. -pub const MICRODESCRIPTORS_FILENAME: &'static str = "microdescriptors.txt"; +pub const MICRODESCRIPTORS_FILENAME: &str = "microdescriptors.txt"; /// CERTIFICATE_FILENAME is the name of the certificate. -pub const CERTIFICATE_FILENAME: &'static str = "certificate.txt"; +pub const CERTIFICATE_FILENAME: &str = "certificate.txt"; /// CHURN_FILENAME is the name of the churn info file. -pub const CHURN_FILENAME: &'static str = "churn.txt"; +pub const CHURN_FILENAME: &str = "churn.txt"; /// A directory manager that loads the directory information from flat files read from the cache /// directory. diff --git a/tests/client.rs b/tests/client.rs index 1b5b730..f8861b5 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -1,6 +1,10 @@ use http::request::{Builder, Parts}; use http::Request; use lightarti_rest::Client; +use lightarti_rest::CERTIFICATE_FILENAME; +use lightarti_rest::CHURN_FILENAME; +use lightarti_rest::CONSENSUS_FILENAME; +use lightarti_rest::MICRODESCRIPTORS_FILENAME; use url::Url; mod utils; @@ -85,6 +89,39 @@ async fn test_client(req: Request>) { ) } +#[tokio::test] +// Tests that no error is raised if directory is left intact. +// If this tests raises an error, please check if your local copy of directory_cache is up to date. +async fn test_required_files_ok() { + let cache = utils::setup_cache(); + let res = Client::new(cache.path()).await; + assert!(res.is_ok()); +} + +#[tokio::test] +// Tests that an error is raised by FlatFileDirMgr::check_directory if any of the required files +// are missing. The authority.json file is checked for in Client::tor_config. +async fn test_required_files_missing() { + for filename in [ + CONSENSUS_FILENAME, + MICRODESCRIPTORS_FILENAME, + CERTIFICATE_FILENAME, + CHURN_FILENAME, + ] + .iter() + { + let cache = utils::setup_cache(); + let _ = std::fs::remove_file(cache.path().join(filename)); + let res = Client::new(cache.path()).await; + let error = res.err().expect(""); + let root_cause = error.root_cause(); + assert_eq!( + format!("{}", root_cause), + "Corrupt cache: required files missing in cache" + ); + } +} + fn clone_request(header: &Parts, body: &[u8]) -> Request> { let mut builder = Builder::new() .method(header.method.clone()) From 2c4679829e23d6095afe72023ea68a20ef7c11d2 Mon Sep 17 00:00:00 2001 From: Carine Dengler Date: Tue, 7 Nov 2023 17:39:57 +0100 Subject: [PATCH 7/8] test: check if cache dir exists and contains authority.json --- src/client.rs | 20 +++++++++++++++++--- src/lib.rs | 1 + tests/client.rs | 18 +++++++++++++++++- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/client.rs b/src/client.rs index 4ee859a..16ee4c3 100644 --- a/src/client.rs +++ b/src/client.rs @@ -10,8 +10,9 @@ use tokio_rustls::{ TlsConnector, }; use tor_config::CfgPath; +use tor_dirmgr::Error; use tor_rtcompat::tokio::TokioRustlsRuntime as Runtime; -use tracing::{trace, warn}; +use tracing::{debug, trace, warn}; use crate::{ flatfiledirmgr::FlatFileDirMgrBuilder, @@ -20,9 +21,10 @@ use crate::{ /// Client using the Tor network pub struct Client(TorClient); +/// AUTHORITY_FILENAME is the name of the file containing the authorities. +pub const AUTHORITY_FILENAME: &str = "authority.json"; impl Client { - const AUTHORITY_FILENAME: &'static str = "authority.json"; /// Create a new client with the given cache directory pub async fn new(cache: &Path) -> Result { let runtime = Runtime::current().context("get runtime")?; @@ -37,14 +39,26 @@ impl Client { Ok(Self(tor_client)) } + fn check_directory(cache_path: &Path) -> Result<()> { + if !cache_path.is_dir() { + return Err(Error::CacheCorruption("directory cache does not exist").into()); + } + if !cache_path.join(AUTHORITY_FILENAME).exists() { + debug!("required file missing: {}", AUTHORITY_FILENAME); + return Err(Error::CacheCorruption("required files missing in cache").into()); + } + Ok(()) + } + fn tor_config(cache_path: &Path) -> Result { let mut cfg_builder = TorClientConfig::builder(); + Self::check_directory(cache_path)?; cfg_builder .storage() .cache_dir(CfgPath::new_literal(cache_path)) .state_dir(CfgPath::new_literal(cache_path)); - let auth_path = cache_path.join(Self::AUTHORITY_FILENAME); + let auth_path = cache_path.join(AUTHORITY_FILENAME); let auth_raw = fs::read_to_string(auth_path.clone()) .context(format!("Failed to read {}", auth_path.to_string_lossy()))?; let auth = serde_json::from_str(auth_raw.as_str())?; diff --git a/src/lib.rs b/src/lib.rs index 2e93f35..c33ef07 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,6 +8,7 @@ mod flatfiledirmgr; mod http; pub use client::Client; +pub use client::AUTHORITY_FILENAME; pub use flatfiledirmgr::CERTIFICATE_FILENAME; pub use flatfiledirmgr::CHURN_FILENAME; pub use flatfiledirmgr::CONSENSUS_FILENAME; diff --git a/tests/client.rs b/tests/client.rs index f8861b5..d71c863 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -1,6 +1,7 @@ use http::request::{Builder, Parts}; use http::Request; use lightarti_rest::Client; +use lightarti_rest::AUTHORITY_FILENAME; use lightarti_rest::CERTIFICATE_FILENAME; use lightarti_rest::CHURN_FILENAME; use lightarti_rest::CONSENSUS_FILENAME; @@ -100,13 +101,15 @@ async fn test_required_files_ok() { #[tokio::test] // Tests that an error is raised by FlatFileDirMgr::check_directory if any of the required files -// are missing. The authority.json file is checked for in Client::tor_config. +// are missing. The authority.json file is checked for in Client::check_directory since it is used +// before the other files are read in. async fn test_required_files_missing() { for filename in [ CONSENSUS_FILENAME, MICRODESCRIPTORS_FILENAME, CERTIFICATE_FILENAME, CHURN_FILENAME, + AUTHORITY_FILENAME, ] .iter() { @@ -122,6 +125,19 @@ async fn test_required_files_missing() { } } +#[tokio::test] +async fn test_directory_not_existing() { + let cache = utils::setup_cache(); + let _ = std::fs::remove_dir_all(cache.path()); + let res = Client::new(cache.path()).await; + let error = res.err().expect(""); + let root_cause = error.root_cause(); + assert_eq!( + format!("{}", root_cause), + "Corrupt cache: directory cache does not exist" + ); +} + fn clone_request(header: &Parts, body: &[u8]) -> Request> { let mut builder = Builder::new() .method(header.method.clone()) From 5b0d808742af68620b041cb7acfde4133446b1bf Mon Sep 17 00:00:00 2001 From: Carine Dengler Date: Wed, 8 Nov 2023 15:11:56 +0100 Subject: [PATCH 8/8] fix: apply code review suggestions --- src/client.rs | 2 +- src/flatfiledirmgr.rs | 16 ++++++---------- tests/client.rs | 2 +- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/client.rs b/src/client.rs index 16ee4c3..ea1d2f2 100644 --- a/src/client.rs +++ b/src/client.rs @@ -45,7 +45,7 @@ impl Client { } if !cache_path.join(AUTHORITY_FILENAME).exists() { debug!("required file missing: {}", AUTHORITY_FILENAME); - return Err(Error::CacheCorruption("required files missing in cache").into()); + return Err(Error::CacheCorruption("required file(s) missing in cache").into()); } Ok(()) } diff --git a/src/flatfiledirmgr.rs b/src/flatfiledirmgr.rs index 144f024..dbff74f 100644 --- a/src/flatfiledirmgr.rs +++ b/src/flatfiledirmgr.rs @@ -82,22 +82,18 @@ impl FlatFileDirMgr { /// Check cache directory content. fn check_directory(cache_path: &Path) -> Result<()> { - let mut any_missing = false; - for filename in [ + let missing_files: Vec<&&str> = [ CONSENSUS_FILENAME, MICRODESCRIPTORS_FILENAME, CERTIFICATE_FILENAME, CHURN_FILENAME, ] .iter() - { - if !cache_path.join(filename).exists() { - any_missing = true; - debug!("required file missing: {filename}"); - } - } - if any_missing { - return Err(Error::CacheCorruption("required files missing in cache")); + .filter(|filename| !cache_path.join(filename).exists()) + .collect(); + if !missing_files.is_empty() { + debug!("required file(s) missing: {missing_files:?}"); + return Err(Error::CacheCorruption("required file(s) missing in cache")); } Ok(()) } diff --git a/tests/client.rs b/tests/client.rs index d71c863..1a4508a 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -120,7 +120,7 @@ async fn test_required_files_missing() { let root_cause = error.root_cause(); assert_eq!( format!("{}", root_cause), - "Corrupt cache: required files missing in cache" + "Corrupt cache: required file(s) missing in cache" ); } }