Skip to content

Commit

Permalink
Don't inherit fallbacks when using Router::nest_service
Browse files Browse the repository at this point in the history
I think it makes sense to only inherit fallbacks for routers nested with
`Router::nest` and not `Router::nest_service`. It also simplifies the
code and avoids some unfortunate extension work arounds.
  • Loading branch information
davidpdrsn committed Apr 22, 2023
1 parent db300ef commit 6ba8d3c
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 81 deletions.
3 changes: 3 additions & 0 deletions axum/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
type inference issues when calling `ServiceExt` methods on a `Router` ([#1835])
- **breaking:** Removed `axum::Server` as it was removed in hyper 1.0. Instead
use `axum::serve(listener, service)` or hyper/hyper-util for more configuration options ([#1868])
- **breaking:** Only inherit fallbacks for routers nested with `Router::nest`.
Routers nested with `Router::nest_service` will no longer inherit fallbacks ([#1956])

[#1664]: https://github.com/tokio-rs/axum/pull/1664
[#1751]: https://github.com/tokio-rs/axum/pull/1751
Expand All @@ -58,6 +60,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#1835]: https://github.com/tokio-rs/axum/pull/1835
[#1850]: https://github.com/tokio-rs/axum/pull/1850
[#1868]: https://github.com/tokio-rs/axum/pull/1868
[#1956]: https://github.com/tokio-rs/axum/pull/1956

# 0.6.16 (18. April, 2023)

Expand Down
43 changes: 6 additions & 37 deletions axum/src/routing/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use std::{
marker::PhantomData,
task::{Context, Poll},
};
use sync_wrapper::SyncWrapper;
use tower_layer::Layer;
use tower_service::Service;

Expand Down Expand Up @@ -285,43 +284,15 @@ where
}
}

pub(crate) fn call_with_state(
&mut self,
mut req: Request,
state: S,
) -> RouteFuture<Infallible> {
// required for opaque routers to still inherit the fallback
// TODO(david): remove this feature in 0.7
if !self.default_fallback {
req.extensions_mut().insert(SuperFallback(SyncWrapper::new(
self.fallback_router.clone(),
)));
}

pub(crate) fn call_with_state(&mut self, req: Request, state: S) -> RouteFuture<Infallible> {
match self.path_router.call_with_state(req, state) {
Ok(future) => future,
Err((mut req, state)) => {
let super_fallback = req
.extensions_mut()
.remove::<SuperFallback<S>>()
.map(|SuperFallback(path_router)| path_router.into_inner());

if let Some(mut super_fallback) = super_fallback {
return super_fallback
.call_with_state(req, state)
.unwrap_or_else(|_| unreachable!());
}

match self.fallback_router.call_with_state(req, state) {
Ok(future) => future,
Err((_req, _state)) => {
unreachable!(
"the default fallback added in `Router::new` \
matches everything"
)
}
Err((req, state)) => match self.fallback_router.call_with_state(req, state) {
Ok(future) => future,
Err((_req, _state)) => {
unreachable!("the default fallback added in `Router::new` matches everything")
}
}
},
}
}

Expand Down Expand Up @@ -662,8 +633,6 @@ impl<S> fmt::Debug for Endpoint<S> {
}
}

struct SuperFallback<S>(SyncWrapper<PathRouter<S, true>>);

#[test]
#[allow(warnings)]
fn traits() {
Expand Down
44 changes: 0 additions & 44 deletions axum/src/routing/tests/fallback.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use tower::ServiceExt;

use super::*;
use crate::middleware::{map_request, map_response};

Expand Down Expand Up @@ -166,48 +164,6 @@ async fn also_inherits_default_layered_fallback() {
assert_eq!(res.text().await, "outer");
}

#[crate::test]
async fn fallback_inherited_into_nested_router_service() {
let inner = Router::new()
.route(
"/bar",
get(|State(state): State<&'static str>| async move { state }),
)
.with_state("inner");

// with a different state
let app = Router::new()
.nest_service("/foo", inner)
.fallback(outer_fallback);

let client = TestClient::new(app);
let res = client.get("/foo/not-found").send().await;
assert_eq!(res.status(), StatusCode::NOT_FOUND);
assert_eq!(res.text().await, "outer");
}

#[crate::test]
async fn fallback_inherited_into_nested_opaque_service() {
let inner = Router::new()
.route(
"/bar",
get(|State(state): State<&'static str>| async move { state }),
)
.with_state("inner")
// even if the service is made more opaque it should still inherit the fallback
.boxed_clone();

// with a different state
let app = Router::new()
.nest_service("/foo", inner)
.fallback(outer_fallback);

let client = TestClient::new(app);
let res = client.get("/foo/not-found").send().await;
assert_eq!(res.status(), StatusCode::NOT_FOUND);
assert_eq!(res.text().await, "outer");
}

#[crate::test]
async fn nest_fallback_on_inner() {
let app = Router::new()
Expand Down

0 comments on commit 6ba8d3c

Please sign in to comment.