tor-config source: just ConfigurationSource, not FoundConfigFile

FoundConfigFile existed to hide something that ConfigurationSource now
exposes.
This commit is contained in:
Ian Jackson 2022-08-24 19:43:48 +01:00
parent 9c00ec7da4
commit 2662fd0d71
2 changed files with 28 additions and 68 deletions

View File

@ -9,7 +9,7 @@ use std::time::Duration;
use arti_client::config::Reconfigure; use arti_client::config::Reconfigure;
use arti_client::TorClient; use arti_client::TorClient;
use notify::Watcher; use notify::Watcher;
use tor_config::{sources::FoundConfigFiles, ConfigurationSources}; use tor_config::{sources::FoundConfigFiles, ConfigurationSource, ConfigurationSources};
use tor_rtcompat::Runtime; use tor_rtcompat::Runtime;
use tracing::{debug, error, info, warn}; use tracing::{debug, error, info, warn};
@ -26,15 +26,14 @@ fn prepare_watcher(
sources: &ConfigurationSources, sources: &ConfigurationSources,
) -> anyhow::Result<(FileWatcher, FoundConfigFiles)> { ) -> anyhow::Result<(FileWatcher, FoundConfigFiles)> {
let mut watcher = FileWatcher::new(DEBOUNCE_INTERVAL)?; let mut watcher = FileWatcher::new(DEBOUNCE_INTERVAL)?;
let files = sources.scan()?; let sources = sources.scan()?;
for file in files.iter() { for source in sources.iter() {
if file.was_dir() { match source {
watcher.watch_dir(file)?; ConfigurationSource::Dir(dir) => watcher.watch_dir(dir)?,
} else { ConfigurationSource::File(file) => watcher.watch_file(file)?,
watcher.watch_file(file)?;
} }
} }
Ok((watcher, files)) Ok((watcher, sources))
} }
/// Launch a thread to watch our configuration files. /// Launch a thread to watch our configuration files.

View File

@ -133,45 +133,12 @@ pub struct FoundConfigFiles<'srcs> {
/// A configuration source file or directory, found or not found on the filesystem /// A configuration source file or directory, found or not found on the filesystem
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct FoundConfigFile { struct FoundConfigFile {
/// The path of the (putative) object /// The path of the (putative) object
path: PathBuf, source: ConfigurationSource,
/// Were we expecting this to definitely exist /// Were we expecting this to definitely exist
must_read: MustRead, must_read: MustRead,
/// What happened when we looked for it
ty: FoundType,
}
/// Was this filesystem object a file or a directory?
#[derive(Debug, Copy, Clone)]
enum FoundType {
/// File
File,
/// Directory
Dir,
}
impl FoundConfigFile {
/// Get the path
pub fn path(&self) -> &Path {
&self.path
}
/// Was this a directory, when we found it ?
pub fn was_dir(&self) -> bool {
match self.ty {
FoundType::Dir => true,
FoundType::File => false,
}
}
}
impl AsRef<Path> for FoundConfigFile {
fn as_ref(&self) -> &Path {
self.path()
}
} }
impl ConfigurationSources { impl ConfigurationSources {
@ -268,7 +235,7 @@ impl ConfigurationSources {
pub fn scan(&self) -> Result<FoundConfigFiles, ConfigError> { pub fn scan(&self) -> Result<FoundConfigFiles, ConfigError> {
let mut out = vec![]; let mut out = vec![];
for &(ref found, must_read) in &self.files { for &(ref source, must_read) in &self.files {
let required = must_read == MustRead::MustRead; let required = must_read == MustRead::MustRead;
// Returns Err(error) if we shuold bail, // Returns Err(error) if we shuold bail,
@ -280,7 +247,7 @@ impl ConfigurationSources {
Err(ConfigError::Foreign( Err(ConfigError::Foreign(
anyhow::anyhow!(format!( anyhow::anyhow!(format!(
"unable to access config path: {:?}: {}", "unable to access config path: {:?}: {}",
&found.as_path(), &source.as_path(),
e e
)) ))
.into(), .into(),
@ -289,7 +256,7 @@ impl ConfigurationSources {
}; };
use ConfigurationSource as CS; use ConfigurationSource as CS;
match &found { match &source {
CS::Dir(found) => { CS::Dir(found) => {
let dir = match fs::read_dir(&found) { let dir = match fs::read_dir(&found) {
Ok(y) => y, Ok(y) => y,
@ -299,8 +266,7 @@ impl ConfigurationSources {
} }
}; };
out.push(FoundConfigFile { out.push(FoundConfigFile {
path: found.clone(), source: source.clone(),
ty: FoundType::Dir,
must_read, must_read,
}); });
// Rebinding `found` avoids using the directory name by mistake. // Rebinding `found` avoids using the directory name by mistake.
@ -325,16 +291,14 @@ impl ConfigurationSources {
} }
entries.sort(); entries.sort();
out.extend(entries.into_iter().map(|path| FoundConfigFile { out.extend(entries.into_iter().map(|path| FoundConfigFile {
path, source: CS::File(path),
must_read: MustRead::TolerateAbsence, must_read: MustRead::TolerateAbsence,
ty: FoundType::File,
})); }));
} }
CS::File(found) => { CS::File(_) => {
out.push(FoundConfigFile { out.push(FoundConfigFile {
path: found.clone(), source: source.clone(),
must_read, must_read,
ty: FoundType::File,
}); });
} }
} }
@ -351,32 +315,29 @@ impl FoundConfigFiles<'_> {
/// Iterate over the filesystem objects that the scan found /// Iterate over the filesystem objects that the scan found
// //
// This ought really to be `impl IntoIterator for &Self` but that's awkward without TAIT // This ought really to be `impl IntoIterator for &Self` but that's awkward without TAIT
pub fn iter(&self) -> impl Iterator<Item = &FoundConfigFile> { pub fn iter(&self) -> impl Iterator<Item = &ConfigurationSource> {
self.files.iter() self.files.iter().map(|f| &f.source)
} }
/// Add every file and commandline source to `builder`, returning a new /// Add every file and commandline source to `builder`, returning a new
/// builder. /// builder.
fn add_sources(self, mut builder: ConfigBuilder) -> Result<ConfigBuilder, ConfigError> { fn add_sources(self, mut builder: ConfigBuilder) -> Result<ConfigBuilder, ConfigError> {
for FoundConfigFile { for FoundConfigFile { source, must_read } in self.files {
path, use ConfigurationSource as CS;
must_read,
ty,
} in self.files
{
let required = must_read == MustRead::MustRead; let required = must_read == MustRead::MustRead;
match ty { let file = match source {
FoundType::File => {} CS::File(file) => file,
FoundType::Dir => continue, CS::Dir(_) => continue,
} };
match self match self
.sources .sources
.mistrust .mistrust
.verifier() .verifier()
.permit_readable() .permit_readable()
.check(&path) .check(&file)
{ {
Ok(()) => {} Ok(()) => {}
Err(fs_mistrust::Error::NotFound(_)) if !required => {} Err(fs_mistrust::Error::NotFound(_)) if !required => {}
@ -385,7 +346,7 @@ impl FoundConfigFiles<'_> {
// Not going to use File::with_name here, since it doesn't // Not going to use File::with_name here, since it doesn't
// quite do what we want. // quite do what we want.
let f: config::File<_, _> = path.into(); let f: config::File<_, _> = file.into();
builder = builder.add_source(f.format(config::FileFormat::Toml).required(required)); builder = builder.add_source(f.format(config::FileFormat::Toml).required(required));
} }
@ -521,7 +482,7 @@ world = \"nonsense\"
assert_eq!( assert_eq!(
found found
.iter() .iter()
.map(|p| p.path().strip_prefix(&td).unwrap().to_str().unwrap()) .map(|p| p.as_path().strip_prefix(&td).unwrap().to_str().unwrap())
.collect_vec(), .collect_vec(),
&["1.toml", "extra.d", "extra.d/2.toml"] &["1.toml", "extra.d", "extra.d/2.toml"]
); );