Merge branch 'master' into fix_test_response

Author: yusuke-ota

.github/workflows/ci.yaml

@@ -45,7 +45,13 @@ jobs:
       uses: actions-rs/cargo@v1
       with:
         command: check
-        args: --all --benches --bins --examples --tests --features unstable
+        args: --all --bins --examples --tests --features unstable
+
+    - name: check benches
+      uses: actions-rs/cargo@v1
+      with:
+        command: check
+        args: --benches --features __internal__bench
 
     - name: tests
       uses: actions-rs/cargo@v1

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "tide"
-version = "0.8.1"
+version = "0.9.0"
 description = "Serve the web – HTTP server framework"
 authors = [
     "Aaron Turon <[email protected]>",
@@ -28,14 +28,15 @@ default = ["h1-server"]
 h1-server = ["async-h1"]
 docs = ["unstable"]
 unstable = []
+# DO NOT USE. Only exists to expose internals so they can be benchmarked.
+__internal__bench = []
 
 [dependencies]
-async-h1 = { version = "1.1.2", optional = true }
+async-h1 = { version = "2.0.0", optional = true }
 async-sse = "2.1.0"
-async-std = { version = "1.4.0", features = ["unstable"] }
-cookie = { version = "0.12.0", features = ["percent-encode"]}
+async-std = { version = "1.6.0", features = ["unstable"] }
 femme = "2.0.1"
-http-types = "1.0.1"
+http-types = "2.0.0"
 kv-log-macro = "1.0.4"
 mime = "0.3.14"
 mime_guess = "2.0.3"
@@ -45,13 +46,19 @@ serde_json = "1.0.41"
 serde_qs = "0.5.0"
 
 [dev-dependencies]
-async-std = { version = "1.4.0", features = ["unstable", "attributes"] }
+async-std = { version = "1.6.0", features = ["unstable", "attributes"] }
 juniper = "0.14.1"
 portpicker = "0.1.0"
 serde = { version = "1.0.102", features = ["derive"] }
-surf = "2.0.0-alpha.1"
+criterion = "0.3.1"
+surf = { version = "2.0.0-alpha.2", default-features = false, features = ["h1-client"] }
 
 [[test]]
 name = "nested"
 path = "tests/nested.rs"
 required-features = ["unstable"]
+
+[[bench]]
+name = "router"
+harness = false
+

benches/router.rs

@@ -0,0 +1,23 @@
+use criterion::{black_box, criterion_group, criterion_main, Criterion};
+use http_types::Method;
+use tide::router::Router;
+
+fn criterion_benchmark(c: &mut Criterion) {
+    let mut router = Router::<()>::new();
+    router.add(
+        "hello",
+        Method::Get,
+        Box::new(|_| async move { Ok("hello world") }),
+    );
+
+    c.bench_function("route-match", |b| {
+        b.iter(|| black_box(router.route("/hello", Method::Get)))
+    });
+
+    c.bench_function("route-root", |b| {
+        b.iter(|| black_box(router.route("", Method::Get)))
+    });
+}
+
+criterion_group!(benches, criterion_benchmark);
+criterion_main!(benches);

examples/fib.rs

@@ -17,7 +17,7 @@ async fn fibsum(req: Request<()>) -> tide::Result<String> {
     // Compute the nth number in the fibonacci sequence
     let fib_n = fib(n);
     // Stop the stopwatch
-    let duration = Instant::now().duration_since(start).as_secs();
+    let duration = start.elapsed().as_secs();
     // Return the answer
     let res = format!(
         "The fib of {} is {}.\nIt was computed in {} secs.\n",

examples/graphql.rs

@@ -95,7 +95,7 @@ async fn handle_graphql(mut cx: Request<State>) -> tide::Result {
 async fn handle_graphiql(_: Request<State>) -> tide::Result {
     let res = Response::new(StatusCode::Ok)
         .body_string(juniper::http::graphiql::graphiql_source("/graphql"))
-        .set_header("content-type".parse().unwrap(), "text/html;charset=utf-8");
+        .set_mime(mime::TEXT_HTML_UTF_8);
     Ok(res)
 }
 

src/cookies/middleware.rs

@@ -2,7 +2,7 @@ use crate::response::CookieEvent;
 use crate::utils::BoxFuture;
 use crate::{Middleware, Next, Request};
 
-use crate::http::cookies::{Cookie, CookieJar};
+use crate::http::cookies::{Cookie, CookieJar, Delta};
 use crate::http::headers;
 
 use std::sync::{Arc, RwLock};
@@ -13,19 +13,20 @@ use std::sync::{Arc, RwLock};
 ///
 /// ```
 /// # use tide::{Request, Response, StatusCode};
+/// # use tide::http::cookies::Cookie;
 /// let mut app = tide::Server::new();
 /// app.at("/get").get(|cx: Request<()>| async move { Ok(cx.cookie("testCookie").unwrap().value().to_string()) });
 /// app.at("/set").get(|_| async {
 ///     let mut res = Response::new(StatusCode::Ok);
-///     res.set_cookie(cookie::Cookie::new("testCookie", "NewCookieValue"));
+///     res.set_cookie(Cookie::new("testCookie", "NewCookieValue"));
 ///     Ok(res)
 /// });
 /// ```
 #[derive(Debug, Clone, Default)]
 pub(crate) struct CookiesMiddleware;
 
 impl CookiesMiddleware {
-    /// Creates a new CookiesMiddleware.
+    /// Creates a new `CookiesMiddleware`.
     pub fn new() -> Self {
         Self::default()
     }
@@ -38,30 +39,35 @@ impl<State: Send + Sync + 'static> Middleware<State> for CookiesMiddleware {
         next: Next<'a, State>,
     ) -> BoxFuture<'a, crate::Result> {
         Box::pin(async move {
-            let cookie_jar = if let Some(cookie_data) = ctx.local::<CookieData>() {
+            let cookie_jar = if let Some(cookie_data) = ctx.ext::<CookieData>() {
                 cookie_data.content.clone()
             } else {
-                // no cookie data in local context, so we need to create it
                 let cookie_data = CookieData::from_request(&ctx);
+                // no cookie data in ext context, so we try to create it
                 let content = cookie_data.content.clone();
-                ctx = ctx.set_local(cookie_data);
+                ctx = ctx.set_ext(cookie_data);
                 content
             };
 
             let mut res = next.run(ctx).await?;
 
+            // Don't do anything if there are no cookies.
+            if res.cookie_events.is_empty() {
+                return Ok(res);
+            }
+
+            let jar = &mut *cookie_jar.write().unwrap();
+
             // add modifications from response to original
             for cookie in res.cookie_events.drain(..) {
                 match cookie {
-                    CookieEvent::Added(cookie) => cookie_jar.write().unwrap().add(cookie.clone()),
-                    CookieEvent::Removed(cookie) => {
-                        cookie_jar.write().unwrap().remove(cookie.clone())
-                    }
+                    CookieEvent::Added(cookie) => jar.add(cookie.clone()),
+                    CookieEvent::Removed(cookie) => jar.remove(cookie.clone()),
                 }
             }
 
             // iterate over added and removed cookies
-            for cookie in cookie_jar.read().unwrap().delta() {
+            for cookie in jar.delta() {
                 let encoded_cookie = cookie.encoded().to_string();
                 res = res.append_header(headers::SET_COOKIE, encoded_cookie);
             }
@@ -70,25 +76,61 @@ impl<State: Send + Sync + 'static> Middleware<State> for CookiesMiddleware {
     }
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Default, Clone)]
 pub(crate) struct CookieData {
-    pub(crate) content: Arc<RwLock<CookieJar>>,
+    pub(crate) content: Arc<RwLock<LazyJar>>,
+}
+
+#[derive(Debug, Default, Clone)]
+/// Wrapper around `CookieJar`, that initializes only when actually used.
+pub(crate) struct LazyJar(Option<CookieJar>);
+
+impl LazyJar {
+    fn add(&mut self, cookie: Cookie<'static>) {
+        self.get_jar().add(cookie)
+    }
+
+    fn remove(&mut self, cookie: Cookie<'static>) {
+        self.get_jar().remove(cookie)
+    }
+
+    fn delta(&mut self) -> Delta<'_> {
+        self.get_jar().delta()
+    }
+
+    pub(crate) fn get(&self, name: &str) -> Option<&Cookie<'static>> {
+        if let Some(jar) = &self.0 {
+            return jar.get(name);
+        }
+        None
+    }
+
+    fn get_jar(&mut self) -> &mut CookieJar {
+        if self.0.is_none() {
+            self.0 = Some(CookieJar::new());
+        }
+
+        self.0.as_mut().unwrap()
+    }
 }
 
 impl CookieData {
     pub(crate) fn from_request<S>(req: &Request<S>) -> Self {
-        let mut jar = CookieJar::new();
-
-        if let Some(cookie_headers) = req.header(&headers::COOKIE) {
+        let jar = if let Some(cookie_headers) = req.header(&headers::COOKIE) {
+            let mut jar = CookieJar::new();
             for cookie_header in cookie_headers {
                 // spec says there should be only one, so this is permissive
-                for pair in cookie_header.as_str().split(";") {
+                for pair in cookie_header.as_str().split(';') {
                     if let Ok(cookie) = Cookie::parse_encoded(String::from(pair)) {
                         jar.add_original(cookie);
                     }
                 }
             }
-        }
+
+            LazyJar(Some(jar))
+        } else {
+            LazyJar::default()
+        };
 
         CookieData {
             content: Arc::new(RwLock::new(jar)),

src/endpoint.rs

@@ -27,10 +27,8 @@ use crate::{Middleware, Request, Response};
 ///     Ok(String::from("hello"))
 /// }
 ///
-/// fn main() {
-///     let mut app = tide::Server::new();
-///     app.at("/hello").get(hello);
-/// }
+/// let mut app = tide::Server::new();
+/// app.at("/hello").get(hello);
 /// ```
 ///
 /// An endpoint with similar functionality that does not make use of the `async` keyword would look something like this:
@@ -41,10 +39,8 @@ use crate::{Middleware, Request, Response};
 ///     async_std::future::ready(Ok(String::from("hello")))
 /// }
 ///
-/// fn main() {
-///     let mut app = tide::Server::new();
-///     app.at("/hello").get(hello);
-/// }
+/// let mut app = tide::Server::new();
+/// app.at("/hello").get(hello);
 /// ```
 ///
 /// Tide routes will also accept endpoints with `Fn` signatures of this form, but using the `async` keyword has better ergonomics.

src/fs/serve_dir.rs

@@ -57,12 +57,11 @@ impl<State> Endpoint<State> for ServeDir {
                 Ok(file) => file,
             };
 
-            let len = match file.metadata().await {
-                Ok(metadata) => metadata.len() as usize,
-                Err(_) => {
-                    log::warn!("Could not retrieve metadata");
-                    return Ok(Response::new(StatusCode::InternalServerError));
-                }
+            let len = if let Ok(metadata) = file.metadata().await {
+                metadata.len() as usize
+            } else {
+                log::warn!("Could not retrieve metadata");
+                return Ok(Response::new(StatusCode::InternalServerError));
             };
 
             let body = Body::from_reader(BufReader::new(file), Some(len));

src/lib.rs

@@ -193,7 +193,11 @@ mod redirect;
 mod request;
 mod response;
 mod route;
+
+#[cfg(not(feature = "__internal__bench"))]
 mod router;
+#[cfg(feature = "__internal__bench")]
+pub mod router;
 mod server;
 mod utils;
 
@@ -227,6 +231,7 @@ pub use http_types::{self as http, Body, Error, Status, StatusCode};
 /// #
 /// # Ok(()) }) }
 /// ```
+#[must_use]
 pub fn new() -> server::Server<()> {
     Server::new()
 }

src/log/middleware.rs

@@ -12,13 +12,14 @@ use crate::{Middleware, Next, Request};
 /// let mut app = tide::Server::new();
 /// app.middleware(tide::log::LogMiddleware::new());
 /// ```
-#[derive(Debug, Clone)]
+#[derive(Debug, Default, Clone)]
 pub struct LogMiddleware {
     _priv: (),
 }
 
 impl LogMiddleware {
     /// Create a new instance of `LogMiddleware`.
+    #[must_use]
     pub fn new() -> Self {
         Self { _priv: () }
     }
@@ -44,21 +45,21 @@ impl LogMiddleware {
                         method: method,
                         path: path,
                         status: status as u16,
-                        duration: format!("{}ms", start.elapsed().as_millis()),
+                        duration: format!("{:?}", start.elapsed()),
                     });
                 } else if status.is_client_error() {
                     log::warn!("--> Response sent", {
                         method: method,
                         path: path,
                         status: status as u16,
-                        duration: format!("{}ms", start.elapsed().as_millis()),
+                        duration: format!("{:?}", start.elapsed()),
                     });
                 } else {
                     log::info!("--> Response sent", {
                         method: method,
                         path: path,
                         status: status as u16,
-                        duration: format!("{}ms", start.elapsed().as_millis()),
+                        duration: format!("{:?}", start.elapsed()),
                     });
                 }
                 Ok(res)
@@ -68,7 +69,7 @@ impl LogMiddleware {
                     method: method,
                     path: path,
                     status: err.status() as u16,
-                    duration: format!("{}ms", start.elapsed().as_millis()),
+                    duration: format!("{:?}", start.elapsed()),
                 });
                 Err(err)
             }

src/middleware.rs

@@ -47,6 +47,7 @@ pub struct Next<'a, State> {
 
 impl<'a, State: 'static> Next<'a, State> {
     /// Asynchronously execute the remaining middleware chain.
+    #[must_use]
     pub fn run(mut self, req: Request<State>) -> BoxFuture<'a, crate::Result> {
         if let Some((current, next)) = self.next_middleware.split_first() {
             self.next_middleware = next;

src/redirect.rs

@@ -16,6 +16,7 @@
 //! # Ok(()) }) }
 //! ```
 
+use crate::http::headers::LOCATION;
 use crate::utils::BoxFuture;
 use crate::StatusCode;
 use crate::{Endpoint, Request, Response};
@@ -103,6 +104,6 @@ impl<T: AsRef<str>> Into<Response> for Redirect<T> {
 
 impl<T: AsRef<str>> Into<Response> for &Redirect<T> {
     fn into(self) -> Response {
-        Response::new(self.status).set_header("location".parse().unwrap(), &self.location)
+        Response::new(self.status).set_header(LOCATION, self.location.as_ref())
     }
 }