tokio-rs/axum
1
0
Fork 0
mirror of https://github.com/tokio-rs/axum.git synced 2026-08-28 00:00:20 +02:00
Code Issues Packages Projects Releases Wiki Activity

Break Router::nest into nest and nest_service methods

Browse Source
This commit is contained in:
David Pedersen
2022-06-11 13:19:27 +02:00
parent dbdbd0165e
commit 2ddd0a230c
9 changed files with 136 additions and 238 deletions
Download Patch File Download Diff File Expand all files Collapse all files
axum-extra/src/routing/resource.rs
+2 -2
View File
@@ -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
}
axum-extra/src/routing/spa.rs
+1 -1
View File
@@ -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),
)
axum/CHANGELOG.md
+11
View File
@@ -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
axum/Cargo.toml
+3 -1
View File
@@ -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"
axum/src/extract/matched_path.rs
+4 -2
View File
@@ -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;
axum/src/extract/path/mod.rs
+2 -2
View File
@@ -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]
axum/src/routing/mod.rs
+67 -98
View File
@@ -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<B = Body> {
routes: HashMap<RouteId, Endpoint<B>>,
node: Node,
fallback: Fallback<B>,
nested_at_root: bool,
}
impl<B> Clone for Router<B> {
@@ -75,7 +74,6 @@ impl<B> Clone for Router<B> {
routes: self.routes.clone(),
node: self.node.clone(),
fallback: self.fallback.clone(),
nested_at_root: self.nested_at_root,
}
}
}
@@ -95,13 +93,11 @@ impl<B> fmt::Debug for Router<B> {
.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<B> Router<B>
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,8 +166,57 @@ where
self
}
// TODO(david): update docs
#[doc = include_str!("../docs/routing/nest.md")]
pub fn nest<T>(mut self, mut path: &str, svc: T) -> Self
pub fn nest(mut self, mut path: &str, router: Router<B>) -> 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<str> = 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
}
/// TODO
pub fn nest_service<T>(mut self, mut path: &str, svc: T) -> Self
where
T: Service<Request<B>, Response = Response, Error = Infallible> + Clone + Send + 'static,
T::Future: Send + 'static,
@@ -188,64 +232,21 @@ where
let prefix = path;
if path == "/" {
self.nested_at_root = true;
}
let path = if path.ends_with('/') {
format!("{}*{}", path, NEST_TAIL_PARAM)
} else {
format!("{}/*{}", path, NEST_TAIL_PARAM)
};
match try_downcast::<Router<B>, _>(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;
let svc = strip_prefix::StripPrefix::new(svc, prefix);
self = self.route(&path, svc.clone());
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<str> = 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());
}
// 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.route(&path, strip_prefix::StripPrefix::new(svc, prefix));
}
}
// `/*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::<MatchedPath>() {
// 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<B> Service<Request<B>> for Router<B>
axum/src/routing/tests/mod.rs
+16 -68
View File
@@ -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<Tree>| async move { Json(tree) }),
async fn wildcard_doesnt_match_just_trailing_slash() {
let app = Router::new().route(
"/x/*path",
get(|Path(path): Path<String>| 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::<Value>().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()
axum/src/routing/tests/nest.rs
+30 -64
View File
@@ -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/");