From e85538b047641c7630ece37295baa01b47cf192f Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Wed, 23 Nov 2022 18:43:42 +0100 Subject: [PATCH] wip --- axum-extra/src/routing/resource.rs | 6 +- axum-extra/src/routing/spa.rs | 2 +- axum/benches/benches.rs | 43 +-- axum/src/boxed.rs | 8 +- axum/src/handler/mod.rs | 4 +- axum/src/lib.rs | 2 +- axum/src/middleware/from_fn.rs | 1 - axum/src/routing/method_routing.rs | 428 +++++++++++--------------- axum/src/routing/mod.rs | 161 ++++------ axum/src/routing/service.rs | 224 -------------- axum/src/routing/tests/fallback.rs | 2 +- axum/src/routing/tests/get_to_head.rs | 2 - axum/src/routing/tests/mod.rs | 8 +- axum/src/test_helpers/mod.rs | 2 +- axum/src/test_helpers/test_client.rs | 8 +- 15 files changed, 271 insertions(+), 630 deletions(-) delete mode 100644 axum/src/routing/service.rs diff --git a/axum-extra/src/routing/resource.rs b/axum-extra/src/routing/resource.rs index 25d56643..19c6236a 100644 --- a/axum-extra/src/routing/resource.rs +++ b/axum-extra/src/routing/resource.rs @@ -147,7 +147,7 @@ impl From> for Router { mod tests { #[allow(unused_imports)] use super::*; - use axum::{extract::Path, http::Method, routing::RouterService, Router}; + use axum::{extract::Path, http::Method, Router}; use http::Request; use tower::{Service, ServiceExt}; @@ -162,7 +162,7 @@ mod tests { .update(|Path(id): Path| async move { format!("users#update id={}", id) }) .destroy(|Path(id): Path| async move { format!("users#destroy id={}", id) }); - let mut app = Router::new().merge(users).into_service(); + let mut app = Router::new().merge(users); assert_eq!( call_route(&mut app, Method::GET, "/users").await, @@ -205,7 +205,7 @@ mod tests { ); } - async fn call_route(app: &mut RouterService, method: Method, uri: &str) -> String { + async fn call_route(app: &mut Router, method: Method, uri: &str) -> String { let res = app .ready() .await diff --git a/axum-extra/src/routing/spa.rs b/axum-extra/src/routing/spa.rs index 5fc6d852..ad8d8af1 100644 --- a/axum-extra/src/routing/spa.rs +++ b/axum-extra/src/routing/spa.rs @@ -270,7 +270,7 @@ mod tests { #[allow(dead_code)] fn works_with_router_with_state() { - let _: axum::RouterService = Router::new() + let _: Router = Router::new() .merge(SpaRouter::new("/assets", "test_files")) .route("/", get(|_: axum::extract::State| async {})) .with_state(String::new()); diff --git a/axum/benches/benches.rs b/axum/benches/benches.rs index 0ff269f0..3a7dd998 100644 --- a/axum/benches/benches.rs +++ b/axum/benches/benches.rs @@ -1,7 +1,7 @@ use axum::{ extract::State, routing::{get, post}, - Extension, Json, Router, RouterService, Server, + Extension, Json, Router, Server, }; use hyper::server::conn::AddrIncoming; use serde::{Deserialize, Serialize}; @@ -17,13 +17,9 @@ fn main() { ensure_rewrk_is_installed(); } - benchmark("minimal").run(|| Router::new().into_service()); + benchmark("minimal").run(Router::new); - benchmark("basic").run(|| { - Router::new() - .route("/", get(|| async { "Hello, World!" })) - .into_service() - }); + benchmark("basic").run(|| Router::new().route("/", get(|| async { "Hello, World!" }))); benchmark("routing").path("/foo/bar/baz").run(|| { let mut app = Router::new(); @@ -34,32 +30,26 @@ fn main() { } } } - app.route("/foo/bar/baz", get(|| async {})).into_service() + app.route("/foo/bar/baz", get(|| async {})) }); benchmark("receive-json") .method("post") .headers(&[("content-type", "application/json")]) .body(r#"{"n": 123, "s": "hi there", "b": false}"#) - .run(|| { - Router::new() - .route("/", post(|_: Json| async {})) - .into_service() - }); + .run(|| Router::new().route("/", post(|_: Json| async {}))); benchmark("send-json").run(|| { - Router::new() - .route( - "/", - get(|| async { - Json(Payload { - n: 123, - s: "hi there".to_owned(), - b: false, - }) - }), - ) - .into_service() + Router::new().route( + "/", + get(|| async { + Json(Payload { + n: 123, + s: "hi there".to_owned(), + b: false, + }) + }), + ) }); let state = AppState { @@ -75,7 +65,6 @@ fn main() { Router::new() .route("/", get(|_: Extension| async {})) .layer(Extension(state.clone())) - .into_service() }); benchmark("state").run(|| { @@ -133,7 +122,7 @@ impl BenchmarkBuilder { fn run(self, f: F) where - F: FnOnce() -> RouterService, + F: FnOnce() -> Router<()>, { // support only running some benchmarks with // ``` diff --git a/axum/src/boxed.rs b/axum/src/boxed.rs index eeb70970..6aaea39a 100644 --- a/axum/src/boxed.rs +++ b/axum/src/boxed.rs @@ -21,6 +21,7 @@ where where H: Handler, T: 'static, + B: HttpBody, { Self(Box::new(MakeErasedHandler { handler, @@ -98,7 +99,7 @@ impl ErasedIntoRoute for MakeErasedHandler where H: Clone + Send + 'static, S: 'static, - B: 'static, + B: HttpBody + 'static, { fn clone_box(&self) -> Box> { Box::new(self.clone()) @@ -113,7 +114,7 @@ where request: Request, state: S, ) -> RouteFuture { - todo!() + self.into_route(state).call(request) } } @@ -193,8 +194,7 @@ where } fn call_with_state(self: Box, request: Request, state: S) -> RouteFuture { - let route = (self.layer)(self.inner.into_route(state)); - route.call(request) + (self.layer)(self.inner.into_route(state)).call(request) } } diff --git a/axum/src/handler/mod.rs b/axum/src/handler/mod.rs index 6a2df1e6..49e9b591 100644 --- a/axum/src/handler/mod.rs +++ b/axum/src/handler/mod.rs @@ -359,7 +359,7 @@ mod tests { format!("you said: {}", body) } - let client = TestClient::from_service(handle.into_service()); + let client = TestClient::new(handle.into_service()); let res = client.post("/").body("hi there!").send().await; assert_eq!(res.status(), StatusCode::OK); @@ -382,7 +382,7 @@ mod tests { .layer(MapRequestBodyLayer::new(body::boxed)) .with_state("foo"); - let client = TestClient::from_service(svc); + let client = TestClient::new(svc); let res = client.get("/").send().await; assert_eq!(res.text().await, "foo"); } diff --git a/axum/src/lib.rs b/axum/src/lib.rs index 5d2a8de0..eafb6014 100644 --- a/axum/src/lib.rs +++ b/axum/src/lib.rs @@ -475,7 +475,7 @@ pub use self::extension::Extension; #[cfg(feature = "json")] pub use self::json::Json; #[doc(inline)] -pub use self::routing::{Router, RouterService}; +pub use self::routing::Router; #[doc(inline)] #[cfg(feature = "headers")] diff --git a/axum/src/middleware/from_fn.rs b/axum/src/middleware/from_fn.rs index e9a525ee..cd28a603 100644 --- a/axum/src/middleware/from_fn.rs +++ b/axum/src/middleware/from_fn.rs @@ -381,7 +381,6 @@ mod tests { .layer(from_fn(insert_header)); let res = app - .into_service() .oneshot(Request::builder().uri("/").body(Body::empty()).unwrap()) .await .unwrap(); diff --git a/axum/src/routing/method_routing.rs b/axum/src/routing/method_routing.rs index 4b5d5b3f..618e9078 100644 --- a/axum/src/routing/method_routing.rs +++ b/axum/src/routing/method_routing.rs @@ -1,6 +1,6 @@ //! Route to services and handlers based on HTTP methods. -use super::{FallbackRoute, IntoMakeService}; +use super::IntoMakeService; #[cfg(feature = "tokio")] use crate::extract::connect_info::IntoMakeServiceWithConnectInfo; use crate::{ @@ -83,7 +83,7 @@ macro_rules! top_level_service_fn { T: Service> + Clone + Send + 'static, T::Response: IntoResponse + 'static, T::Future: Send + 'static, - B: Send + 'static, + B: HttpBody + Send + 'static, S: Clone, { on_service(MethodFilter::$method, svc) @@ -143,7 +143,7 @@ macro_rules! top_level_handler_fn { pub fn $name(handler: H) -> MethodRouter where H: Handler, - B: Send + 'static, + B: HttpBody + Send + 'static, T: 'static, S: Clone + Send + Sync + 'static, { @@ -327,7 +327,7 @@ where T: Service> + Clone + Send + 'static, T::Response: IntoResponse + 'static, T::Future: Send + 'static, - B: Send + 'static, + B: HttpBody + Send + 'static, S: Clone, { MethodRouter::new().on_service(filter, svc) @@ -391,7 +391,7 @@ where T: Service> + Clone + Send + 'static, T::Response: IntoResponse + 'static, T::Future: Send + 'static, - B: Send + 'static, + B: HttpBody + Send + 'static, S: Clone, { MethodRouter::new() @@ -430,7 +430,7 @@ top_level_handler_fn!(trace, TRACE); pub fn on(filter: MethodFilter, handler: H) -> MethodRouter where H: Handler, - B: Send + 'static, + B: HttpBody + Send + 'static, T: 'static, S: Clone + Send + Sync + 'static, { @@ -477,7 +477,7 @@ where pub fn any(handler: H) -> MethodRouter where H: Handler, - B: Send + 'static, + B: HttpBody + Send + 'static, T: 'static, S: Clone + Send + Sync + 'static, { @@ -571,7 +571,7 @@ impl fmt::Debug for MethodRouter { impl MethodRouter where - B: Send + 'static, + B: HttpBody + Send + 'static, S: Clone, { /// Chain an additional handler that will accept requests matching the given @@ -633,7 +633,7 @@ where impl MethodRouter<(), B, Infallible> where - B: Send + 'static, + B: HttpBody + Send + 'static, { /// Convert the handler into a [`MakeService`]. /// @@ -707,7 +707,7 @@ where impl MethodRouter where - B: Send + 'static, + B: HttpBody + Send + 'static, S: Clone, { /// Create a default `MethodRouter` that will respond with `405 Method Not Allowed` to all @@ -731,21 +731,19 @@ where } } - /// Provide the state. - /// - /// See [`State`](crate::extract::State) for more details about accessing state. - pub fn with_state(self, state: S) -> WithState { - WithState { - get: self.get.into_route(&state), - head: self.head.into_route(&state), - delete: self.delete.into_route(&state), - options: self.options.into_route(&state), - patch: self.patch.into_route(&state), - post: self.post.into_route(&state), - put: self.put.into_route(&state), - trace: self.trace.into_route(&state), - fallback: self.fallback.into_fallback_route(&state), + /// TODO(david): docs + pub fn with_state(self, state: S) -> MethodRouter { + MethodRouter { + get: self.get.with_state(state.clone()), + head: self.head.with_state(state.clone()), + delete: self.delete.with_state(state.clone()), + options: self.options.with_state(state.clone()), + patch: self.patch.with_state(state.clone()), + post: self.post.with_state(state.clone()), + put: self.put.with_state(state.clone()), + trace: self.trace.with_state(state.clone()), allow_header: self.allow_header, + fallback: self.fallback.with_state(state), } } @@ -918,10 +916,7 @@ where } #[doc = include_str!("../docs/method_routing/layer.md")] - pub fn layer( - self, - layer: L, - ) -> MethodRouter + pub fn layer(self, layer: L) -> MethodRouter where L: Layer> + Clone + Send + 'static, L::Service: Service> + Clone + Send + 'static, @@ -930,6 +925,8 @@ where >>::Future: Send + 'static, E: 'static, S: 'static, + NewReqBody: HttpBody + 'static, + NewError: 'static, { let layer_fn = move |route: Route| route.layer(layer.clone()); @@ -1069,226 +1066,8 @@ where self.allow_header = AllowHeader::Skip; self } -} -fn append_allow_header(allow_header: &mut AllowHeader, method: &'static str) { - match allow_header { - AllowHeader::None => { - *allow_header = AllowHeader::Bytes(BytesMut::from(method)); - } - AllowHeader::Skip => {} - AllowHeader::Bytes(allow_header) => { - if let Ok(s) = std::str::from_utf8(allow_header) { - if !s.contains(method) { - allow_header.extend_from_slice(b","); - allow_header.extend_from_slice(method.as_bytes()); - } - } else { - #[cfg(debug_assertions)] - panic!("`allow_header` contained invalid uft-8. This should never happen") - } - } - } -} - -impl Service> for MethodRouter<(), B, E> -where - B: HttpBody + Send + 'static, -{ - type Response = Response; - type Error = E; - type Future = RouteFuture; - - #[inline] - fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(())) - } - - fn call(&mut self, req: Request) -> Self::Future { - self.clone().with_state(()).call(req) - } -} - -impl Clone for MethodRouter { - fn clone(&self) -> Self { - Self { - get: self.get.clone(), - head: self.head.clone(), - delete: self.delete.clone(), - options: self.options.clone(), - patch: self.patch.clone(), - post: self.post.clone(), - put: self.put.clone(), - trace: self.trace.clone(), - fallback: self.fallback.clone(), - allow_header: self.allow_header.clone(), - } - } -} - -impl Default for MethodRouter -where - B: Send + 'static, - S: Clone, -{ - fn default() -> Self { - Self::new() - } -} - -enum MethodEndpoint { - None, - Route(Route), - BoxedHandler(BoxedIntoRoute), -} - -impl MethodEndpoint -where - S: Clone, -{ - fn is_some(&self) -> bool { - matches!(self, Self::Route(_) | Self::BoxedHandler(_)) - } - - fn is_none(&self) -> bool { - matches!(self, Self::None) - } - - fn map(self, f: F) -> MethodEndpoint - where - S: 'static, - B: 'static, - E: 'static, - F: FnOnce(Route) -> Route + Clone + Send + 'static, - B2: 'static, - E2: 'static, - { - match self { - Self::None => MethodEndpoint::None, - Self::Route(route) => MethodEndpoint::Route(f(route)), - Self::BoxedHandler(handler) => MethodEndpoint::BoxedHandler(handler.map(f)), - } - } - - fn into_route(self, state: &S) -> Option> { - match self { - Self::None => None, - Self::Route(route) => Some(route), - Self::BoxedHandler(handler) => Some(handler.into_route(state.clone())), - } - } -} - -impl Clone for MethodEndpoint { - fn clone(&self) -> Self { - match self { - Self::None => Self::None, - Self::Route(inner) => Self::Route(inner.clone()), - Self::BoxedHandler(inner) => Self::BoxedHandler(inner.clone()), - } - } -} - -impl fmt::Debug for MethodEndpoint { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::None => f.debug_tuple("None").finish(), - Self::Route(inner) => inner.fmt(f), - Self::BoxedHandler(_) => f.debug_tuple("BoxedHandler").finish(), - } - } -} - -/// A [`MethodRouter`] which has access to some state. -/// -/// Implements [`Service`]. -/// -/// The state can be extracted with [`State`](crate::extract::State). -/// -/// Created with [`MethodRouter::with_state`] -pub struct WithState { - get: Option>, - head: Option>, - delete: Option>, - options: Option>, - patch: Option>, - post: Option>, - put: Option>, - trace: Option>, - fallback: FallbackRoute, - allow_header: AllowHeader, -} - -impl WithState { - /// Convert the handler into a [`MakeService`]. - /// - /// See [`MethodRouter::into_make_service`] for more details. - /// - /// [`MakeService`]: tower::make::MakeService - pub fn into_make_service(self) -> IntoMakeService { - IntoMakeService::new(self) - } - - /// Convert the router into a [`MakeService`] which stores information - /// about the incoming connection. - /// - /// See [`MethodRouter::into_make_service_with_connect_info`] for more details. - /// - /// [`MakeService`]: tower::make::MakeService - #[cfg(feature = "tokio")] - pub fn into_make_service_with_connect_info(self) -> IntoMakeServiceWithConnectInfo { - IntoMakeServiceWithConnectInfo::new(self) - } -} - -impl Clone for WithState { - fn clone(&self) -> Self { - Self { - get: self.get.clone(), - head: self.head.clone(), - delete: self.delete.clone(), - options: self.options.clone(), - patch: self.patch.clone(), - post: self.post.clone(), - put: self.put.clone(), - trace: self.trace.clone(), - fallback: self.fallback.clone(), - allow_header: self.allow_header.clone(), - } - } -} - -impl fmt::Debug for WithState { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("WithState") - .field("get", &self.get) - .field("head", &self.head) - .field("delete", &self.delete) - .field("options", &self.options) - .field("patch", &self.patch) - .field("post", &self.post) - .field("put", &self.put) - .field("trace", &self.trace) - .field("fallback", &self.fallback) - .field("allow_header", &self.allow_header) - .finish() - } -} - -impl Service> for WithState -where - B: HttpBody + Send, -{ - type Response = Response; - type Error = E; - type Future = RouteFuture; - - #[inline] - fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(())) - } - - fn call(&mut self, req: Request) -> Self::Future { + pub(crate) fn call_with_state(&mut self, req: Request, state: S) -> RouteFuture { macro_rules! call { ( $req:expr, @@ -1297,9 +1076,17 @@ where $svc:expr ) => { if $method == Method::$method_variant { - if let Some(svc) = $svc { - return RouteFuture::from_future(svc.oneshot_inner($req)) - .strip_body($method == Method::HEAD); + match $svc { + MethodEndpoint::None => {} + MethodEndpoint::Route(route) => { + return RouteFuture::from_future(route.oneshot_inner($req)) + .strip_body($method == Method::HEAD); + } + MethodEndpoint::BoxedHandler(handler) => { + let mut route = handler.clone().into_route(state); + return RouteFuture::from_future(route.oneshot_inner($req)) + .strip_body($method == Method::HEAD); + } } } }; @@ -1331,7 +1118,15 @@ where call!(req, method, DELETE, delete); call!(req, method, TRACE, trace); - let future = RouteFuture::from_future(fallback.oneshot_inner(req)); + let future = match fallback { + Fallback::Default(route) | Fallback::Service(route) => { + RouteFuture::from_future(route.oneshot_inner(req)) + } + Fallback::BoxedHandler(handler) => { + let mut route = handler.clone().into_route(state); + RouteFuture::from_future(route.oneshot_inner(req)) + } + }; match allow_header { AllowHeader::None => future.allow_header(Bytes::new()), @@ -1341,6 +1136,137 @@ where } } +fn append_allow_header(allow_header: &mut AllowHeader, method: &'static str) { + match allow_header { + AllowHeader::None => { + *allow_header = AllowHeader::Bytes(BytesMut::from(method)); + } + AllowHeader::Skip => {} + AllowHeader::Bytes(allow_header) => { + if let Ok(s) = std::str::from_utf8(allow_header) { + if !s.contains(method) { + allow_header.extend_from_slice(b","); + allow_header.extend_from_slice(method.as_bytes()); + } + } else { + #[cfg(debug_assertions)] + panic!("`allow_header` contained invalid uft-8. This should never happen") + } + } + } +} + +impl Clone for MethodRouter { + fn clone(&self) -> Self { + Self { + get: self.get.clone(), + head: self.head.clone(), + delete: self.delete.clone(), + options: self.options.clone(), + patch: self.patch.clone(), + post: self.post.clone(), + put: self.put.clone(), + trace: self.trace.clone(), + fallback: self.fallback.clone(), + allow_header: self.allow_header.clone(), + } + } +} + +impl Default for MethodRouter +where + B: HttpBody + Send + 'static, + S: Clone, +{ + fn default() -> Self { + Self::new() + } +} + +enum MethodEndpoint { + None, + Route(Route), + BoxedHandler(BoxedIntoRoute), +} + +impl MethodEndpoint +where + S: Clone, +{ + fn is_some(&self) -> bool { + matches!(self, Self::Route(_) | Self::BoxedHandler(_)) + } + + fn is_none(&self) -> bool { + matches!(self, Self::None) + } + + fn map(self, f: F) -> MethodEndpoint + where + S: 'static, + B: 'static, + E: 'static, + F: FnOnce(Route) -> Route + Clone + Send + 'static, + B2: HttpBody + 'static, + E2: 'static, + { + match self { + Self::None => MethodEndpoint::None, + Self::Route(route) => MethodEndpoint::Route(f(route)), + Self::BoxedHandler(handler) => MethodEndpoint::BoxedHandler(handler.map(f)), + } + } + + fn with_state(self, state: S) -> MethodEndpoint { + match self { + MethodEndpoint::None => MethodEndpoint::None, + MethodEndpoint::Route(route) => MethodEndpoint::Route(route), + MethodEndpoint::BoxedHandler(handler) => { + MethodEndpoint::Route(handler.into_route(state)) + } + } + } +} + +impl Clone for MethodEndpoint { + fn clone(&self) -> Self { + match self { + Self::None => Self::None, + Self::Route(inner) => Self::Route(inner.clone()), + Self::BoxedHandler(inner) => Self::BoxedHandler(inner.clone()), + } + } +} + +impl fmt::Debug for MethodEndpoint { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::None => f.debug_tuple("None").finish(), + Self::Route(inner) => inner.fmt(f), + Self::BoxedHandler(_) => f.debug_tuple("BoxedHandler").finish(), + } + } +} + +impl Service> for MethodRouter<(), B, E> +where + B: HttpBody + Send + 'static, +{ + type Response = Response; + type Error = E; + type Future = RouteFuture; + + #[inline] + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + #[inline] + fn call(&mut self, req: Request) -> Self::Future { + self.call_with_state(req, ()) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 7f08f3fd..d010bc4c 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -20,7 +20,6 @@ use std::{ task::{Context, Poll}, }; use sync_wrapper::SyncWrapper; -use tower::util::{BoxCloneService, Oneshot}; use tower_layer::Layer; use tower_service::Service; @@ -34,14 +33,10 @@ mod route; mod strip_prefix; pub(crate) mod url_params; -mod service; #[cfg(test)] mod tests; -pub use self::{ - into_make_service::IntoMakeService, method_filter::MethodFilter, route::Route, - service::RouterService, -}; +pub use self::{into_make_service::IntoMakeService, method_filter::MethodFilter, route::Route}; pub use self::method_routing::{ any, any_service, delete, delete_service, get, get_service, head, head_service, on, on_service, @@ -175,10 +170,10 @@ where T::Response: IntoResponse, T::Future: Send + 'static, { - let service = match try_downcast::, _>(service) { + let service = match try_downcast::, _>(service) { Ok(_) => { panic!( - "Invalid route: `Router::route_service` cannot be used with `RouterService`s. \ + "Invalid route: `Router::route_service` cannot be used with `Router`s. \ Use `Router::nest` instead" ); } @@ -325,7 +320,7 @@ where >>::Response: IntoResponse + 'static, >>::Error: Into + 'static, >>::Future: Send + 'static, - NewReqBody: 'static, + NewReqBody: HttpBody + 'static, { let routes = self .routes @@ -401,11 +396,32 @@ where self } - /// Convert this router into a [`RouterService`] by providing the state. - /// - /// Once this method has been called you cannot add more routes. So it must be called as last. - pub fn with_state(self, state: S) -> RouterService { - RouterService::new(self, state) + /// TODO(david): docs + pub fn with_state(self, state: S) -> Router { + let routes = self + .routes + .into_iter() + .map(|(id, endpoint)| { + let endpoint: Endpoint = match endpoint { + Endpoint::MethodRouter(method_router) => { + Endpoint::MethodRouter(method_router.with_state(state.clone())) + } + Endpoint::Route(route) => Endpoint::Route(route), + Endpoint::NestedRouter(router) => { + Endpoint::Route(router.into_route(state.clone())) + } + }; + (id, endpoint) + }) + .collect(); + + let fallback = self.fallback.with_state(state); + + Router { + routes, + node: self.node, + fallback, + } } pub(crate) fn call_with_state( @@ -446,29 +462,22 @@ where MatchError::NotFound | MatchError::ExtraTrailingSlash | MatchError::MissingTrailingSlash, - ) => { - match &mut self.fallback { - Fallback::Default(fallback) => { - if let Some(super_fallback) = - req.extensions_mut().remove::>() - { - let mut super_fallback = super_fallback.0.into_inner(); - super_fallback.call(req) - } else { - fallback.call(req) - } - } - Fallback::Service(fallback) => fallback.call(req), - Fallback::BoxedHandler(handler) => { - todo!() - // handler.clone().into_route(state).call(req) + ) => match &mut self.fallback { + Fallback::Default(fallback) => { + if let Some(super_fallback) = req.extensions_mut().remove::>() + { + let mut super_fallback = super_fallback.0.into_inner(); + super_fallback.call(req) + } else { + fallback.call(req) } } - } + Fallback::Service(fallback) => fallback.call(req), + Fallback::BoxedHandler(handler) => handler.clone().into_route(state).call(req), + }, } } - // TODO(david): fix duplication #[inline] fn call_route( &self, @@ -494,12 +503,8 @@ where .clone(); match endpont { - Endpoint::MethodRouter(method_router) => { - // method_router.call(req) - todo!() - } + Endpoint::MethodRouter(mut method_router) => method_router.call_with_state(req, state), Endpoint::Route(mut route) => route.call(req), - // TODO(david): optimize? Endpoint::NestedRouter(router) => router.call_with_state(req, state), } } @@ -509,16 +514,6 @@ impl Router<(), B> where B: HttpBody + Send + 'static, { - /// Convert this router into a [`RouterService`]. - /// - /// This is a convenience method for routers that don't have any state (i.e. the state type is - /// `()`). Use [`Router::with_state`] otherwise. - /// - /// Once this method has been called you cannot add more routes. So it must be called as last. - pub fn into_service(self) -> RouterService { - RouterService::new(self, ()) - } - /// Convert this router into a [`MakeService`], that is a [`Service`] whose /// response is another service. /// @@ -545,16 +540,18 @@ where /// `()`). Use [`RouterService::into_make_service`] otherwise. /// /// [`MakeService`]: tower::make::MakeService - pub fn into_make_service(self) -> IntoMakeService> { - IntoMakeService::new(self.into_service()) + pub fn into_make_service(self) -> IntoMakeService { + // call `Router::with_state` such that everything is turned into `Route` eagerly + // rather than doing that per request + IntoMakeService::new(self.with_state(())) } #[doc = include_str!("../docs/routing/into_make_service_with_connect_info.md")] #[cfg(feature = "tokio")] - pub fn into_make_service_with_connect_info( - self, - ) -> IntoMakeServiceWithConnectInfo, C> { - IntoMakeServiceWithConnectInfo::new(self.into_service()) + pub fn into_make_service_with_connect_info(self) -> IntoMakeServiceWithConnectInfo { + // call `Router::with_state` such that everything is turned into `Route` eagerly + // rather than doing that per request + IntoMakeServiceWithConnectInfo::new(self.with_state(())) } } @@ -637,16 +634,6 @@ where } } - fn into_fallback_route(self, state: &S) -> FallbackRoute { - match self { - Self::Default(route) => FallbackRoute::Default(route), - Self::Service(route) => FallbackRoute::Service(route), - Self::BoxedHandler(handler) => { - FallbackRoute::Service(handler.into_route(state.clone())) - } - } - } - fn map(self, f: F) -> Fallback where S: 'static, @@ -662,6 +649,14 @@ where Self::BoxedHandler(handler) => Fallback::BoxedHandler(handler.map(f)), } } + + fn with_state(self, state: S) -> Fallback { + match self { + Fallback::Default(route) => Fallback::Default(route), + Fallback::Service(route) => Fallback::Service(route), + Fallback::BoxedHandler(handler) => Fallback::Service(handler.into_route(state)), + } + } } impl Clone for Fallback { @@ -690,24 +685,6 @@ pub(crate) enum FallbackRoute { Service(Route), } -impl FallbackRoute { - fn layer(self, layer: L) -> FallbackRoute - where - L: Layer> + Clone + Send + 'static, - L::Service: Service> + Clone + Send + 'static, - >>::Response: IntoResponse + 'static, - >>::Error: Into + 'static, - >>::Future: Send + 'static, - NewReqBody: 'static, - NewError: 'static, - { - match self { - FallbackRoute::Default(route) => FallbackRoute::Default(route.layer(layer)), - FallbackRoute::Service(route) => FallbackRoute::Service(route.layer(layer)), - } - } -} - impl fmt::Debug for FallbackRoute { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -726,18 +703,6 @@ impl Clone for FallbackRoute { } } -impl FallbackRoute { - pub(crate) fn oneshot_inner( - &mut self, - req: Request, - ) -> Oneshot, Response, E>, Request> { - match self { - FallbackRoute::Default(inner) => inner.oneshot_inner(req), - FallbackRoute::Service(inner) => inner.oneshot_inner(req), - } - } -} - #[allow(clippy::large_enum_variant)] // This type is only used at init time, probably fine enum Endpoint { MethodRouter(MethodRouter), @@ -750,14 +715,6 @@ where B: HttpBody + Send + 'static, S: Clone + Send + Sync + 'static, { - fn into_route(self, state: S) -> Route { - match self { - Endpoint::MethodRouter(method_router) => Route::new(method_router.with_state(state)), - Endpoint::Route(route) => route, - Endpoint::NestedRouter(router) => router.into_route(state), - } - } - fn layer(self, layer: L) -> Endpoint where L: Layer> + Clone + Send + 'static, @@ -765,7 +722,7 @@ where >>::Response: IntoResponse + 'static, >>::Error: Into + 'static, >>::Future: Send + 'static, - NewReqBody: 'static, + NewReqBody: HttpBody + 'static, { match self { Endpoint::MethodRouter(method_router) => { diff --git a/axum/src/routing/service.rs b/axum/src/routing/service.rs deleted file mode 100644 index 3622abdb..00000000 --- a/axum/src/routing/service.rs +++ /dev/null @@ -1,224 +0,0 @@ -use super::{ - future::RouteFuture, url_params, FallbackRoute, IntoMakeService, Node, Route, RouteId, Router, - SuperFallback, -}; -use crate::{ - body::{Body, HttpBody}, - response::Response, -}; -use axum_core::response::IntoResponse; -use http::Request; -use matchit::MatchError; -use std::{ - collections::HashMap, - convert::Infallible, - sync::Arc, - task::{Context, Poll}, -}; -use sync_wrapper::SyncWrapper; -use tower::Service; -use tower_layer::Layer; - -/// A [`Router`] converted into a [`Service`]. -#[derive(Debug)] -pub struct RouterService { - routes: HashMap>, - node: Arc, - fallback: FallbackRoute, -} - -impl RouterService -where - B: HttpBody + Send + 'static, -{ - pub(super) fn new(router: Router, state: S) -> Self - where - S: Clone + Send + Sync + 'static, - { - let fallback = router.fallback.into_fallback_route(&state); - - let routes = router - .routes - .into_iter() - .map(|(route_id, endpoint)| { - let route = endpoint.into_route(state.clone()); - (route_id, route) - }) - .collect(); - - Self { - routes, - node: router.node, - fallback, - } - } - - #[inline] - fn call_route( - &self, - match_: matchit::Match<&RouteId>, - mut req: Request, - ) -> RouteFuture { - let id = *match_.value; - - #[cfg(feature = "matched-path")] - crate::extract::matched_path::set_matched_path_for_request( - id, - &self.node.route_id_to_path, - req.extensions_mut(), - ); - - url_params::insert_url_params(req.extensions_mut(), match_.params); - - let mut route = self - .routes - .get(&id) - .expect("no route for id. This is a bug in axum. Please file an issue") - .clone(); - - route.call(req) - } - - /// Apply a [`tower::Layer`] to all routes in the router. - /// - /// See [`Router::layer`] for more details. - pub fn layer(self, layer: L) -> RouterService - where - L: Layer> + Clone + Send + 'static, - L::Service: Service> + Clone + Send + 'static, - >>::Response: IntoResponse + 'static, - >>::Error: Into + 'static, - >>::Future: Send + 'static, - NewReqBody: 'static, - { - let routes = self - .routes - .into_iter() - .map(|(id, route)| (id, route.layer(layer.clone()))) - .collect(); - - let fallback = self.fallback.layer(layer); - - RouterService { - routes, - node: self.node, - fallback, - } - } - - /// Apply a [`tower::Layer`] to the router that will only run if the request matches - /// a route. - /// - /// See [`Router::route_layer`] for more details. - pub fn route_layer(self, layer: L) -> Self - where - L: Layer> + Clone + Send + 'static, - L::Service: Service> + Clone + Send + 'static, - >>::Response: IntoResponse + 'static, - >>::Error: Into + 'static, - >>::Future: Send + 'static, - { - let routes = self - .routes - .into_iter() - .map(|(id, route)| (id, route.layer(layer.clone()))) - .collect(); - - Self { - routes, - node: self.node, - fallback: self.fallback, - } - } - - /// Convert the `RouterService` into a [`MakeService`]. - /// - /// See [`Router::into_make_service`] for more details. - /// - /// [`MakeService`]: tower::make::MakeService - pub fn into_make_service(self) -> IntoMakeService { - IntoMakeService::new(self) - } - - /// Convert the `RouterService` into a [`MakeService`] which stores information - /// about the incoming connection. - /// - /// See [`Router::into_make_service_with_connect_info`] for more details. - /// - /// [`MakeService`]: tower::make::MakeService - #[cfg(feature = "tokio")] - pub fn into_make_service_with_connect_info( - self, - ) -> crate::extract::connect_info::IntoMakeServiceWithConnectInfo { - crate::extract::connect_info::IntoMakeServiceWithConnectInfo::new(self) - } -} - -impl Clone for RouterService { - fn clone(&self) -> Self { - Self { - routes: self.routes.clone(), - node: Arc::clone(&self.node), - fallback: self.fallback.clone(), - } - } -} - -impl Service> for RouterService -where - B: HttpBody + Send + 'static, -{ - type Response = Response; - type Error = Infallible; - type Future = RouteFuture; - - #[inline] - fn poll_ready(&mut self, _: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(())) - } - - #[inline] - fn call(&mut self, mut req: Request) -> Self::Future { - #[cfg(feature = "original-uri")] - { - use crate::extract::OriginalUri; - - if req.extensions().get::().is_none() { - let original_uri = OriginalUri(req.uri().clone()); - req.extensions_mut().insert(original_uri); - } - } - - let path = req.uri().path().to_owned(); - - match self.node.at(&path) { - Ok(match_) => { - match &self.fallback { - FallbackRoute::Default(_) => {} - FallbackRoute::Service(fallback) => { - req.extensions_mut() - .insert(SuperFallback(SyncWrapper::new(fallback.clone()))); - } - } - - self.call_route(match_, req) - } - Err( - MatchError::NotFound - | MatchError::ExtraTrailingSlash - | MatchError::MissingTrailingSlash, - ) => match &mut self.fallback { - FallbackRoute::Default(fallback) => { - if let Some(super_fallback) = req.extensions_mut().remove::>() - { - let mut super_fallback = super_fallback.0.into_inner(); - super_fallback.call(req) - } else { - fallback.call(req) - } - } - FallbackRoute::Service(fallback) => fallback.call(req), - }, - } - } -} diff --git a/axum/src/routing/tests/fallback.rs b/axum/src/routing/tests/fallback.rs index 923e10d1..8f070ad7 100644 --- a/axum/src/routing/tests/fallback.rs +++ b/axum/src/routing/tests/fallback.rs @@ -56,7 +56,7 @@ async fn fallback_accessing_state() { .fallback(|State(state): State<&'static str>| async move { state }) .with_state("state"); - let client = TestClient::from_service(app); + let client = TestClient::new(app); let res = client.get("/does-not-exist").send().await; assert_eq!(res.status(), StatusCode::OK); diff --git a/axum/src/routing/tests/get_to_head.rs b/axum/src/routing/tests/get_to_head.rs index f0cd201c..21888e6e 100644 --- a/axum/src/routing/tests/get_to_head.rs +++ b/axum/src/routing/tests/get_to_head.rs @@ -19,7 +19,6 @@ mod for_handlers { // don't use reqwest because it always strips bodies from HEAD responses let res = app - .into_service() .oneshot( Request::builder() .uri("/") @@ -55,7 +54,6 @@ mod for_services { // don't use reqwest because it always strips bodies from HEAD responses let res = app - .into_service() .oneshot( Request::builder() .uri("/") diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index 43db1512..ea09501e 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -447,11 +447,11 @@ async fn middleware_still_run_for_unmatched_requests() { #[tokio::test] #[should_panic(expected = "\ - Invalid route: `Router::route_service` cannot be used with `RouterService`s. \ + Invalid route: `Router::route_service` cannot be used with `Router`s. \ Use `Router::nest` instead\ ")] async fn routing_to_router_panics() { - TestClient::new(Router::new().route_service("/", Router::new().into_service())); + TestClient::new(Router::new().route_service("/", Router::new())); } #[tokio::test] @@ -761,7 +761,7 @@ async fn extract_state() { }; let app = Router::new().route("/", get(handler)).with_state(state); - let client = TestClient::from_service(app); + let client = TestClient::new(app); let res = client.get("/").send().await; assert_eq!(res.status(), StatusCode::OK); @@ -776,7 +776,7 @@ async fn explicitly_set_state() { ) .with_state("..."); - let client = TestClient::from_service(app); + let client = TestClient::new(app); let res = client.get("/").send().await; assert_eq!(res.text().await, "foo"); } diff --git a/axum/src/test_helpers/mod.rs b/axum/src/test_helpers/mod.rs index cddb38e5..0b60ee90 100644 --- a/axum/src/test_helpers/mod.rs +++ b/axum/src/test_helpers/mod.rs @@ -1,6 +1,6 @@ #![allow(clippy::disallowed_names)] -use crate::{body::HttpBody, BoxError, Router}; +use crate::{body::HttpBody, BoxError}; mod test_client; pub(crate) use self::test_client::*; diff --git a/axum/src/test_helpers/test_client.rs b/axum/src/test_helpers/test_client.rs index 296b8131..45a72b6c 100644 --- a/axum/src/test_helpers/test_client.rs +++ b/axum/src/test_helpers/test_client.rs @@ -1,4 +1,4 @@ -use super::{BoxError, HttpBody, Router}; +use super::{BoxError, HttpBody}; use bytes::Bytes; use http::{ header::{HeaderName, HeaderValue}, @@ -15,11 +15,7 @@ pub(crate) struct TestClient { } impl TestClient { - pub(crate) fn new(router: Router<(), Body>) -> Self { - Self::from_service(router.into_service()) - } - - pub(crate) fn from_service(svc: S) -> Self + pub(crate) fn new(svc: S) -> Self where S: Service, Response = http::Response> + Clone + Send + 'static, ResBody: HttpBody + Send + 'static,