refactor: test & function rename + improved readability

Signed-off-by: Aminu Oluwaseun Joshua <[email protected]>

Author: seun-ja

Cargo.lock

@@ -12,6 +12,5 @@ spin-locked-app = { path = "../locked-app" }
 thiserror = { workspace = true }
 
 [dev-dependencies]
-spin-world = { path = "../world" }
 tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
 toml = { workspace = true }

crates/expressions/Cargo.toml

@@ -92,8 +92,8 @@ impl ProviderResolver {
         Ok(PreparedResolver { variables })
     }
 
-    /// Validates `Provider`(s) that are `ProviderVariableKind::Static` provide their variables at startup.
-    pub async fn validate_variables(&self) -> Result<()> {
+    /// Ensures that all required variables are resolvable at startup
+    pub async fn ensure_required_variables_resolved(&self) -> Result<()> {
         // If a dynamic provider is available, skip validation.
         if self
             .providers

crates/expressions/src/lib.rs

@@ -1,5 +1,6 @@
 use std::collections::HashMap;
 
+use async_trait::async_trait;
 use spin_expressions::{provider::ProviderVariableKind, Key, Provider, ProviderResolver};
 use spin_locked_app::Variable;
 
@@ -14,18 +15,12 @@ impl ResolverTester {
         Self::default()
     }
 
-    fn with_dynamic_provider(mut self) -> Self {
-        self.providers.push(Box::new(DynamicProvider));
+    fn with_provider(mut self, provider: impl Provider + 'static) -> Self {
+        self.providers.push(Box::new(provider));
         self
     }
 
-    fn with_static_provider(mut self, key: &str, value: Option<&str>) -> Self {
-        self.providers
-            .push(Box::new(StaticProvider::with_variable(key, value)));
-        self
-    }
-
-    fn with_variable(mut self, (key, default): (&str, Option<&str>)) -> Self {
+    fn with_variable(mut self, key: &str, default: Option<&str>) -> Self {
         self.variables.insert(
             key.to_string(),
             Variable {
@@ -41,32 +36,33 @@ impl ResolverTester {
         let mut provider_resolver = ProviderResolver::new(self.variables)?;
 
         for provider in self.providers {
-            provider_resolver.add_provider(provider as _);
+            provider_resolver.add_provider(provider);
         }
 
         Ok(provider_resolver)
     }
 }
 
 #[tokio::test(flavor = "multi_thread")]
-async fn single_static_provider_with_no_variable_provided_is_valid() -> anyhow::Result<()> {
+async fn if_single_static_provider_with_no_key_to_resolve_is_valid() -> anyhow::Result<()> {
     let resolver = ResolverTester::new()
-        .with_static_provider("foo", Some("bar"))
+        .with_provider(StaticProvider::with_variable("foo", Some("bar")))
         .make_resolver()?;
 
-    resolver.validate_variables().await?;
+    resolver.ensure_required_variables_resolved().await?;
 
     Ok(())
 }
 
 #[tokio::test(flavor = "multi_thread")]
-async fn if_single_static_provider_has_variable_value_validation_succeeds() -> anyhow::Result<()> {
+async fn if_single_static_provider_has_data_for_variable_key_to_resolve_it_succeeds(
+) -> anyhow::Result<()> {
     let resolver = ResolverTester::new()
-        .with_static_provider("foo", Some("bar"))
-        .with_variable(("foo", None))
+        .with_provider(StaticProvider::with_variable("foo", Some("bar")))
+        .with_variable("foo", None)
         .make_resolver()?;
 
-    resolver.validate_variables().await?;
+    resolver.ensure_required_variables_resolved().await?;
 
     Ok(())
 }
@@ -75,38 +71,38 @@ async fn if_single_static_provider_has_variable_value_validation_succeeds() -> a
 async fn if_there_is_a_single_static_provider_and_it_does_not_contain_a_required_variable_then_validation_fails(
 ) -> anyhow::Result<()> {
     let resolver = ResolverTester::new()
-        .with_static_provider("foo", Some("bar"))
-        .with_variable(("bar", None))
+        .with_provider(StaticProvider::with_variable("foo", Some("bar")))
+        .with_variable("bar", None)
         .make_resolver()?;
 
-    assert!(resolver.validate_variables().await.is_err());
+    assert!(resolver.ensure_required_variables_resolved().await.is_err());
 
     Ok(())
 }
 
 #[tokio::test(flavor = "multi_thread")]
-async fn if_there_is_a_dynamic_provider_then_validation_succeeds_even_if_a_static_provider_without_the_variable_is_in_play(
+async fn if_there_is_a_dynamic_provider_then_validation_succeeds_even_without_default_value_in_play(
 ) -> anyhow::Result<()> {
     let resolver = ResolverTester::new()
-        .with_dynamic_provider()
-        .with_variable(("bar", None))
+        .with_provider(DynamicProvider)
+        .with_variable("bar", None)
         .make_resolver()?;
 
-    resolver.validate_variables().await?;
+    resolver.ensure_required_variables_resolved().await?;
 
     Ok(())
 }
 
 #[tokio::test(flavor = "multi_thread")]
-async fn if_there_is_a_dynamic_provider_and_a_static_provider_then_validation_succeeds_even_if_a_static_provider_without_the_variable_is_in_play(
+async fn if_there_is_a_dynamic_provider_and_static_provider_but_the_variable_to_be_resolved_is_not_in_play(
 ) -> anyhow::Result<()> {
     let resolver = ResolverTester::new()
-        .with_dynamic_provider()
-        .with_static_provider("foo", Some("bar"))
-        .with_variable(("baz", None))
+        .with_provider(DynamicProvider)
+        .with_provider(StaticProvider::with_variable("foo", Some("bar")))
+        .with_variable("baz", None)
         .make_resolver()?;
 
-    resolver.validate_variables().await?;
+    resolver.ensure_required_variables_resolved().await?;
 
     Ok(())
 }
@@ -115,52 +111,78 @@ async fn if_there_is_a_dynamic_provider_and_a_static_provider_then_validation_su
 async fn if_there_is_a_dynamic_provider_and_a_static_provider_then_validation_succeeds_even_if_a_static_provider_with_the_variable_is_in_play(
 ) -> anyhow::Result<()> {
     let resolver = ResolverTester::new()
-        .with_dynamic_provider()
-        .with_static_provider("foo", Some("bar"))
-        .with_variable(("baz", Some("coo")))
+        .with_provider(DynamicProvider)
+        .with_provider(StaticProvider::with_variable("foo", Some("bar")))
+        .with_variable("baz", Some("coo"))
         .make_resolver()?;
 
-    resolver.validate_variables().await?;
+    resolver.ensure_required_variables_resolved().await?;
 
     Ok(())
 }
 
 #[tokio::test(flavor = "multi_thread")]
-async fn if_there_is_two_static_providers_where_one_has_data_is_valid() -> anyhow::Result<()> {
+async fn if_there_are_two_static_providers_where_one_has_data_is_valid() -> anyhow::Result<()> {
     let resolver = ResolverTester::new()
-        .with_static_provider("foo", Some("bar"))
-        .with_static_provider("baz", Some("hay"))
-        .with_variable(("foo", None))
+        .with_provider(StaticProvider::with_variable("foo", Some("bar")))
+        .with_provider(StaticProvider::with_variable("baz", Some("hay")))
+        .with_variable("foo", None)
         .make_resolver()?;
 
-    resolver.validate_variables().await?;
+    resolver.ensure_required_variables_resolved().await?;
 
     Ok(())
 }
-
+// This is a bit of an edge case, but we want to ensure that if there are two or more static providers
+// and the first one does not have data for the variable to be resolved, but the second or subsequent one does,
+// then validation still succeeds.
 #[tokio::test(flavor = "multi_thread")]
-async fn if_there_is_two_static_providers_where_first_provider_does_not_have_data_while_second_provider_does(
+async fn if_there_are_two_static_providers_where_first_provider_does_not_have_data_while_second_provider_does(
 ) -> anyhow::Result<()> {
     let resolver = ResolverTester::new()
-        .with_static_provider("foo", Some("bar"))
-        .with_static_provider("baz", Some("hay"))
-        .with_variable(("baz", None))
+        .with_provider(StaticProvider::with_variable("foo", Some("bar")))
+        .with_provider(StaticProvider::with_variable("baz", Some("hay")))
+        .with_variable("baz", None)
         .make_resolver()?;
 
-    resolver.validate_variables().await?;
+    resolver.ensure_required_variables_resolved().await?;
 
     Ok(())
 }
 
 #[tokio::test(flavor = "multi_thread")]
 async fn if_there_is_two_static_providers_neither_having_data_is_invalid() -> anyhow::Result<()> {
     let resolver = ResolverTester::new()
-        .with_static_provider("foo", Some("bar"))
-        .with_static_provider("baz", Some("hay"))
-        .with_variable(("hello", None))
+        .with_provider(StaticProvider::with_variable("foo", Some("bar")))
+        .with_provider(StaticProvider::with_variable("baz", Some("hay")))
+        .with_variable("hello", None)
+        .make_resolver()?;
+
+    assert!(resolver.ensure_required_variables_resolved().await.is_err());
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn no_provider_data_available_but_variable_default_value_needed_is_invalid(
+) -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_variable("hello", None)
+        .make_resolver()?;
+
+    assert!(resolver.ensure_required_variables_resolved().await.is_err());
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn no_provider_data_available_but_variable_has_default_value_needed_is_valid(
+) -> anyhow::Result<()> {
+    let resolver = ResolverTester::new()
+        .with_variable("hello", Some("World"))
         .make_resolver()?;
 
-    assert!(resolver.validate_variables().await.is_err());
+    resolver.ensure_required_variables_resolved().await?;
 
     Ok(())
 }
@@ -178,7 +200,7 @@ impl StaticProvider {
     }
 }
 
-#[spin_world::async_trait]
+#[async_trait]
 impl Provider for StaticProvider {
     async fn get(&self, key: &Key) -> anyhow::Result<Option<String>> {
         Ok(self.variables.get(key.as_str()).cloned().flatten())
@@ -192,7 +214,7 @@ impl Provider for StaticProvider {
 #[derive(Debug)]
 struct DynamicProvider;
 
-#[spin_world::async_trait]
+#[async_trait]
 impl Provider for DynamicProvider {
     async fn get(&self, _key: &Key) -> anyhow::Result<Option<String>> {
         panic!("validation should never call get for a dynamic provider")

crates/expressions/tests/validation_test.rs

@@ -14,7 +14,9 @@ impl<F: RuntimeFactors, U> ExecutorHooks<F, U> for VariablesValidatorHook {
         let variables_factor_app_state = configured_app.app_state::<VariablesFactor>()?;
 
         let expression_resolver = variables_factor_app_state.expression_resolver();
-        expression_resolver.validate_variables().await?;
+        expression_resolver
+            .ensure_required_variables_resolved()
+            .await?;
 
         Ok(())
     }