Remove custom default label handling for key-value

Signed-off-by: Ryan Levick <[email protected]>

Author: rylev

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

@@ -12,7 +12,6 @@ use spin_factors::{
     ConfigureAppContext, Factor, FactorInstanceBuilder, InitContext, PrepareContext, RuntimeFactors,
 };
 use spin_locked_app::MetadataKey;
-use util::DefaultManagerGetter;
 
 /// Metadata key for key-value stores.
 pub const KEY_VALUE_STORES_KEY: MetadataKey<Vec<String>> = MetadataKey::new("key_value_stores");
@@ -21,19 +20,15 @@ pub use runtime_config::RuntimeConfig;
 pub use util::{CachingStoreManager, DelegatingStoreManager};
 
 /// A factor that provides key-value storage.
+#[derive(Default)]
 pub struct KeyValueFactor {
-    default_label_resolver: Arc<dyn DefaultLabelResolver>,
+    _priv: (),
 }
 
 impl KeyValueFactor {
     /// Create a new KeyValueFactor.
-    ///
-    /// 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 {
-            default_label_resolver: Arc::new(default_label_resolver),
-        }
+    pub fn new() -> Self {
+        Self { _priv: () }
     }
 }
 
@@ -53,11 +48,8 @@ impl Factor for KeyValueFactor {
         mut ctx: ConfigureAppContext<T, Self>,
     ) -> anyhow::Result<Self::AppState> {
         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| default_label_resolver.default(label));
 
-        let delegating_manager = DelegatingStoreManager::new(store_managers, default_fn);
+        let delegating_manager = DelegatingStoreManager::new(store_managers);
         let caching_manager = CachingStoreManager::new(delegating_manager);
         let store_manager_manager = Arc::new(caching_manager);
 
@@ -73,8 +65,7 @@ impl Factor for KeyValueFactor {
             for label in &key_value_stores {
                 // TODO: port nicer errors from KeyValueComponent (via error type?)
                 ensure!(
-                    store_manager_manager.is_defined(label)
-                        || self.default_label_resolver.default(label).is_some(),
+                    store_manager_manager.is_defined(label),
                     "unknown key_value_stores label {label:?} for component {component_id:?}"
                 );
             }
@@ -164,17 +155,3 @@ 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)
-    }
-}

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

@@ -1,7 +1,6 @@
 //! Runtime configuration implementation used by Spin CLI.
 
-use crate::StoreManager;
-use crate::{DefaultLabelResolver, RuntimeConfig};
+use crate::{RuntimeConfig, StoreManager};
 use anyhow::Context as _;
 use serde::de::DeserializeOwned;
 use serde::{Deserialize, Serialize};
@@ -99,6 +98,8 @@ impl RuntimeConfigResolver {
     }
 
     /// Resolves a toml table into a runtime config.
+    ///
+    /// The default stores are also added to the runtime config.
     pub fn resolve_from_toml(
         &self,
         table: Option<&impl GetTomlValue>,
@@ -115,6 +116,17 @@ impl RuntimeConfigResolver {
             })?;
             runtime_config.add_store_manager(label.clone(), store_manager);
         }
+
+        for (&label, config) in &self.defaults {
+            if !runtime_config.store_managers.contains_key(label) {
+                let store_manager = self
+                    .store_manager_from_config(config.clone())
+                    .with_context(|| {
+                        format!("could not configure key-value store with label '{label}'")
+                    })?;
+                runtime_config.add_store_manager(label.to_owned(), store_manager);
+            }
+        }
         Ok(Some(runtime_config))
     }
 
@@ -134,15 +146,6 @@ 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())
-    }
-}
-
 #[derive(Deserialize, Clone)]
 pub struct StoreConfig {
     #[serde(rename = "type")]

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

@@ -28,24 +28,14 @@ impl StoreManager for EmptyStoreManager {
     }
 }
 
-/// A function that takes a store label and returns the default store manager, if one exists.
-pub type DefaultManagerGetter = Arc<dyn Fn(&str) -> Option<Arc<dyn StoreManager>> + Send + Sync>;
-
 pub struct DelegatingStoreManager {
     delegates: HashMap<String, Arc<dyn StoreManager>>,
-    default_manager: DefaultManagerGetter,
 }
 
 impl DelegatingStoreManager {
-    pub fn new(
-        delegates: impl IntoIterator<Item = (String, Arc<dyn StoreManager>)>,
-        default_manager: DefaultManagerGetter,
-    ) -> Self {
+    pub fn new(delegates: impl IntoIterator<Item = (String, Arc<dyn StoreManager>)>) -> Self {
         let delegates = delegates.into_iter().collect();
-        Self {
-            delegates,
-            default_manager,
-        }
+        Self { delegates }
     }
 }
 
@@ -54,12 +44,7 @@ impl StoreManager for DelegatingStoreManager {
     async fn get(&self, name: &str) -> Result<Arc<dyn Store>, Error> {
         match self.delegates.get(name) {
             Some(store) => store.get(name).await,
-            None => {
-                (self.default_manager)(name)
-                    .ok_or(Error::NoSuchStore)?
-                    .get(name)
-                    .await
-            }
+            None => Err(Error::NoSuchStore),
         }
     }
 
@@ -71,7 +56,7 @@ impl StoreManager for DelegatingStoreManager {
         if let Some(store) = self.delegates.get(store_name) {
             return store.summary(store_name);
         }
-        (self.default_manager)(store_name)?.summary(store_name)
+        None
     }
 }