From 1c703ac1d446b25aed021e72762ed262fa71cb54 Mon Sep 17 00:00:00 2001
From: Jens Reimann <jreimann@redhat.com>
Date: Wed, 2 Feb 2022 09:45:24 +0100
Subject: [PATCH] Allow using `Option<Middleware>` to enable/disable a
 middleware

Currently, there is `Condition`, which accepts a boolean
(to enable/disable) and an instance to the actual middleware.
The downside of that is, that such a middleware needs to be constructed
in any case. Even if the middleware is used or not.

However, the middleware is not used when it is disabled. Only the type
seems required. So this PR adds a `from_option` function, which allows
passing in an `Option` instead of boolean and instance. If the option
"is some" it is enabled. Otherwise, not.
---
 actix-web/CHANGES.md                  |  1 +
 actix-web/src/middleware/condition.rs | 67 ++++++++++++++++++++++++---
 2 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md
index 0144cb912..ee4987b87 100644
--- a/actix-web/CHANGES.md
+++ b/actix-web/CHANGES.md
@@ -18,6 +18,7 @@
 - Add `Route::wrap()` to allow individual routes to use middleware. [#2725]
 - Add `ServiceConfig::default_service()`. [#2338] [#2743]
 - Implement `ResponseError` for `std::convert::Infallible`
+- Add `Condition::from_option()` to allow creating a conditional middleware from an `Option`. [#2623]
 
 ### Changed
 - Minimum supported Rust version (MSRV) is now 1.56 due to transitive `hashbrown` dependency.
diff --git a/actix-web/src/middleware/condition.rs b/actix-web/src/middleware/condition.rs
index 65f25a67c..e46ce08ff 100644
--- a/actix-web/src/middleware/condition.rs
+++ b/actix-web/src/middleware/condition.rs
@@ -26,18 +26,31 @@ use crate::{
 /// let app = App::new()
 ///     .wrap(Condition::new(enable_normalize, NormalizePath::default()));
 /// ```
+/// Or you can use an `Option` to create a new instance:
+/// ```
+/// use actix_web::middleware::{Condition, NormalizePath};
+/// use actix_web::App;
+///
+/// let app = App::new()
+///     .wrap(Condition::from_option(Some(NormalizePath::default())));
+/// ```
 pub struct Condition<T> {
-    transformer: T,
-    enable: bool,
+    transformer: Option<T>,
 }
 
 impl<T> Condition<T> {
     pub fn new(enable: bool, transformer: T) -> Self {
         Self {
-            transformer,
-            enable,
+            transformer: match enable {
+                true => Some(transformer),
+                false => None,
+            },
         }
     }
+
+    pub fn from_option(transformer: Option<T>) -> Self {
+        Self { transformer }
+    }
 }
 
 impl<S, T, Req, BE, BD, Err> Transform<S, Req> for Condition<T>
@@ -55,8 +68,8 @@ where
     type Future = LocalBoxFuture<'static, Result<Self::Transform, Self::InitError>>;
 
     fn new_transform(&self, service: S) -> Self::Future {
-        if self.enable {
-            let fut = self.transformer.new_transform(service);
+        if let Some(transformer) = &self.transformer {
+            let fut = transformer.new_transform(service);
             async move {
                 let wrapped_svc = fut.await?;
                 Ok(ConditionMiddleware::Enable(wrapped_svc))
@@ -131,6 +144,7 @@ where
 #[cfg(test)]
 mod tests {
     use actix_service::IntoService as _;
+    use futures_util::future::ok;
 
     use super::*;
     use crate::{
@@ -167,6 +181,18 @@ mod tests {
         let _ = Condition::new(true, middleware::ErrorHandlers::<Bytes>::new());
     }
 
+    fn create_optional_mw<B>(enabled: bool) -> Option<ErrorHandlers<B>>
+    where
+        B: 'static,
+    {
+        match enabled {
+            true => Some(
+                ErrorHandlers::new().handler(StatusCode::INTERNAL_SERVER_ERROR, render_500),
+            ),
+            false => None,
+        }
+    }
+
     #[actix_rt::test]
     async fn test_handler_enabled() {
         let srv = |req: ServiceRequest| async move {
@@ -204,4 +230,33 @@ mod tests {
             test::call_service(&mw, TestRequest::default().to_srv_request()).await;
         assert_eq!(resp.headers().get(CONTENT_TYPE), None);
     }
+
+    #[actix_rt::test]
+    async fn test_handler_some() {
+        let srv = |req: ServiceRequest| {
+            ok(req.into_response(HttpResponse::InternalServerError().finish()))
+        };
+
+        let mw = Condition::from_option(create_optional_mw(true))
+            .new_transform(srv.into_service())
+            .await
+            .unwrap();
+        let resp = test::call_service(&mw, TestRequest::default().to_srv_request()).await;
+        assert_eq!(resp.headers().get(CONTENT_TYPE).unwrap(), "0001");
+    }
+
+    #[actix_rt::test]
+    async fn test_handler_none() {
+        let srv = |req: ServiceRequest| {
+            ok(req.into_response(HttpResponse::InternalServerError().finish()))
+        };
+
+        let mw = Condition::from_option(create_optional_mw(false))
+            .new_transform(srv.into_service())
+            .await
+            .unwrap();
+
+        let resp = test::call_service(&mw, TestRequest::default().to_srv_request()).await;
+        assert_eq!(resp.headers().get(CONTENT_TYPE), None);
+    }
 }