Merge pull request #2757 from fermyon/handle-state-dir

Handle State Directory

Author: rylev

crates/factor-key-value-spin/src/lib.rs

@@ -3,41 +3,79 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use anyhow::Context;
+use anyhow::{bail, Context};
 use serde::{Deserialize, Serialize};
 use spin_factor_key_value::runtime_config::spin::MakeKeyValueStore;
 use spin_key_value_sqlite::{DatabaseLocation, KeyValueSqlite};
 
 /// A key-value store that uses SQLite as the backend.
 pub struct SpinKeyValueStore {
     /// The base path or directory for the SQLite database file.
-    base_path: PathBuf,
+    base_path: Option<PathBuf>,
 }
 
 impl SpinKeyValueStore {
     /// Create a new SpinKeyValueStore with the given base path.
-    pub fn new(base_path: PathBuf) -> Self {
+    ///
+    /// If the database directory is None, the database will always be in-memory.
+    /// If it's `Some`, the database will be stored at the combined `base_path` and
+    /// the `path` specified in the runtime configuration.
+    pub fn new(base_path: Option<PathBuf>) -> Self {
         Self { base_path }
     }
 }
 
-/// Runtime configuration for the SQLite key-value store.
+impl MakeKeyValueStore for SpinKeyValueStore {
+    const RUNTIME_CONFIG_TYPE: &'static str = "spin";
+
+    type RuntimeConfig = SpinKeyValueRuntimeConfig;
+
+    type StoreManager = KeyValueSqlite;
+
+    fn make_store(
+        &self,
+        runtime_config: Self::RuntimeConfig,
+    ) -> anyhow::Result<Self::StoreManager> {
+        let location = match (&self.base_path, &runtime_config.path) {
+            // If both the base path and the path are specified, resolve the path against the base path
+            (Some(base_path), Some(path)) => {
+                let path = resolve_relative_path(path, base_path);
+                DatabaseLocation::Path(path)
+            }
+            // If the base path is `None` but path is an absolute path, use the absolute path
+            (None, Some(path)) if path.is_absolute() => DatabaseLocation::Path(path.clone()),
+            // If the base path is `None` but path is a relative path, error out
+            (None, Some(path)) => {
+                bail!(
+                    "key-value store path '{}' is relative, but no base path is set",
+                    path.display()
+                )
+            }
+            // Otherwise, use an in-memory database
+            (None | Some(_), None) => DatabaseLocation::InMemory,
+        };
+        if let DatabaseLocation::Path(path) = &location {
+            // Create the store's parent directory if necessary
+            if let Some(parent) = path.parent().filter(|p| !p.exists()) {
+                fs::create_dir_all(parent)
+                    .context("Failed to create key value store's parent directory")?;
+            }
+        }
+        Ok(KeyValueSqlite::new(location))
+    }
+}
+
+/// The serialized runtime configuration for the SQLite key-value store.
 #[derive(Deserialize, Serialize)]
 pub struct SpinKeyValueRuntimeConfig {
     /// The path to the SQLite database file.
     path: Option<PathBuf>,
 }
 
 impl SpinKeyValueRuntimeConfig {
-    /// The default filename for the SQLite database.
-    const DEFAULT_SPIN_STORE_FILENAME: &'static str = "sqlite_key_value.db";
-
-    /// Create a new runtime configuration with the given directory.
-    ///
-    /// If the database directory is None, the database is in-memory.
-    /// If the database directory is Some, the database is stored in a file in the given directory.
-    pub fn default(default_database_dir: Option<PathBuf>) -> Self {
-        let path = default_database_dir.map(|dir| dir.join(Self::DEFAULT_SPIN_STORE_FILENAME));
+    /// Create a new SpinKeyValueRuntimeConfig with the given parent directory
+    /// where the key-value store will live.
+    pub fn new(path: Option<PathBuf>) -> Self {
         Self { path }
     }
 }
@@ -51,28 +89,3 @@ fn resolve_relative_path(path: &Path, base_dir: &Path) -> PathBuf {
     }
     base_dir.join(path)
 }
-
-impl MakeKeyValueStore for SpinKeyValueStore {
-    const RUNTIME_CONFIG_TYPE: &'static str = "spin";
-
-    type RuntimeConfig = SpinKeyValueRuntimeConfig;
-
-    type StoreManager = KeyValueSqlite;
-
-    fn make_store(
-        &self,
-        runtime_config: Self::RuntimeConfig,
-    ) -> anyhow::Result<Self::StoreManager> {
-        let location = match runtime_config.path {
-            Some(path) => {
-                let path = resolve_relative_path(&path, &self.base_path);
-                // Create the store's parent directory if necessary
-                fs::create_dir_all(path.parent().unwrap())
-                    .context("Failed to create key value store")?;
-                DatabaseLocation::Path(path)
-            }
-            None => DatabaseLocation::InMemory,
-        };
-        Ok(KeyValueSqlite::new(location))
-    }
-}

crates/factor-key-value/src/runtime_config.rs

@@ -13,6 +13,8 @@ pub struct RuntimeConfig {
 
 impl RuntimeConfig {
     /// Adds a store manager for the store with the given label to the runtime configuration.
+    ///
+    /// If a store manager already exists for the given label, it will be replaced.
     pub fn add_store_manager(&mut self, label: String, store_manager: Arc<dyn StoreManager>) {
         self.store_managers.insert(label, store_manager);
     }

crates/factor-key-value/src/runtime_config/spin.rs

@@ -28,11 +28,12 @@ type StoreFromToml =
 /// Creates a `StoreFromToml` function from a `MakeKeyValueStore` implementation.
 fn store_from_toml_fn<T: MakeKeyValueStore>(provider_type: T) -> StoreFromToml {
     Arc::new(move |table| {
-        let runtime_config: T::RuntimeConfig =
-            table.try_into().context("could not parse runtime config")?;
+        let runtime_config: T::RuntimeConfig = table
+            .try_into()
+            .context("could not parse key-value runtime config")?;
         let provider = provider_type
             .make_store(runtime_config)
-            .context("could not make store")?;
+            .context("could not make key-value store from runtime config")?;
         Ok(Arc::new(provider))
     })
 }
@@ -62,8 +63,20 @@ impl RuntimeConfigResolver {
     ///
     /// Users must ensure that the store type for `config` has been registered with
     /// the resolver using [`Self::register_store_type`].
-    pub fn add_default_store(&mut self, label: &'static str, config: StoreConfig) {
-        self.defaults.insert(label, config);
+    pub fn add_default_store<T>(
+        &mut self,
+        label: &'static str,
+        config: T::RuntimeConfig,
+    ) -> anyhow::Result<()>
+    where
+        T: MakeKeyValueStore,
+        T::RuntimeConfig: Serialize,
+    {
+        self.defaults.insert(
+            label,
+            StoreConfig::new(T::RUNTIME_CONFIG_TYPE.to_owned(), config)?,
+        );
+        Ok(())
     }
 
     /// Registers a store type to the resolver.
@@ -96,7 +109,9 @@ impl RuntimeConfigResolver {
 
         let mut runtime_config = RuntimeConfig::default();
         for (label, config) in table {
-            let store_manager = self.store_manager_from_config(config)?;
+            let store_manager = self.store_manager_from_config(config).with_context(|| {
+                format!("could not configure key-value store with label '{label}'")
+            })?;
             runtime_config.add_store_manager(label.clone(), store_manager);
         }
         Ok(Some(runtime_config))
@@ -121,6 +136,8 @@ impl RuntimeConfigResolver {
 impl DefaultLabelResolver for RuntimeConfigResolver {
     fn default(&self, label: &str) -> Option<Arc<dyn StoreManager>> {
         let config = self.defaults.get(label)?;
+        // TODO(rylev): The unwrap here is not ideal. We should return a Result instead.
+        // Piping that through `DefaultLabelResolver` is a bit awkward, though.
         Some(self.store_manager_from_config(config.clone()).unwrap())
     }
 }