diff --git a/crates/factor-key-value/src/delegating_resolver.rs b/crates/factor-key-value/src/delegating_resolver.rs index 235a2afcb..73ce2605e 100644 --- a/crates/factor-key-value/src/delegating_resolver.rs +++ b/crates/factor-key-value/src/delegating_resolver.rs @@ -1,5 +1,6 @@ use crate::runtime_config::RuntimeConfigResolver; use crate::store::{store_from_toml_fn, MakeKeyValueStore, StoreFromToml}; +use serde::Deserialize; use spin_key_value::StoreManager; use std::{collections::HashMap, sync::Arc}; @@ -9,20 +10,13 @@ pub struct DelegatingRuntimeConfigResolver { defaults: HashMap<&'static str, StoreConfig>, } -type StoreConfig = (&'static str, toml::value::Table); - impl DelegatingRuntimeConfigResolver { pub fn new() -> Self { Self::default() } - pub fn add_default_store( - &mut self, - label: &'static str, - store_kind: &'static str, - config: toml::value::Table, - ) { - self.defaults.insert(label, (store_kind, config)); + pub fn add_default_store(&mut self, label: &'static str, config: StoreConfig) { + self.defaults.insert(label, config); } } @@ -43,20 +37,27 @@ impl DelegatingRuntimeConfigResolver { } impl RuntimeConfigResolver for DelegatingRuntimeConfigResolver { - fn get_store( - &self, - store_kind: &str, - config: toml::Table, - ) -> anyhow::Result> { + type Config = StoreConfig; + + fn get_store(&self, config: StoreConfig) -> anyhow::Result> { + 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) + store_from_toml(config.config) } fn default_store(&self, label: &str) -> Option> { - let (store_kind, config) = self.defaults.get(label)?; - self.get_store(store_kind, config.to_owned()).ok() + let config = self.defaults.get(label)?; + self.get_store(config.clone()).ok() } } + +#[derive(Deserialize, Clone)] +pub struct StoreConfig { + #[serde(rename = "type")] + pub type_: String, + #[serde(flatten)] + pub config: toml::Table, +} diff --git a/crates/factor-key-value/src/lib.rs b/crates/factor-key-value/src/lib.rs index bc2016467..2a7cbeea3 100644 --- a/crates/factor-key-value/src/lib.rs +++ b/crates/factor-key-value/src/lib.rs @@ -8,7 +8,7 @@ use std::{ }; use anyhow::ensure; -use runtime_config::RuntimeConfig; +use runtime_config::{RuntimeConfig, RuntimeConfigResolver}; use spin_factors::{ ConfigureAppContext, Factor, FactorInstanceBuilder, InitContext, InstanceBuilders, PrepareContext, RuntimeFactors, @@ -19,22 +19,20 @@ use spin_key_value::{ }; pub use store::MakeKeyValueStore; -pub struct KeyValueFactor { - runtime_config_resolver: Arc, +pub struct KeyValueFactor { + runtime_config_resolver: Arc, } -impl KeyValueFactor { - pub fn new( - runtime_config_resolver: impl runtime_config::RuntimeConfigResolver + 'static, - ) -> Self { +impl KeyValueFactor { + pub fn new(runtime_config_resolver: R) -> Self { Self { runtime_config_resolver: Arc::new(runtime_config_resolver), } } } -impl Factor for KeyValueFactor { - type RuntimeConfig = RuntimeConfig; +impl Factor for KeyValueFactor { + type RuntimeConfig = RuntimeConfig; type AppState = AppState; type InstanceBuilder = InstanceBuilder; @@ -51,17 +49,8 @@ impl Factor for KeyValueFactor { // Build StoreManager from runtime config let mut store_managers: HashMap> = HashMap::new(); if let Some(runtime_config) = ctx.take_runtime_config() { - for ( - store_label, - runtime_config::StoreConfig { - type_: store_kind, - config, - }, - ) in runtime_config.store_configs - { - let store = self - .runtime_config_resolver - .get_store(&store_kind, config)?; + for (store_label, config) in runtime_config.store_configs { + let store = self.runtime_config_resolver.get_store(config)?; store_managers.insert(store_label, store); } } diff --git a/crates/factor-key-value/src/runtime_config.rs b/crates/factor-key-value/src/runtime_config.rs index 721cf5be4..b7d719570 100644 --- a/crates/factor-key-value/src/runtime_config.rs +++ b/crates/factor-key-value/src/runtime_config.rs @@ -1,38 +1,26 @@ use std::{collections::HashMap, sync::Arc}; -use serde::Deserialize; +use serde::{de::DeserializeOwned, Deserialize}; use spin_factors::{anyhow, FactorRuntimeConfig}; use spin_key_value::StoreManager; #[derive(Deserialize)] #[serde(transparent)] -pub struct RuntimeConfig { - pub store_configs: HashMap, +pub struct RuntimeConfig { + pub store_configs: HashMap, } -impl FactorRuntimeConfig for RuntimeConfig { +impl FactorRuntimeConfig for RuntimeConfig { const KEY: &'static str = "key_value_store"; } -#[derive(Deserialize)] -pub struct StoreConfig { - #[serde(rename = "type")] - pub type_: String, - #[serde(flatten)] - pub config: toml::Table, -} - /// Resolves some piece of runtime configuration to a connection pool pub trait RuntimeConfigResolver: Send + Sync { - /// Get a store manager for a given store kind and the raw configuration. - /// - /// `store_kind` is equivalent to the `type` field in the - /// `[key_value_store.$storename]` runtime configuration table. - fn get_store( - &self, - store_kind: &str, - config: toml::Table, - ) -> anyhow::Result>; + /// The type of configuration that this resolver can handle. + type Config: DeserializeOwned; + + /// Get a store manager for a given config. + fn get_store(&self, config: Self::Config) -> anyhow::Result>; /// Returns a default store manager for a given label. Should only be called /// if there is no runtime configuration for the label. diff --git a/crates/factor-sqlite/src/lib.rs b/crates/factor-sqlite/src/lib.rs index bdc5d3abb..1c270b9a4 100644 --- a/crates/factor-sqlite/src/lib.rs +++ b/crates/factor-sqlite/src/lib.rs @@ -7,28 +7,29 @@ use std::sync::Arc; use host::InstanceState; use async_trait::async_trait; +use runtime_config::RuntimeConfigResolver; use spin_factors::{anyhow, Factor}; use spin_locked_app::MetadataKey; use spin_world::v1::sqlite as v1; use spin_world::v2::sqlite as v2; -pub struct SqliteFactor { - runtime_config_resolver: Arc, +pub struct SqliteFactor { + runtime_config_resolver: Arc, } -impl SqliteFactor { +impl SqliteFactor { /// Create a new `SqliteFactor` - pub fn new( - runtime_config_resolver: impl runtime_config::RuntimeConfigResolver + 'static, - ) -> Self { + /// + /// Takes a `runtime_config_resolver` that can resolve a runtime configuration into a connection pool. + pub fn new(runtime_config_resolver: R) -> Self { Self { runtime_config_resolver: Arc::new(runtime_config_resolver), } } } -impl Factor for SqliteFactor { - type RuntimeConfig = runtime_config::RuntimeConfig; +impl Factor for SqliteFactor { + type RuntimeConfig = runtime_config::RuntimeConfig; type AppState = AppState; type InstanceBuilder = InstanceState; @@ -47,17 +48,8 @@ impl Factor for SqliteFactor { ) -> anyhow::Result { let mut connection_pools = HashMap::new(); if let Some(runtime_config) = ctx.take_runtime_config() { - for ( - database_label, - runtime_config::StoreConfig { - type_: database_kind, - config, - }, - ) in runtime_config.store_configs - { - let pool = self - .runtime_config_resolver - .get_pool(&database_kind, config)?; + for (database_label, config) in runtime_config.store_configs { + let pool = self.runtime_config_resolver.get_pool(config)?; connection_pools.insert(database_label, pool); } } diff --git a/crates/factor-sqlite/src/runtime_config.rs b/crates/factor-sqlite/src/runtime_config.rs index 2089d079a..10e6f8e72 100644 --- a/crates/factor-sqlite/src/runtime_config.rs +++ b/crates/factor-sqlite/src/runtime_config.rs @@ -3,40 +3,28 @@ pub mod spin; use std::{collections::HashMap, sync::Arc}; -use serde::Deserialize; +use serde::{de::DeserializeOwned, Deserialize}; use spin_factors::{anyhow, FactorRuntimeConfig}; use crate::ConnectionPool; #[derive(Deserialize)] #[serde(transparent)] -pub struct RuntimeConfig { - pub store_configs: HashMap, +pub struct RuntimeConfig { + pub store_configs: HashMap, } -impl FactorRuntimeConfig for RuntimeConfig { +impl FactorRuntimeConfig for RuntimeConfig { const KEY: &'static str = "sqlite_database"; } -#[derive(Deserialize)] -pub struct StoreConfig { - #[serde(rename = "type")] - pub type_: String, - #[serde(flatten)] - pub config: toml::Table, -} - /// Resolves some piece of runtime configuration to a connection pool pub trait RuntimeConfigResolver: Send + Sync { - /// Get a connection pool for a given database kind and the raw configuration. + type Config: DeserializeOwned; + + /// Get a connection pool for a given config. /// - /// `database_kind` is equivalent to the `type` field in the - /// `[sqlite_database.$databasename]` runtime configuration table. - fn get_pool( - &self, - database_kind: &str, - config: toml::Table, - ) -> anyhow::Result>; + fn get_pool(&self, config: Self::Config) -> anyhow::Result>; /// If there is no runtime configuration for a given database label, return a default connection pool. /// diff --git a/crates/factor-sqlite/src/runtime_config/spin.rs b/crates/factor-sqlite/src/runtime_config/spin.rs index fa8b618a5..003120661 100644 --- a/crates/factor-sqlite/src/runtime_config/spin.rs +++ b/crates/factor-sqlite/src/runtime_config/spin.rs @@ -50,22 +50,29 @@ impl SpinSqliteRuntimeConfig { } } +#[derive(Deserialize)] +pub struct RuntimeConfig { + #[serde(rename = "type")] + pub type_: String, + #[serde(flatten)] + pub config: toml::Table, +} + impl RuntimeConfigResolver for SpinSqliteRuntimeConfig { - fn get_pool( - &self, - database_kind: &str, - config: toml::Table, - ) -> anyhow::Result> { + type Config = RuntimeConfig; + + fn get_pool(&self, config: RuntimeConfig) -> anyhow::Result> { + let database_kind = config.type_.as_str(); let pool = match database_kind { "spin" => { - let config: LocalDatabase = config.try_into()?; + let config: LocalDatabase = config.config.try_into()?; config.pool(&self.local_database_dir)? } "libsql" => { - let config: LibSqlDatabase = config.try_into()?; + let config: LibSqlDatabase = config.config.try_into()?; config.pool()? } - _ => anyhow::bail!("Unknown database kind: {}", database_kind), + _ => anyhow::bail!("Unknown database kind: {database_kind}"), }; Ok(Arc::new(pool)) } diff --git a/crates/factor-sqlite/tests/factor.rs b/crates/factor-sqlite/tests/factor.rs index eb40212d9..6897e015b 100644 --- a/crates/factor-sqlite/tests/factor.rs +++ b/crates/factor-sqlite/tests/factor.rs @@ -1,6 +1,6 @@ use std::{collections::HashSet, sync::Arc}; -use factor_sqlite::SqliteFactor; +use factor_sqlite::{runtime_config::spin::RuntimeConfig, SqliteFactor}; use spin_factors::{ anyhow::{self, bail}, RuntimeFactors, @@ -9,7 +9,7 @@ use spin_factors_test::{toml, TestEnvironment}; #[derive(RuntimeFactors)] struct TestFactors { - sqlite: SqliteFactor, + sqlite: SqliteFactor, } #[tokio::test] @@ -70,7 +70,9 @@ async fn no_error_when_database_is_configured() -> anyhow::Result<()> { [sqlite_database.foo] type = "sqlite" }; - assert!(env.build_instance_state(factors).await.is_ok()); + if let Err(e) = env.build_instance_state(factors).await { + bail!("Expected build_instance_state to succeed but it errored: {e}"); + } Ok(()) } @@ -89,12 +91,13 @@ impl RuntimeConfigResolver { } impl factor_sqlite::runtime_config::RuntimeConfigResolver for RuntimeConfigResolver { + type Config = RuntimeConfig; + fn get_pool( &self, - database_kind: &str, - config: toml::Table, + config: RuntimeConfig, ) -> anyhow::Result> { - let _ = (database_kind, config); + let _ = config; Ok(Arc::new(InvalidConnectionPool)) } diff --git a/crates/factors/tests/smoke.rs b/crates/factors/tests/smoke.rs index ee1bcb6b4..45a727c85 100644 --- a/crates/factors/tests/smoke.rs +++ b/crates/factors/tests/smoke.rs @@ -4,7 +4,8 @@ use anyhow::Context; use http_body_util::BodyExt; use spin_app::App; use spin_factor_key_value::{ - delegating_resolver::DelegatingRuntimeConfigResolver, KeyValueFactor, MakeKeyValueStore, + delegating_resolver::{DelegatingRuntimeConfigResolver, StoreConfig}, + KeyValueFactor, MakeKeyValueStore, }; use spin_factor_key_value_redis::RedisKeyValueStore; use spin_factor_key_value_spin::{SpinKeyValueRuntimeConfig, SpinKeyValueStore}; @@ -21,7 +22,7 @@ struct Factors { variables: VariablesFactor, outbound_networking: OutboundNetworkingFactor, outbound_http: OutboundHttpFactor, - key_value: KeyValueFactor, + key_value: KeyValueFactor, } struct Data { @@ -42,8 +43,10 @@ async fn smoke_test_works() -> anyhow::Result<()> { SpinKeyValueRuntimeConfig::default(Some(PathBuf::from("tests/smoke-app/.spin"))); key_value_resolver.add_default_store( "default", - SpinKeyValueStore::RUNTIME_CONFIG_TYPE, - toml::value::Table::try_from(default_config)?, + StoreConfig { + type_: SpinKeyValueStore::RUNTIME_CONFIG_TYPE.to_string(), + config: toml::value::Table::try_from(default_config)?, + }, ); key_value_resolver.add_store_type(SpinKeyValueStore::new(None)?)?; key_value_resolver.add_store_type(RedisKeyValueStore)?;