Skip to content

Commit

Permalink
Merge pull request #2807 from fermyon/small-changes
Browse files Browse the repository at this point in the history
A collection of small changes
  • Loading branch information
rylev authored Sep 6, 2024
2 parents ee08554 + 76e7c2d commit ba32722
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 37 deletions.
2 changes: 1 addition & 1 deletion crates/app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub const APP_DESCRIPTION_KEY: MetadataKey = MetadataKey::new("description");
pub const OCI_IMAGE_DIGEST_KEY: MetadataKey = MetadataKey::new("oci_image_digest");

/// An `App` holds loaded configuration for a Spin application.
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct App {
id: String,
locked: LockedApp,
Expand Down
4 changes: 2 additions & 2 deletions crates/factor-key-value/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ use host::KEY_VALUE_STORES_KEY;
use spin_factors::{
ConfigureAppContext, Factor, FactorInstanceBuilder, InitContext, PrepareContext, RuntimeFactors,
};
use util::{CachingStoreManager, DefaultManagerGetter};
use util::DefaultManagerGetter;

pub use host::{log_error, Error, KeyValueDispatch, Store, StoreManager};
pub use runtime_config::RuntimeConfig;
pub use util::DelegatingStoreManager;
pub use util::{CachingStoreManager, DelegatingStoreManager};

/// A factor that provides key-value storage.
pub struct KeyValueFactor {
Expand Down
3 changes: 2 additions & 1 deletion crates/factor-sqlite/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ impl v2::HostConnection for InstanceState {
}
let conn = (self.get_connection_creator)(&database)
.ok_or(v2::Error::NoSuchDatabase)?
.create_connection()?;
.create_connection(&database)
.await?;
self.connections
.push(conn)
.map_err(|()| v2::Error::Io("too many connections opened".to_string()))
Expand Down
22 changes: 18 additions & 4 deletions crates/factor-sqlite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,13 @@ impl AppState {
/// Get a connection for a given database label.
///
/// Returns `None` if there is no connection creator for the given label.
pub fn get_connection(&self, label: &str) -> Option<Result<Box<dyn Connection>, v2::Error>> {
let connection = (self.get_connection_creator)(label)?.create_connection();
pub async fn get_connection(
&self,
label: &str,
) -> Option<Result<Box<dyn Connection>, v2::Error>> {
let connection = (self.get_connection_creator)(label)?
.create_connection(label)
.await;
Some(connection)
}

Expand All @@ -182,18 +187,27 @@ impl AppState {
}

/// A creator of a connections for a particular SQLite database.
#[async_trait]
pub trait ConnectionCreator: Send + Sync {
/// Get a *new* [`Connection`]
///
/// The connection should be a new connection, not a reused one.
fn create_connection(&self) -> Result<Box<dyn Connection + 'static>, v2::Error>;
async fn create_connection(
&self,
label: &str,
) -> Result<Box<dyn Connection + 'static>, v2::Error>;
}

#[async_trait]
impl<F> ConnectionCreator for F
where
F: Fn() -> anyhow::Result<Box<dyn Connection + 'static>> + Send + Sync + 'static,
{
fn create_connection(&self) -> Result<Box<dyn Connection + 'static>, v2::Error> {
async fn create_connection(
&self,
label: &str,
) -> Result<Box<dyn Connection + 'static>, v2::Error> {
let _ = label;
(self)().map_err(|_| v2::Error::InvalidConnection)
}
}
Expand Down
6 changes: 5 additions & 1 deletion crates/factor-sqlite/tests/factor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use spin_factors::{
};
use spin_factors_test::{toml, TestEnvironment};
use spin_sqlite::RuntimeConfigResolver;
use spin_world::async_trait;

#[derive(RuntimeFactors)]
struct TestFactors {
Expand Down Expand Up @@ -143,11 +144,14 @@ impl spin_factor_sqlite::DefaultLabelResolver for DefaultLabelResolver {
/// A connection creator that always returns an error.
struct InvalidConnectionCreator;

#[async_trait]
impl spin_factor_sqlite::ConnectionCreator for InvalidConnectionCreator {
fn create_connection(
async fn create_connection(
&self,
label: &str,
) -> Result<Box<dyn spin_factor_sqlite::Connection + 'static>, spin_world::v2::sqlite::Error>
{
let _ = label;
Err(spin_world::v2::sqlite::Error::InvalidConnection)
}
}
10 changes: 9 additions & 1 deletion crates/factor-variables/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ pub struct VariablesFactor {
_priv: (),
}

impl VariablesFactor {
/// Creates a new `VariablesFactor`.
pub fn new() -> Self {
Default::default()
}
}

impl Factor for VariablesFactor {
type RuntimeConfig = RuntimeConfig;
type AppState = AppState;
Expand All @@ -43,7 +50,8 @@ impl Factor for VariablesFactor {
)?;
}

for provider in ctx.take_runtime_config().unwrap_or_default() {
let providers = ctx.take_runtime_config().unwrap_or_default();
for provider in providers {
expression_resolver.add_provider(provider);
}

Expand Down
51 changes: 34 additions & 17 deletions crates/factors-executor/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::{collections::HashMap, sync::Arc};

use anyhow::Context;
use spin_app::{App, AppComponent};
Expand Down Expand Up @@ -35,10 +35,12 @@ impl<T: RuntimeFactors, U: Send + 'static> FactorsExecutor<T, U> {
hooks: Default::default(),
})
}
}

impl<T: RuntimeFactors, U: Send + 'static> FactorsExecutor<T, U> {
/// Adds the given [`ExecutorHooks`] to this executor.
pub fn core_engine(&self) -> &spin_core::Engine<InstanceState<T::InstanceState, U>> {
&self.core_engine
}

// Adds the given [`ExecutorHooks`] to this executor.
///
/// Hooks are run in the order they are added.
pub fn add_hooks(&mut self, hooks: impl ExecutorHooks<T, U> + 'static) {
Expand All @@ -47,17 +49,17 @@ impl<T: RuntimeFactors, U: Send + 'static> FactorsExecutor<T, U> {

/// Loads a [`FactorsApp`] with this executor.
pub async fn load_app(
mut self,
self: Arc<Self>,
app: App,
runtime_config: T::RuntimeConfig,
mut component_loader: impl ComponentLoader,
component_loader: &impl ComponentLoader,
) -> anyhow::Result<FactorsExecutorApp<T, U>> {
let configured_app = self
.factors
.configure_app(app, runtime_config)
.context("failed to configure app")?;

for hooks in &mut self.hooks {
for hooks in &self.hooks {
hooks.configure_app(&configured_app).await?;
}

Expand All @@ -73,7 +75,7 @@ impl<T: RuntimeFactors, U: Send + 'static> FactorsExecutor<T, U> {
}

Ok(FactorsExecutorApp {
executor: self,
executor: self.clone(),
configured_app,
component_instance_pres,
})
Expand All @@ -86,7 +88,7 @@ where
T: RuntimeFactors,
{
/// Configure app hooks run immediately after [`RuntimeFactors::configure_app`].
async fn configure_app(&mut self, configured_app: &ConfiguredApp<T>) -> anyhow::Result<()> {
async fn configure_app(&self, configured_app: &ConfiguredApp<T>) -> anyhow::Result<()> {
let _ = configured_app;
Ok(())
}
Expand All @@ -103,7 +105,7 @@ where
pub trait ComponentLoader {
/// Loads a [`Component`] for the given [`AppComponent`].
async fn load_component(
&mut self,
&self,
engine: &spin_core::wasmtime::Engine,
component: &AppComponent,
) -> anyhow::Result<Component>;
Expand All @@ -117,7 +119,7 @@ type InstancePre<T, U> =
/// It is generic over the executor's [`RuntimeFactors`] and any ad-hoc additional
/// per-instance state needed by the caller.
pub struct FactorsExecutorApp<T: RuntimeFactors, U> {
executor: FactorsExecutor<T, U>,
executor: Arc<FactorsExecutor<T, U>>,
configured_app: ConfiguredApp<T>,
// Maps component IDs -> InstancePres
component_instance_pres: HashMap<String, InstancePre<T, U>>,
Expand Down Expand Up @@ -249,13 +251,28 @@ impl<T, U> InstanceState<T, U> {
&self.core
}

/// Provides mutable access to the [`spin_core::State`].
pub fn core_state_mut(&mut self) -> &mut spin_core::State {
&mut self.core
}

/// Provides access to the [`RuntimeFactors::InstanceState`].
pub fn factors_instance_state(&mut self) -> &mut T {
pub fn factors_instance_state(&self) -> &T {
&self.factors
}

/// Provides mutable access to the [`RuntimeFactors::InstanceState`].
pub fn factors_instance_state_mut(&mut self) -> &mut T {
&mut self.factors
}

/// Provides access to the `Self::ExecutorInstanceState`.
pub fn executor_instance_state(&mut self) -> &mut U {
/// Provides access to the ad-hoc executor instance state.
pub fn executor_instance_state(&self) -> &U {
&self.executor
}

/// Provides mutable access to the ad-hoc executor instance state.
pub fn executor_instance_state_mut(&mut self) -> &mut U {
&mut self.executor
}
}
Expand Down Expand Up @@ -295,10 +312,10 @@ mod tests {
let app = App::new("test-app", locked);

let engine_builder = spin_core::Engine::builder(&Default::default())?;
let executor = FactorsExecutor::new(engine_builder, env.factors)?;
let executor = Arc::new(FactorsExecutor::new(engine_builder, env.factors)?);

let factors_app = executor
.load_app(app, Default::default(), DummyComponentLoader)
.load_app(app, Default::default(), &DummyComponentLoader)
.await?;

let mut instance_builder = factors_app.prepare("empty")?;
Expand All @@ -321,7 +338,7 @@ mod tests {
#[async_trait]
impl ComponentLoader for DummyComponentLoader {
async fn load_component(
&mut self,
&self,
engine: &spin_core::wasmtime::Engine,
_component: &AppComponent,
) -> anyhow::Result<Component> {
Expand Down
2 changes: 1 addition & 1 deletion crates/trigger-http/src/wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl HttpExecutor for WasiHttpExecutor {
}));

let mut wasi_http = spin_factor_outbound_http::OutboundHttpFactor::get_wasi_http_impl(
store.data_mut().factors_instance_state(),
store.data_mut().factors_instance_state_mut(),
)
.context("missing OutboundHttpFactor")?;

Expand Down
7 changes: 4 additions & 3 deletions crates/trigger/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ mod sqlite_statements;
mod stdio;
mod summary;

use std::future::Future;
use std::path::PathBuf;
use std::{future::Future, sync::Arc};

use anyhow::{Context, Result};
use clap::{Args, IntoApp, Parser};
Expand Down Expand Up @@ -345,7 +345,7 @@ impl<T: Trigger<B::Factors>, B: RuntimeFactorsBuilder> TriggerAppBuilder<T, B> {
#[async_trait]
impl ComponentLoader for SimpleComponentLoader {
async fn load_component(
&mut self,
&self,
engine: &spin_core::wasmtime::Engine,
component: &spin_factors::AppComponent,
) -> anyhow::Result<spin_core::Component> {
Expand Down Expand Up @@ -373,11 +373,12 @@ impl<T: Trigger<B::Factors>, B: RuntimeFactorsBuilder> TriggerAppBuilder<T, B> {

let mut executor = FactorsExecutor::new(core_engine_builder, factors)?;
B::configure_app(&mut executor, &runtime_config, &common_options, &options)?;
let executor = Arc::new(executor);

let configured_app = {
let _sloth_guard = warn_if_wasm_build_slothful();
executor
.load_app(app, runtime_config.into(), SimpleComponentLoader)
.load_app(app, runtime_config.into(), &SimpleComponentLoader)
.await?
};

Expand Down
2 changes: 1 addition & 1 deletion crates/trigger/src/cli/initial_kv_setter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const DEFAULT_KEY_VALUE_STORE_LABEL: &str = "default";
#[async_trait]
impl<F: RuntimeFactors, U> ExecutorHooks<F, U> for InitialKvSetterHook {
async fn configure_app(
&mut self,
&self,
configured_app: &spin_factors::ConfiguredApp<F>,
) -> anyhow::Result<()> {
if self.kv_pairs.is_empty() {
Expand Down
10 changes: 8 additions & 2 deletions crates/trigger/src/cli/sqlite_statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ impl SqlStatementExecutorHook {
let get_database = |label| async move {
sqlite
.get_connection(label)
.await
.transpose()
.with_context(|| format!("failed connect to database with label '{label}'"))
};
Expand Down Expand Up @@ -70,7 +71,7 @@ where
F: RuntimeFactors,
{
async fn configure_app(
&mut self,
&self,
configured_app: &spin_factors::ConfiguredApp<F>,
) -> anyhow::Result<()> {
let Some(sqlite) = configured_app.app_state::<SqliteFactor>().ok() else {
Expand Down Expand Up @@ -175,8 +176,13 @@ mod tests {
}
}

#[async_trait]
impl ConnectionCreator for MockCreator {
fn create_connection(&self) -> Result<Box<dyn Connection + 'static>, v2::Error> {
async fn create_connection(
&self,
label: &str,
) -> Result<Box<dyn Connection + 'static>, v2::Error> {
let _ = label;
Ok(Box::new(MockConnection {
tx: self.tx.clone(),
}))
Expand Down
2 changes: 1 addition & 1 deletion crates/trigger/src/cli/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl StdioLoggingExecutorHooks {
#[async_trait]
impl<F: RuntimeFactors, U> ExecutorHooks<F, U> for StdioLoggingExecutorHooks {
async fn configure_app(
&mut self,
&self,
configured_app: &spin_factors::ConfiguredApp<F>,
) -> anyhow::Result<()> {
self.validate_follows(configured_app.app())?;
Expand Down
5 changes: 3 additions & 2 deletions crates/trigger/src/cli/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub struct KeyValueDefaultStoreSummaryHook;
#[async_trait]
impl<F: RuntimeFactors, U> ExecutorHooks<F, U> for KeyValueDefaultStoreSummaryHook {
async fn configure_app(
&mut self,
&self,
configured_app: &spin_factors::ConfiguredApp<F>,
) -> anyhow::Result<()> {
let Ok(kv_app_state) = configured_app.app_state::<KeyValueFactor>() else {
Expand All @@ -33,7 +33,7 @@ pub struct SqliteDefaultStoreSummaryHook;
#[async_trait]
impl<F: RuntimeFactors, U> ExecutorHooks<F, U> for SqliteDefaultStoreSummaryHook {
async fn configure_app(
&mut self,
&self,
configured_app: &spin_factors::ConfiguredApp<F>,
) -> anyhow::Result<()> {
let Ok(sqlite_app_state) = configured_app.app_state::<SqliteFactor>() else {
Expand All @@ -45,6 +45,7 @@ impl<F: RuntimeFactors, U> ExecutorHooks<F, U> for SqliteDefaultStoreSummaryHook
}
if let Some(default_database_summary) = sqlite_app_state
.get_connection("default")
.await
.and_then(Result::ok)
.and_then(|conn| conn.summary())
{
Expand Down

0 comments on commit ba32722

Please sign in to comment.