-
Notifications
You must be signed in to change notification settings - Fork 1k
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
"matchit" based router #363
Conversation
cc @ibraheemdev 😊 |
047047f
to
c598fa3
Compare
👀 I've been meaning to fix the fact that routes like |
Probably yeah but I don't think its a big issue tbh. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing I've noticed that doesn't relate to a specific code location: There doesn't seem to be any #[should_panic]
tests for overlapping routes.
I'm adding those as docs tests right this moment. |
Have you thought about how #257 would interact with this, by the way? For that, overlapping routes would have to be allowedin some form. Personally I think one reasonable way to allow them would be by requiring users to explicitly "order" / "rank" the otherwise-overlapping routes, and trying them in the specified order. This would also allow both See also my comment on another issue where I mentioned how Rocket does this kind of thing. |
No I haven't thought about that 🤔 I think extractors being able to cause some kind of fallback is nice but also a bit worried that it can lead to complex routing setups that are hard to understand. Regardless I don't think this makes adding such a feature impossible since as you mention some sort of ranking could be added to matchit. So routes would only be considered overlapping if they had the same rank. I guess that could work. |
b611168
to
b778ee4
Compare
Co-authored-by: Jonas Platte <jplatte@users.noreply.github.com>
b778ee4
to
c6eb8cd
Compare
I think this is in a pretty decent state now. Will read through the changes during the weekend and hopefully merge :) |
In #363 I added `MaybeSharedNode` which makes `Router` faster to clone _if_ you're using `into_make_service`. However I would like instead like to find a solution that works even when you're not using `into_make_service`. Using an `Arc<RwLock<Node<_>>>` is probably the only solution but would like to experiment. For now I'm gonna rollback `MaybeSharedNode`. Changing it wont require user facing changes.
In #363 I added `MaybeSharedNode` which makes `Router` faster to clone _if_ you're using `into_make_service`. However I would like instead like to find a solution that works even when you're not using `into_make_service`. Using an `Arc<RwLock<Node<_>>>` is probably the only solution but would like to experiment. For now I'm gonna rollback `MaybeSharedNode`. Changing it wont require user facing changes.
- Overall: - **fixed:** All known compile time issues are resolved, including those with `boxed` and those introduced by Rust 1.56 ([#404]) - **breaking:** The router's type is now always `Router` regardless of how many routes or middleware are applied ([#404]) This means router types are all always nameable: ```rust fn my_routes() -> Router { Router::new().route( "/users", post(|| async { "Hello, World!" }), ) } ``` - **breaking:** Added feature flags for HTTP1 and JSON. This enables removing a few dependencies if your app only uses HTTP2 or doesn't use JSON. Its only a breaking change if you depend on axum with `default_features = false`. ([#286]) - **breaking:** `Route::boxed` and `BoxRoute` have been removed as they're no longer necessary ([#404]) - **breaking:** `Nested`, `Or` types are now private. They no longer had to be public because `Router` is internally boxed ([#404]) - **breaking:** Remove `routing::Layered` as it didn't actually do anything and thus wasn't necessary - **breaking:** Vendor `AddExtensionLayer` and `AddExtension` to reduce public dependencies - **breaking:** `body::BoxBody` is now a type alias for `http_body::combinators::UnsyncBoxBody` and thus is no longer `Sync`. This is because bodies are streams and requiring streams to be `Sync` is unnecessary. - **added:** Implement `IntoResponse` for `http_body::combinators::UnsyncBoxBody`. - **added:** Add `Handler::into_make_service` for serving a handler without a `Router`. - **added:** Add `Handler::into_make_service_with_connect_info` for serving a handler without a `Router`, and storing info about the incoming connection. - **breaking:** axum's minimum support rust version is not 1.54 - Routing: - Big internal refactoring of routing leading to several improvements ([#363]) - **added:** Wildcard routes like `.route("/api/users/*rest", service)` are now supported. - **fixed:** The order routes are added in no longer matters. - **fixed:** Adding a conflicting route will now cause a panic instead of silently making a route unreachable. - **fixed:** Route matching is faster as number of routes increase. - **fixed:** Correctly handle trailing slashes in routes: - If a route with a trailing slash exists and a request without a trailing slash is received, axum will send a 301 redirection to the route with the trailing slash. - Or vice versa if a route without a trailing slash exists and a request with a trailing slash is received. - This can be overridden by explicitly defining two routes: One with and one without trailing a slash. - **breaking:** Method routing for handlers have been moved from `axum::handler` to `axum::routing`. So `axum::handler::get` now lives at `axum::routing::get` ([#405]) - **breaking:** Method routing for services have been moved from `axum::service` to `axum::routing`. So `axum::service::get` now lives at, etc. `axum::routing::service_method_routing::get`, etc. ([#405]) - **breaking:** `Router::or` renamed to `Router::merge` and will now panic on overlapping routes. It now only accepts `Router`s and not general `Service`s. Use `Router::fallback` for adding fallback routes ([#408]) - **added:** `Router::fallback` for adding handlers for request that didn't match any routes. `Router::fallback` must be use instead of `nest("/", _)` ([#408]) - **breaking:** `EmptyRouter` has been renamed to `MethodNotAllowed` as its only used in method routers and not in path routers (`Router`) - **breaking:** Remove support for routing based on the `CONNECT` method. An example of combining axum with and HTTP proxy can be found [here][proxy] ([#428]) - Extractors: - **fixed:** Expand accepted content types for JSON requests ([#378]) - **fixed:** Support deserializing `i128` and `u128` in `extract::Path` - **breaking:** Automatically do percent decoding in `extract::Path` ([#272]) - **breaking:** Change `Connected::connect_info` to return `Self` and remove the associated type `ConnectInfo` ([#396]) - **added:** Add `extract::MatchedPath` for accessing path in router that matched the request ([#412]) - Error handling: - **breaking:** Simplify error handling model ([#402]): - All services part of the router are now required to be infallible. - Error handling utilities have been moved to an `error_handling` module. - `Router::check_infallible` has been removed since routers are always infallible with the error handling changes. - Error handling closures must now handle all errors and thus always return something that implements `IntoResponse`. With these changes handling errors from fallible middleware is done like so: ```rust,no_run use axum::{ routing::get, http::StatusCode, error_handling::HandleErrorLayer, response::IntoResponse, Router, BoxError, }; use tower::ServiceBuilder; use std::time::Duration; let middleware_stack = ServiceBuilder::new() // Handle errors from middleware // // This middleware most be added above any fallible // ones if you're using `ServiceBuilder`, due to how ordering works .layer(HandleErrorLayer::new(handle_error)) // Return an error after 30 seconds .timeout(Duration::from_secs(30)); let app = Router::new() .route("/", get(|| async { /* ... */ })) .layer(middleware_stack); fn handle_error(_error: BoxError) -> impl IntoResponse { StatusCode::REQUEST_TIMEOUT } ``` And handling errors from fallible leaf services is done like so: ```rust use axum::{ Router, service, body::Body, routing::service_method_routing::get, response::IntoResponse, http::{Request, Response}, error_handling::HandleErrorExt, // for `.handle_error` }; use std::{io, convert::Infallible}; use tower::service_fn; let app = Router::new() .route( "/", get(service_fn(|_req: Request<Body>| async { let contents = tokio::fs::read_to_string("some_file").await?; Ok::<_, io::Error>(Response::new(Body::from(contents))) })) .handle_error(handle_io_error), ); fn handle_io_error(error: io::Error) -> impl IntoResponse { // ... } ``` - Misc: - `InvalidWebsocketVersionHeader` has been renamed to `InvalidWebSocketVersionHeader` ([#416]) - `WebsocketKeyHeaderMissing` has been renamed to `WebSocketKeyHeaderMissing` ([#416]) [#339]: #339 [#286]: #286 [#272]: #272 [#378]: #378 [#363]: #363 [#396]: #396 [#402]: #402 [#404]: #404 [#405]: #405 [#408]: #408 [#412]: #412 [#416]: #416 [#428]: #428 [proxy]: https://github.com/tokio-rs/axum/blob/main/examples/http-proxy/src/main.rs
- Overall: - **fixed:** All known compile time issues are resolved, including those with `boxed` and those introduced by Rust 1.56 ([#404]) - **breaking:** The router's type is now always `Router` regardless of how many routes or middleware are applied ([#404]) This means router types are all always nameable: ```rust fn my_routes() -> Router { Router::new().route( "/users", post(|| async { "Hello, World!" }), ) } ``` - **breaking:** Added feature flags for HTTP1 and JSON. This enables removing a few dependencies if your app only uses HTTP2 or doesn't use JSON. Its only a breaking change if you depend on axum with `default_features = false`. ([#286]) - **breaking:** `Route::boxed` and `BoxRoute` have been removed as they're no longer necessary ([#404]) - **breaking:** `Nested`, `Or` types are now private. They no longer had to be public because `Router` is internally boxed ([#404]) - **breaking:** Remove `routing::Layered` as it didn't actually do anything and thus wasn't necessary - **breaking:** Vendor `AddExtensionLayer` and `AddExtension` to reduce public dependencies - **breaking:** `body::BoxBody` is now a type alias for `http_body::combinators::UnsyncBoxBody` and thus is no longer `Sync`. This is because bodies are streams and requiring streams to be `Sync` is unnecessary. - **added:** Implement `IntoResponse` for `http_body::combinators::UnsyncBoxBody`. - **added:** Add `Handler::into_make_service` for serving a handler without a `Router`. - **added:** Add `Handler::into_make_service_with_connect_info` for serving a handler without a `Router`, and storing info about the incoming connection. - **breaking:** axum's minimum support rust version is not 1.54 - Routing: - Big internal refactoring of routing leading to several improvements ([#363]) - **added:** Wildcard routes like `.route("/api/users/*rest", service)` are now supported. - **fixed:** The order routes are added in no longer matters. - **fixed:** Adding a conflicting route will now cause a panic instead of silently making a route unreachable. - **fixed:** Route matching is faster as number of routes increase. - **fixed:** Correctly handle trailing slashes in routes: - If a route with a trailing slash exists and a request without a trailing slash is received, axum will send a 301 redirection to the route with the trailing slash. - Or vice versa if a route without a trailing slash exists and a request with a trailing slash is received. - This can be overridden by explicitly defining two routes: One with and one without trailing a slash. - **breaking:** Method routing for handlers have been moved from `axum::handler` to `axum::routing`. So `axum::handler::get` now lives at `axum::routing::get` ([#405]) - **breaking:** Method routing for services have been moved from `axum::service` to `axum::routing`. So `axum::service::get` now lives at, etc. `axum::routing::service_method_routing::get`, etc. ([#405]) - **breaking:** `Router::or` renamed to `Router::merge` and will now panic on overlapping routes. It now only accepts `Router`s and not general `Service`s. Use `Router::fallback` for adding fallback routes ([#408]) - **added:** `Router::fallback` for adding handlers for request that didn't match any routes. `Router::fallback` must be use instead of `nest("/", _)` ([#408]) - **breaking:** `EmptyRouter` has been renamed to `MethodNotAllowed` as its only used in method routers and not in path routers (`Router`) - **breaking:** Remove support for routing based on the `CONNECT` method. An example of combining axum with and HTTP proxy can be found [here][proxy] ([#428]) - Extractors: - **fixed:** Expand accepted content types for JSON requests ([#378]) - **fixed:** Support deserializing `i128` and `u128` in `extract::Path` - **breaking:** Automatically do percent decoding in `extract::Path` ([#272]) - **breaking:** Change `Connected::connect_info` to return `Self` and remove the associated type `ConnectInfo` ([#396]) - **added:** Add `extract::MatchedPath` for accessing path in router that matched the request ([#412]) - Error handling: - **breaking:** Simplify error handling model ([#402]): - All services part of the router are now required to be infallible. - Error handling utilities have been moved to an `error_handling` module. - `Router::check_infallible` has been removed since routers are always infallible with the error handling changes. - Error handling closures must now handle all errors and thus always return something that implements `IntoResponse`. With these changes handling errors from fallible middleware is done like so: ```rust,no_run use axum::{ routing::get, http::StatusCode, error_handling::HandleErrorLayer, response::IntoResponse, Router, BoxError, }; use tower::ServiceBuilder; use std::time::Duration; let middleware_stack = ServiceBuilder::new() // Handle errors from middleware // // This middleware most be added above any fallible // ones if you're using `ServiceBuilder`, due to how ordering works .layer(HandleErrorLayer::new(handle_error)) // Return an error after 30 seconds .timeout(Duration::from_secs(30)); let app = Router::new() .route("/", get(|| async { /* ... */ })) .layer(middleware_stack); fn handle_error(_error: BoxError) -> impl IntoResponse { StatusCode::REQUEST_TIMEOUT } ``` And handling errors from fallible leaf services is done like so: ```rust use axum::{ Router, service, body::Body, routing::service_method_routing::get, response::IntoResponse, http::{Request, Response}, error_handling::HandleErrorExt, // for `.handle_error` }; use std::{io, convert::Infallible}; use tower::service_fn; let app = Router::new() .route( "/", get(service_fn(|_req: Request<Body>| async { let contents = tokio::fs::read_to_string("some_file").await?; Ok::<_, io::Error>(Response::new(Body::from(contents))) })) .handle_error(handle_io_error), ); fn handle_io_error(error: io::Error) -> impl IntoResponse { // ... } ``` - Misc: - `InvalidWebsocketVersionHeader` has been renamed to `InvalidWebSocketVersionHeader` ([#416]) - `WebsocketKeyHeaderMissing` has been renamed to `WebSocketKeyHeaderMissing` ([#416]) [#339]: #339 [#286]: #286 [#272]: #272 [#378]: #378 [#363]: #363 [#396]: #396 [#402]: #402 [#404]: #404 [#405]: #405 [#408]: #408 [#412]: #412 [#416]: #416 [#428]: #428 [proxy]: https://github.com/tokio-rs/axum/blob/main/examples/http-proxy/src/main.rs
…registered route: /favicon.ico. Note that `nest("/", _)` conflicts with all routes. Use `Router::fallback` instead Refs: tokio-rs/axum#363
.route("/foo/*, _)
.or
s sinceor
takes anytower::Service
. I think thats fine and want to keepor
this general.nest
still accepts anytower::Service
which is how I think we should keep it.TODO
route
andnest
when adding conflicting routenest
cannot contain wildcards because axum internally makes it a wildcard/foo
and/:key
now conflictingNode
impact performance?Router::or
#380 (comment)Fixes #240
Fixes #259
Fixes #109