diff --git a/Cargo.toml b/Cargo.toml index a221fbc5..a84f6fdd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,8 +2,8 @@ members = [ "axum", "axum-core", - "axum-extra", - "axum-macros", + # "axum-extra", + # "axum-macros", # internal crate used to bump the minimum versions we # get for some dependencies which otherwise wouldn't build diff --git a/axum/src/handler/into_extension_service.rs b/axum/src/handler/into_extension_service.rs index a0b91f69..19e68281 100644 --- a/axum/src/handler/into_extension_service.rs +++ b/axum/src/handler/into_extension_service.rs @@ -1,5 +1,5 @@ use super::Handler; -use crate::response::Response; +use crate::{extract::State, response::Response}; use http::Request; use std::{ convert::Infallible, @@ -58,7 +58,16 @@ where use futures_util::future::FutureExt; let handler = self.handler.clone(); - let state = req.extensions().get::().unwrap().clone(); + let State(state) = req + .extensions() + .get::>() + .unwrap_or_else(|| { + panic!( + "no state of type `{}` was found. Please file an issue", + std::any::type_name::>() + ) + }) + .clone(); let future = Handler::call(handler, state, req); let future = future.map(Ok as _); diff --git a/axum/src/lib.rs b/axum/src/lib.rs index c5f19505..aead8fde 100644 --- a/axum/src/lib.rs +++ b/axum/src/lib.rs @@ -382,8 +382,8 @@ rust_2018_idioms, future_incompatible, nonstandard_style, - missing_debug_implementations, - missing_docs + // missing_debug_implementations, + // missing_docs )] #![deny(unreachable_pub, private_in_public)] #![allow(elided_lifetimes_in_paths, clippy::type_complexity)] diff --git a/axum/src/routing/method_routing.rs b/axum/src/routing/method_routing.rs index d63ede9a..0a84e39b 100644 --- a/axum/src/routing/method_routing.rs +++ b/axum/src/routing/method_routing.rs @@ -648,12 +648,30 @@ where where H: Handler, T: 'static, - S: Clone + Send + Sync + 'static, { self.fallback_boxed_response_body(IntoExtensionService::new(handler)) } } +impl MethodRouter { + pub(crate) fn change_state_marker(self) -> MethodRouter { + MethodRouter { + state: self.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 where B: Send + 'static, diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index e88daad8..e3851c2b 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -3,7 +3,7 @@ use self::{future::RouteFuture, not_found::NotFound}; use crate::{ body::{boxed, Body, Bytes, HttpBody}, - extract::connect_info::IntoMakeServiceWithConnectInfo, + extract::{connect_info::IntoMakeServiceWithConnectInfo, State}, handler::{Handler, IntoExtensionService}, response::Response, routing::strip_prefix::StripPrefix, @@ -168,6 +168,14 @@ where _marker: PhantomData, } } + + pub fn map_state(self, f: F) -> Router + where + // TODO(david): which Fn? + F: FnOnce(OuterState) -> S, + { + todo!() + } } impl Router @@ -194,7 +202,7 @@ where impl Router where B: HttpBody + Send + 'static, - S: 'static, + S: Clone + 'static, R: 'static, { #[doc = include_str!("../docs/routing/route.md")] @@ -205,7 +213,41 @@ where // routers containing handlers method_router: MethodRouter, ) -> Self { - self + validate_path_for_route(path); + + let id = RouteId::next(); + + match self + .node + .path_to_route_id + .get(path) + .and_then(|route_id| self.routes.get(route_id).map(|svc| (*route_id, svc))) + { + Some((route_id, Endpoint::MethodRouter(prev_method_router))) => { + // 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); + + self + } + Some((_, Endpoint::Route(_))) => { + // if the endpoint isn't a `MethodRouter` then we have no way of merging things so + // just panic + panic!("A route for `{}` with a different HTTP method already exists and the routes could not be merge", path) + } + None => { + // the state will be provided later in `::call`, so its safe to + // ignore that it hasn't been provided yet + let service = Endpoint::MethodRouter(method_router.change_state_marker()); + + self.insert_endpoint(path, id, service); + + self + } + } } /// TODO(david): docs @@ -214,57 +256,32 @@ where T: Service, Response = Response, Error = Infallible> + Clone + Send + 'static, T::Future: Send + 'static, { + 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, + }; + + validate_path_for_route(path); + + let id = RouteId::next(); + let service = Endpoint::Route(Route::new(service)); + + self.insert_endpoint(path, 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 `/`"); - // } + fn insert_endpoint(&mut self, path: &str, id: RouteId, endpoint: Endpoint) { + 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); - // // 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 + self.routes.insert(id, endpoint); } #[doc = include_str!("../docs/routing/nest.md")] @@ -599,7 +616,7 @@ where // 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())); + .insert(State(self.state.as_ref().unwrap().clone())); debug_assert!(prev.is_none()); match self.node.at(&path) { @@ -622,6 +639,14 @@ pub enum MissingState {} #[derive(Copy, Clone, Debug)] pub enum WithState {} +fn validate_path_for_route(path: &str) { + 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 `/`"); + } +} + fn validate_path_for_nest(path: &mut &str) { if path.is_empty() { // nesting at `""` and `"/"` should mean the same thing diff --git a/axum/src/routing/tests/merge.rs b/axum/src/routing/tests/merge.rs index c5eb4dab..b7c7b142 100644 --- a/axum/src/routing/tests/merge.rs +++ b/axum/src/routing/tests/merge.rs @@ -414,3 +414,8 @@ async fn middleware_that_return_early() { ); assert_eq!(client.get("/public").send().await.status(), StatusCode::OK); } + +#[tokio::test] +async fn merging_with_different_state() { + todo!() +} diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index 1648daf1..37c6dcd7 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -1,7 +1,7 @@ use crate::{ body::{Bytes, Empty}, error_handling::HandleErrorLayer, - extract::{self, Path}, + extract::{self, Path, State}, handler::Handler, response::IntoResponse, routing::{delete, get, get_service, on, on_service, patch, patch_service, post, MethodFilter}, @@ -34,31 +34,19 @@ async fn hello_world() { "Hello, World!" } - async fn foo(_: Request) -> &'static str { - "foo" - } + let app = Router::without_state() + .route("/", get(root)) + .route_service("/foo", get(root).state(())); - async fn users_create(_: Request) -> &'static str { - "users#create" - } - - let app = Router::new() - .route("/", get(root).post(foo)) - .route("/users", post(users_create)); - - let client = TestClient::new(app.state(())); + let client = TestClient::new(app); let res = client.get("/").send().await; let body = res.text().await; assert_eq!(body, "Hello, World!"); - let res = client.post("/").send().await; + let res = client.get("/foo").send().await; let body = res.text().await; - assert_eq!(body, "foo"); - - let res = client.post("/users").send().await; - let body = res.text().await; - assert_eq!(body, "users#create"); + assert_eq!(body, "Hello, World!"); } #[tokio::test] @@ -455,6 +443,17 @@ async fn routing_to_router_panics() { ); } +#[tokio::test] +#[should_panic( + expected = "A route for `/` with a different HTTP method already exists and the routes could not be merge" +)] +async fn conflicting_method_router_and_opaque() { + let app = Router::without_state() + .route_service("/", get(|| async {}).state(())) + .route("/", post(|| async {})); + TestClient::new(app); +} + #[tokio::test] async fn route_layer() { let app = Router::new() @@ -502,7 +501,7 @@ async fn different_methods_added_in_different_routes() { #[tokio::test] async fn different_methods_added_in_different_routes_deeply_nested() { - let app = Router::new() + let app = Router::with_state(()) .route("/foo/bar/baz", get(|| async { "GET" })) .nest( "/foo", @@ -512,7 +511,7 @@ async fn different_methods_added_in_different_routes_deeply_nested() { ), ); - let client = TestClient::new(app.state(())); + let client = TestClient::new(app); let res = client.get("/foo/bar/baz").send().await; let body = res.text().await; @@ -677,3 +676,22 @@ async fn limited_body_with_streaming_body() { .await; assert_eq!(res.status(), StatusCode::PAYLOAD_TOO_LARGE); } + +#[tokio::test] +async fn extracting_state() { + #[derive(Clone)] + struct AppState { + value: &'static str, + } + + async fn handler(State(app_state): State) -> &'static str { + app_state.value + } + + let app = Router::with_state(AppState { value: "foo" }).route("/", get(handler)); + + let client = TestClient::new(app); + + let res = client.get("/").send().await; + assert_eq!(res.text().await, "foo"); +} diff --git a/axum/src/routing/tests/nest.rs b/axum/src/routing/tests/nest.rs index bb191d7e..efb751ad 100644 --- a/axum/src/routing/tests/nest.rs +++ b/axum/src/routing/tests/nest.rs @@ -386,6 +386,33 @@ async fn nest_with_and_without_trailing() { assert_eq!(res.status(), StatusCode::OK); } +#[tokio::test] +async fn nesting_with_different_state() { + #[derive(Clone)] + struct State { + inner: InnerState, + } + + #[derive(Clone)] + struct InnerState {} + + impl From for InnerState { + fn from(state: State) -> Self { + state.inner + } + } + + let inner_router = Router::::new(); + + let router_router = Router::::new() + .state(State { + inner: InnerState {}, + }) + .nest("/", inner_router.map_state(Into::into)); + + todo!(); +} + macro_rules! nested_route_test { ( $name:ident,