diff --git a/Cargo.lock b/Cargo.lock index a005a2f3f..6a11d47fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -421,6 +421,7 @@ dependencies = [ "criterion", "derive_more", "lazy_static", + "log", "nom", "nom_locate", "phf", diff --git a/Cargo.toml b/Cargo.toml index d21adb78a..91c32c9e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,6 +52,7 @@ wildmatch = "2.1.1" clap = { version = "4.4.7", features = ["derive"] } cli-table = "0.4.7" textwrap = "0.16.0" +log = "0.4.20" [dev-dependencies] criterion = { version = "0.5", features = ["html_reports"] } diff --git a/src/bin/config-store.rs b/src/bin/config-store.rs index 6a6d2a1c5..64b67a062 100644 --- a/src/bin/config-store.rs +++ b/src/bin/config-store.rs @@ -1,9 +1,11 @@ +use anyhow::anyhow; use clap::{Parser, Subcommand}; use derive_more::Display; 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")] @@ -45,13 +47,13 @@ enum Engine { } impl std::str::FromStr for Engine { - type Err = (); + type Err = anyhow::Error; fn from_str(s: &str) -> Result { match s { "sqlite" => Ok(Engine::Sqlite), "json" => Ok(Engine::Json), - _ => Err(()), + _ => Err(anyhow!("problem reading config")), } } } @@ -69,25 +71,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::try_from(&args.global_opts.path)?), + Engine::Json => Box::new(JsonStorageAdapter::try_from(&args.global_opts.path)?), }; - let mut store = ConfigStore::new(storage_box, true); + let mut store = ConfigStore::from_storage(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 +98,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..beb54d667 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(storage: Box, preload: bool) -> Result { let mut store = ConfigStore { settings: HashMap::new(), settings_info: HashMap::new(), @@ -55,17 +57,17 @@ impl ConfigStore { }; // Populate the settings from the json file - store.populate_settings(); + store.populate_settings()?; // 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 @@ -134,7 +136,7 @@ impl ConfigStore { } /// Populates the settings in the store from the settings.json file - fn populate_settings(&mut self) { + fn populate_settings(&mut self) -> Result<()> { let json_data: Value = serde_json::from_str(SETTINGS_JSON).expect("Failed to parse settings.json"); @@ -150,8 +152,7 @@ impl ConfigStore { let info = SettingInfo { key: key.clone(), description: entry.description, - default: Setting::from_string(entry.default.as_str()) - .expect("cannot parse default setting"), + default: Setting::from_str(&entry.default)?, last_accessed: 0, }; @@ -161,6 +162,8 @@ impl ConfigStore { } } } + + Ok(()) } } @@ -171,7 +174,8 @@ mod test { #[test] fn config_store() { - let mut store = ConfigStore::new(Box::new(MemoryStorageAdapter::new()), true); + let mut store = + ConfigStore::from_storage(Box::new(MemoryStorageAdapter::new()), true).unwrap(); let setting = store.get("dns.local_resolver.enabled"); assert_eq!(setting, Setting::Bool(false)); @@ -183,7 +187,8 @@ mod test { #[test] #[should_panic] fn invalid_setting() { - let mut store = ConfigStore::new(Box::new(MemoryStorageAdapter::new()), true); + let mut store = + ConfigStore::from_storage(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..8a55e5d7a 100644 --- a/src/config/storage/json_storage.rs +++ b/src/config/storage/json_storage.rs @@ -1,5 +1,7 @@ use crate::config::settings::Setting; use crate::config::Store; +use crate::types::{Error, Result}; +use log::warn; use serde_json::Value; use std::collections::HashMap; use std::fs; @@ -13,8 +15,10 @@ pub struct JsonStorageAdapter { elements: HashMap, } -impl JsonStorageAdapter { - pub fn new(path: &str) -> Self { +impl TryFrom<&String> for JsonStorageAdapter { + type Error = Error; + + fn try_from(path: &String) -> Result { let file = match fs::metadata(path) { Ok(metadata) => { if !metadata.is_file() { @@ -31,8 +35,7 @@ impl JsonStorageAdapter { let json = "{}"; let mut file = File::create(path).expect("cannot create json file"); - file.write_all(json.as_bytes()) - .expect("failed to write to file"); + file.write_all(json.as_bytes())?; file } @@ -46,7 +49,7 @@ impl JsonStorageAdapter { adapter.read_file(); - adapter + Ok(adapter) } } @@ -61,8 +64,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()) } } @@ -79,8 +82,13 @@ impl JsonStorageAdapter { if let Value::Object(settings) = parsed_json { self.elements = HashMap::new(); for (key, value) in settings.iter() { - if let Ok(setting) = serde_json::from_value(value.clone()) { - self.elements.insert(key.clone(), setting); + match serde_json::from_value(value.clone()) { + Ok(setting) => { + self.elements.insert(key.clone(), setting); + } + Err(err) => { + warn!("problem reading setting from json: {err}"); + } } } } 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..046ca175a 100644 --- a/src/config/storage/sqlite_storage.rs +++ b/src/config/storage/sqlite_storage.rs @@ -1,13 +1,18 @@ use crate::config::settings::Setting; use crate::config::Store; +use crate::types::{Error, Result}; +use log::warn; use std::collections::HashMap; +use std::str::FromStr; pub struct SqliteStorageAdapter { connection: sqlite::Connection, } -impl SqliteStorageAdapter { - pub fn new(path: &str) -> Self { +impl TryFrom<&String> for SqliteStorageAdapter { + type Error = Error; + + fn try_from(path: &String) -> Result { let conn = sqlite::open(path).expect("cannot open db file"); let query = "CREATE TABLE IF NOT EXISTS settings ( @@ -15,9 +20,9 @@ impl SqliteStorageAdapter { key TEXT NOT NULL, value TEXT NOT NULL )"; - conn.execute(query).unwrap(); + conn.execute(query)?; - SqliteStorageAdapter { connection: conn } + Ok(SqliteStorageAdapter { connection: conn }) } } @@ -27,7 +32,13 @@ impl Store for SqliteStorageAdapter { let mut statement = self.connection.prepare(query).unwrap(); statement.bind((":key", key)).unwrap(); - Setting::from_string(key) + match Setting::from_str(key) { + Ok(setting) => Some(setting), + Err(err) => { + warn!("problem reading from sqlite: {err}"); + None + } + } } fn set_setting(&mut self, key: &str, value: Setting) { @@ -41,7 +52,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 +60,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..35fec7ffb 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), @@ -32,6 +35,9 @@ pub enum Error { #[error("json parsing error: {0}")] JsonSerde(#[from] serde_json::Error), + #[error("sqlite error: {0}")] + Sqlite(#[from] sqlite::Error), + #[error("test error: {0}")] Test(String),