From c3ce33df0564f6838dcd9cfdcd9a4681145c9322 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 5 Jan 2022 18:02:28 +0300 Subject: [PATCH] unify generics across App, Scope and Resource (#2572) Co-authored-by: Rob Ede --- src/app.rs | 150 +++++++++++++++++++-------------------- src/middleware/compat.rs | 25 +++++-- src/middleware/mod.rs | 2 +- src/resource.rs | 125 ++++++++++++-------------------- src/scope.rs | 116 ++++++++++++------------------ 5 files changed, 186 insertions(+), 232 deletions(-) diff --git a/src/app.rs b/src/app.rs index 3fddc055..da33ebc4 100644 --- a/src/app.rs +++ b/src/app.rs @@ -51,7 +51,10 @@ impl App { } } -impl App { +impl App +where + T: ServiceFactory, +{ /// Set application (root level) data. /// /// Application data stored with `App::app_data()` method is available through the @@ -317,65 +320,63 @@ impl App { self } - /// Registers middleware, in the form of a middleware component (type), - /// that runs during inbound and/or outbound processing in the request - /// life-cycle (request -> response), modifying request/response as - /// necessary, across all requests managed by the *Application*. + /// Registers an app-wide middleware. /// - /// Use middleware when you need to read or modify *every* request or - /// response in some way. + /// Registers middleware, in the form of a middleware compo nen t (type), that runs during + /// inbound and/or outbound processing in the request life-cycle (request -> response), + /// modifying request/response as necessary, across all requests managed by the `App`. /// - /// Notice that the keyword for registering middleware is `wrap`. As you - /// register middleware using `wrap` in the App builder, imagine wrapping - /// layers around an inner App. The first middleware layer exposed to a - /// Request is the outermost layer-- the *last* registered in - /// the builder chain. Consequently, the *first* middleware registered - /// in the builder chain is the *last* to execute during request processing. + /// Use middleware when you need to read or modify *every* request or response in some way. /// + /// Middleware can be applied similarly to individual `Scope`s and `Resource`s. + /// See [`Scope::wrap`](crate::Scope::wrap) and [`Resource::wrap`]. + /// + /// # Middleware Order + /// Notice that the keyword for registering middleware is `wrap`. As you register middleware + /// using `wrap` in the App builder, imagine wrapping layers around an inner App. The first + /// middleware layer exposed to a Request is the outermost layer (i.e., the *last* registered in + /// the builder chain). Consequently, the *first* middleware registered in the builder chain is + /// the *last* to start executing during request processing. + /// + /// Ordering is less obvious when wrapped services also have middleware applied. In this case, + /// middlewares are run in reverse order for `App` _and then_ in reverse order for the + /// wrapped service. + /// + /// # Examples /// ``` - /// use actix_service::Service; /// use actix_web::{middleware, web, App}; - /// use actix_web::http::header::{CONTENT_TYPE, HeaderValue}; /// /// async fn index() -> &'static str { /// "Welcome!" /// } /// - /// fn main() { - /// let app = App::new() - /// .wrap(middleware::Logger::default()) - /// .route("/index.html", web::get().to(index)); - /// } + /// let app = App::new() + /// .wrap(middleware::Logger::default()) + /// .route("/index.html", web::get().to(index)); /// ``` - pub fn wrap( + #[doc(alias = "middleware")] + #[doc(alias = "use")] // nodejs terminology + pub fn wrap( self, mw: M, ) -> App< impl ServiceFactory< ServiceRequest, Config = (), - Response = ServiceResponse, + Response = ServiceResponse, Error = Error, InitError = (), >, > where - T: ServiceFactory< - ServiceRequest, - Response = ServiceResponse, - Error = Error, - Config = (), - InitError = (), - >, - B: MessageBody, M: Transform< - T::Service, - ServiceRequest, - Response = ServiceResponse, - Error = Error, - InitError = (), - >, - B1: MessageBody, + T::Service, + ServiceRequest, + Response = ServiceResponse, + Error = Error, + InitError = (), + > + 'static, + B: MessageBody, { App { endpoint: apply(mw, self.endpoint), @@ -388,61 +389,57 @@ impl App { } } - /// Registers middleware, in the form of a closure, that runs during inbound - /// and/or outbound processing in the request life-cycle (request -> response), - /// modifying request/response as necessary, across all requests managed by - /// the *Application*. + /// Registers an app-wide function middleware. + /// + /// `mw` is a closure that runs during inbound and/or outbound processing in the request + /// life-cycle (request -> response), modifying request/response as necessary, across all + /// requests handled by the `App`. /// /// Use middleware when you need to read or modify *every* request or response in some way. /// + /// Middleware can also be applied to individual `Scope`s and `Resource`s. + /// + /// See [`App::wrap`] for details on how middlewares compose with each other. + /// + /// # Examples /// ``` - /// use actix_service::Service; - /// use actix_web::{web, App}; + /// use actix_web::{dev::Service as _, middleware, web, App}; /// use actix_web::http::header::{CONTENT_TYPE, HeaderValue}; /// /// async fn index() -> &'static str { /// "Welcome!" /// } /// - /// fn main() { - /// let app = App::new() - /// .wrap_fn(|req, srv| { - /// let fut = srv.call(req); - /// async { - /// let mut res = fut.await?; - /// res.headers_mut().insert( - /// CONTENT_TYPE, HeaderValue::from_static("text/plain"), - /// ); - /// Ok(res) - /// } - /// }) - /// .route("/index.html", web::get().to(index)); - /// } + /// let app = App::new() + /// .wrap_fn(|req, srv| { + /// let fut = srv.call(req); + /// async { + /// let mut res = fut.await?; + /// res.headers_mut() + /// .insert(CONTENT_TYPE, HeaderValue::from_static("text/plain")); + /// Ok(res) + /// } + /// }) + /// .route("/index.html", web::get().to(index)); /// ``` - pub fn wrap_fn( + #[doc(alias = "middleware")] + #[doc(alias = "use")] // nodejs terminology + pub fn wrap_fn( self, mw: F, ) -> App< impl ServiceFactory< ServiceRequest, Config = (), - Response = ServiceResponse, + Response = ServiceResponse, Error = Error, InitError = (), >, > where - T: ServiceFactory< - ServiceRequest, - Response = ServiceResponse, - Error = Error, - Config = (), - InitError = (), - >, + F: Fn(ServiceRequest, &T::Service) -> R + Clone + 'static, + R: Future, Error>>, B: MessageBody, - F: Fn(ServiceRequest, &T::Service) -> R + Clone, - R: Future, Error>>, - B1: MessageBody, { App { endpoint: apply_fn_factory(self.endpoint, mw), @@ -458,15 +455,14 @@ impl App { impl IntoServiceFactory, Request> for App where - B: MessageBody, T: ServiceFactory< - ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), - >, - T::Future: 'static, + ServiceRequest, + Config = (), + Response = ServiceResponse, + Error = Error, + InitError = (), + > + 'static, + B: MessageBody, { fn into_factory(self) -> AppInit { AppInit { diff --git a/src/middleware/compat.rs b/src/middleware/compat.rs index 18c9ff6a..ee8b8a49 100644 --- a/src/middleware/compat.rs +++ b/src/middleware/compat.rs @@ -150,11 +150,13 @@ mod tests { use actix_service::IntoService; - use crate::dev::ServiceRequest; - use crate::http::StatusCode; - use crate::middleware::{self, Condition, Logger}; - use crate::test::{call_service, init_service, TestRequest}; - use crate::{web, App, HttpResponse}; + use crate::{ + dev::ServiceRequest, + http::StatusCode, + middleware::{self, Condition, Logger}, + test::{self, call_service, init_service, TestRequest}, + web, App, HttpResponse, + }; #[actix_rt::test] #[cfg(all(feature = "cookies", feature = "__compress"))] @@ -219,4 +221,17 @@ mod tests { let resp = call_service(&mw, TestRequest::default().to_srv_request()).await; assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } + + #[actix_rt::test] + async fn compat_noop_is_noop() { + let srv = test::ok_service(); + + let mw = Compat::noop() + .new_transform(srv.into_service()) + .await + .unwrap(); + + let resp = call_service(&mw, TestRequest::default().to_srv_request()).await; + assert_eq!(resp.status(), StatusCode::OK); + } } diff --git a/src/middleware/mod.rs b/src/middleware/mod.rs index a781052a..0a61ad6c 100644 --- a/src/middleware/mod.rs +++ b/src/middleware/mod.rs @@ -1,4 +1,4 @@ -//! Commonly used middleware. +//! A collection of common middleware. mod compat; mod condition; diff --git a/src/resource.rs b/src/resource.rs index 8da0a8a8..193757ea 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -1,6 +1,6 @@ -use std::{cell::RefCell, fmt, future::Future, marker::PhantomData, rc::Rc}; +use std::{cell::RefCell, fmt, future::Future, rc::Rc}; -use actix_http::{body::BoxBody, Extensions}; +use actix_http::Extensions; use actix_router::{IntoPatterns, Patterns}; use actix_service::{ apply, apply_fn_factory, boxed, fn_service, IntoServiceFactory, Service, ServiceFactory, @@ -42,7 +42,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 { +pub struct Resource { endpoint: T, rdef: Patterns, name: Option, @@ -51,7 +51,6 @@ pub struct Resource { guards: Vec>, default: BoxedHttpServiceFactory, factory_ref: Rc>>, - _phantom: PhantomData, } impl Resource { @@ -69,21 +68,13 @@ impl Resource { default: boxed::factory(fn_service(|req: ServiceRequest| async { Ok(req.into_response(HttpResponse::MethodNotAllowed())) })), - _phantom: PhantomData, } } } -impl Resource +impl Resource where - T: ServiceFactory< - ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), - >, - B: MessageBody, + T: ServiceFactory, { /// Set resource name. /// @@ -241,35 +232,35 @@ where self } - /// Register a resource middleware. + /// Registers a resource middleware. /// - /// This is similar to `App's` middlewares, but middleware get invoked on resource level. - /// Resource level middlewares are not allowed to change response - /// type (i.e modify response's body). + /// `mw` is a middleware component (type), that can modify the request and response across all + /// routes managed by this `Resource`. /// - /// **Note**: middlewares get called in opposite order of middlewares registration. - pub fn wrap( + /// See [`App::wrap`](crate::App::wrap) for more details. + #[doc(alias = "middleware")] + #[doc(alias = "use")] // nodejs terminology + pub fn wrap( self, mw: M, ) -> Resource< impl ServiceFactory< ServiceRequest, Config = (), - Response = ServiceResponse, + Response = ServiceResponse, Error = Error, InitError = (), >, - B1, > where M: Transform< - T::Service, - ServiceRequest, - Response = ServiceResponse, - Error = Error, - InitError = (), - >, - B1: MessageBody, + T::Service, + ServiceRequest, + Response = ServiceResponse, + Error = Error, + InitError = (), + > + 'static, + B: MessageBody, { Resource { endpoint: apply(mw, self.endpoint), @@ -280,61 +271,34 @@ where default: self.default, app_data: self.app_data, factory_ref: self.factory_ref, - _phantom: PhantomData, } } - /// Register a resource middleware function. + /// Registers a resource function middleware. /// - /// This function accepts instance of `ServiceRequest` type and - /// mutable reference to the next middleware in chain. + /// `mw` is a closure that runs during inbound and/or outbound processing in the request + /// life-cycle (request -> response), modifying request/response as necessary, across all + /// requests handled by the `Resource`. /// - /// This is similar to `App's` middlewares, but middleware get invoked on resource level. - /// Resource level middlewares are not allowed to change response - /// type (i.e modify response's body). - /// - /// ``` - /// use actix_service::Service; - /// use actix_web::{web, App}; - /// use actix_web::http::header::{CONTENT_TYPE, HeaderValue}; - /// - /// async fn index() -> &'static str { - /// "Welcome!" - /// } - /// - /// fn main() { - /// let app = App::new().service( - /// web::resource("/index.html") - /// .wrap_fn(|req, srv| { - /// let fut = srv.call(req); - /// async { - /// let mut res = fut.await?; - /// res.headers_mut().insert( - /// CONTENT_TYPE, HeaderValue::from_static("text/plain"), - /// ); - /// Ok(res) - /// } - /// }) - /// .route(web::get().to(index))); - /// } - /// ``` - pub fn wrap_fn( + /// See [`App::wrap_fn`](crate::App::wrap_fn) for examples and more details. + #[doc(alias = "middleware")] + #[doc(alias = "use")] // nodejs terminology + pub fn wrap_fn( self, mw: F, ) -> Resource< impl ServiceFactory< ServiceRequest, Config = (), - Response = ServiceResponse, + Response = ServiceResponse, Error = Error, InitError = (), >, - B1, > where - F: Fn(ServiceRequest, &T::Service) -> R + Clone, - R: Future, Error>>, - B1: MessageBody, + F: Fn(ServiceRequest, &T::Service) -> R + Clone + 'static, + R: Future, Error>>, + B: MessageBody, { Resource { endpoint: apply_fn_factory(self.endpoint, mw), @@ -345,7 +309,6 @@ where default: self.default, app_data: self.app_data, factory_ref: self.factory_ref, - _phantom: PhantomData, } } @@ -373,7 +336,7 @@ where } } -impl HttpServiceFactory for Resource +impl HttpServiceFactory for Resource where T: ServiceFactory< ServiceRequest, @@ -517,7 +480,7 @@ mod tests { header::{self, HeaderValue}, Method, StatusCode, }, - middleware::{Compat, DefaultHeaders}, + middleware::DefaultHeaders, service::{ServiceRequest, ServiceResponse}, test::{call_service, init_service, TestRequest}, web, App, Error, HttpMessage, HttpResponse, @@ -525,31 +488,35 @@ mod tests { #[test] fn can_be_returned_from_fn() { - fn my_resource() -> Resource { - web::resource("/test").route(web::get().to(|| async { "hello" })) + fn my_resource_1() -> Resource { + web::resource("/test1").route(web::get().to(|| async { "hello" })) } - fn my_compat_resource() -> Resource< + fn my_resource_2() -> Resource< impl ServiceFactory< ServiceRequest, Config = (), - Response = ServiceResponse, + Response = ServiceResponse, Error = Error, InitError = (), >, > { - web::resource("/test-compat") + web::resource("/test2") .wrap_fn(|req, srv| { let fut = srv.call(req); async { Ok(fut.await?.map_into_right_body::<()>()) } }) - .wrap(Compat::noop()) .route(web::get().to(|| async { "hello" })) } + fn my_resource_3() -> impl HttpServiceFactory { + web::resource("/test2").route(web::get().to(|| async { "hello" })) + } + App::new() - .service(my_resource()) - .service(my_compat_resource()); + .service(my_resource_1()) + .service(my_resource_2()) + .service(my_resource_3()); } #[actix_rt::test] diff --git a/src/scope.rs b/src/scope.rs index fa9807f4..c05ce054 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,9 +1,6 @@ -use std::{cell::RefCell, fmt, future::Future, marker::PhantomData, mem, rc::Rc}; +use std::{cell::RefCell, fmt, future::Future, mem, rc::Rc}; -use actix_http::{ - body::{BoxBody, MessageBody}, - Extensions, -}; +use actix_http::{body::MessageBody, Extensions}; use actix_router::{ResourceDef, Router}; use actix_service::{ apply, apply_fn_factory, boxed, IntoServiceFactory, Service, ServiceFactory, @@ -57,7 +54,7 @@ type Guards = Vec>; /// /// [pat]: crate::dev::ResourceDef#prefix-resources /// [dynamic segments]: crate::dev::ResourceDef#dynamic-segments -pub struct Scope { +pub struct Scope { endpoint: T, rdef: String, app_data: Option, @@ -66,7 +63,6 @@ pub struct Scope { default: Option>, external: Vec, factory_ref: Rc>>, - _phantom: PhantomData, } impl Scope { @@ -83,21 +79,13 @@ impl Scope { default: None, external: Vec::new(), factory_ref, - _phantom: Default::default(), } } } -impl Scope +impl Scope where - T: ServiceFactory< - ServiceRequest, - Config = (), - Response = ServiceResponse, - Error = Error, - InitError = (), - >, - B: 'static, + T: ServiceFactory, { /// Add match guard to a scope. /// @@ -296,32 +284,35 @@ where self } - /// Registers middleware, in the form of a middleware component (type), that runs during inbound - /// processing in the request life-cycle (request -> response), modifying request as necessary, - /// across all requests managed by the *Scope*. + /// Registers a scope-wide middleware. /// - /// Use middleware when you need to read or modify *every* request in some way. - pub fn wrap( + /// `mw` is a middleware component (type), that can modify the request and response across all + /// sub-resources managed by this `Scope`. + /// + /// See [`App::wrap`](crate::App::wrap) for more details. + #[doc(alias = "middleware")] + #[doc(alias = "use")] // nodejs terminology + pub fn wrap( self, mw: M, ) -> Scope< impl ServiceFactory< ServiceRequest, Config = (), - Response = ServiceResponse, + Response = ServiceResponse, Error = Error, InitError = (), >, - B1, > where M: Transform< - T::Service, - ServiceRequest, - Response = ServiceResponse, - Error = Error, - InitError = (), - >, + T::Service, + ServiceRequest, + Response = ServiceResponse, + Error = Error, + InitError = (), + > + 'static, + B: MessageBody, { Scope { endpoint: apply(mw, self.endpoint), @@ -332,54 +323,34 @@ where default: self.default, external: self.external, factory_ref: self.factory_ref, - _phantom: PhantomData, } } - /// Registers middleware, in the form of a closure, that runs during inbound processing in the - /// request life-cycle (request -> response), modifying request as necessary, across all - /// requests managed by the *Scope*. + /// Registers a scope-wide function middleware. /// - /// # Examples - /// ``` - /// use actix_service::Service; - /// use actix_web::{web, App}; - /// use actix_web::http::header::{CONTENT_TYPE, HeaderValue}; + /// `mw` is a closure that runs during inbound and/or outbound processing in the request + /// life-cycle (request -> response), modifying request/response as necessary, across all + /// requests handled by the `Scope`. /// - /// async fn index() -> &'static str { - /// "Welcome!" - /// } - /// - /// let app = App::new().service( - /// web::scope("/app") - /// .wrap_fn(|req, srv| { - /// let fut = srv.call(req); - /// async { - /// let mut res = fut.await?; - /// res.headers_mut().insert( - /// CONTENT_TYPE, HeaderValue::from_static("text/plain"), - /// ); - /// Ok(res) - /// } - /// }) - /// .route("/index.html", web::get().to(index))); - /// ``` - pub fn wrap_fn( + /// See [`App::wrap_fn`](crate::App::wrap_fn) for examples and more details. + #[doc(alias = "middleware")] + #[doc(alias = "use")] // nodejs terminology + pub fn wrap_fn( self, mw: F, ) -> Scope< impl ServiceFactory< ServiceRequest, Config = (), - Response = ServiceResponse, + Response = ServiceResponse, Error = Error, InitError = (), >, - B1, > where - F: Fn(ServiceRequest, &T::Service) -> R + Clone, - R: Future, Error>>, + F: Fn(ServiceRequest, &T::Service) -> R + Clone + 'static, + R: Future, Error>>, + B: MessageBody, { Scope { endpoint: apply_fn_factory(self.endpoint, mw), @@ -390,12 +361,11 @@ where default: self.default, external: self.external, factory_ref: self.factory_ref, - _phantom: PhantomData, } } } -impl HttpServiceFactory for Scope +impl HttpServiceFactory for Scope where T: ServiceFactory< ServiceRequest, @@ -596,7 +566,7 @@ mod tests { header::{self, HeaderValue}, Method, StatusCode, }, - middleware::{Compat, DefaultHeaders}, + middleware::DefaultHeaders, service::{ServiceRequest, ServiceResponse}, test::{assert_body_eq, call_service, init_service, read_body, TestRequest}, web, App, HttpMessage, HttpRequest, HttpResponse, @@ -604,16 +574,16 @@ mod tests { #[test] fn can_be_returned_from_fn() { - fn my_scope() -> Scope { + fn my_scope_1() -> Scope { web::scope("/test") .service(web::resource("").route(web::get().to(|| async { "hello" }))) } - fn my_compat_scope() -> Scope< + fn my_scope_2() -> Scope< impl ServiceFactory< ServiceRequest, Config = (), - Response = ServiceResponse, + Response = ServiceResponse, Error = Error, InitError = (), >, @@ -623,11 +593,17 @@ mod tests { let fut = srv.call(req); async { Ok(fut.await?.map_into_right_body::<()>()) } }) - .wrap(Compat::noop()) .service(web::resource("").route(web::get().to(|| async { "hello" }))) } - App::new().service(my_scope()).service(my_compat_scope()); + fn my_scope_3() -> impl HttpServiceFactory { + my_scope_2() + } + + App::new() + .service(my_scope_1()) + .service(my_scope_2()) + .service(my_scope_3()); } #[actix_rt::test]