From d3619b3474541994c8c58c7d17f55c5f0ece4a6b Mon Sep 17 00:00:00 2001
From: Ryan Levick <ryan.levick@fermyon.com>
Date: Mon, 29 Jul 2024 13:09:42 +0200
Subject: [PATCH 1/3] Make KeyValue factor non-generic

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
---
 .../src/delegating_resolver.rs                | 88 +++++++++++++------
 crates/factor-key-value/src/lib.rs            | 58 ++++++------
 crates/factor-key-value/src/runtime_config.rs | 29 +++---
 crates/factor-key-value/src/store.rs          |  2 +-
 crates/factor-key-value/tests/factor_test.rs  | 80 +++++++++--------
 crates/factors/tests/smoke.rs                 | 31 +++----
 crates/key-value/Cargo.toml                   |  2 +-
 7 files changed, 161 insertions(+), 129 deletions(-)

diff --git a/crates/factor-key-value/src/delegating_resolver.rs b/crates/factor-key-value/src/delegating_resolver.rs
index 89ecc2e1d..b4120a76b 100644
--- a/crates/factor-key-value/src/delegating_resolver.rs
+++ b/crates/factor-key-value/src/delegating_resolver.rs
@@ -1,16 +1,20 @@
-use crate::runtime_config::RuntimeConfigResolver;
-use crate::store::{store_from_toml_fn, MakeKeyValueStore, StoreFromToml};
+use crate::{
+    store::{store_from_toml_fn, MakeKeyValueStore, StoreFromToml},
+    DefaultLabelResolver, RuntimeConfig,
+};
+use anyhow::Context;
 use serde::{Deserialize, Serialize};
 use spin_key_value::StoreManager;
 use std::{collections::HashMap, sync::Arc};
 
-/// A RuntimeConfigResolver that delegates to the appropriate key-value store
-/// for a given label.
+/// Converts from toml based runtime configuration into a [`RuntimeConfig`].
 ///
-/// The store types are registered with the resolver using `add_store_type`. The
-/// default store for a label is registered using `add_default_store`.
+/// Also acts as [`DefaultLabelResolver`].
+///
+/// The various store types (i.e., the "type" field in the toml field) are registered with the
+/// resolver using `add_store_type`. The default store for a label is registered using `add_default_store`.
 #[derive(Default)]
-pub struct DelegatingRuntimeConfigResolver {
+pub struct RuntimeConfigResolver {
     /// A map of store types to a function that returns the appropriate store
     /// manager from runtime config TOML.
     store_types: HashMap<&'static str, StoreFromToml>,
@@ -18,18 +22,25 @@ pub struct DelegatingRuntimeConfigResolver {
     defaults: HashMap<&'static str, StoreConfig>,
 }
 
-impl DelegatingRuntimeConfigResolver {
-    /// Create a new DelegatingRuntimeConfigResolver.
+impl RuntimeConfigResolver {
+    /// Create a new RuntimeConfigResolver.
     pub fn new() -> Self {
-        Self::default()
+        <Self as Default>::default()
     }
 
+    /// Adds a default store configuration for a label.
+    ///
+    /// 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);
     }
 
-    /// Adds a store type to the resolver.
-    pub fn add_store_type<T: MakeKeyValueStore>(&mut self, store_type: T) -> anyhow::Result<()> {
+    /// Registers a store type to the resolver.
+    pub fn register_store_type<T: MakeKeyValueStore>(
+        &mut self,
+        store_type: T,
+    ) -> anyhow::Result<()> {
         if self
             .store_types
             .insert(T::RUNTIME_CONFIG_TYPE, store_from_toml_fn(store_type))
@@ -42,26 +53,49 @@ impl DelegatingRuntimeConfigResolver {
         }
         Ok(())
     }
-}
 
-impl RuntimeConfigResolver for DelegatingRuntimeConfigResolver {
-    type Config = StoreConfig;
-
-    fn get_store(&self, config: StoreConfig) -> anyhow::Result<Arc<dyn StoreManager>> {
-        let store_kind = config.type_.as_str();
-        let store_from_toml = self
-            .store_types
-            .get(store_kind)
-            .ok_or_else(|| anyhow::anyhow!("unknown store kind: {}", store_kind))?;
-        store_from_toml(config.config)
+    /// Resolves a toml table into a runtime config.
+    pub fn resolve_from_toml(
+        &self,
+        table: &Option<toml::Table>,
+    ) -> anyhow::Result<Option<RuntimeConfig>> {
+        let Some(table) = table.as_ref().and_then(|t| t.get("key_value_store")) else {
+            return Ok(None);
+        };
+        let mut store_configs = HashMap::new();
+        for (label, config) in table
+            .as_table()
+            .context("expected a 'key_value_store' to contain toml table")?
+        {
+            let config: StoreConfig = config.clone().try_into()?;
+            let store_manager = self.store_manager_from_config(config)?;
+            store_configs.insert(label.clone(), store_manager);
+        }
+        Ok(Some(RuntimeConfig {
+            store_managers: store_configs,
+        }))
     }
 
-    /// Get the default store manager for the given label.
+    /// Given a [`StoreConfig`], returns a store manager.
     ///
-    /// Returns None if no default store is registered for the label.
-    fn default_store(&self, label: &str) -> Option<Arc<dyn StoreManager>> {
+    /// Errors if there is no [`MakeKeyValueStore`] registered for the store config's type
+    /// or if the store manager cannot be created from the config.
+    fn store_manager_from_config(
+        &self,
+        config: StoreConfig,
+    ) -> anyhow::Result<Arc<dyn StoreManager>> {
+        let config_type = config.type_.as_str();
+        let maker = self.store_types.get(config_type).with_context(|| {
+            format!("the store type '{config_type}' was not registered with the config resolver")
+        })?;
+        maker(config.config)
+    }
+}
+
+impl DefaultLabelResolver for RuntimeConfigResolver {
+    fn default(&self, label: &str) -> Option<Arc<dyn StoreManager>> {
         let config = self.defaults.get(label)?;
-        self.get_store(config.clone()).ok()
+        Some(self.store_manager_from_config(config.clone()).unwrap())
     }
 }
 
diff --git a/crates/factor-key-value/src/lib.rs b/crates/factor-key-value/src/lib.rs
index f83b417ab..ea27fa0ce 100644
--- a/crates/factor-key-value/src/lib.rs
+++ b/crates/factor-key-value/src/lib.rs
@@ -1,7 +1,7 @@
 pub mod delegating_resolver;
 mod runtime_config;
 mod store;
-pub use delegating_resolver::{DelegatingRuntimeConfigResolver, StoreConfig};
+pub use delegating_resolver::{RuntimeConfigResolver, StoreConfig};
 
 use std::{
     collections::{HashMap, HashSet},
@@ -9,7 +9,6 @@ use std::{
 };
 
 use anyhow::ensure;
-use runtime_config::{RuntimeConfig, RuntimeConfigResolver};
 use spin_factors::{
     ConfigureAppContext, Factor, FactorInstanceBuilder, InitContext, InstanceBuilders,
     PrepareContext, RuntimeFactors,
@@ -18,27 +17,29 @@ use spin_key_value::{
     CachingStoreManager, DefaultManagerGetter, DelegatingStoreManager, KeyValueDispatch,
     StoreManager, KEY_VALUE_STORES_KEY,
 };
+
+pub use runtime_config::RuntimeConfig;
 pub use store::MakeKeyValueStore;
 
 /// A factor that provides key-value storage.
-pub struct KeyValueFactor<R> {
-    /// Resolves runtime configuration into store managers.
-    runtime_config_resolver: Arc<R>,
+pub struct KeyValueFactor {
+    default_label_resolver: Arc<dyn DefaultLabelResolver>,
 }
 
-impl<R> KeyValueFactor<R> {
+impl KeyValueFactor {
     /// Create a new KeyValueFactor.  
     ///  
-    /// The `runtime_config_resolver` is used to resolve runtime configuration into store managers.
-    pub fn new(runtime_config_resolver: R) -> Self {
+    /// The `default_label_resolver` is used to resolver store managers for labels that
+    /// are not defined in the runtime configuration.
+    pub fn new(default_label_resolver: impl DefaultLabelResolver + 'static) -> Self {
         Self {
-            runtime_config_resolver: Arc::new(runtime_config_resolver),
+            default_label_resolver: Arc::new(default_label_resolver),
         }
     }
 }
 
-impl<R: RuntimeConfigResolver + 'static> Factor for KeyValueFactor<R> {
-    type RuntimeConfig = RuntimeConfig<R::Config>;
+impl Factor for KeyValueFactor {
+    type RuntimeConfig = RuntimeConfig;
     type AppState = AppState;
     type InstanceBuilder = InstanceBuilder;
 
@@ -52,23 +53,10 @@ impl<R: RuntimeConfigResolver + 'static> Factor for KeyValueFactor<R> {
         &self,
         mut ctx: ConfigureAppContext<T, Self>,
     ) -> anyhow::Result<Self::AppState> {
-        // Build store manager from runtime config
-        let mut store_managers: HashMap<String, Arc<dyn StoreManager>> = HashMap::new();
-        if let Some(runtime_config) = ctx.take_runtime_config() {
-            for (store_label, config) in runtime_config.store_configs {
-                if let std::collections::hash_map::Entry::Vacant(e) =
-                    store_managers.entry(store_label)
-                {
-                    // Only add manager for labels that are not already configured. Runtime config
-                    // takes top-down precedence.
-                    let store = self.runtime_config_resolver.get_store(config)?;
-                    e.insert(store);
-                }
-            }
-        }
-        let resolver_clone = self.runtime_config_resolver.clone();
+        let store_managers = ctx.take_runtime_config().unwrap_or_default();
+        let default_label_resolver = self.default_label_resolver.clone();
         let default_fn: DefaultManagerGetter =
-            Arc::new(move |label| resolver_clone.default_store(label));
+            Arc::new(move |label| default_label_resolver.default(label));
 
         let delegating_manager = DelegatingStoreManager::new(store_managers, default_fn);
         let caching_manager = CachingStoreManager::new(delegating_manager);
@@ -87,7 +75,7 @@ impl<R: RuntimeConfigResolver + 'static> Factor for KeyValueFactor<R> {
                 // TODO: port nicer errors from KeyValueComponent (via error type?)
                 ensure!(
                     store_manager_manager.is_defined(label)
-                        || self.runtime_config_resolver.default_store(label).is_some(),
+                        || self.default_label_resolver.default(label).is_some(),
                     "unknown key_value_stores label {label:?} for component {component_id:?}"
                 );
             }
@@ -159,3 +147,17 @@ impl FactorInstanceBuilder for InstanceBuilder {
         Ok(dispatch)
     }
 }
+
+/// Resolves a label to a default [`StoreManager`].
+pub trait DefaultLabelResolver: Send + Sync {
+    /// If there is no runtime configuration for a given store label, return a default store manager.
+    ///
+    /// If `Option::None` is returned, the store is not allowed.
+    fn default(&self, label: &str) -> Option<Arc<dyn StoreManager>>;
+}
+
+impl<T: DefaultLabelResolver> DefaultLabelResolver for Arc<T> {
+    fn default(&self, label: &str) -> Option<Arc<dyn StoreManager>> {
+        self.as_ref().default(label)
+    }
+}
diff --git a/crates/factor-key-value/src/runtime_config.rs b/crates/factor-key-value/src/runtime_config.rs
index 02f411abd..6959ff858 100644
--- a/crates/factor-key-value/src/runtime_config.rs
+++ b/crates/factor-key-value/src/runtime_config.rs
@@ -1,28 +1,19 @@
 use std::{collections::HashMap, sync::Arc};
 
-use serde::{de::DeserializeOwned, Deserialize};
-use spin_factors::anyhow;
 use spin_key_value::StoreManager;
 
 /// Runtime configuration for all key value stores.
-#[derive(Deserialize)]
-#[serde(transparent)]
-pub struct RuntimeConfig<C> {
-    /// Map of store names to store configurations.
-    pub store_configs: HashMap<String, C>,
+#[derive(Default)]
+pub struct RuntimeConfig {
+    /// Map of store names to store managers.
+    pub store_managers: HashMap<String, Arc<dyn StoreManager>>,
 }
 
-/// Resolves some piece of runtime configuration to a key value store manager.
-pub trait RuntimeConfigResolver: Send + Sync {
-    /// The type of configuration that this resolver can handle.
-    type Config: DeserializeOwned;
+impl IntoIterator for RuntimeConfig {
+    type Item = (String, Arc<dyn StoreManager>);
+    type IntoIter = std::collections::hash_map::IntoIter<String, Arc<dyn StoreManager>>;
 
-    /// Get a store manager for a given config.
-    fn get_store(&self, config: Self::Config) -> anyhow::Result<Arc<dyn StoreManager>>;
-
-    /// Returns a default store manager for a given label. Should only be called
-    /// if there is no runtime configuration for the label.
-    ///
-    /// If `Option::None` is returned, the database is not allowed.
-    fn default_store(&self, label: &str) -> Option<Arc<dyn StoreManager>>;
+    fn into_iter(self) -> Self::IntoIter {
+        self.store_managers.into_iter()
+    }
 }
diff --git a/crates/factor-key-value/src/store.rs b/crates/factor-key-value/src/store.rs
index 24c5e95bb..4dcf4e3b0 100644
--- a/crates/factor-key-value/src/store.rs
+++ b/crates/factor-key-value/src/store.rs
@@ -4,7 +4,7 @@ use anyhow::Context;
 use serde::de::DeserializeOwned;
 use spin_key_value::StoreManager;
 
-/// Defines the construction of a key value store.
+/// Defines the construction of a key value store from a serializable runtime config.
 pub trait MakeKeyValueStore: 'static + Send + Sync {
     /// Unique type identifier for the store.
     const RUNTIME_CONFIG_TYPE: &'static str;
diff --git a/crates/factor-key-value/tests/factor_test.rs b/crates/factor-key-value/tests/factor_test.rs
index c0c24eac0..246ee0266 100644
--- a/crates/factor-key-value/tests/factor_test.rs
+++ b/crates/factor-key-value/tests/factor_test.rs
@@ -1,24 +1,21 @@
 use anyhow::Context;
 use spin_factor_key_value::{
-    DelegatingRuntimeConfigResolver, KeyValueFactor, MakeKeyValueStore, StoreConfig,
+    KeyValueFactor, MakeKeyValueStore, RuntimeConfig, RuntimeConfigResolver, StoreConfig,
 };
 use spin_factor_key_value_redis::RedisKeyValueStore;
 use spin_factor_key_value_spin::{SpinKeyValueRuntimeConfig, SpinKeyValueStore};
-use spin_factors::{
-    Factor, FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer, RuntimeFactors,
-};
+use spin_factors::{FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer, RuntimeFactors};
 use spin_factors_test::{toml, TestEnvironment};
-use std::collections::HashSet;
+use std::{collections::HashSet, sync::Arc};
 
 #[derive(RuntimeFactors)]
 struct TestFactors {
-    key_value: KeyValueFactor<DelegatingRuntimeConfigResolver>,
+    key_value: KeyValueFactor,
 }
 
-fn default_key_value_resolver(
-) -> anyhow::Result<(DelegatingRuntimeConfigResolver, tempdir::TempDir)> {
-    let mut test_resolver = DelegatingRuntimeConfigResolver::new();
-    test_resolver.add_store_type(SpinKeyValueStore::new(
+fn default_key_value_resolver() -> anyhow::Result<(RuntimeConfigResolver, tempdir::TempDir)> {
+    let mut test_resolver = RuntimeConfigResolver::new();
+    test_resolver.register_store_type(SpinKeyValueStore::new(
         std::env::current_dir().context("failed to get current directory")?,
     ))?;
     let tmp_dir = tempdir::TempDir::new("example")?;
@@ -59,12 +56,13 @@ async fn run_test_with_config_and_stores_for_label(
     store_types: Vec<impl MakeKeyValueStore>,
     labels: Vec<&str>,
 ) -> anyhow::Result<()> {
-    let mut test_resolver = DelegatingRuntimeConfigResolver::new();
+    let mut test_resolver = RuntimeConfigResolver::new();
     for store_type in store_types {
-        test_resolver.add_store_type(store_type)?;
+        test_resolver.register_store_type(store_type)?;
     }
+    let test_resolver = Arc::new(test_resolver);
     let factors = TestFactors {
-        key_value: KeyValueFactor::new(test_resolver),
+        key_value: KeyValueFactor::new(test_resolver.clone()),
     };
     let labels_clone = labels.clone();
     let env = TestEnvironment::new(factors)
@@ -73,7 +71,7 @@ async fn run_test_with_config_and_stores_for_label(
             source = "does-not-exist.wasm"
             key_value_stores = labels_clone
         })
-        .runtime_config(TomlConfig(runtime_config))?;
+        .runtime_config(TomlConfig::new(test_resolver, runtime_config))?;
     let state = env.build_instance_state().await?;
     assert_eq!(
         labels,
@@ -192,11 +190,12 @@ async fn misconfigured_spin_key_value_fails() -> anyhow::Result<()> {
 #[tokio::test]
 async fn multiple_custom_key_value_uses_first_store() -> anyhow::Result<()> {
     let tmp_dir = tempdir::TempDir::new("example")?;
-    let mut test_resolver = DelegatingRuntimeConfigResolver::new();
-    test_resolver.add_store_type(RedisKeyValueStore)?;
-    test_resolver.add_store_type(SpinKeyValueStore::new(tmp_dir.path().to_owned()))?;
+    let mut test_resolver = RuntimeConfigResolver::new();
+    test_resolver.register_store_type(RedisKeyValueStore)?;
+    test_resolver.register_store_type(SpinKeyValueStore::new(tmp_dir.path().to_owned()))?;
+    let test_resolver = Arc::new(test_resolver);
     let factors = TestFactors {
-        key_value: KeyValueFactor::new(test_resolver),
+        key_value: KeyValueFactor::new(test_resolver.clone()),
     };
     let env = TestEnvironment::new(factors)
         .extend_manifest(toml! {
@@ -204,15 +203,18 @@ async fn multiple_custom_key_value_uses_first_store() -> anyhow::Result<()> {
             source = "does-not-exist.wasm"
             key_value_stores = ["custom"]
         })
-        .runtime_config(TomlConfig(Some(toml::toml! {
-            [key_value_store.custom]
-            type = "spin"
-            path = "custom.db"
-
-            [key_value_store.custom]
-            type = "redis"
-            url = "redis://localhost:6379"
-        })))?;
+        .runtime_config(TomlConfig::new(
+            test_resolver,
+            Some(toml::toml! {
+                [key_value_store.custom]
+                type = "spin"
+                path = "custom.db"
+
+                [key_value_store.custom]
+                type = "redis"
+                url = "redis://localhost:6379"
+            }),
+        ))?;
     let state = env.build_instance_state().await?;
 
     assert_eq!(
@@ -224,7 +226,16 @@ async fn multiple_custom_key_value_uses_first_store() -> anyhow::Result<()> {
     Ok(())
 }
 
-struct TomlConfig(Option<toml::Table>);
+struct TomlConfig {
+    resolver: Arc<RuntimeConfigResolver>,
+    toml: Option<toml::Table>,
+}
+
+impl TomlConfig {
+    fn new(resolver: Arc<RuntimeConfigResolver>, toml: Option<toml::Table>) -> Self {
+        Self { resolver, toml }
+    }
+}
 
 impl TryFrom<TomlConfig> for TestFactorsRuntimeConfig {
     type Error = anyhow::Error;
@@ -234,16 +245,9 @@ impl TryFrom<TomlConfig> for TestFactorsRuntimeConfig {
     }
 }
 
-impl FactorRuntimeConfigSource<KeyValueFactor<DelegatingRuntimeConfigResolver>> for TomlConfig {
-    fn get_runtime_config(
-        &mut self,
-    ) -> anyhow::Result<
-        Option<<KeyValueFactor<DelegatingRuntimeConfigResolver> as Factor>::RuntimeConfig>,
-    > {
-        let Some(table) = self.0.as_ref().and_then(|t| t.get("key_value_store")) else {
-            return Ok(None);
-        };
-        Ok(Some(table.clone().try_into()?))
+impl FactorRuntimeConfigSource<KeyValueFactor> for TomlConfig {
+    fn get_runtime_config(&mut self) -> anyhow::Result<Option<RuntimeConfig>> {
+        self.resolver.resolve_from_toml(&self.toml)
     }
 }
 
diff --git a/crates/factors/tests/smoke.rs b/crates/factors/tests/smoke.rs
index 205775ca1..94105b729 100644
--- a/crates/factors/tests/smoke.rs
+++ b/crates/factors/tests/smoke.rs
@@ -1,10 +1,10 @@
-use std::path::PathBuf;
+use std::{path::PathBuf, sync::Arc};
 
 use anyhow::Context;
 use http_body_util::BodyExt;
 use spin_app::App;
 use spin_factor_key_value::{
-    delegating_resolver::{DelegatingRuntimeConfigResolver, StoreConfig},
+    delegating_resolver::{RuntimeConfigResolver, StoreConfig},
     KeyValueFactor, MakeKeyValueStore,
 };
 use spin_factor_key_value_redis::RedisKeyValueStore;
@@ -27,7 +27,7 @@ struct Factors {
     variables: VariablesFactor<VariableProviderConfiguration>,
     outbound_networking: OutboundNetworkingFactor,
     outbound_http: OutboundHttpFactor,
-    key_value: KeyValueFactor<DelegatingRuntimeConfigResolver>,
+    key_value: KeyValueFactor,
 }
 
 struct Data {
@@ -43,7 +43,7 @@ impl AsMut<FactorsInstanceState> for Data {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn smoke_test_works() -> anyhow::Result<()> {
-    let mut key_value_resolver = DelegatingRuntimeConfigResolver::default();
+    let mut key_value_resolver = RuntimeConfigResolver::default();
     let default_config =
         SpinKeyValueRuntimeConfig::default(Some(PathBuf::from("tests/smoke-app/.spin")));
     key_value_resolver.add_default_store(
@@ -53,17 +53,18 @@ async fn smoke_test_works() -> anyhow::Result<()> {
             config: toml::value::Table::try_from(default_config)?,
         },
     );
-    key_value_resolver.add_store_type(SpinKeyValueStore::new(
+    key_value_resolver.register_store_type(SpinKeyValueStore::new(
         std::env::current_dir().context("failed to get current directory")?,
     ))?;
-    key_value_resolver.add_store_type(RedisKeyValueStore)?;
+    key_value_resolver.register_store_type(RedisKeyValueStore)?;
+    let key_value_resolver = Arc::new(key_value_resolver);
 
     let mut factors = Factors {
         wasi: WasiFactor::new(DummyFilesMounter),
         variables: VariablesFactor::default(),
         outbound_networking: OutboundNetworkingFactor,
         outbound_http: OutboundHttpFactor,
-        key_value: KeyValueFactor::new(key_value_resolver),
+        key_value: KeyValueFactor::new(key_value_resolver.clone()),
     };
 
     factors.variables.add_provider_resolver(StaticVariables)?;
@@ -81,7 +82,8 @@ async fn smoke_test_works() -> anyhow::Result<()> {
 
     factors.init(&mut linker)?;
 
-    let configured_app = factors.configure_app(app, TestSource.try_into()?)?;
+    let source = TestSource { key_value_resolver };
+    let configured_app = factors.configure_app(app, source.try_into()?)?;
     let builders = factors.prepare(&configured_app, "smoke-app")?;
     let state = factors.build_instance_state(builders)?;
 
@@ -143,7 +145,9 @@ async fn smoke_test_works() -> anyhow::Result<()> {
     Ok(())
 }
 
-struct TestSource;
+struct TestSource {
+    key_value_resolver: Arc<RuntimeConfigResolver>,
+}
 
 impl TryFrom<TestSource> for FactorsRuntimeConfig {
     type Error = anyhow::Error;
@@ -153,19 +157,16 @@ impl TryFrom<TestSource> for FactorsRuntimeConfig {
     }
 }
 
-impl FactorRuntimeConfigSource<KeyValueFactor<DelegatingRuntimeConfigResolver>> for TestSource {
+impl FactorRuntimeConfigSource<KeyValueFactor> for TestSource {
     fn get_runtime_config(
         &mut self,
-    ) -> anyhow::Result<
-        Option<<KeyValueFactor<DelegatingRuntimeConfigResolver> as Factor>::RuntimeConfig>,
-    > {
+    ) -> anyhow::Result<Option<<KeyValueFactor as Factor>::RuntimeConfig>> {
         let config = toml::toml! {
             [other]
             type = "redis"
             url = "redis://localhost:6379"
         };
-
-        Ok(Some(config.try_into()?))
+        self.key_value_resolver.resolve_from_toml(&Some(config))
     }
 }
 
diff --git a/crates/key-value/Cargo.toml b/crates/key-value/Cargo.toml
index fa8a44073..e5b53aec5 100644
--- a/crates/key-value/Cargo.toml
+++ b/crates/key-value/Cargo.toml
@@ -9,7 +9,7 @@ doctest = false
 
 [dependencies]
 anyhow = "1.0"
-tokio = { version = "1", features = ["macros", "sync"] }
+tokio = { version = "1", features = ["macros", "sync", "rt"] }
 spin-app = { path = "../app" }
 spin-core = { path = "../core" }
 spin-world = { path = "../world" }

From c4d6f456d51419296a6cae05722d66639dfbd7f6 Mon Sep 17 00:00:00 2001
From: Ryan Levick <ryan.levick@fermyon.com>
Date: Mon, 29 Jul 2024 13:52:43 +0200
Subject: [PATCH 2/3] Dedicated Spin CLI module

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
---
 crates/factor-key-value-azure/src/lib.rs      |  2 +-
 crates/factor-key-value-redis/src/lib.rs      |  2 +-
 crates/factor-key-value-spin/src/lib.rs       |  6 +--
 crates/factor-key-value/src/lib.rs            |  6 +--
 crates/factor-key-value/src/runtime_config.rs |  2 +
 .../spin.rs}                                  | 40 ++++++++++++++++---
 crates/factor-key-value/src/store.rs          | 35 ----------------
 crates/factor-key-value/tests/factor_test.rs  |  3 +-
 crates/factors/tests/smoke.rs                 |  4 +-
 9 files changed, 47 insertions(+), 53 deletions(-)
 rename crates/factor-key-value/src/{delegating_resolver.rs => runtime_config/spin.rs} (73%)
 delete mode 100644 crates/factor-key-value/src/store.rs

diff --git a/crates/factor-key-value-azure/src/lib.rs b/crates/factor-key-value-azure/src/lib.rs
index 949bfb186..812a1df07 100644
--- a/crates/factor-key-value-azure/src/lib.rs
+++ b/crates/factor-key-value-azure/src/lib.rs
@@ -1,5 +1,5 @@
 use serde::Deserialize;
-use spin_factor_key_value::MakeKeyValueStore;
+use spin_factor_key_value::runtime_config::spin::MakeKeyValueStore;
 use spin_key_value_azure::{
     KeyValueAzureCosmos, KeyValueAzureCosmosAuthOptions, KeyValueAzureCosmosRuntimeConfigOptions,
 };
diff --git a/crates/factor-key-value-redis/src/lib.rs b/crates/factor-key-value-redis/src/lib.rs
index 39fadd26f..6bea163e1 100644
--- a/crates/factor-key-value-redis/src/lib.rs
+++ b/crates/factor-key-value-redis/src/lib.rs
@@ -1,5 +1,5 @@
 use serde::Deserialize;
-use spin_factor_key_value::MakeKeyValueStore;
+use spin_factor_key_value::runtime_config::spin::MakeKeyValueStore;
 use spin_key_value_redis::KeyValueRedis;
 
 /// A key-value store that uses Redis as the backend.
diff --git a/crates/factor-key-value-spin/src/lib.rs b/crates/factor-key-value-spin/src/lib.rs
index 7c107b4e7..38e9ef442 100644
--- a/crates/factor-key-value-spin/src/lib.rs
+++ b/crates/factor-key-value-spin/src/lib.rs
@@ -5,7 +5,7 @@ use std::{
 
 use anyhow::Context;
 use serde::{Deserialize, Serialize};
-use spin_factor_key_value::MakeKeyValueStore;
+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.
@@ -32,10 +32,10 @@ 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 state directory.
+    /// 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 state directory.
+    /// 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));
         Self { path }
diff --git a/crates/factor-key-value/src/lib.rs b/crates/factor-key-value/src/lib.rs
index ea27fa0ce..54bec6242 100644
--- a/crates/factor-key-value/src/lib.rs
+++ b/crates/factor-key-value/src/lib.rs
@@ -1,7 +1,4 @@
-pub mod delegating_resolver;
-mod runtime_config;
-mod store;
-pub use delegating_resolver::{RuntimeConfigResolver, StoreConfig};
+pub mod runtime_config;
 
 use std::{
     collections::{HashMap, HashSet},
@@ -19,7 +16,6 @@ use spin_key_value::{
 };
 
 pub use runtime_config::RuntimeConfig;
-pub use store::MakeKeyValueStore;
 
 /// A factor that provides key-value storage.
 pub struct KeyValueFactor {
diff --git a/crates/factor-key-value/src/runtime_config.rs b/crates/factor-key-value/src/runtime_config.rs
index 6959ff858..e2b3c9ff9 100644
--- a/crates/factor-key-value/src/runtime_config.rs
+++ b/crates/factor-key-value/src/runtime_config.rs
@@ -1,3 +1,5 @@
+pub mod spin;
+
 use std::{collections::HashMap, sync::Arc};
 
 use spin_key_value::StoreManager;
diff --git a/crates/factor-key-value/src/delegating_resolver.rs b/crates/factor-key-value/src/runtime_config/spin.rs
similarity index 73%
rename from crates/factor-key-value/src/delegating_resolver.rs
rename to crates/factor-key-value/src/runtime_config/spin.rs
index b4120a76b..36b5266d8 100644
--- a/crates/factor-key-value/src/delegating_resolver.rs
+++ b/crates/factor-key-value/src/runtime_config/spin.rs
@@ -1,12 +1,42 @@
-use crate::{
-    store::{store_from_toml_fn, MakeKeyValueStore, StoreFromToml},
-    DefaultLabelResolver, RuntimeConfig,
-};
-use anyhow::Context;
+//! Runtime configuration implementation used by Spin CLI.
+
+use crate::{DefaultLabelResolver, RuntimeConfig};
+use anyhow::Context as _;
+use serde::de::DeserializeOwned;
 use serde::{Deserialize, Serialize};
 use spin_key_value::StoreManager;
 use std::{collections::HashMap, sync::Arc};
 
+/// Defines the construction of a key value store from a serialized runtime config.
+pub trait MakeKeyValueStore: 'static + Send + Sync {
+    /// Unique type identifier for the store.
+    const RUNTIME_CONFIG_TYPE: &'static str;
+    /// Runtime configuration for the store.
+    type RuntimeConfig: DeserializeOwned;
+    /// The store manager for the store.
+    type StoreManager: StoreManager;
+
+    /// Creates a new store manager from the runtime configuration.
+    fn make_store(&self, runtime_config: Self::RuntimeConfig)
+        -> anyhow::Result<Self::StoreManager>;
+}
+
+/// A function that creates a store manager from a TOML table.
+type StoreFromToml =
+    Box<dyn Fn(toml::Table) -> anyhow::Result<Arc<dyn StoreManager>> + Send + Sync>;
+
+/// Creates a `StoreFromToml` function from a `MakeKeyValueStore` implementation.
+fn store_from_toml_fn<T: MakeKeyValueStore>(provider_type: T) -> StoreFromToml {
+    Box::new(move |table| {
+        let runtime_config: T::RuntimeConfig =
+            table.try_into().context("could not parse runtime config")?;
+        let provider = provider_type
+            .make_store(runtime_config)
+            .context("could not make store")?;
+        Ok(Arc::new(provider))
+    })
+}
+
 /// Converts from toml based runtime configuration into a [`RuntimeConfig`].
 ///
 /// Also acts as [`DefaultLabelResolver`].
diff --git a/crates/factor-key-value/src/store.rs b/crates/factor-key-value/src/store.rs
deleted file mode 100644
index 4dcf4e3b0..000000000
--- a/crates/factor-key-value/src/store.rs
+++ /dev/null
@@ -1,35 +0,0 @@
-use std::sync::Arc;
-
-use anyhow::Context;
-use serde::de::DeserializeOwned;
-use spin_key_value::StoreManager;
-
-/// Defines the construction of a key value store from a serializable runtime config.
-pub trait MakeKeyValueStore: 'static + Send + Sync {
-    /// Unique type identifier for the store.
-    const RUNTIME_CONFIG_TYPE: &'static str;
-    /// Runtime configuration for the store.
-    type RuntimeConfig: DeserializeOwned;
-    /// The store manager for the store.
-    type StoreManager: StoreManager;
-
-    /// Creates a new store manager from the runtime configuration.
-    fn make_store(&self, runtime_config: Self::RuntimeConfig)
-        -> anyhow::Result<Self::StoreManager>;
-}
-
-/// A function that creates a store manager from a TOML table.
-pub(crate) type StoreFromToml =
-    Box<dyn Fn(toml::Table) -> anyhow::Result<Arc<dyn StoreManager>> + Send + Sync>;
-
-/// Creates a `StoreFromToml` function from a `MakeKeyValueStore` implementation.
-pub(crate) fn store_from_toml_fn<T: MakeKeyValueStore>(provider_type: T) -> StoreFromToml {
-    Box::new(move |table| {
-        let runtime_config: T::RuntimeConfig =
-            table.try_into().context("could not parse runtime config")?;
-        let provider = provider_type
-            .make_store(runtime_config)
-            .context("could not make store")?;
-        Ok(Arc::new(provider))
-    })
-}
diff --git a/crates/factor-key-value/tests/factor_test.rs b/crates/factor-key-value/tests/factor_test.rs
index 246ee0266..978160601 100644
--- a/crates/factor-key-value/tests/factor_test.rs
+++ b/crates/factor-key-value/tests/factor_test.rs
@@ -1,6 +1,7 @@
 use anyhow::Context;
 use spin_factor_key_value::{
-    KeyValueFactor, MakeKeyValueStore, RuntimeConfig, RuntimeConfigResolver, StoreConfig,
+    runtime_config::spin::{MakeKeyValueStore, RuntimeConfigResolver, StoreConfig},
+    KeyValueFactor, RuntimeConfig,
 };
 use spin_factor_key_value_redis::RedisKeyValueStore;
 use spin_factor_key_value_spin::{SpinKeyValueRuntimeConfig, SpinKeyValueStore};
diff --git a/crates/factors/tests/smoke.rs b/crates/factors/tests/smoke.rs
index 94105b729..37ed6e10f 100644
--- a/crates/factors/tests/smoke.rs
+++ b/crates/factors/tests/smoke.rs
@@ -4,8 +4,8 @@ use anyhow::Context;
 use http_body_util::BodyExt;
 use spin_app::App;
 use spin_factor_key_value::{
-    delegating_resolver::{RuntimeConfigResolver, StoreConfig},
-    KeyValueFactor, MakeKeyValueStore,
+    runtime_config::spin::{MakeKeyValueStore, RuntimeConfigResolver, StoreConfig},
+    KeyValueFactor,
 };
 use spin_factor_key_value_redis::RedisKeyValueStore;
 use spin_factor_key_value_spin::{SpinKeyValueRuntimeConfig, SpinKeyValueStore};

From afebc8efe8d2c4bd94271d82b0fbcb47506306b7 Mon Sep 17 00:00:00 2001
From: Ryan Levick <ryan.levick@fermyon.com>
Date: Tue, 30 Jul 2024 11:40:40 +0200
Subject: [PATCH 3/3] PR feedback

Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
---
 crates/factor-key-value/src/lib.rs               |  2 +-
 crates/factor-key-value/src/runtime_config.rs    |  9 ++++++++-
 .../factor-key-value/src/runtime_config/spin.rs  | 16 ++++++----------
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/crates/factor-key-value/src/lib.rs b/crates/factor-key-value/src/lib.rs
index 54bec6242..1e71bb86a 100644
--- a/crates/factor-key-value/src/lib.rs
+++ b/crates/factor-key-value/src/lib.rs
@@ -25,7 +25,7 @@ pub struct KeyValueFactor {
 impl KeyValueFactor {
     /// Create a new KeyValueFactor.  
     ///  
-    /// The `default_label_resolver` is used to resolver store managers for labels that
+    /// The `default_label_resolver` is used to resolve store managers for labels that
     /// are not defined in the runtime configuration.
     pub fn new(default_label_resolver: impl DefaultLabelResolver + 'static) -> Self {
         Self {
diff --git a/crates/factor-key-value/src/runtime_config.rs b/crates/factor-key-value/src/runtime_config.rs
index e2b3c9ff9..d5895e75a 100644
--- a/crates/factor-key-value/src/runtime_config.rs
+++ b/crates/factor-key-value/src/runtime_config.rs
@@ -8,7 +8,14 @@ use spin_key_value::StoreManager;
 #[derive(Default)]
 pub struct RuntimeConfig {
     /// Map of store names to store managers.
-    pub store_managers: HashMap<String, Arc<dyn StoreManager>>,
+    store_managers: HashMap<String, Arc<dyn StoreManager>>,
+}
+
+impl RuntimeConfig {
+    /// Adds a store manager for the store with the given label to the runtime configuration.
+    pub fn add_store_manager(&mut self, label: String, store_manager: Arc<dyn StoreManager>) {
+        self.store_managers.insert(label, store_manager);
+    }
 }
 
 impl IntoIterator for RuntimeConfig {
diff --git a/crates/factor-key-value/src/runtime_config/spin.rs b/crates/factor-key-value/src/runtime_config/spin.rs
index 36b5266d8..20c218447 100644
--- a/crates/factor-key-value/src/runtime_config/spin.rs
+++ b/crates/factor-key-value/src/runtime_config/spin.rs
@@ -92,18 +92,14 @@ impl RuntimeConfigResolver {
         let Some(table) = table.as_ref().and_then(|t| t.get("key_value_store")) else {
             return Ok(None);
         };
-        let mut store_configs = HashMap::new();
-        for (label, config) in table
-            .as_table()
-            .context("expected a 'key_value_store' to contain toml table")?
-        {
-            let config: StoreConfig = config.clone().try_into()?;
+        let table: HashMap<String, StoreConfig> = table.clone().try_into()?;
+
+        let mut runtime_config = RuntimeConfig::default();
+        for (label, config) in table {
             let store_manager = self.store_manager_from_config(config)?;
-            store_configs.insert(label.clone(), store_manager);
+            runtime_config.add_store_manager(label.clone(), store_manager);
         }
-        Ok(Some(RuntimeConfig {
-            store_managers: store_configs,
-        }))
+        Ok(Some(runtime_config))
     }
 
     /// Given a [`StoreConfig`], returns a store manager.