another attempt

src/bin/config-store.rs

@@ -2,9 +2,9 @@ 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 gosub_engine::config::storage::json::JsonStorageAdapter;
+use gosub_engine::config::storage::sqlite::SqliteStorageAdapter;
+use gosub_engine::config::{ConfigStore, StoreAdapter};
 use std::str::FromStr;
 
 #[derive(Debug, Parser)]
@@ -74,7 +74,7 @@ struct GlobalOpts {
 fn main() -> anyhow::Result<()> {
     let args = Cli::parse();
 
-    let storage_box: Box<dyn Store> = match args.global_opts.engine {
+    let storage_box: Box<dyn StoreAdapter> = match args.global_opts.engine {
         Engine::Sqlite => Box::new(SqliteStorageAdapter::try_from(&args.global_opts.path)?),
         Engine::Json => Box::new(JsonStorageAdapter::try_from(&args.global_opts.path)?),
     };

src/config.rs

@@ -5,123 +5,67 @@ use crate::config::settings::{Setting, SettingInfo};
 use serde_derive::Deserialize;
 use serde_json::Value;
 use std::collections::HashMap;
-use std::{fmt, mem};
+use std::mem;
 use std::str::FromStr;
-use std::sync::atomic::AtomicUsize;
+use lazy_static::lazy_static;
 use wildmatch::WildMatch;
-use crate::config::storage::nop_storage::NopStorage;
+use crate::config::storage::nop::NopStorage;
+use std::sync::RwLock;
 
 const SETTINGS_JSON: &str = include_str!("./config/settings.json");
 
-// Heavily inspired by the log crate
-static mut CONFIG_STORE: &dyn Store = &NopStorage{};
-static CONFIG_STORE_STATE: AtomicUsize = AtomicUsize::new(0);
-
-const UNINITIALIZED: usize = 0;     // The config_store is not initialized yet. No calls to config!() are allowed (will panic)
-const INITIALIZING: usize = 1;      // The config_store is currently being initialized. No config!() is allowed, and the next call to config_store() will block until the store is initialized
-const INITIALIZED: usize = 2;       // The config_store is initialized config!() is ok. New setting of the config store will fail
-
-/// Store is the interface for storing and retrieving settings
-/// This can be used to store settings in a database, json file, etc
-pub trait Store: Sync + Send {
+/// StoreAdapter is the interface for storing and retrieving settings
+/// This can be used to storage settings in a database, json file, etc
+pub trait StoreAdapter {
     /// Retrieves a setting from the storage
     fn get(&self, key: &str) -> Option<Setting>;
-    /// Stores a given setting to the storage
-    fn set(&mut self, key: &str, value: Setting);
+
+    /// Stores a given setting to the storage. Note that "self" is self and not "mut self". We need to be able
+    /// to storage settings in a non-mutable way. That is mostly possible it seems with a mutex lock that we
+    /// can get mutable.
+    fn set(&self, key: &str, value: Setting);
+
     /// 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 all(&self) -> crate::types::Result<HashMap<String, Setting>>;
 }
 
-impl<T> Store for &'_ T where T: Store + ?Sized,
-{
-    fn get(&self, key: &str) -> Option<Setting> {
-        (**self).get(key)
-    }
-
-    fn set(&mut self, key: &str, value: Setting) {
-        (mut **self).set(key, value)
-    }
-
-    fn all(&self) -> crate::types::Result<HashMap<String, Setting>> {
-        (**self).all()
-    }
-}
-
-pub fn set_config_store(store: &'static dyn Store) -> Result<(), SetStoreError> {
-    set_config_store_inner(|| store)
-}
-
-fn set_config_store_inner<F>(make_store: F) -> Result<(), SetStoreError>
-where F: FnOnce() -> &'static dyn Store,
-{
-    let old_state = match CONFIG_STORE_STATE.compare_exchange(
-        UNINITIALIZED,
-        INITIALIZING,
-        std::sync::atomic::Ordering::SeqCst,
-        std::sync::atomic::Ordering::SeqCst,
-    ) {
-        Ok(s) | Err(s) => s,
-    };
-
-    match old_state {
-        UNINITIALIZED => {
-            unsafe {
-                CONFIG_STORE = make_store();
-            }
-            CONFIG_STORE_STATE.store(INITIALIZED, std::sync::atomic::Ordering::SeqCst);
-            Ok(())
-        }
-        INITIALIZING => {
-            while CONFIG_STORE_STATE.load(std::sync::atomic::Ordering::SeqCst) == INITIALIZING {
-                std::thread::yield_now();
-            }
-            Err(SetStoreError(()))
-        }
-        _ => Err(SetStoreError(())),
-    }
-}
-
-#[allow(missing_copy_implementations)]
-#[derive(Debug)]
-pub struct SetStoreError(());
-
-impl fmt::Display for SetStoreError {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        fmt.write_str("Config store already initialized")
-    }
-}
-
-impl std::error::Error for SetStoreError {}
-
-pub fn config_store() -> &'static dyn Store {
-    if CONFIG_STORE_STATE.load(std::sync::atomic::Ordering::SeqCst) != INITIALIZED {
-        static NOP_STORE: NopStorage = NopStorage{};
-        &NOP_STORE
-    } else {
-        unsafe { CONFIG_STORE }
-    }
+lazy_static! {
+    static ref CONFIG_STORE: RwLock<ConfigStore> = RwLock::new(ConfigStore::new());
 }
 
-
-
 #[macro_export]
 macro_rules! config {
     (string $key:expr) => {
-        config_store().get($key).unwrap().to_string()
+        match crate::config::CONFIG_STORE.get($key) {
+            Some(setting) => setting.to_string(),
+            None => String::new(),
+        }
     };
     (bool $key:expr) => {
-        config_store().get($key).unwrap().to_bool()
+        match crate::config::CONFIG_STORE.get($key) {
+            Some(setting) => setting.to_bool(),
+            None => false,
+        }
     };
     (uint $key:expr) => {
-        config_store().get($key).unwrap().to_uint()
+        match crate::config::CONFIG_STORE.get($key) {
+            Some(setting) => setting.to_uint(),
+            None => 0,
+        }
     };
     (sint $key:expr) => {
-        config_store().get($key).unwrap().to_sint()
+        match crate::config::CONFIG_STORE.get($key) {
+            Some(setting) => setting.to_sint(),
+            None => 0,
+        }
     };
     (map $key:expr) => {
-        config_store().get($key).unwrap().to_map()
+        match crate::config::CONFIG_STORE.get($key) {
+            Some(setting) => setting.to_map(),
+            None => std::collections::HashMap::new(),
+        }
     };
 }
 
@@ -134,7 +78,7 @@ struct JsonEntry {
     description: String,
 }
 
-/// Configuration store is the place where the gosub engine can find all configurable options
+/// Configuration storage is the place where the gosub engine can find all configurable options
 pub struct ConfigStore {
     /// A hashmap of all settings so we can search o(1) time
     settings: HashMap<String, Setting>,
@@ -143,34 +87,34 @@ pub struct ConfigStore {
     /// Keys of all settings so we can iterate keys easily
     setting_keys: Vec<String>,
     /// The storage adapter used for persisting and loading keys
-    storage: Box<dyn Store>,
+    storage: Box<dyn StoreAdapter>,
 }
 
 impl ConfigStore {
-    /// Creates a new store with the given storage adapter and preloads the store if needed
-    pub fn from_storage(storage: Box<dyn Store>, preload: bool) -> crate::types::Result<Self> {
+    pub fn new() -> Self {
         let mut store = ConfigStore {
             settings: HashMap::new(),
             settings_info: HashMap::new(),
             setting_keys: Vec::new(),
-            storage,
+            storage: Box::new(NopStorage {}),
         };
 
-        // Populate the settings from the json file
-        store.populate_settings()?;
+        store.populate_default_settings();
+
+        store
+    }
+
+    pub fn set_store(&mut self, store: Box<dyn StoreAdapter>) {
+        self.storage = store;
 
-        // preload the settings if requested
-        if preload {
-            let all_settings = store.storage.all()?;
+        if let Ok(all_settings) = self.storage.all() {
             for (key, value) in all_settings {
-                store.settings.insert(key, value);
+                self.settings.insert(key, value);
             }
         }
-
-        Ok(store)
     }
 
-    /// Returns true when the store knows about the given key
+    /// Returns true when the storage knows about the given key
     pub fn has(&self, key: &str) -> bool {
         self.settings.contains_key(key)
     }
@@ -197,7 +141,7 @@ impl ConfigStore {
     }
 
     /// Returns the setting with the given key. If the setting is not found in the current
-    /// store, it will load the key from the storage. If the key is still not found, it will
+    /// storage, it will load the key from the storage. If the key is still not found, it will
     /// return the default value for the given key. Note that if the key is not found and no
     /// default value is specified, this function will panic.
     pub fn get(&mut self, key: &str) -> Setting {
@@ -235,8 +179,8 @@ impl ConfigStore {
         self.storage.set(key, value);
     }
 
-    /// Populates the settings in the store from the settings.json file
-    fn populate_settings(&mut self) -> crate::types::Result<()> {
+    /// Populates the settings in the storage from the settings.json file
+    fn populate_default_settings(&mut self) -> crate::types::Result<()> {
         let json_data: Value =
             serde_json::from_str(SETTINGS_JSON).expect("Failed to parse settings.json");
 
@@ -269,18 +213,19 @@ impl ConfigStore {
 
 #[cfg(test)]
 mod test {
+    use simple_logger::SimpleLogger;
     use super::*;
-    use crate::config::storage::memory_storage::MemoryStorageAdapter;
+    use crate::config::storage::memory::MemoryStorageAdapter;
 
     #[test]
     fn config_store() {
-        let mut store =
-            ConfigStore::from_storage(Box::new(MemoryStorageAdapter::new()), true).unwrap();
-        let setting = store.get("dns.local_resolver.enabled");
+        CONFIG_STORE.set_store(Box::new(MemoryStorageAdapter::new()));
+
+        let setting = CONFIG_STORE.get("dns.local_resolver.enabled");
         assert_eq!(setting, Setting::Bool(true));
 
-        store.set("dns.local_resolver.enabled", Setting::Bool(false));
-        let setting = store.get("dns.local_resolver.enabled");
+        CONFIG_STORE.set("dns.local_resolver.enabled", Setting::Bool(false));
+        let setting = CONFIG_STORE.get("dns.local_resolver.enabled");
         assert_eq!(setting, Setting::Bool(false));
     }
 
@@ -297,10 +242,25 @@ mod test {
 
     #[test]
     fn find_settings() {
+        SimpleLogger::new().init().unwrap();
+
+        let enabled = config!(bool "dns.local_resolver.enabled");
+        assert_eq!(enabled, false);
+
+        let max_entries = config!(uint "dns.local_resolver.max_entries");
+        assert_eq!(max_entries, 0);
+
+        let memory_store = Box::new(MemoryStorageAdapter::new());
+        assert!(CONFIG_STORE.set_store(memory_store).is_ok());
+
         let enabled = config!(bool "dns.local_resolver.enabled");
         assert_eq!(enabled, true);
 
         let max_entries = config!(uint "dns.local_resolver.max_entries");
-        assert_eq!(max_entries, 100);
+        assert_eq!(max_entries, 1000);
+
+        let knight = config!(bool "sir.not.appearing.in.this.movie");
+        assert_eq!(knight, true);
+
     }
 }

src/config/storage.rs

@@ -1,4 +1,4 @@
-pub mod json_storage;
-pub mod memory_storage;
-pub mod sqlite_storage;
-pub mod nop_storage;
+pub mod json;
+pub mod memory;
+pub mod sqlite;
+pub mod nop;