From 23b5d1212bcfc9207f2e29307864f74bb477da65 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Tue, 23 Jul 2024 12:11:47 +0200 Subject: [PATCH] Rename and document for clarity Signed-off-by: Ryan Levick --- crates/factor-outbound-networking/src/lib.rs | 5 +- .../factor-outbound-pg/tests/factor_test.rs | 2 +- .../tests/factor_test.rs | 2 +- crates/factor-variables/src/lib.rs | 48 +++++++++++-------- crates/factor-variables/src/provider.rs | 4 +- crates/factor-variables/src/spin_cli/env.rs | 6 +-- crates/factor-variables/src/spin_cli/mod.rs | 3 ++ .../factor-variables/src/spin_cli/statik.rs | 6 +-- crates/factor-variables/tests/factor_test.rs | 8 ++-- crates/factors/tests/smoke.rs | 4 +- 10 files changed, 52 insertions(+), 36 deletions(-) diff --git a/crates/factor-outbound-networking/src/lib.rs b/crates/factor-outbound-networking/src/lib.rs index b04262e3f..9d7f04b2d 100644 --- a/crates/factor-outbound-networking/src/lib.rs +++ b/crates/factor-outbound-networking/src/lib.rs @@ -59,7 +59,10 @@ impl Factor for OutboundNetworkingFactor { .get(ctx.app_component().id()) .cloned() .context("missing component allowed hosts")?; - let resolver = builders.get_mut::()?.resolver().clone(); + let resolver = builders + .get_mut::()? + .expression_resolver() + .clone(); let allowed_hosts_future = async move { let prepared = resolver.prepare().await?; AllowedHostsConfig::parse(&hosts, &prepared) diff --git a/crates/factor-outbound-pg/tests/factor_test.rs b/crates/factor-outbound-pg/tests/factor_test.rs index 4f2f78852..f6f91fa85 100644 --- a/crates/factor-outbound-pg/tests/factor_test.rs +++ b/crates/factor-outbound-pg/tests/factor_test.rs @@ -31,7 +31,7 @@ async fn disallowed_host_fails() -> anyhow::Result<()> { networking: OutboundNetworkingFactor, pg: OutboundPgFactor, }; - factors.variables.add_provider_type(StaticVariables)?; + factors.variables.add_provider_resolver(StaticVariables)?; let env = test_env(); let mut state = env.build_instance_state(factors).await?; diff --git a/crates/factor-outbound-redis/tests/factor_test.rs b/crates/factor-outbound-redis/tests/factor_test.rs index 552a871cc..e0d42dd54 100644 --- a/crates/factor-outbound-redis/tests/factor_test.rs +++ b/crates/factor-outbound-redis/tests/factor_test.rs @@ -28,7 +28,7 @@ fn get_test_factors() -> TestFactors { async fn no_outbound_hosts_fails() -> anyhow::Result<()> { let mut factors = get_test_factors(); - factors.variables.add_provider_type(StaticVariables)?; + factors.variables.add_provider_resolver(StaticVariables)?; let env = TestEnvironment { manifest: toml! { diff --git a/crates/factor-variables/src/lib.rs b/crates/factor-variables/src/lib.rs index 94ad9441c..ffa7b0787 100644 --- a/crates/factor-variables/src/lib.rs +++ b/crates/factor-variables/src/lib.rs @@ -4,33 +4,40 @@ pub mod spin_cli; use std::sync::Arc; use serde::{de::DeserializeOwned, Deserialize}; -use spin_expressions::ProviderResolver; +use spin_expressions::ProviderResolver as ExpressionResolver; use spin_factors::{ anyhow, ConfigureAppContext, Factor, FactorRuntimeConfig, InitContext, InstanceBuilders, PrepareContext, RuntimeFactors, SelfInstanceBuilder, }; use spin_world::{async_trait, v1, v2::variables}; -pub use provider::MakeVariablesProvider; +pub use provider::ProviderResolver; +/// A factor for providing variables to components. +/// +/// The factor is generic over the type of runtime configuration used to configure the providers. pub struct VariablesFactor { - provider_types: Vec>>, + provider_resolvers: Vec>>, } impl Default for VariablesFactor { fn default() -> Self { Self { - provider_types: Default::default(), + provider_resolvers: Default::default(), } } } impl VariablesFactor { - pub fn add_provider_type>( + /// Adds a provider resolver to the factor. + /// + /// Each added provider will be called in order with the runtime configuration. This order + /// will be the order in which the providers are called to resolve variables. + pub fn add_provider_resolver>( &mut self, provider_type: T, ) -> anyhow::Result<()> { - self.provider_types.push(Box::new(provider_type) as _); + self.provider_resolvers.push(Box::new(provider_type)); Ok(()) } } @@ -51,11 +58,11 @@ impl Factor for VariablesFactor { mut ctx: ConfigureAppContext, ) -> anyhow::Result { let app = ctx.app(); - let mut resolver = - ProviderResolver::new(app.variables().map(|(key, val)| (key.clone(), val.clone())))?; + let mut expression_resolver = + ExpressionResolver::new(app.variables().map(|(key, val)| (key.clone(), val.clone())))?; for component in app.components() { - resolver.add_component_variables( + expression_resolver.add_component_variables( component.id(), component.config().map(|(k, v)| (k.into(), v.into())), )?; @@ -63,16 +70,16 @@ impl Factor for VariablesFactor { if let Some(runtime_config) = ctx.take_runtime_config() { for config in runtime_config.provider_configs { - for make_provider in self.provider_types.iter() { - if let Some(provider) = make_provider.make_provider(&config)? { - resolver.add_provider(provider); + for provider_resolver in self.provider_resolvers.iter() { + if let Some(provider) = provider_resolver.resolve_provider(&config)? { + expression_resolver.add_provider(provider); } } } } Ok(AppState { - resolver: Arc::new(resolver), + expression_resolver: Arc::new(expression_resolver), }) } @@ -82,14 +89,15 @@ impl Factor for VariablesFactor { _builders: &mut InstanceBuilders, ) -> anyhow::Result { let component_id = ctx.app_component().id().to_string(); - let resolver = ctx.app_state().resolver.clone(); + let expression_resolver = ctx.app_state().expression_resolver.clone(); Ok(InstanceState { component_id, - resolver, + expression_resolver, }) } } +/// The runtime configuration for the variables factor. #[derive(Deserialize)] #[serde(transparent)] pub struct RuntimeConfig { @@ -101,17 +109,17 @@ impl FactorRuntimeConfig for RuntimeConfig { } pub struct AppState { - resolver: Arc, + expression_resolver: Arc, } pub struct InstanceState { component_id: String, - resolver: Arc, + expression_resolver: Arc, } impl InstanceState { - pub fn resolver(&self) -> &Arc { - &self.resolver + pub fn expression_resolver(&self) -> &Arc { + &self.expression_resolver } } @@ -121,7 +129,7 @@ impl SelfInstanceBuilder for InstanceState {} impl variables::Host for InstanceState { async fn get(&mut self, key: String) -> Result { let key = spin_expressions::Key::new(&key).map_err(expressions_to_variables_err)?; - self.resolver + self.expression_resolver .resolve(&self.component_id, key) .await .map_err(expressions_to_variables_err) diff --git a/crates/factor-variables/src/provider.rs b/crates/factor-variables/src/provider.rs index 6a8795fb9..82821fb32 100644 --- a/crates/factor-variables/src/provider.rs +++ b/crates/factor-variables/src/provider.rs @@ -3,14 +3,14 @@ use spin_expressions::Provider; use spin_factors::anyhow; /// A trait for converting a runtime configuration into a variables provider. -pub trait MakeVariablesProvider: 'static { +pub trait ProviderResolver: 'static { /// Serialized configuration for the provider. type RuntimeConfig: DeserializeOwned; /// Create a variables provider from the given runtime configuration. /// /// Returns `Ok(None)` if the provider is not applicable to the given configuration. - fn make_provider( + fn resolve_provider( &self, runtime_config: &Self::RuntimeConfig, ) -> anyhow::Result>>; diff --git a/crates/factor-variables/src/spin_cli/env.rs b/crates/factor-variables/src/spin_cli/env.rs index ca4ad6fa8..31717923b 100644 --- a/crates/factor-variables/src/spin_cli/env.rs +++ b/crates/factor-variables/src/spin_cli/env.rs @@ -11,17 +11,17 @@ use spin_factors::anyhow::{self, Context as _}; use spin_world::async_trait; use tracing::{instrument, Level}; -use crate::MakeVariablesProvider; +use crate::ProviderResolver; use super::RuntimeConfig; /// Creator of a environment variables provider. pub struct EnvVariables; -impl MakeVariablesProvider for EnvVariables { +impl ProviderResolver for EnvVariables { type RuntimeConfig = RuntimeConfig; - fn make_provider( + fn resolve_provider( &self, runtime_config: &Self::RuntimeConfig, ) -> anyhow::Result>> { diff --git a/crates/factor-variables/src/spin_cli/mod.rs b/crates/factor-variables/src/spin_cli/mod.rs index 448625d8d..dd1b79bfd 100644 --- a/crates/factor-variables/src/spin_cli/mod.rs +++ b/crates/factor-variables/src/spin_cli/mod.rs @@ -9,9 +9,12 @@ pub use statik::StaticVariables; use serde::Deserialize; use statik::StaticVariablesProvider; +/// The runtime configuration for the variables factor used in the Spin CLI. #[derive(Debug, Deserialize)] #[serde(rename_all = "snake_case", tag = "type")] pub enum RuntimeConfig { + /// A static provider of variables. Static(StaticVariablesProvider), + /// An environment variable provider. Env(env::EnvVariablesConfig), } diff --git a/crates/factor-variables/src/spin_cli/statik.rs b/crates/factor-variables/src/spin_cli/statik.rs index 8e31cff12..a63694057 100644 --- a/crates/factor-variables/src/spin_cli/statik.rs +++ b/crates/factor-variables/src/spin_cli/statik.rs @@ -4,17 +4,17 @@ use serde::Deserialize; use spin_expressions::{async_trait::async_trait, Key, Provider}; use spin_factors::anyhow; -use crate::MakeVariablesProvider; +use crate::ProviderResolver; use super::RuntimeConfig; /// Creator of a static variables provider. pub struct StaticVariables; -impl MakeVariablesProvider for StaticVariables { +impl ProviderResolver for StaticVariables { type RuntimeConfig = RuntimeConfig; - fn make_provider( + fn resolve_provider( &self, runtime_config: &Self::RuntimeConfig, ) -> anyhow::Result>> { diff --git a/crates/factor-variables/tests/factor_test.rs b/crates/factor-variables/tests/factor_test.rs index 734c9fa21..aaf7ceb51 100644 --- a/crates/factor-variables/tests/factor_test.rs +++ b/crates/factor-variables/tests/factor_test.rs @@ -1,4 +1,4 @@ -use spin_factor_variables::spin_cli::{RuntimeConfig, StaticVariables}; +use spin_factor_variables::spin_cli::{EnvVariables, RuntimeConfig, StaticVariables}; use spin_factor_variables::VariablesFactor; use spin_factors::{anyhow, RuntimeFactors}; use spin_factors_test::{toml, TestEnvironment}; @@ -30,13 +30,15 @@ async fn static_provider_works() -> anyhow::Result<()> { let mut factors = TestFactors { variables: VariablesFactor::default(), }; - factors.variables.add_provider_type(StaticVariables)?; + factors.variables.add_provider_resolver(StaticVariables)?; + // The env provider will be ignored since there's no configuration for it. + factors.variables.add_provider_resolver(EnvVariables)?; let env = test_env(); let state = env.build_instance_state(factors).await?; let val = state .variables - .resolver() + .expression_resolver() .resolve("test-component", "baz".try_into().unwrap()) .await?; assert_eq!(val, ""); diff --git a/crates/factors/tests/smoke.rs b/crates/factors/tests/smoke.rs index 45a727c85..e32e9b8cf 100644 --- a/crates/factors/tests/smoke.rs +++ b/crates/factors/tests/smoke.rs @@ -59,7 +59,7 @@ async fn smoke_test_works() -> anyhow::Result<()> { key_value: KeyValueFactor::new(key_value_resolver), }; - factors.variables.add_provider_type(StaticVariables)?; + factors.variables.add_provider_resolver(StaticVariables)?; let locked = spin_loader::from_file( "tests/smoke-app/spin.toml", @@ -81,7 +81,7 @@ async fn smoke_test_works() -> anyhow::Result<()> { assert_eq!( state .variables - .resolver() + .expression_resolver() .resolve("smoke-app", "other".try_into().unwrap()) .await .unwrap(),