From 7ee20de87f7937491ff7c9dae56019b7b4ed653d Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Sat, 20 Feb 2021 17:05:49 -0800 Subject: [PATCH 1/4] try using routefinder --- Cargo.toml | 2 +- src/request.rs | 49 +++++++++++++++++++++++-------- src/route.rs | 22 +++++--------- src/router.rs | 37 ++++++++++++++--------- tests/wildcard.rs | 75 ++++++++++++----------------------------------- 5 files changed, 86 insertions(+), 99 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f523b414d..b1185e26f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,9 +45,9 @@ http-types = { version = "2.11.0", default-features = false, features = ["fs"] } kv-log-macro = "1.0.7" log = { version = "0.4.13", features = ["kv_unstable_std"] } pin-project-lite = "0.2.0" -route-recognizer = "0.2.0" serde = "1.0.117" serde_json = "1.0.59" +routefinder = "0.1.1" [dev-dependencies] async-std = { version = "1.6.5", features = ["unstable", "attributes"] } diff --git a/src/request.rs b/src/request.rs index f1f137c5a..e6b46762d 100644 --- a/src/request.rs +++ b/src/request.rs @@ -1,6 +1,6 @@ use async_std::io::{self, prelude::*}; use async_std::task::{Context, Poll}; -use route_recognizer::Params; +use routefinder::Captures; use std::ops::Index; use std::pin::Pin; @@ -27,13 +27,13 @@ pin_project_lite::pin_project! { pub(crate) state: State, #[pin] pub(crate) req: http::Request, - pub(crate) route_params: Vec, + pub(crate) route_params: Vec, } } impl Request { /// Create a new `Request`. - pub(crate) fn new(state: State, req: http_types::Request, route_params: Vec) -> Self { + pub(crate) fn new(state: State, req: http_types::Request, route_params: Vec) -> Self { Self { state, req, @@ -266,8 +266,7 @@ impl Request { /// /// Returns the parameter as a `&str`, borrowed from this `Request`. /// - /// The name should *not* include the leading `:` or the trailing `*` (if - /// any). + /// The name should *not* include the leading `:`. /// /// # Errors /// @@ -297,10 +296,40 @@ impl Request { self.route_params .iter() .rev() - .find_map(|params| params.find(key)) + .find_map(|captures| captures.get(key)) .ok_or_else(|| format_err!("Param \"{}\" not found", key.to_string())) } + /// Fetch the wildcard from the route, if it exists + /// + /// Returns the parameter as a `&str`, borrowed from this `Request`. + /// + /// # Examples + /// + /// ```no_run + /// # use async_std::task::block_on; + /// # fn main() -> Result<(), std::io::Error> { block_on(async { + /// # + /// use tide::{Request, Result}; + /// + /// async fn greet(req: Request<()>) -> Result { + /// let name = req.wildcard().unwrap_or("world"); + /// Ok(format!("Hello, {}!", name)) + /// } + /// + /// let mut app = tide::new(); + /// app.at("/hello/*").get(greet); + /// app.listen("127.0.0.1:8080").await?; + /// # + /// # Ok(()) })} + /// ``` + pub fn wildcard(&self) -> Option<&str> { + self.route_params + .iter() + .rev() + .find_map(|captures| captures.wildcard()) + } + /// Parse the URL query component into a struct, using [serde_qs](https://docs.rs/serde_qs). To /// get the entire query as an unparsed string, use `request.url().query()`. /// @@ -565,7 +594,7 @@ impl From> for http::Request { impl From for Request { fn from(request: http_types::Request) -> Request { - Request::new(State::default(), request, Vec::::new()) + Request::new(State::default(), request, vec![]) } } @@ -635,9 +664,3 @@ impl Index<&str> for Request { &self.req[name] } } - -pub(crate) fn rest(route_params: &[Params]) -> Option<&str> { - route_params - .last() - .and_then(|params| params.find("--tide-path-rest")) -} diff --git a/src/route.rs b/src/route.rs index 66fda966c..c77e602dc 100644 --- a/src/route.rs +++ b/src/route.rs @@ -153,13 +153,7 @@ impl<'a, State: Clone + Send + Sync + 'static> Route<'a, State> { pub fn method(&mut self, method: http_types::Method, ep: impl Endpoint) -> &mut Self { if self.prefix { let ep = StripPrefixEndpoint::new(ep); - - self.router.add( - &self.path, - method, - MiddlewareEndpoint::wrap_with_middleware(ep.clone(), &self.middleware), - ); - let wildcard = self.at("*--tide-path-rest"); + let wildcard = self.at("*"); wildcard.router.add( &wildcard.path, method, @@ -181,12 +175,7 @@ impl<'a, State: Clone + Send + Sync + 'static> Route<'a, State> { pub fn all(&mut self, ep: impl Endpoint) -> &mut Self { if self.prefix { let ep = StripPrefixEndpoint::new(ep); - - self.router.add_all( - &self.path, - MiddlewareEndpoint::wrap_with_middleware(ep.clone(), &self.middleware), - ); - let wildcard = self.at("*--tide-path-rest"); + let wildcard = self.at("*"); wildcard.router.add_all( &wildcard.path, MiddlewareEndpoint::wrap_with_middleware(ep, &wildcard.middleware), @@ -283,7 +272,12 @@ where route_params, } = req; - let rest = crate::request::rest(&route_params).unwrap_or(""); + let rest = route_params + .iter() + .rev() + .find_map(|captures| captures.wildcard()) + .unwrap_or_default(); + req.url_mut().set_path(rest); self.0 diff --git a/src/router.rs b/src/router.rs index e59e26005..d80cc6b4a 100644 --- a/src/router.rs +++ b/src/router.rs @@ -1,4 +1,4 @@ -use route_recognizer::{Match, Params, Router as MethodRouter}; +use routefinder::{Captures, Router as MethodRouter}; use std::collections::HashMap; use crate::endpoint::DynEndpoint; @@ -14,11 +14,19 @@ pub(crate) struct Router { all_method_router: MethodRouter>>, } +impl std::fmt::Debug for Router { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Router") + .field("method_map", &self.method_map) + .field("all_method_router", &self.all_method_router) + .finish() + } +} + /// The result of routing a URL -#[allow(missing_debug_implementations)] pub(crate) struct Selection<'a, State> { pub(crate) endpoint: &'a DynEndpoint, - pub(crate) params: Params, + pub(crate) params: Captures, } impl Router { @@ -39,26 +47,27 @@ impl Router { .entry(method) .or_insert_with(MethodRouter::new) .add(path, ep) + .unwrap() } pub(crate) fn add_all(&mut self, path: &str, ep: Box>) { - self.all_method_router.add(path, ep) + self.all_method_router.add(path, ep).unwrap() } pub(crate) fn route(&self, path: &str, method: http_types::Method) -> Selection<'_, State> { - if let Some(Match { handler, params }) = self + if let Some(m) = self .method_map .get(&method) - .and_then(|r| r.recognize(path).ok()) + .and_then(|r| r.best_match(path)) { Selection { - endpoint: &**handler, - params, + endpoint: m.handler(), + params: m.captures(), } - } else if let Ok(Match { handler, params }) = self.all_method_router.recognize(path) { + } else if let Some(m) = self.all_method_router.best_match(path) { Selection { - endpoint: &**handler, - params, + endpoint: m.handler(), + params: m.captures(), } } else if method == http_types::Method::Head { // If it is a HTTP HEAD request then check if there is a callback in the endpoints map @@ -69,18 +78,18 @@ impl Router { .method_map .iter() .filter(|(k, _)| **k != method) - .any(|(_, r)| r.recognize(path).is_ok()) + .any(|(_, r)| r.best_match(path).is_some()) { // If this `path` can be handled by a callback registered with a different HTTP method // should return 405 Method Not Allowed Selection { endpoint: &method_not_allowed, - params: Params::new(), + params: Captures::default(), } } else { Selection { endpoint: ¬_found_endpoint, - params: Params::new(), + params: Captures::default(), } } } diff --git a/tests/wildcard.rs b/tests/wildcard.rs index 6cca1c0db..bb45752c2 100644 --- a/tests/wildcard.rs +++ b/tests/wildcard.rs @@ -22,18 +22,22 @@ async fn add_two(req: Request<()>) -> Result { Ok((one + two).to_string()) } -async fn echo_path(req: Request<()>) -> Result { - match req.param("path") { +async fn echo_param(req: Request<()>) -> tide::Result { + match req.param("param") { Ok(path) => Ok(path.into()), - Err(mut err) => { - err.set_status(StatusCode::BadRequest); - Err(err) - } + Err(_) => Ok(StatusCode::NotFound.into()), + } +} + +async fn echo_wildcard(req: Request<()>) -> tide::Result { + match req.wildcard() { + Some(path) => Ok(path.into()), + None => Ok(StatusCode::NotFound.into()), } } #[async_std::test] -async fn wildcard() -> tide::Result<()> { +async fn param() -> tide::Result<()> { let mut app = tide::Server::new(); app.at("/add_one/:num").get(add_one); assert_eq!(app.get("/add_one/3").recv_string().await?, "4"); @@ -61,20 +65,21 @@ async fn not_found_error() -> tide::Result<()> { } #[async_std::test] -async fn wild_path() -> tide::Result<()> { +async fn wildcard() -> tide::Result<()> { let mut app = tide::new(); - app.at("/echo/*path").get(echo_path); + app.at("/echo/*").get(echo_wildcard); assert_eq!(app.get("/echo/some_path").recv_string().await?, "some_path"); assert_eq!( app.get("/echo/multi/segment/path").recv_string().await?, "multi/segment/path" ); - assert_eq!(app.get("/echo/").await?.status(), StatusCode::NotFound); + assert_eq!(app.get("/echo/").await?.status(), StatusCode::Ok); + assert_eq!(app.get("/echo").await?.status(), StatusCode::Ok); Ok(()) } #[async_std::test] -async fn multi_wildcard() -> tide::Result<()> { +async fn multi_param() -> tide::Result<()> { let mut app = tide::new(); app.at("/add_two/:one/:two/").get(add_two); assert_eq!(app.get("/add_two/1/2/").recv_string().await?, "3"); @@ -84,9 +89,9 @@ async fn multi_wildcard() -> tide::Result<()> { } #[async_std::test] -async fn wild_last_segment() -> tide::Result<()> { +async fn wildcard_last_segment() -> tide::Result<()> { let mut app = tide::new(); - app.at("/echo/:path/*").get(echo_path); + app.at("/echo/:param/*").get(echo_param); assert_eq!(app.get("/echo/one/two").recv_string().await?, "one"); assert_eq!( app.get("/echo/one/two/three/four").recv_string().await?, @@ -95,50 +100,6 @@ async fn wild_last_segment() -> tide::Result<()> { Ok(()) } -#[async_std::test] -async fn invalid_wildcard() -> tide::Result<()> { - let mut app = tide::new(); - app.at("/echo/*path/:one/").get(echo_path); - assert_eq!( - app.get("/echo/one/two").await?.status(), - StatusCode::NotFound - ); - Ok(()) -} - -#[async_std::test] -async fn nameless_wildcard() -> tide::Result<()> { - let mut app = tide::Server::new(); - app.at("/echo/:").get(|_| async { Ok("") }); - assert_eq!( - app.get("/echo/one/two").await?.status(), - StatusCode::NotFound - ); - assert_eq!(app.get("/echo/one").await?.status(), StatusCode::Ok); - Ok(()) -} - -#[async_std::test] -async fn nameless_internal_wildcard() -> tide::Result<()> { - let mut app = tide::new(); - app.at("/echo/:/:path").get(echo_path); - assert_eq!(app.get("/echo/one").await?.status(), StatusCode::NotFound); - assert_eq!(app.get("/echo/one/two").recv_string().await?, "two"); - Ok(()) -} - -#[async_std::test] -async fn nameless_internal_wildcard2() -> tide::Result<()> { - let mut app = tide::new(); - app.at("/echo/:/:path").get(|req: Request<()>| async move { - assert_eq!(req.param("path")?, "two"); - Ok("") - }); - - assert!(app.get("/echo/one/two").await?.status().is_success()); - Ok(()) -} - #[async_std::test] async fn ambiguous_router_wildcard_vs_star() -> tide::Result<()> { let mut app = tide::new(); From e5d1ba9c597ab1b4f353cd18f46a9fcef16e5399 Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Tue, 10 Aug 2021 12:26:23 -0700 Subject: [PATCH 2/4] upgrade routefinder --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index b1185e26f..94f7b44ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,7 +47,7 @@ log = { version = "0.4.13", features = ["kv_unstable_std"] } pin-project-lite = "0.2.0" serde = "1.0.117" serde_json = "1.0.59" -routefinder = "0.1.1" +routefinder = "0.3.1" [dev-dependencies] async-std = { version = "1.6.5", features = ["unstable", "attributes"] } From cd9b4628b10be7f0599815a5dc03c756a7fdc63e Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Tue, 10 Aug 2021 12:51:56 -0700 Subject: [PATCH 3/4] add key and value 'static lifetimes to captures --- src/request.rs | 8 ++++++-- src/router.rs | 6 +++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/request.rs b/src/request.rs index e6b46762d..854154e66 100644 --- a/src/request.rs +++ b/src/request.rs @@ -27,13 +27,17 @@ pin_project_lite::pin_project! { pub(crate) state: State, #[pin] pub(crate) req: http::Request, - pub(crate) route_params: Vec, + pub(crate) route_params: Vec>, } } impl Request { /// Create a new `Request`. - pub(crate) fn new(state: State, req: http_types::Request, route_params: Vec) -> Self { + pub(crate) fn new( + state: State, + req: http_types::Request, + route_params: Vec>, + ) -> Self { Self { state, req, diff --git a/src/router.rs b/src/router.rs index d80cc6b4a..70b1ab350 100644 --- a/src/router.rs +++ b/src/router.rs @@ -26,7 +26,7 @@ impl std::fmt::Debug for Router { /// The result of routing a URL pub(crate) struct Selection<'a, State> { pub(crate) endpoint: &'a DynEndpoint, - pub(crate) params: Captures, + pub(crate) params: Captures<'static, 'static>, } impl Router { @@ -62,12 +62,12 @@ impl Router { { Selection { endpoint: m.handler(), - params: m.captures(), + params: m.captures().into_owned(), } } else if let Some(m) = self.all_method_router.best_match(path) { Selection { endpoint: m.handler(), - params: m.captures(), + params: m.captures().into_owned(), } } else if method == http_types::Method::Head { // If it is a HTTP HEAD request then check if there is a callback in the endpoints map From a3b0f68c230611bc462f6c540043d4637723fb6f Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Thu, 12 Aug 2021 19:34:39 -0700 Subject: [PATCH 4/4] routefinder 0.4.0 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 94f7b44ac..519c789a0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,7 +47,7 @@ log = { version = "0.4.13", features = ["kv_unstable_std"] } pin-project-lite = "0.2.0" serde = "1.0.117" serde_json = "1.0.59" -routefinder = "0.3.1" +routefinder = "0.4.0" [dev-dependencies] async-std = { version = "1.6.5", features = ["unstable", "attributes"] }