diff --git a/Cargo.lock b/Cargo.lock index feefb19e..b9cfa970 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -879,6 +879,27 @@ dependencies = [ "zeroize", ] +[[package]] +name = "derive_more" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "093242cf7570c207c83073cf82f79706fe7b8317e98620a47d5be7c3d8497678" +dependencies = [ + "derive_more-impl", +] + +[[package]] +name = "derive_more-impl" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bda628edc44c4bb645fbe0f758797143e4e07926f7ebf4e9bdfbd3d2ce621df3" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.95", + "unicode-xid", +] + [[package]] name = "digest" version = "0.10.7" @@ -2022,6 +2043,7 @@ dependencies = [ "built", "chrono", "clap", + "derive_more", "futures", "httpmock", "opentelemetry", @@ -3624,6 +3646,12 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e70f2a8b45122e719eb623c01822704c4e0907e7e426a05927e1a1cfff5b75d0" +[[package]] +name = "unicode-xid" +version = "0.2.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" + [[package]] name = "untrusted" version = "0.9.0" diff --git a/Cargo.toml b/Cargo.toml index d51a832d..c3b4be6f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ axum = "0.8.1" axum-extra = { version = "0.10.0", features = ["typed-header"] } chrono = "0.4.39" clap = { version = "4.5.30", features = ["cargo", "derive", "env", "string"] } +derive_more = { version = "2.0.1", features = ["error", "display", "from", "deref"] } futures = "0.3.31" opentelemetry = "0.28.0" opentelemetry-otlp = { version = "0.28.0", features = ["grpc-tonic"] } diff --git a/src/db_service.rs b/src/db_service.rs index 9aaed470..75eb632c 100644 --- a/src/db_service.rs +++ b/src/db_service.rs @@ -374,66 +374,30 @@ impl fmt::Debug for SqliteScanPathService { } mod error { - use std::error::Error; - use std::fmt::{self, Display}; + use derive_more::{Display, Error, From}; - #[derive(Debug)] + #[derive(Debug, Display, Error, From)] pub enum ConfigurationError { - MissingBeamline(String), + #[display("No configuration available for beamline {_0:?}")] + MissingBeamline(#[error(ignore)] String), + #[display("Error reading configuration: {_0}")] Db(sqlx::Error), } - impl Display for ConfigurationError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ConfigurationError::MissingBeamline(bl) => { - write!(f, "No configuration available for beamline {bl:?}") - } - ConfigurationError::Db(e) => write!(f, "Error reading configuration: {e}"), - } - } - } - - impl Error for ConfigurationError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - match self { - ConfigurationError::MissingBeamline(_) => None, - ConfigurationError::Db(e) => Some(e), - } - } - } - - impl From for ConfigurationError { - fn from(value: sqlx::Error) -> Self { - Self::Db(value) - } - } - - #[derive(Debug)] + #[derive(Debug, Display, From)] pub enum NewConfigurationError { + #[display("Missing field {_0:?} for new configuration")] MissingField(String), + #[from] + #[display("Error inserting new configuration: {_0}")] Db(sqlx::Error), } - impl Display for NewConfigurationError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - NewConfigurationError::MissingField(name) => { - write!(f, "Missing field {name:?} for new configuration") - } - NewConfigurationError::Db(e) => write!(f, "Error inserting new configuration: {e}"), - } - } - } + impl From<&str> for NewConfigurationError { fn from(value: &str) -> Self { Self::MissingField(value.into()) } } - impl From for NewConfigurationError { - fn from(value: sqlx::Error) -> Self { - Self::Db(value) - } - } } #[cfg(test)] diff --git a/src/graphql.rs b/src/graphql.rs index a6ab5358..39dab3cc 100644 --- a/src/graphql.rs +++ b/src/graphql.rs @@ -14,7 +14,6 @@ use std::any; use std::borrow::Cow; -use std::error::Error; use std::fmt::Display; use std::future::Future; use std::io::Write; @@ -38,6 +37,7 @@ use axum_extra::headers::authorization::Bearer; use axum_extra::headers::Authorization; use axum_extra::TypedHeader; use chrono::{Datelike, Local}; +use derive_more::{Display, Error}; use tokio::net::TcpListener; use tracing::{info, instrument, trace, warn}; @@ -156,7 +156,8 @@ struct CurrentConfiguration { } /// Error to be returned when a path contains non-unicode characters -#[derive(Debug)] +#[derive(Debug, Display, Error)] +#[display("Path contains non-unicode characters")] struct NonUnicodePath; /// Try and convert a path to a string (via `OsString`), returning a `NonUnicodePath` @@ -167,14 +168,6 @@ fn path_to_string(path: PathBuf) -> Result { .map_err(|_| NonUnicodePath) } -impl Display for NonUnicodePath { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("Path contains non-unicode characters") - } -} - -impl Error for NonUnicodePath {} - #[Object] /// The path to a visit directory and the components used to build it impl VisitPath { @@ -556,12 +549,14 @@ where /// Name of subdirectory within visit directory where data should be written. /// Can be nested (eg foo/bar) but cannot include links to parent directories (eg ../foo). // Derived Default is OK without validation as empty path is a valid subdirectory -#[derive(Debug, Default)] +#[derive(Debug, Display, Default)] pub struct Subdirectory(String); -#[derive(Debug)] +#[derive(Debug, Display, Error)] pub enum InvalidSubdirectory { - InvalidComponent(usize), + #[display("Segment {_0} of path is not valid for a subdirectory")] + InvalidComponent(#[error(ignore)] usize), + #[display("Subdirectory cannot be absolute")] AbsolutePath, } @@ -596,25 +591,6 @@ impl ScalarType for Subdirectory { } } -impl Display for InvalidSubdirectory { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - InvalidSubdirectory::InvalidComponent(s) => { - write!(f, "Segment {s} of path is not valid for a subdirectory") - } - InvalidSubdirectory::AbsolutePath => f.write_str("Subdirectory cannot be absolute"), - } - } -} - -impl Error for InvalidSubdirectory {} - -impl Display for Subdirectory { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.0.fmt(f) - } -} - /// Detector name #[derive(Debug)] pub struct Detector(String); diff --git a/src/graphql/auth.rs b/src/graphql/auth.rs index 86da23a6..9800a423 100644 --- a/src/graphql/auth.rs +++ b/src/graphql/auth.rs @@ -12,11 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::fmt::Display; use std::str::FromStr; use axum_extra::headers::authorization::Bearer; use axum_extra::headers::Authorization; +use derive_more::{Display, Error, From}; use serde::{Deserialize, Serialize}; use tracing::info; @@ -162,38 +162,16 @@ impl PolicyCheck { } } -#[derive(Debug)] +#[derive(Debug, Display, Error, From)] pub enum AuthError { + #[display("Invalid authorization configuration")] ServerError(reqwest::Error), + #[display("Authentication failed")] Failed, + #[display("No authentication token was provided")] Missing, } -impl Display for AuthError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - AuthError::ServerError(_) => write!(f, "Invalid authorization configuration"), - AuthError::Failed => write!(f, "Authentication failed"), - AuthError::Missing => f.write_str("No authentication token was provided"), - } - } -} - -impl std::error::Error for AuthError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - AuthError::ServerError(e) => Some(e), - _ => None, - } - } -} - -impl From for AuthError { - fn from(value: reqwest::Error) -> Self { - Self::ServerError(value) - } -} - #[cfg(test)] mod tests { use std::str::FromStr as _; diff --git a/src/numtracker.rs b/src/numtracker.rs index 1a6eb4c3..be7da055 100644 --- a/src/numtracker.rs +++ b/src/numtracker.rs @@ -13,10 +13,10 @@ // limitations under the License. use std::collections::HashMap; -use std::fmt::{self, Display}; use std::io::Error; use std::path::{Path, PathBuf}; +use derive_more::{Display, Error}; #[cfg(test)] pub use tests::TempTracker; use tokio::fs as async_fs; @@ -162,24 +162,17 @@ impl GdaNumTracker<'_, '_> { } /// Error returned when an extension would result in directory traversal - eg '.foo/../../bar' -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Display, Error, Clone, Copy)] +#[display("Extension is not valid")] pub struct InvalidExtension; -impl Display for InvalidExtension { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("Extension is not valid") - } -} - -impl std::error::Error for InvalidExtension {} - #[cfg(test)] mod tests { - use std::ops::Deref; use std::path::Path; use std::time::Duration; use std::{fs, io}; + use derive_more::Deref; use rstest::{fixture, rstest}; use tempfile::{tempdir, TempDir}; use tokio::time::timeout; @@ -187,7 +180,8 @@ mod tests { use super::{InvalidExtension, NumTracker}; /// Wrapper around a NumTracker to ensure the tempdir is not dropped while it is still required - pub struct TempTracker(pub NumTracker, pub TempDir); + #[derive(Deref)] + pub struct TempTracker(#[deref] pub NumTracker, pub TempDir); impl TempTracker { pub fn new(init: F) -> Self where @@ -198,13 +192,6 @@ mod tests { Self(NumTracker::for_root_directory(Some(&root)).unwrap(), root) } } - impl Deref for TempTracker { - type Target = NumTracker; - - fn deref(&self) -> &Self::Target { - &self.0 - } - } #[rstest::fixture] fn root() -> TempDir { diff --git a/src/paths.rs b/src/paths.rs index d775a7e4..df0dff9e 100644 --- a/src/paths.rs +++ b/src/paths.rs @@ -13,73 +13,46 @@ // limitations under the License. use std::collections::HashSet; -use std::error::Error; -use std::fmt::{self, Debug, Display}; +use std::fmt::{Debug, Display}; use std::hash::Hash; +use derive_more::{Display, Error, From}; + use crate::template::{PathTemplate, PathTemplateError}; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Display, Clone, Copy, PartialEq, Eq, Hash)] pub enum BeamlineField { + #[display("year")] Year, + #[display("visit")] Visit, + #[display("proposal")] Proposal, + #[display("instrument")] Instrument, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Display, Clone, Copy, PartialEq, Eq, Hash)] pub enum ScanField { + #[display("subdirectory")] Subdirectory, + #[display("scan_number")] ScanNumber, + #[display("{_0}")] Beamline(BeamlineField), } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Display, Clone, Copy, PartialEq, Eq, Hash)] pub enum DetectorField { + #[display("detector")] Detector, + #[display("{_0}")] Scan(ScanField), } -impl Display for BeamlineField { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - BeamlineField::Year => f.write_str("year"), - BeamlineField::Visit => f.write_str("visit"), - BeamlineField::Proposal => f.write_str("proposal"), - BeamlineField::Instrument => f.write_str("instrument"), - } - } -} - -impl Display for ScanField { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ScanField::Subdirectory => f.write_str("subdirectory"), - ScanField::ScanNumber => f.write_str("scan_number"), - ScanField::Beamline(bl) => write!(f, "{bl}"), - } - } -} - -impl Display for DetectorField { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - DetectorField::Detector => f.write_str("detector"), - DetectorField::Scan(sc) => write!(f, "{sc}"), - } - } -} - -#[derive(Debug)] -pub struct InvalidKey(String); - -impl Display for InvalidKey { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "Unrecognised key: {}", self.0) - } -} - -impl Error for InvalidKey {} +#[derive(Debug, Display, Error)] +#[display("Unrecognised key: {_0:?}")] +pub struct InvalidKey(#[error(ignore)] String); impl TryFrom for BeamlineField { type Error = InvalidKey; @@ -143,40 +116,17 @@ pub trait PathSpec { fn describe() -> &'static str; } -#[derive(Debug, PartialEq)] +#[derive(Debug, Display, Error, From, PartialEq)] pub enum InvalidPathTemplate { + #[display("{_0}")] + #[from] TemplateError(PathTemplateError), + #[display("Path should be absolute")] ShouldBeAbsolute, + #[display("Path should be relative")] ShouldBeRelative, - MissingField(String), -} - -impl Display for InvalidPathTemplate { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - InvalidPathTemplate::TemplateError(e) => write!(f, "{e}"), - InvalidPathTemplate::ShouldBeAbsolute => f.write_str("Path should be absolute"), - InvalidPathTemplate::ShouldBeRelative => f.write_str("Path should be relative"), - InvalidPathTemplate::MissingField(fld) => { - write!(f, "Template should reference missing field: {fld:?}") - } - } - } -} - -impl Error for InvalidPathTemplate { - fn source(&self) -> Option<&(dyn Error + 'static)> { - match self { - InvalidPathTemplate::TemplateError(e) => Some(e), - _ => None, - } - } -} - -impl From for InvalidPathTemplate { - fn from(value: PathTemplateError) -> Self { - Self::TemplateError(value) - } + #[display("Template should reference missing field: {_0:?}")] + MissingField(#[error(ignore)] String), } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/src/template.rs b/src/template.rs index 66ed7d78..7b497253 100644 --- a/src/template.rs +++ b/src/template.rs @@ -13,10 +13,11 @@ // limitations under the License. use std::borrow::Cow; -use std::error::Error; use std::fmt::{Debug, Display}; use std::path::{Component, PathBuf}; +use derive_more::{Display, Error}; + pub trait FieldSource { fn resolve(&self, field: &F) -> Cow<'_, str>; } @@ -90,30 +91,14 @@ impl PathType { } } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Display, Error, PartialEq, Eq)] pub enum PathTemplateError { + #[display("Path is not valid")] InvalidPath, + #[display("{_0}")] TemplateError(TemplateError), } -impl Display for PathTemplateError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - PathTemplateError::InvalidPath => f.write_str("Path is not valid"), - PathTemplateError::TemplateError(e) => write!(f, "{e}"), - } - } -} - -impl Error for PathTemplateError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - match self { - PathTemplateError::InvalidPath => None, - PathTemplateError::TemplateError(e) => Some(e), - } - } -} - #[derive(Debug)] enum ParseState { /// We haven't started parsing anything yet @@ -127,48 +112,30 @@ enum ParseState { PendingLiteral(String), } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Display, Error, PartialEq, Eq)] +#[display("Error parsing template: {kind} at {position}")] pub struct TemplateError { position: usize, kind: ErrorKind, } -impl Display for TemplateError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "Error parsing template: {} at {}", - self.kind, self.position - ) - } -} - -impl Error for TemplateError {} - /// The reasons why a Template could be invalid -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Debug, Display, PartialEq, Eq, Clone, Copy)] pub enum ErrorKind { /// Template placeholders cannot contain other placeholders + #[display("Nested placeholder")] Nested, /// Placeholders cannot be empty + #[display("Empty placeholder")] Empty, /// A placeholder was opened but not closed + #[display("Unclosed placeholder")] Incomplete, /// The placeholder was not a recognised key + #[display("Invalid placeholder")] Unrecognised, } -impl Display for ErrorKind { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - ErrorKind::Nested => f.write_str("Nested placeholder"), - ErrorKind::Empty => f.write_str("Empty placeholder"), - ErrorKind::Incomplete => f.write_str("Unclosed placeholder"), - ErrorKind::Unrecognised => f.write_str("Invalid placeholder"), - } - } -} - impl TemplateError { fn new(position: usize, kind: ErrorKind) -> Self { Self { position, kind }