From 29c831649dd48cb7e7dddf17956f5bcd2c561000 Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Thu, 19 Nov 2020 23:39:07 -0500 Subject: [PATCH 1/4] Changed the add_route API to allow the use of simpler, async handlers --- Cargo.toml | 1 - examples/certificates.rs | 64 +++++++++++++++++++--------------------- examples/document.rs | 59 +++++++++++++++++------------------- examples/routing.rs | 20 ++++--------- examples/serve_dir.rs | 13 +++----- src/lib.rs | 10 ++++--- 6 files changed, 73 insertions(+), 94 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9ad991d..14847b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,5 +28,4 @@ mime_guess = { version = "2.0.3", optional = true } [dev-dependencies] env_logger = "0.8.1" -futures-util = "0.3.7" tokio = { version = "0.3.1", features = ["macros", "rt-multi-thread", "sync"] } diff --git a/examples/certificates.rs b/examples/certificates.rs index 143c71c..6fdcafe 100644 --- a/examples/certificates.rs +++ b/examples/certificates.rs @@ -1,6 +1,4 @@ use anyhow::*; -use futures_core::future::BoxFuture; -use futures_util::FutureExt; use log::LevelFilter; use tokio::sync::RwLock; use northstar::{Certificate, GEMINI_MIME, GEMINI_PORT, Request, Response, Server}; @@ -31,44 +29,42 @@ async fn main() -> Result<()> { /// selecting a username. They'll then get a message confirming their account creation. /// Any time this user visits the site in the future, they'll get a personalized welcome /// message. -fn handle_request(users: Arc>>, request: Request) -> BoxFuture<'static, Result> { - async move { - if let Some(Certificate(cert_bytes)) = request.certificate() { - // The user provided a certificate - let users_read = users.read().await; - if let Some(user) = users_read.get(cert_bytes) { - // The user has already registered +async fn handle_request(users: Arc>>, request: Request) -> Result { + if let Some(Certificate(cert_bytes)) = request.certificate() { + // The user provided a certificate + let users_read = users.read().await; + if let Some(user) = users_read.get(cert_bytes) { + // The user has already registered + Ok( + Response::success_with_body( + &GEMINI_MIME, + format!("Welcome {}!", user) + ) + ) + } else { + // The user still needs to register + drop(users_read); + if let Some(query_part) = request.uri().query() { + // The user provided some input (a username request) + let username = query_part.as_str(); + let mut users_write = users.write().await; + users_write.insert(cert_bytes.clone(), username.to_owned()); Ok( Response::success_with_body( &GEMINI_MIME, - format!("Welcome {}!", user) + format!( + "Your account has been created {}! Welcome!", + username + ) ) ) } else { - // The user still needs to register - drop(users_read); - if let Some(query_part) = request.uri().query() { - // The user provided some input (a username request) - let username = query_part.as_str(); - let mut users_write = users.write().await; - users_write.insert(cert_bytes.clone(), username.to_owned()); - Ok( - Response::success_with_body( - &GEMINI_MIME, - format!( - "Your account has been created {}! Welcome!", - username - ) - ) - ) - } else { - // The user didn't provide input, and should be prompted - Response::input("What username would you like?") - } + // The user didn't provide input, and should be prompted + Response::input("What username would you like?") } - } else { - // The user didn't provide a certificate - Ok(Response::client_certificate_required()) } - }.boxed() + } else { + // The user didn't provide a certificate + Ok(Response::client_certificate_required()) + } } diff --git a/examples/document.rs b/examples/document.rs index cc889c6..bc72f49 100644 --- a/examples/document.rs +++ b/examples/document.rs @@ -1,6 +1,4 @@ use anyhow::*; -use futures_core::future::BoxFuture; -use futures_util::FutureExt; use log::LevelFilter; use northstar::{Server, Request, Response, GEMINI_PORT, Document}; use northstar::document::HeadingLevel::*; @@ -17,36 +15,33 @@ async fn main() -> Result<()> { .await } -fn handle_request(_request: Request) -> BoxFuture<'static, Result> { - async move { - let mut document = Document::new(); +async fn handle_request(_request: Request) -> Result { + let mut document = Document::new(); - document - .add_preformatted(include_str!("northstar_logo.txt")) - .add_blank_line() - .add_link("https://docs.rs/northstar", "Documentation") - .add_link("https://github.com/panicbit/northstar", "GitHub") - .add_blank_line() - .add_heading(H1, "Usage") - .add_blank_line() - .add_text("Add the latest version of northstar to your `Cargo.toml`.") - .add_blank_line() - .add_heading(H2, "Manually") - .add_blank_line() - .add_preformatted_with_alt("toml", r#"northstar = "0.3.0" # check crates.io for the latest version"#) - .add_blank_line() - .add_heading(H2, "Automatically") - .add_blank_line() - .add_preformatted_with_alt("sh", "cargo add northstar") - .add_blank_line() - .add_heading(H1, "Generating a key & certificate") - .add_blank_line() - .add_preformatted_with_alt("sh", concat!( - "mkdir cert && cd cert\n", - "openssl req -x509 -nodes -newkey rsa:4096 -keyout key.pem -out cert.pem -days 365", - )); + document + .add_preformatted(include_str!("northstar_logo.txt")) + .add_blank_line() + .add_link("https://docs.rs/northstar", "Documentation") + .add_link("https://github.com/panicbit/northstar", "GitHub") + .add_blank_line() + .add_heading(H1, "Usage") + .add_blank_line() + .add_text("Add the latest version of northstar to your `Cargo.toml`.") + .add_blank_line() + .add_heading(H2, "Manually") + .add_blank_line() + .add_preformatted_with_alt("toml", r#"northstar = "0.3.0" # check crates.io for the latest version"#) + .add_blank_line() + .add_heading(H2, "Automatically") + .add_blank_line() + .add_preformatted_with_alt("sh", "cargo add northstar") + .add_blank_line() + .add_heading(H1, "Generating a key & certificate") + .add_blank_line() + .add_preformatted_with_alt("sh", concat!( + "mkdir cert && cd cert\n", + "openssl req -x509 -nodes -newkey rsa:4096 -keyout key.pem -out cert.pem -days 365", + )); - Ok(Response::document(document)) - } - .boxed() + Ok(Response::document(document)) } diff --git a/examples/routing.rs b/examples/routing.rs index 742a620..4bbf9c3 100644 --- a/examples/routing.rs +++ b/examples/routing.rs @@ -1,6 +1,4 @@ use anyhow::*; -use futures_core::future::BoxFuture; -use futures_util::FutureExt; use log::LevelFilter; use northstar::{Document, document::HeadingLevel, Request, Response, GEMINI_PORT}; @@ -18,25 +16,19 @@ async fn main() -> Result<()> { .await } -fn handle_base(_: Request) -> BoxFuture<'static, Result> { +async fn handle_base(_: Request) -> Result { let doc = generate_doc("base"); - async move { - Ok(Response::document(doc)) - }.boxed() + Ok(Response::document(doc)) } -fn handle_short(_: Request) -> BoxFuture<'static, Result> { +async fn handle_short(_: Request) -> Result { let doc = generate_doc("short"); - async move { - Ok(Response::document(doc)) - }.boxed() + Ok(Response::document(doc)) } -fn handle_long(_: Request) -> BoxFuture<'static, Result> { +async fn handle_long(_: Request) -> Result { let doc = generate_doc("long"); - async move { - Ok(Response::document(doc)) - }.boxed() + Ok(Response::document(doc)) } fn generate_doc(route_name: &str) -> Document { diff --git a/examples/serve_dir.rs b/examples/serve_dir.rs index de3e0b0..bb81add 100644 --- a/examples/serve_dir.rs +++ b/examples/serve_dir.rs @@ -1,6 +1,4 @@ use anyhow::*; -use futures_core::future::BoxFuture; -use futures_util::FutureExt; use log::LevelFilter; use northstar::{Server, Request, Response, GEMINI_PORT}; @@ -16,12 +14,9 @@ async fn main() -> Result<()> { .await } -fn handle_request(request: Request) -> BoxFuture<'static, Result> { - async move { - let path = request.path_segments(); - let response = northstar::util::serve_dir("public", &path).await?; +async fn handle_request(request: Request) -> Result { + let path = request.path_segments(); + let response = northstar::util::serve_dir("public", &path).await?; - Ok(response) - } - .boxed() + Ok(response) } diff --git a/src/lib.rs b/src/lib.rs index 1812967..7f7f4d6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,7 +8,7 @@ use std::{ path::PathBuf, time::Duration, }; -use futures_core::future::BoxFuture; +use futures_core::future::{BoxFuture, Future}; use tokio::{ prelude::*, io::{self, BufStream}, @@ -290,11 +290,13 @@ impl Builder { /// "endpoint". Entering a relative or malformed path will result in a panic. /// /// For more information about routing mechanics, see the docs for [`RoutingNode`]. - pub fn add_route(mut self, path: &'static str, handler: H) -> Self + pub fn add_route(mut self, path: &'static str, handler: H) -> Self where - H: Fn(Request) -> HandlerResponse + Send + Sync + 'static, + H: Send + Sync + 'static + Fn(Request) -> F, + F: Send + Sync + 'static + Future> { - self.routes.add_route(path, Arc::new(handler)); + let wrapped = Arc::new(move|req| Box::pin((handler)(req)) as HandlerResponse); + self.routes.add_route(path, wrapped); self } From 5612ce1085883c72cee603fe09d9f04a95966552 Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Thu, 19 Nov 2020 23:51:25 -0500 Subject: [PATCH 2/4] Removed unnecessary dependency on futures-rs --- Cargo.toml | 1 - src/lib.rs | 5 +++-- src/util.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 14847b4..0f004c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,6 @@ tokio = { version = "0.3.1", features = ["io-util","net","time", "rt"] } mime = "0.3.16" uriparse = "0.6.3" percent-encoding = "2.1.0" -futures-core = "0.3.7" log = "0.4.11" webpki = "0.21.0" lazy_static = "1.4.0" diff --git a/src/lib.rs b/src/lib.rs index 7f7f4d6..b2f97a8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,8 +7,9 @@ use std::{ sync::Arc, path::PathBuf, time::Duration, + pin::Pin, }; -use futures_core::future::{BoxFuture, Future}; +use std::future::Future; use tokio::{ prelude::*, io::{self, BufStream}, @@ -36,7 +37,7 @@ pub const REQUEST_URI_MAX_LEN: usize = 1024; pub const GEMINI_PORT: u16 = 1965; pub (crate) type Handler = Arc HandlerResponse + Send + Sync>; -pub (crate) type HandlerResponse = BoxFuture<'static, Result>; +pub (crate) type HandlerResponse = Pin> + Send>>; #[derive(Clone)] pub struct Server { diff --git a/src/util.rs b/src/util.rs index 5c623aa..33ca6d6 100644 --- a/src/util.rs +++ b/src/util.rs @@ -13,7 +13,7 @@ use crate::types::{Document, document::HeadingLevel::*}; use crate::types::Response; use std::panic::{catch_unwind, AssertUnwindSafe}; use std::task::Poll; -use futures_core::future::Future; +use std::future::Future; use tokio::time; #[cfg(feature="serve_dir")] From f0798b66a338fe6fc2293c44983181fbee1b01f5 Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Thu, 19 Nov 2020 23:52:34 -0500 Subject: [PATCH 3/4] Update changelog for improved handlers --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 435e8ee..8bbbd46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - build time and size by [@Alch-Emi](https://github.com/Alch-Emi) ### Changed - Added route API [@Alch-Emi](https://github.com/Alch-Emi) +- API for adding handlers now accepts async handlers [@Alch-Emi](https://github.com/Alch-Emi) ## [0.3.0] - 2020-11-14 ### Added From 95a4a8d75d17da9adbcaf3b00cbede3ac3a0ec45 Mon Sep 17 00:00:00 2001 From: Emi Tatsuo Date: Sun, 22 Nov 2020 11:55:33 -0500 Subject: [PATCH 4/4] Reduce number of required `Arc`s Should improve performance because cloning an `Arc` is expensive --- src/lib.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 86bfab4..95d3425 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -36,13 +36,12 @@ pub use types::*; pub const REQUEST_URI_MAX_LEN: usize = 1024; pub const GEMINI_PORT: u16 = 1965; -type Handler = Arc HandlerResponse + Send + Sync>; +type Handler = Box HandlerResponse + Send + Sync>; pub (crate) type HandlerResponse = Pin> + Send>>; #[derive(Clone)] pub struct Server { tls_acceptor: TlsAcceptor, - listener: Arc, routes: Arc>, timeout: Duration, complex_timeout: Option, @@ -53,9 +52,9 @@ impl Server { Builder::bind(addr) } - async fn serve(self) -> Result<()> { + async fn serve(self, listener: TcpListener) -> Result<()> { loop { - let (stream, _addr) = self.listener.accept().await + let (stream, _addr) = listener.accept().await .context("Failed to accept client")?; let this = self.clone(); @@ -298,7 +297,7 @@ impl Builder { H: Send + Sync + 'static + Fn(Request) -> F, F: Send + Sync + 'static + Future> { - let wrapped = Arc::new(move|req| Box::pin((handler)(req)) as HandlerResponse); + let wrapped = Box::new(move|req| Box::pin((handler)(req)) as HandlerResponse); self.routes.add_route(path, wrapped); self } @@ -314,13 +313,12 @@ impl Builder { let server = Server { tls_acceptor: TlsAcceptor::from(config), - listener: Arc::new(listener), routes: Arc::new(self.routes), timeout: self.timeout, complex_timeout: self.complex_body_timeout_override, }; - server.serve().await + server.serve(listener).await } }