Skip to content

Commit

Permalink
Fix compile time regression by boxing routes internally (#404)
Browse files Browse the repository at this point in the history
This is a reimplementation of #401 but with the new matchit based router.

Fixes #399
  • Loading branch information
davidpdrsn authored Oct 24, 2021
1 parent 1a764bb commit 0ee7379
Show file tree
Hide file tree
Showing 13 changed files with 203 additions and 242 deletions.
33 changes: 25 additions & 8 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

# Unreleased

- **fixed:** All known compile time issues are resolved, including those with
`boxed` and those introduced by Rust 1.56 ([#404])
- Big internal refactoring of routing leading to several improvements ([#363])
- Wildcard routes like `.route("/api/users/*rest", service)` are now supported.
- The order routes are added in no longer matters.
- Adding a conflicting route will now cause a panic instead of silently making
- **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.
- Route matching is faster as number of routes increase.
- **fixed:** Route matching is faster as number of routes increase.
- **breaking:** The routes `/foo` and `/:key` are considered to overlap and
will cause a panic when constructing the router. This might be fixed in the future.
- Improve performance of `BoxRoute` ([#339])
- Expand accepted content types for JSON requests ([#378])
- **fixed:** Expand accepted content types for JSON requests ([#378])
- **breaking:** The router's type is now always `Router` regardless of how many routes or
middleware are applies ([#404])

This means router types are all always nameable:

```rust
fn my_routes() -> Router {
Router::new().route(
"/users",
post(|| async { "Hello, World!" }),
)
}
```
- **breaking:** `Route::boxed` and `BoxRoute` have been removed as they're no longer
necessary ([#404])
- **breaking:** `Route`, `Nested`, `Or` types are now private. They no longer had to be
public because `Router` is internally boxed ([#404])
- **breaking:** Automatically do percent decoding in `extract::Path`
([#272])
- **breaking:** `Router::boxed` now the inner service to implement `Clone` and
`Sync` in addition to the previous trait bounds ([#339])
- **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])
Expand Down Expand Up @@ -103,6 +119,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#363]: https://github.com/tokio-rs/axum/pull/363
[#396]: https://github.com/tokio-rs/axum/pull/396
[#402]: https://github.com/tokio-rs/axum/pull/402
[#404]: https://github.com/tokio-rs/axum/pull/404

# 0.2.8 (07. October, 2021)

Expand Down
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ ws = ["tokio-tungstenite", "sha-1", "base64"]
async-trait = "0.1.43"
bitflags = "1.0"
bytes = "1.0"
dyn-clone = "1.0"
futures-util = { version = "0.3", default-features = false, features = ["alloc"] }
http = "0.2"
http-body = "0.4.3"
Expand Down
4 changes: 1 addition & 3 deletions examples/key-value-store/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use axum::{
handler::{delete, get, Handler},
http::StatusCode,
response::IntoResponse,
routing::BoxRoute,
Router,
};
use std::{
Expand Down Expand Up @@ -108,7 +107,7 @@ async fn list_keys(Extension(state): Extension<SharedState>) -> String {
.join("\n")
}

fn admin_routes() -> Router<BoxRoute> {
fn admin_routes() -> Router {
async fn delete_all_keys(Extension(state): Extension<SharedState>) {
state.write().unwrap().db.clear();
}
Expand All @@ -122,7 +121,6 @@ fn admin_routes() -> Router<BoxRoute> {
.route("/key/:key", delete(remove_key))
// Require bearer auth for all admin routes
.layer(RequireAuthorizationLayer::bearer("secret-token"))
.boxed()
}

fn handle_error(error: BoxError) -> impl IntoResponse {
Expand Down
6 changes: 2 additions & 4 deletions examples/testing/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use axum::{
handler::{get, post},
routing::BoxRoute,
Json, Router,
};
use tower_http::trace::TraceLayer;
Expand All @@ -32,7 +31,7 @@ async fn main() {
/// Having a function that produces our app makes it easy to call it from tests
/// without having to create an HTTP server.
#[allow(dead_code)]
fn app() -> Router<BoxRoute> {
fn app() -> Router {
Router::new()
.route("/", get(|| async { "Hello, World!" }))
.route(
Expand All @@ -43,7 +42,6 @@ fn app() -> Router<BoxRoute> {
)
// We can still add middleware
.layer(TraceLayer::new_for_http())
.boxed()
}

#[cfg(test)]
Expand All @@ -59,7 +57,7 @@ mod tests {
async fn hello_world() {
let app = app();

// `BoxRoute<Body>` implements `tower::Service<Request<Body>>` so we can
// `Router` implements `tower::Service<Request<Body>>` so we can
// call it like any tower service, no need to run an HTTP server.
let response = app
.oneshot(Request::builder().uri("/").body(Body::empty()).unwrap())
Expand Down
43 changes: 24 additions & 19 deletions examples/tls-rustls/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,31 @@
//! cargo run -p example-tls-rustls
//! ```
use axum::{handler::get, Router};
// NOTE: This example is currently broken since axum-server requires `S: Sync`,
// that isn't necessary and will be fixed in a future release

#[tokio::main]
async fn main() {
// Set the RUST_LOG, if it hasn't been explicitly defined
if std::env::var_os("RUST_LOG").is_none() {
std::env::set_var("RUST_LOG", "example_tls_rustls=debug")
}
tracing_subscriber::fmt::init();
fn main() {}

let app = Router::new().route("/", get(handler));
// use axum::{handler::get, Router};

axum_server::bind_rustls("127.0.0.1:3000")
.private_key_file("examples/tls-rustls/self_signed_certs/key.pem")
.certificate_file("examples/tls-rustls/self_signed_certs/cert.pem")
.serve(app)
.await
.unwrap();
}
// #[tokio::main]
// async fn main() {
// // Set the RUST_LOG, if it hasn't been explicitly defined
// if std::env::var_os("RUST_LOG").is_none() {
// std::env::set_var("RUST_LOG", "example_tls_rustls=debug")
// }
// tracing_subscriber::fmt::init();

async fn handler() -> &'static str {
"Hello, World!"
}
// // let app = Router::new().route("/", get(handler));

// // axum_server::bind_rustls("127.0.0.1:3000")
// // .private_key_file("examples/tls-rustls/self_signed_certs/key.pem")
// // .certificate_file("examples/tls-rustls/self_signed_certs/cert.pem")
// // .serve(app)
// // .await
// // .unwrap();
// }

// async fn handler() -> &'static str {
// "Hello, World!"
// }
19 changes: 7 additions & 12 deletions src/clone_box_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,18 @@ use std::task::{Context, Poll};
use tower::ServiceExt;
use tower_service::Service;

/// A `Clone + Send + Sync` boxed `Service`
/// A `Clone + Send` boxed `Service`
pub(crate) struct CloneBoxService<T, U, E>(
Box<
dyn CloneService<T, Response = U, Error = E, Future = BoxFuture<'static, Result<U, E>>>
+ Send
+ Sync,
+ Send,
>,
);

impl<T, U, E> CloneBoxService<T, U, E> {
pub(crate) fn new<S>(inner: S) -> Self
where
S: Service<T, Response = U, Error = E> + Clone + Send + Sync + 'static,
S: Service<T, Response = U, Error = E> + Clone + Send + 'static,
S::Future: Send + 'static,
{
let inner = inner.map_future(|f| Box::pin(f) as _);
Expand Down Expand Up @@ -48,22 +47,18 @@ trait CloneService<R>: Service<R> {
&self,
) -> Box<
dyn CloneService<R, Response = Self::Response, Error = Self::Error, Future = Self::Future>
+ Send
+ Sync,
+ Send,
>;
}

impl<R, T> CloneService<R> for T
where
T: Service<R> + Send + Sync + Clone + 'static,
T: Service<R> + Send + Clone + 'static,
{
fn clone_box(
&self,
) -> Box<
dyn CloneService<R, Response = T::Response, Error = T::Error, Future = T::Future>
+ Send
+ Sync,
> {
) -> Box<dyn CloneService<R, Response = T::Response, Error = T::Error, Future = T::Future> + Send>
{
Box::new(self.clone())
}
}
4 changes: 1 addition & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,15 +360,13 @@
//! http::Request,
//! handler::get,
//! Router,
//! routing::BoxRoute
//! };
//! use tower_http::services::ServeFile;
//! use http::Response;
//!
//! fn api_routes() -> Router<BoxRoute> {
//! fn api_routes() -> Router {
//! Router::new()
//! .route("/users", get(|_: Request<Body>| async { /* ... */ }))
//! .boxed()
//! }
//!
//! let app = Router::new()
Expand Down
35 changes: 6 additions & 29 deletions src/routing/future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,58 +2,35 @@
use crate::{
body::BoxBody,
clone_box_service::CloneBoxService,
routing::{FromEmptyRouter, UriStack},
};
use http::{Request, Response};
use pin_project_lite::pin_project;
use std::{
convert::Infallible,
fmt,
future::Future,
pin::Pin,
task::{Context, Poll},
};
use tower::util::Oneshot;
use tower_service::Service;

pub use super::or::ResponseFuture as OrResponseFuture;

opaque_future! {
/// Response future for [`EmptyRouter`](super::EmptyRouter).
pub type EmptyRouterFuture<E> =
std::future::Ready<Result<Response<BoxBody>, E>>;
}

pin_project! {
/// The response future for [`BoxRoute`](super::BoxRoute).
pub struct BoxRouteFuture<B> {
#[pin]
pub(super) inner: Oneshot<
CloneBoxService<Request<B>, Response<BoxBody>, Infallible>,
Request<B>,
>,
}
}

impl<B> Future for BoxRouteFuture<B> {
type Output = Result<Response<BoxBody>, Infallible>;

fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
self.project().inner.poll(cx)
}
}

impl<B> fmt::Debug for BoxRouteFuture<B> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("BoxRouteFuture").finish()
}
opaque_future! {
/// Response future for [`Routes`](super::Routes).
pub type RoutesFuture =
futures_util::future::BoxFuture<'static, Result<Response<BoxBody>, Infallible>>;
}

pin_project! {
/// The response future for [`Route`](super::Route).
#[derive(Debug)]
pub struct RouteFuture<S, F, B>
pub(crate) struct RouteFuture<S, F, B>
where
S: Service<Request<B>>,
F: Service<Request<B>>
Expand Down Expand Up @@ -119,7 +96,7 @@ where
pin_project! {
/// The response future for [`Nested`](super::Nested).
#[derive(Debug)]
pub struct NestedFuture<S, F, B>
pub(crate) struct NestedFuture<S, F, B>
where
S: Service<Request<B>>,
F: Service<Request<B>>
Expand Down
Loading

0 comments on commit 0ee7379

Please sign in to comment.