From beeef4758e5ec0d521179a799a3237c2c0368911 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 3 Jan 2022 13:58:05 +0100 Subject: Some movement of helper code and refactoring of error handling --- src/model/Cargo.toml | 1 + src/model/block.rs | 8 +++--- src/model/bucket_helper.rs | 63 ---------------------------------------------- src/model/garage.rs | 6 ++--- src/model/helper/bucket.rs | 56 +++++++++++++++++++++++++++++++++++++++++ src/model/helper/error.rs | 51 +++++++++++++++++++++++++++++++++++++ src/model/helper/mod.rs | 2 ++ src/model/lib.rs | 2 +- 8 files changed, 117 insertions(+), 72 deletions(-) delete mode 100644 src/model/bucket_helper.rs create mode 100644 src/model/helper/bucket.rs create mode 100644 src/model/helper/error.rs create mode 100644 src/model/helper/mod.rs (limited to 'src/model') diff --git a/src/model/Cargo.toml b/src/model/Cargo.toml index 03881f5d..14e49557 100644 --- a/src/model/Cargo.toml +++ b/src/model/Cargo.toml @@ -21,6 +21,7 @@ garage_model_050 = { package = "garage_model", version = "0.5.1" } async-trait = "0.1.7" arc-swap = "1.0" +err-derive = "0.3" hex = "0.4" log = "0.4" rand = "0.8" diff --git a/src/model/block.rs b/src/model/block.rs index 6df8e265..1173c7b3 100644 --- a/src/model/block.rs +++ b/src/model/block.rs @@ -594,10 +594,8 @@ impl BlockManager { need_nodes.push(*node); } } - _ => { - return Err(Error::Message( - "Unexpected response to NeedBlockQuery RPC".to_string(), - )); + m => { + return Err(Error::unexpected_rpc_message(m)); } } } @@ -730,7 +728,7 @@ impl EndpointHandler for BlockManager { BlockRpc::PutBlock { hash, data } => self.write_block(hash, data).await, BlockRpc::GetBlock(h) => self.read_block(h).await, BlockRpc::NeedBlockQuery(h) => self.need_block(h).await.map(BlockRpc::NeedBlockReply), - _ => Err(Error::BadRpc("Unexpected RPC message".to_string())), + m => Err(Error::unexpected_rpc_message(m)), } } } diff --git a/src/model/bucket_helper.rs b/src/model/bucket_helper.rs deleted file mode 100644 index b55ebc4b..00000000 --- a/src/model/bucket_helper.rs +++ /dev/null @@ -1,63 +0,0 @@ -use garage_util::data::*; -use garage_util::error::*; - -use garage_table::util::EmptyKey; - -use crate::bucket_table::Bucket; -use crate::garage::Garage; - -pub struct BucketHelper<'a>(pub(crate) &'a Garage); - -impl<'a> BucketHelper<'a> { - #[allow(clippy::ptr_arg)] - pub async fn resolve_global_bucket_name( - &self, - bucket_name: &String, - ) -> Result, Error> { - // Bucket names in Garage are aliases, true bucket identifiers - // are 32-byte UUIDs. This function resolves bucket names into - // their full identifier by looking up in the bucket_alias_table. - // This function also allows buckets to be identified by their - // full UUID (hex-encoded). Here, if the name to be resolved is a - // hex string of the correct length, it is directly parsed as a bucket - // identifier which is returned. There is no risk of this conflicting - // with an actual bucket name: bucket names are max 63 chars long by - // the AWS spec, and hex-encoded UUIDs are 64 chars long. - let hexbucket = hex::decode(bucket_name.as_str()) - .ok() - .map(|by| Uuid::try_from(&by)) - .flatten(); - if let Some(bucket_id) = hexbucket { - Ok(self - .0 - .bucket_table - .get(&bucket_id, &EmptyKey) - .await? - .filter(|x| !x.state.is_deleted()) - .map(|_| bucket_id)) - } else { - Ok(self - .0 - .bucket_alias_table - .get(&EmptyKey, bucket_name) - .await? - .map(|x| x.state.get().as_option().map(|x| x.bucket_id)) - .flatten()) - } - } - - pub async fn get_existing_bucket(&self, bucket_id: Uuid) -> Result { - self.0 - .bucket_table - .get(&bucket_id, &EmptyKey) - .await? - .filter(|b| !b.is_deleted()) - .map(Ok) - .unwrap_or_else(|| { - Err(Error::BadRpc(format!( - "Bucket {:?} does not exist", - bucket_id - ))) - }) - } -} diff --git a/src/model/garage.rs b/src/model/garage.rs index 9db1843c..78b4433a 100644 --- a/src/model/garage.rs +++ b/src/model/garage.rs @@ -15,8 +15,8 @@ use garage_table::*; use crate::block::*; use crate::block_ref_table::*; use crate::bucket_alias_table::*; -use crate::bucket_helper::*; use crate::bucket_table::*; +use crate::helper; use crate::key_table::*; use crate::object_table::*; use crate::version_table::*; @@ -162,7 +162,7 @@ impl Garage { self.block_manager.garage.swap(None); } - pub fn bucket_helper(&self) -> BucketHelper { - BucketHelper(self) + pub fn bucket_helper(&self) -> helper::bucket::BucketHelper { + helper::bucket::BucketHelper(self) } } diff --git a/src/model/helper/bucket.rs b/src/model/helper/bucket.rs new file mode 100644 index 00000000..e89a723d --- /dev/null +++ b/src/model/helper/bucket.rs @@ -0,0 +1,56 @@ +use garage_table::util::EmptyKey; +use garage_util::data::*; + +use crate::bucket_table::Bucket; +use crate::garage::Garage; +use crate::helper::error::*; + +pub struct BucketHelper<'a>(pub(crate) &'a Garage); + +impl<'a> BucketHelper<'a> { + #[allow(clippy::ptr_arg)] + pub async fn resolve_global_bucket_name( + &self, + bucket_name: &String, + ) -> Result, Error> { + // Bucket names in Garage are aliases, true bucket identifiers + // are 32-byte UUIDs. This function resolves bucket names into + // their full identifier by looking up in the bucket_alias_table. + // This function also allows buckets to be identified by their + // full UUID (hex-encoded). Here, if the name to be resolved is a + // hex string of the correct length, it is directly parsed as a bucket + // identifier which is returned. There is no risk of this conflicting + // with an actual bucket name: bucket names are max 63 chars long by + // the AWS spec, and hex-encoded UUIDs are 64 chars long. + let hexbucket = hex::decode(bucket_name.as_str()) + .ok() + .map(|by| Uuid::try_from(&by)) + .flatten(); + if let Some(bucket_id) = hexbucket { + Ok(self + .0 + .bucket_table + .get(&bucket_id, &EmptyKey) + .await? + .filter(|x| !x.state.is_deleted()) + .map(|_| bucket_id)) + } else { + Ok(self + .0 + .bucket_alias_table + .get(&EmptyKey, bucket_name) + .await? + .map(|x| x.state.get().as_option().map(|x| x.bucket_id)) + .flatten()) + } + } + + pub async fn get_existing_bucket(&self, bucket_id: Uuid) -> Result { + self.0 + .bucket_table + .get(&bucket_id, &EmptyKey) + .await? + .filter(|b| !b.is_deleted()) + .ok_or_bad_request(format!("Bucket {:?} does not exist", bucket_id)) + } +} diff --git a/src/model/helper/error.rs b/src/model/helper/error.rs new file mode 100644 index 00000000..b9b515f3 --- /dev/null +++ b/src/model/helper/error.rs @@ -0,0 +1,51 @@ +use err_derive::Error; +use serde::{Deserialize, Serialize}; + +use garage_util::error::Error as GarageError; + +#[derive(Debug, Error, Serialize, Deserialize)] +pub enum Error { + #[error(display = "Internal error: {}", _0)] + Internal(#[error(source)] GarageError), + + #[error(display = "Bad request: {}", _0)] + BadRequest(String), +} + +impl From for Error { + fn from(e: netapp::error::Error) -> Self { + Error::Internal(GarageError::Netapp(e)) + } +} + +pub trait OkOrBadRequest { + type S; + fn ok_or_bad_request>(self, reason: M) -> Result; +} + +impl OkOrBadRequest for Result +where + E: std::fmt::Display, +{ + type S = T; + fn ok_or_bad_request>(self, reason: M) -> Result { + match self { + Ok(x) => Ok(x), + Err(e) => Err(Error::BadRequest(format!( + "{}: {}", + reason.as_ref(), + e.to_string() + ))), + } + } +} + +impl OkOrBadRequest for Option { + type S = T; + fn ok_or_bad_request>(self, reason: M) -> Result { + match self { + Some(x) => Ok(x), + None => Err(Error::BadRequest(reason.as_ref().to_string())), + } + } +} diff --git a/src/model/helper/mod.rs b/src/model/helper/mod.rs new file mode 100644 index 00000000..2f4e8898 --- /dev/null +++ b/src/model/helper/mod.rs @@ -0,0 +1,2 @@ +pub mod bucket; +pub mod error; diff --git a/src/model/lib.rs b/src/model/lib.rs index e7d7e98b..9deaae9d 100644 --- a/src/model/lib.rs +++ b/src/model/lib.rs @@ -12,6 +12,6 @@ pub mod version_table; pub mod block; -pub mod bucket_helper; pub mod garage; +pub mod helper; pub mod migrate; -- cgit v1.2.3