From ab5d2ae5f710e74ee92af218f09ce116e4880ea5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Ml=C3=A1dek?= Date: Sun, 5 May 2024 20:30:03 +0200 Subject: [PATCH] wip poc --- axum/src/routing/mod.rs | 21 ++++++++- axum/src/routing/path_router.rs | 65 +++++++++++++++---------- axum/src/routing/tests/mod.rs | 30 ++++++------ axum/src/routing/tests/nest.rs | 84 +++++++++++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 41 deletions(-) diff --git a/axum/src/routing/mod.rs b/axum/src/routing/mod.rs index 6564df7d62..8a729d4d3b 100644 --- a/axum/src/routing/mod.rs +++ b/axum/src/routing/mod.rs @@ -57,6 +57,16 @@ macro_rules! panic_on_err { #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub(crate) struct RouteId(u32); +// TODO Docs - in short sets where a root route (`/`) of a nested router will be available. +// For example if a router is nested at `/foo`, the root route can be available at `/foo`, at `/foo/`, or at both locations. +// We could also extend this with variants like `PermanentRedirectToEmpty` and `PermanentRedirectToSlash`. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum NestedRootRouteBehavior { + OnlyEmpty, + OnlySlash, + EmptyAndSlash, +} + /// The router type for composing handlers and services. #[must_use] pub struct Router { @@ -187,6 +197,11 @@ where #[doc = include_str!("../docs/routing/nest.md")] #[track_caller] pub fn nest(self, path: &str, router: Router) -> Self { + self.nest_with_root(path, router, NestedRootRouteBehavior::EmptyAndSlash) + } + + + pub fn nest_with_root(self, path: &str, router: Router, root_behavior: NestedRootRouteBehavior) -> Self { let RouterInner { path_router, fallback_router, @@ -198,10 +213,12 @@ where } = router.into_inner(); self.tap_inner_mut(|this| { - panic_on_err!(this.path_router.nest(path, path_router)); + panic_on_err!(this.path_router.nest(path, path_router, root_behavior)); if !default_fallback { - panic_on_err!(this.fallback_router.nest(path, fallback_router)); + panic_on_err!(this + .fallback_router + .nest(path, fallback_router, root_behavior)); } }) } diff --git a/axum/src/routing/path_router.rs b/axum/src/routing/path_router.rs index 345d66712f..56f00cfe07 100644 --- a/axum/src/routing/path_router.rs +++ b/axum/src/routing/path_router.rs @@ -7,7 +7,7 @@ use tower_service::Service; use super::{ future::RouteFuture, not_found::NotFound, strip_prefix::StripPrefix, url_params, Endpoint, - MethodRouter, Route, RouteId, FALLBACK_PARAM_PATH, NEST_TAIL_PARAM, + MethodRouter, NestedRootRouteBehavior, Route, RouteId, FALLBACK_PARAM_PATH, NEST_TAIL_PARAM, }; pub(super) struct PathRouter { @@ -161,6 +161,7 @@ where &mut self, path_to_nest_at: &str, router: PathRouter, + root_behavior: NestedRootRouteBehavior, ) -> Result<(), Cow<'static, str>> { let prefix = validate_nest_path(path_to_nest_at); @@ -176,18 +177,18 @@ where .get(&id) .expect("no path for route id. This is a bug in axum. Please file an issue"); - let path = path_for_nested_route(prefix, inner_path); - - let layer = ( - StripPrefix::layer(prefix), - SetNestedPath::layer(path_to_nest_at), - ); - match endpoint.layer(layer) { - Endpoint::MethodRouter(method_router) => { - self.route(&path, method_router)?; - } - Endpoint::Route(route) => { - self.route_endpoint(&path, Endpoint::Route(route))?; + for path in paths_for_nested_route(prefix, inner_path, root_behavior) { + let layer = ( + StripPrefix::layer(prefix), + SetNestedPath::layer(path_to_nest_at), + ); + match endpoint.clone().layer(layer) { + Endpoint::MethodRouter(method_router) => { + self.route(&path, method_router)?; + } + Endpoint::Route(route) => { + self.route_endpoint(&path, Endpoint::Route(route))?; + } } } } @@ -208,11 +209,7 @@ where let path = validate_nest_path(path_to_nest_at); let prefix = path; - let path = if path.ends_with('/') { - format!("{path}*{NEST_TAIL_PARAM}") - } else { - format!("{path}/*{NEST_TAIL_PARAM}") - }; + let path = format!("{path}/*{NEST_TAIL_PARAM}"); let layer = ( StripPrefix::layer(prefix), @@ -470,18 +467,38 @@ fn validate_nest_path(path: &str) -> &str { panic!("Invalid route: nested routes cannot contain wildcards (*)"); } + if path.ends_with('/') { + panic!("Invalid route: nested routes cannot end with `/`"); + } + path } -pub(crate) fn path_for_nested_route<'a>(prefix: &'a str, path: &'a str) -> Cow<'a, str> { +/// This returns all paths that should be registered for a given prefix and +/// path. This usually returns exactly one path, but may return two for root +/// path with [`RootRouteBehavior::EmptyAndSlash`]. +pub(crate) fn paths_for_nested_route<'a>( + prefix: &'a str, + path: &'a str, + root_behavior: NestedRootRouteBehavior, +) -> impl Iterator> { debug_assert!(prefix.starts_with('/')); + debug_assert!(!prefix.ends_with('/')); debug_assert!(path.starts_with('/')); - if prefix.ends_with('/') { - format!("{prefix}{}", path.trim_start_matches('/')).into() - } else if path == "/" { - prefix.into() + if path == "/" { + match root_behavior { + NestedRootRouteBehavior::OnlyEmpty => Some(prefix.into()).into_iter().chain(None), + NestedRootRouteBehavior::OnlySlash => { + Some(format!("{prefix}/").into()).into_iter().chain(None) + } + NestedRootRouteBehavior::EmptyAndSlash => Some(prefix.into()) + .into_iter() + .chain(Some(format!("{prefix}/").into())), + } } else { - format!("{prefix}{path}").into() + Some(format!("{prefix}{path}").into()) + .into_iter() + .chain(None) } } diff --git a/axum/src/routing/tests/mod.rs b/axum/src/routing/tests/mod.rs index 144c870dfa..ebf076aaca 100644 --- a/axum/src/routing/tests/mod.rs +++ b/axum/src/routing/tests/mod.rs @@ -6,7 +6,7 @@ use crate::{ response::{IntoResponse, Response}, routing::{ delete, get, get_service, on, on_service, patch, patch_service, - path_router::path_for_nested_route, post, MethodFilter, + path_router::paths_for_nested_route, post, MethodFilter, NestedRootRouteBehavior, }, test_helpers::{ tracing_helpers::{capture_tracing, TracingEvent}, @@ -888,20 +888,20 @@ fn method_router_fallback_with_state() { .with_state(state); } -#[test] -fn test_path_for_nested_route() { - assert_eq!(path_for_nested_route("/", "/"), "/"); - - assert_eq!(path_for_nested_route("/a", "/"), "/a"); - assert_eq!(path_for_nested_route("/", "/b"), "/b"); - assert_eq!(path_for_nested_route("/a/", "/"), "/a/"); - assert_eq!(path_for_nested_route("/", "/b/"), "/b/"); - - assert_eq!(path_for_nested_route("/a", "/b"), "/a/b"); - assert_eq!(path_for_nested_route("/a/", "/b"), "/a/b"); - assert_eq!(path_for_nested_route("/a", "/b/"), "/a/b/"); - assert_eq!(path_for_nested_route("/a/", "/b/"), "/a/b/"); -} +// #[test] +// fn test_path_for_nested_route() { +// assert_eq!(path_for_nested_route("/", "/"), "/"); + +// assert_eq!(path_for_nested_route("/a", "/"), "/a"); +// assert_eq!(path_for_nested_route("/", "/b"), "/b"); +// assert_eq!(path_for_nested_route("/a/", "/"), "/a/"); +// assert_eq!(path_for_nested_route("/", "/b/"), "/b/"); + +// assert_eq!(path_for_nested_route("/a", "/b"), "/a/b"); +// assert_eq!(path_for_nested_route("/a/", "/b"), "/a/b"); +// assert_eq!(path_for_nested_route("/a", "/b/"), "/a/b/"); +// assert_eq!(path_for_nested_route("/a/", "/b/"), "/a/b/"); +// } #[crate::test] async fn state_isnt_cloned_too_much() { diff --git a/axum/src/routing/tests/nest.rs b/axum/src/routing/tests/nest.rs index 40df1f1ad8..865bee5fd4 100644 --- a/axum/src/routing/tests/nest.rs +++ b/axum/src/routing/tests/nest.rs @@ -346,6 +346,90 @@ async fn nest_with_and_without_trailing() { assert_eq!(res.status(), StatusCode::OK); } +#[crate::test] +async fn nest_root_default_behavior() { + // Default behavior -- is the same as before + let app = Router::new().nest("/foo", Router::new().route("/", get(|| async {}))); + + let client = TestClient::new(app); + + let res = client.get("/foo").await; + assert_eq!(res.status(), StatusCode::OK); + + let res = client.get("/foo/").await; + assert_eq!(res.status(), StatusCode::OK); + + let res = client.get("/").await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); +} + +#[tokio::test] +async fn nest_root_behavior() { + { + let app = Router::new().nest_with_root( + "/foo", + Router::new().route("/", get(|| async {})), + NestedRootRouteBehavior::OnlyEmpty, + ); + + let client = TestClient::new(app); + + let res = client.get("/foo").await; + assert_eq!(res.status(), StatusCode::OK); + + let res = client.get("/foo/").await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + + let res = client.get("/").await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + + let res = client.get("/foo//").await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + } + { + let app = Router::new().nest_with_root( + "/foo", + Router::new().route("/", get(|| async {})), + NestedRootRouteBehavior::OnlySlash, + ); + + let client = TestClient::new(app); + + let res = client.get("/foo").await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + + let res = client.get("/foo/").await; + assert_eq!(res.status(), StatusCode::OK); + + let res = client.get("/").await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + + let res = client.get("/foo//").await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + } + { + let app = Router::new().nest_with_root( + "/foo", + Router::new().route("/", get(|| async {})), + NestedRootRouteBehavior::EmptyAndSlash, + ); + + let client = TestClient::new(app); + + let res = client.get("/foo").await; + assert_eq!(res.status(), StatusCode::OK); + + let res = client.get("/foo/").await; + assert_eq!(res.status(), StatusCode::OK); + + let res = client.get("/").await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + + let res = client.get("/foo//").await; + assert_eq!(res.status(), StatusCode::NOT_FOUND); + } +} + #[tokio::test] async fn nesting_with_root_inner_router() { let app = Router::new()