Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support fallbacks for nested routers #1161

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions axum-extra/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ 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])

[#1086]: https://github.com/tokio-rs/axum/pull/1086
[#1119]: https://github.com/tokio-rs/axum/pull/1119

# 0.3.5 (27. June, 2022)
Expand Down
20 changes: 6 additions & 14 deletions axum-extra/src/routing/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,29 +137,21 @@ 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<T>(mut self, svc: T) -> Self
where
T: Service<Request<B>, Response = Response, Error = Infallible> + Clone + Send + 'static,
T::Future: Send + 'static,
{
pub fn nest(mut self, router: Router<B>) -> Self {
let path = self.show_update_destroy_path();
self.router = self.router.nest(&path, svc);
self.router = self.router.nest(&path, router);
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<T>(mut self, svc: T) -> Self
where
T: Service<Request<B>, Response = Response, Error = Infallible> + Clone + Send + 'static,
T::Future: Send + 'static,
{
pub fn nest_collection(mut self, router: Router<B>) -> Self {
let path = self.index_create_path();
self.router = self.router.nest(&path, svc);
self.router = self.router.nest(&path, router);
self
}

Expand Down
2 changes: 1 addition & 1 deletion axum-extra/src/routing/spa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand Down
12 changes: 12 additions & 0 deletions axum/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ 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<Infallible>` 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:** 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
`/`. e.g. `/foo.js` will match `/*filepath` with a value of `foo.js`, _not_
`/foo.js` ([#1086])
- **fixed:** Routes like `/foo` and `/*rest` are no longer considered
overlapping. `/foo` will take priority ([#1086])
- **breaking:** Remove trailing slash redirects. Previously if you added a route
for `/foo`, axum would redirect calls to `/foo/` to `/foo` (or vice versa for
`/foo/`). That is no longer supported and such requests will now be sent to
Expand All @@ -20,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **added:** Support running extractors from `middleware::from_fn` functions ([#1088])

[#1077]: https://github.com/tokio-rs/axum/pull/1077
[#1086]: https://github.com/tokio-rs/axum/pull/1086
[#1088]: https://github.com/tokio-rs/axum/pull/1088
[#1102]: https://github.com/tokio-rs/axum/pull/1102
[#1119]: https://github.com/tokio-rs/axum/pull/1119
Expand Down
2 changes: 1 addition & 1 deletion axum/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ 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"
matchit = "0.6"
memchr = "2.4.1"
mime = "0.3.16"
percent-encoding = "2.1"
Expand Down
100 changes: 66 additions & 34 deletions axum/src/docs/routing/nest.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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("/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
Expand All @@ -103,18 +73,79 @@ 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("/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 outer router's fallback if they don't
have a matching route, whereas `nested_service` will not.
Comment on lines +93 to +95

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in #1086 (comment), these docs need to be updated to describe how nested routers handle fallbacks, as well as the following example (which uses nest_service to allow the nested router to have a fallback).

Comment on lines +93 to +95
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.
When [fallbacks] are called differs between `nest` and `nest_service`. Routers
nested with `nest` will delegate to the outer router's fallback if they don't
have a matching route, whereas `nest_service` will not.


```rust
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 {}));

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
.fallback(fallback.into_service());
Comment on lines +111 to +114
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be correct to add this? It's the semantics I would intuitively expect.

Suggested change
// 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());
// 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, or its own fallback service
.fallback(fallback.into_service());

# let _: Router = app;

async fn fallback() {}
```

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

- If the route overlaps with another route. See [`Router::route`]
Expand All @@ -125,3 +156,4 @@ for more details.
`Router` only allows a single fallback.

[`OriginalUri`]: crate::extract::OriginalUri
[fallbacks]: Router::fallback
40 changes: 40 additions & 0 deletions axum/src/docs/routing/nest_service.md
Original file line number Diff line number Diff line change
@@ -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`]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know where the link definition for this is, but we should make sure to link to a specific version of tower-http, given the example below is going to stop working with the next breaking release (due to io::Error => Infallible).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention is so somewhat coordinate the next release of axum and tower-http but yeah will need to be updated.

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
18 changes: 2 additions & 16 deletions axum/src/docs/routing/route.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -184,22 +186,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
Expand Down
44 changes: 42 additions & 2 deletions axum/src/extract/matched_path.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use crate::routing::NEST_TAIL_PARAM_CAPTURE;

use super::{rejection::*, FromRequest, RequestParts};
use async_trait::async_trait;
use http::Request;
use std::sync::Arc;

/// Access the path in the router that matches the request.
Expand Down Expand Up @@ -81,6 +84,25 @@ where
}
}

pub(crate) fn insert_matched_path<B>(matched_path: &Arc<str>, req: &mut Request<B>) {
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));
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -149,12 +171,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;
Expand All @@ -176,4 +200,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");
}
}
4 changes: 2 additions & 2 deletions axum/src/extract/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub mod ws;
mod content_length_limit;
mod host;
mod raw_query;
mod request_parts;
pub(crate) mod request_parts;

#[doc(inline)]
pub use axum_core::extract::{FromRequest, RequestParts};
Expand Down Expand Up @@ -41,7 +41,7 @@ pub use crate::Extension;
pub use crate::form::Form;

#[cfg(feature = "matched-path")]
mod matched_path;
pub(crate) mod matched_path;

#[cfg(feature = "matched-path")]
#[doc(inline)]
Expand Down
4 changes: 2 additions & 2 deletions axum/src/extract/path/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading