diff --git a/crates/templates/src/app_info.rs b/crates/templates/src/app_info.rs index 9a6df58b78..1fe8e8d985 100644 --- a/crates/templates/src/app_info.rs +++ b/crates/templates/src/app_info.rs @@ -2,29 +2,13 @@ // interest to the template system. spin_loader does too // much processing to fit our needs here. -use std::{ - collections::HashSet, - path::{Path, PathBuf}, - sync::LazyLock, -}; - -use anyhow::ensure; -use serde::Deserialize; -use spin_manifest::schema::v1; - -use crate::store::TemplateLayout; +use std::path::Path; pub(crate) struct AppInfo { manifest_format: u32, - trigger_type: Option, // None = v2 template does not contain any triggers yet } impl AppInfo { - pub fn from_layout(layout: &TemplateLayout) -> Option> { - Self::layout_manifest_path(layout) - .map(|manifest_path| Self::from_existent_template(&manifest_path)) - } - pub fn from_file(manifest_path: &Path) -> Option> { if manifest_path.exists() { Some(Self::from_existent_file(manifest_path)) @@ -33,15 +17,6 @@ impl AppInfo { } } - fn layout_manifest_path(layout: &TemplateLayout) -> Option { - let manifest_path = layout.content_dir().join("spin.toml"); - if manifest_path.exists() { - Some(manifest_path) - } else { - None - } - } - fn from_existent_file(manifest_path: &Path) -> anyhow::Result { let manifest_str = std::fs::read_to_string(manifest_path)?; Self::from_manifest_text(&manifest_str) @@ -53,104 +28,12 @@ impl AppInfo { spin_manifest::ManifestVersion::V1 => 1, spin_manifest::ManifestVersion::V2 => 2, }; - let trigger_type = match manifest_version { - spin_manifest::ManifestVersion::V1 => Some( - toml::from_str::(manifest_str)? - .trigger - .trigger_type, - ), - spin_manifest::ManifestVersion::V2 => { - let triggers = toml::from_str::(manifest_str)? - .trigger - .unwrap_or_default(); - let type_count = triggers.len(); - ensure!( - type_count <= 1, - "only 1 trigger type currently supported; got {type_count}" - ); - triggers.into_iter().next().map(|t| t.0) - } - }; - Ok(Self { - manifest_format, - trigger_type, - }) - } - - fn from_existent_template(manifest_path: &Path) -> anyhow::Result { - // This has to be cruder, because (with the v2 style of component) a template - // is no longer valid TOML, so `from_existent_file` fails at manifest - // version inference. - let read_to_string = std::fs::read_to_string(manifest_path)?; - let manifest_tpl_str = read_to_string; - - Self::from_template_text(&manifest_tpl_str) - } - - fn from_template_text(manifest_tpl_str: &str) -> anyhow::Result { - // TODO: investigate using a TOML parser or regex to be more accurate - let is_v1_tpl = manifest_tpl_str.contains("spin_manifest_version = \"1\""); - let is_v2_tpl = manifest_tpl_str.contains("spin_manifest_version = 2"); - if is_v1_tpl { - // V1 manifest templates are valid TOML - return Self::from_manifest_text(manifest_tpl_str); - } - if !is_v2_tpl { - // The system will default to being permissive in this case - anyhow::bail!("Unsure of template manifest version"); - } - - Self::from_v2_template_text(manifest_tpl_str) - } - - fn from_v2_template_text(manifest_tpl_str: &str) -> anyhow::Result { - let trigger_types: HashSet<_> = manifest_tpl_str - .lines() - .filter_map(infer_trigger_type_from_raw_line) - .collect(); - let type_count = trigger_types.len(); - ensure!( - type_count <= 1, - "only 1 trigger type currently supported; got {type_count}" - ); - let trigger_type = trigger_types.into_iter().next(); - - Ok(Self { - manifest_format: 2, - trigger_type, - }) + Ok(Self { manifest_format }) } pub fn manifest_format(&self) -> u32 { self.manifest_format } - - pub fn trigger_type(&self) -> Option<&str> { - self.trigger_type.as_deref() - } -} - -static EXTRACT_TRIGGER: LazyLock = LazyLock::new(|| { - regex::Regex::new(r"^\s*\[\[trigger\.(?[a-zA-Z0-9-]+)") - .expect("Invalid unknown filter regex") -}); - -fn infer_trigger_type_from_raw_line(line: &str) -> Option { - EXTRACT_TRIGGER - .captures(line) - .map(|c| c["trigger"].to_owned()) -} - -#[derive(Deserialize)] -struct ManifestV1TriggerProbe { - // `trigger = { type = "", ...}` - trigger: v1::AppTriggerV1, -} - -#[derive(Deserialize)] -struct ManifestV2TriggerProbe { - /// `[trigger.]` - empty will not have a trigger table in v2 - trigger: Option, } #[cfg(test)] @@ -158,79 +41,38 @@ mod test { use super::*; #[test] - fn can_extract_triggers() { - assert_eq!( - "http", - infer_trigger_type_from_raw_line("[[trigger.http]]").unwrap() - ); - assert_eq!( - "http", - infer_trigger_type_from_raw_line(" [[trigger.http]]").unwrap() - ); - assert_eq!( - "fie", - infer_trigger_type_from_raw_line(" [[trigger.fie]]").unwrap() - ); - assert_eq!( - "x-y", - infer_trigger_type_from_raw_line(" [[trigger.x-y]]").unwrap() - ); - - assert_eq!(None, infer_trigger_type_from_raw_line("# [[trigger.http]]")); - assert_eq!(None, infer_trigger_type_from_raw_line("trigger. But,")); - assert_eq!(None, infer_trigger_type_from_raw_line("[[trigger. snerk")); - } - - #[test] - fn can_read_app_info_from_template_v1() { - let tpl = r#"spin_manifest_version = "1" - name = "{{ thingy }}" + fn can_detect_v1_manifest_format() { + let manifest = r#"spin_manifest_version = "1" + name = "test" version = "1.2.3" - trigger = { type = "triggy", arg = "{{ another-thingy }}" } + trigger = { type = "http" } [[component]] - id = "{{ thingy | kebab_case }}" - source = "path/to/{{ thingy | snake_case }}.wasm" + id = "test" + source = "test.wasm" [component.trigger] - spork = "{{ utensil }}" + route = "/" "#; - let info = AppInfo::from_template_text(tpl).unwrap(); + let info = AppInfo::from_manifest_text(manifest).unwrap(); assert_eq!(1, info.manifest_format); - assert_eq!("triggy", info.trigger_type.unwrap()); } #[test] - fn can_read_app_info_from_template_v2() { - let tpl = r#"spin_manifest_version = 2 - name = "{{ thingy }}" + fn can_detect_v2_manifest_format() { + let manifest = r#"spin_manifest_version = 2 + name = "test" version = "1.2.3" - [application.trigger.triggy] - arg = "{{ another-thingy }}" - - [[trigger.triggy]] - spork = "{{ utensil }}" - component = "{{ thingy | kebab_case }}" - - [component.{{ thingy | kebab_case }}] - source = "path/to/{{ thingy | snake_case }}.wasm" - "#; - - let info = AppInfo::from_template_text(tpl).unwrap(); - assert_eq!(2, info.manifest_format); - assert_eq!("triggy", info.trigger_type.unwrap()); - } + [[trigger.http]] + route = "/" + component = "test" - #[test] - fn can_read_app_info_from_triggerless_template_v2() { - let tpl = r#"spin_manifest_version = 2 - name = "{{ thingy }}" - version = "1.2.3" + [component.test] + source = "test.wasm" "#; - let info = AppInfo::from_template_text(tpl).unwrap(); + let info = AppInfo::from_manifest_text(manifest).unwrap(); assert_eq!(2, info.manifest_format); - assert_eq!(None, info.trigger_type); } } diff --git a/crates/templates/src/manager.rs b/crates/templates/src/manager.rs index 39b0f5e7c8..1759d8d25d 100644 --- a/crates/templates/src/manager.rs +++ b/crates/templates/src/manager.rs @@ -1132,7 +1132,7 @@ mod tests { } #[tokio::test] - async fn cannot_add_component_that_does_not_match_manifest() { + async fn cannot_add_component_to_v1_manifest() { let manager = TempManager::new_with_this_repo_templates().await; let dest_temp_dir = tempdir().unwrap(); diff --git a/crates/templates/src/reader.rs b/crates/templates/src/reader.rs index 40b05a2a55..cdfe4b9012 100644 --- a/crates/templates/src/reader.rs +++ b/crates/templates/src/reader.rs @@ -17,7 +17,10 @@ pub(crate) enum RawTemplateManifest { pub(crate) struct RawTemplateManifestV1 { pub id: String, pub description: Option, - pub trigger_type: Option, + // Retained so existing templates that declare `trigger_type` continue + // to parse under `deny_unknown_fields`; the value is no longer used. + #[allow(dead_code)] + pub trigger_type: Option, pub tags: Option>, pub new_application: Option, pub add_component: Option, diff --git a/crates/templates/src/run.rs b/crates/templates/src/run.rs index 7ddc47371f..b736bfb019 100644 --- a/crates/templates/src/run.rs +++ b/crates/templates/src/run.rs @@ -97,7 +97,7 @@ impl Run { interaction: impl InteractionStrategy, ) -> anyhow::Result> { self.validate_version()?; - self.validate_trigger()?; + self.validate_add_component_manifest_version()?; // TODO: rationalise `path` and `dir` let to = self.generation_target_dir(); @@ -247,14 +247,14 @@ impl Run { } } - fn validate_trigger(&self) -> anyhow::Result<()> { + fn validate_add_component_manifest_version(&self) -> anyhow::Result<()> { match &self.options.variant { TemplateVariantInfo::NewApplication => Ok(()), TemplateVariantInfo::AddComponent { manifest_path } => { match crate::app_info::AppInfo::from_file(manifest_path) { - Some(Ok(app_info)) if app_info.manifest_format() == 1 => self - .template - .check_compatible_trigger(app_info.trigger_type()), + Some(Ok(app_info)) if app_info.manifest_format() == 1 => Err(anyhow!( + "Components cannot be added to Spin manifest version 1 applications.", + )), _ => Ok(()), // Fail forgiving - don't block the user if things are under construction } } diff --git a/crates/templates/src/template.rs b/crates/templates/src/template.rs index cd0c10a8c5..f91d17c1b2 100644 --- a/crates/templates/src/template.rs +++ b/crates/templates/src/template.rs @@ -25,7 +25,6 @@ pub struct Template { tags: HashSet, description: Option, installed_from: InstalledFrom, - trigger: TemplateTriggerCompatibility, variants: HashMap, parameters: Vec, extra_outputs: Vec, @@ -117,12 +116,6 @@ pub(crate) enum Condition { Always(bool), } -#[derive(Clone, Debug, Eq, PartialEq, Hash)] -pub(crate) enum TemplateTriggerCompatibility { - Any, - Only(String), -} - #[derive(Clone, Debug)] pub(crate) enum TemplateParameterDataType { String(StringConstraints), @@ -199,7 +192,6 @@ impl Template { tags: raw.tags.map(Self::normalize_tags).unwrap_or_default(), description: raw.description.clone(), installed_from, - trigger: Self::parse_trigger_type(raw.trigger_type, layout), variants: Self::parse_template_variants(raw.new_application, raw.add_component), parameters: Self::parse_parameters(&raw.parameters)?, extra_outputs: Self::parse_extra_outputs(&raw.outputs)?, @@ -333,26 +325,6 @@ impl Template { tags.into_iter().map(|tag| tag.to_lowercase()).collect() } - fn parse_trigger_type( - raw: Option, - layout: &TemplateLayout, - ) -> TemplateTriggerCompatibility { - match raw { - None => Self::infer_trigger_type(layout), - Some(t) => TemplateTriggerCompatibility::Only(t), - } - } - - fn infer_trigger_type(layout: &TemplateLayout) -> TemplateTriggerCompatibility { - match crate::app_info::AppInfo::from_layout(layout) { - Some(Ok(app_info)) => match app_info.trigger_type() { - None => TemplateTriggerCompatibility::Any, - Some(t) => TemplateTriggerCompatibility::Only(t.to_owned()), - }, - _ => TemplateTriggerCompatibility::Any, // Fail forgiving - } - } - fn parse_template_variants( new_application: Option, add_component: Option, @@ -457,26 +429,6 @@ impl Template { .collect() } - pub(crate) fn check_compatible_trigger(&self, app_trigger: Option<&str>) -> anyhow::Result<()> { - // The application we are merging into might not have a trigger yet, in which case - // we're good to go. - let Some(app_trigger) = app_trigger else { - return Ok(()); - }; - match &self.trigger { - TemplateTriggerCompatibility::Any => Ok(()), - TemplateTriggerCompatibility::Only(t) => { - if app_trigger == t { - Ok(()) - } else { - Err(anyhow!( - "Component trigger type '{t}' does not match application trigger type '{app_trigger}'" - )) - } - } - } - } - pub(crate) fn check_compatible_manifest_format( &self, manifest_format: u32, @@ -769,7 +721,6 @@ mod test { tags: HashSet::new(), description: None, installed_from: InstalledFrom::Unknown, - trigger: TemplateTriggerCompatibility::Any, variants, parameters: vec![], extra_outputs: vec![],