Skip to content

Commit e236a7f

Browse files
committed
Simplify default SQLite database handling.
Instead of treating the SQLite Factor's runtime config as 1 on 1 the same as the Spin CLI runtime config toml, we inject handling of the "default" label into the factor's runtime config struct. This makes handling of label resolution happen in just one place instead of spliting it into two. Signed-off-by: Ryan Levick <[email protected]>
1 parent 3ef8673 commit e236a7f

File tree

6 files changed

+26
-64
lines changed

6 files changed

+26
-64
lines changed

crates/factor-sqlite/src/lib.rs

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,13 @@ use spin_world::v2::sqlite as v2;
1515
pub use runtime_config::RuntimeConfig;
1616

1717
pub struct SqliteFactor {
18-
default_label_resolver: Arc<dyn DefaultLabelResolver>,
18+
_priv: (),
1919
}
2020

2121
impl SqliteFactor {
2222
/// Create a new `SqliteFactor`
23-
///
24-
/// Takes a `default_label_resolver` for how to handle when a database label doesn't
25-
/// have a corresponding runtime configuration.
26-
pub fn new(default_label_resolver: impl DefaultLabelResolver + 'static) -> Self {
27-
Self {
28-
default_label_resolver: Arc::new(default_label_resolver),
29-
}
23+
pub fn new() -> Self {
24+
Self { _priv: () }
3025
}
3126
}
3227

@@ -69,13 +64,8 @@ impl Factor for SqliteFactor {
6964
))
7065
})
7166
.collect::<anyhow::Result<HashMap<_, _>>>()?;
72-
let resolver = self.default_label_resolver.clone();
73-
let get_connection_creator: host::ConnectionCreatorGetter = Arc::new(move |label| {
74-
connection_creators
75-
.get(label)
76-
.cloned()
77-
.or_else(|| resolver.default(label))
78-
});
67+
let get_connection_creator: host::ConnectionCreatorGetter =
68+
Arc::new(move |label| connection_creators.get(label).cloned());
7969

8070
ensure_allowed_databases_are_configured(&allowed_databases, |label| {
8171
get_connection_creator(label).is_some()
@@ -138,14 +128,6 @@ fn ensure_allowed_databases_are_configured(
138128
/// Metadata key for a list of allowed databases for a component.
139129
pub const ALLOWED_DATABASES_KEY: MetadataKey<Vec<String>> = MetadataKey::new("databases");
140130

141-
/// Resolves a label to a default connection creator.
142-
pub trait DefaultLabelResolver: Send + Sync {
143-
/// If there is no runtime configuration for a given database label, return a default connection creator.
144-
///
145-
/// If `Option::None` is returned, the database is not allowed.
146-
fn default(&self, label: &str) -> Option<Arc<dyn ConnectionCreator>>;
147-
}
148-
149131
#[derive(Clone)]
150132
pub struct AppState {
151133
/// A map from component id to a set of allowed database labels.

crates/factor-sqlite/tests/factor_test.rs

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,8 @@ struct TestFactors {
1717

1818
#[tokio::test]
1919
async fn sqlite_works() -> anyhow::Result<()> {
20-
let test_resolver = DefaultLabelResolver::new(Some("default"));
2120
let factors = TestFactors {
22-
sqlite: SqliteFactor::new(test_resolver),
21+
sqlite: SqliteFactor::new(),
2322
};
2423
let env = TestEnvironment::new(factors).extend_manifest(toml! {
2524
[component.test-component]
@@ -38,9 +37,8 @@ async fn sqlite_works() -> anyhow::Result<()> {
3837

3938
#[tokio::test]
4039
async fn errors_when_non_configured_database_used() -> anyhow::Result<()> {
41-
let test_resolver = DefaultLabelResolver::new(None);
4240
let factors = TestFactors {
43-
sqlite: SqliteFactor::new(test_resolver),
41+
sqlite: SqliteFactor::new(),
4442
};
4543
let env = TestEnvironment::new(factors).extend_manifest(toml! {
4644
[component.test-component]
@@ -60,9 +58,8 @@ async fn errors_when_non_configured_database_used() -> anyhow::Result<()> {
6058

6159
#[tokio::test]
6260
async fn no_error_when_database_is_configured() -> anyhow::Result<()> {
63-
let test_resolver = DefaultLabelResolver::new(None);
6461
let factors = TestFactors {
65-
sqlite: SqliteFactor::new(test_resolver),
62+
sqlite: SqliteFactor::new(),
6663
};
6764
let runtime_config = toml! {
6865
[sqlite_database.foo]
@@ -119,28 +116,6 @@ impl TryFrom<TomlRuntimeSource<'_>> for TestFactorsRuntimeConfig {
119116
}
120117
}
121118

122-
/// Will return an `InvalidConnectionCreator` for the supplied default database.
123-
struct DefaultLabelResolver {
124-
default: Option<String>,
125-
}
126-
127-
impl DefaultLabelResolver {
128-
fn new(default: Option<&str>) -> Self {
129-
Self {
130-
default: default.map(Into::into),
131-
}
132-
}
133-
}
134-
135-
impl spin_factor_sqlite::DefaultLabelResolver for DefaultLabelResolver {
136-
fn default(&self, label: &str) -> Option<Arc<dyn spin_factor_sqlite::ConnectionCreator>> {
137-
let Some(default) = &self.default else {
138-
return None;
139-
};
140-
(default == label).then_some(Arc::new(InvalidConnectionCreator))
141-
}
142-
}
143-
144119
/// A connection creator that always returns an error.
145120
struct InvalidConnectionCreator;
146121

crates/runtime-config/src/lib.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,18 @@ impl FactorRuntimeConfigSource<OutboundMqttFactor> for TomlRuntimeConfigSource<'
368368

369369
impl FactorRuntimeConfigSource<SqliteFactor> for TomlRuntimeConfigSource<'_, '_> {
370370
fn get_runtime_config(&mut self) -> anyhow::Result<Option<spin_factor_sqlite::RuntimeConfig>> {
371-
self.sqlite.resolve_from_toml(&self.toml.table)
371+
Ok(self
372+
.sqlite
373+
.resolve_from_toml(&self.toml.table)?
374+
.map(|mut config| {
375+
// If the user did not provide configuration for the default label, add it.
376+
if !config.connection_creators.contains_key("default") {
377+
config
378+
.connection_creators
379+
.insert("default".to_owned(), self.sqlite.default());
380+
}
381+
config
382+
}))
372383
}
373384
}
374385

crates/runtime-factors/src/build.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ impl RuntimeFactorsBuilder for FactorsBuilder {
3939
config.working_dir.clone(),
4040
args.allow_transient_write,
4141
runtime_config.key_value_resolver.clone(),
42-
runtime_config.sqlite_resolver.clone(),
4342
use_gpu,
4443
)
4544
.context("failed to create factors")?;

crates/runtime-factors/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ impl TriggerFactors {
4141
working_dir: impl Into<PathBuf>,
4242
allow_transient_writes: bool,
4343
default_key_value_label_resolver: impl spin_factor_key_value::DefaultLabelResolver + 'static,
44-
default_sqlite_label_resolver: impl spin_factor_sqlite::DefaultLabelResolver + 'static,
4544
use_gpu: bool,
4645
) -> anyhow::Result<Self> {
4746
Ok(Self {
@@ -50,7 +49,7 @@ impl TriggerFactors {
5049
key_value: KeyValueFactor::new(default_key_value_label_resolver),
5150
outbound_networking: outbound_networking_factor(),
5251
outbound_http: OutboundHttpFactor::default(),
53-
sqlite: SqliteFactor::new(default_sqlite_label_resolver),
52+
sqlite: SqliteFactor::new(),
5453
redis: OutboundRedisFactor::new(),
5554
mqtt: OutboundMqttFactor::new(NetworkedMqttClient::creator()),
5655
pg: OutboundPgFactor::new(),

crates/sqlite/src/lib.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::{
66
};
77

88
use serde::Deserialize;
9-
use spin_factor_sqlite::{ConnectionCreator, DefaultLabelResolver};
9+
use spin_factor_sqlite::ConnectionCreator;
1010
use spin_factors::{
1111
anyhow::{self, Context as _},
1212
runtime_config::toml::GetTomlValue,
@@ -97,13 +97,9 @@ pub struct TomlRuntimeConfig {
9797
pub config: toml::Table,
9898
}
9999

100-
impl DefaultLabelResolver for RuntimeConfigResolver {
101-
fn default(&self, label: &str) -> Option<Arc<dyn ConnectionCreator>> {
102-
// Only default the database labeled "default".
103-
if label != "default" {
104-
return None;
105-
}
106-
100+
impl RuntimeConfigResolver {
101+
/// The [`ConnectionCreator`] for the 'default' label.
102+
pub fn default(&self) -> Arc<dyn ConnectionCreator> {
107103
let path = self
108104
.default_database_dir
109105
.as_deref()
@@ -113,7 +109,7 @@ impl DefaultLabelResolver for RuntimeConfigResolver {
113109
let connection = spin_sqlite_inproc::InProcConnection::new(location)?;
114110
Ok(Box::new(connection) as _)
115111
};
116-
Some(Arc::new(factory))
112+
Arc::new(factory)
117113
}
118114
}
119115

0 commit comments

Comments
 (0)