Revert RequestHandler type refactoring

Signed-off-by: Ryan Levick <[email protected]>

Author: rylev

crates/trigger-http2/src/outbound_http.rs

@@ -11,17 +11,17 @@ use spin_http::routes::RouteMatch;
 use spin_outbound_networking::parse_service_chaining_target;
 use wasmtime_wasi_http::types::IncomingResponse;
 
-use crate::server::RequestHandler;
+use crate::HttpServer;
 
 /// An outbound HTTP interceptor that handles service chaining requests.
 pub struct OutboundHttpInterceptor {
-    handler: Arc<RequestHandler>,
+    server: Arc<HttpServer>,
     origin: SelfRequestOrigin,
 }
 
 impl OutboundHttpInterceptor {
-    pub fn new(handler: Arc<RequestHandler>, origin: SelfRequestOrigin) -> Self {
-        Self { handler, origin }
+    pub fn new(server: Arc<HttpServer>, origin: SelfRequestOrigin) -> Self {
+        Self { server, origin }
     }
 }
 
@@ -41,10 +41,10 @@ impl spin_factor_outbound_http::OutboundHttpInterceptor for OutboundHttpIntercep
             let route_match = RouteMatch::synthetic(&component_id, uri.path());
             let req = std::mem::take(request);
             let between_bytes_timeout = config.between_bytes_timeout;
-            let server = self.handler.clone();
+            let server = self.server.clone();
             let resp_fut = async move {
                 match server
-                    .handle_trigger_route(req, &route_match, Scheme::HTTP, CHAINED_CLIENT_ADDR)
+                    .handle_trigger_route(req, route_match, Scheme::HTTP, CHAINED_CLIENT_ADDR)
                     .await
                 {
                     Ok(resp) => Ok(Ok(IncomingResponse {

crates/trigger-http2/src/server.rs

@@ -38,9 +38,18 @@ use crate::{
 
 /// An HTTP server which runs Spin apps.
 pub struct HttpServer {
+    /// The address the server is listening on.
+    listen_addr: SocketAddr,
+    /// The TLS configuration for the server.
     tls_config: Option<TlsConfig>,
+    /// Request router.
     router: Router,
-    handler: Arc<RequestHandler>,
+    /// The app being triggered.
+    trigger_app: TriggerApp,
+    // Component ID -> component trigger config
+    component_trigger_configs: HashMap<String, HttpTriggerConfig>,
+    // Component ID -> handler type
+    component_handler_types: HashMap<String, HandlerType>,
 }
 
 impl HttpServer {
@@ -52,27 +61,32 @@ impl HttpServer {
         router: Router,
         component_trigger_configs: HashMap<String, HttpTriggerConfig>,
     ) -> anyhow::Result<Self> {
+        let component_handler_types = component_trigger_configs
+            .keys()
+            .map(|component_id| {
+                let component = trigger_app.get_component(component_id)?;
+                let handler_type = HandlerType::from_component(trigger_app.engine(), component)?;
+                Ok((component_id.clone(), handler_type))
+            })
+            .collect::<anyhow::Result<_>>()?;
         Ok(Self {
+            listen_addr,
             tls_config,
             router,
-            handler: Arc::new(RequestHandler::new(
-                listen_addr,
-                trigger_app,
-                component_trigger_configs,
-            )?),
+            trigger_app,
+            component_trigger_configs,
+            component_handler_types,
         })
     }
 
     /// Serve incoming requests over the provided [`TcpListener`].
     pub async fn serve(self: Arc<Self>) -> anyhow::Result<()> {
-        let listener = TcpListener::bind(self.handler.listen_addr)
-            .await
-            .with_context(|| {
-                format!(
-                    "Unable to listen on {listen_addr}",
-                    listen_addr = self.handler.listen_addr
-                )
-            })?;
+        let listener = TcpListener::bind(self.listen_addr).await.with_context(|| {
+            format!(
+                "Unable to listen on {listen_addr}",
+                listen_addr = self.listen_addr
+            )
+        })?;
         if let Some(tls_config) = self.tls_config.clone() {
             self.serve_https(listener, tls_config).await?;
         } else {
@@ -112,7 +126,7 @@ impl HttpServer {
     ///
     /// This method handles well known paths and routes requests to the handler when the router
     /// matches the requests path.
-    async fn handle(
+    pub async fn handle(
         self: &Arc<Self>,
         mut req: Request<Body>,
         server_scheme: Scheme,
@@ -150,15 +164,70 @@ impl HttpServer {
     /// Handles a successful route match.
     pub async fn handle_trigger_route(
         self: &Arc<Self>,
-        req: Request<Body>,
+        mut req: Request<Body>,
         route_match: RouteMatch,
         server_scheme: Scheme,
         client_addr: SocketAddr,
     ) -> anyhow::Result<Response<Body>> {
-        let res = self
-            .handler
-            .handle_trigger_route(req, &route_match, server_scheme, client_addr)
-            .await;
+        set_req_uri(&mut req, server_scheme.clone())?;
+        let app_id = self
+            .trigger_app
+            .app()
+            .get_metadata(APP_NAME_KEY)?
+            .unwrap_or_else(|| "<unnamed>".into());
+
+        let component_id = route_match.component_id();
+
+        spin_telemetry::metrics::monotonic_counter!(
+            spin.request_count = 1,
+            trigger_type = "http",
+            app_id = app_id,
+            component_id = component_id
+        );
+
+        let mut instance_builder = self.trigger_app.prepare(component_id)?;
+
+        // Set up outbound HTTP request origin and service chaining
+        let origin = SelfRequestOrigin::create(server_scheme, &self.listen_addr)?;
+        instance_builder
+            .factor_builders()
+            .outbound_http()
+            .set_request_interceptor(OutboundHttpInterceptor::new(self.clone(), origin))?;
+
+        // Prepare HTTP executor
+        let trigger_config = self.component_trigger_configs.get(component_id).unwrap();
+        let handler_type = self.component_handler_types.get(component_id).unwrap();
+        let executor = trigger_config
+            .executor
+            .as_ref()
+            .unwrap_or(&HttpExecutorType::Http);
+
+        let res = match executor {
+            HttpExecutorType::Http => match handler_type {
+                HandlerType::Spin => {
+                    SpinHttpExecutor
+                        .execute(instance_builder, &route_match, req, client_addr)
+                        .await
+                }
+                HandlerType::Wasi0_2
+                | HandlerType::Wasi2023_11_10
+                | HandlerType::Wasi2023_10_18 => {
+                    WasiHttpExecutor {
+                        handler_type: *handler_type,
+                    }
+                    .execute(instance_builder, &route_match, req, client_addr)
+                    .await
+                }
+            },
+            HttpExecutorType::Wagi(wagi_config) => {
+                let executor = WagiHttpExecutor {
+                    wagi_config: wagi_config.clone(),
+                };
+                executor
+                    .execute(instance_builder, &route_match, req, client_addr)
+                    .await
+            }
+        };
         match res {
             Ok(res) => Ok(MatchedRoute::with_response_extension(
                 res,
@@ -174,7 +243,7 @@ impl HttpServer {
 
     /// Returns spin status information.
     fn app_info(&self, route: String) -> anyhow::Result<Response<Body>> {
-        let info = AppInfo::new(self.handler.trigger_app.app());
+        let info = AppInfo::new(self.trigger_app.app());
         let body = serde_json::to_vec_pretty(&info)?;
         Ok(MatchedRoute::with_response_extension(
             Response::builder()
@@ -278,7 +347,7 @@ impl HttpServer {
         println!("Available Routes:");
         for (route, component_id) in self.router.routes() {
             println!("  {}: {}{}", component_id, base_url, route);
-            if let Some(component) = self.handler.trigger_app.app().get_component(component_id) {
+            if let Some(component) = self.trigger_app.app().get_component(component_id) {
                 if let Some(description) = component.get_metadata(APP_DESCRIPTION_KEY)? {
                     println!("    {}", description);
                 }
@@ -288,111 +357,6 @@ impl HttpServer {
     }
 }
 
-/// Handles a routed HTTP trigger request.
-pub struct RequestHandler {
-    /// The address the server is listening on.
-    pub(crate) listen_addr: SocketAddr,
-    /// The app being triggered.
-    trigger_app: TriggerApp,
-    // Component ID -> component trigger config
-    component_trigger_configs: HashMap<String, HttpTriggerConfig>,
-    // Component ID -> handler type
-    component_handler_types: HashMap<String, HandlerType>,
-}
-
-impl RequestHandler {
-    /// Create a new [`RequestHandler`]
-    pub fn new(
-        listen_addr: SocketAddr,
-        trigger_app: TriggerApp,
-        component_trigger_configs: HashMap<String, HttpTriggerConfig>,
-    ) -> anyhow::Result<Self> {
-        let component_handler_types = component_trigger_configs
-            .keys()
-            .map(|component_id| {
-                let component = trigger_app.get_component(component_id)?;
-                let handler_type = HandlerType::from_component(trigger_app.engine(), component)?;
-                Ok((component_id.clone(), handler_type))
-            })
-            .collect::<anyhow::Result<_>>()?;
-        Ok(Self {
-            listen_addr,
-            trigger_app,
-            component_trigger_configs,
-            component_handler_types,
-        })
-    }
-
-    /// Handle a routed request.
-    pub async fn handle_trigger_route(
-        self: &Arc<Self>,
-        mut req: Request<Body>,
-        route_match: &RouteMatch,
-        server_scheme: Scheme,
-        client_addr: SocketAddr,
-    ) -> anyhow::Result<Response<Body>> {
-        set_req_uri(&mut req, server_scheme.clone())?;
-        let app_id = self
-            .trigger_app
-            .app()
-            .get_metadata(APP_NAME_KEY)?
-            .unwrap_or_else(|| "<unnamed>".into());
-
-        let component_id = route_match.component_id();
-
-        spin_telemetry::metrics::monotonic_counter!(
-            spin.request_count = 1,
-            trigger_type = "http",
-            app_id = app_id,
-            component_id = component_id
-        );
-
-        let mut instance_builder = self.trigger_app.prepare(component_id)?;
-
-        // Set up outbound HTTP request origin and service chaining
-        let origin = SelfRequestOrigin::create(server_scheme, &self.listen_addr)?;
-        instance_builder
-            .factor_builders()
-            .outbound_http()
-            .set_request_interceptor(OutboundHttpInterceptor::new(self.clone(), origin))?;
-
-        // Prepare HTTP executor
-        let trigger_config = self.component_trigger_configs.get(component_id).unwrap();
-        let handler_type = self.component_handler_types.get(component_id).unwrap();
-        let executor = trigger_config
-            .executor
-            .as_ref()
-            .unwrap_or(&HttpExecutorType::Http);
-
-        match executor {
-            HttpExecutorType::Http => match handler_type {
-                HandlerType::Spin => {
-                    SpinHttpExecutor
-                        .execute(instance_builder, route_match, req, client_addr)
-                        .await
-                }
-                HandlerType::Wasi0_2
-                | HandlerType::Wasi2023_11_10
-                | HandlerType::Wasi2023_10_18 => {
-                    WasiHttpExecutor {
-                        handler_type: *handler_type,
-                    }
-                    .execute(instance_builder, route_match, req, client_addr)
-                    .await
-                }
-            },
-            HttpExecutorType::Wagi(wagi_config) => {
-                let executor = WagiHttpExecutor {
-                    wagi_config: wagi_config.clone(),
-                };
-                executor
-                    .execute(instance_builder, route_match, req, client_addr)
-                    .await
-            }
-        }
-    }
-}
-
 /// The incoming request's scheme and authority
 ///
 /// The incoming request's URI is relative to the server, so we need to set the scheme and authority.

tests/conformance-tests/src/lib.rs

@@ -1,5 +1,4 @@
 use anyhow::Context as _;
-use test_environment::http::Request;
 use testing_framework::runtimes::spin_cli::{SpinCli, SpinConfig};
 
 /// Run a single conformance test against the supplied spin binary.
@@ -57,11 +56,9 @@ pub fn run_test(
         let conformance_tests::config::Invocation::Http(mut invocation) = invocation;
         invocation.request.substitute_from_env(&mut env)?;
         let spin = env.runtime_mut();
-        let actual = invocation.request.send(|request| {
-            let request =
-                Request::full(request.method, request.path, request.headers, request.body);
-            spin.make_http_request(request)
-        })?;
+        let actual = invocation
+            .request
+            .send(|request| spin.make_http_request(request))?;
 
         conformance_tests::assertions::assert_response(&invocation.response, &actual)
             .with_context(|| {

tests/testing-framework/Cargo.toml

@@ -14,12 +14,12 @@ regex = "1.10.2"
 reqwest = { workspace = true }
 temp-dir = "0.1.11"
 test-environment = { workspace = true }
-spin-factors-executor = { path = "../../crates/factors-executor" }
 spin-app = { path = "../../crates/app" }
-spin-trigger-http2 = { path = "../../crates/trigger-http2" }
+spin-factors-executor = { path = "../../crates/factors-executor" }
 spin-http = { path = "../../crates/http" }
-spin-trigger2 = { path = "../../crates/trigger2" }
 spin-loader = { path = "../../crates/loader" }
+spin-trigger2 = { path = "../../crates/trigger2" }
+spin-trigger-http2 = { path = "../../crates/trigger-http2" }
 toml = "0.8.6"
 tokio = "1.23"
 wasmtime-wasi-http = { workspace = true }

tests/testing-framework/src/runtimes/in_process_spin.rs

@@ -3,7 +3,6 @@
 use std::{path::PathBuf, sync::Arc};
 
 use anyhow::Context as _;
-use spin_http::routes::RouteMatch;
 use spin_trigger2::cli::{TriggerAppBuilder, TriggerAppOptions};
 use spin_trigger_http2::{HttpServer, HttpTrigger};
 use test_environment::{
@@ -54,12 +53,10 @@ impl InProcessSpin {
             }
             // TODO(rylev): convert body as well
             let req = builder.body(spin_http::body::empty()).unwrap();
-            let route_match = RouteMatch::synthetic("test", "/");
             let response = self
                 .server
-                .handle_trigger_route(
+                .handle(
                     req,
-                    route_match,
                     http::uri::Scheme::HTTP,
                     (std::net::Ipv4Addr::LOCALHOST, 7000).into(),
                 )