From e43bdf0ecf3ad8a8de7694658f73143e1ca066fc Mon Sep 17 00:00:00 2001
From: David Pedersen <david.pdrsn@gmail.com>
Date: Mon, 25 Oct 2021 23:28:22 +0200
Subject: [PATCH] Rename `EmptyRouter` to `MethodNotAllowed` (#411)

It is no longer needed in `Router` so can be changed to always return
`403 Method Not Allowed`.
---
 CHANGELOG.md                         |  2 +
 src/handler/mod.rs                   |  4 +-
 src/routing/future.rs                |  4 +-
 src/routing/handler_method_router.rs | 26 ++++++------
 src/routing/mod.rs                   | 61 +++++++++++-----------------
 src/routing/service_method_router.rs | 26 ++++++------
 6 files changed, 56 insertions(+), 67 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9d393ded..67fd703a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -134,6 +134,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
       with a trailing slash is received.
     - This can be overridden by explicitly defining two routes: One with and one
       without trailing a slash.
+- **breaking:** `EmptyRouter` has been renamed to `MethodNotAllowed` as its only
+  used in method routers and not in path routers (`Router`)
 
 [#339]: https://github.com/tokio-rs/axum/pull/339
 [#286]: https://github.com/tokio-rs/axum/pull/286
diff --git a/src/handler/mod.rs b/src/handler/mod.rs
index 1116bc58..6d82d47b 100644
--- a/src/handler/mod.rs
+++ b/src/handler/mod.rs
@@ -4,7 +4,7 @@ use crate::{
     body::{box_body, BoxBody},
     extract::{FromRequest, RequestParts},
     response::IntoResponse,
-    routing::{EmptyRouter, MethodRouter},
+    routing::{MethodNotAllowed, MethodRouter},
     BoxError,
 };
 use async_trait::async_trait;
@@ -78,7 +78,7 @@ pub trait Handler<B, T>: Clone + Send + Sized + 'static {
     /// ```
     fn layer<L>(self, layer: L) -> Layered<L::Service, T>
     where
-        L: Layer<MethodRouter<Self, B, T, EmptyRouter>>,
+        L: Layer<MethodRouter<Self, B, T, MethodNotAllowed>>,
     {
         Layered::new(layer.layer(crate::routing::any(self)))
     }
diff --git a/src/routing/future.rs b/src/routing/future.rs
index 07ec279c..c17c5798 100644
--- a/src/routing/future.rs
+++ b/src/routing/future.rs
@@ -42,8 +42,8 @@ opaque_future! {
 }
 
 opaque_future! {
-    /// Response future for [`EmptyRouter`](super::EmptyRouter).
-    pub type EmptyRouterFuture<E> =
+    /// Response future for [`MethodNotAllowed`](super::MethodNotAllowed).
+    pub type MethodNotAllowedFuture<E> =
         std::future::Ready<Result<Response<BoxBody>, E>>;
 }
 
diff --git a/src/routing/handler_method_router.rs b/src/routing/handler_method_router.rs
index 6ae950e1..59fc8898 100644
--- a/src/routing/handler_method_router.rs
+++ b/src/routing/handler_method_router.rs
@@ -3,7 +3,7 @@
 use crate::{
     body::{box_body, BoxBody},
     handler::Handler,
-    routing::{EmptyRouter, MethodFilter},
+    routing::{MethodFilter, MethodNotAllowed},
     util::{Either, EitherProj},
 };
 use futures_util::{future::BoxFuture, ready};
@@ -57,7 +57,7 @@ use tower_service::Service;
 /// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap();
 /// # };
 /// ```
-pub fn any<H, B, T>(handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn any<H, B, T>(handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
@@ -67,7 +67,7 @@ where
 /// Route `CONNECT` requests to the given handler.
 ///
 /// See [`get`] for an example.
-pub fn connect<H, B, T>(handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn connect<H, B, T>(handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
@@ -77,7 +77,7 @@ where
 /// Route `DELETE` requests to the given handler.
 ///
 /// See [`get`] for an example.
-pub fn delete<H, B, T>(handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn delete<H, B, T>(handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
@@ -106,7 +106,7 @@ where
 /// Note that `get` routes will also be called for `HEAD` requests but will have
 /// the response body removed. Make sure to add explicit `HEAD` routes
 /// afterwards.
-pub fn get<H, B, T>(handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn get<H, B, T>(handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
@@ -116,7 +116,7 @@ where
 /// Route `HEAD` requests to the given handler.
 ///
 /// See [`get`] for an example.
-pub fn head<H, B, T>(handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn head<H, B, T>(handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
@@ -126,7 +126,7 @@ where
 /// Route `OPTIONS` requests to the given handler.
 ///
 /// See [`get`] for an example.
-pub fn options<H, B, T>(handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn options<H, B, T>(handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
@@ -136,7 +136,7 @@ where
 /// Route `PATCH` requests to the given handler.
 ///
 /// See [`get`] for an example.
-pub fn patch<H, B, T>(handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn patch<H, B, T>(handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
@@ -146,7 +146,7 @@ where
 /// Route `POST` requests to the given handler.
 ///
 /// See [`get`] for an example.
-pub fn post<H, B, T>(handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn post<H, B, T>(handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
@@ -156,7 +156,7 @@ where
 /// Route `PUT` requests to the given handler.
 ///
 /// See [`get`] for an example.
-pub fn put<H, B, T>(handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn put<H, B, T>(handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
@@ -166,7 +166,7 @@ where
 /// Route `TRACE` requests to the given handler.
 ///
 /// See [`get`] for an example.
-pub fn trace<H, B, T>(handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn trace<H, B, T>(handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
@@ -192,14 +192,14 @@ where
 /// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap();
 /// # };
 /// ```
-pub fn on<H, B, T>(method: MethodFilter, handler: H) -> MethodRouter<H, B, T, EmptyRouter>
+pub fn on<H, B, T>(method: MethodFilter, handler: H) -> MethodRouter<H, B, T, MethodNotAllowed>
 where
     H: Handler<B, T>,
 {
     MethodRouter {
         method,
         handler,
-        fallback: EmptyRouter::method_not_allowed(),
+        fallback: MethodNotAllowed::new(),
         _marker: PhantomData,
     }
 }
diff --git a/src/routing/mod.rs b/src/routing/mod.rs
index 22eeba21..3a6403ad 100644
--- a/src/routing/mod.rs
+++ b/src/routing/mod.rs
@@ -1,6 +1,6 @@
 //! Routing between [`Service`]s and handlers.
 
-use self::future::{EmptyRouterFuture, NestedFuture, RouteFuture, RouterFuture};
+use self::future::{MethodNotAllowedFuture, NestedFuture, RouteFuture, RouterFuture};
 use crate::{
     body::{box_body, Body, BoxBody},
     clone_box_service::CloneBoxService,
@@ -806,7 +806,10 @@ where
                 } else if let Some(fallback) = &self.fallback {
                     RouterFuture::from_oneshot(fallback.clone().oneshot(req))
                 } else {
-                    let res = EmptyRouter::<Infallible>::not_found().call_sync(req);
+                    let res = Response::builder()
+                        .status(StatusCode::NOT_FOUND)
+                        .body(crate::body::empty())
+                        .unwrap();
                     RouterFuture::from_response(res)
                 }
             }
@@ -871,72 +874,56 @@ pub(crate) struct InvalidUtf8InPathParam {
     pub(crate) key: ByteStr,
 }
 
-/// A [`Service`] that responds with `404 Not Found` or `405 Method not allowed`
-/// to all requests.
+/// A [`Service`] that responds with `405 Method not allowed` to all requests.
 ///
-/// This is used as the bottom service in a router stack. You shouldn't have to
+/// This is used as the bottom service in a method router. You shouldn't have to
 /// use it manually.
-pub struct EmptyRouter<E = Infallible> {
-    status: StatusCode,
+pub struct MethodNotAllowed<E = Infallible> {
     _marker: PhantomData<fn() -> E>,
 }
 
-impl<E> EmptyRouter<E> {
-    pub(crate) fn not_found() -> Self {
+impl<E> MethodNotAllowed<E> {
+    pub(crate) fn new() -> Self {
         Self {
-            status: StatusCode::NOT_FOUND,
             _marker: PhantomData,
         }
     }
-
-    pub(crate) fn method_not_allowed() -> Self {
-        Self {
-            status: StatusCode::METHOD_NOT_ALLOWED,
-            _marker: PhantomData,
-        }
-    }
-
-    fn call_sync<B>(&mut self, _req: Request<B>) -> Response<BoxBody>
-    where
-        B: Send + Sync + 'static,
-    {
-        let mut res = Response::new(crate::body::empty());
-        *res.status_mut() = self.status;
-        res
-    }
 }
 
-impl<E> Clone for EmptyRouter<E> {
+impl<E> Clone for MethodNotAllowed<E> {
     fn clone(&self) -> Self {
         Self {
-            status: self.status,
             _marker: PhantomData,
         }
     }
 }
 
-impl<E> fmt::Debug for EmptyRouter<E> {
+impl<E> fmt::Debug for MethodNotAllowed<E> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_tuple("EmptyRouter").finish()
+        f.debug_tuple("MethodNotAllowed").finish()
     }
 }
 
-impl<B, E> Service<Request<B>> for EmptyRouter<E>
+impl<B, E> Service<Request<B>> for MethodNotAllowed<E>
 where
     B: Send + Sync + 'static,
 {
     type Response = Response<BoxBody>;
     type Error = E;
-    type Future = EmptyRouterFuture<E>;
+    type Future = MethodNotAllowedFuture<E>;
 
+    #[inline]
     fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
         Poll::Ready(Ok(()))
     }
 
-    fn call(&mut self, request: Request<B>) -> Self::Future {
-        let res = self.call_sync(request);
+    fn call(&mut self, _req: Request<B>) -> Self::Future {
+        let res = Response::builder()
+            .status(StatusCode::METHOD_NOT_ALLOWED)
+            .body(crate::body::empty())
+            .unwrap();
 
-        EmptyRouterFuture {
+        MethodNotAllowedFuture {
             future: ready(Ok(res)),
         }
     }
@@ -1120,8 +1107,8 @@ mod tests {
 
         assert_send::<Route<()>>();
 
-        assert_send::<EmptyRouter<NotSendSync>>();
-        assert_sync::<EmptyRouter<NotSendSync>>();
+        assert_send::<MethodNotAllowed<NotSendSync>>();
+        assert_sync::<MethodNotAllowed<NotSendSync>>();
 
         assert_send::<Nested<()>>();
         assert_sync::<Nested<()>>();
diff --git a/src/routing/service_method_router.rs b/src/routing/service_method_router.rs
index 49c4d491..36c0f5c7 100644
--- a/src/routing/service_method_router.rs
+++ b/src/routing/service_method_router.rs
@@ -98,7 +98,7 @@
 
 use crate::{
     body::{box_body, BoxBody},
-    routing::{EmptyRouter, MethodFilter},
+    routing::{MethodFilter, MethodNotAllowed},
     util::{Either, EitherProj},
     BoxError,
 };
@@ -123,7 +123,7 @@ use tower_service::Service;
 ///
 /// Note that this only accepts the standard HTTP methods. If you need to
 /// support non-standard methods you can route directly to a [`Service`].
-pub fn any<S, B>(svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn any<S, B>(svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
@@ -133,7 +133,7 @@ where
 /// Route `CONNECT` requests to the given service.
 ///
 /// See [`get`] for an example.
-pub fn connect<S, B>(svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn connect<S, B>(svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
@@ -143,7 +143,7 @@ where
 /// Route `DELETE` requests to the given service.
 ///
 /// See [`get`] for an example.
-pub fn delete<S, B>(svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn delete<S, B>(svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
@@ -178,7 +178,7 @@ where
 /// Note that `get` routes will also be called for `HEAD` requests but will have
 /// the response body removed. Make sure to add explicit `HEAD` routes
 /// afterwards.
-pub fn get<S, B>(svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn get<S, B>(svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
@@ -188,7 +188,7 @@ where
 /// Route `HEAD` requests to the given service.
 ///
 /// See [`get`] for an example.
-pub fn head<S, B>(svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn head<S, B>(svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
@@ -198,7 +198,7 @@ where
 /// Route `OPTIONS` requests to the given service.
 ///
 /// See [`get`] for an example.
-pub fn options<S, B>(svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn options<S, B>(svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
@@ -208,7 +208,7 @@ where
 /// Route `PATCH` requests to the given service.
 ///
 /// See [`get`] for an example.
-pub fn patch<S, B>(svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn patch<S, B>(svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
@@ -218,7 +218,7 @@ where
 /// Route `POST` requests to the given service.
 ///
 /// See [`get`] for an example.
-pub fn post<S, B>(svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn post<S, B>(svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
@@ -228,7 +228,7 @@ where
 /// Route `PUT` requests to the given service.
 ///
 /// See [`get`] for an example.
-pub fn put<S, B>(svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn put<S, B>(svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
@@ -238,7 +238,7 @@ where
 /// Route `TRACE` requests to the given service.
 ///
 /// See [`get`] for an example.
-pub fn trace<S, B>(svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn trace<S, B>(svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
@@ -270,14 +270,14 @@ where
 /// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap();
 /// # };
 /// ```
-pub fn on<S, B>(method: MethodFilter, svc: S) -> MethodRouter<S, EmptyRouter<S::Error>, B>
+pub fn on<S, B>(method: MethodFilter, svc: S) -> MethodRouter<S, MethodNotAllowed<S::Error>, B>
 where
     S: Service<Request<B>> + Clone,
 {
     MethodRouter {
         method,
         svc,
-        fallback: EmptyRouter::method_not_allowed(),
+        fallback: MethodNotAllowed::new(),
         _request_body: PhantomData,
     }
 }