From dbc7ddadb705f6ca525d7d4e00c0e9f0d92a023d Mon Sep 17 00:00:00 2001 From: Dmitriy Pleshevskiy Date: Mon, 22 Feb 2021 23:06:08 +0300 Subject: [PATCH] refac: made the error object simpler --- migra-cli/src/commands/list.rs | 8 +-- migra-cli/src/config.rs | 46 ++++++++--------- migra-cli/src/error.rs | 93 +++++++--------------------------- migra-cli/tests/commands.rs | 8 +-- 4 files changed, 47 insertions(+), 108 deletions(-) diff --git a/migra-cli/src/commands/list.rs b/migra-cli/src/commands/list.rs index dd4bf6f..887ad50 100644 --- a/migra-cli/src/commands/list.rs +++ b/migra-cli/src/commands/list.rs @@ -1,6 +1,6 @@ use crate::config::Config; use crate::databases::*; -use crate::error::{ErrorKind, StdResult}; +use crate::error::{Error, StdResult}; use crate::migration::{ filter_pending_migrations, DatabaseMigrationManager, Migration, MigrationManager, }; @@ -20,9 +20,9 @@ pub(crate) fn print_migration_lists(config: Config) -> StdResult<()> { applied_migration_names } - Err(e) if *e.kind() == ErrorKind::MissedEnvVar(String::new()) => { - println!("{}", e.kind()); - println!("No connection to database"); + Err(e) if e == Error::MissedEnvVar(String::new()) => { + eprintln!("WARNING: {}", e); + eprintln!("WARNING: No connection to database"); Vec::new() } diff --git a/migra-cli/src/config.rs b/migra-cli/src/config.rs index 0724f39..a9e0494 100644 --- a/migra-cli/src/config.rs +++ b/migra-cli/src/config.rs @@ -1,4 +1,4 @@ -use crate::error::{Error, ErrorKind}; +use crate::error::{Error, MigraResult}; use crate::migration::Migration; use crate::path::PathBuilder; use serde::{Deserialize, Serialize}; @@ -31,18 +31,17 @@ pub(crate) struct DatabaseConfig { } impl DatabaseConfig { - pub fn client(&self) -> crate::error::Result { + pub fn client(&self) -> MigraResult { Ok(SupportedDatabaseClient::Postgres) } - pub fn connection_string(&self) -> crate::error::Result { + pub fn connection_string(&self) -> MigraResult { let connection = self .connection .clone() .unwrap_or_else(|| String::from(DEFAULT_DATABASE_CONNECTION_ENV)); if let Some(connection_env) = connection.strip_prefix("$") { - env::var(connection_env) - .map_err(|e| Error::new(ErrorKind::MissedEnvVar(connection_env.to_string()), e)) + env::var(connection_env).map_err(|_| Error::MissedEnvVar(connection_env.to_string())) } else { Ok(connection) } @@ -62,35 +61,32 @@ impl Default for Config { } } -fn recursive_find_config_file() -> io::Result { - let current_dir = std::env::current_dir()?; - - let mut read_dir = Some(current_dir.as_path()); - - loop { - if let Some(dir) = read_dir { - let migra_file_path = PathBuilder::from(dir).append(MIGRA_TOML_FILENAME).build(); - if !migra_file_path.exists() { - read_dir = dir.parent(); - continue; - } - - return Ok(migra_file_path); - } else { - return Err(io::Error::from(io::ErrorKind::NotFound)); - } +fn search_for_directory_containing_file(path: &Path, file_name: &str) -> MigraResult { + let file_path = path.join(file_name); + if file_path.is_file() { + Ok(path.to_owned()) + } else { + path.parent() + .ok_or(Error::RootNotFound) + .and_then(|p| search_for_directory_containing_file(p, file_name)) } } +fn recursive_find_project_root() -> MigraResult { + let current_dir = std::env::current_dir()?; + + search_for_directory_containing_file(¤t_dir, MIGRA_TOML_FILENAME) +} + impl Config { - pub fn read(config_path: Option) -> io::Result { + pub fn read(config_path: Option) -> MigraResult { let config_path = match config_path { Some(mut config_path) if config_path.is_dir() => { config_path.push(MIGRA_TOML_FILENAME); Some(config_path) } Some(config_path) => Some(config_path), - None => recursive_find_config_file().ok(), + None => recursive_find_project_root().ok(), }; match config_path { @@ -123,7 +119,7 @@ impl Config { .build() } - pub fn migrations(&self) -> io::Result> { + pub fn migrations(&self) -> MigraResult> { let mut entries = match self.migration_dir_path().read_dir() { Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(Vec::new()), entries => entries? diff --git a/migra-cli/src/error.rs b/migra-cli/src/error.rs index 847ec2f..e97e7d5 100644 --- a/migra-cli/src/error.rs +++ b/migra-cli/src/error.rs @@ -1,102 +1,43 @@ -use std::error::Error as StdError; +use std::error; use std::fmt; +use std::io; use std::mem; use std::result; pub type StdResult = result::Result>; -pub type Result = result::Result; +pub type MigraResult = result::Result; #[derive(Debug)] -pub struct Error { - repr: Repr, -} - -enum Repr { - Simple(ErrorKind), - Custom(Box), -} - -#[derive(Debug)] -struct Custom { - kind: ErrorKind, - error: Box, -} - -#[derive(Debug, Clone)] -pub enum ErrorKind { +pub enum Error { + RootNotFound, MissedEnvVar(String), + + IoError(io::Error), } -impl fmt::Display for ErrorKind { +impl fmt::Display for Error { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { - ErrorKind::MissedEnvVar(ref name) => { + Error::RootNotFound => fmt.write_str("Cannot find root directory"), + Error::MissedEnvVar(ref name) => { write!(fmt, r#"Missed "{}" environment variable"#, name) } + Error::IoError(ref error) => write!(fmt, "{}", error), } } } -impl PartialEq for ErrorKind { +impl error::Error for Error {} + +impl PartialEq for Error { fn eq(&self, other: &Self) -> bool { mem::discriminant(self) == mem::discriminant(other) } } -impl From for Error { +impl From for Error { #[inline] - fn from(kind: ErrorKind) -> Error { - Error { - repr: Repr::Simple(kind), - } - } -} - -impl Error { - pub fn new(kind: ErrorKind, error: E) -> Error - where - E: Into>, - { - Self::_new(kind, error.into()) - } - - fn _new(kind: ErrorKind, error: Box) -> Error { - Error { - repr: Repr::Custom(Box::new(Custom { kind, error })), - } - } - - pub fn kind(&self) -> &ErrorKind { - match &self.repr { - Repr::Custom(ref c) => &c.kind, - Repr::Simple(kind) => &kind, - } - } -} - -impl fmt::Debug for Repr { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Repr::Custom(ref c) => fmt::Debug::fmt(&c, fmt), - Repr::Simple(kind) => fmt.debug_tuple("Kind").field(&kind).finish(), - } - } -} - -impl fmt::Display for Error { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - match &self.repr { - Repr::Custom(ref c) => c.error.fmt(fmt), - Repr::Simple(kind) => write!(fmt, "{}", kind), - } - } -} - -impl StdError for Error { - fn source(&self) -> Option<&(dyn StdError + 'static)> { - match self.repr { - Repr::Simple(..) => None, - Repr::Custom(ref c) => c.error.source(), - } + fn from(err: io::Error) -> Error { + Error::IoError(err) } } diff --git a/migra-cli/tests/commands.rs b/migra-cli/tests/commands.rs index 6e6a1b1..796574b 100644 --- a/migra-cli/tests/commands.rs +++ b/migra-cli/tests/commands.rs @@ -98,10 +98,12 @@ mod list { .arg("ls") .assert() .success() + .stderr(contains( + r#"WARNING: Missed "DATABASE_URL" environment variable +WARNING: No connection to database"#, + )) .stdout(contains( - r#"Missed "DATABASE_URL" environment variable -No connection to database - + r#" Pending migrations: —"#, ));