diff --git a/CHANGES.md b/CHANGES.md index b3d3c180c..1621df27a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,12 +1,18 @@ # Changes ## Unreleased - 2020-xx-xx -* Implement Logger middleware regex exclude pattern [#1723] -* Print unconfigured `Data` type when attempting extraction. [#1743] +### Added +* Implement `exclude_regex` for Logger middleware. [#1723] +* Add request-local data extractor `web::ReqData`. [#1729] +### Changed +* Print non-configured `Data` type when attempting extraction. [#1743] + +[#1729]: https://github.com/actix/actix-web/pull/1729 [#1723]: https://github.com/actix/actix-web/pull/1723 [#1743]: https://github.com/actix/actix-web/pull/1743 + ## 3.1.0 - 2020-09-29 ### Changed * Add `TrailingSlash::MergeOnly` behaviour to `NormalizePath`, which allows `NormalizePath` diff --git a/src/data.rs b/src/data.rs index dad97ba87..19c258ff0 100644 --- a/src/data.rs +++ b/src/data.rs @@ -20,22 +20,20 @@ pub(crate) type FnDataFactory = /// Application data. /// -/// Application data is a piece of arbitrary data attached to the app. -/// Application data is available to all routes and can be added -/// during the application configuration process via `App::data()`. +/// Application level data is a piece of arbitrary data attached to the app, scope, or resource. +/// Application data is available to all routes and can be added during the application +/// configuration process via `App::data()`. /// -/// Application data can be accessed by using `Data` -/// extractor where `T` is data type. +/// Application data can be accessed by using `Data` extractor where `T` is data type. /// -/// **Note**: http server accepts an application factory rather than -/// an application instance. Http server constructs an application -/// instance for each thread, thus application data must be constructed -/// multiple times. If you want to share data between different -/// threads, a shareable object should be used, e.g. `Send + Sync`. Application -/// data does not need to be `Send` or `Sync`. Internally `Data` uses `Arc`. +/// **Note**: http server accepts an application factory rather than an application instance. HTTP +/// server constructs an application instance for each thread, thus application data must be +/// constructed multiple times. If you want to share data between different threads, a shareable +/// object should be used, e.g. `Send + Sync`. Application data does not need to be `Send` +/// or `Sync`. Internally `Data` uses `Arc`. /// -/// If route data is not set for a handler, using `Data` extractor would -/// cause *Internal Server Error* response. +/// If route data is not set for a handler, using `Data` extractor would cause *Internal +/// Server Error* response. /// /// ```rust /// use std::sync::Mutex; diff --git a/src/request_data.rs b/src/request_data.rs index 3eef9e07a..8994f2b7b 100644 --- a/src/request_data.rs +++ b/src/request_data.rs @@ -1,19 +1,60 @@ -use std::ops::{Deref, DerefMut}; +use std::{any::type_name, ops::Deref}; use actix_http::error::{Error, ErrorInternalServerError}; use futures_util::future; use crate::{dev::Payload, FromRequest, HttpRequest}; -/// Request data. +/// Request-local data. /// -/// Request data is a piece of arbitrary data attached to a request. +/// Request-local data is arbitrary data attached to an individual request, usually +/// by middleware. It can be set via `extensions_mut` on [`HttpRequest`](htr_ext_mut) +/// or [`ServiceRequest`](srv_ext_mut). /// -/// It can be set via [`HttpMessage::extensions_mut`]. +/// Unlike app data, request data is dropped when the request has finished processing. This makes it +/// useful as a kind of messaging system between middleware and request handlers. It uses the same +/// types-as-keys storage system as app data. /// -/// [`HttpMessage::extensions_mut`]: crate::HttpMessage::extensions_mut -#[derive(Clone, Debug)] -pub struct ReqData(pub T); +/// # Mutating Request Data +/// Note that since extractors must output owned data, only types that `impl Clone` can use this +/// extractor. A clone is taken of the required request data and can, therefore, not be directly +/// mutated in-place. To mutate request data, continue to use [`HttpRequest::extensions_mut`] or +/// re-insert the cloned data back into the extensions map. A `DerefMut` impl is intentionally not +/// provided to make this potential foot-gun more obvious. +/// +/// # Example +/// ```rust,no_run +/// # use actix_web::{web, HttpResponse, HttpRequest, Responder}; +/// +/// #[derive(Debug, Clone, PartialEq)] +/// struct FlagFromMiddleware(String); +/// +/// /// Use the `Data` extractor to access data in a handler. +/// async fn handler( +/// req: HttpRequest, +/// opt_flag: Option>, +/// ) -> impl Responder { +/// if let Some(flag) = opt_flag { +/// // using an optional extractor since the middleware may +/// // not be guaranteed to add our flag to request data +/// assert_eq!(&flag.into_inner(), req.extensions().get::().unwrap()); +/// } +/// +/// HttpResponse::Ok() +/// } +/// ``` +/// +/// [`htr_ext_mut`]: crate::HttpRequest::extensions_mut +/// [`srv_ext_mut`]: crate::ServiceRequest::extensions_mut +#[derive(Debug, Clone)] +pub struct ReqData(T); + +impl ReqData { + /// Consumes the `ReqData`, returning it's wrapped data. + pub fn into_inner(self) -> T { + self.0 + } +} impl Deref for ReqData { type Target = T; @@ -23,12 +64,6 @@ impl Deref for ReqData { } } -impl DerefMut for ReqData { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 - } -} - impl FromRequest for ReqData { type Config = (); type Error = Error; @@ -40,8 +75,9 @@ impl FromRequest for ReqData { } else { log::debug!( "Failed to construct App-level ReqData extractor. \ - Request path: {:?}", - req.path() + Request path: {:?} (type: {})", + req.path(), + type_name::(), ); future::err(ErrorInternalServerError( "Missing expected request extension data", @@ -49,3 +85,91 @@ impl FromRequest for ReqData { } } } + +#[cfg(test)] +mod tests { + use std::{cell::RefCell, rc::Rc}; + + use futures_util::TryFutureExt as _; + + use super::*; + use crate::{ + dev::Service, + http::{Method, StatusCode}, + test::{init_service, TestRequest}, + web, App, HttpMessage, HttpResponse, + }; + + #[actix_rt::test] + async fn req_data_extractor() { + let mut srv = init_service( + App::new() + .wrap_fn(|req, srv| { + if req.method() == Method::POST { + req.extensions_mut().insert(42u32); + } + + srv.call(req) + }) + .service(web::resource("/test").to( + |req: HttpRequest, data: Option>| { + if req.method() != Method::POST { + assert!(data.is_none()); + } + + if let Some(data) = data { + assert_eq!(*data, 42); + assert_eq!( + Some(data.into_inner()), + req.extensions().get::().copied() + ); + } + + HttpResponse::Ok() + }, + )), + ) + .await; + + let req = TestRequest::get().uri("/test").to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + + let req = TestRequest::post().uri("/test").to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } + + #[actix_rt::test] + async fn req_data_internal_mutability() { + let mut srv = init_service( + App::new() + .wrap_fn(|req, srv| { + let data_before = Rc::new(RefCell::new(42u32)); + req.extensions_mut().insert(data_before); + + srv.call(req).map_ok(|res| { + { + let ext = res.request().extensions(); + let data_after = ext.get::>>().unwrap(); + assert_eq!(*data_after.borrow(), 53u32); + } + + res + }) + }) + .default_service(web::to(|data: ReqData>>| { + assert_eq!(*data.borrow(), 42); + *data.borrow_mut() += 11; + assert_eq!(*data.borrow(), 53); + + HttpResponse::Ok() + })), + ) + .await; + + let req = TestRequest::get().uri("/test").to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } +}