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: fermyon#763. Signed-off-by: Konstantin Shabanov <[email protected]>
etehtsea · Oct 3, 2022 · 09ca258 · 09ca258
1 parent 032971f
commit 09ca258
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 37 deletions.
diff --git a/crates/http/benches/baseline.rs b/crates/http/benches/baseline.rs
@@ -7,7 +7,7 @@ use futures::future::join_all;
 use http::uri::Scheme;
 use http::Request;
 use spin_http::HttpTrigger;
-use spin_testing::{assert_http_response_success, TestConfig};
+use spin_testing::{assert_http_response_success, HttpTestConfig};
 use tokio::runtime::Runtime;
 use tokio::task;
 
@@ -26,20 +26,20 @@ fn bench_startup(c: &mut Criterion) {
     let mut group = c.benchmark_group("startup");
     group.bench_function("spin-executor", |b| {
         b.to_async(&async_runtime).iter(|| async {
-            let trigger = TestConfig::default()
+            let trigger = HttpTestConfig::default()
                 .test_program("spin-http-benchmark.wasm")
                 .http_spin_trigger("/")
-                .build_http_trigger()
+                .build_trigger()
                 .await;
             run_concurrent_requests(Arc::new(trigger), 0, 1).await;
         });
     });
     group.bench_function("spin-wagi-executor", |b| {
         b.to_async(&async_runtime).iter(|| async {
-            let trigger = TestConfig::default()
+            let trigger = HttpTestConfig::default()
                 .test_program("wagi-benchmark.wasm")
                 .http_wagi_trigger("/", Default::default())
-                .build_http_trigger()
+                .build_trigger()
                 .await;
             run_concurrent_requests(Arc::new(trigger), 0, 1).await;
         });
@@ -50,12 +50,12 @@ fn bench_startup(c: &mut Criterion) {
 fn bench_spin_concurrency_minimal(c: &mut Criterion) {
     let async_runtime = Runtime::new().unwrap();
 
-    let spin_trigger = Arc::new(
+    let spin_trigger: Arc<HttpTrigger> = Arc::new(
         async_runtime.block_on(
-            TestConfig::default()
+            HttpTestConfig::default()
                 .test_program("spin-http-benchmark.wasm")
                 .http_spin_trigger("/")
-                .build_http_trigger(),
+                .build_trigger(),
         ),
     );
 
@@ -86,12 +86,12 @@ fn bench_spin_concurrency_minimal(c: &mut Criterion) {
 fn bench_wagi_concurrency_minimal(c: &mut Criterion) {
     let async_runtime = Runtime::new().unwrap();
 
-    let wagi_trigger = Arc::new(
+    let wagi_trigger: Arc<HttpTrigger> = Arc::new(
         async_runtime.block_on(
-            TestConfig::default()
+            HttpTestConfig::default()
                 .test_program("wagi-benchmark.wasm")
                 .http_wagi_trigger("/", Default::default())
-                .build_http_trigger(),
+                .build_trigger(),
         ),
     );
 

diff --git 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<RuntimeData>;
 
-/// 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<Self>,
@@ -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<Self>) -> Result<Self> {
         let base = engine
             .app()
-            .get_metadata(HTTP_BASE_METADATA_KEY)?
-            .unwrap_or("/")
-            .to_string();
+            .require_metadata::<TriggerMetadata>("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
@@ -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
@@ -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<PathBuf>,
     http_trigger_config: HttpTriggerConfig,
+}
+
+#[derive(Default)]
+pub struct RedisTestConfig {
+    module_path: Option<PathBuf>,
     redis_channel: String,
 }
 
-impl TestConfig {
+impl HttpTestConfig {
     pub fn module_path(&mut self, path: impl Into<PathBuf>) -> &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<String>) -> &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<Executor: TriggerExecutor>(&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<PathBuf>) -> &mut Self {
+        init_tracing();
+        self.module_path = Some(path.into());
+        self
+    }
+
+    pub fn test_program(&mut self, name: impl AsRef<Path>) -> &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<Executor: TriggerExecutor>(&self) -> Executor
+    pub async fn build_trigger<Executor: TriggerExecutor>(&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
@@ -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)?;
                     },