From 76373278b010482dd73d6c9ece9a8f29fee51395 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 24 Jul 2021 22:25:40 +0200 Subject: [PATCH] Remove `Source` trait and `Config::from_sources` I'm not sure about a good high level API. --- examples/simple.rs | 14 ++++---- src/error.rs | 22 +++++++++---- src/file.rs | 82 ++++++++++++++++++++++------------------------ src/lib.rs | 22 ------------- 4 files changed, 63 insertions(+), 77 deletions(-) diff --git a/examples/simple.rs b/examples/simple.rs index 8ed4fec..1abea80 100644 --- a/examples/simple.rs +++ b/examples/simple.rs @@ -1,4 +1,4 @@ -use std::{net::IpAddr, path::{Path, PathBuf}}; +use std::{net::IpAddr, path::PathBuf}; use confique::{Config, toml::FormatOptions}; #[derive(Debug, Config)] @@ -41,13 +41,13 @@ fn main() -> Result<(), anyhow::Error> { print!("{}", confique::toml::format::(FormatOptions::default())); println!("--------------------------------------------------------"); - let r = Conf::from_sources(&[ - &Path::new("examples/files/simple.toml"), - &Path::new("examples/files/etc/simple.yaml"), - ])?; + // let r = Conf::from_sources(&[ + // &Path::new("examples/files/simple.toml"), + // &Path::new("examples/files/etc/simple.yaml"), + // ])?; - println!(); - println!("LOADED CONFIGURATION: {:#?}", r); + // println!(); + // println!("LOADED CONFIGURATION: {:#?}", r); Ok(()) } diff --git a/src/error.rs b/src/error.rs index 2e82d7e..3c9d00d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,4 @@ -use std::{ffi::OsString, fmt, path::PathBuf}; +use std::{fmt, path::PathBuf}; /// Type describing all errors that can occur in this library. @@ -33,13 +33,17 @@ pub(crate) enum ErrorInner { /// feature of confique was not enabled. UnsupportedFileFormat { path: PathBuf, - extension: OsString, }, /// Returned by the [`Source`] impls for `Path` and `PathBuf` if the path /// does not contain a file extension. MissingFileExtension { path: PathBuf, + }, + + /// A file source was marked as required but the file does not exist. + MissingRequiredFile { + path: PathBuf, } } @@ -50,7 +54,8 @@ impl std::error::Error for Error { ErrorInner::Deserialization { err, .. } => Some(&**err), ErrorInner::MissingValue(_) | ErrorInner::UnsupportedFileFormat { .. } - | ErrorInner::MissingFileExtension { .. } => None, + | ErrorInner::MissingFileExtension { .. } + | ErrorInner::MissingRequiredFile { .. } => None, } } } @@ -76,10 +81,9 @@ impl fmt::Display for Error { ErrorInner::Deserialization { source: None, .. } => { std::write!(f, "failed to deserialize configuration") } - ErrorInner::UnsupportedFileFormat { path, extension } => { + ErrorInner::UnsupportedFileFormat { path } => { std::write!(f, - "unknown configuration file format '{}' of '{}'", - extension.to_string_lossy(), + "unknown configuration file format/extension: '{}'", path.display(), ) } @@ -89,6 +93,12 @@ impl fmt::Display for Error { path.display(), ) } + ErrorInner::MissingRequiredFile { path } => { + std::write!(f, + "required configuration file does not exist: '{}'", + path.display(), + ) + } } } } diff --git a/src/file.rs b/src/file.rs index 72e4f4f..6a74ecd 100644 --- a/src/file.rs +++ b/src/file.rs @@ -1,33 +1,12 @@ -use std::{ffi::OsStr, fs, io, path::{Path, PathBuf}}; +use std::{ffi::OsStr, fs, io, path::PathBuf}; -use crate::{Config, Error, Partial, Source, error::ErrorInner}; +use crate::{Error, Partial, error::ErrorInner}; -impl Source for &Path { - fn load(&self) -> Result { - let ext = self.extension().ok_or_else(|| { - ErrorInner::MissingFileExtension { path: self.into() } - })?; - let format = FileFormat::from_extension(ext).ok_or_else(|| { - ErrorInner::UnsupportedFileFormat { extension: ext.into(), path: self.into() } - })?; - - >::load(&File::new(self, format)) - } -} - -impl Source for PathBuf { - fn load(&self) -> Result { - <&Path as Source>::load(&&**self) - } -} - /// A file as source for configuration. /// -/// Most of the time, you can problably use the [`Source`] impl for -/// `Path`/`PathBuf`, but this type gives you more control. For one, you can -/// explicitly set the file format. You can also mark a file as required, -/// meaning that an error will be returned if the file does not exist. +/// By default, the file is considered optional, meaning that on [`File::load`], +/// if the file does not exist, `Partial::empty()` is returned. pub struct File { path: PathBuf, format: FileFormat, @@ -35,8 +14,23 @@ pub struct File { } impl File { - /// Creates an optional file source. - pub fn new(path: impl Into, format: FileFormat) -> Self { + /// Configuration file with the given path. The format is inferred by the + /// file extension. If the path does not have an extension or it is + /// unknown, an error is returned. + pub fn new(path: impl Into) -> Result { + let path = path.into(); + let ext = path.extension().ok_or_else(|| { + ErrorInner::MissingFileExtension { path: path.clone() } + })?; + let format = FileFormat::from_extension(ext).ok_or_else(|| { + ErrorInner::UnsupportedFileFormat { path: path.clone() } + })?; + + Ok(Self::with_format(path, format)) + } + + /// Config file with specified file format. + pub fn with_format(path: impl Into, format: FileFormat) -> Self { Self { path: path.into(), format, @@ -44,30 +38,34 @@ impl File { } } - /// Marks this file as required, meaning that `>::load` - /// will return an error if the file does not exist. Otherwise, an empty - /// layer (all values are `None`) is returned. + /// Marks this file as required, meaning that [`File::load`] will return an + /// error if the file does not exist. Otherwise, an empty layer (all values + /// are `None`) is returned. pub fn required(mut self) -> Self { self.required = true; self } -} -impl Source for File { - // Unfortunately, if no file format is enabled, this emits unused variable - // warnings. This should not happen as `self`, a type containing an empty - // enum, is in scope, meaning that the code cannot be reached. - #[cfg_attr( - not(any(feature = "toml", feature = "yaml")), - allow(unused_variables), - )] - fn load(&self) -> Result { + /// Attempts to load the file into the partial configuration `P`. + pub fn load(&self) -> Result { + // Unfortunately, if no file format is enabled, this emits unused variable + // warnings. This should not happen as `self`, a type containing an empty + // enum, is in scope, meaning that the code cannot be reached. + #![cfg_attr( + not(any(feature = "toml", feature = "yaml")), + allow(unused_variables), + )] + // Load file contents. If the file does not exist and was not marked as // required, we just return an empty layer. let file_content = match fs::read(&self.path) { Ok(v) => v, - Err(e) if e.kind() == io::ErrorKind::NotFound && !self.required => { - return Ok(C::Partial::empty()); + Err(e) if e.kind() == io::ErrorKind::NotFound => { + if self.required { + return Err(ErrorInner::MissingRequiredFile { path: self.path.clone() }.into()); + } else { + return Ok(P::empty()); + } } Err(e) => { return Err(ErrorInner::Io { diff --git a/src/lib.rs b/src/lib.rs index 6fe5a95..9b4f63a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -55,21 +55,6 @@ pub trait Config: Sized { /// If any required values are not defined in `partial`, an [`Error`] is /// returned. fn from_partial(partial: Self::Partial) -> Result; - - /// Tries to load configuration values from all given sources, merging all - /// layers and returning the result. Sources earlier in the given slice have - /// a higher priority. - /// - /// TODO: example - fn from_sources(sources: &[&dyn Source]) -> Result { - let mut partial = Self::Partial::default_values(); - for src in sources.iter().rev() { - let layer = src.load()?; - partial = layer.with_fallback(partial); - } - - Self::from_partial(partial) - } } /// A potentially partial configuration object that can be directly deserialized @@ -89,10 +74,3 @@ pub trait Partial: for<'de> Deserialize<'de> { /// [`Option::or`]. fn with_fallback(self, fallback: Self) -> Self; } - -/// A source of configuration values for the configuration object `C`, e.g. a -/// file or environment variables. -pub trait Source { - /// Attempts to load a potentially partially configuration object. - fn load(&self) -> Result; -}