From 2d8d2f5ab08c7db6ccfeba5a15a5085165a06ed3 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Thu, 24 Jun 2021 15:10:51 +0100 Subject: [PATCH] app data doc improvements --- src/app.rs | 85 +++++++++++++++++++----------- src/app_service.rs | 9 +++- src/config.rs | 11 ++-- src/data.rs | 5 ++ src/resource.rs | 76 ++++++++++++++------------- src/scope.rs | 128 +++++++++++++++++++++++---------------------- src/service.rs | 3 +- src/web.rs | 1 - 8 files changed, 181 insertions(+), 137 deletions(-) diff --git a/src/app.rs b/src/app.rs index 8c622dd3..677f7380 100644 --- a/src/app.rs +++ b/src/app.rs @@ -68,36 +68,71 @@ where InitError = (), >, { - /// Set application data. Application data could be accessed - /// by using `Data` extractor where `T` is data type. + /// Set application (root level) data. /// - /// **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 shared object should be used, e.g. `Arc`. Internally `Data` type - /// uses `Arc` so data could be created outside of app factory and clones could - /// be stored via `App::app_data()` method. + /// Application data stored with `App::app_data()` method is available through the + /// [`HttpRequest::app_data`](crate::HttpRequest::app_data) method at runtime. + /// + /// # [`Data`] + /// Any [`Data`] type added here can utilize it's extractor implementation in handlers. + /// Types not wrapped in `Data` cannot use this extractor. See [its docs](Data) for more + /// about its usage and patterns. /// /// ``` /// use std::cell::Cell; - /// use actix_web::{web, App, HttpResponse, Responder}; + /// use actix_web::{web, App, HttpRequest, HttpResponse, Responder}; /// /// struct MyData { - /// counter: Cell, + /// count: std::cell::Cell, /// } /// - /// async fn index(data: web::Data) -> impl Responder { - /// data.counter.set(data.counter.get() + 1); - /// HttpResponse::Ok() + /// async fn handler(req: HttpRequest, counter: web::Data) -> impl Responder { + /// // note this cannot use the Data extractor because it was not added with it + /// let incr = *req.app_data::().unwrap(); + /// assert_eq!(incr, 3); + /// + /// // update counter using other value from app data + /// counter.count.set(counter.count.get() + incr); + /// + /// HttpResponse::Ok().body(counter.count.get().to_string()) /// } /// - /// let app = App::new() - /// .data(MyData{ counter: Cell::new(0) }) - /// .service( - /// web::resource("/index.html").route( - /// web::get().to(index))); + /// let app = App::new().service( + /// web::resource("/") + /// .app_data(3usize) + /// .app_data(web::Data::new(MyData { count: Default::default() })) + /// .route(web::get().to(handler)) + /// ); /// ``` + /// + /// # Shared Mutable State + /// [`HttpServer::new`](crate::HttpServer::new) accepts an application factory rather than an + /// application instance; the factory closure is called on each worker thread independently. + /// Therefore, if you want to share a data object between different workers, a shareable object + /// needs to be created first, outside the `HttpServer::new` closure and cloned into it. + /// [`Data`] is an example of such a sharable object. + /// + /// ```ignore + /// let counter = web::Data::new(AppStateWithCounter { + /// counter: Mutex::new(0), + /// }); + /// + /// HttpServer::new(move || { + /// // move counter object into the closure and clone for each worker + /// + /// App::new() + /// .app_data(counter.clone()) + /// .route("/", web::get().to(handler)) + /// }) + /// ``` + pub fn app_data(mut self, ext: U) -> Self { + self.extensions.insert(ext); + self + } + + /// Add application (root) data after wrapping in `Data`. + /// + /// Deprecated in favor of [`app_data`](Self::app_data). #[deprecated(since = "4.0.0", note = "Use `.app_data(Data::new(val))` instead.")] pub fn data(self, data: U) -> Self { self.app_data(Data::new(data)) @@ -138,18 +173,6 @@ where self } - /// Set application level arbitrary data item. - /// - /// Application data stored with `App::app_data()` method is available - /// via `HttpRequest::app_data()` method at runtime. - /// - /// This method could be used for storing `Data` as well, in that case - /// data could be accessed by using `Data` extractor. - pub fn app_data(mut self, ext: U) -> Self { - self.extensions.insert(ext); - self - } - /// Run external configuration as part of the application building /// process /// diff --git a/src/app_service.rs b/src/app_service.rs index a9247b19..feb9f22a 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -144,7 +144,9 @@ where } } -/// Service that takes a [`Request`] and delegates to a service that take a [`ServiceRequest`]. +/// The [`Service`] that is passed to `actix-http`'s server builder. +/// +/// Wraps a service receiving a [`ServiceRequest`] into one receiving a [`Request`]. pub struct AppInitService where T: Service, Error = Error>, @@ -275,6 +277,7 @@ impl ServiceFactory for AppRoutingFactory { } } +/// The Actix Web router default entry point. pub struct AppRouting { router: Router, default: HttpService, @@ -299,6 +302,10 @@ impl Service for AppRouting { true }); + // you might expect to find `req.add_data_container()` called here but `HttpRequest` objects + // are created with the root data already set (in `AppInitService::call`) and root data is + // retained when releasing requests back to the pool + if let Some((srv, _info)) = res { srv.call(req) } else { diff --git a/src/config.rs b/src/config.rs index d22bc856..96614119 100644 --- a/src/config.rs +++ b/src/config.rs @@ -62,6 +62,8 @@ impl AppService { (self.config, self.services) } + /// Clones inner config and default service, returning new `AppService` with empty service list + /// marked as non-root. pub(crate) fn clone_config(&self) -> Self { AppService { config: self.config.clone(), @@ -71,12 +73,12 @@ impl AppService { } } - /// Service configuration + /// Returns reference to configuration. pub fn config(&self) -> &AppConfig { &self.config } - /// Default resource + /// Returns default handler factory. pub fn default_service(&self) -> Rc { self.default.clone() } @@ -116,7 +118,7 @@ impl AppConfig { AppConfig { secure, host, addr } } - /// Needed in actix-test crate. + /// Needed in actix-test crate. Semver exempt. #[doc(hidden)] pub fn __priv_test_new(secure: bool, host: String, addr: SocketAddr) -> Self { AppConfig::new(secure, host, addr) @@ -192,6 +194,7 @@ impl ServiceConfig { /// Add shared app data item. /// /// Counterpart to [`App::data()`](crate::App::data). + #[deprecated(since = "4.0.0", note = "Use `.app_data(Data::new(val))` instead.")] pub fn data(&mut self, data: U) -> &mut Self { self.app_data(Data::new(data)); self @@ -257,6 +260,8 @@ mod tests { use crate::test::{call_service, init_service, read_body, TestRequest}; use crate::{web, App, HttpRequest, HttpResponse}; + // allow deprecated `ServiceConfig::data` + #[allow(deprecated)] #[actix_rt::test] async fn test_data() { let cfg = |cfg: &mut ServiceConfig| { diff --git a/src/data.rs b/src/data.rs index 51db6ce4..174faba3 100644 --- a/src/data.rs +++ b/src/data.rs @@ -36,6 +36,11 @@ pub(crate) type FnDataFactory = /// If route data is not set for a handler, using `Data` extractor would cause *Internal /// Server Error* response. /// +// TODO: document `dyn T` functionality through converting an Arc +// TODO: note equivalence of req.app_data> and Data extractor +// TODO: note that data must be inserted using Data in order to extract it +/// +/// # Examples /// ``` /// use std::sync::Mutex; /// use actix_web::{web, App, HttpResponse, Responder}; diff --git a/src/resource.rs b/src/resource.rs index 9455895e..7a4c1248 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -169,41 +169,38 @@ where self } - /// Provide resource specific data. This method allows to add extractor - /// configuration or specific state available via `Data` extractor. - /// Provided data is available for all routes registered for the current resource. - /// Resource data overrides data registered by `App::data()` method. - /// - /// ``` - /// use actix_web::{web, App, FromRequest}; - /// - /// /// extract text data from request - /// async fn index(body: String) -> String { - /// format!("Body {}!", body) - /// } - /// - /// fn main() { - /// let app = App::new().service( - /// web::resource("/index.html") - /// // limit size of the payload - /// .data(String::configure(|cfg| { - /// cfg.limit(4096) - /// })) - /// .route( - /// web::get() - /// // register handler - /// .to(index) - /// )); - /// } - /// ``` - #[deprecated(since = "4.0.0", note = "Use `.app_data(Data::new(val))` instead.")] - pub fn data(self, data: U) -> Self { - self.app_data(Data::new(data)) - } - /// Add resource data. /// - /// Data of different types from parent contexts will still be accessible. + /// Data of different types from parent contexts will still be accessible. Any `Data` types + /// set here can be extracted in handlers using the `Data` extractor. + /// + /// # Examples + /// ``` + /// use std::cell::Cell; + /// use actix_web::{web, App, HttpRequest, HttpResponse, Responder}; + /// + /// struct MyData { + /// count: std::cell::Cell, + /// } + /// + /// async fn handler(req: HttpRequest, counter: web::Data) -> impl Responder { + /// // note this cannot use the Data extractor because it was not added with it + /// let incr = *req.app_data::().unwrap(); + /// assert_eq!(incr, 3); + /// + /// // update counter using other value from app data + /// counter.count.set(counter.count.get() + incr); + /// + /// HttpResponse::Ok().body(counter.count.get().to_string()) + /// } + /// + /// let app = App::new().service( + /// web::resource("/") + /// .app_data(3usize) + /// .app_data(web::Data::new(MyData { count: Default::default() })) + /// .route(web::get().to(handler)) + /// ); + /// ``` pub fn app_data(mut self, data: U) -> Self { self.app_data .get_or_insert_with(Extensions::new) @@ -212,6 +209,14 @@ where self } + /// Add resource data after wrapping in `Data`. + /// + /// Deprecated in favor of [`app_data`](Self::app_data). + #[deprecated(since = "4.0.0", note = "Use `.app_data(Data::new(val))` instead.")] + pub fn data(self, data: U) -> Self { + self.app_data(Data::new(data)) + } + /// Register a new route and add handler. This route matches all requests. /// /// ``` @@ -227,7 +232,6 @@ where /// This is shortcut for: /// /// ``` - /// # extern crate actix_web; /// # use actix_web::*; /// # fn index(req: HttpRequest) -> HttpResponse { unimplemented!() } /// App::new().service(web::resource("/").route(web::route().to(index))); @@ -695,7 +699,7 @@ mod tests { assert_eq!(resp.status(), StatusCode::NO_CONTENT); } - // allow deprecated App::data + // allow deprecated `{App, Resource}::data` #[allow(deprecated)] #[actix_rt::test] async fn test_data() { @@ -729,7 +733,7 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); } - // allow deprecated App::data + // allow deprecated `{App, Resource}::data` #[allow(deprecated)] #[actix_rt::test] async fn test_data_default_service() { diff --git a/src/scope.rs b/src/scope.rs index 86304074..0caf06ee 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,28 +1,23 @@ -use std::cell::RefCell; -use std::fmt; -use std::future::Future; -use std::rc::Rc; +use std::{cell::RefCell, fmt, future::Future, mem, rc::Rc}; use actix_http::Extensions; use actix_router::{ResourceDef, Router}; -use actix_service::boxed::{self, BoxService, BoxServiceFactory}; use actix_service::{ - apply, apply_fn_factory, IntoServiceFactory, Service, ServiceFactory, ServiceFactoryExt, - Transform, + apply, apply_fn_factory, + boxed::{self, BoxService, BoxServiceFactory}, + IntoServiceFactory, Service, ServiceFactory, ServiceFactoryExt, Transform, }; use futures_core::future::LocalBoxFuture; use futures_util::future::join_all; -use crate::config::ServiceConfig; -use crate::data::Data; -use crate::dev::{AppService, HttpServiceFactory}; -use crate::error::Error; -use crate::guard::Guard; -use crate::resource::Resource; -use crate::rmap::ResourceMap; -use crate::route::Route; -use crate::service::{ - AppServiceFactory, ServiceFactoryWrapper, ServiceRequest, ServiceResponse, +use crate::{ + config::ServiceConfig, + data::Data, + dev::{AppService, HttpServiceFactory}, + guard::Guard, + rmap::ResourceMap, + service::{AppServiceFactory, ServiceFactoryWrapper, ServiceRequest, ServiceResponse}, + Error, Resource, Route, }; type Guards = Vec>; @@ -71,16 +66,17 @@ pub struct Scope { impl Scope { /// Create a new scope pub fn new(path: &str) -> Scope { - let fref = Rc::new(RefCell::new(None)); + let factory_ref = Rc::new(RefCell::new(None)); + Scope { - endpoint: ScopeEndpoint::new(fref.clone()), + endpoint: ScopeEndpoint::new(Rc::clone(&factory_ref)), rdef: path.to_string(), app_data: None, guards: Vec::new(), services: Vec::new(), default: None, external: Vec::new(), - factory_ref: fref, + factory_ref, } } } @@ -120,40 +116,38 @@ where self } - /// Set or override application data. Application data could be accessed - /// by using `Data` extractor where `T` is data type. - /// - /// ``` - /// use std::cell::Cell; - /// use actix_web::{web, App, HttpResponse, Responder}; - /// - /// struct MyData { - /// counter: Cell, - /// } - /// - /// async fn index(data: web::Data) -> impl Responder { - /// data.counter.set(data.counter.get() + 1); - /// HttpResponse::Ok() - /// } - /// - /// fn main() { - /// let app = App::new().service( - /// web::scope("/app") - /// .data(MyData{ counter: Cell::new(0) }) - /// .service( - /// web::resource("/index.html").route( - /// web::get().to(index))) - /// ); - /// } - /// ``` - #[deprecated(since = "4.0.0", note = "Use `.app_data(Data::new(val))` instead.")] - pub fn data(self, data: U) -> Self { - self.app_data(Data::new(data)) - } - /// Add scope data. /// - /// Data of different types from parent contexts will still be accessible. + /// Data of different types from parent contexts will still be accessible. Any `Data` types + /// set here can be extracted in handlers using the `Data` extractor. + /// + /// # Examples + /// ``` + /// use std::cell::Cell; + /// use actix_web::{web, App, HttpRequest, HttpResponse, Responder}; + /// + /// struct MyData { + /// count: std::cell::Cell, + /// } + /// + /// async fn handler(req: HttpRequest, counter: web::Data) -> impl Responder { + /// // note this cannot use the Data extractor because it was not added with it + /// let incr = *req.app_data::().unwrap(); + /// assert_eq!(incr, 3); + /// + /// // update counter using other value from app data + /// counter.count.set(counter.count.get() + incr); + /// + /// HttpResponse::Ok().body(counter.count.get().to_string()) + /// } + /// + /// let app = App::new().service( + /// web::scope("/app") + /// .app_data(3usize) + /// .app_data(web::Data::new(MyData { count: Default::default() })) + /// .route("/", web::get().to(handler)) + /// ); + /// ``` pub fn app_data(mut self, data: U) -> Self { self.app_data .get_or_insert_with(Extensions::new) @@ -162,15 +156,20 @@ where self } - /// Run external configuration as part of the scope building - /// process + /// Add scope data after wrapping in `Data`. /// - /// This function is useful for moving parts of configuration to a - /// different module or even library. For example, - /// some of the resource's configuration could be moved to different module. + /// Deprecated in favor of [`app_data`](Self::app_data). + #[deprecated(since = "4.0.0", note = "Use `.app_data(Data::new(val))` instead.")] + pub fn data(self, data: U) -> Self { + self.app_data(Data::new(data)) + } + + /// Run external configuration as part of the scope building process. + /// + /// This function is useful for moving parts of configuration to a different module or library. + /// For example, some of the resource's configuration could be moved to different module. /// /// ``` - /// # extern crate actix_web; /// use actix_web::{web, middleware, App, HttpResponse}; /// /// // this function could be located in different module @@ -191,18 +190,21 @@ where /// .route("/index.html", web::get().to(|| HttpResponse::Ok())); /// } /// ``` - pub fn configure(mut self, f: F) -> Self + pub fn configure(mut self, cfg_fn: F) -> Self where F: FnOnce(&mut ServiceConfig), { let mut cfg = ServiceConfig::new(); - f(&mut cfg); + cfg_fn(&mut cfg); + self.services.extend(cfg.services); self.external.extend(cfg.external); + // TODO: add Extensions::is_empty check and conditionally insert data self.app_data .get_or_insert_with(Extensions::new) .extend(cfg.app_data); + self } @@ -419,7 +421,7 @@ where let mut rmap = ResourceMap::new(ResourceDef::root_prefix(&self.rdef)); // external resources - for mut rdef in std::mem::take(&mut self.external) { + for mut rdef in mem::take(&mut self.external) { rmap.add(&mut rdef, None); } @@ -991,7 +993,7 @@ mod tests { ); } - // allow deprecated App::data + // allow deprecated {App, Scope}::data #[allow(deprecated)] #[actix_rt::test] async fn test_override_data() { @@ -1011,7 +1013,7 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); } - // allow deprecated App::data + // allow deprecated `{App, Scope}::data` #[allow(deprecated)] #[actix_rt::test] async fn test_override_data_default_service() { diff --git a/src/service.rs b/src/service.rs index 8d73a87f..59257746 100644 --- a/src/service.rs +++ b/src/service.rs @@ -17,9 +17,8 @@ use crate::{ dev::insert_slash, guard::Guard, info::ConnectionInfo, - request::HttpRequest, rmap::ResourceMap, - Error, HttpResponse, + Error, HttpRequest, HttpResponse, }; pub trait HttpServiceFactory { diff --git a/src/web.rs b/src/web.rs index 8662848a..40ac4627 100644 --- a/src/web.rs +++ b/src/web.rs @@ -43,7 +43,6 @@ pub use crate::types::*; /// the exposed `Params` object: /// /// ``` -/// # extern crate actix_web; /// use actix_web::{web, App, HttpResponse}; /// /// let app = App::new().service(