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

Fix compile time regression by boxing routes internally #404

Merged
merged 3 commits into from
Oct 24, 2021
Merged
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
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