From 30f0893462a0df9f085fed90b401c8aff053a8fa Mon Sep 17 00:00:00 2001 From: Konstantin Shabanov Date: Sun, 2 Oct 2022 20:34:30 +0600 Subject: [PATCH] chore(spin-http): Fix base mapping Set base path in spin-http rather than joining it as part of the route. Split test configs in spin-testing because the trigger's configuration is different for http and redis. Refs: #763. Signed-off-by: Konstantin Shabanov --- crates/http/src/lib.rs | 23 ++++++----- crates/redis/src/tests.rs | 7 ++-- crates/testing/src/lib.rs | 78 +++++++++++++++++++++++++++++++----- crates/trigger/src/locked.rs | 3 +- 4 files changed, 85 insertions(+), 26 deletions(-) diff --git a/crates/http/src/lib.rs b/crates/http/src/lib.rs index dd1111ea6a..8200394fe4 100644 --- a/crates/http/src/lib.rs +++ b/crates/http/src/lib.rs @@ -38,9 +38,6 @@ wit_bindgen_wasmtime::import!({paths: ["../../wit/ephemeral/spin-http.wit"], asy pub(crate) type RuntimeData = spin_http::SpinHttpData; pub(crate) type Store = spin_core::Store; -/// App metadata key for storing HTTP trigger "base" value -pub const HTTP_BASE_METADATA_KEY: &str = "http_base"; - /// The Spin HTTP trigger. pub struct HttpTrigger { engine: TriggerAppEngine, @@ -107,6 +104,13 @@ pub enum HttpExecutorType { Wagi(WagiTriggerConfig), } +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +struct TriggerMetadata { + r#type: String, + base: String, +} + #[async_trait] impl TriggerExecutor for HttpTrigger { const TRIGGER_TYPE: &'static str = "http"; @@ -117,9 +121,8 @@ impl TriggerExecutor for HttpTrigger { fn new(engine: TriggerAppEngine) -> Result { let base = engine .app() - .get_metadata(HTTP_BASE_METADATA_KEY)? - .unwrap_or("/") - .to_string(); + .require_metadata::("trigger")? + .base; let component_routes = engine .trigger_configs() @@ -533,10 +536,10 @@ mod tests { #[tokio::test] async fn test_spin_http() -> Result<()> { - let trigger = spin_testing::TestConfig::default() + let trigger: HttpTrigger = spin_testing::HttpTestConfig::default() .test_program("rust-http-test.wasm") .http_spin_trigger("/test") - .build_http_trigger() + .build_trigger() .await; let body = Body::from("Fermyon".as_bytes().to_vec()); @@ -558,10 +561,10 @@ mod tests { #[tokio::test] async fn test_wagi_http() -> Result<()> { - let trigger = spin_testing::TestConfig::default() + let trigger: HttpTrigger = spin_testing::HttpTestConfig::default() .test_program("wagi-test.wasm") .http_wagi_trigger("/test", Default::default()) - .build_http_trigger() + .build_trigger() .await; let body = Body::from("Fermyon".as_bytes().to_vec()); diff --git a/crates/redis/src/tests.rs b/crates/redis/src/tests.rs index 951ae655d3..29620f7cf4 100644 --- a/crates/redis/src/tests.rs +++ b/crates/redis/src/tests.rs @@ -1,7 +1,7 @@ use super::*; use anyhow::Result; use redis::{Msg, Value}; -use spin_testing::{tokio, TestConfig}; +use spin_testing::{tokio, RedisTestConfig}; fn create_trigger_event(channel: &str, payload: &str) -> redis::Msg { Msg::from_value(&redis::Value::Bulk(vec![ @@ -14,10 +14,9 @@ fn create_trigger_event(channel: &str, payload: &str) -> redis::Msg { #[tokio::test] async fn test_pubsub() -> Result<()> { - let trigger: RedisTrigger = TestConfig::default() + let trigger: RedisTrigger = RedisTestConfig::default() .test_program("redis-rust.wasm") - .redis_trigger("messages") - .build_trigger() + .build_trigger("messages") .await; let msg = create_trigger_event("messages", "hello"); diff --git a/crates/testing/src/lib.rs b/crates/testing/src/lib.rs index b55d06c5f4..91fd951f18 100644 --- a/crates/testing/src/lib.rs +++ b/crates/testing/src/lib.rs @@ -16,7 +16,7 @@ use spin_app::{ AppComponent, Loader, }; use spin_core::{Module, StoreBuilder}; -use spin_http::{HttpExecutorType, HttpTrigger, HttpTriggerConfig, WagiTriggerConfig}; +use spin_http::{HttpExecutorType, HttpTriggerConfig, WagiTriggerConfig}; use spin_trigger::{TriggerExecutor, TriggerExecutorBuilder}; pub use tokio; @@ -37,13 +37,18 @@ macro_rules! from_json { } #[derive(Default)] -pub struct TestConfig { +pub struct HttpTestConfig { module_path: Option, http_trigger_config: HttpTriggerConfig, +} + +#[derive(Default)] +pub struct RedisTestConfig { + module_path: Option, redis_channel: String, } -impl TestConfig { +impl HttpTestConfig { pub fn module_path(&mut self, path: impl Into) -> &mut Self { init_tracing(); self.module_path = Some(path.into()); @@ -80,9 +85,22 @@ impl TestConfig { self } - pub fn redis_trigger(&mut self, channel: impl Into) -> &mut Self { - self.redis_channel = channel.into(); - self + pub fn build_loader(&self) -> impl Loader { + init_tracing(); + TestLoader { + app: self.build_locked_app(), + module_path: self.module_path.clone().expect("module path to be set"), + } + } + + pub async fn build_trigger(&self) -> Executor + where + Executor::TriggerConfig: DeserializeOwned, + { + TriggerExecutorBuilder::new(self.build_loader()) + .build(TEST_APP_URI.to_string()) + .await + .unwrap() } pub fn build_locked_app(&self) -> LockedApp { @@ -100,7 +118,7 @@ impl TestConfig { "trigger_config": self.http_trigger_config, }, ]); - let metadata = from_json!({"name": "test-app", "trigger": {"address": "test-redis-host", "type": "redis"}}); + let metadata = from_json!({"name": "test-app", "trigger": {"type": "http", "base": "/"}}); let variables = Default::default(); LockedApp { spin_lock_version: spin_app::locked::FixedVersion, @@ -110,6 +128,22 @@ impl TestConfig { variables, } } +} + +impl RedisTestConfig { + pub fn module_path(&mut self, path: impl Into) -> &mut Self { + init_tracing(); + self.module_path = Some(path.into()); + self + } + + pub fn test_program(&mut self, name: impl AsRef) -> &mut Self { + self.module_path( + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../../target/test-programs") + .join(name), + ) + } pub fn build_loader(&self) -> impl Loader { init_tracing(); @@ -119,18 +153,42 @@ impl TestConfig { } } - pub async fn build_trigger(&self) -> Executor + pub async fn build_trigger(&mut self, channel: &str) -> Executor where Executor::TriggerConfig: DeserializeOwned, { + self.redis_channel = channel.into(); + TriggerExecutorBuilder::new(self.build_loader()) .build(TEST_APP_URI.to_string()) .await .unwrap() } - pub async fn build_http_trigger(&self) -> HttpTrigger { - self.build_trigger().await + pub fn build_locked_app(&self) -> LockedApp { + let components = from_json!([{ + "id": "test-component", + "source": { + "content_type": "application/wasm", + "digest": "test-source", + }, + }]); + let triggers = from_json!([ + { + "id": "trigger--test-app", + "trigger_type": "redis", + "trigger_config": {"channel": self.redis_channel, "component": "test-component"}, + }, + ]); + let metadata = from_json!({"name": "test-app", "trigger": {"address": "test-redis-host", "type": "redis"}}); + let variables = Default::default(); + LockedApp { + spin_lock_version: spin_app::locked::FixedVersion, + components, + triggers, + metadata, + variables, + } } } diff --git a/crates/trigger/src/locked.rs b/crates/trigger/src/locked.rs index d57b709da0..dc875aaccf 100644 --- a/crates/trigger/src/locked.rs +++ b/crates/trigger/src/locked.rs @@ -92,9 +92,8 @@ impl LockedAppBuilder { let trigger_type; match (app_trigger, config) { - (ApplicationTrigger::Http(HttpTriggerConfiguration{base}), TriggerConfig::Http(HttpConfig{ route, executor })) => { + (ApplicationTrigger::Http(HttpTriggerConfiguration{base: _}), TriggerConfig::Http(HttpConfig{ route, executor })) => { trigger_type = "http"; - let route = base.trim_end_matches('/').to_string() + &route; builder.string("route", route); builder.serializable("executor", executor)?; },