From faaff8f66eba5087c331ce765d5a53cdfd0a17ca Mon Sep 17 00:00:00 2001 From: Eric Walker Date: Mon, 13 Nov 2023 20:12:05 -0700 Subject: [PATCH] Add basic error handling --- src/bin/config-store.rs | 23 ++++--- src/config.rs | 16 +++-- src/config/settings.rs | 95 +++++++++++++++------------- src/config/storage/json_storage.rs | 5 +- src/config/storage/memory_storage.rs | 5 +- src/config/storage/sqlite_storage.rs | 10 +-- src/types.rs | 3 + 7 files changed, 88 insertions(+), 69 deletions(-) diff --git a/src/bin/config-store.rs b/src/bin/config-store.rs index 6a6d2a1c5..4d44efd9b 100644 --- a/src/bin/config-store.rs +++ b/src/bin/config-store.rs @@ -4,6 +4,7 @@ use gosub_engine::config::settings::Setting; use gosub_engine::config::storage::json_storage::JsonStorageAdapter; use gosub_engine::config::storage::sqlite_storage::SqliteStorageAdapter; use gosub_engine::config::{ConfigStore, Store}; +use std::str::FromStr; #[derive(Debug, Parser)] #[clap(name = "Config-Store", version = "0.1.0", author = "Gosub")] @@ -69,25 +70,25 @@ struct GlobalOpts { path: String, } -fn main() { +fn main() -> anyhow::Result<()> { let args = Cli::parse(); let storage_box: Box = match args.global_opts.engine { - Engine::Sqlite => Box::new(SqliteStorageAdapter::new(args.global_opts.path.as_str())), - Engine::Json => Box::new(JsonStorageAdapter::new(args.global_opts.path.as_str())), + Engine::Sqlite => Box::new(SqliteStorageAdapter::new(&args.global_opts.path)), + Engine::Json => Box::new(JsonStorageAdapter::new(&args.global_opts.path)), }; - let mut store = ConfigStore::new(storage_box, true); + let mut store = ConfigStore::from(storage_box, true)?; match args.command { Commands::View { key } => { if !store.has(&key) { println!("Key not found"); - return; + return Ok(()); } - let info = store.get_info(key.as_str()).unwrap(); - let value = store.get(key.as_str()); + let info = store.get_info(&key).unwrap(); + let value = store.get(&key); println!("Key : {}", key); println!("Current Value : {}", value); @@ -96,18 +97,20 @@ fn main() { } Commands::List => { for key in store.find("*") { - let value = store.get(key.as_str()); + let value = store.get(&key); println!("{:40}: {}", key, value); } } Commands::Set { key, value } => { - store.set(&key, Setting::from_string(&value).expect("incorrect value")); + store.set(&key, Setting::from_str(&value).expect("incorrect value")); } Commands::Search { key } => { - for key in store.find(key.as_str()) { + for key in store.find(&key) { let value = store.get(&key); println!("{:40}: {}", key, value); } } } + + Ok(()) } diff --git a/src/config.rs b/src/config.rs index 31d8c702d..006e709be 100644 --- a/src/config.rs +++ b/src/config.rs @@ -2,10 +2,12 @@ pub mod settings; pub mod storage; use crate::config::settings::{Setting, SettingInfo}; +use crate::types::Result; use serde_derive::Deserialize; use serde_json::Value; use std::collections::HashMap; use std::mem; +use std::str::FromStr; use wildmatch::WildMatch; const SETTINGS_JSON: &str = include_str!("./config/settings.json"); @@ -29,7 +31,7 @@ pub trait Store { /// Retrieves all the settings in the storage in one go. This is used for preloading the settings /// into the ConfigStore and is more performant normally than calling get_setting manually for each /// setting. - fn get_all_settings(&self) -> HashMap; + fn get_all_settings(&self) -> Result>; } /// Configuration store is the place where the gosub engine can find all configurable options @@ -46,7 +48,7 @@ pub struct ConfigStore { impl ConfigStore { /// Creates a new store with the given storage adapter and preloads the store if needed - pub fn new(storage: Box, preload: bool) -> Self { + pub fn from(storage: Box, preload: bool) -> Result { let mut store = ConfigStore { settings: HashMap::new(), settings_info: HashMap::new(), @@ -59,13 +61,13 @@ impl ConfigStore { // preload the settings if requested if preload { - let all_settings = store.storage.get_all_settings(); + let all_settings = store.storage.get_all_settings()?; for (key, value) in all_settings { store.settings.insert(key, value); } } - store + Ok(store) } /// Returns true when the store knows about the given key @@ -150,7 +152,7 @@ impl ConfigStore { let info = SettingInfo { key: key.clone(), description: entry.description, - default: Setting::from_string(entry.default.as_str()) + default: Setting::from_str(&entry.default) .expect("cannot parse default setting"), last_accessed: 0, }; @@ -171,7 +173,7 @@ mod test { #[test] fn config_store() { - let mut store = ConfigStore::new(Box::new(MemoryStorageAdapter::new()), true); + let mut store = ConfigStore::from(Box::new(MemoryStorageAdapter::new()), true).unwrap(); let setting = store.get("dns.local_resolver.enabled"); assert_eq!(setting, Setting::Bool(false)); @@ -183,7 +185,7 @@ mod test { #[test] #[should_panic] fn invalid_setting() { - let mut store = ConfigStore::new(Box::new(MemoryStorageAdapter::new()), true); + let mut store = ConfigStore::from(Box::new(MemoryStorageAdapter::new()), true).unwrap(); store.set( "dns.local_resolver.enabled", Setting::String("wont accept strings".into()), diff --git a/src/config/settings.rs b/src/config/settings.rs index a04dd5a58..df5526363 100644 --- a/src/config/settings.rs +++ b/src/config/settings.rs @@ -1,5 +1,7 @@ +use crate::types::{Error, Result}; use core::fmt::Display; use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use std::str::FromStr; /// A setting can be either a signed integer, unsigned integer, string, map or boolean. /// Maps could be created by using comma separated strings maybe @@ -13,7 +15,7 @@ pub enum Setting { } impl Serialize for Setting { - fn serialize(&self, serializer: S) -> Result + fn serialize(&self, serializer: S) -> std::result::Result where S: Serializer, { @@ -23,16 +25,13 @@ impl Serialize for Setting { } impl<'de> Deserialize<'de> for Setting { - fn deserialize(deserializer: D) -> Result + fn deserialize(deserializer: D) -> std::result::Result where D: Deserializer<'de>, { let value = String::deserialize(deserializer)?; - - match Setting::from_string(value.as_str()) { - None => Err(serde::de::Error::custom("Cannot deserialize")), - Some(setting) => Ok(setting), - } + Setting::from_str(&value) + .map_err(|err| serde::de::Error::custom(format!("cannot deserialize: {err}"))) } } @@ -56,7 +55,9 @@ impl Display for Setting { } } -impl Setting { +impl FromStr for Setting { + type Err = Error; + // first element is the type: // b:true // i:-123 @@ -65,32 +66,38 @@ impl Setting { // m:foo,bar,baz /// Converts a string to a setting or None when the string is invalid - pub fn from_string(key: &str) -> Option { + fn from_str(key: &str) -> Result { let (key_type, key_value) = key.split_once(':').unwrap(); - match key_type { - "b" => match key_value.parse::() { - Ok(value) => Some(Setting::Bool(value)), - Err(_) => None, - }, - "i" => match key_value.parse::() { - Ok(value) => Some(Setting::SInt(value)), - Err(_) => None, - }, - "u" => match key_value.parse::() { - Ok(value) => Some(Setting::UInt(value)), - Err(_) => None, - }, - "s" => Some(Setting::String(key_value.to_string())), + let setting = match key_type { + "b" => Setting::Bool( + key_value + .parse::() + .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?, + ), + "i" => Setting::SInt( + key_value + .parse::() + .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?, + ), + "u" => Setting::UInt( + key_value + .parse::() + .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?, + ), + "s" => Setting::String(key_value.to_string()), + "m" => { let mut values = Vec::new(); for value in key_value.split(',') { values.push(value.to_string()); } - Some(Setting::Map(values)) + Setting::Map(values) } - _ => None, - } + _ => return Err(Error::Config(format!("unknown setting: {key_value}"))), + }; + + Ok(setting) } } @@ -113,34 +120,34 @@ mod test { #[test] fn setting() { - let s = Setting::from_string("b:true"); - assert_eq!(s, Some(Setting::Bool(true))); + let s = Setting::from_str("b:true").unwrap(); + assert_eq!(s, Setting::Bool(true)); - let s = Setting::from_string("i:-1"); - assert_eq!(s, Some(Setting::SInt(-1))); + let s = Setting::from_str("i:-1").unwrap(); + assert_eq!(s, Setting::SInt(-1)); - let s = Setting::from_string("i:1"); - assert_eq!(s, Some(Setting::SInt(1))); + let s = Setting::from_str("i:1").unwrap(); + assert_eq!(s, Setting::SInt(1)); - let s = Setting::from_string("s:hello world"); - assert_eq!(s, Some(Setting::String("hello world".into()))); + let s = Setting::from_str("s:hello world").unwrap(); + assert_eq!(s, Setting::String("hello world".into())); - let s = Setting::from_string("m:foo,bar,baz"); + let s = Setting::from_str("m:foo,bar,baz").unwrap(); assert_eq!( s, - Some(Setting::Map(vec!["foo".into(), "bar".into(), "baz".into()])) + Setting::Map(vec!["foo".into(), "bar".into(), "baz".into()]) ); - let s = Setting::from_string("notexist:true"); - assert_eq!(s, None); + let s = Setting::from_str("notexist:true"); + assert!(matches!(s, Err(Error::Config(_)))); - let s = Setting::from_string("b:foobar"); - assert_eq!(s, None); + let s = Setting::from_str("b:foobar"); + assert!(matches!(s, Err(Error::Config(_)))); - let s = Setting::from_string("i:foobar"); - assert_eq!(s, None); + let s = Setting::from_str("i:foobar"); + assert!(matches!(s, Err(Error::Config(_)))); - let s = Setting::from_string("u:-1"); - assert_eq!(s, None); + let s = Setting::from_str("u:-1"); + assert!(matches!(s, Err(Error::Config(_)))); } } diff --git a/src/config/storage/json_storage.rs b/src/config/storage/json_storage.rs index 85c7f3b36..910d4a8fe 100644 --- a/src/config/storage/json_storage.rs +++ b/src/config/storage/json_storage.rs @@ -1,5 +1,6 @@ use crate::config::settings::Setting; use crate::config::Store; +use crate::types::Result; use serde_json::Value; use std::collections::HashMap; use std::fs; @@ -61,8 +62,8 @@ impl Store for JsonStorageAdapter { self.write_file() } - fn get_all_settings(&self) -> HashMap { - self.elements.clone() + fn get_all_settings(&self) -> Result> { + Ok(self.elements.clone()) } } diff --git a/src/config/storage/memory_storage.rs b/src/config/storage/memory_storage.rs index a7988f7f4..b40d9df3b 100644 --- a/src/config/storage/memory_storage.rs +++ b/src/config/storage/memory_storage.rs @@ -1,5 +1,6 @@ use crate::config::settings::Setting; use crate::config::Store; +use crate::types::Result; use std::collections::HashMap; #[derive(Default)] @@ -25,7 +26,7 @@ impl Store for MemoryStorageAdapter { self.store.insert(key.to_string(), value); } - fn get_all_settings(&self) -> HashMap { - self.store.clone() + fn get_all_settings(&self) -> Result> { + Ok(self.store.clone()) } } diff --git a/src/config/storage/sqlite_storage.rs b/src/config/storage/sqlite_storage.rs index 9477e604b..da24e2e45 100644 --- a/src/config/storage/sqlite_storage.rs +++ b/src/config/storage/sqlite_storage.rs @@ -1,6 +1,8 @@ use crate::config::settings::Setting; use crate::config::Store; +use crate::types::Result; use std::collections::HashMap; +use std::str::FromStr; pub struct SqliteStorageAdapter { connection: sqlite::Connection, @@ -27,7 +29,7 @@ impl Store for SqliteStorageAdapter { let mut statement = self.connection.prepare(query).unwrap(); statement.bind((":key", key)).unwrap(); - Setting::from_string(key) + Setting::from_str(key).ok() } fn set_setting(&mut self, key: &str, value: Setting) { @@ -41,7 +43,7 @@ impl Store for SqliteStorageAdapter { statement.next().unwrap(); } - fn get_all_settings(&self) -> HashMap { + fn get_all_settings(&self) -> Result> { let query = "SELECT * FROM settings"; let mut statement = self.connection.prepare(query).unwrap(); @@ -49,9 +51,9 @@ impl Store for SqliteStorageAdapter { while let sqlite::State::Row = statement.next().unwrap() { let key = statement.read::(1).unwrap(); let value = statement.read::(2).unwrap(); - settings.insert(key, Setting::from_string(value.as_str()).unwrap()); + settings.insert(key, Setting::from_str(&value)?); } - settings + Ok(settings) } } diff --git a/src/types.rs b/src/types.rs index 639f310dc..1f01a9dca 100644 --- a/src/types.rs +++ b/src/types.rs @@ -17,6 +17,9 @@ pub struct ParseError { /// Serious errors and errors from third-party libraries #[derive(Error, Debug)] pub enum Error { + #[error("config error: {0}")] + Config(String), + #[error("ureq error")] Request(#[from] Box),