allow any body type in Resource (#2526)

CHANGES.md

@@ -1,6 +1,10 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
+### Changed
+- No longer require `Resource` service body type to be boxed. [#2526]
+
+[#2526]: https://github.com/actix/actix-web/pull/2526
 
 
 ## 4.0.0-beta.15 - 2021-12-17

actix-http/Cargo.toml

@@ -55,7 +55,6 @@ bytestring = "1"
 derive_more = "0.99.5"
 encoding_rs = "0.8"
 futures-core = { version = "0.3.7", default-features = false, features = ["alloc"] }
-futures-task = { version = "0.3.7", default-features = false, features = ["alloc"] }
 h2 = "0.3.9"
 http = "0.2.5"
 httparse = "1.5.1"

src/middleware/compat.rs

@@ -38,6 +38,15 @@ pub struct Compat<T> {
     transform: T,
 }
 
+#[cfg(test)]
+impl Compat<super::Noop> {
+    pub(crate) fn noop() -> Self {
+        Self {
+            transform: super::Noop,
+        }
+    }
+}
+
 impl<T> Compat<T> {
     /// Wrap a middleware to give it broader compatibility.
     pub fn new(middleware: T) -> Self {

src/middleware/mod.rs

@@ -5,6 +5,8 @@ mod condition;
 mod default_headers;
 mod err_handlers;
 mod logger;
+#[cfg(test)]
+mod noop;
 mod normalize;
 
 pub use self::compat::Compat;
@@ -12,6 +14,8 @@ pub use self::condition::Condition;
 pub use self::default_headers::DefaultHeaders;
 pub use self::err_handlers::{ErrorHandlerResponse, ErrorHandlers};
 pub use self::logger::Logger;
+#[cfg(test)]
+pub(crate) use self::noop::Noop;
 pub use self::normalize::{NormalizePath, TrailingSlash};
 
 #[cfg(feature = "__compress")]

src/middleware/noop.rs

@@ -0,0 +1,37 @@
+//! A no-op middleware. See [Noop] for docs.
+
+use actix_utils::future::{ready, Ready};
+
+use crate::dev::{Service, Transform};
+
+/// A no-op middleware that passes through request and response untouched.
+pub(crate) struct Noop;
+
+impl<S: Service<Req>, Req> Transform<S, Req> for Noop {
+    type Response = S::Response;
+    type Error = S::Error;
+    type Transform = NoopService<S>;
+    type InitError = ();
+    type Future = Ready<Result<Self::Transform, Self::InitError>>;
+
+    fn new_transform(&self, service: S) -> Self::Future {
+        ready(Ok(NoopService { service }))
+    }
+}
+
+#[doc(hidden)]
+pub(crate) struct NoopService<S> {
+    service: S,
+}
+
+impl<S: Service<Req>, Req> Service<Req> for NoopService<S> {
+    type Response = S::Response;
+    type Error = S::Error;
+    type Future = S::Future;
+
+    crate::dev::forward_ready!(service);
+
+    fn call(&self, req: Req) -> Self::Future {
+        self.service.call(req)
+    }
+}

src/resource.rs

@@ -1,6 +1,6 @@
-use std::{cell::RefCell, fmt, future::Future, rc::Rc};
+use std::{cell::RefCell, fmt, future::Future, marker::PhantomData, rc::Rc};
 
-use actix_http::Extensions;
+use actix_http::{body::BoxBody, Extensions};
 use actix_router::{IntoPatterns, Patterns};
 use actix_service::{
     apply, apply_fn_factory, boxed, fn_service, IntoServiceFactory, Service, ServiceFactory,
@@ -45,7 +45,7 @@ use crate::{
 ///
 /// If no matching route could be found, *405* response code get returned.
 /// Default behavior could be overridden with `default_resource()` method.
-pub struct Resource<T = ResourceEndpoint> {
+pub struct Resource<T = ResourceEndpoint, B = BoxBody> {
     endpoint: T,
     rdef: Patterns,
     name: Option<String>,
@@ -54,6 +54,7 @@ pub struct Resource<T = ResourceEndpoint> {
     guards: Vec<Box<dyn Guard>>,
     default: BoxedHttpServiceFactory,
     factory_ref: Rc<RefCell<Option<ResourceFactory>>>,
+    _phantom: PhantomData<B>,
 }
 
 impl Resource {
@@ -71,19 +72,21 @@ impl Resource {
             default: boxed::factory(fn_service(|req: ServiceRequest| async {
                 Ok(req.into_response(HttpResponse::MethodNotAllowed()))
             })),
+            _phantom: PhantomData,
         }
     }
 }
 
-impl<T> Resource<T>
+impl<T, B> Resource<T, B>
 where
     T: ServiceFactory<
         ServiceRequest,
         Config = (),
-        Response = ServiceResponse,
+        Response = ServiceResponse<B>,
         Error = Error,
         InitError = (),
     >,
+    B: MessageBody,
 {
     /// Set resource name.
     ///
@@ -252,26 +255,28 @@ where
     /// type (i.e modify response's body).
     ///
     /// **Note**: middlewares get called in opposite order of middlewares registration.
-    pub fn wrap<M>(
+    pub fn wrap<M, B1>(
         self,
         mw: M,
     ) -> Resource<
         impl ServiceFactory<
             ServiceRequest,
             Config = (),
-            Response = ServiceResponse,
+            Response = ServiceResponse<B1>,
             Error = Error,
             InitError = (),
         >,
+        B1,
     >
     where
         M: Transform<
             T::Service,
             ServiceRequest,
-            Response = ServiceResponse,
+            Response = ServiceResponse<B1>,
             Error = Error,
             InitError = (),
         >,
+        B1: MessageBody,
     {
         Resource {
             endpoint: apply(mw, self.endpoint),
@@ -282,6 +287,7 @@ where
             default: self.default,
             app_data: self.app_data,
             factory_ref: self.factory_ref,
+            _phantom: PhantomData,
         }
     }
 
@@ -319,21 +325,23 @@ where
     ///             .route(web::get().to(index)));
     /// }
     /// ```
-    pub fn wrap_fn<F, R>(
+    pub fn wrap_fn<F, R, B1>(
         self,
         mw: F,
     ) -> Resource<
         impl ServiceFactory<
             ServiceRequest,
             Config = (),
-            Response = ServiceResponse,
+            Response = ServiceResponse<B1>,
             Error = Error,
             InitError = (),
         >,
+        B1,
     >
     where
         F: Fn(ServiceRequest, &T::Service) -> R + Clone,
-        R: Future<Output = Result<ServiceResponse, Error>>,
+        R: Future<Output = Result<ServiceResponse<B1>, Error>>,
+        B1: MessageBody,
     {
         Resource {
             endpoint: apply_fn_factory(self.endpoint, mw),
@@ -344,6 +352,7 @@ where
             default: self.default,
             app_data: self.app_data,
             factory_ref: self.factory_ref,
+            _phantom: PhantomData,
         }
     }
 
@@ -371,15 +380,16 @@ where
     }
 }
 
-impl<T> HttpServiceFactory for Resource<T>
+impl<T, B> HttpServiceFactory for Resource<T, B>
 where
     T: ServiceFactory<
             ServiceRequest,
             Config = (),
-            Response = ServiceResponse,
+            Response = ServiceResponse<B>,
             Error = Error,
             InitError = (),
         > + 'static,
+    B: MessageBody + 'static,
 {
     fn register(mut self, config: &mut AppService) {
         let guards = if self.guards.is_empty() {
@@ -411,7 +421,9 @@ where
                 req.add_data_container(Rc::clone(data));
             }
 
-            srv.call(req)
+            let fut = srv.call(req);
+
+            async { Ok(fut.await?.map_into_boxed_body()) }
         });
 
         config.register_service(rdef, guards, endpoint, None)
@@ -534,11 +546,11 @@ mod tests {
             >,
         > {
             web::resource("/test-compat")
-                // .wrap_fn(|req, srv| {
+                .wrap_fn(|req, srv| {
-                //     let fut = srv.call(req);
+                    let fut = srv.call(req);
-                //     async { Ok(fut.await?.map_into_right_body::<()>()) }
+                    async { Ok(fut.await?.map_into_right_body::<()>()) }
-                // })
+                })
-                .wrap(Compat::new(DefaultHeaders::new()))
+                .wrap(Compat::noop())
                 .route(web::get().to(|| async { "hello" }))
         }
 
@@ -801,4 +813,26 @@ mod tests {
         let resp = call_service(&srv, req).await;
         assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
     }
+
+    #[actix_rt::test]
+    async fn test_middleware_body_type() {
+        let srv = init_service(
+            App::new().service(
+                web::resource("/test")
+                    .wrap_fn(|req, srv| {
+                        let fut = srv.call(req);
+                        async { Ok(fut.await?.map_into_right_body::<()>()) }
+                    })
+                    .route(web::get().to(|| async { "hello" })),
+            ),
+        )
+        .await;
+
+        // test if `try_into_bytes()` is preserved across scope layer
+        use actix_http::body::MessageBody as _;
+        let req = TestRequest::with_uri("/test").to_request();
+        let resp = call_service(&srv, req).await;
+        let body = resp.into_body();
+        assert_eq!(body.try_into_bytes().unwrap(), b"hello".as_ref());
+    }
 }

src/scope.rs

@@ -616,11 +616,11 @@ mod tests {
             >,
         > {
             web::scope("/test-compat")
-                // .wrap_fn(|req, srv| {
+                .wrap_fn(|req, srv| {
-                //     let fut = srv.call(req);
+                    let fut = srv.call(req);
-                //     async { Ok(fut.await?.map_into_right_body::<()>()) }
+                    async { Ok(fut.await?.map_into_right_body::<()>()) }
-                // })
+                })
-                .wrap(Compat::new(DefaultHeaders::new()))
+                .wrap(Compat::noop())
                 .service(web::resource("").route(web::get().to(|| async { "hello" })))
         }