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

Update matchit and fix nesting inconsistencies #1086

Merged
merged 23 commits into from Aug 11, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
2ddd0a2
Break `Router::nest` into `nest` and `nest_service` methods
davidpdrsn Jun 11, 2022
23721f2
Merge branch 'main' into separate-nesting-opaque-services
davidpdrsn Jun 29, 2022
2c54bb5
fix doc tests
davidpdrsn Jun 29, 2022
014067e
update docs
davidpdrsn Jun 29, 2022
79daaf0
fix
davidpdrsn Jun 29, 2022
276a950
Only accept `Router` in `Resource::{nest, nest_collection}`
davidpdrsn Jun 29, 2022
b220778
update changelog
davidpdrsn Jun 29, 2022
fb5219f
fix docs
davidpdrsn Jun 29, 2022
657a089
fix `MatchedPath` with `Router`s nested with `nest_service`
davidpdrsn Jun 29, 2022
bbf1852
Apply suggestions from code review
davidpdrsn Jun 29, 2022
7ed35d2
Merge branch 'main' into separate-nesting-opaque-services
davidpdrsn Jun 29, 2022
ca95dd6
Merge branch 'main' into separate-nesting-opaque-services
davidpdrsn Jul 3, 2022
581b0d9
adjust docs for fallbacks
davidpdrsn Jul 4, 2022
f18df8a
Always nest services as opaque
davidpdrsn Jul 26, 2022
a8b0608
Merge branch 'main' into separate-nesting-opaque-services
davidpdrsn Jul 26, 2022
b166548
fix old docs reference
davidpdrsn Jul 26, 2022
4630476
more tests for `MatchedPath` with nested handlers
davidpdrsn Jul 26, 2022
f6db084
Merge branch 'main' into separate-nesting-opaque-services
davidpdrsn Jul 26, 2022
32e67aa
minor clean up
davidpdrsn Aug 11, 2022
4c39785
Merge branch 'main' into separate-nesting-opaque-services
davidpdrsn Aug 11, 2022
be864ea
use identifier captures in format strings
davidpdrsn Aug 11, 2022
171171d
Apply suggestions from code review
davidpdrsn Aug 11, 2022
35c616d
fix test
davidpdrsn Aug 11, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion axum-extra/CHANGELOG.md
Expand Up @@ -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])
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved

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

# 0.3.5 (27. June, 2022)

Expand Down
20 changes: 6 additions & 14 deletions axum-extra/src/routing/resource.rs
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
jplatte marked this conversation as resolved.
Show resolved Hide resolved
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
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
Expand Up @@ -11,8 +11,20 @@ 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
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved
`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])
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved
- **fixed:** Routes like `/foo` and `/*rest` are no longer considered
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

Expand Down
2 changes: 1 addition & 1 deletion axum/Cargo.toml
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
70 changes: 36 additions & 34 deletions 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.
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,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("/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
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved
// called for requests starting with `/foo` if `nested_router` doesn't have
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved
// 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`.
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved

# Panics

- If the route overlaps with another route. See [`Router::route`]
Expand All @@ -125,3 +126,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
@@ -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
18 changes: 2 additions & 16 deletions axum/src/docs/routing/route.md
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
22 changes: 20 additions & 2 deletions axum/src/extract/matched_path.rs
Expand Up @@ -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
davidpdrsn marked this conversation as resolved.
Show resolved Hide resolved
// 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 +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");
}
}
4 changes: 2 additions & 2 deletions axum/src/extract/path/mod.rs
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