From c2bc16f3d8cbaf828a5fe77479bf789206c52aec Mon Sep 17 00:00:00 2001 From: Joey Hines Date: Sun, 17 Oct 2021 12:21:01 -0600 Subject: [PATCH] Added better error handling for API+Database + GeoffreyDatabaseError implements Into + The database Option and Result were combined into just Result to reduce complexity --- geoffrey_api/src/commands/add_location.rs | 2 +- geoffrey_api/src/helper/mod.rs | 4 +--- geoffrey_db/src/database.rs | 21 +++++++++---------- geoffrey_db/src/error.rs | 15 +++++++++++++ .../src/models/response/api_error.rs | 5 +++-- 5 files changed, 30 insertions(+), 17 deletions(-) diff --git a/geoffrey_api/src/commands/add_location.rs b/geoffrey_api/src/commands/add_location.rs index 1ff0147..4342444 100644 --- a/geoffrey_api/src/commands/add_location.rs +++ b/geoffrey_api/src/commands/add_location.rs @@ -42,7 +42,7 @@ impl Command for AddLocation { .insert(location) .map_err(|err| GeoffreyAPIError::DatabaseError(err.to_string())) } else { - Err(GeoffreyAPIError::PlayerNotFound) + Err(GeoffreyAPIError::PlayerNotRegistered) } } } diff --git a/geoffrey_api/src/helper/mod.rs b/geoffrey_api/src/helper/mod.rs index 70d7d9a..23ab8fa 100644 --- a/geoffrey_api/src/helper/mod.rs +++ b/geoffrey_api/src/helper/mod.rs @@ -2,13 +2,11 @@ use crate::Result; use geoffrey_db::database::Database; use geoffrey_models::models::parameters::CommandRequest; use geoffrey_models::models::player::Player; -use geoffrey_models::models::response::api_error::GeoffreyAPIError; pub fn get_player_from_req(db: &Database, req: &CommandRequest) -> Result> { if let Some(user_id) = &req.user { Ok(db - .filter(|_, player: &Player| player.has_user_id(user_id)) - .map_err(|err| GeoffreyAPIError::DatabaseError(err.to_string()))? + .filter(|_, player: &Player| player.has_user_id(user_id)).map_err(|e| e.into())? .next()) } else { Ok(None) diff --git a/geoffrey_db/src/database.rs b/geoffrey_db/src/database.rs index af33b8a..ef1952a 100644 --- a/geoffrey_db/src/database.rs +++ b/geoffrey_db/src/database.rs @@ -48,7 +48,7 @@ impl Database { Ok(model) } - pub fn get(&self, id: u64) -> Result> + pub fn get(&self, id: u64) -> Result where T: GeoffreyDatabaseModel, { @@ -56,9 +56,9 @@ impl Database { let id_bytes = id.to_be_bytes(); if let Some(bytes) = tree.get(id_bytes)? { - Ok(Some(T::try_from_bytes(&bytes)?)) + Ok(T::try_from_bytes(&bytes)?) } else { - Ok(None) + Err(GeoffreyDBError::NotFound) } } @@ -125,7 +125,7 @@ mod tests { #[test] fn test_insert() { - let player = Player::new("CoolZero123", UserID::DiscordUUID(0u64)); + let player = Player::new("CoolZero123", UserID::DiscordUUID {discord_uuid: 0u64}); let p2 = DB.insert::(player.clone()).unwrap(); @@ -137,8 +137,8 @@ mod tests { #[test] fn test_unique_insert() { - let player1 = Player::new("CoolZero123", UserID::DiscordUUID(0u64)); - let player2 = Player::new("CoolZero123", UserID::DiscordUUID(0u64)); + let player1 = Player::new("CoolZero123", UserID::DiscordUUID {discord_uuid: 0u64}); + let player2 = Player::new("CoolZero123", UserID::DiscordUUID {discord_uuid: 0u64}); DB.insert::(player1.clone()).unwrap(); assert_eq!(DB.insert::(player2.clone()).is_err(), true); @@ -148,20 +148,19 @@ mod tests { #[test] fn test_get() { - let player = Player::new("CoolZero123", UserID::DiscordUUID(0u64)); + let player = Player::new("CoolZero123", UserID::DiscordUUID {discord_uuid: 0u64}); let p2 = DB.insert::(player.clone()).unwrap(); let p3 = DB.get::(p2.id().unwrap()).unwrap(); - assert!(p3.is_some()); - assert_eq!(p3.unwrap().name, player.name); + assert_eq!(p3.name, player.name); cleanup(); } #[test] fn test_filter() { - let player = Player::new("CoolZero123", UserID::DiscordUUID(0u64)); + let player = Player::new("CoolZero123", UserID::DiscordUUID {discord_uuid: 0u64}); let player = DB.insert::(player.clone()).unwrap(); let loc = Location::new( "Test Shop", @@ -191,7 +190,7 @@ mod tests { let insert_count = 1000; let timer = Instant::now(); for i in 0..insert_count { - let player = Player::new("test", UserID::DiscordUUID(i)); + let player = Player::new("test", UserID::DiscordUUID {discord_uuid: i}); DB.insert::(player).unwrap(); } diff --git a/geoffrey_db/src/error.rs b/geoffrey_db/src/error.rs index 36d086d..6e2bf5d 100644 --- a/geoffrey_db/src/error.rs +++ b/geoffrey_db/src/error.rs @@ -1,3 +1,5 @@ +use geoffrey_models::models::response::api_error::GeoffreyAPIError; + pub type Result = std::result::Result; #[derive(Debug)] @@ -5,6 +7,7 @@ pub enum GeoffreyDBError { SledError(sled::Error), SerdeJsonError(serde_json::Error), NotUnique, + NotFound, } impl std::error::Error for GeoffreyDBError {} @@ -15,6 +18,7 @@ impl std::fmt::Display for GeoffreyDBError { GeoffreyDBError::SledError(e) => write!(f, "Sled Error: {}", e), GeoffreyDBError::SerdeJsonError(e) => write!(f, "Serde JSON Error: {}", e), GeoffreyDBError::NotUnique => write!(f, "Entry is not unique."), + GeoffreyDBError::NotFound => write!(f, "Entry was not found."), } } } @@ -30,3 +34,14 @@ impl From for GeoffreyDBError { GeoffreyDBError::SerdeJsonError(e) } } + +impl Into for GeoffreyDBError { + fn into(self) -> GeoffreyAPIError { + match self { + GeoffreyDBError::SledError(_) => GeoffreyAPIError::DatabaseError(self.to_string()), + GeoffreyDBError::SerdeJsonError(_) => GeoffreyAPIError::DatabaseError(self.to_string()), + GeoffreyDBError::NotUnique => GeoffreyAPIError::EntryNotUnique, + GeoffreyDBError::NotFound => GeoffreyAPIError::EntryNotFound + } + } +} diff --git a/geoffrey_models/src/models/response/api_error.rs b/geoffrey_models/src/models/response/api_error.rs index c27f10e..34700dc 100644 --- a/geoffrey_models/src/models/response/api_error.rs +++ b/geoffrey_models/src/models/response/api_error.rs @@ -2,8 +2,9 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Serialize, Deserialize)] pub enum GeoffreyAPIError { - PlayerNotFound, - LocationNotFound, + PlayerNotRegistered, + EntryNotFound, PermissionInsufficient, + EntryNotUnique, DatabaseError(String), }