Initial review feedback

Signed-off-by: itowlson <ivan.towlson@fermyon.com>

Author: itowlson

crates/compose/src/lib.rs

@@ -35,9 +35,11 @@ pub async fn compose<
 >(
     loader: &L,
     component: &L::Component,
-    complicator: impl Fn(Vec<u8>) -> Fut,
+    apply_trigger_deps: impl Fn(Vec<u8>) -> Fut,
 ) -> Result<Vec<u8>, ComposeError> {
-    Composer::new(loader).compose(component, complicator).await
+    Composer::new(loader)
+        .compose(component, apply_trigger_deps)
+        .await
 }
 
 /// A Spin component dependency. This abstracts over the metadata associated with the
@@ -209,7 +211,7 @@ impl<'a, L: ComponentSourceLoader> Composer<'a, L> {
     async fn compose<Fut: std::future::Future<Output = Result<Vec<u8>, ComposeError>>>(
         mut self,
         component: &L::Component,
-        complicator: impl Fn(Vec<u8>) -> Fut,
+        apply_trigger_deps: impl Fn(Vec<u8>) -> Fut,
     ) -> Result<Vec<u8>, ComposeError> {
         let source = self
             .loader
@@ -244,7 +246,7 @@ impl<'a, L: ComponentSourceLoader> Composer<'a, L> {
                 .map_err(|e| ComposeError::EncodeError(e.into()))?
         };
 
-        let with_extras = complicator(fulfilled_source).await?;
+        let with_extras = apply_trigger_deps(fulfilled_source).await?;
 
         Ok(with_extras)
     }

crates/factors-executor/src/lib.rs

@@ -157,9 +157,7 @@ impl TriggerDependenciesComposer for () {
         if trigger_dependencies.is_empty() {
             Ok(component)
         } else {
-            Err(anyhow::anyhow!(
-                "this trigger should not have complications"
-            ))
+            Err(anyhow::anyhow!("this trigger should not have dependencies"))
         }
     }
 }
@@ -508,7 +506,7 @@ mod tests {
             &self,
             engine: &spin_core::wasmtime::Engine,
             _component: &AppComponent,
-            _complicator: &impl TriggerDependenciesComposer,
+            _trigger_dependencies_composer: &impl TriggerDependenciesComposer,
         ) -> anyhow::Result<Component> {
             Ok(Component::new(engine, "(component)")?)
         }

crates/loader/src/local/trigger_components.rs

@@ -67,7 +67,7 @@ fn needs_splitting(locked: &LockedApp) -> HashMap<String, Vec<LockedTrigger>> {
 
 fn move_deps_from_triggers_to_components(locked: &mut LockedApp) {
     for trigger in &mut locked.triggers {
-        if !trigger.trigger_dependencies.is_empty()
+        if has_trigger_deps(trigger)
             && let Some(component_id) = component_id(trigger)
             && let Some(component) = get_component_mut(&mut locked.components, &component_id)
         {

crates/loader/tests/extra-components/inoffensive.toml

@@ -6,12 +6,12 @@ name = "file-errors"
 [[trigger.http]]
 route = "/a"
 component = "a"
-components.middleware = ["m1", "m2"]
+dependencies.middleware = [{ component = "m1" }, { component = "m2" }]
 
 [[trigger.http]]
 route = "/b"
 component = "b"
-components.middleware = ["m1", "m3"]
+dependencies.middleware = [{ component = "m1" }, { component = "m3" }]
 
 [component.a]
 source = "a.dummy.wasm.txt"

crates/manifest/src/schema/v2.rs

@@ -1019,6 +1019,27 @@ mod tests {
         option: Option<bool>,
     }
 
+    fn as_reference(spec: &ComponentSpec) -> Option<&str> {
+        match spec {
+            ComponentSpec::Reference(id) => Some(id.as_ref()),
+            ComponentSpec::Inline(_) => None,
+        }
+    }
+
+    fn as_inline(spec: &ComponentSpec) -> Option<&Component> {
+        match spec {
+            ComponentSpec::Reference(_) => None,
+            ComponentSpec::Inline(c) => Some(c),
+        }
+    }
+
+    fn as_local(source: &ComponentSource) -> Option<&str> {
+        match source {
+            ComponentSource::Local(path) => Some(path),
+            _ => None,
+        }
+    }
+
     #[test]
     fn deserializing_trigger_configs() {
         let manifest = AppManifest::deserialize(toml! {
@@ -1559,4 +1580,77 @@ mod tests {
             ComponentDependency::Version(v) if v == "1.2.3",
         ));
     }
+
+    #[test]
+    fn can_deserialise_one_or_many_one_ref() {
+        let manifest = AppManifest::deserialize(toml! {
+            spin_manifest_version = 2
+            [application]
+            name = "test"
+            [[trigger.fake]]
+            component = "test1"
+            components = { babble = "test2" }
+        })
+        .expect("manifest should be valid");
+
+        let trigger = manifest.triggers.get("fake").unwrap()[0].clone();
+
+        assert_eq!(
+            Some("test1"),
+            as_reference(trigger.component.as_ref().unwrap())
+        );
+        assert_eq!(1, trigger.components.len());
+        let babble_comps = &trigger.components.get("babble").as_ref().unwrap().0;
+        assert_eq!(1, babble_comps.len());
+        assert_eq!(Some("test2"), as_reference(&babble_comps[0]));
+    }
+
+    #[test]
+    fn can_deserialise_one_or_many_one_inline() {
+        let manifest = AppManifest::deserialize(toml! {
+            spin_manifest_version = 2
+            [application]
+            name = "test"
+            [[trigger.fake]]
+            component = "test1"
+            components = { babble = { source = "fie.wasm", allowed_outbound_hosts = ["http://example.com"] } }
+        })
+        .expect("manifest should be valid");
+
+        let trigger = manifest.triggers.get("fake").unwrap()[0].clone();
+
+        assert_eq!(1, trigger.components.len());
+        let babble_comps = &trigger.components.get("babble").as_ref().unwrap().0;
+        assert_eq!(1, babble_comps.len());
+        let single = as_inline(&babble_comps[0]).expect("should have deserialised to inline");
+        assert_eq!(Some("fie.wasm"), as_local(&single.source));
+        assert_eq!(1, single.allowed_outbound_hosts.len());
+    }
+
+    #[test]
+    fn can_deserialise_one_or_many_many() {
+        let manifest = AppManifest::deserialize(toml! {
+            spin_manifest_version = 2
+            [application]
+            name = "test"
+            [[trigger.fake]]
+            component = "test1"
+            components = { babble = ["test2", { source = "fie.wasm", allowed_outbound_hosts = ["http://example.com"] }, "test3"] }
+        })
+        .expect("manifest should be valid");
+
+        let trigger = manifest.triggers.get("fake").unwrap()[0].clone();
+
+        assert_eq!(1, trigger.components.len());
+        let babble_comps = &trigger.components.get("babble").as_ref().unwrap().0;
+        assert_eq!(3, babble_comps.len());
+
+        assert_eq!(Some("test2"), as_reference(&babble_comps[0]));
+
+        let inline = as_inline(&babble_comps[1]).expect("should have deserialised to inline");
+        assert_eq!(Some("fie.wasm"), as_local(&inline.source));
+        assert_eq!(1, inline.allowed_outbound_hosts.len());
+
+        assert_eq!(Some("test3"), as_reference(&babble_comps[2]));
+    }
 }

crates/oci/src/client.rs

@@ -992,8 +992,8 @@ async fn precompose_using_trigger(
         )));
     }
 
-    let complicated = trigger_out.stdout;
-    Ok(complicated)
+    let composed = trigger_out.stdout;
+    Ok(composed)
 }
 
 /// Takes a relative path and turns it into a format that is safe

crates/trigger/src/loader.rs

@@ -70,22 +70,22 @@ impl ComponentLoader {
     pub(crate) async fn load_composed(
         &self,
         component: &AppComponent<'_>,
-        complicator: &impl spin_factors_executor::TriggerDependenciesComposer,
+        trigger_dependencies_composer: &impl spin_factors_executor::TriggerDependenciesComposer,
     ) -> anyhow::Result<Vec<u8>> {
         let loader = ComponentSourceLoaderFs;
 
         let trigger_deps = &component.locked.trigger_dependencies;
 
-        let complications = load_complications(&mut trigger_deps.iter(), &loader).await?;
+        let trigger_deps = load_trigger_dependencies(&mut trigger_deps.iter(), &loader).await?;
 
-        let complicate = async |c: Vec<u8>| {
-            complicator
-                .compose_trigger_dependencies(&complications, c)
+        let apply_trigger_deps = async |c: Vec<u8>| {
+            trigger_dependencies_composer
+                .compose_trigger_dependencies(&trigger_deps, c)
                 .await
                 .map_err(spin_compose::ComposeError::PrepareError)
         };
 
-        let composed = spin_compose::compose(&loader, component.locked, complicate)
+        let composed = spin_compose::compose(&loader, component.locked, apply_trigger_deps)
             .await
             .with_context(|| {
                 format!(
@@ -104,7 +104,7 @@ impl<T: RuntimeFactors, U> spin_factors_executor::ComponentLoader<T, U> for Comp
         &self,
         engine: &wasmtime::Engine,
         component: &AppComponent,
-        complicator: &impl spin_factors_executor::TriggerDependenciesComposer,
+        trigger_dependencies_composer: &impl spin_factors_executor::TriggerDependenciesComposer,
     ) -> anyhow::Result<Component> {
         let source = component
             .source()
@@ -122,16 +122,18 @@ impl<T: RuntimeFactors, U> spin_factors_executor::ComponentLoader<T, U> for Comp
             return Ok(component);
         }
 
-        let composed = self.load_composed(component, complicator).await?;
+        let composed = self
+            .load_composed(component, trigger_dependencies_composer)
+            .await?;
 
         let component = spin_core::Component::new(engine, composed)
             .with_context(|| format!("failed to compile component from {}", quoted_path(&path)))?;
         Ok(component)
     }
 }
 
-pub(crate) async fn load_complications(
-    extras: &mut impl ExactSizeIterator<
+pub(crate) async fn load_trigger_dependencies(
+    trigger_dependencies: &mut impl ExactSizeIterator<
         Item = (&String, &Vec<spin_app::locked::LockedComponentDependency>),
     >,
     loader: &spin_compose::ComponentSourceLoaderFs,
@@ -142,25 +144,25 @@ pub(crate) async fn load_complications(
     use spin_factors_executor::TriggerDependency;
     use std::collections::HashMap;
 
-    let mut complications = HashMap::with_capacity(extras.len());
+    let mut resolved_trigger_deps = HashMap::with_capacity(trigger_dependencies.len());
 
-    for (role, role_components) in extras {
-        let mut complications_for_role = Vec::with_capacity(role_components.len());
+    for (role, role_components) in trigger_dependencies {
+        let mut deps_for_role = Vec::with_capacity(role_components.len());
 
         for locked_dep in role_components {
-            let data = load_complication_data(loader, &locked_dep.source).await?;
-            complications_for_role.push(TriggerDependency {
+            let data = load_trigger_dep_data(loader, &locked_dep.source).await?;
+            deps_for_role.push(TriggerDependency {
                 data,
                 dependency: locked_dep.clone(),
             });
         }
-        complications.insert(role.clone(), complications_for_role);
+        resolved_trigger_deps.insert(role.clone(), deps_for_role);
     }
 
-    Ok(complications)
+    Ok(resolved_trigger_deps)
 }
 
-async fn load_complication_data(
+async fn load_trigger_dep_data(
     loader: &ComponentSourceLoaderFs,
     source: &spin_app::locked::LockedComponentSource,
 ) -> anyhow::Result<TriggerDependencyData> {