diff --git a/dropshot/examples/pagination-multiple-sorts.rs b/dropshot/examples/pagination-multiple-sorts.rs index 7f673f741..b4ed716e3 100644 --- a/dropshot/examples/pagination-multiple-sorts.rs +++ b/dropshot/examples/pagination-multiple-sorts.rs @@ -304,9 +304,10 @@ async fn main() -> Result<(), String> { .map_err(|error| format!("failed to create logger: {}", error))?; let mut api = ApiDescription::new(); api.register(example_list_projects).unwrap(); - let server = HttpServerStarter::new(&config_dropshot, api, ctx, &log) - .map_err(|error| format!("failed to create server: {}", error))? - .start(); + let server = HttpServerStarter::new(&config_dropshot, ctx, &log) + .api(api) + .start() + .map_err(|error| format!("failed to create server: {}", error))?; // Print out some example requests to start with. print_example_requests(log, &server.local_addr()); diff --git a/dropshot/src/router.rs b/dropshot/src/router.rs index ca5b38a11..fc7edb3d2 100644 --- a/dropshot/src/router.rs +++ b/dropshot/src/router.rs @@ -81,14 +81,15 @@ pub struct HttpRouter { #[derive(Debug)] struct HttpRouterNode { /// Handlers, etc. for each of the HTTP methods defined for this node. - methods: BTreeMap>, + pub methods: BTreeMap>, /// Edges linking to child nodes. - edges: Option>, + pub edges: Option>, } #[derive(Debug)] enum HttpRouterEdges { /// Outgoing edges for literal paths. + // TODO why box here? Literals(BTreeMap>>), /// Outgoing edge for variable-named paths. VariableSingle(String, Box>), @@ -218,6 +219,28 @@ impl HttpRouterNode { pub fn new() -> Self { HttpRouterNode { methods: BTreeMap::new(), edges: None } } + + // Recursive merge into the target router. + fn merge(self, router: &mut HttpRouter) { + let Self { methods, edges } = self; + + // Insert all endpoints. + methods.into_iter().for_each(|(_, endpoint)| router.insert(endpoint)); + + // Recur as needed. + match edges { + Some(HttpRouterEdges::Literals(children)) => { + children.into_values().for_each(|child| { + child.merge(router); + }) + } + Some(HttpRouterEdges::VariableSingle(_, edge)) + | Some(HttpRouterEdges::VariableRest(_, edge)) => { + edge.merge(router) + } + None => (), + } + } } impl HttpRouter { @@ -396,6 +419,10 @@ impl HttpRouter { node.methods.insert(methodname, endpoint); } + pub fn merge(&mut self, other: HttpRouter) { + other.root.merge(self) + } + /// Look up the route handler for an HTTP request having method `method` and /// URI path `path`. A successful lookup produces a `RouterLookupResult`, /// which includes both the handler that can process this request and a map @@ -491,6 +518,19 @@ impl HttpRouter { } } +impl Extend<(String, String, ApiEndpoint)> + for HttpRouter +{ + fn extend< + T: IntoIterator)>, + >( + &mut self, + iter: T, + ) { + iter.into_iter().for_each(|(_, _, endpoint)| self.insert(endpoint)) + } +} + /// Insert a variable into the set after checking for duplicates. fn insert_var( path: &str, diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 795530cfe..fa8c2fd23 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -99,29 +99,56 @@ pub struct ServerConfig { } pub struct HttpServerStarter { - app_state: Arc>, - local_addr: SocketAddr, - wrapped: WrappedHttpServerStarter, - handler_waitgroup: WaitGroup, + // app_state: Arc>, + // local_addr: SocketAddr, + // wrapped: WrappedHttpServerStarter, + // handler_waitgroup: WaitGroup, + config: ConfigDropshot, + apis: Vec>, + private: C, + logger: Logger, + tls: Option, } impl HttpServerStarter { - pub fn new( - config: &ConfigDropshot, - api: ApiDescription, - private: C, - log: &Logger, - ) -> Result, GenericError> { - Self::new_with_tls(config, api, private, log, None) + pub fn new(config: &ConfigDropshot, private: C, log: &Logger) -> Self { + Self::new_with_tls(config, private, log, None) } pub fn new_with_tls( config: &ConfigDropshot, - api: ApiDescription, private: C, log: &Logger, tls: Option, - ) -> Result, GenericError> { + ) -> Self { + Self { + config: config.clone(), + apis: Default::default(), + private, + logger: log.clone(), + tls, + } + } + + /// Add an API to the server. + pub fn api(mut self, api: ApiDescription) -> Self { + self.apis.push(api); + self + } + + fn pre_start( + self, + ) -> Result< + ( + Arc>, + SocketAddr, + WrappedHttpServerStarter, + WaitGroup, + ), + GenericError, + > { + let Self { config, apis, private, logger, tls } = self; + let server_config = ServerConfig { // We start aggressively to ensure test coverage. request_body_max_bytes: config.request_body_max_bytes, @@ -130,59 +157,62 @@ impl HttpServerStarter { default_handler_task_mode: config.default_handler_task_mode, }; + // TODO build up the router + let mut router = HttpRouter::new(); + for api in apis { + router.merge(api.into_router()) + } + let handler_waitgroup = WaitGroup::new(); - let starter = match &tls { + let (wrapped, app_state, local_addr) = match &tls { Some(tls) => { let (starter, app_state, local_addr) = InnerHttpsServerStarter::new( - config, + &config, server_config, - api, + router, private, - log, + &logger, tls, handler_waitgroup.worker(), )?; - HttpServerStarter { + ( + WrappedHttpServerStarter::Https(starter), app_state, local_addr, - wrapped: WrappedHttpServerStarter::Https(starter), - handler_waitgroup, - } + ) } None => { let (starter, app_state, local_addr) = InnerHttpServerStarter::new( - config, + &config, server_config, - api, + router, private, - log, + &logger, handler_waitgroup.worker(), )?; - HttpServerStarter { - app_state, - local_addr, - wrapped: WrappedHttpServerStarter::Http(starter), - handler_waitgroup, - } + (WrappedHttpServerStarter::Http(starter), app_state, local_addr) } }; - for (path, method, _) in &starter.app_state.router { - debug!(starter.app_state.log, "registered endpoint"; + for (path, method, _) in &app_state.router { + debug!(app_state.log, "registered endpoint"; "method" => &method, "path" => &path ); } - Ok(starter) + Ok((app_state, local_addr, wrapped, handler_waitgroup)) } - pub fn start(self) -> HttpServer { + pub fn start(self) -> Result, GenericError> { + let (app_state, local_addr, wrapped, handler_waitgroup) = + self.pre_start()?; + let (tx, rx) = tokio::sync::oneshot::channel::<()>(); - let log_close = self.app_state.log.new(o!()); - let join_handle = match self.wrapped { + let log_close = app_state.log.new(o!()); + let join_handle = match wrapped { WrappedHttpServerStarter::Http(http) => http.start(rx, log_close), WrappedHttpServerStarter::Https(https) => { https.start(rx, log_close) @@ -192,9 +222,8 @@ impl HttpServerStarter { r.map_err(|e| format!("waiting for server: {e}"))? .map_err(|e| format!("server stopped: {e}")) }); - info!(self.app_state.log, "listening"); + info!(app_state.log, "listening"); - let handler_waitgroup = self.handler_waitgroup; let join_handle = async move { // After the server shuts down, we also want to wait for any // detached handler futures to complete. @@ -207,7 +236,7 @@ impl HttpServerStarter { let probe_registration = match usdt::register_probes() { Ok(_) => { debug!( - self.app_state.log, + app_state.log, "successfully registered DTrace USDT probes" ); ProbeRegistration::Succeeded @@ -215,7 +244,7 @@ impl HttpServerStarter { Err(e) => { let msg = e.to_string(); error!( - self.app_state.log, + app_state.log, "failed to register DTrace USDT probes: {}", msg ); ProbeRegistration::Failed(msg) @@ -224,19 +253,19 @@ impl HttpServerStarter { #[cfg(not(feature = "usdt-probes"))] let probe_registration = { debug!( - self.app_state.log, + app_state.log, "DTrace USDT probes compiled out, not registering" ); ProbeRegistration::Disabled }; - HttpServer { + Ok(HttpServer { probe_registration, - app_state: self.app_state, - local_addr: self.local_addr, + app_state, + local_addr, closer: CloseHandle { close_channel: Some(tx) }, join_future: join_handle.boxed().shared(), - } + }) } } @@ -276,7 +305,7 @@ impl InnerHttpServerStarter { fn new( config: &ConfigDropshot, server_config: ServerConfig, - api: ApiDescription, + router: HttpRouter, private: C, log: &Logger, handler_waitgroup_worker: waitgroup::Worker, @@ -287,7 +316,7 @@ impl InnerHttpServerStarter { let app_state = Arc::new(DropshotState { private, config: server_config, - router: api.into_router(), + router, log: log.new(o!("local_addr" => local_addr)), local_addr, tls_acceptor: None, @@ -561,7 +590,7 @@ impl InnerHttpsServerStarter { fn new( config: &ConfigDropshot, server_config: ServerConfig, - api: ApiDescription, + router: HttpRouter, private: C, log: &Logger, tls: &ConfigTls, @@ -588,7 +617,7 @@ impl InnerHttpsServerStarter { let app_state = Arc::new(DropshotState { private, config: server_config, - router: api.into_router(), + router, log: logger, local_addr, tls_acceptor: Some(acceptor), @@ -1115,9 +1144,10 @@ mod test { let log_context = LogContext::new("test server", &config_logging); let log = &log_context.log; - let server = HttpServerStarter::new(&config_dropshot, api, 0, log) - .unwrap() - .start(); + let server = HttpServerStarter::new(&config_dropshot, 0, log) + .api(api) + .start() + .unwrap(); (server, TestConfig { log_context }) } diff --git a/dropshot/src/test_util.rs b/dropshot/src/test_util.rs index f94eeb35a..2fd975446 100644 --- a/dropshot/src/test_util.rs +++ b/dropshot/src/test_util.rs @@ -491,10 +491,10 @@ impl TestContext { ); // Set up the server itself. - let server = - HttpServerStarter::new(&config_dropshot, api, private, &log) - .unwrap() - .start(); + let server = HttpServerStarter::new(&config_dropshot, private, &log) + .api(api) + .start() + .unwrap(); let server_addr = server.local_addr(); let client_log = log.new(o!("http_client" => "dropshot test suite"));