Skip to content

Commit

Permalink
Revert RequestHandler type refactoring
Browse files Browse the repository at this point in the history 
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
  • Loading branch information
@rylev
rylev committed Aug 21, 2024
1 parent 6a67cef commit 32f6da4
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 146 deletions.
12 changes: 6 additions & 6 deletions crates/trigger-http2/src/outbound_http.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}

Expand All @@ -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 {
Expand Down
218 changes: 91 additions & 127 deletions crates/trigger-http2/src/server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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()
Expand Down Expand Up @@ -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);
}
Expand All @@ -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.
Expand Down
9 changes: 3 additions & 6 deletions tests/conformance-tests/src/lib.rs
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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(|| {
Expand Down
6 changes: 3 additions & 3 deletions tests/testing-framework/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
5 changes: 1 addition & 4 deletions tests/testing-framework/src/runtimes/in_process_spin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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(),
)
Expand Down

0 comments on commit 32f6da4

Please sign in to comment.