Skip to content

Commit d44d76d

Browse files
committed
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: spinframework#763. Signed-off-by: Konstantin Shabanov <[email protected]>
1 parent 6741cb5 commit d44d76d

File tree

5 files changed

+96
-37
lines changed

5 files changed

+96
-37
lines changed

crates/http/benches/baseline.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use futures::future::join_all;
77
use http::uri::Scheme;
88
use http::Request;
99
use spin_http::HttpTrigger;
10-
use spin_testing::{assert_http_response_success, TestConfig};
10+
use spin_testing::{assert_http_response_success, HttpTestConfig};
1111
use tokio::runtime::Runtime;
1212
use tokio::task;
1313

@@ -26,20 +26,20 @@ fn bench_startup(c: &mut Criterion) {
2626
let mut group = c.benchmark_group("startup");
2727
group.bench_function("spin-executor", |b| {
2828
b.to_async(&async_runtime).iter(|| async {
29-
let trigger = TestConfig::default()
29+
let trigger = HttpTestConfig::default()
3030
.test_program("spin-http-benchmark.wasm")
3131
.http_spin_trigger("/")
32-
.build_http_trigger()
32+
.build_trigger()
3333
.await;
3434
run_concurrent_requests(Arc::new(trigger), 0, 1).await;
3535
});
3636
});
3737
group.bench_function("spin-wagi-executor", |b| {
3838
b.to_async(&async_runtime).iter(|| async {
39-
let trigger = TestConfig::default()
39+
let trigger = HttpTestConfig::default()
4040
.test_program("wagi-benchmark.wasm")
4141
.http_wagi_trigger("/", Default::default())
42-
.build_http_trigger()
42+
.build_trigger()
4343
.await;
4444
run_concurrent_requests(Arc::new(trigger), 0, 1).await;
4545
});
@@ -50,12 +50,12 @@ fn bench_startup(c: &mut Criterion) {
5050
fn bench_spin_concurrency_minimal(c: &mut Criterion) {
5151
let async_runtime = Runtime::new().unwrap();
5252

53-
let spin_trigger = Arc::new(
53+
let spin_trigger: Arc<HttpTrigger> = Arc::new(
5454
async_runtime.block_on(
55-
TestConfig::default()
55+
HttpTestConfig::default()
5656
.test_program("spin-http-benchmark.wasm")
5757
.http_spin_trigger("/")
58-
.build_http_trigger(),
58+
.build_trigger(),
5959
),
6060
);
6161

@@ -86,12 +86,12 @@ fn bench_spin_concurrency_minimal(c: &mut Criterion) {
8686
fn bench_wagi_concurrency_minimal(c: &mut Criterion) {
8787
let async_runtime = Runtime::new().unwrap();
8888

89-
let wagi_trigger = Arc::new(
89+
let wagi_trigger: Arc<HttpTrigger> = Arc::new(
9090
async_runtime.block_on(
91-
TestConfig::default()
91+
HttpTestConfig::default()
9292
.test_program("wagi-benchmark.wasm")
9393
.http_wagi_trigger("/", Default::default())
94-
.build_http_trigger(),
94+
.build_trigger(),
9595
),
9696
);
9797

crates/http/src/lib.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,6 @@ wit_bindgen_wasmtime::import!({paths: ["../../wit/ephemeral/spin-http.wit"], asy
3838
pub(crate) type RuntimeData = spin_http::SpinHttpData;
3939
pub(crate) type Store = spin_core::Store<RuntimeData>;
4040

41-
/// App metadata key for storing HTTP trigger "base" value
42-
pub const HTTP_BASE_METADATA_KEY: &str = "http_base";
43-
4441
/// The Spin HTTP trigger.
4542
pub struct HttpTrigger {
4643
engine: TriggerAppEngine<Self>,
@@ -107,6 +104,13 @@ pub enum HttpExecutorType {
107104
Wagi(WagiTriggerConfig),
108105
}
109106

107+
#[derive(Clone, Debug, Default, Deserialize, Serialize)]
108+
#[serde(deny_unknown_fields)]
109+
struct TriggerMetadata {
110+
r#type: String,
111+
base: String,
112+
}
113+
110114
#[async_trait]
111115
impl TriggerExecutor for HttpTrigger {
112116
const TRIGGER_TYPE: &'static str = "http";
@@ -117,9 +121,8 @@ impl TriggerExecutor for HttpTrigger {
117121
fn new(engine: TriggerAppEngine<Self>) -> Result<Self> {
118122
let base = engine
119123
.app()
120-
.get_metadata(HTTP_BASE_METADATA_KEY)?
121-
.unwrap_or("/")
122-
.to_string();
124+
.require_metadata::<TriggerMetadata>("trigger")?
125+
.base;
123126

124127
let component_routes = engine
125128
.trigger_configs()
@@ -533,10 +536,10 @@ mod tests {
533536

534537
#[tokio::test]
535538
async fn test_spin_http() -> Result<()> {
536-
let trigger = spin_testing::TestConfig::default()
539+
let trigger: HttpTrigger = spin_testing::HttpTestConfig::default()
537540
.test_program("rust-http-test.wasm")
538541
.http_spin_trigger("/test")
539-
.build_http_trigger()
542+
.build_trigger()
540543
.await;
541544

542545
let body = Body::from("Fermyon".as_bytes().to_vec());
@@ -558,10 +561,10 @@ mod tests {
558561

559562
#[tokio::test]
560563
async fn test_wagi_http() -> Result<()> {
561-
let trigger = spin_testing::TestConfig::default()
564+
let trigger: HttpTrigger = spin_testing::HttpTestConfig::default()
562565
.test_program("wagi-test.wasm")
563566
.http_wagi_trigger("/test", Default::default())
564-
.build_http_trigger()
567+
.build_trigger()
565568
.await;
566569

567570
let body = Body::from("Fermyon".as_bytes().to_vec());

crates/redis/src/tests.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use super::*;
22
use anyhow::Result;
33
use redis::{Msg, Value};
4-
use spin_testing::{tokio, TestConfig};
4+
use spin_testing::{tokio, RedisTestConfig};
55

66
fn create_trigger_event(channel: &str, payload: &str) -> redis::Msg {
77
Msg::from_value(&redis::Value::Bulk(vec![
@@ -14,10 +14,9 @@ fn create_trigger_event(channel: &str, payload: &str) -> redis::Msg {
1414

1515
#[tokio::test]
1616
async fn test_pubsub() -> Result<()> {
17-
let trigger: RedisTrigger = TestConfig::default()
17+
let trigger: RedisTrigger = RedisTestConfig::default()
1818
.test_program("redis-rust.wasm")
19-
.redis_trigger("messages")
20-
.build_trigger()
19+
.build_trigger("messages")
2120
.await;
2221

2322
let msg = create_trigger_event("messages", "hello");

crates/testing/src/lib.rs

Lines changed: 68 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use spin_app::{
1616
AppComponent, Loader,
1717
};
1818
use spin_core::{Module, StoreBuilder};
19-
use spin_http::{HttpExecutorType, HttpTrigger, HttpTriggerConfig, WagiTriggerConfig};
19+
use spin_http::{HttpExecutorType, HttpTriggerConfig, WagiTriggerConfig};
2020
use spin_trigger::{TriggerExecutor, TriggerExecutorBuilder};
2121

2222
pub use tokio;
@@ -37,13 +37,18 @@ macro_rules! from_json {
3737
}
3838

3939
#[derive(Default)]
40-
pub struct TestConfig {
40+
pub struct HttpTestConfig {
4141
module_path: Option<PathBuf>,
4242
http_trigger_config: HttpTriggerConfig,
43+
}
44+
45+
#[derive(Default)]
46+
pub struct RedisTestConfig {
47+
module_path: Option<PathBuf>,
4348
redis_channel: String,
4449
}
4550

46-
impl TestConfig {
51+
impl HttpTestConfig {
4752
pub fn module_path(&mut self, path: impl Into<PathBuf>) -> &mut Self {
4853
init_tracing();
4954
self.module_path = Some(path.into());
@@ -80,9 +85,22 @@ impl TestConfig {
8085
self
8186
}
8287

83-
pub fn redis_trigger(&mut self, channel: impl Into<String>) -> &mut Self {
84-
self.redis_channel = channel.into();
85-
self
88+
pub fn build_loader(&self) -> impl Loader {
89+
init_tracing();
90+
TestLoader {
91+
app: self.build_locked_app(),
92+
module_path: self.module_path.clone().expect("module path to be set"),
93+
}
94+
}
95+
96+
pub async fn build_trigger<Executor: TriggerExecutor>(&self) -> Executor
97+
where
98+
Executor::TriggerConfig: DeserializeOwned,
99+
{
100+
TriggerExecutorBuilder::new(self.build_loader())
101+
.build(TEST_APP_URI.to_string())
102+
.await
103+
.unwrap()
86104
}
87105

88106
pub fn build_locked_app(&self) -> LockedApp {
@@ -100,7 +118,7 @@ impl TestConfig {
100118
"trigger_config": self.http_trigger_config,
101119
},
102120
]);
103-
let metadata = from_json!({"name": "test-app", "trigger": {"address": "test-redis-host", "type": "redis"}});
121+
let metadata = from_json!({"name": "test-app", "trigger": {"type": "http", "base": "/"}});
104122
let variables = Default::default();
105123
LockedApp {
106124
spin_lock_version: spin_app::locked::FixedVersion,
@@ -110,6 +128,22 @@ impl TestConfig {
110128
variables,
111129
}
112130
}
131+
}
132+
133+
impl RedisTestConfig {
134+
pub fn module_path(&mut self, path: impl Into<PathBuf>) -> &mut Self {
135+
init_tracing();
136+
self.module_path = Some(path.into());
137+
self
138+
}
139+
140+
pub fn test_program(&mut self, name: impl AsRef<Path>) -> &mut Self {
141+
self.module_path(
142+
PathBuf::from(env!("CARGO_MANIFEST_DIR"))
143+
.join("../../target/test-programs")
144+
.join(name),
145+
)
146+
}
113147

114148
pub fn build_loader(&self) -> impl Loader {
115149
init_tracing();
@@ -119,18 +153,42 @@ impl TestConfig {
119153
}
120154
}
121155

122-
pub async fn build_trigger<Executor: TriggerExecutor>(&self) -> Executor
156+
pub async fn build_trigger<Executor: TriggerExecutor>(&mut self, channel: &str) -> Executor
123157
where
124158
Executor::TriggerConfig: DeserializeOwned,
125159
{
160+
self.redis_channel = channel.into();
161+
126162
TriggerExecutorBuilder::new(self.build_loader())
127163
.build(TEST_APP_URI.to_string())
128164
.await
129165
.unwrap()
130166
}
131167

132-
pub async fn build_http_trigger(&self) -> HttpTrigger {
133-
self.build_trigger().await
168+
pub fn build_locked_app(&self) -> LockedApp {
169+
let components = from_json!([{
170+
"id": "test-component",
171+
"source": {
172+
"content_type": "application/wasm",
173+
"digest": "test-source",
174+
},
175+
}]);
176+
let triggers = from_json!([
177+
{
178+
"id": "trigger--test-app",
179+
"trigger_type": "redis",
180+
"trigger_config": {"channel": self.redis_channel, "component": "test-component"},
181+
},
182+
]);
183+
let metadata = from_json!({"name": "test-app", "trigger": {"address": "test-redis-host", "type": "redis"}});
184+
let variables = Default::default();
185+
LockedApp {
186+
spin_lock_version: spin_app::locked::FixedVersion,
187+
components,
188+
triggers,
189+
metadata,
190+
variables,
191+
}
134192
}
135193
}
136194

crates/trigger/src/locked.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,8 @@ impl LockedAppBuilder {
9292

9393
let trigger_type;
9494
match (app_trigger, config) {
95-
(ApplicationTrigger::Http(HttpTriggerConfiguration{base}), TriggerConfig::Http(HttpConfig{ route, executor })) => {
95+
(ApplicationTrigger::Http(HttpTriggerConfiguration{base: _}), TriggerConfig::Http(HttpConfig{ route, executor })) => {
9696
trigger_type = "http";
97-
let route = base.trim_end_matches('/').to_string() + &route;
9897
builder.string("route", route);
9998
builder.serializable("executor", executor)?;
10099
},

0 commit comments

Comments
 (0)