diff --git a/axum/src/extract/matched_path.rs b/axum/src/extract/matched_path.rs index cdc04863..1cd26a45 100644 --- a/axum/src/extract/matched_path.rs +++ b/axum/src/extract/matched_path.rs @@ -149,7 +149,7 @@ mod tests { "/public", Router::new().route("/assets/*path", get(handler)), ) - .nest_service("/foo", handler.into_service()) + .nest_service("/foo", handler.into_service(())) .layer(tower::layer::layer_fn(SetMatchedPathExtension)) .state(()); diff --git a/axum/src/handler/into_extension_service.rs b/axum/src/handler/into_extension_service.rs new file mode 100644 index 00000000..a0b91f69 --- /dev/null +++ b/axum/src/handler/into_extension_service.rs @@ -0,0 +1,67 @@ +use super::Handler; +use crate::response::Response; +use http::Request; +use std::{ + convert::Infallible, + marker::PhantomData, + task::{Context, Poll}, +}; +use tower_service::Service; + +/// A `Handler` converted into a `Service` that reads the state from request extensions. Panics if +/// the state is missing. +pub(crate) struct IntoExtensionService { + handler: H, + _marker: PhantomData (S, T, B)>, +} + +impl IntoExtensionService { + pub(crate) fn new(handler: H) -> Self { + Self { + handler, + _marker: PhantomData, + } + } +} + +impl Clone for IntoExtensionService +where + H: Clone, +{ + fn clone(&self) -> Self { + Self { + handler: self.handler.clone(), + _marker: PhantomData, + } + } +} + +impl Service> for IntoExtensionService +where + H: Handler + Clone + Send + 'static, + B: Send + 'static, + S: Clone + Send + Sync + 'static, +{ + type Response = Response; + type Error = Infallible; + type Future = super::future::IntoServiceFuture; + + #[inline] + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + // `IntoService` can only be constructed from async functions which are always ready, or + // from `Layered` which bufferes in `::call` and is therefore + // also always ready. + Poll::Ready(Ok(())) + } + + fn call(&mut self, req: Request) -> Self::Future { + use futures_util::future::FutureExt; + + let handler = self.handler.clone(); + let state = req.extensions().get::().unwrap().clone(); + let future = Handler::call(handler, state, req); + let future = future.map(Ok as _); + + super::future::IntoServiceFuture::new(future) + } +} diff --git a/axum/src/handler/into_service.rs b/axum/src/handler/into_service.rs index 180ebdb1..c6f11f56 100644 --- a/axum/src/handler/into_service.rs +++ b/axum/src/handler/into_service.rs @@ -18,13 +18,6 @@ pub struct IntoService { _marker: PhantomData (T, B)>, } -#[test] -fn traits() { - use crate::test_helpers::*; - assert_send::>(); - assert_sync::>(); -} - impl IntoService { pub(super) fn new(handler: H, state: S) -> Self { Self { @@ -89,3 +82,10 @@ where super::future::IntoServiceFuture::new(future) } } + +#[test] +fn traits() { + use crate::test_helpers::*; + assert_send::>(); + assert_sync::>(); +} diff --git a/axum/src/handler/mod.rs b/axum/src/handler/mod.rs index dbf3d5ba..510fb4ae 100644 --- a/axum/src/handler/mod.rs +++ b/axum/src/handler/mod.rs @@ -43,16 +43,19 @@ use crate::{ BoxError, }; use http::Request; -use std::{fmt, future::Future, marker::PhantomData, pin::Pin}; +use std::{convert::Infallible, fmt, future::Future, marker::PhantomData, pin::Pin}; use tower::ServiceExt; use tower_layer::Layer; use tower_service::Service; -pub mod future; +mod into_extension_service; mod into_service; +pub(crate) use self::into_extension_service::IntoExtensionService; pub use self::into_service::IntoService; +pub mod future; + /// Trait for async functions that can be used to handle requests. /// /// You shouldn't need to depend on this trait directly. It is automatically @@ -61,7 +64,8 @@ pub use self::into_service::IntoService; /// See the [module docs](crate::handler) for more details. /// #[doc = include_str!("../docs/debugging_handler_type_errors.md")] -pub trait Handler: Clone + Send + Sized + 'static { +// TODO(david): Add back `B = Body` default +pub trait Handler: Clone + Send + Sized + 'static { /// The type of future calling this handler returns. type Future: Future + Send + 'static; @@ -104,13 +108,15 @@ pub trait Handler: Clone + Send + Sized + 'static { /// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap(); /// # }; /// ``` - fn layer(self, layer: L) -> Layered + fn layer(self, layer: L) -> Layered where L: Layer>, { - // TODO(david): write this, somehow - todo!() - // Layered::new(layer.layer(self.into_service())) + Layered { + handler: self, + layer, + _marker: PhantomData, + } } /// Convert the handler into a [`Service`]. @@ -145,6 +151,7 @@ pub trait Handler: Clone + Send + Sized + 'static { /// ``` /// /// [`Router::fallback`]: crate::routing::Router::fallback + // TODO(david): remove this fn into_service(self, state: S) -> IntoService { IntoService::new(self, state) } @@ -172,6 +179,7 @@ pub trait Handler: Clone + Send + Sized + 'static { /// ``` /// /// [`MakeService`]: tower::make::MakeService + // TODO(david): remove this fn into_make_service(self, state: S) -> IntoMakeService> { IntoMakeService::new(self.into_service(state)) } @@ -204,6 +212,7 @@ pub trait Handler: Clone + Send + Sized + 'static { /// /// [`MakeService`]: tower::make::MakeService /// [`Router::into_make_service_with_connect_info`]: crate::routing::Router::into_make_service_with_connect_info + // TODO(david): remove this fn into_make_service_with_connect_info( self, state: S, @@ -264,63 +273,79 @@ all_the_tuples!(impl_handler); /// A [`Service`] created from a [`Handler`] by applying a Tower middleware. /// /// Created with [`Handler::layer`]. See that method for more details. -pub struct Layered { - svc: S, - _input: PhantomData T>, +pub struct Layered { + handler: H, + layer: L, + _marker: PhantomData<(S, B)>, } -impl fmt::Debug for Layered +impl Clone for Layered where - S: fmt::Debug, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Layered").field("svc", &self.svc).finish() - } -} - -impl Clone for Layered -where - S: Clone, + H: Clone, + L: Clone, { fn clone(&self) -> Self { - Self::new(self.svc.clone()) + Self { + handler: self.handler.clone(), + layer: self.layer.clone(), + _marker: self._marker, + } } } -impl Handler for Layered +impl Copy for Layered where - S: Service, Response = Response> + Clone + Send + 'static, - S::Error: IntoResponse, - S::Future: Send, - T: 'static, - ReqBody: Send + 'static, + H: Copy, + L: Copy, +{ +} + +impl fmt::Debug for Layered +where + L: fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let Self { + handler: _, + layer, + _marker, + } = self; + f.debug_struct("Layered").field("layer", &layer).finish() + } +} + +impl Handler for Layered +where + H: Handler + Clone + Send + 'static, + S: Send + 'static, + L: Layer> + Clone + Send + 'static, + L::Service: Service, Response = Response, Error = Infallible> + + Clone + + Send + + 'static, + >>::Future: Send, + B: Send + 'static, ResBody: HttpBody + Send + 'static, ResBody::Error: Into, { - type Future = future::LayeredFuture; + type Future = future::LayeredFuture; - fn call(self, state: St, req: Request) -> Self::Future { + fn call(self, state: S, req: Request) -> Self::Future { use futures_util::future::{FutureExt, Map}; - let future: Map<_, fn(Result) -> _> = - self.svc.oneshot(req).map(|result| match result { + let svc = self.handler.into_service(state); + let svc = self.layer.layer(svc); + + let future: Map<_, fn(Result, Infallible>) -> _> = + svc.oneshot(req).map(|result| match result { Ok(res) => res.map(boxed), - Err(res) => res.into_response(), + Err(err) => match err {}, }); future::LayeredFuture::new(future) } } -impl Layered { - pub(crate) fn new(svc: S) -> Self { - Self { - svc, - _input: PhantomData, - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/axum/src/routing/method_routing.rs b/axum/src/routing/method_routing.rs index 3ac71594..d63ede9a 100644 --- a/axum/src/routing/method_routing.rs +++ b/axum/src/routing/method_routing.rs @@ -1,9 +1,9 @@ -use super::IntoMakeService; +use super::{IntoMakeService, MissingState, WithState}; use crate::{ body::{boxed, Body, Bytes, Empty, HttpBody}, error_handling::{HandleError, HandleErrorLayer}, - extract::connect_info::IntoMakeServiceWithConnectInfo, - handler::Handler, + extract::{connect_info::IntoMakeServiceWithConnectInfo, State}, + handler::{Handler, IntoExtensionService}, http::{Method, Request, StatusCode}, response::Response, routing::{future::RouteFuture, Fallback, MethodFilter, Route}, @@ -76,10 +76,10 @@ macro_rules! top_level_service_fn { $name:ident, $method:ident ) => { $(#[$m])+ - pub fn $name(svc: S) -> MethodRouter + pub fn $name(svc: T) -> MethodRouter where - S: Service, Response = Response> + Clone + Send + 'static, - S::Future: Send + 'static, + T: Service, Response = Response> + Clone + Send + 'static, + T::Future: Send + 'static, ResBody: HttpBody + Send + 'static, ResBody::Error: Into, { @@ -137,11 +137,12 @@ macro_rules! top_level_handler_fn { $name:ident, $method:ident ) => { $(#[$m])+ - pub fn $name(handler: H) -> MethodRouter + pub fn $name(handler: H) -> MethodRouter where - H: Handler, + H: Handler, B: Send + 'static, T: 'static, + S: Clone + Send + Sync + 'static, { on(MethodFilter::$method, handler) } @@ -208,13 +209,13 @@ macro_rules! chained_service_fn { $name:ident, $method:ident ) => { $(#[$m])+ - pub fn $name(self, svc: S) -> Self + pub fn $name(self, svc: T) -> Self where - S: Service, Response = Response, Error = E> + T: Service, Response = Response, Error = E> + Clone + Send + 'static, - S::Future: Send + 'static, + T::Future: Send + 'static, ResBody: HttpBody + Send + 'static, ResBody::Error: Into, { @@ -274,8 +275,9 @@ macro_rules! chained_handler_fn { $(#[$m])+ pub fn $name(self, handler: H) -> Self where - H: Handler, + H: Handler, T: 'static, + S: Clone + Send + Sync + 'static, { self.on(MethodFilter::$method, handler) } @@ -316,13 +318,13 @@ top_level_service_fn!(trace_service, TRACE); /// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap(); /// # }; /// ``` -pub fn on_service( +pub fn on_service( filter: MethodFilter, - svc: S, -) -> MethodRouter + svc: T, +) -> MethodRouter where - S: Service, Response = Response> + Clone + Send + 'static, - S::Future: Send + 'static, + T: Service, Response = Response> + Clone + Send + 'static, + T::Future: Send + 'static, ResBody: HttpBody + Send + 'static, ResBody::Error: Into, { @@ -382,14 +384,18 @@ where /// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap(); /// # }; /// ``` -pub fn any_service(svc: S) -> MethodRouter +pub fn any_service( + svc: T, +) -> MethodRouter where - S: Service, Response = Response> + Clone + Send + 'static, - S::Future: Send + 'static, + T: Service, Response = Response> + Clone + Send + 'static, + T::Future: Send + 'static, ResBody: HttpBody + Send + 'static, ResBody::Error: Into, { - MethodRouter::new().fallback(svc).skip_allow_header() + MethodRouter::new() + .fallback_service(svc) + .skip_allow_header() } top_level_handler_fn!(delete, DELETE); @@ -420,11 +426,15 @@ top_level_handler_fn!(trace, TRACE); /// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap(); /// # }; /// ``` -pub fn on(filter: MethodFilter, handler: H) -> MethodRouter +pub fn on( + filter: MethodFilter, + handler: H, +) -> MethodRouter where - H: Handler, + H: Handler, B: Send + 'static, T: 'static, + S: Clone + Send + Sync + 'static, { MethodRouter::new().on(filter, handler) } @@ -466,20 +476,26 @@ where /// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap(); /// # }; /// ``` -pub fn any(handler: H) -> MethodRouter +pub fn any(handler: H) -> MethodRouter where - H: Handler, + H: Handler, B: Send + 'static, T: 'static, + S: Clone + Send + Sync + 'static, { MethodRouter::new() - .fallback_boxed_response_body(handler.into_service()) + .fallback_boxed_response_body(IntoExtensionService::new(handler)) .skip_allow_header() } /// A [`Service`] that accepts requests based on a [`MethodFilter`] and /// allows chaining additional handlers and services. -pub struct MethodRouter { +// TODO(david): Bring back `B = Body, E = Infallible` defaults +pub struct MethodRouter { + // Invariant: If `R == MissingState` then `state` is `None` + // If `R == WithState` then state is `Some` + // `R` cannot have other values + state: Option, get: Option>, head: Option>, delete: Option>, @@ -490,7 +506,7 @@ pub struct MethodRouter { trace: Option>, fallback: Fallback, allow_header: AllowHeader, - _request_body: PhantomData (B, E)>, + _marker: PhantomData, } #[derive(Clone)] @@ -503,9 +519,13 @@ enum AllowHeader { Bytes(BytesMut), } -impl fmt::Debug for MethodRouter { +impl fmt::Debug for MethodRouter +where + S: fmt::Debug, +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("MethodRouter") + .field("state", &self.state) .field("get", &self.get) .field("head", &self.head) .field("delete", &self.delete) @@ -519,7 +539,7 @@ impl fmt::Debug for MethodRouter { } } -impl MethodRouter { +impl MethodRouter { /// Create a default `MethodRouter` that will respond with `405 Method Not Allowed` to all /// requests. pub fn new() -> Self { @@ -530,6 +550,7 @@ impl MethodRouter { })); Self { + state: None, get: None, head: None, delete: None, @@ -540,14 +561,47 @@ impl MethodRouter { trace: None, allow_header: AllowHeader::None, fallback: Fallback::Default(fallback), - _request_body: PhantomData, + _marker: PhantomData, + } + } + + /// TODO(david): docs + pub fn state(self, state: S) -> MethodRouter { + MethodRouter { + state: Some(state), + get: self.get, + head: self.head, + delete: self.delete, + options: self.options, + patch: self.patch, + post: self.post, + put: self.put, + trace: self.trace, + fallback: self.fallback, + allow_header: self.allow_header, + _marker: PhantomData, } } } -impl MethodRouter +impl MethodRouter { + /// TODO(david): docs + pub fn with_state(state: S) -> Self { + MethodRouter::new().state(state) + } +} + +impl MethodRouter<(), B, E, WithState> { + /// TODO(david): docs + pub fn without_state() -> Self { + MethodRouter::with_state(()) + } +} + +impl MethodRouter where B: Send + 'static, + S: Clone + Send + Sync + 'static, { /// Chain an additional handler that will accept requests matching the given /// `MethodFilter`. @@ -574,10 +628,10 @@ where /// ``` pub fn on(self, filter: MethodFilter, handler: H) -> Self where - H: Handler, + H: Handler, T: 'static, { - self.on_service_boxed_response_body(filter, handler.into_service()) + self.on_service_boxed_response_body(filter, IntoExtensionService::new(handler)) } chained_handler_fn!(delete, DELETE); @@ -589,6 +643,22 @@ where chained_handler_fn!(put, PUT); chained_handler_fn!(trace, TRACE); + #[doc = include_str!("../docs/routing/fallback.md")] + pub fn fallback(mut self, handler: H) -> Self + where + H: Handler, + T: 'static, + S: Clone + Send + Sync + 'static, + { + self.fallback_boxed_response_body(IntoExtensionService::new(handler)) + } +} + +impl MethodRouter +where + B: Send + 'static, + S: Clone + Send + Sync + 'static, +{ /// Convert the handler into a [`MakeService`]. /// /// This allows you to serve a single handler if you don't need any routing: @@ -658,7 +728,7 @@ where } } -impl MethodRouter { +impl MethodRouter { /// Chain an additional service that will accept requests matching the given /// `MethodFilter`. /// @@ -684,13 +754,10 @@ impl MethodRouter { /// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap(); /// # }; /// ``` - pub fn on_service(self, filter: MethodFilter, svc: S) -> Self + pub fn on_service(self, filter: MethodFilter, svc: T) -> Self where - S: Service, Response = Response, Error = E> - + Clone - + Send - + 'static, - S::Future: Send + 'static, + T: Service, Response = Response, Error = E> + Clone + Send + 'static, + T::Future: Send + 'static, ResBody: HttpBody + Send + 'static, ResBody::Error: Into, { @@ -707,13 +774,10 @@ impl MethodRouter { chained_service_fn!(trace_service, TRACE); #[doc = include_str!("../docs/method_routing/fallback.md")] - pub fn fallback(mut self, svc: S) -> Self + pub fn fallback_service(mut self, svc: T) -> Self where - S: Service, Response = Response, Error = E> - + Clone - + Send - + 'static, - S::Future: Send + 'static, + T: Service, Response = Response, Error = E> + Clone + Send + 'static, + T::Future: Send + 'static, ResBody: HttpBody + Send + 'static, ResBody::Error: Into, { @@ -721,10 +785,10 @@ impl MethodRouter { self } - fn fallback_boxed_response_body(mut self, svc: S) -> Self + fn fallback_boxed_response_body(mut self, svc: T) -> Self where - S: Service, Response = Response, Error = E> + Clone + Send + 'static, - S::Future: Send + 'static, + T: Service, Response = Response, Error = E> + Clone + Send + 'static, + T::Future: Send + 'static, { self.fallback = Fallback::Custom(Route::new(svc)); self @@ -734,9 +798,9 @@ impl MethodRouter { pub fn layer( self, layer: L, - ) -> MethodRouter + ) -> MethodRouter where - L: Layer>, + L: Layer>, L::Service: Service, Response = Response, Error = NewError> + Clone + Send @@ -753,6 +817,7 @@ impl MethodRouter { let layer_fn = |s| layer.layer(s); MethodRouter { + state: self.state, get: self.get.map(layer_fn), head: self.head.map(layer_fn), delete: self.delete.map(layer_fn), @@ -763,19 +828,19 @@ impl MethodRouter { trace: self.trace.map(layer_fn), fallback: self.fallback.map(layer_fn), allow_header: self.allow_header, - _request_body: PhantomData, + _marker: self._marker, } } #[doc = include_str!("../docs/method_routing/route_layer.md")] - pub fn route_layer(self, layer: L) -> MethodRouter + pub fn route_layer(self, layer: L) -> MethodRouter where - L: Layer>, - L::Service: Service, Response = Response, Error = E> + L: Layer>, + L::Service: Service, Response = Response, Error = E> + Clone + Send + 'static, - >>::Future: Send + 'static, + >>::Future: Send + 'static, NewResBody: HttpBody + Send + 'static, NewResBody::Error: Into, { @@ -787,6 +852,7 @@ impl MethodRouter { let layer_fn = |s| layer.layer(s); MethodRouter { + state: self.state, get: self.get.map(layer_fn), head: self.head.map(layer_fn), delete: self.delete.map(layer_fn), @@ -797,12 +863,12 @@ impl MethodRouter { trace: self.trace.map(layer_fn), fallback: self.fallback, allow_header: self.allow_header, - _request_body: PhantomData, + _marker: self._marker, } } #[doc = include_str!("../docs/method_routing/merge.md")] - pub fn merge(self, other: MethodRouter) -> Self { + pub fn merge(self, other: MethodRouter) -> Self { macro_rules! merge { ( $first:ident, $second:ident ) => { match ($first, $second) { @@ -819,6 +885,7 @@ impl MethodRouter { } let Self { + state, get, head, delete, @@ -829,10 +896,11 @@ impl MethodRouter { trace, fallback, allow_header, - _request_body: _, + _marker: _, } = self; - let Self { + let MethodRouter { + state: state_other, get: get_other, head: head_other, delete: delete_other, @@ -843,8 +911,9 @@ impl MethodRouter { trace: trace_other, fallback: fallback_other, allow_header: allow_header_other, - _request_body: _, + _marker: _, } = other; + debug_assert!(state_other.is_none()); let get = merge!(get, get_other); let head = merge!(head, head_other); @@ -877,6 +946,7 @@ impl MethodRouter { }; Self { + state, get, head, delete, @@ -887,30 +957,30 @@ impl MethodRouter { trace, fallback, allow_header, - _request_body: PhantomData, + _marker: PhantomData, } } /// Apply a [`HandleErrorLayer`]. /// /// This is a convenience method for doing `self.layer(HandleErrorLayer::new(f))`. - pub fn handle_error(self, f: F) -> MethodRouter + pub fn handle_error(self, f: F) -> MethodRouter where F: Clone + Send + 'static, - HandleError, F, T>: - Service, Response = Response, Error = Infallible>, - , F, T> as Service>>::Future: Send, + HandleError, F, T>: + Service, Response = Response, Error = Infallible>, + , F, T> as Service>>::Future: Send, T: 'static, E: 'static, - ReqBody: 'static, + B: 'static, { self.layer(HandleErrorLayer::new(f)) } - fn on_service_boxed_response_body(self, filter: MethodFilter, svc: S) -> Self + fn on_service_boxed_response_body(self, filter: MethodFilter, svc: T) -> Self where - S: Service, Response = Response, Error = E> + Clone + Send + 'static, - S::Future: Send + 'static, + T: Service, Response = Response, Error = E> + Clone + Send + 'static, + T::Future: Send + 'static, { macro_rules! set_service { ( @@ -940,6 +1010,7 @@ impl MethodRouter { // written with a pattern match like this to ensure we update all fields let Self { + state, mut get, mut head, mut delete, @@ -950,7 +1021,7 @@ impl MethodRouter { mut trace, fallback, mut allow_header, - _request_body: _, + _marker, } = self; let svc = Some(Route::new(svc)); set_service!( @@ -969,6 +1040,7 @@ impl MethodRouter { ] ); Self { + state, get, head, delete, @@ -979,7 +1051,7 @@ impl MethodRouter { trace, fallback, allow_header, - _request_body: PhantomData, + _marker, } } @@ -1009,9 +1081,13 @@ fn append_allow_header(allow_header: &mut AllowHeader, method: &'static str) { } } -impl Clone for MethodRouter { +impl Clone for MethodRouter +where + S: Clone, +{ fn clone(&self) -> Self { Self { + state: self.state.clone(), get: self.get.clone(), head: self.head.clone(), delete: self.delete.clone(), @@ -1022,12 +1098,12 @@ impl Clone for MethodRouter { trace: self.trace.clone(), fallback: self.fallback.clone(), allow_header: self.allow_header.clone(), - _request_body: PhantomData, + _marker: self._marker, } } } -impl Default for MethodRouter +impl Default for MethodRouter where B: Send + 'static, { @@ -1036,9 +1112,10 @@ where } } -impl Service> for MethodRouter +impl Service> for MethodRouter where B: HttpBody, + S: Clone + Send + Sync + 'static, { type Response = Response; type Error = E; @@ -1049,7 +1126,7 @@ where Poll::Ready(Ok(())) } - fn call(&mut self, req: Request) -> Self::Future { + fn call(&mut self, mut req: Request) -> Self::Future { macro_rules! call { ( $req:expr, @@ -1070,6 +1147,7 @@ where // written with a pattern match like this to ensure we call all routes let Self { + state, get, head, delete, @@ -1080,9 +1158,17 @@ where trace, fallback, allow_header, - _request_body: _, + _marker, } = self; + if req.extensions().get::>().is_none() { + // the `unwrap` is safe because `self.state` is always some if `R = WithState`, which it is + let prev = req + .extensions_mut() + .insert(State(state.as_ref().unwrap().clone())); + debug_assert!(prev.is_none()); + } + call!(req, method, HEAD, head); call!(req, method, HEAD, get); call!(req, method, GET, get); @@ -1120,7 +1206,7 @@ mod tests { #[tokio::test] async fn method_not_allowed_by_default() { - let mut svc = MethodRouter::new(); + let mut svc = MethodRouter::new().state(()); let (status, _, body) = call(Method::GET, &mut svc).await; assert_eq!(status, StatusCode::METHOD_NOT_ALLOWED); assert!(body.is_empty()); @@ -1128,7 +1214,7 @@ mod tests { #[tokio::test] async fn get_handler() { - let mut svc = MethodRouter::new().get(ok); + let mut svc = MethodRouter::without_state().get(ok); let (status, _, body) = call(Method::GET, &mut svc).await; assert_eq!(status, StatusCode::OK); assert_eq!(body, "ok"); @@ -1136,7 +1222,7 @@ mod tests { #[tokio::test] async fn get_accepts_head() { - let mut svc = MethodRouter::new().get(ok); + let mut svc = MethodRouter::without_state().get(ok); let (status, _, body) = call(Method::HEAD, &mut svc).await; assert_eq!(status, StatusCode::OK); assert!(body.is_empty()); @@ -1144,7 +1230,7 @@ mod tests { #[tokio::test] async fn head_takes_precedence_over_get() { - let mut svc = MethodRouter::new().head(created).get(ok); + let mut svc = MethodRouter::without_state().head(created).get(ok); let (status, _, body) = call(Method::HEAD, &mut svc).await; assert_eq!(status, StatusCode::CREATED); assert!(body.is_empty()); @@ -1152,7 +1238,7 @@ mod tests { #[tokio::test] async fn merge() { - let mut svc = get(ok).merge(post(ok)); + let mut svc = get(ok).merge(post(ok)).state(()); let (status, _, _) = call(Method::GET, &mut svc).await; assert_eq!(status, StatusCode::OK); @@ -1163,7 +1249,7 @@ mod tests { #[tokio::test] async fn layer() { - let mut svc = MethodRouter::new() + let mut svc = MethodRouter::without_state() .get(|| async { std::future::pending::<()>().await }) .layer(RequireAuthorizationLayer::bearer("password")); @@ -1178,7 +1264,7 @@ mod tests { #[tokio::test] async fn route_layer() { - let mut svc = MethodRouter::new() + let mut svc = MethodRouter::without_state() .get(|| async { std::future::pending::<()>().await }) .route_layer(RequireAuthorizationLayer::bearer("password")); @@ -1204,7 +1290,7 @@ mod tests { delete_service(ServeDir::new(".")) .handle_error(|_| async { StatusCode::NOT_FOUND }), ) - .fallback((|| async { StatusCode::NOT_FOUND }).into_service()) + .fallback(|| async { StatusCode::NOT_FOUND }) .put(ok) .layer( ServiceBuilder::new() @@ -1221,7 +1307,7 @@ mod tests { #[tokio::test] async fn sets_allow_header() { - let mut svc = MethodRouter::new().put(ok).patch(ok); + let mut svc = MethodRouter::without_state().put(ok).patch(ok); let (status, headers, _) = call(Method::GET, &mut svc).await; assert_eq!(status, StatusCode::METHOD_NOT_ALLOWED); assert_eq!(headers[ALLOW], "PUT,PATCH"); @@ -1229,7 +1315,7 @@ mod tests { #[tokio::test] async fn sets_allow_header_get_head() { - let mut svc = MethodRouter::new().get(ok).head(ok); + let mut svc = MethodRouter::without_state().get(ok).head(ok); let (status, headers, _) = call(Method::PUT, &mut svc).await; assert_eq!(status, StatusCode::METHOD_NOT_ALLOWED); assert_eq!(headers[ALLOW], "GET,HEAD"); @@ -1237,7 +1323,7 @@ mod tests { #[tokio::test] async fn empty_allow_header_by_default() { - let mut svc = MethodRouter::new(); + let mut svc = MethodRouter::without_state(); let (status, headers, _) = call(Method::PATCH, &mut svc).await; assert_eq!(status, StatusCode::METHOD_NOT_ALLOWED); assert_eq!(headers[ALLOW], ""); @@ -1247,7 +1333,7 @@ mod tests { async fn allow_header_when_merging() { let a = put(ok).patch(ok); let b = get(ok).head(ok); - let mut svc = a.merge(b); + let mut svc = a.merge(b).state(()); let (status, headers, _) = call(Method::DELETE, &mut svc).await; assert_eq!(status, StatusCode::METHOD_NOT_ALLOWED); @@ -1256,7 +1342,7 @@ mod tests { #[tokio::test] async fn allow_header_any() { - let mut svc = any(ok); + let mut svc = any(ok).state(()); let (status, headers, _) = call(Method::GET, &mut svc).await; assert_eq!(status, StatusCode::OK); @@ -1265,9 +1351,9 @@ mod tests { #[tokio::test] async fn allow_header_with_fallback() { - let mut svc = MethodRouter::new().get(ok).fallback( - (|| async { (StatusCode::METHOD_NOT_ALLOWED, "Method not allowed") }).into_service(), - ); + let mut svc = MethodRouter::without_state() + .get(ok) + .fallback(|| async { (StatusCode::METHOD_NOT_ALLOWED, "Method not allowed") }); let (status, headers, _) = call(Method::DELETE, &mut svc).await; assert_eq!(status, StatusCode::METHOD_NOT_ALLOWED); @@ -1289,9 +1375,7 @@ mod tests { } } - let mut svc = MethodRouter::new() - .get(ok) - .fallback(fallback.into_service()); + let mut svc = MethodRouter::without_state().get(ok).fallback(fallback); let (status, _, _) = call(Method::GET, &mut svc).await; assert_eq!(status, StatusCode::OK); @@ -1309,7 +1393,7 @@ mod tests { expected = "Overlapping method route. Cannot add two method routes that both handle `GET`" )] async fn handler_overlaps() { - let _: MethodRouter = get(ok).get(ok); + let _: MethodRouter<(), Body, Infallible, _> = get(ok).get(ok); } #[tokio::test] @@ -1317,17 +1401,18 @@ mod tests { expected = "Overlapping method route. Cannot add two method routes that both handle `POST`" )] async fn service_overlaps() { - let _: MethodRouter = post_service(ok.into_service()).post_service(ok.into_service()); + let _: MethodRouter<(), Body, Infallible, _> = + post_service(ok.into_service(())).post_service(ok.into_service(())); } #[tokio::test] async fn get_head_does_not_overlap() { - let _: MethodRouter = get(ok).head(ok); + let _: MethodRouter<(), Body, Infallible, _> = get(ok).head(ok); } #[tokio::test] async fn head_get_does_not_overlap() { - let _: MethodRouter = head(ok).get(ok); + let _: MethodRouter<(), Body, Infallible, _> = head(ok).get(ok); } async fn call(method: Method, svc: &mut S) -> (StatusCode, HeaderMap, String) diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index b7417231..e88daad8 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -4,6 +4,7 @@ use self::{future::RouteFuture, not_found::NotFound}; use crate::{ body::{boxed, Body, Bytes, HttpBody}, extract::connect_info::IntoMakeServiceWithConnectInfo, + handler::{Handler, IntoExtensionService}, response::Response, routing::strip_prefix::StripPrefix, util::try_downcast, @@ -68,7 +69,7 @@ pub struct Router { // If `R == WithState` then state is `Some` // `R` cannot have other values state: Option, - routes: HashMap>, + routes: HashMap>, node: Arc, fallback: Fallback, _marker: PhantomData, @@ -141,10 +142,27 @@ where } /// TODO(david): docs - pub fn state(self, state: S) -> Router { + pub fn state(self, state: S) -> Router + where + S: Clone, + { + let routes = self + .routes + .into_iter() + .map(|(id, endpoint)| { + let endpoint = match endpoint { + Endpoint::MethodRouter(router) => { + Endpoint::MethodRouter(router.state(state.clone())) + } + Endpoint::Route(route) => Endpoint::Route(route), + }; + (id, endpoint) + }) + .collect(); + Router { state: Some(state), - routes: self.routes, + routes, node: self.node, fallback: self.fallback, _marker: PhantomData, @@ -155,6 +173,7 @@ where impl Router where B: HttpBody + Send + 'static, + S: Clone, { /// TODO(david): docs pub fn with_state(state: S) -> Self { @@ -162,6 +181,16 @@ where } } +impl Router<(), B, WithState> +where + B: HttpBody + Send + 'static, +{ + /// TODO(david): docs + pub fn without_state() -> Self { + Router::with_state(()) + } +} + impl Router where B: HttpBody + Send + 'static, @@ -169,72 +198,78 @@ where R: 'static, { #[doc = include_str!("../docs/routing/route.md")] - pub fn route(mut self, path: &str, service: T) -> Self + pub fn route( + mut self, + path: &str, + // TODO(david): constrain this so it only accepts methods + // routers containing handlers + method_router: MethodRouter, + ) -> Self { + self + } + + /// TODO(david): docs + pub fn route_service(mut self, path: &str, service: T) -> Self where T: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, T::Future: Send + 'static, { - if path.is_empty() { - panic!("Paths must start with a `/`. Use \"/\" for root routes"); - } else if !path.starts_with('/') { - panic!("Paths must start with a `/`"); - } - - // Downcase to `WithState` rather than `R` because `Router` only implements - // `Service` if `R == WithState` so any other type of `R` cannot be passed to `.router` in - // the first place - let service = match try_downcast::, _>(service) { - Ok(_) => { - panic!("Invalid route: `Router::route` cannot be used with `Router`s. Use `Router::nest` instead") - } - Err(svc) => svc, - }; - - let id = RouteId::next(); - - let service = match try_downcast::, _>(service) { - Ok(method_router) => { - if let Some((route_id, Endpoint::MethodRouter(prev_method_router))) = self - .node - .path_to_route_id - .get(path) - .and_then(|route_id| self.routes.get(route_id).map(|svc| (*route_id, svc))) - { - // if we're adding a new `MethodRouter` to a route that already has one just - // merge them. This makes `.route("/", get(_)).route("/", post(_))` work - let service = - Endpoint::MethodRouter(prev_method_router.clone().merge(method_router)); - self.routes.insert(route_id, service); - return self; - } else { - Endpoint::MethodRouter(method_router) - } - } - Err(service) => Endpoint::Route(Route::new(service)), - }; - - let mut node = - Arc::try_unwrap(Arc::clone(&self.node)).unwrap_or_else(|node| (*node).clone()); - if let Err(err) = node.insert(path, id) { - panic!("Invalid route: {}", err); - } - self.node = Arc::new(node); - - self.routes.insert(id, service); - self + + // if path.is_empty() { + // panic!("Paths must start with a `/`. Use \"/\" for root routes"); + // } else if !path.starts_with('/') { + // panic!("Paths must start with a `/`"); + // } + + // // Downcase to `WithState` rather than `R` because `Router` only implements + // // `Service` if `R == WithState` so any other type of `R` cannot be passed to `.router` in + // // the first place + // let service = match try_downcast::, _>(service) { + // Ok(_) => { + // panic!("Invalid route: `Router::route` cannot be used with `Router`s. Use `Router::nest` instead") + // } + // Err(svc) => svc, + // }; + + // let id = RouteId::next(); + + // let service = match try_downcast::, _>(service) { + // Ok(method_router) => { + // if let Some((route_id, Endpoint::MethodRouter(prev_method_router))) = self + // .node + // .path_to_route_id + // .get(path) + // .and_then(|route_id| self.routes.get(route_id).map(|svc| (*route_id, svc))) + // { + // // if we're adding a new `MethodRouter` to a route that already has one just + // // merge them. This makes `.route("/", get(_)).route("/", post(_))` work + // let service = + // Endpoint::MethodRouter(prev_method_router.clone().merge(method_router)); + // self.routes.insert(route_id, service); + // return self; + // } else { + // Endpoint::MethodRouter(method_router) + // } + // } + // Err(service) => Endpoint::Route(Route::new(service)), + // }; + + // let mut node = + // Arc::try_unwrap(Arc::clone(&self.node)).unwrap_or_else(|node| (*node).clone()); + // if let Err(err) = node.insert(path, id) { + // panic!("Invalid route: {}", err); + // } + // self.node = Arc::new(node); + + // self.routes.insert(id, service); + + // self } #[doc = include_str!("../docs/routing/nest.md")] pub fn nest(mut self, mut path: &str, router: Router) -> Self { - if path.is_empty() { - // nesting at `""` and `"/"` should mean the same thing - path = "/"; - } - - if path.contains('*') { - panic!("Invalid route: nested routes cannot contain wildcards (*)"); - } + validate_path_for_nest(&mut path); let prefix = path; @@ -268,7 +303,9 @@ where &full_path, method_router.layer(layer_fn(|s| StripPrefix::new(s, prefix))), ), - Endpoint::Route(route) => self.route(&full_path, StripPrefix::new(route, prefix)), + Endpoint::Route(route) => { + self.route_service(&full_path, StripPrefix::new(route, prefix)) + } }; } @@ -283,14 +320,7 @@ where T: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, T::Future: Send + 'static, { - if path.is_empty() { - // nesting at `""` and `"/"` should mean the same thing - path = "/"; - } - - if path.contains('*') { - panic!("Invalid route: nested routes cannot contain wildcards (*)"); - } + validate_path_for_nest(&mut path); let prefix = path; @@ -301,14 +331,14 @@ where }; let svc = strip_prefix::StripPrefix::new(svc, prefix); - self = self.route(&path, svc.clone()); + self = self.route_service(&path, svc.clone()); // `/*rest` is not matched by `/` so we need to also register a router at the // prefix itself. Otherwise if you were to nest at `/foo` then `/foo` itself // wouldn't match, which it should - self = self.route(prefix, svc.clone()); + self = self.route_service(prefix, svc.clone()); // same goes for `/foo/`, that should also match - self = self.route(&format!("{}/", prefix), svc); + self = self.route_service(&format!("{}/", prefix), svc); self } @@ -335,7 +365,7 @@ where .expect("no path for route id. This is a bug in axum. Please file an issue"); self = match route { Endpoint::MethodRouter(route) => self.route(path, route), - Endpoint::Route(route) => self.route(path, route), + Endpoint::Route(route) => self.route_service(path, route), }; } @@ -433,7 +463,17 @@ where } #[doc = include_str!("../docs/routing/fallback.md")] - pub fn fallback(mut self, svc: T) -> Self + pub fn fallback(mut self, handler: H) -> Self + where + H: Handler, + T: 'static, + S: Clone + Send + Sync + 'static, + { + self.fallback_service(IntoExtensionService::new(handler)) + } + + /// TODO(david): docs + pub fn fallback_service(mut self, svc: T) -> Self where T: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, T::Future: Send + 'static, @@ -441,7 +481,13 @@ where self.fallback = Fallback::Custom(Route::new(svc)); self } +} +impl Router +where + B: HttpBody + Send + 'static, + S: Clone + Send + Sync + 'static, +{ /// Convert this router into a [`MakeService`], that is a [`Service`] whose /// response is another service. /// @@ -473,13 +519,7 @@ where pub fn into_make_service_with_connect_info(self) -> IntoMakeServiceWithConnectInfo { IntoMakeServiceWithConnectInfo::new(self) } -} -impl Router -where - B: HttpBody + Send + 'static, - S: Clone + Send + Sync + 'static, -{ #[inline] fn call_route( &self, @@ -515,10 +555,6 @@ where url_params::insert_url_params(req.extensions_mut(), match_.params); - // the `unwrap` is safe because `self.state` is always some if `R = WithState`, which it is - req.extensions_mut() - .insert(crate::extract::State(self.state.as_ref().unwrap().clone())); - let mut route = self .routes .get(&id) @@ -560,6 +596,12 @@ where let path = req.uri().path().to_owned(); + // the `unwrap` is safe because `self.state` is always some if `R = WithState`, which it is + let prev = req + .extensions_mut() + .insert(crate::extract::State(self.state.as_ref().unwrap().clone())); + debug_assert!(prev.is_none()); + match self.node.at(&path) { Ok(match_) => self.call_route(match_, req), Err( @@ -580,6 +622,17 @@ pub enum MissingState {} #[derive(Copy, Clone, Debug)] pub enum WithState {} +fn validate_path_for_nest(path: &mut &str) { + if path.is_empty() { + // nesting at `""` and `"/"` should mean the same thing + *path = "/"; + } + + if path.contains('*') { + panic!("Invalid route: nested routes cannot contain wildcards (*)"); + } +} + /// Wrapper around `matchit::Router` that supports merging two `Router`s. #[derive(Clone, Default)] struct Node { @@ -656,12 +709,15 @@ impl Fallback { } } -enum Endpoint { - MethodRouter(MethodRouter), +enum Endpoint { + MethodRouter(MethodRouter), Route(Route), } -impl Clone for Endpoint { +impl Clone for Endpoint +where + S: Clone, +{ fn clone(&self) -> Self { match self { Endpoint::MethodRouter(inner) => Endpoint::MethodRouter(inner.clone()), @@ -670,7 +726,10 @@ impl Clone for Endpoint { } } -impl fmt::Debug for Endpoint { +impl fmt::Debug for Endpoint +where + S: fmt::Debug, +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::MethodRouter(inner) => inner.fmt(f), diff --git a/axum/src/routing/tests/fallback.rs b/axum/src/routing/tests/fallback.rs index bd25e43e..6320aa9b 100644 --- a/axum/src/routing/tests/fallback.rs +++ b/axum/src/routing/tests/fallback.rs @@ -1,11 +1,10 @@ use super::*; -use crate::handler::Handler; #[tokio::test] async fn basic() { let app = Router::new() .route("/foo", get(|| async {})) - .fallback((|| async { "fallback" }).into_service()) + .fallback(|| async { "fallback" }) .state(()); let client = TestClient::new(app); @@ -21,7 +20,7 @@ async fn basic() { async fn nest() { let app = Router::new() .nest("/foo", Router::new().route("/bar", get(|| async {}))) - .fallback((|| async { "fallback" }).into_service()) + .fallback(|| async { "fallback" }) .state(()); let client = TestClient::new(app); @@ -38,10 +37,7 @@ async fn or() { let one = Router::new().route("/one", get(|| async {})); let two = Router::new().route("/two", get(|| async {})); - let app = one - .merge(two) - .fallback((|| async { "fallback" }).into_service()) - .state(()); + let app = one.merge(two).fallback(|| async { "fallback" }).state(()); let client = TestClient::new(app); diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index f644c2d7..1648daf1 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -147,7 +147,10 @@ async fn routing_between_services() { }), ), ) - .route("/two", on_service(MethodFilter::GET, handle.into_service())); + .route( + "/two", + on_service(MethodFilter::GET, handle.into_service(())), + ); let client = TestClient::new(app.state(())); @@ -445,7 +448,11 @@ async fn middleware_still_run_for_unmatched_requests() { expected = "Invalid route: `Router::route` cannot be used with `Router`s. Use `Router::nest` instead" )] async fn routing_to_router_panics() { - TestClient::new(Router::new().route("/", Router::new().state(())).state(())); + TestClient::new( + Router::new() + .route_service("/", Router::new().state(())) + .state(()), + ); } #[tokio::test] @@ -520,8 +527,8 @@ async fn different_methods_added_in_different_routes_deeply_nested() { #[should_panic(expected = "Cannot merge two `Router`s that both have a fallback")] async fn merging_routers_with_fallbacks_panics() { async fn fallback() {} - let one = Router::new().fallback(fallback.into_service()); - let two = Router::new().fallback(fallback.into_service()); + let one = Router::new().fallback(fallback); + let two = Router::new().fallback(fallback); TestClient::new(one.merge(two).state(())); } @@ -529,7 +536,7 @@ async fn merging_routers_with_fallbacks_panics() { #[should_panic(expected = "Cannot nest `Router`s that has a fallback")] async fn nesting_router_with_fallbacks_panics() { async fn fallback() {} - let one = Router::new().fallback(fallback.into_service()); + let one = Router::new().fallback(fallback); let app = Router::new().nest("/", one); TestClient::new(app.state(())); } @@ -569,7 +576,7 @@ async fn head_content_length_through_hyper_server() { #[tokio::test] async fn head_content_length_through_hyper_server_that_hits_fallback() { - let app = Router::new().fallback((|| async { "foo" }).into_service()); + let app = Router::new().fallback(|| async { "foo" }); let client = TestClient::new(app.state(())); diff --git a/axum/src/routing/tests/nest.rs b/axum/src/routing/tests/nest.rs index 7819b620..bb191d7e 100644 --- a/axum/src/routing/tests/nest.rs +++ b/axum/src/routing/tests/nest.rs @@ -114,7 +114,10 @@ async fn nesting_router_at_empty_path() { #[tokio::test] async fn nesting_handler_at_root() { - let app = Router::new().nest_service("/", get(|uri: Uri| async move { uri.to_string() })); + let app = Router::new().nest_service( + "/", + get(|uri: Uri| async move { uri.to_string() }).state(()), + ); let client = TestClient::new(app.state(())); @@ -183,7 +186,7 @@ async fn nested_service_sees_stripped_uri() { "/foo", Router::new().nest( "/bar", - Router::new().route( + Router::new().route_service( "/baz", service_fn(|req: Request| async move { let body = boxed(Body::from(req.uri().to_string())); @@ -204,12 +207,15 @@ async fn nested_service_sees_stripped_uri() { async fn nest_static_file_server() { let app = Router::new().nest_service( "/static", - get_service(ServeDir::new(".")).handle_error(|error| async move { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Unhandled internal error: {}", error), - ) - }), + get_service(ServeDir::new(".")) + .handle_error(|error| async move { + ( + StatusCode::INTERNAL_SERVER_ERROR, + format!("Unhandled internal error: {}", error), + ) + }) + // TODO(david): having to do this on services isn't good + .state(()), ); let client = TestClient::new(app.state(())); @@ -330,7 +336,7 @@ async fn outer_middleware_still_see_whole_url() { .route("/foo", get(handler)) .route("/foo/bar", get(handler)) .nest("/one", Router::new().route("/two", get(handler))) - .fallback(handler.into_service()) + .fallback(handler) .layer(tower::layer::layer_fn(SetUriExtension)); let client = TestClient::new(app.state(())); @@ -366,7 +372,7 @@ async fn nest_at_capture() { #[tokio::test] async fn nest_with_and_without_trailing() { - let app = Router::new().nest_service("/foo", get(|| async {})); + let app = Router::new().nest_service("/foo", get(|| async {}).state(())); let client = TestClient::new(app.state(()));