From 2ddd0a230c5b7e4506a7c757e503952d17fd6c41 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Sat, 11 Jun 2022 12:30:12 +0200 Subject: [PATCH 01/17] Break `Router::nest` into `nest` and `nest_service` methods --- axum-extra/src/routing/resource.rs | 4 +- axum-extra/src/routing/spa.rs | 2 +- axum/CHANGELOG.md | 11 ++ axum/Cargo.toml | 4 +- axum/src/extract/matched_path.rs | 6 +- axum/src/extract/path/mod.rs | 4 +- axum/src/routing/mod.rs | 165 ++++++++++++----------------- axum/src/routing/tests/mod.rs | 84 +++------------ axum/src/routing/tests/nest.rs | 94 ++++++---------- 9 files changed, 136 insertions(+), 238 deletions(-) diff --git a/axum-extra/src/routing/resource.rs b/axum-extra/src/routing/resource.rs index 98105facd9..86e835056c 100644 --- a/axum-extra/src/routing/resource.rs +++ b/axum-extra/src/routing/resource.rs @@ -146,7 +146,7 @@ where T::Future: Send + 'static, { let path = self.show_update_destroy_path(); - self.router = self.router.nest(&path, svc); + self.router = self.router.nest_service(&path, svc); self } @@ -159,7 +159,7 @@ where T::Future: Send + 'static, { let path = self.index_create_path(); - self.router = self.router.nest(&path, svc); + self.router = self.router.nest_service(&path, svc); self } diff --git a/axum-extra/src/routing/spa.rs b/axum-extra/src/routing/spa.rs index 594b15c237..f8e3d0b0ea 100644 --- a/axum-extra/src/routing/spa.rs +++ b/axum-extra/src/routing/spa.rs @@ -161,7 +161,7 @@ where .handle_error(spa.handle_error.clone()); Router::new() - .nest(&spa.paths.assets_path, assets_service) + .nest_service(&spa.paths.assets_path, assets_service) .fallback( get_service(ServeFile::new(&spa.paths.index_file)).handle_error(spa.handle_error), ) diff --git a/axum/CHANGELOG.md b/axum/CHANGELOG.md index 3bd2a04d68..a3c74aed13 100644 --- a/axum/CHANGELOG.md +++ b/axum/CHANGELOG.md @@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **added:** Support resolving host name via `Forwarded` header in `Host` extractor ([#1078]) +- **breaking:** `Router::nest` now only accepts `Router`s. Use + `Router::nest_service` to nest opaque services +- **added:** Add `Router::nest_service` for nesting opaque services. Use this to + nest services like `tower::services::ServeDir` +- **breaking:** The route `/foo/` no longer matches `/foo/*rest`. If you want + to match `/foo/` you have to add a route specifically for that +- **breaking:** Path params for wildcard routes no longer include the prefix + `/`. e.g. `/foo.js` will match `/*filepath` with a value of `foo.js`, _not_ + `/foo.js` +- **fixed:** Routes like `/foo` and `/*rest` are no longer considered + overlapping. `/foo` will take priority [#1078]: https://github.com/tokio-rs/axum/pull/1078 diff --git a/axum/Cargo.toml b/axum/Cargo.toml index c5de4abfd8..bb66b2f4f9 100644 --- a/axum/Cargo.toml +++ b/axum/Cargo.toml @@ -33,7 +33,9 @@ http = "0.2.5" http-body = "0.4.4" hyper = { version = "0.14.14", features = ["server", "tcp", "stream"] } itoa = "1.0.1" -matchit = "0.5.0" +# TODO(david): cannot ship until matchit has released a new version but we can +# start making changes +matchit = { git = "https://github.com/ibraheemdev/matchit", branch = "catchall-revamp" } memchr = "2.4.1" mime = "0.3.16" percent-encoding = "2.1" diff --git a/axum/src/extract/matched_path.rs b/axum/src/extract/matched_path.rs index 8965bd30bf..9a3381e1a7 100644 --- a/axum/src/extract/matched_path.rs +++ b/axum/src/extract/matched_path.rs @@ -149,12 +149,14 @@ mod tests { "/public", Router::new().route("/assets/*path", get(handler)), ) - .nest("/foo", handler.into_service()) + .nest_service("/foo", handler.into_service()) .layer(tower::layer::layer_fn(SetMatchedPathExtension)); let client = TestClient::new(app); - let res = client.get("/foo").send().await; + // we cannot call `/foo` because `nest_service("/foo", _)` registers routes + // for `/foo/*rest` and `/foo` + let res = client.get("/public").send().await; assert_eq!(res.text().await, "/:key"); let res = client.get("/api/users/123").send().await; diff --git a/axum/src/extract/path/mod.rs b/axum/src/extract/path/mod.rs index f559ae1fb0..20a23ce70b 100644 --- a/axum/src/extract/path/mod.rs +++ b/axum/src/extract/path/mod.rs @@ -499,10 +499,10 @@ mod tests { let client = TestClient::new(app); let res = client.get("/foo/bar/baz").send().await; - assert_eq!(res.text().await, "/bar/baz"); + assert_eq!(res.text().await, "bar/baz"); let res = client.get("/bar/baz/qux").send().await; - assert_eq!(res.text().await, "/baz/qux"); + assert_eq!(res.text().await, "baz/qux"); } #[tokio::test] diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index ca178a75ca..b7955e152b 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -3,7 +3,7 @@ use self::{future::RouteFuture, not_found::NotFound}; use crate::{ body::{boxed, Body, Bytes, HttpBody}, - extract::connect_info::IntoMakeServiceWithConnectInfo, + extract::{connect_info::IntoMakeServiceWithConnectInfo, MatchedPath}, response::{IntoResponse, Redirect, Response}, routing::strip_prefix::StripPrefix, util::try_downcast, @@ -66,7 +66,6 @@ pub struct Router { routes: HashMap>, node: Node, fallback: Fallback, - nested_at_root: bool, } impl Clone for Router { @@ -75,7 +74,6 @@ impl Clone for Router { routes: self.routes.clone(), node: self.node.clone(), fallback: self.fallback.clone(), - nested_at_root: self.nested_at_root, } } } @@ -95,13 +93,11 @@ impl fmt::Debug for Router { .field("routes", &self.routes) .field("node", &self.node) .field("fallback", &self.fallback) - .field("nested_at_root", &self.nested_at_root) .finish() } } pub(crate) const NEST_TAIL_PARAM: &str = "__private__axum_nest_tail_param"; -const NEST_TAIL_PARAM_CAPTURE: &str = "/*__private__axum_nest_tail_param"; impl Router where @@ -116,7 +112,6 @@ where routes: Default::default(), node: Default::default(), fallback: Fallback::Default(Route::new(NotFound)), - nested_at_root: false, } } @@ -163,7 +158,7 @@ where }; if let Err(err) = self.node.insert(path, id) { - self.panic_on_matchit_error(err); + panic!("Invalid route: {}", err); } self.routes.insert(id, service); @@ -171,12 +166,9 @@ where self } + // TODO(david): update docs #[doc = include_str!("../docs/routing/nest.md")] - pub fn nest(mut self, mut path: &str, svc: T) -> Self - where - T: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, - T::Future: Send + 'static, - { + pub fn nest(mut self, mut path: &str, router: Router) -> Self { if path.is_empty() { // nesting at `""` and `"/"` should mean the same thing path = "/"; @@ -188,65 +180,74 @@ where let prefix = path; - if path == "/" { - self.nested_at_root = true; + let Router { + mut routes, + node, + fallback, + } = router; + + if let Fallback::Custom(_) = fallback { + panic!("Cannot nest `Router`s that has a fallback"); } - match try_downcast::, _>(svc) { - // if the user is nesting a `Router` we can implement nesting - // by simplying copying all the routes and adding the prefix in - // front - Ok(router) => { - let Router { - mut routes, - node, - fallback, - // nesting a router that has something nested at root - // doesn't mean something is nested at root in _this_ router - // thus we don't need to propagate that - nested_at_root: _, - } = router; - - if let Fallback::Custom(_) = fallback { - panic!("Cannot nest `Router`s that has a fallback"); - } + for (id, nested_path) in node.route_id_to_path { + let route = routes.remove(&id).unwrap(); + let full_path: Cow = if &*nested_path == "/" { + path.into() + } else if path == "/" { + (&*nested_path).into() + } else if let Some(path) = path.strip_suffix('/') { + format!("{}{}", path, nested_path).into() + } else { + format!("{}{}", path, nested_path).into() + }; + self = match route { + Endpoint::MethodRouter(method_router) => self.route( + &full_path, + method_router.layer(layer_fn(|s| StripPrefix::new(s, prefix))), + ), + Endpoint::Route(route) => self.route(&full_path, StripPrefix::new(route, prefix)), + }; + } - for (id, nested_path) in node.route_id_to_path { - let route = routes.remove(&id).unwrap(); - let full_path: Cow = if &*nested_path == "/" { - path.into() - } else if path == "/" { - (&*nested_path).into() - } else if let Some(path) = path.strip_suffix('/') { - format!("{}{}", path, nested_path).into() - } else { - format!("{}{}", path, nested_path).into() - }; - self = match route { - Endpoint::MethodRouter(method_router) => self.route( - &full_path, - method_router.layer(layer_fn(|s| StripPrefix::new(s, prefix))), - ), - Endpoint::Route(route) => { - self.route(&full_path, StripPrefix::new(route, prefix)) - } - }; - } + debug_assert!(routes.is_empty()); - debug_assert!(routes.is_empty()); - } - // otherwise we add a wildcard route to the service - Err(svc) => { - let path = if path.ends_with('/') { - format!("{}*{}", path, NEST_TAIL_PARAM) - } else { - format!("{}/*{}", path, NEST_TAIL_PARAM) - }; + self + } - self = self.route(&path, strip_prefix::StripPrefix::new(svc, prefix)); - } + /// TODO + pub fn nest_service(mut self, mut path: &str, svc: T) -> Self + where + T: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, + T::Future: Send + 'static, + { + if path.is_empty() { + // nesting at `""` and `"/"` should mean the same thing + path = "/"; } + if path.contains('*') { + panic!("Invalid route: nested routes cannot contain wildcards (*)"); + } + + let prefix = path; + + let path = if path.ends_with('/') { + format!("{}*{}", path, NEST_TAIL_PARAM) + } else { + format!("{}/*{}", path, NEST_TAIL_PARAM) + }; + + let svc = strip_prefix::StripPrefix::new(svc, prefix); + self = self.route(&path, svc.clone()); + + // `/*rest` is not matched by `/` so we need to also register a router at the + // prefix itself. Otherwise if you were to nest at `/foo` then `/foo` itself + // wouldn't match, which it should + self = self.route(prefix, svc.clone()); + // same goes for `/foo/`, that should also match + self = self.route(&format!("{}/", prefix), svc); + self } @@ -259,7 +260,6 @@ where routes, node, fallback, - nested_at_root, } = other.into(); for (id, route) in routes { @@ -282,8 +282,6 @@ where } }; - self.nested_at_root = self.nested_at_root || nested_at_root; - self } @@ -324,7 +322,6 @@ where routes, node: self.node, fallback, - nested_at_root: self.nested_at_root, } } @@ -363,7 +360,6 @@ where routes, node: self.node, fallback: self.fallback, - nested_at_root: self.nested_at_root, } } @@ -419,24 +415,8 @@ where #[cfg(feature = "matched-path")] if let Some(matched_path) = self.node.route_id_to_path.get(&id) { - use crate::extract::MatchedPath; - - let matched_path = if let Some(previous) = req.extensions_mut().get::() { - // a previous `MatchedPath` might exist if we're inside a nested Router - let previous = if let Some(previous) = - previous.as_str().strip_suffix(NEST_TAIL_PARAM_CAPTURE) - { - previous - } else { - previous.as_str() - }; - - let matched_path = format!("{}{}", previous, matched_path); - matched_path.into() - } else { - Arc::clone(matched_path) - }; - req.extensions_mut().insert(MatchedPath(matched_path)); + req.extensions_mut() + .insert(MatchedPath(Arc::clone(matched_path))); } else { #[cfg(debug_assertions)] panic!("should always have a matched path for a route id"); @@ -455,17 +435,6 @@ where Endpoint::Route(inner) => inner.call(req), } } - - fn panic_on_matchit_error(&self, err: matchit::InsertError) { - if self.nested_at_root { - panic!( - "Invalid route: {}. Note that `nest(\"/\", _)` conflicts with all routes. Use `Router::fallback` instead", - err, - ); - } else { - panic!("Invalid route: {}", err); - } - } } impl Service> for Router diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index b253bc5282..7616434990 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -8,10 +8,9 @@ use crate::{ test_helpers::*, BoxError, Json, Router, }; -use http::{header::CONTENT_LENGTH, HeaderMap, Method, Request, Response, StatusCode, Uri}; +use http::{header::CONTENT_LENGTH, HeaderMap, Request, Response, StatusCode, Uri}; use hyper::Body; -use serde::Deserialize; -use serde_json::{json, Value}; +use serde_json::json; use std::{ convert::Infallible, future::{ready, Ready}, @@ -387,47 +386,24 @@ async fn without_trailing_slash_post() { assert_eq!(res.headers().get("location").unwrap(), "/foo/"); } -// for https://github.com/tokio-rs/axum/issues/420 #[tokio::test] -async fn wildcard_with_trailing_slash() { - #[derive(Deserialize, serde::Serialize)] - struct Tree { - user: String, - repo: String, - path: String, - } - - let app: Router = Router::new().route( - "/:user/:repo/tree/*path", - get(|Path(tree): Path| async move { Json(tree) }), +async fn wildcard_doesnt_match_just_trailing_slash() { + let app = Router::new().route( + "/x/*path", + get(|Path(path): Path| async move { path }), ); - // low level check that the correct redirect happens - let res = app - .clone() - .oneshot( - Request::builder() - .method(Method::GET) - .uri("/user1/repo1/tree") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT); - assert_eq!(res.headers()["location"], "/user1/repo1/tree/"); - - // check that the params are deserialized correctly let client = TestClient::new(app); - let res = client.get("/user1/repo1/tree/").send().await; - assert_eq!( - res.json::().await, - json!({ - "user": "user1", - "repo": "repo1", - "path": "/", - }) - ); + + let res = client.get("/x").send().await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + + let res = client.get("/x/").send().await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + + let res = client.get("/x/foo/bar").send().await; + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.text().await, "foo/bar"); } #[tokio::test] @@ -531,34 +507,6 @@ async fn route_layer() { assert_eq!(res.status(), StatusCode::UNAUTHORIZED); } -#[tokio::test] -#[should_panic( - expected = "Invalid route: insertion failed due to conflict with previously registered \ - route: /*__private__axum_nest_tail_param. \ - Note that `nest(\"/\", _)` conflicts with all routes. \ - Use `Router::fallback` instead" -)] -async fn good_error_message_if_using_nest_root() { - let app = Router::new() - .nest("/", get(|| async {})) - .route("/", get(|| async {})); - TestClient::new(app); -} - -#[tokio::test] -#[should_panic( - expected = "Invalid route: insertion failed due to conflict with previously registered \ - route: /*__private__axum_nest_tail_param. \ - Note that `nest(\"/\", _)` conflicts with all routes. \ - Use `Router::fallback` instead" -)] -async fn good_error_message_if_using_nest_root_when_merging() { - let one = Router::new().nest("/", get(|| async {})); - let two = Router::new().route("/", get(|| async {})); - let app = one.merge(two); - TestClient::new(app); -} - #[tokio::test] async fn different_methods_added_in_different_routes() { let app = Router::new() diff --git a/axum/src/routing/tests/nest.rs b/axum/src/routing/tests/nest.rs index 31149c94f8..83b80a63e4 100644 --- a/axum/src/routing/tests/nest.rs +++ b/axum/src/routing/tests/nest.rs @@ -1,8 +1,5 @@ use super::*; -use crate::{ - body::boxed, - extract::{Extension, MatchedPath}, -}; +use crate::{body::boxed, extract::Extension}; use std::collections::HashMap; use tower_http::services::ServeDir; @@ -117,7 +114,7 @@ async fn nesting_router_at_empty_path() { #[tokio::test] async fn nesting_handler_at_root() { - let app = Router::new().nest("/", get(|uri: Uri| async move { uri.to_string() })); + let app = Router::new().nest_service("/", get(|uri: Uri| async move { uri.to_string() })); let client = TestClient::new(app); @@ -205,7 +202,7 @@ async fn nested_service_sees_stripped_uri() { #[tokio::test] async fn nest_static_file_server() { - let app = Router::new().nest( + let app = Router::new().nest_service( "/static", get_service(ServeDir::new(".")).handle_error(|error| async move { ( @@ -357,7 +354,7 @@ async fn nest_at_capture() { ) .boxed_clone(); - let app = Router::new().nest("/:a", api_routes); + let app = Router::new().nest_service("/:a", api_routes); let client = TestClient::new(app); @@ -366,64 +363,48 @@ async fn nest_at_capture() { assert_eq!(res.text().await, "a=foo b=bar"); } +#[tokio::test] +async fn nest_with_and_without_trailing() { + let app = Router::new().nest_service("/foo", get(|| async {})); + + let client = TestClient::new(app); + + let res = client.get("/foo").send().await; + assert_eq!(res.status(), StatusCode::OK); + + let res = client.get("/foo/").send().await; + assert_eq!(res.status(), StatusCode::OK); + + let res = client.get("/foo/bar").send().await; + assert_eq!(res.status(), StatusCode::OK); +} + macro_rules! nested_route_test { ( $name:ident, + // the path we nest the inner router at nest = $nested_path:literal, + // the route the inner router accepts route = $route_path:literal, - expected = $expected_path:literal, - opaque_redirect = $opaque_redirect:expr $(,)? + // the route we expect to be able to call + expected = $expected_path:literal $(,)? ) => { #[tokio::test] async fn $name() { - let inner = Router::new().route( - $route_path, - get(|matched: MatchedPath| async move { matched.as_str().to_owned() }), - ); + let inner = Router::new().route($route_path, get(|| async {})); let app = Router::new().nest($nested_path, inner); let client = TestClient::new(app); let res = client.get($expected_path).send().await; let status = res.status(); - let matched_path = res.text().await; assert_eq!(status, StatusCode::OK, "Router"); - let inner = Router::new() - .route( - $route_path, - get(|matched: MatchedPath| async move { matched.as_str().to_owned() }), - ) - .boxed_clone(); - let app = Router::new().nest($nested_path, inner); + let inner = Router::new().route($route_path, get(|| async {})); + let app = Router::new().nest_service($nested_path, inner); let client = TestClient::new(app); - let res = client.get($expected_path).send().await; - if $opaque_redirect { - assert_eq!(res.status(), StatusCode::PERMANENT_REDIRECT, "opaque"); - - let location = res.headers()[http::header::LOCATION].to_str().unwrap(); - let res = client.get(location).send().await; - assert_eq!(res.status(), StatusCode::OK, "opaque with redirect"); - assert_eq!(res.text().await, location); - } else { - assert_eq!(res.status(), StatusCode::OK, "opaque"); - assert_eq!(res.text().await, matched_path); - } + let res = client.get(dbg!($expected_path)).send().await; + assert_eq!(res.status(), StatusCode::OK, "opaque"); } }; - - ( - $name:ident, - nest = $nested_path:literal, - route = $route_path:literal, - expected = $expected_path:literal $(,)? - ) => { - nested_route_test!( - $name, - nest = $nested_path, - route = $route_path, - expected = $expected_path, - opaque_redirect = false, - ); - }; } // test cases taken from https://github.com/tokio-rs/axum/issues/714#issuecomment-1058144460 @@ -433,22 +414,7 @@ nested_route_test!(nest_3, nest = "", route = "/a/", expected = "/a/"); nested_route_test!(nest_4, nest = "/", route = "/", expected = "/"); nested_route_test!(nest_5, nest = "/", route = "/a", expected = "/a"); nested_route_test!(nest_6, nest = "/", route = "/a/", expected = "/a/"); - -// This case is different for opaque services. -// -// The internal route becomes `/a/*__private__axum_nest_tail_param` which, according to matchit -// doesn't match `/a`. However matchit detects that a route for `/a/` exists and so it issues a -// redirect to `/a/`, which ends up calling the inner route as expected. -// -// So while the behavior isn't identical, the outcome is the same -nested_route_test!( - nest_7, - nest = "/a", - route = "/", - expected = "/a", - opaque_redirect = true, -); - +nested_route_test!(nest_7, nest = "/a", route = "/", expected = "/a",); nested_route_test!(nest_8, nest = "/a", route = "/a", expected = "/a/a"); nested_route_test!(nest_9, nest = "/a", route = "/a/", expected = "/a/a/"); nested_route_test!(nest_11, nest = "/a/", route = "/", expected = "/a/"); From 2c54bb5b3b161f7c03ddd41105dfe0587c162f70 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 29 Jun 2022 21:32:41 +0200 Subject: [PATCH 02/17] fix doc tests --- axum/src/docs/routing/nest.md | 4 ++-- axum/src/docs/routing/route.md | 16 ---------------- axum/src/routing/tests/nest.rs | 2 +- 3 files changed, 3 insertions(+), 19 deletions(-) diff --git a/axum/src/docs/routing/nest.md b/axum/src/docs/routing/nest.md index a63b14dfbc..5ec1df84b2 100644 --- a/axum/src/docs/routing/nest.md +++ b/axum/src/docs/routing/nest.md @@ -88,7 +88,7 @@ let serve_dir_service = get_service(ServeDir::new("public")) ) }); -let app = Router::new().nest("/public", serve_dir_service); +let app = Router::new().nest_service("/public", serve_dir_service); # async { # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap(); # }; @@ -107,7 +107,7 @@ let app = Router::new() .route("/foo/*rest", get(|uri: Uri| async { // `uri` will contain `/foo` })) - .nest("/bar", get(|uri: Uri| async { + .nest_service("/bar", get(|uri: Uri| async { // `uri` will _not_ contain `/bar` })); # async { diff --git a/axum/src/docs/routing/route.md b/axum/src/docs/routing/route.md index e0a753a005..15f0278808 100644 --- a/axum/src/docs/routing/route.md +++ b/axum/src/docs/routing/route.md @@ -184,22 +184,6 @@ let app = Router::new() The static route `/foo` and the dynamic route `/:key` are not considered to overlap and `/foo` will take precedence. -Take care when using [`Router::nest`] as it behaves like a wildcard route. -Therefore this setup panics: - -```rust,should_panic -use axum::{routing::get, Router}; - -let app = Router::new() - // this is similar to `/api/*` - .nest("/api", get(|| async {})) - // which overlaps with this route - .route("/api/users", get(|| async {})); -# async { -# axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap(); -# }; -``` - Also panics if `path` is empty. ## Nesting diff --git a/axum/src/routing/tests/nest.rs b/axum/src/routing/tests/nest.rs index 83b80a63e4..6c2a6c6737 100644 --- a/axum/src/routing/tests/nest.rs +++ b/axum/src/routing/tests/nest.rs @@ -414,7 +414,7 @@ nested_route_test!(nest_3, nest = "", route = "/a/", expected = "/a/"); nested_route_test!(nest_4, nest = "/", route = "/", expected = "/"); nested_route_test!(nest_5, nest = "/", route = "/a", expected = "/a"); nested_route_test!(nest_6, nest = "/", route = "/a/", expected = "/a/"); -nested_route_test!(nest_7, nest = "/a", route = "/", expected = "/a",); +nested_route_test!(nest_7, nest = "/a/", route = "/", expected = "/a"); nested_route_test!(nest_8, nest = "/a", route = "/a", expected = "/a/a"); nested_route_test!(nest_9, nest = "/a", route = "/a/", expected = "/a/a/"); nested_route_test!(nest_11, nest = "/a/", route = "/", expected = "/a/"); From 014067ebf33444e42251b54f1446d0bfc93ac908 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 29 Jun 2022 22:09:43 +0200 Subject: [PATCH 03/17] update docs --- axum/src/docs/routing/nest.md | 70 ++++++++++++++------------- axum/src/docs/routing/nest_service.md | 40 +++++++++++++++ axum/src/docs/routing/route.md | 2 + axum/src/routing/mod.rs | 3 +- 4 files changed, 79 insertions(+), 36 deletions(-) create mode 100644 axum/src/docs/routing/nest_service.md diff --git a/axum/src/docs/routing/nest.md b/axum/src/docs/routing/nest.md index 5ec1df84b2..4ce0ad3379 100644 --- a/axum/src/docs/routing/nest.md +++ b/axum/src/docs/routing/nest.md @@ -1,4 +1,4 @@ -Nest a group of routes (or a [`Service`]) at some path. +Nest a router at some path. This allows you to break your application into smaller pieces and compose them together. @@ -64,36 +64,6 @@ let app = Router::new().nest("/:version/api", users_api); # }; ``` -# Nesting services - -`nest` also accepts any [`Service`]. This can for example be used with -[`tower_http::services::ServeDir`] to serve static files from a directory: - -```rust -use axum::{ - Router, - routing::get_service, - http::StatusCode, - error_handling::HandleErrorLayer, -}; -use std::{io, convert::Infallible}; -use tower_http::services::ServeDir; - -// Serves files inside the `public` directory at `GET /public/*` -let serve_dir_service = get_service(ServeDir::new("public")) - .handle_error(|error: io::Error| async move { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Unhandled internal error: {}", error), - ) - }); - -let app = Router::new().nest_service("/public", serve_dir_service); -# async { -# axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap(); -# }; -``` - # Differences to wildcard routes Nested routes are similar to wildcard routes. The difference is that @@ -103,18 +73,49 @@ the prefix stripped: ```rust use axum::{routing::get, http::Uri, Router}; +let nested_router = Router::new() + .route("/", get(|uri: Uri| async { + // `uri` will _not_ contain `/bar` + })); + let app = Router::new() .route("/foo/*rest", get(|uri: Uri| async { // `uri` will contain `/foo` })) - .nest_service("/bar", get(|uri: Uri| async { - // `uri` will _not_ contain `/bar` - })); + .nest("/bar", nested_router); # async { # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap(); # }; ``` +# Differences between `nest` and `nest_service` + +When [fallbacks] are called differs between `nest` and `nested_service`. Routers +nested with `nest` will delegate to the fallback if they don't have a matching +route, whereas `nested_service` will not. + +```rust +use axum::{Router, routing::{get, any}, handler::Handler}; + +let nested_router = Router::new().route("/users", get(|| async {})); + +let nested_service = Router::new().route("/app.js", get(|| async {})); + +async fn fallback() {} + +let app = Router::new() + .nest("/api", nested_router) + .nest_service("/assets", nested_service) + // the fallback is not called for request starting with `/bar` but will be + // called for requests starting with `/foo` if `nested_router` doesn't have + // a matching route + .fallback(fallback.into_service()); +# let _: Router = app; +``` + +Note that you would normally use [`tower_http::services::ServeDir`] for serving +static files and thus not calling `nest_service` with a `Router`. + # Panics - If the route overlaps with another route. See [`Router::route`] @@ -125,3 +126,4 @@ for more details. `Router` only allows a single fallback. [`OriginalUri`]: crate::extract::OriginalUri +[fallbacks]: Router::fallback diff --git a/axum/src/docs/routing/nest_service.md b/axum/src/docs/routing/nest_service.md new file mode 100644 index 0000000000..bb61bcea13 --- /dev/null +++ b/axum/src/docs/routing/nest_service.md @@ -0,0 +1,40 @@ +Nest a [`Service`] at some path. + +`nest_service` behaves in the same way as `nest` in terms of + +- [How the URI changes](#how-the-uri-changes) +- [Captures from outer routes](#captures-from-outer-routes) +- [Differences to wildcard routes](#differences-to-wildcard-routes) + +But differs with regards to [fallbacks]. See ["Differences between `nest` and +`nest_service`"](#differences-between-nest-and-nest_service) for more details. + +# Example + +`nest_service` can for example be used with [`tower_http::services::ServeDir`] +to serve static files from a directory: + +```rust +use axum::{ + Router, + routing::get_service, + http::StatusCode, + error_handling::HandleErrorLayer, +}; +use std::{io, convert::Infallible}; +use tower_http::services::ServeDir; + +// Serves files inside the `public` directory at `GET /assets/*` +let serve_dir_service = get_service(ServeDir::new("public")) + .handle_error(|error: io::Error| async move { + ( + StatusCode::INTERNAL_SERVER_ERROR, + format!("Unhandled internal error: {}", error), + ) + }); + +let app = Router::new().nest_service("/assets", serve_dir_service); +# let _: Router = app; +``` + +[fallbacks]: Router::fallback diff --git a/axum/src/docs/routing/route.md b/axum/src/docs/routing/route.md index 15f0278808..cd9d703e1b 100644 --- a/axum/src/docs/routing/route.md +++ b/axum/src/docs/routing/route.md @@ -51,6 +51,8 @@ Examples: - `/:id/:repo/*tree` Wildcard captures can also be extracted using [`Path`](crate::extract::Path). +Note that the leading slash is not included, i.e. for the route `/foo/*rest` and +the path `/foo/bar/baz` the value of `rest` will be `bar/baz`. # Accepting multiple methods diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index acb6a93f6d..44ba414a6e 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -169,7 +169,6 @@ where self } - // TODO(david): update docs #[doc = include_str!("../docs/routing/nest.md")] pub fn nest(mut self, mut path: &str, router: Router) -> Self { if path.is_empty() { @@ -218,7 +217,7 @@ where self } - /// TODO + #[doc = include_str!("../docs/routing/nest_service.md")] pub fn nest_service(mut self, mut path: &str, svc: T) -> Self where T: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, From 79daaf0e04ce19564b7a2833fefb07bd07c5605d Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 29 Jun 2022 22:13:17 +0200 Subject: [PATCH 04/17] fix --- axum/src/routing/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 44ba414a6e..563d27d2f5 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -3,7 +3,7 @@ use self::{future::RouteFuture, not_found::NotFound}; use crate::{ body::{boxed, Body, Bytes, HttpBody}, - extract::{connect_info::IntoMakeServiceWithConnectInfo, MatchedPath}, + extract::connect_info::IntoMakeServiceWithConnectInfo, response::{IntoResponse, Redirect, Response}, routing::strip_prefix::StripPrefix, util::try_downcast, @@ -417,7 +417,7 @@ where #[cfg(feature = "matched-path")] if let Some(matched_path) = self.node.route_id_to_path.get(&id) { req.extensions_mut() - .insert(MatchedPath(Arc::clone(matched_path))); + .insert(crate::extract::MatchedPath(Arc::clone(matched_path))); } else { #[cfg(debug_assertions)] panic!("should always have a matched path for a route id"); From 276a950b67b43757f37c36a17b4700f00036599a Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 29 Jun 2022 22:20:12 +0200 Subject: [PATCH 05/17] Only accept `Router` in `Resource::{nest, nest_collection}` --- axum-extra/src/routing/resource.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/axum-extra/src/routing/resource.rs b/axum-extra/src/routing/resource.rs index af870b2b03..482571f152 100644 --- a/axum-extra/src/routing/resource.rs +++ b/axum-extra/src/routing/resource.rs @@ -140,26 +140,18 @@ where /// Nest another route at the "member level". /// /// The routes will be nested at `/{resource_name}/:{resource_name}_id`. - pub fn nest(mut self, svc: T) -> Self - where - T: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, - T::Future: Send + 'static, - { + pub fn nest(mut self, router: Router) -> Self { let path = self.show_update_destroy_path(); - self.router = self.router.nest_service(&path, svc); + self.router = self.router.nest(&path, router); self } /// Nest another route at the "collection level". /// /// The routes will be nested at `/{resource_name}`. - pub fn nest_collection(mut self, svc: T) -> Self - where - T: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, - T::Future: Send + 'static, - { + pub fn nest_collection(mut self, router: Router) -> Self { let path = self.index_create_path(); - self.router = self.router.nest_service(&path, svc); + self.router = self.router.nest(&path, router); self } @@ -204,7 +196,6 @@ mod tests { .edit(|Path(id): Path| async move { format!("users#edit id={}", id) }) .update(|Path(id): Path| async move { format!("users#update id={}", id) }) .destroy(|Path(id): Path| async move { format!("users#destroy id={}", id) }) - // TODO(david): figure out if we need `nest_service` + `nest` methods on `Resource` .nest(Router::new().route( "/tweets", get(|Path(id): Path| async move { format!("users#tweets id={}", id) }), From b22077862eb04ec49db5c7d0b81692cc237a2bd1 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 29 Jun 2022 22:20:24 +0200 Subject: [PATCH 06/17] update changelog --- axum-extra/CHANGELOG.md | 5 ++++- axum/CHANGELOG.md | 11 ++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/axum-extra/CHANGELOG.md b/axum-extra/CHANGELOG.md index 4ce0796291..434d8436a1 100644 --- a/axum-extra/CHANGELOG.md +++ b/axum-extra/CHANGELOG.md @@ -7,7 +7,10 @@ and this project adheres to [Semantic Versioning]. # Unreleased -- None. +- **breaking:** `Resource::nest` and `Resource::nest_collection` now only + accepts `Router`s ([#1086]) + +[#1086]: https://github.com/tokio-rs/axum/pull/1086 # 0.3.5 (27. June, 2022) diff --git a/axum/CHANGELOG.md b/axum/CHANGELOG.md index 051eeeecdc..3855992ae4 100644 --- a/axum/CHANGELOG.md +++ b/axum/CHANGELOG.md @@ -12,18 +12,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **breaking:** Allow `Error: Into` for `Route::{layer, route_layer}` ([#924]) - **breaking:** `MethodRouter` now panics on overlapping routes ([#1102]) - **breaking:** `Router::nest` now only accepts `Router`s. Use - `Router::nest_service` to nest opaque services + `Router::nest_service` to nest opaque services ([#1086]) - **added:** Add `Router::nest_service` for nesting opaque services. Use this to - nest services like `tower::services::ServeDir` + nest services like `tower::services::ServeDir` ([#1086]) - **breaking:** The request `/foo/` no longer matches `/foo/*rest`. If you want - to match `/foo/` you have to add a route specifically for that + to match `/foo/` you have to add a route specifically for that ([#1086]) - **breaking:** Path params for wildcard routes no longer include the prefix `/`. e.g. `/foo.js` will match `/*filepath` with a value of `foo.js`, _not_ - `/foo.js` + `/foo.js` ([#1086]) - **fixed:** Routes like `/foo` and `/*rest` are no longer considered - overlapping. `/foo` will take priority + overlapping. `/foo` will take priority ([#1086]) [#1077]: https://github.com/tokio-rs/axum/pull/1077 +[#1086]: https://github.com/tokio-rs/axum/pull/1086 [#1102]: https://github.com/tokio-rs/axum/pull/1102 [#924]: https://github.com/tokio-rs/axum/pull/924 From fb5219f9cd77ac6c4f7b7455b3e8e27ae8126c4e Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 29 Jun 2022 22:21:28 +0200 Subject: [PATCH 07/17] fix docs --- axum-extra/src/routing/resource.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/axum-extra/src/routing/resource.rs b/axum-extra/src/routing/resource.rs index 482571f152..720c0407d7 100644 --- a/axum-extra/src/routing/resource.rs +++ b/axum-extra/src/routing/resource.rs @@ -137,7 +137,7 @@ where self.route(&path, delete(handler)) } - /// Nest another route at the "member level". + /// Nest another router at the "member level". /// /// The routes will be nested at `/{resource_name}/:{resource_name}_id`. pub fn nest(mut self, router: Router) -> Self { @@ -146,7 +146,7 @@ where self } - /// Nest another route at the "collection level". + /// Nest another router at the "collection level". /// /// The routes will be nested at `/{resource_name}`. pub fn nest_collection(mut self, router: Router) -> Self { From 657a0893b51f581b0090747cfdbe95070d126483 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 29 Jun 2022 22:48:44 +0200 Subject: [PATCH 08/17] fix `MatchedPath` with `Router`s nested with `nest_service` --- axum/src/extract/matched_path.rs | 16 ++++++++++++++++ axum/src/routing/mod.rs | 21 +++++++++++++++++++-- axum/src/routing/tests/nest.rs | 2 +- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/axum/src/extract/matched_path.rs b/axum/src/extract/matched_path.rs index 9a3381e1a7..192983cd68 100644 --- a/axum/src/extract/matched_path.rs +++ b/axum/src/extract/matched_path.rs @@ -178,4 +178,20 @@ mod tests { ), ); } + + #[tokio::test] + async fn nested_opaque_routers_append_to_matched_path() { + let app = Router::new().nest_service( + "/:a", + Router::new().route( + "/:b", + get(|path: MatchedPath| async move { path.as_str().to_owned() }), + ), + ); + + let client = TestClient::new(app); + + let res = client.get("/foo/bar").send().await; + assert_eq!(res.text().await, "/:a/:b"); + } } diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 563d27d2f5..a4a5258558 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -98,6 +98,7 @@ impl fmt::Debug for Router { } pub(crate) const NEST_TAIL_PARAM: &str = "__private__axum_nest_tail_param"; +const NEST_TAIL_PARAM_CAPTURE: &str = "/*__private__axum_nest_tail_param"; impl Router where @@ -416,8 +417,24 @@ where #[cfg(feature = "matched-path")] if let Some(matched_path) = self.node.route_id_to_path.get(&id) { - req.extensions_mut() - .insert(crate::extract::MatchedPath(Arc::clone(matched_path))); + use crate::extract::MatchedPath; + + let matched_path = if let Some(previous) = req.extensions_mut().get::() { + // a previous `MatchedPath` might exist if we're inside a nested Router + let previous = if let Some(previous) = + previous.as_str().strip_suffix(NEST_TAIL_PARAM_CAPTURE) + { + previous + } else { + previous.as_str() + }; + + let matched_path = format!("{}{}", previous, matched_path); + matched_path.into() + } else { + Arc::clone(matched_path) + }; + req.extensions_mut().insert(MatchedPath(matched_path)); } else { #[cfg(debug_assertions)] panic!("should always have a matched path for a route id"); diff --git a/axum/src/routing/tests/nest.rs b/axum/src/routing/tests/nest.rs index 6c2a6c6737..7113d2b75c 100644 --- a/axum/src/routing/tests/nest.rs +++ b/axum/src/routing/tests/nest.rs @@ -414,7 +414,7 @@ nested_route_test!(nest_3, nest = "", route = "/a/", expected = "/a/"); nested_route_test!(nest_4, nest = "/", route = "/", expected = "/"); nested_route_test!(nest_5, nest = "/", route = "/a", expected = "/a"); nested_route_test!(nest_6, nest = "/", route = "/a/", expected = "/a/"); -nested_route_test!(nest_7, nest = "/a/", route = "/", expected = "/a"); +nested_route_test!(nest_7, nest = "/a", route = "/", expected = "/a"); nested_route_test!(nest_8, nest = "/a", route = "/a", expected = "/a/a"); nested_route_test!(nest_9, nest = "/a", route = "/a/", expected = "/a/a/"); nested_route_test!(nest_11, nest = "/a/", route = "/", expected = "/a/"); From bbf1852c56dd8fa8ccba3deab7109597ce567a74 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Thu, 30 Jun 2022 00:26:49 +0200 Subject: [PATCH 09/17] Apply suggestions from code review Co-authored-by: Jonas Platte --- axum/src/docs/routing/nest.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/axum/src/docs/routing/nest.md b/axum/src/docs/routing/nest.md index 4ce0ad3379..b3199ff046 100644 --- a/axum/src/docs/routing/nest.md +++ b/axum/src/docs/routing/nest.md @@ -106,15 +106,15 @@ async fn fallback() {} let app = Router::new() .nest("/api", nested_router) .nest_service("/assets", nested_service) - // the fallback is not called for request starting with `/bar` but will be - // called for requests starting with `/foo` if `nested_router` doesn't have + // the fallback is not called for request starting with `/assets` but will be + // called for requests starting with `/api` if `nested_router` doesn't have // a matching route .fallback(fallback.into_service()); # let _: Router = app; ``` Note that you would normally use [`tower_http::services::ServeDir`] for serving -static files and thus not calling `nest_service` with a `Router`. +static files and thus not call `nest_service` with a `Router`. # Panics From 581b0d9a19668ba757e5f96f3c47bf5156bc9504 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Mon, 4 Jul 2022 18:52:43 +0200 Subject: [PATCH 10/17] adjust docs for fallbacks --- axum/src/docs/routing/nest.md | 44 +++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/axum/src/docs/routing/nest.md b/axum/src/docs/routing/nest.md index b3199ff046..7f2d5486d4 100644 --- a/axum/src/docs/routing/nest.md +++ b/axum/src/docs/routing/nest.md @@ -91,18 +91,20 @@ let app = Router::new() # Differences between `nest` and `nest_service` When [fallbacks] are called differs between `nest` and `nested_service`. Routers -nested with `nest` will delegate to the fallback if they don't have a matching -route, whereas `nested_service` will not. +nested with `nest` will delegate to the outer router's fallback if they don't +have a matching route, whereas `nested_service` will not. ```rust -use axum::{Router, routing::{get, any}, handler::Handler}; +use axum::{ + Router, + routing::get, + handler::Handler, +}; let nested_router = Router::new().route("/users", get(|| async {})); let nested_service = Router::new().route("/app.js", get(|| async {})); -async fn fallback() {} - let app = Router::new() .nest("/api", nested_router) .nest_service("/assets", nested_service) @@ -111,10 +113,38 @@ let app = Router::new() // a matching route .fallback(fallback.into_service()); # let _: Router = app; + +async fn fallback() {} ``` -Note that you would normally use [`tower_http::services::ServeDir`] for serving -static files and thus not call `nest_service` with a `Router`. +You can still add fallbacks explicitly to the inner router: + +```rust +use axum::{ + Router, + routing::get, + handler::Handler, +}; + +let nested_service = Router::new() + .route("/app.js", get(|| async {})) + .fallback(nested_service_fallback.into_service()); + +let app = Router::new() + .nest_service("/assets", nested_service) + .fallback(outer_router_fallback.into_service()); +# let _: Router = app; + +// this handler is used for `nested_service` +async fn nested_service_fallback() { + // .. +} + +// this handler is used for the outer router +async fn outer_router_fallback() { + // ... +} +``` # Panics From f18df8a24ec4075b7529f12ccdebb093d9824802 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Tue, 26 Jul 2022 12:42:48 +0200 Subject: [PATCH 11/17] Always nest services as opaque --- axum-extra/CHANGELOG.md | 5 +- axum-extra/src/routing/resource.rs | 50 +----------- axum-extra/src/routing/spa.rs | 2 +- axum/CHANGELOG.md | 6 +- axum/src/docs/routing/nest.md | 70 ++++++++--------- axum/src/docs/routing/nest_service.md | 40 ---------- axum/src/extract/matched_path.rs | 22 +++--- axum/src/routing/mod.rs | 62 ++------------- axum/src/routing/tests/merge.rs | 4 +- axum/src/routing/tests/mod.rs | 32 -------- axum/src/routing/tests/nest.rs | 108 ++++++++++++++++---------- 11 files changed, 127 insertions(+), 274 deletions(-) delete mode 100644 axum/src/docs/routing/nest_service.md diff --git a/axum-extra/CHANGELOG.md b/axum-extra/CHANGELOG.md index 0f3dd4bd06..51ba6f8c18 100644 --- a/axum-extra/CHANGELOG.md +++ b/axum-extra/CHANGELOG.md @@ -9,8 +9,9 @@ and this project adheres to [Semantic Versioning]. - **added:** Add `RouterExt::route_with_tsr` for adding routes with an additional "trailing slash redirect" route ([#1119]) -- **breaking:** `Resource::nest` and `Resource::nest_collection` now only - accepts `Router`s ([#1086]) +- **breaking:** `Resource::nest` and `Resource::nest_collection` has been + removed. You can instead convert the `Resource` into a `Router` and + add additional routes as necessary ([#1086]) [#1086]: https://github.com/tokio-rs/axum/pull/1086 [#1119]: https://github.com/tokio-rs/axum/pull/1119 diff --git a/axum-extra/src/routing/resource.rs b/axum-extra/src/routing/resource.rs index 720c0407d7..d4917213c0 100644 --- a/axum-extra/src/routing/resource.rs +++ b/axum-extra/src/routing/resource.rs @@ -31,18 +31,7 @@ use tower_service::Service; /// // `PUT or PATCH /users/:users_id` /// .update(|Path(user_id): Path| async {}) /// // `DELETE /users/:users_id` -/// .destroy(|Path(user_id): Path| async {}) -/// // Nest another router at the "member level" -/// // This defines a route for `GET /users/:users_id/tweets` -/// .nest(Router::new().route( -/// "/tweets", -/// get(|Path(user_id): Path| async {}), -/// )) -/// // Nest another router at the "collection level" -/// // This defines a route for `GET /users/featured` -/// .nest_collection( -/// Router::new().route("/featured", get(|| async {})), -/// ); +/// .destroy(|Path(user_id): Path| async {}); /// /// let app = Router::new().merge(users); /// # let _: Router = app; @@ -137,24 +126,6 @@ where self.route(&path, delete(handler)) } - /// Nest another router at the "member level". - /// - /// The routes will be nested at `/{resource_name}/:{resource_name}_id`. - pub fn nest(mut self, router: Router) -> Self { - let path = self.show_update_destroy_path(); - self.router = self.router.nest(&path, router); - self - } - - /// Nest another router at the "collection level". - /// - /// The routes will be nested at `/{resource_name}`. - pub fn nest_collection(mut self, router: Router) -> Self { - let path = self.index_create_path(); - self.router = self.router.nest(&path, router); - self - } - fn index_create_path(&self) -> String { format!("/{}", self.name) } @@ -195,14 +166,7 @@ mod tests { .show(|Path(id): Path| async move { format!("users#show id={}", id) }) .edit(|Path(id): Path| async move { format!("users#edit id={}", id) }) .update(|Path(id): Path| async move { format!("users#update id={}", id) }) - .destroy(|Path(id): Path| async move { format!("users#destroy id={}", id) }) - .nest(Router::new().route( - "/tweets", - get(|Path(id): Path| async move { format!("users#tweets id={}", id) }), - )) - .nest_collection( - Router::new().route("/featured", get(|| async move { "users#featured" })), - ); + .destroy(|Path(id): Path| async move { format!("users#destroy id={}", id) }); let mut app = Router::new().merge(users); @@ -245,16 +209,6 @@ mod tests { call_route(&mut app, Method::DELETE, "/users/1").await, "users#destroy id=1" ); - - assert_eq!( - call_route(&mut app, Method::GET, "/users/1/tweets").await, - "users#tweets id=1" - ); - - assert_eq!( - call_route(&mut app, Method::GET, "/users/featured").await, - "users#featured" - ); } async fn call_route(app: &mut Router, method: Method, uri: &str) -> String { diff --git a/axum-extra/src/routing/spa.rs b/axum-extra/src/routing/spa.rs index f8e3d0b0ea..594b15c237 100644 --- a/axum-extra/src/routing/spa.rs +++ b/axum-extra/src/routing/spa.rs @@ -161,7 +161,7 @@ where .handle_error(spa.handle_error.clone()); Router::new() - .nest_service(&spa.paths.assets_path, assets_service) + .nest(&spa.paths.assets_path, assets_service) .fallback( get_service(ServeFile::new(&spa.paths.index_file)).handle_error(spa.handle_error), ) diff --git a/axum/CHANGELOG.md b/axum/CHANGELOG.md index 54d5ee9a25..5aa4efb929 100644 --- a/axum/CHANGELOG.md +++ b/axum/CHANGELOG.md @@ -11,10 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Use `axum::middleware::from_extractor` instead ([#1077]) - **breaking:** Allow `Error: Into` for `Route::{layer, route_layer}` ([#924]) - **breaking:** `MethodRouter` now panics on overlapping routes ([#1102]) -- **breaking:** `Router::nest` now only accepts `Router`s. Use - `Router::nest_service` to nest opaque services ([#1086]) -- **added:** Add `Router::nest_service` for nesting opaque services. Use this to - nest services like `tower::services::ServeDir` ([#1086]) +- **breaking:** Nested `Router`s will no longer delegate to the outer `Router`'s + fallback. Instead you must explicitly set a fallback on the inner `Router` ([#1086]) - **breaking:** The request `/foo/` no longer matches `/foo/*rest`. If you want to match `/foo/` you have to add a route specifically for that ([#1086]) - **breaking:** Path params for wildcard routes no longer include the prefix diff --git a/axum/src/docs/routing/nest.md b/axum/src/docs/routing/nest.md index 7f2d5486d4..6851c79829 100644 --- a/axum/src/docs/routing/nest.md +++ b/axum/src/docs/routing/nest.md @@ -88,62 +88,56 @@ let app = Router::new() # }; ``` -# Differences between `nest` and `nest_service` +# Fallbacks -When [fallbacks] are called differs between `nest` and `nested_service`. Routers -nested with `nest` will delegate to the outer router's fallback if they don't -have a matching route, whereas `nested_service` will not. +If a request matches the prefix but the nested router doesn't have matching +route the outer fallback will _not_ be called: ```rust -use axum::{ - Router, - routing::get, - handler::Handler, -}; +use axum::{routing::get, http::StatusCode, handler::Handler, Router}; -let nested_router = Router::new().route("/users", get(|| async {})); +async fn fallback() -> (StatusCode, &'static str) { + (StatusCode::NOT_FOUND, "Not Found") +} -let nested_service = Router::new().route("/app.js", get(|| async {})); +let api_routes = Router::new().nest("/users", get(|| async {})); let app = Router::new() - .nest("/api", nested_router) - .nest_service("/assets", nested_service) - // the fallback is not called for request starting with `/assets` but will be - // called for requests starting with `/api` if `nested_router` doesn't have - // a matching route + .nest("/api", api_routes) .fallback(fallback.into_service()); # let _: Router = app; - -async fn fallback() {} ``` -You can still add fallbacks explicitly to the inner router: +Here requests like `GET /api/not-found` will go into `api_routes` and then to +the fallback of `api_routes` which will return an empty `404 Not Found` +response. The outer fallback declared on `app` will _not_ be called. -```rust -use axum::{ - Router, - routing::get, - handler::Handler, -}; +Think of nested services as swallowing requests that matches the prefix and +not falling back to outer router even if they don't have a matching route. -let nested_service = Router::new() - .route("/app.js", get(|| async {})) - .fallback(nested_service_fallback.into_service()); +You can still add separate fallbacks to nested routers: -let app = Router::new() - .nest_service("/assets", nested_service) - .fallback(outer_router_fallback.into_service()); -# let _: Router = app; +```rust +use axum::{routing::get, http::StatusCode, handler::Handler, Json, Router}; +use serde_json::{json, Value}; -// this handler is used for `nested_service` -async fn nested_service_fallback() { - // .. +async fn fallback() -> (StatusCode, &'static str) { + (StatusCode::NOT_FOUND, "Not Found") } -// this handler is used for the outer router -async fn outer_router_fallback() { - // ... +async fn api_fallback() -> (StatusCode, Json) { + (StatusCode::NOT_FOUND, Json(json!({ "error": "Not Found" }))) } + +let api_routes = Router::new() + .nest("/users", get(|| async {})) + // add dedicated fallback for requests starting with `/api` + .fallback(api_fallback.into_service()); + +let app = Router::new() + .nest("/api", api_routes) + .fallback(fallback.into_service()); +# let _: Router = app; ``` # Panics diff --git a/axum/src/docs/routing/nest_service.md b/axum/src/docs/routing/nest_service.md deleted file mode 100644 index bb61bcea13..0000000000 --- a/axum/src/docs/routing/nest_service.md +++ /dev/null @@ -1,40 +0,0 @@ -Nest a [`Service`] at some path. - -`nest_service` behaves in the same way as `nest` in terms of - -- [How the URI changes](#how-the-uri-changes) -- [Captures from outer routes](#captures-from-outer-routes) -- [Differences to wildcard routes](#differences-to-wildcard-routes) - -But differs with regards to [fallbacks]. See ["Differences between `nest` and -`nest_service`"](#differences-between-nest-and-nest_service) for more details. - -# Example - -`nest_service` can for example be used with [`tower_http::services::ServeDir`] -to serve static files from a directory: - -```rust -use axum::{ - Router, - routing::get_service, - http::StatusCode, - error_handling::HandleErrorLayer, -}; -use std::{io, convert::Infallible}; -use tower_http::services::ServeDir; - -// Serves files inside the `public` directory at `GET /assets/*` -let serve_dir_service = get_service(ServeDir::new("public")) - .handle_error(|error: io::Error| async move { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Unhandled internal error: {}", error), - ) - }); - -let app = Router::new().nest_service("/assets", serve_dir_service); -# let _: Router = app; -``` - -[fallbacks]: Router::fallback diff --git a/axum/src/extract/matched_path.rs b/axum/src/extract/matched_path.rs index 192983cd68..fbda634d83 100644 --- a/axum/src/extract/matched_path.rs +++ b/axum/src/extract/matched_path.rs @@ -85,8 +85,9 @@ where mod tests { use super::*; use crate::{extract::Extension, handler::Handler, routing::get, test_helpers::*, Router}; - use http::Request; + use http::{Request, StatusCode}; use std::task::{Context, Poll}; + use tower::layer::layer_fn; use tower_service::Service; #[derive(Clone)] @@ -139,7 +140,7 @@ mod tests { ) } - let app = Router::new() + let app: _ = Router::new() .route( "/:key", get(|path: MatchedPath| async move { path.as_str().to_owned() }), @@ -147,17 +148,20 @@ mod tests { .nest("/api", api) .nest( "/public", - Router::new().route("/assets/*path", get(handler)), + Router::new() + .route("/assets/*path", get(handler)) + // have to set the middleware here since otherwise the + // matched path is just `/public/*` since we're nesting + // this router + .layer(layer_fn(SetMatchedPathExtension)), ) - .nest_service("/foo", handler.into_service()) - .layer(tower::layer::layer_fn(SetMatchedPathExtension)); + .nest("/foo", handler.into_service()) + .layer(layer_fn(SetMatchedPathExtension)); let client = TestClient::new(app); - // we cannot call `/foo` because `nest_service("/foo", _)` registers routes - // for `/foo/*rest` and `/foo` let res = client.get("/public").send().await; - assert_eq!(res.text().await, "/:key"); + assert_eq!(res.status(), StatusCode::NOT_FOUND); let res = client.get("/api/users/123").send().await; assert_eq!(res.text().await, "/api/users/:id"); @@ -181,7 +185,7 @@ mod tests { #[tokio::test] async fn nested_opaque_routers_append_to_matched_path() { - let app = Router::new().nest_service( + let app = Router::new().nest( "/:a", Router::new().route( "/:b", diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 4c0a07b571..a4e98a092c 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -5,21 +5,19 @@ use crate::{ body::{boxed, Body, Bytes, HttpBody}, extract::connect_info::IntoMakeServiceWithConnectInfo, response::Response, - routing::strip_prefix::StripPrefix, util::try_downcast, BoxError, }; use http::Request; use matchit::MatchError; use std::{ - borrow::Cow, collections::HashMap, convert::Infallible, fmt, sync::Arc, task::{Context, Poll}, }; -use tower::{layer::layer_fn, ServiceBuilder}; +use tower::ServiceBuilder; use tower_http::map_response_body::MapResponseBodyLayer; use tower_layer::Layer; use tower_service::Service; @@ -161,7 +159,7 @@ where let mut node = Arc::try_unwrap(Arc::clone(&self.node)).unwrap_or_else(|node| (*node).clone()); if let Err(err) = node.insert(path, id) { - panic!("Invalid route: {}", err); + panic!("Invalid route {:?}. {}", path, err); } self.node = Arc::new(node); @@ -171,55 +169,7 @@ where } #[doc = include_str!("../docs/routing/nest.md")] - pub fn nest(mut self, mut path: &str, router: Router) -> Self { - if path.is_empty() { - // nesting at `""` and `"/"` should mean the same thing - path = "/"; - } - - if path.contains('*') { - panic!("Invalid route: nested routes cannot contain wildcards (*)"); - } - - let prefix = path; - - let Router { - mut routes, - node, - fallback, - } = router; - - if let Fallback::Custom(_) = fallback { - panic!("Cannot nest `Router`s that has a fallback"); - } - - for (id, nested_path) in &node.route_id_to_path { - let route = routes.remove(id).unwrap(); - let full_path: Cow = if &**nested_path == "/" { - path.into() - } else if path == "/" { - (&**nested_path).into() - } else if let Some(path) = path.strip_suffix('/') { - format!("{}{}", path, nested_path).into() - } else { - format!("{}{}", path, nested_path).into() - }; - self = match route { - Endpoint::MethodRouter(method_router) => self.route( - &full_path, - method_router.layer(layer_fn(|s| StripPrefix::new(s, prefix))), - ), - Endpoint::Route(route) => self.route(&full_path, StripPrefix::new(route, prefix)), - }; - } - - debug_assert!(routes.is_empty()); - - self - } - - #[doc = include_str!("../docs/routing/nest_service.md")] - pub fn nest_service(mut self, mut path: &str, svc: T) -> Self + pub fn nest(mut self, mut path: &str, svc: T) -> Self where T: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, T::Future: Send + 'static, @@ -248,8 +198,10 @@ where // prefix itself. Otherwise if you were to nest at `/foo` then `/foo` itself // wouldn't match, which it should self = self.route(prefix, svc.clone()); - // same goes for `/foo/`, that should also match - self = self.route(&format!("{}/", prefix), svc); + if !prefix.ends_with('/') { + // same goes for `/foo/`, that should also match + self = self.route(&format!("{}/", prefix), svc); + } self } diff --git a/axum/src/routing/tests/merge.rs b/axum/src/routing/tests/merge.rs index 58af01511b..afb0bcb94f 100644 --- a/axum/src/routing/tests/merge.rs +++ b/axum/src/routing/tests/merge.rs @@ -240,7 +240,7 @@ async fn all_the_uris( #[tokio::test] async fn nesting_and_seeing_the_right_uri() { - let one = Router::new().nest("/foo", Router::new().route("/bar", get(all_the_uris))); + let one = Router::new().nest("/foo/", Router::new().route("/bar", get(all_the_uris))); let two = Router::new().route("/foo", get(all_the_uris)); let client = TestClient::new(one.merge(two)); @@ -271,7 +271,7 @@ async fn nesting_and_seeing_the_right_uri() { #[tokio::test] async fn nesting_and_seeing_the_right_uri_at_more_levels_of_nesting() { let one = Router::new().nest( - "/foo", + "/foo/", Router::new().nest("/bar", Router::new().route("/baz", get(all_the_uris))), ); let two = Router::new().route("/foo", get(all_the_uris)); diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index ab61ea49aa..d25bfccf2f 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -493,29 +493,6 @@ async fn different_methods_added_in_different_routes() { assert_eq!(body, "POST"); } -#[tokio::test] -async fn different_methods_added_in_different_routes_deeply_nested() { - let app = Router::new() - .route("/foo/bar/baz", get(|| async { "GET" })) - .nest( - "/foo", - Router::new().nest( - "/bar", - Router::new().route("/baz", post(|| async { "POST" })), - ), - ); - - let client = TestClient::new(app); - - let res = client.get("/foo/bar/baz").send().await; - let body = res.text().await; - assert_eq!(body, "GET"); - - let res = client.post("/foo/bar/baz").send().await; - let body = res.text().await; - assert_eq!(body, "POST"); -} - #[tokio::test] #[should_panic(expected = "Cannot merge two `Router`s that both have a fallback")] async fn merging_routers_with_fallbacks_panics() { @@ -525,15 +502,6 @@ async fn merging_routers_with_fallbacks_panics() { TestClient::new(one.merge(two)); } -#[tokio::test] -#[should_panic(expected = "Cannot nest `Router`s that has a fallback")] -async fn nesting_router_with_fallbacks_panics() { - async fn fallback() {} - let one = Router::new().fallback(fallback.into_service()); - let app = Router::new().nest("/", one); - TestClient::new(app); -} - #[tokio::test] async fn merging_routers_with_same_paths_but_different_methods() { let one = Router::new().route("/", get(|| async { "GET" })); diff --git a/axum/src/routing/tests/nest.rs b/axum/src/routing/tests/nest.rs index 7113d2b75c..c90595adb4 100644 --- a/axum/src/routing/tests/nest.rs +++ b/axum/src/routing/tests/nest.rs @@ -14,7 +14,6 @@ async fn nesting_apps() { "/users/:id", get( |params: extract::Path>| async move { - dbg!(¶ms); format!( "{}: users#show ({})", params.get("version").unwrap(), @@ -114,7 +113,7 @@ async fn nesting_router_at_empty_path() { #[tokio::test] async fn nesting_handler_at_root() { - let app = Router::new().nest_service("/", get(|uri: Uri| async move { uri.to_string() })); + let app = Router::new().nest("/", get(|uri: Uri| async move { uri.to_string() })); let client = TestClient::new(app); @@ -202,7 +201,7 @@ async fn nested_service_sees_stripped_uri() { #[tokio::test] async fn nest_static_file_server() { - let app = Router::new().nest_service( + let app = Router::new().nest( "/static", get_service(ServeDir::new(".")).handle_error(|error| async move { ( @@ -236,38 +235,12 @@ async fn nested_multiple_routes() { assert_eq!(client.get("/api/teams").send().await.text().await, "teams"); } -#[tokio::test] -async fn nested_with_other_route_also_matching_with_route_first() { - let app = Router::new().route("/api", get(|| async { "api" })).nest( - "/api", - Router::new() - .route("/users", get(|| async { "users" })) - .route("/teams", get(|| async { "teams" })), - ); - - let client = TestClient::new(app); - - assert_eq!(client.get("/api").send().await.text().await, "api"); - assert_eq!(client.get("/api/users").send().await.text().await, "users"); - assert_eq!(client.get("/api/teams").send().await.text().await, "teams"); -} - -#[tokio::test] -async fn nested_with_other_route_also_matching_with_route_last() { - let app = Router::new() - .nest( - "/api", - Router::new() - .route("/users", get(|| async { "users" })) - .route("/teams", get(|| async { "teams" })), - ) - .route("/api", get(|| async { "api" })); - - let client = TestClient::new(app); - - assert_eq!(client.get("/api").send().await.text().await, "api"); - assert_eq!(client.get("/api/users").send().await.text().await, "users"); - assert_eq!(client.get("/api/teams").send().await.text().await, "teams"); +#[test] +#[should_panic = "Invalid route \"/\". insertion failed due to conflict with previously registered route: /*__private__axum_nest_tail_param"] +fn nested_at_root_with_other_routes() { + let _: Router = Router::new() + .nest("/", Router::new().route("/users", get(|| async {}))) + .route("/", get(|| async {})); } #[tokio::test] @@ -354,7 +327,7 @@ async fn nest_at_capture() { ) .boxed_clone(); - let app = Router::new().nest_service("/:a", api_routes); + let app = Router::new().nest("/:a", api_routes); let client = TestClient::new(app); @@ -365,7 +338,7 @@ async fn nest_at_capture() { #[tokio::test] async fn nest_with_and_without_trailing() { - let app = Router::new().nest_service("/foo", get(|| async {})); + let app = Router::new().nest("/foo", get(|| async {})); let client = TestClient::new(app); @@ -379,6 +352,61 @@ async fn nest_with_and_without_trailing() { assert_eq!(res.status(), StatusCode::OK); } +#[tokio::test] +async fn doesnt_call_outer_fallback() { + let app = Router::new() + .nest("/foo", Router::new().route("/", get(|| async {}))) + .fallback((|| async { (StatusCode::NOT_FOUND, "outer fallback") }).into_service()); + + let client = TestClient::new(app); + + let res = client.get("/foo").send().await; + assert_eq!(res.status(), StatusCode::OK); + + let res = client.get("/foo/not-found").send().await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + // the default fallback returns an empty body + assert_eq!(res.text().await, ""); +} + +#[tokio::test] +async fn nesting_with_root_inner_router() { + let app = Router::new().nest( + "/foo", + Router::new().route("/", get(|| async { "inner route" })), + ); + + let client = TestClient::new(app); + + // `/foo/` does match the `/foo` prefix and the remaining path is technically + // empty, which is the same as `/` which matches `.route("/", _)` + let res = client.get("/foo").send().await; + assert_eq!(res.status(), StatusCode::OK); + + // `/foo/` does match the `/foo` prefix and the remaining path is `/` + // which matches `.route("/", _)` + let res = client.get("/foo/").send().await; + assert_eq!(res.status(), StatusCode::OK); +} + +#[tokio::test] +async fn fallback_on_inner() { + let app = Router::new() + .nest( + "/foo", + Router::new() + .route("/", get(|| async {})) + .fallback((|| async { (StatusCode::NOT_FOUND, "inner fallback") }).into_service()), + ) + .fallback((|| async { (StatusCode::NOT_FOUND, "outer fallback") }).into_service()); + + let client = TestClient::new(app); + + let res = client.get("/foo/not-found").send().await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + assert_eq!(res.text().await, "inner fallback"); +} + macro_rules! nested_route_test { ( $name:ident, @@ -397,12 +425,6 @@ macro_rules! nested_route_test { let res = client.get($expected_path).send().await; let status = res.status(); assert_eq!(status, StatusCode::OK, "Router"); - - let inner = Router::new().route($route_path, get(|| async {})); - let app = Router::new().nest_service($nested_path, inner); - let client = TestClient::new(app); - let res = client.get(dbg!($expected_path)).send().await; - assert_eq!(res.status(), StatusCode::OK, "opaque"); } }; } From b166548c0ea79098ecb7f6eab1c490ad869005ca Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Tue, 26 Jul 2022 13:03:06 +0200 Subject: [PATCH 12/17] fix old docs reference --- axum/src/docs/routing/nest.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/axum/src/docs/routing/nest.md b/axum/src/docs/routing/nest.md index 6851c79829..01f16367f4 100644 --- a/axum/src/docs/routing/nest.md +++ b/axum/src/docs/routing/nest.md @@ -1,4 +1,4 @@ -Nest a router at some path. +Nest a [`Service`] at some path. This allows you to break your application into smaller pieces and compose them together. From 4630476b137167cc2c2b621df379e4335f954c31 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Tue, 26 Jul 2022 13:03:20 +0200 Subject: [PATCH 13/17] more tests for `MatchedPath` with nested handlers --- axum/src/extract/matched_path.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/axum/src/extract/matched_path.rs b/axum/src/extract/matched_path.rs index fbda634d83..7eed9097a1 100644 --- a/axum/src/extract/matched_path.rs +++ b/axum/src/extract/matched_path.rs @@ -160,18 +160,25 @@ mod tests { let client = TestClient::new(app); - let res = client.get("/public").send().await; - assert_eq!(res.status(), StatusCode::NOT_FOUND); - let res = client.get("/api/users/123").send().await; assert_eq!(res.text().await, "/api/users/:id"); + // the router nested at `/public` doesn't handle `/` + let res = client.get("/public").send().await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + let res = client.get("/public/assets/css/style.css").send().await; assert_eq!( res.text().await, "extractor = /public/assets/*path, middleware = /public/assets/*path" ); + let res = client.get("/foo").send().await; + assert_eq!(res.text().await, "extractor = /foo, middleware = /foo"); + + let res = client.get("/foo/").send().await; + assert_eq!(res.text().await, "extractor = /foo/, middleware = /foo/"); + let res = client.get("/foo/bar/baz").send().await; assert_eq!( res.text().await, From 32e67aab3306fe464d45e2a731689aa2f49e2179 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Thu, 11 Aug 2022 11:40:51 +0200 Subject: [PATCH 14/17] minor clean up --- axum/src/extract/matched_path.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/axum/src/extract/matched_path.rs b/axum/src/extract/matched_path.rs index 7eed9097a1..d0cb01afd6 100644 --- a/axum/src/extract/matched_path.rs +++ b/axum/src/extract/matched_path.rs @@ -140,7 +140,7 @@ mod tests { ) } - let app: _ = Router::new() + let app = Router::new() .route( "/:key", get(|path: MatchedPath| async move { path.as_str().to_owned() }), From be864eacada61de771c00d3a524e673eef79b6a2 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Thu, 11 Aug 2022 11:43:30 +0200 Subject: [PATCH 15/17] use identifier captures in format strings --- axum/src/routing/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 6f5092ef03..01db8e55ab 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -171,7 +171,7 @@ where let mut node = Arc::try_unwrap(Arc::clone(&self.node)).unwrap_or_else(|node| (*node).clone()); if let Err(err) = node.insert(path, id) { - panic!("Invalid route {:?}. {}", path, err); + panic!("Invalid route {path:?}: {err}"); } self.node = Arc::new(node); } @@ -195,9 +195,9 @@ where let prefix = path; let path = if path.ends_with('/') { - format!("{}*{}", path, NEST_TAIL_PARAM) + format!("{path}*{NEST_TAIL_PARAM}") } else { - format!("{}/*{}", path, NEST_TAIL_PARAM) + format!("{path}/*{NEST_TAIL_PARAM}") }; let svc = strip_prefix::StripPrefix::new(svc, prefix); @@ -209,7 +209,7 @@ where self = self.route(prefix, svc.clone()); if !prefix.ends_with('/') { // same goes for `/foo/`, that should also match - self = self.route(&format!("{}/", prefix), svc); + self = self.route(&format!("{prefix}/"), svc); } self From 171171d592eaa20f1f682372b7696be24deeaaa1 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Thu, 11 Aug 2022 11:50:19 +0200 Subject: [PATCH 16/17] Apply suggestions from code review Co-authored-by: Jonas Platte --- axum/src/docs/routing/nest.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/axum/src/docs/routing/nest.md b/axum/src/docs/routing/nest.md index 01f16367f4..c1d14907ef 100644 --- a/axum/src/docs/routing/nest.md +++ b/axum/src/docs/routing/nest.md @@ -90,8 +90,8 @@ let app = Router::new() # Fallbacks -If a request matches the prefix but the nested router doesn't have matching -route the outer fallback will _not_ be called: +When nesting a router, if a request matches the prefix but the nested router doesn't have a matching +route, the outer fallback will _not_ be called: ```rust use axum::{routing::get, http::StatusCode, handler::Handler, Router}; From 35c616d1e80274091495ce8a92966db69df1ae12 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Thu, 11 Aug 2022 12:12:29 +0200 Subject: [PATCH 17/17] fix test --- axum/src/routing/tests/nest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/axum/src/routing/tests/nest.rs b/axum/src/routing/tests/nest.rs index c90595adb4..cabadaf545 100644 --- a/axum/src/routing/tests/nest.rs +++ b/axum/src/routing/tests/nest.rs @@ -236,7 +236,7 @@ async fn nested_multiple_routes() { } #[test] -#[should_panic = "Invalid route \"/\". insertion failed due to conflict with previously registered route: /*__private__axum_nest_tail_param"] +#[should_panic = "Invalid route \"/\": insertion failed due to conflict with previously registered route: /*__private__axum_nest_tail_param"] fn nested_at_root_with_other_routes() { let _: Router = Router::new() .nest("/", Router::new().route("/users", get(|| async {})))