PR feedback

rylev · rylev · commit 1d0c79ea94d1 · 2024-09-04T11:11:35.000+02:00
Signed-off-by: Ryan Levick &lt;ryan.levick@fermyon.com&gt;
diff --git a/crates/factor-key-value/src/lib.rs b/crates/factor-key-value/src/lib.rs
@@ -10,8 +10,8 @@ use std::{
 use anyhow::ensure;
 use host::KEY_VALUE_STORES_KEY;
 use spin_factors::{
-    ConfigureAppContext, Factor, FactorInstanceBuilder, InitContext, InstanceBuilders,
-    PrepareContext, RuntimeFactors,
+    ConfigureAppContext, Factor, FactorInstanceBuilder, InitContext, PrepareContext,
+    PreparedInstanceBuilders, RuntimeFactors,
 };
 use util::{CachingStoreManager, DefaultManagerGetter};
 
@@ -90,7 +90,7 @@ impl Factor for KeyValueFactor {
     fn prepare<T: RuntimeFactors>(
         &self,
         ctx: PrepareContext<Self>,
-        _builders: &mut InstanceBuilders<T>,
+        _builders: &mut PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<InstanceBuilder> {
         let app_state = ctx.app_state();
         let allowed_stores = app_state
diff --git a/crates/factor-llm/src/lib.rs b/crates/factor-llm/src/lib.rs
@@ -6,7 +6,7 @@ use std::sync::Arc;
 
 use async_trait::async_trait;
 use spin_factors::{
-    ConfigureAppContext, Factor, InstanceBuilders, PrepareContext, RuntimeFactors,
+    ConfigureAppContext, Factor, PrepareContext, PreparedInstanceBuilders, RuntimeFactors,
     SelfInstanceBuilder,
 };
 use spin_locked_app::MetadataKey;
@@ -78,7 +78,7 @@ impl Factor for LlmFactor {
     fn prepare<T: RuntimeFactors>(
         &self,
         ctx: PrepareContext<Self>,
-        _builders: &mut InstanceBuilders<T>,
+        _builders: &mut PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<Self::InstanceBuilder> {
         let allowed_models = ctx
             .app_state()
diff --git a/crates/factor-outbound-http/src/lib.rs b/crates/factor-outbound-http/src/lib.rs
@@ -14,7 +14,7 @@ use spin_factor_outbound_networking::{
     ComponentTlsConfigs, OutboundAllowedHosts, OutboundNetworkingFactor,
 };
 use spin_factors::{
-    anyhow, ConfigureAppContext, Factor, InstanceBuilders, PrepareContext, RuntimeFactors,
+    anyhow, ConfigureAppContext, Factor, PrepareContext, PreparedInstanceBuilders, RuntimeFactors,
     SelfInstanceBuilder,
 };
 use wasmtime_wasi_http::WasiHttpCtx;
@@ -60,7 +60,7 @@ impl Factor for OutboundHttpFactor {
     fn prepare<T: RuntimeFactors>(
         &self,
         _ctx: PrepareContext<Self>,
-        builders: &mut InstanceBuilders<T>,
+        builders: &mut PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<Self::InstanceBuilder> {
         let outbound_networking = builders.get_mut::<OutboundNetworkingFactor>()?;
         let allowed_hosts = outbound_networking.allowed_hosts();
diff --git a/crates/factor-outbound-mqtt/src/lib.rs b/crates/factor-outbound-mqtt/src/lib.rs
@@ -9,7 +9,7 @@ use rumqttc::{AsyncClient, Event, Incoming, Outgoing, QoS};
 use spin_core::async_trait;
 use spin_factor_outbound_networking::OutboundNetworkingFactor;
 use spin_factors::{
-    ConfigureAppContext, Factor, InstanceBuilders, PrepareContext, RuntimeFactors,
+    ConfigureAppContext, Factor, PrepareContext, PreparedInstanceBuilders, RuntimeFactors,
     SelfInstanceBuilder,
 };
 use spin_world::v2::mqtt::{self as v2, Error, Qos};
@@ -50,7 +50,7 @@ impl Factor for OutboundMqttFactor {
     fn prepare<T: RuntimeFactors>(
         &self,
         _ctx: PrepareContext<Self>,
-        builders: &mut InstanceBuilders<T>,
+        builders: &mut PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<Self::InstanceBuilder> {
         let allowed_hosts = builders
             .get_mut::<OutboundNetworkingFactor>()?
diff --git a/crates/factor-outbound-mysql/src/lib.rs b/crates/factor-outbound-mysql/src/lib.rs
@@ -33,7 +33,7 @@ impl<C: Send + Sync + Client + 'static> Factor for OutboundMysqlFactor<C> {
     fn prepare<T: spin_factors::RuntimeFactors>(
         &self,
         _ctx: spin_factors::PrepareContext<Self>,
-        builders: &mut spin_factors::InstanceBuilders<T>,
+        builders: &mut spin_factors::PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<Self::InstanceBuilder> {
         let allowed_hosts = builders
             .get_mut::<OutboundNetworkingFactor>()?
diff --git a/crates/factor-outbound-networking/src/lib.rs b/crates/factor-outbound-networking/src/lib.rs
@@ -13,8 +13,8 @@ use spin_factor_variables::VariablesFactor;
 use spin_factor_wasi::{SocketAddrUse, WasiFactor};
 use spin_factors::{
     anyhow::{self, Context},
-    ConfigureAppContext, Error, Factor, FactorInstanceBuilder, InstanceBuilders, PrepareContext,
-    RuntimeFactors,
+    ConfigureAppContext, Error, Factor, FactorInstanceBuilder, PrepareContext,
+    PreparedInstanceBuilders, RuntimeFactors,
 };
 
 pub use config::{
@@ -83,7 +83,7 @@ impl Factor for OutboundNetworkingFactor {
     fn prepare<T: RuntimeFactors>(
         &self,
         ctx: PrepareContext<Self>,
-        builders: &mut InstanceBuilders<T>,
+        builders: &mut PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<Self::InstanceBuilder> {
         let hosts = ctx
             .app_state()
diff --git a/crates/factor-outbound-pg/src/lib.rs b/crates/factor-outbound-pg/src/lib.rs
@@ -4,7 +4,7 @@ mod host;
 use client::Client;
 use spin_factor_outbound_networking::{OutboundAllowedHosts, OutboundNetworkingFactor};
 use spin_factors::{
-    anyhow, ConfigureAppContext, Factor, InstanceBuilders, PrepareContext, RuntimeFactors,
+    anyhow, ConfigureAppContext, Factor, PrepareContext, PreparedInstanceBuilders, RuntimeFactors,
     SelfInstanceBuilder,
 };
 use tokio_postgres::Client as PgClient;
@@ -37,7 +37,7 @@ impl<C: Send + Sync + Client + 'static> Factor for OutboundPgFactor<C> {
     fn prepare<T: RuntimeFactors>(
         &self,
         _ctx: PrepareContext<Self>,
-        builders: &mut InstanceBuilders<T>,
+        builders: &mut PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<Self::InstanceBuilder> {
         let allowed_hosts = builders
             .get_mut::<OutboundNetworkingFactor>()?
diff --git a/crates/factor-outbound-redis/src/lib.rs b/crates/factor-outbound-redis/src/lib.rs
@@ -3,7 +3,7 @@ mod host;
 use host::InstanceState;
 use spin_factor_outbound_networking::OutboundNetworkingFactor;
 use spin_factors::{
-    anyhow, ConfigureAppContext, Factor, InstanceBuilders, PrepareContext, RuntimeFactors,
+    anyhow, ConfigureAppContext, Factor, PrepareContext, PreparedInstanceBuilders, RuntimeFactors,
     SelfInstanceBuilder,
 };
 
@@ -43,7 +43,7 @@ impl Factor for OutboundRedisFactor {
     fn prepare<T: RuntimeFactors>(
         &self,
         _ctx: PrepareContext<Self>,
-        builders: &mut InstanceBuilders<T>,
+        builders: &mut PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<Self::InstanceBuilder> {
         let allowed_hosts = builders
             .get_mut::<OutboundNetworkingFactor>()?
diff --git a/crates/factor-sqlite/src/lib.rs b/crates/factor-sqlite/src/lib.rs
@@ -87,7 +87,7 @@ impl Factor for SqliteFactor {
     fn prepare<T: spin_factors::RuntimeFactors>(
         &self,
         ctx: spin_factors::PrepareContext<Self>,
-        _builders: &mut spin_factors::InstanceBuilders<T>,
+        _builders: &mut spin_factors::PreparedInstanceBuilders<T>,
     ) -> spin_factors::anyhow::Result<Self::InstanceBuilder> {
         let allowed_databases = ctx
             .app_state()
diff --git a/crates/factor-variables/src/lib.rs b/crates/factor-variables/src/lib.rs
@@ -7,7 +7,7 @@ use std::sync::Arc;
 use runtime_config::RuntimeConfig;
 use spin_expressions::{ProviderResolver as ExpressionResolver, Template};
 use spin_factors::{
-    anyhow, ConfigureAppContext, Factor, InitContext, InstanceBuilders, PrepareContext,
+    anyhow, ConfigureAppContext, Factor, InitContext, PrepareContext, PreparedInstanceBuilders,
     RuntimeFactors, SelfInstanceBuilder,
 };
 
@@ -55,7 +55,7 @@ impl Factor for VariablesFactor {
     fn prepare<T: RuntimeFactors>(
         &self,
         ctx: PrepareContext<Self>,
-        _builders: &mut InstanceBuilders<T>,
+        _builders: &mut PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<InstanceState> {
         let component_id = ctx.app_component().id().to_string();
         let expression_resolver = ctx.app_state().expression_resolver.clone();
diff --git a/crates/factor-wasi/src/lib.rs b/crates/factor-wasi/src/lib.rs
@@ -12,8 +12,8 @@ use std::{
 
 use io::{PipeReadStream, PipedWriteStream};
 use spin_factors::{
-    anyhow, AppComponent, Factor, FactorInstanceBuilder, InitContext, InstanceBuilders,
-    PrepareContext, RuntimeFactors, RuntimeFactorsInstanceState,
+    anyhow, AppComponent, Factor, FactorInstanceBuilder, InitContext, PrepareContext,
+    PreparedInstanceBuilders, RuntimeFactors, RuntimeFactorsInstanceState,
 };
 use wasmtime_wasi::{
     DirPerms, FilePerms, ResourceTable, StdinStream, StdoutStream, WasiCtx, WasiCtxBuilder,
@@ -110,7 +110,7 @@ impl Factor for WasiFactor {
     fn prepare<T: RuntimeFactors>(
         &self,
         ctx: PrepareContext<Self>,
-        _builders: &mut InstanceBuilders<T>,
+        _builders: &mut PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<InstanceBuilder> {
         let mut wasi_ctx = WasiCtxBuilder::new();
 
diff --git a/crates/factors-derive/src/lib.rs b/crates/factors-derive/src/lib.rs
@@ -150,7 +150,7 @@ fn expand_factors(input: &DeriveInput) -> syn::Result<TokenStream> {
                                 configured_app.app_state::<#factor_types>().unwrap(),
                                 &app_component,
                             ),
-                            &mut #factors_path::InstanceBuilders::new(&mut builders),
+                            &mut #factors_path::PreparedInstanceBuilders::new(&mut builders),
                         ).map_err(#Error::factor_prepare_error::<#factor_types>)?
                     );
                 )*
diff --git a/crates/factors-executor/src/lib.rs b/crates/factors-executor/src/lib.rs
@@ -3,7 +3,10 @@ use std::collections::HashMap;
 use anyhow::Context;
 use spin_app::{App, AppComponent};
 use spin_core::{async_trait, Component};
-use spin_factors::{AsInstanceState, ConfiguredApp, RuntimeFactors, RuntimeFactorsInstanceState};
+use spin_factors::{
+    AsInstanceState, ConfiguredApp, Factor, HasInstanceBuilder, RuntimeFactors,
+    RuntimeFactorsInstanceState,
+};
 
 /// A FactorsExecutor manages execution of a Spin app.
 ///
@@ -203,6 +206,11 @@ impl<'a, T: RuntimeFactors, U> FactorsInstanceBuilder<'a, T, U> {
     pub fn factor_builders(&mut self) -> &mut T::InstanceBuilders {
         &mut self.factor_builders
     }
+
+    /// Returns the specific instance builder for the given factor.
+    pub fn factor_builder<F: Factor>(&mut self) -> Option<&mut F::InstanceBuilder> {
+        self.factor_builders().for_factor::<F>()
+    }
 }
 
 impl<'a, T: RuntimeFactors, U: Send> FactorsInstanceBuilder<'a, T, U> {
@@ -300,9 +308,7 @@ mod tests {
         instance_builder.store_builder().max_memory_size(1_000_000);
 
         instance_builder
-            .factor_builders()
-            .wasi
-            .as_mut()
+            .factor_builder::<WasiFactor>()
             .unwrap()
             .args(["foo"]);
 
diff --git a/crates/factors/src/factor.rs b/crates/factors/src/factor.rs
@@ -3,7 +3,8 @@ use std::any::Any;
 use wasmtime::component::{Linker, ResourceTable};
 
 use crate::{
-    prepare::FactorInstanceBuilder, App, Error, InstanceBuilders, PrepareContext, RuntimeFactors,
+    prepare::FactorInstanceBuilder, App, Error, PrepareContext, PreparedInstanceBuilders,
+    RuntimeFactors,
 };
 
 /// A contained (i.e., "factored") piece of runtime functionality.
@@ -65,7 +66,7 @@ pub trait Factor: Any + Sized {
     fn prepare<T: RuntimeFactors>(
         &self,
         ctx: PrepareContext<Self>,
-        _builders: &mut InstanceBuilders<T>,
+        _builders: &mut PreparedInstanceBuilders<T>,
     ) -> anyhow::Result<Self::InstanceBuilder>;
 }
 
diff --git a/crates/factors/src/lib.rs b/crates/factors/src/lib.rs
@@ -12,7 +12,9 @@ pub use spin_factors_derive::RuntimeFactors;
 
 pub use crate::{
     factor::{ConfigureAppContext, ConfiguredApp, Factor, FactorInstanceState, InitContext},
-    prepare::{FactorInstanceBuilder, InstanceBuilders, PrepareContext, SelfInstanceBuilder},
+    prepare::{
+        FactorInstanceBuilder, PrepareContext, PreparedInstanceBuilders, SelfInstanceBuilder,
+    },
     runtime_config::{FactorRuntimeConfigSource, RuntimeConfigSourceFinalizer},
     runtime_factors::{
         AsInstanceState, HasInstanceBuilder, RuntimeFactors, RuntimeFactorsInstanceState,
diff --git a/crates/factors/src/prepare.rs b/crates/factors/src/prepare.rs
@@ -67,11 +67,11 @@ impl<'a, F: Factor> PrepareContext<'a, F> {
 /// The collection of all the already prepared `InstanceBuilder`s.
 ///
 /// Use `InstanceBuilders::get_mut` to get a mutable reference to a specific factor's instance builder.
-pub struct InstanceBuilders<'a, T: RuntimeFactors> {
+pub struct PreparedInstanceBuilders<'a, T: RuntimeFactors> {
     pub(crate) inner: &'a mut T::InstanceBuilders,
 }
 
-impl<'a, T: RuntimeFactors> InstanceBuilders<'a, T> {
+impl<'a, T: RuntimeFactors> PreparedInstanceBuilders<'a, T> {
     #[doc(hidden)]
     pub fn new(inner: &'a mut T::InstanceBuilders) -> Self {
         Self { inner }
diff --git a/crates/runtime-factors/src/lib.rs b/crates/runtime-factors/src/lib.rs
@@ -88,7 +88,7 @@ fn outbound_networking_factor() -> OutboundNetworkingFactor {
 
 /// Options for building a [`TriggerFactors`].
 #[derive(Default, clap::Args)]
-pub struct TriggerAppOptions {
+pub struct TriggerAppArgs {
     /// Set the static assets of the components in the temporary directory as writable.
     #[clap(long = "allow-transient-write")]
     pub allow_transient_write: bool,
diff --git a/crates/runtime-factors/src/runtime_factors.rs b/crates/runtime-factors/src/runtime_factors.rs
@@ -1,49 +1,45 @@
 use std::path::{Path, PathBuf};
 
-use super::{TriggerAppOptions, TriggerFactors, TriggerFactorsRuntimeConfig};
+use super::{TriggerAppArgs, TriggerFactors, TriggerFactorsRuntimeConfig};
 
 use anyhow::Context as _;
 use spin_common::ui::quoted_path;
 use spin_factors_executor::FactorsExecutor;
 use spin_runtime_config::ResolvedRuntimeConfig;
 use spin_trigger::cli::{
-    CommonTriggerOptions, InitialKvSetterHook, KeyValueDefaultStoreSummaryHook,
-    RuntimeFactorsBuilder, SqlStatementExecutorHook, SqliteDefaultStoreSummaryHook,
-    StdioLoggingExecutorHooks,
+    FactorsConfig, InitialKvSetterHook, KeyValueDefaultStoreSummaryHook, RuntimeFactorsBuilder,
+    SqlStatementExecutorHook, SqliteDefaultStoreSummaryHook, StdioLoggingExecutorHooks,
 };
 use toml::Value;
 
 /// A [`RuntimeFactorsBuilder`] for [`TriggerFactors`].
 pub struct FactorsBuilder;
 
 impl RuntimeFactorsBuilder for FactorsBuilder {
-    type Options = TriggerAppOptions;
+    type CliArgs = TriggerAppArgs;
     type Factors = TriggerFactors;
     type RuntimeConfig = ResolvedRuntimeConfig<TriggerFactorsRuntimeConfig>;
 
     fn build(
-        common_options: &CommonTriggerOptions,
-        options: &Self::Options,
+        config: &FactorsConfig,
+        args: &Self::CliArgs,
     ) -> anyhow::Result<(Self::Factors, Self::RuntimeConfig)> {
         // Hardcode `use_gpu` to true for now
         let use_gpu = true;
         let runtime_config = ResolvedRuntimeConfig::<TriggerFactorsRuntimeConfig>::from_file(
-            common_options.runtime_config_file.clone().as_deref(),
-            common_options.local_app_dir.clone().map(PathBuf::from),
-            common_options.state_dir.clone(),
-            common_options.log_dir.clone(),
+            config.runtime_config_file.clone().as_deref(),
+            config.local_app_dir.clone().map(PathBuf::from),
+            config.state_dir.clone(),
+            config.log_dir.clone(),
             use_gpu,
         )?;
 
-        summarize_runtime_config(
-            &runtime_config,
-            common_options.runtime_config_file.as_deref(),
-        );
+        summarize_runtime_config(&runtime_config, config.runtime_config_file.as_deref());
 
         let factors = TriggerFactors::new(
             runtime_config.state_dir(),
-            common_options.working_dir.clone(),
-            options.allow_transient_write,
+            config.working_dir.clone(),
+            args.allow_transient_write,
             runtime_config.key_value_resolver.clone(),
             runtime_config.sqlite_resolver.clone(),
             use_gpu,
@@ -55,17 +51,17 @@ impl RuntimeFactorsBuilder for FactorsBuilder {
     fn configure_app<U: Send + 'static>(
         executor: &mut FactorsExecutor<Self::Factors, U>,
         runtime_config: &Self::RuntimeConfig,
-        common_options: &CommonTriggerOptions,
-        options: &Self::Options,
+        config: &FactorsConfig,
+        args: &Self::CliArgs,
     ) -> anyhow::Result<()> {
         executor.add_hooks(StdioLoggingExecutorHooks::new(
-            common_options.follow_components.clone(),
+            config.follow_components.clone(),
             runtime_config.log_dir(),
         ));
         executor.add_hooks(SqlStatementExecutorHook::new(
-            options.sqlite_statements.clone(),
+            args.sqlite_statements.clone(),
         ));
-        executor.add_hooks(InitialKvSetterHook::new(options.key_values.clone()));
+        executor.add_hooks(InitialKvSetterHook::new(args.key_values.clone()));
         executor.add_hooks(SqliteDefaultStoreSummaryHook);
         executor.add_hooks(KeyValueDefaultStoreSummaryHook);
         Ok(())
diff --git a/crates/trigger-http/src/server.rs b/crates/trigger-http/src/server.rs
diff --git a/crates/trigger-http/src/wagi.rs b/crates/trigger-http/src/wagi.rs
diff --git a/crates/trigger/src/cli.rs b/crates/trigger/src/cli.rs
diff --git a/crates/trigger/src/cli/stdio.rs b/crates/trigger/src/cli/stdio.rs
diff --git a/tests/testing-framework/src/runtimes/in_process_spin.rs b/tests/testing-framework/src/runtimes/in_process_spin.rs
