Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove toml assumption #2665

Merged
merged 4 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions crates/factor-key-value/src/delegating_resolver.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -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);
}
}

Expand All @@ -43,20 +37,27 @@ impl DelegatingRuntimeConfigResolver {
}

impl RuntimeConfigResolver for DelegatingRuntimeConfigResolver {
fn get_store(
&self,
store_kind: &str,
config: toml::Table,
) -> anyhow::Result<Arc<dyn StoreManager>> {
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)
store_from_toml(config.config)
}

fn default_store(&self, label: &str) -> Option<Arc<dyn StoreManager>> {
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,
}
29 changes: 9 additions & 20 deletions crates/factor-key-value/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -19,22 +19,20 @@ use spin_key_value::{
};
pub use store::MakeKeyValueStore;

pub struct KeyValueFactor {
runtime_config_resolver: Arc<dyn runtime_config::RuntimeConfigResolver>,
pub struct KeyValueFactor<R> {
runtime_config_resolver: Arc<R>,
}

impl KeyValueFactor {
pub fn new(
runtime_config_resolver: impl runtime_config::RuntimeConfigResolver + 'static,
) -> Self {
impl<R> KeyValueFactor<R> {
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<R: RuntimeConfigResolver + 'static> Factor for KeyValueFactor<R> {
type RuntimeConfig = RuntimeConfig<R::Config>;
type AppState = AppState;
type InstanceBuilder = InstanceBuilder;

Expand All @@ -51,17 +49,8 @@ impl Factor for KeyValueFactor {
// Build StoreManager 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,
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);
}
}
Expand Down
30 changes: 9 additions & 21 deletions crates/factor-key-value/src/runtime_config.rs
Original file line number Diff line number Diff line change
@@ -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<String, StoreConfig>,
pub struct RuntimeConfig<C> {
pub store_configs: HashMap<String, C>,
}

impl FactorRuntimeConfig for RuntimeConfig {
impl<C: DeserializeOwned> FactorRuntimeConfig for RuntimeConfig<C> {
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<Arc<dyn StoreManager>>;
/// 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<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.
Expand Down
30 changes: 11 additions & 19 deletions crates/factor-sqlite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn runtime_config::RuntimeConfigResolver>,
pub struct SqliteFactor<R> {
runtime_config_resolver: Arc<R>,
}

impl SqliteFactor {
impl<R> SqliteFactor<R> {
/// 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<R: RuntimeConfigResolver + 'static> Factor for SqliteFactor<R> {
type RuntimeConfig = runtime_config::RuntimeConfig<R::Config>;
type AppState = AppState;
type InstanceBuilder = InstanceState;

Expand All @@ -47,17 +48,8 @@ impl Factor for SqliteFactor {
) -> anyhow::Result<Self::AppState> {
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);
}
}
Expand Down
28 changes: 8 additions & 20 deletions crates/factor-sqlite/src/runtime_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, StoreConfig>,
pub struct RuntimeConfig<C> {
pub store_configs: HashMap<String, C>,
}

impl FactorRuntimeConfig for RuntimeConfig {
impl<C: DeserializeOwned> FactorRuntimeConfig for RuntimeConfig<C> {
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<Arc<dyn ConnectionPool>>;
fn get_pool(&self, config: Self::Config) -> anyhow::Result<Arc<dyn ConnectionPool>>;

/// If there is no runtime configuration for a given database label, return a default connection pool.
///
Expand Down
23 changes: 15 additions & 8 deletions crates/factor-sqlite/src/runtime_config/spin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Arc<dyn ConnectionPool>> {
type Config = RuntimeConfig;

fn get_pool(&self, config: RuntimeConfig) -> anyhow::Result<Arc<dyn ConnectionPool>> {
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))
}
Expand Down
15 changes: 9 additions & 6 deletions crates/factor-sqlite/tests/factor.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -9,7 +9,7 @@ use spin_factors_test::{toml, TestEnvironment};

#[derive(RuntimeFactors)]
struct TestFactors {
sqlite: SqliteFactor,
sqlite: SqliteFactor<RuntimeConfigResolver>,
}

#[tokio::test]
Expand Down Expand Up @@ -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(())
}
Expand All @@ -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<Arc<dyn factor_sqlite::ConnectionPool>> {
let _ = (database_kind, config);
let _ = config;
Ok(Arc::new(InvalidConnectionPool))
}

Expand Down
11 changes: 7 additions & 4 deletions crates/factors/tests/smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -21,7 +22,7 @@ struct Factors {
variables: VariablesFactor,
outbound_networking: OutboundNetworkingFactor,
outbound_http: OutboundHttpFactor,
key_value: KeyValueFactor,
key_value: KeyValueFactor<DelegatingRuntimeConfigResolver>,
}

struct Data {
Expand All @@ -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)?,
},
Comment on lines +46 to +49
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would probably benefit from a StoreConfig::new 🤷

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll do this in a follow up.

);
key_value_resolver.add_store_type(SpinKeyValueStore::new(None)?)?;
key_value_resolver.add_store_type(RedisKeyValueStore)?;
Expand Down
Loading