From fa78da81569f23accfe7b293be0c022fd01bbdb3 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Sat, 4 May 2019 19:43:49 -0700 Subject: [PATCH] unify route and app data, it allows to provide global extractor config #775 --- CHANGES.md | 12 +++ src/app.rs | 51 +++++------ src/app_service.rs | 48 ++++++----- src/config.rs | 130 +++++++++++++--------------- src/data.rs | 189 ++++++++--------------------------------- src/extract.rs | 2 +- src/handler.rs | 13 +-- src/middleware/cors.rs | 5 +- src/request.rs | 35 ++++---- src/resource.rs | 56 +++++++++++- src/route.rs | 66 ++------------ src/scope.rs | 1 + src/service.rs | 9 +- src/test.rs | 42 +++++---- src/types/form.rs | 13 ++- src/types/json.rs | 29 +++---- src/types/payload.rs | 15 ++-- src/web.rs | 2 +- 18 files changed, 292 insertions(+), 426 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1d31b3514..cfd7a6df8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,11 +1,19 @@ # Changes +## [1.0.0-beta.3] - 2019-05-04 + ### Added * Add helper function for executing futures `test::block_fn()` ### Changed +* Extractor configuration could be registered with `App::data()` + or with `Resource::data()` #775 + +* Route data is unified with app data, `Route::data()` moved to resource + level to `Resource::data()` + * CORS handling without headers #702 * Allow to construct `Data` instances to avoid double `Arc` for `Send + Sync` types. @@ -14,6 +22,10 @@ * Fix `NormalizePath` middleware impl #806 +### Deleted + +* `App::data_factory()` is deleted. + ## [1.0.0-beta.2] - 2019-04-24 diff --git a/src/app.rs b/src/app.rs index 7e5cd3945..bac71250f 100644 --- a/src/app.rs +++ b/src/app.rs @@ -100,19 +100,6 @@ where self } - /// Set application data factory. This function is - /// similar to `.data()` but it accepts data factory. Data object get - /// constructed asynchronously during application initialization. - pub fn data_factory(mut self, data: F) -> Self - where - F: Fn() -> R + 'static, - R: IntoFuture + 'static, - R::Error: std::fmt::Debug, - { - self.data.push(Box::new(data)); - self - } - /// Run external configuration as part of the application building /// process /// @@ -425,9 +412,9 @@ where { fn into_new_service(self) -> AppInit { AppInit { - data: self.data, + data: Rc::new(self.data), endpoint: self.endpoint, - services: RefCell::new(self.services), + services: Rc::new(RefCell::new(self.services)), external: RefCell::new(self.external), default: self.default, factory_ref: self.factory_ref, @@ -493,24 +480,24 @@ mod tests { assert_eq!(resp.status(), StatusCode::CREATED); } - #[test] - fn test_data_factory() { - let mut srv = - init_service(App::new().data_factory(|| Ok::<_, ()>(10usize)).service( - web::resource("/").to(|_: web::Data| HttpResponse::Ok()), - )); - let req = TestRequest::default().to_request(); - let resp = block_on(srv.call(req)).unwrap(); - assert_eq!(resp.status(), StatusCode::OK); + // #[test] + // fn test_data_factory() { + // let mut srv = + // init_service(App::new().data_factory(|| Ok::<_, ()>(10usize)).service( + // web::resource("/").to(|_: web::Data| HttpResponse::Ok()), + // )); + // let req = TestRequest::default().to_request(); + // let resp = block_on(srv.call(req)).unwrap(); + // assert_eq!(resp.status(), StatusCode::OK); - let mut srv = - init_service(App::new().data_factory(|| Ok::<_, ()>(10u32)).service( - web::resource("/").to(|_: web::Data| HttpResponse::Ok()), - )); - let req = TestRequest::default().to_request(); - let resp = block_on(srv.call(req)).unwrap(); - assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); - } + // let mut srv = + // init_service(App::new().data_factory(|| Ok::<_, ()>(10u32)).service( + // web::resource("/").to(|_: web::Data| HttpResponse::Ok()), + // )); + // let req = TestRequest::default().to_request(); + // let resp = block_on(srv.call(req)).unwrap(); + // assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); + // } fn md( req: ServiceRequest, diff --git a/src/app_service.rs b/src/app_service.rs index 7229a2301..e2f918428 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -2,7 +2,7 @@ use std::cell::RefCell; use std::marker::PhantomData; use std::rc::Rc; -use actix_http::{Request, Response}; +use actix_http::{Extensions, Request, Response}; use actix_router::{Path, ResourceDef, ResourceInfo, Router, Url}; use actix_server_config::ServerConfig; use actix_service::boxed::{self, BoxedNewService, BoxedService}; @@ -11,7 +11,7 @@ use futures::future::{ok, Either, FutureResult}; use futures::{Async, Future, Poll}; use crate::config::{AppConfig, AppService}; -use crate::data::{DataFactory, DataFactoryResult}; +use crate::data::DataFactory; use crate::error::Error; use crate::guard::Guard; use crate::request::{HttpRequest, HttpRequestPool}; @@ -38,9 +38,9 @@ where >, { pub(crate) endpoint: T, - pub(crate) data: Vec>, + pub(crate) data: Rc>>, pub(crate) config: RefCell, - pub(crate) services: RefCell>>, + pub(crate) services: Rc>>>, pub(crate) default: Option>, pub(crate) factory_ref: Rc>>, pub(crate) external: RefCell>, @@ -70,6 +70,7 @@ where }))) }); + // App config { let mut c = self.config.borrow_mut(); let loc_cfg = Rc::get_mut(&mut c.0).unwrap(); @@ -77,7 +78,11 @@ where loc_cfg.addr = cfg.local_addr(); } - let mut config = AppService::new(self.config.borrow().clone(), default.clone()); + let mut config = AppService::new( + self.config.borrow().clone(), + default.clone(), + self.data.clone(), + ); // register services std::mem::replace(&mut *self.services.borrow_mut(), Vec::new()) @@ -86,12 +91,13 @@ where let mut rmap = ResourceMap::new(ResourceDef::new("")); + let (config, services) = config.into_services(); + // complete pipeline creation *self.factory_ref.borrow_mut() = Some(AppRoutingFactory { default, services: Rc::new( - config - .into_services() + services .into_iter() .map(|(mut rdef, srv, guards, nested)| { rmap.add(&mut rdef, nested); @@ -110,11 +116,17 @@ where let rmap = Rc::new(rmap); rmap.finish(rmap.clone()); + // create app data container + let mut data = Extensions::new(); + for f in self.data.iter() { + f.create(&mut data); + } + AppInitResult { endpoint: None, endpoint_fut: self.endpoint.new_service(&()), - data: self.data.iter().map(|s| s.construct()).collect(), - config: self.config.borrow().clone(), + data: Rc::new(data), + config, rmap, _t: PhantomData, } @@ -128,8 +140,8 @@ where endpoint: Option, endpoint_fut: T::Future, rmap: Rc, - data: Vec>, config: AppConfig, + data: Rc, _t: PhantomData, } @@ -146,27 +158,18 @@ where type Error = T::InitError; fn poll(&mut self) -> Poll { - let mut idx = 0; - let mut extensions = self.config.0.extensions.borrow_mut(); - while idx < self.data.len() { - if let Async::Ready(_) = self.data[idx].poll_result(&mut extensions)? { - self.data.remove(idx); - } else { - idx += 1; - } - } - if self.endpoint.is_none() { if let Async::Ready(srv) = self.endpoint_fut.poll()? { self.endpoint = Some(srv); } } - if self.endpoint.is_some() && self.data.is_empty() { + if self.endpoint.is_some() { Ok(Async::Ready(AppInitService { service: self.endpoint.take().unwrap(), rmap: self.rmap.clone(), config: self.config.clone(), + data: self.data.clone(), pool: HttpRequestPool::create(), })) } else { @@ -183,6 +186,7 @@ where service: T, rmap: Rc, config: AppConfig, + data: Rc, pool: &'static HttpRequestPool, } @@ -207,6 +211,7 @@ where inner.path.get_mut().update(&head.uri); inner.path.reset(); inner.head = head; + inner.app_data = self.data.clone(); req } else { HttpRequest::new( @@ -214,6 +219,7 @@ where head, self.rmap.clone(), self.config.clone(), + self.data.clone(), self.pool, ) }; diff --git a/src/config.rs b/src/config.rs index 4c4bfa220..e4e390b15 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,11 +1,9 @@ -use std::cell::{Ref, RefCell}; use std::net::SocketAddr; use std::rc::Rc; use actix_http::Extensions; use actix_router::ResourceDef; use actix_service::{boxed, IntoNewService, NewService}; -use futures::IntoFuture; use crate::data::{Data, DataFactory}; use crate::error::Error; @@ -33,14 +31,20 @@ pub struct AppService { Option, Option>, )>, + route_data: Rc>>, } impl AppService { /// Crate server settings instance - pub(crate) fn new(config: AppConfig, default: Rc) -> Self { + pub(crate) fn new( + config: AppConfig, + default: Rc, + route_data: Rc>>, + ) -> Self { AppService { config, default, + route_data, root: true, services: Vec::new(), } @@ -53,13 +57,16 @@ impl AppService { pub(crate) fn into_services( self, - ) -> Vec<( - ResourceDef, - HttpNewService, - Option, - Option>, - )> { - self.services + ) -> ( + AppConfig, + Vec<( + ResourceDef, + HttpNewService, + Option, + Option>, + )>, + ) { + (self.config, self.services) } pub(crate) fn clone_config(&self) -> Self { @@ -68,6 +75,7 @@ impl AppService { default: self.default.clone(), services: Vec::new(), root: false, + route_data: self.route_data.clone(), } } @@ -81,6 +89,15 @@ impl AppService { self.default.clone() } + /// Set global route data + pub fn set_route_data(&self, extensions: &mut Extensions) -> bool { + for f in self.route_data.iter() { + f.create(extensions); + } + !self.route_data.is_empty() + } + + /// Register http service pub fn register_service( &mut self, rdef: ResourceDef, @@ -133,24 +150,12 @@ impl AppConfig { pub fn local_addr(&self) -> SocketAddr { self.0.addr } - - /// Resource map - pub fn rmap(&self) -> &ResourceMap { - &self.0.rmap - } - - /// Application extensions - pub fn extensions(&self) -> Ref { - self.0.extensions.borrow() - } } pub(crate) struct AppConfigInner { pub(crate) secure: bool, pub(crate) host: String, pub(crate) addr: SocketAddr, - pub(crate) rmap: ResourceMap, - pub(crate) extensions: RefCell, } impl Default for AppConfigInner { @@ -159,8 +164,6 @@ impl Default for AppConfigInner { secure: false, addr: "127.0.0.1:8080".parse().unwrap(), host: "localhost:8080".to_owned(), - rmap: ResourceMap::new(ResourceDef::new("")), - extensions: RefCell::new(Extensions::new()), } } } @@ -188,23 +191,8 @@ impl ServiceConfig { /// by using `Data` extractor where `T` is data type. /// /// This is same as `App::data()` method. - pub fn data(&mut self, data: S) -> &mut Self { - self.data.push(Box::new(Data::new(data))); - self - } - - /// Set application data factory. This function is - /// similar to `.data()` but it accepts data factory. Data object get - /// constructed asynchronously during application initialization. - /// - /// This is same as `App::data_dactory()` method. - pub fn data_factory(&mut self, data: F) -> &mut Self - where - F: Fn() -> R + 'static, - R: IntoFuture + 'static, - R::Error: std::fmt::Debug, - { - self.data.push(Box::new(data)); + pub fn data> + 'static>(&mut self, data: S) -> &mut Self { + self.data.push(Box::new(data.into())); self } @@ -254,8 +242,6 @@ impl ServiceConfig { mod tests { use actix_service::Service; use bytes::Bytes; - use futures::Future; - use tokio_timer::sleep; use super::*; use crate::http::{Method, StatusCode}; @@ -277,37 +263,37 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); } - #[test] - fn test_data_factory() { - let cfg = |cfg: &mut ServiceConfig| { - cfg.data_factory(|| { - sleep(std::time::Duration::from_millis(50)).then(|_| { - println!("READY"); - Ok::<_, ()>(10usize) - }) - }); - }; + // #[test] + // fn test_data_factory() { + // let cfg = |cfg: &mut ServiceConfig| { + // cfg.data_factory(|| { + // sleep(std::time::Duration::from_millis(50)).then(|_| { + // println!("READY"); + // Ok::<_, ()>(10usize) + // }) + // }); + // }; - let mut srv = - init_service(App::new().configure(cfg).service( - web::resource("/").to(|_: web::Data| HttpResponse::Ok()), - )); - let req = TestRequest::default().to_request(); - let resp = block_on(srv.call(req)).unwrap(); - assert_eq!(resp.status(), StatusCode::OK); + // let mut srv = + // init_service(App::new().configure(cfg).service( + // web::resource("/").to(|_: web::Data| HttpResponse::Ok()), + // )); + // let req = TestRequest::default().to_request(); + // let resp = block_on(srv.call(req)).unwrap(); + // assert_eq!(resp.status(), StatusCode::OK); - let cfg2 = |cfg: &mut ServiceConfig| { - cfg.data_factory(|| Ok::<_, ()>(10u32)); - }; - let mut srv = init_service( - App::new() - .service(web::resource("/").to(|_: web::Data| HttpResponse::Ok())) - .configure(cfg2), - ); - let req = TestRequest::default().to_request(); - let resp = block_on(srv.call(req)).unwrap(); - assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); - } + // let cfg2 = |cfg: &mut ServiceConfig| { + // cfg.data_factory(|| Ok::<_, ()>(10u32)); + // }; + // let mut srv = init_service( + // App::new() + // .service(web::resource("/").to(|_: web::Data| HttpResponse::Ok())) + // .configure(cfg2), + // ); + // let req = TestRequest::default().to_request(); + // let resp = block_on(srv.call(req)).unwrap(); + // assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); + // } #[test] fn test_external_resource() { diff --git a/src/data.rs b/src/data.rs index c77ba3e22..f23bfff7f 100644 --- a/src/data.rs +++ b/src/data.rs @@ -3,7 +3,6 @@ use std::sync::Arc; use actix_http::error::{Error, ErrorInternalServerError}; use actix_http::Extensions; -use futures::{Async, Future, IntoFuture, Poll}; use crate::dev::Payload; use crate::extract::FromRequest; @@ -11,11 +10,7 @@ use crate::request::HttpRequest; /// Application data factory pub(crate) trait DataFactory { - fn construct(&self) -> Box; -} - -pub(crate) trait DataFactoryResult { - fn poll_result(&mut self, extensions: &mut Extensions) -> Poll<(), ()>; + fn create(&self, extensions: &mut Extensions) -> bool; } /// Application data. @@ -111,8 +106,8 @@ impl FromRequest for Data { #[inline] fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { - if let Some(st) = req.app_config().extensions().get::>() { - Ok(st.clone()) + if let Some(st) = req.get_app_data::() { + Ok(st) } else { log::debug!( "Failed to construct App-level Data extractor. \ @@ -127,142 +122,12 @@ impl FromRequest for Data { } impl DataFactory for Data { - fn construct(&self) -> Box { - Box::new(DataFut { st: self.clone() }) - } -} - -struct DataFut { - st: Data, -} - -impl DataFactoryResult for DataFut { - fn poll_result(&mut self, extensions: &mut Extensions) -> Poll<(), ()> { - extensions.insert(self.st.clone()); - Ok(Async::Ready(())) - } -} - -impl DataFactory for F -where - F: Fn() -> Out + 'static, - Out: IntoFuture + 'static, - Out::Error: std::fmt::Debug, -{ - fn construct(&self) -> Box { - Box::new(DataFactoryFut { - fut: (*self)().into_future(), - }) - } -} - -struct DataFactoryFut -where - F: Future, - F::Error: std::fmt::Debug, -{ - fut: F, -} - -impl DataFactoryResult for DataFactoryFut -where - F: Future, - F::Error: std::fmt::Debug, -{ - fn poll_result(&mut self, extensions: &mut Extensions) -> Poll<(), ()> { - match self.fut.poll() { - Ok(Async::Ready(s)) => { - extensions.insert(Data::new(s)); - Ok(Async::Ready(())) - } - Ok(Async::NotReady) => Ok(Async::NotReady), - Err(e) => { - log::error!("Can not construct application state: {:?}", e); - Err(()) - } - } - } -} - -/// Route data. -/// -/// Route data is an arbitrary data attached to specific route. -/// Route data could be added to route during route configuration process -/// with `Route::data()` method. Route data is also used as an extractor -/// configuration storage. Route data could be accessed in handler -/// via `RouteData` extractor. -/// -/// If route data is not set for a handler, using `RouteData` extractor -/// would cause *Internal Server Error* response. -/// -/// ```rust -/// # use std::cell::Cell; -/// use actix_web::{web, App}; -/// -/// struct MyData { -/// counter: Cell, -/// } -/// -/// /// Use `RouteData` extractor to access data in handler. -/// fn index(data: web::RouteData) { -/// data.counter.set(data.counter.get() + 1); -/// } -/// -/// fn main() { -/// let app = App::new().service( -/// web::resource("/index.html").route( -/// web::get() -/// // Store `MyData` in route storage -/// .data(MyData{ counter: Cell::new(0) }) -/// // Route data could be used as extractor configuration storage, -/// // limit size of the payload -/// .data(web::PayloadConfig::new(4096)) -/// // register handler -/// .to(index) -/// )); -/// } -/// ``` -pub struct RouteData(Arc); - -impl RouteData { - pub(crate) fn new(state: T) -> RouteData { - RouteData(Arc::new(state)) - } - - /// Get referecnce to inner data object. - pub fn get_ref(&self) -> &T { - self.0.as_ref() - } -} - -impl Deref for RouteData { - type Target = T; - - fn deref(&self) -> &T { - self.0.as_ref() - } -} - -impl Clone for RouteData { - fn clone(&self) -> RouteData { - RouteData(self.0.clone()) - } -} - -impl FromRequest for RouteData { - type Config = (); - type Error = Error; - type Future = Result; - - #[inline] - fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { - if let Some(st) = req.route_data::() { - Ok(st.clone()) + fn create(&self, extensions: &mut Extensions) -> bool { + if !extensions.contains::>() { + let _ = extensions.insert(Data(self.0.clone())); + true } else { - log::debug!("Failed to construct Route-level Data extractor"); - Err(ErrorInternalServerError( - "Route data is not configured, to configure use Route::data()", - )) + false } } } @@ -297,12 +162,13 @@ mod tests { #[test] fn test_route_data_extractor() { - let mut srv = init_service(App::new().service(web::resource("/").route( - web::get().data(10usize).to(|data: web::RouteData| { - let _ = data.clone(); - HttpResponse::Ok() - }), - ))); + let mut srv = + init_service(App::new().service(web::resource("/").data(10usize).route( + web::get().to(|data: web::Data| { + let _ = data.clone(); + HttpResponse::Ok() + }), + ))); let req = TestRequest::default().to_request(); let resp = block_on(srv.call(req)).unwrap(); @@ -311,15 +177,30 @@ mod tests { // different type let mut srv = init_service( App::new().service( - web::resource("/").route( - web::get() - .data(10u32) - .to(|_: web::RouteData| HttpResponse::Ok()), - ), + web::resource("/") + .data(10u32) + .route(web::get().to(|_: web::Data| HttpResponse::Ok())), ), ); let req = TestRequest::default().to_request(); let resp = block_on(srv.call(req)).unwrap(); assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } + + #[test] + fn test_override_data() { + let mut srv = init_service(App::new().data(1usize).service( + web::resource("/").data(10usize).route(web::get().to( + |data: web::Data| { + assert_eq!(*data, 10); + let _ = data.clone(); + HttpResponse::Ok() + }, + )), + )); + + let req = TestRequest::default().to_request(); + let resp = block_on(srv.call(req)).unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } } diff --git a/src/extract.rs b/src/extract.rs index 6d414fbcc..17b5cb40c 100644 --- a/src/extract.rs +++ b/src/extract.rs @@ -283,7 +283,7 @@ mod tests { header::CONTENT_TYPE, "application/x-www-form-urlencoded", ) - .route_data(FormConfig::default().limit(4096)) + .data(FormConfig::default().limit(4096)) .to_http_parts(); let r = block_on(Option::>::from_request(&req, &mut pl)).unwrap(); diff --git a/src/handler.rs b/src/handler.rs index 850c0c92c..245aba9d1 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -1,8 +1,6 @@ -use std::cell::RefCell; use std::marker::PhantomData; -use std::rc::Rc; -use actix_http::{Error, Extensions, Payload, Response}; +use actix_http::{Error, Payload, Response}; use actix_service::{NewService, Service, Void}; use futures::future::{ok, FutureResult}; use futures::{try_ready, Async, Future, IntoFuture, Poll}; @@ -267,15 +265,13 @@ where /// Extract arguments from request pub struct Extract { - config: Rc>>>, service: S, _t: PhantomData, } impl Extract { - pub fn new(config: Rc>>>, service: S) -> Self { + pub fn new(service: S) -> Self { Extract { - config, service, _t: PhantomData, } @@ -297,14 +293,12 @@ where fn new_service(&self, _: &()) -> Self::Future { ok(ExtractService { _t: PhantomData, - config: self.config.borrow().clone(), service: self.service.clone(), }) } } pub struct ExtractService { - config: Option>, service: S, _t: PhantomData, } @@ -324,8 +318,7 @@ where } fn call(&mut self, req: ServiceRequest) -> Self::Future { - let (mut req, mut payload) = req.into_parts(); - req.set_route_data(self.config.clone()); + let (req, mut payload) = req.into_parts(); let fut = T::from_request(&req, &mut payload).into_future(); ExtractResponse { diff --git a/src/middleware/cors.rs b/src/middleware/cors.rs index bd57c66ae..bb4fd567f 100644 --- a/src/middleware/cors.rs +++ b/src/middleware/cors.rs @@ -870,10 +870,7 @@ mod tests { let req = TestRequest::with_header("Origin", "https://www.example.com") .method(Method::OPTIONS) - .header( - header::ACCESS_CONTROL_REQUEST_HEADERS, - "X-Not-Allowed", - ) + .header(header::ACCESS_CONTROL_REQUEST_HEADERS, "X-Not-Allowed") .to_srv_request(); assert!(cors.inner.validate_allowed_method(req.head()).is_err()); diff --git a/src/request.rs b/src/request.rs index ad5b2488f..7b3ab04a2 100644 --- a/src/request.rs +++ b/src/request.rs @@ -7,7 +7,7 @@ use actix_http::{Error, Extensions, HttpMessage, Message, Payload, RequestHead}; use actix_router::{Path, Url}; use crate::config::AppConfig; -use crate::data::{Data, RouteData}; +use crate::data::Data; use crate::error::UrlGenerationError; use crate::extract::FromRequest; use crate::info::ConnectionInfo; @@ -20,9 +20,9 @@ pub struct HttpRequest(pub(crate) Rc); pub(crate) struct HttpRequestInner { pub(crate) head: Message, pub(crate) path: Path, + pub(crate) app_data: Rc, rmap: Rc, config: AppConfig, - route_data: Option>, pool: &'static HttpRequestPool, } @@ -33,6 +33,7 @@ impl HttpRequest { head: Message, rmap: Rc, config: AppConfig, + app_data: Rc, pool: &'static HttpRequestPool, ) -> HttpRequest { HttpRequest(Rc::new(HttpRequestInner { @@ -40,8 +41,8 @@ impl HttpRequest { path, rmap, config, + app_data, pool, - route_data: None, })) } } @@ -195,27 +196,23 @@ impl HttpRequest { /// Get an application data stored with `App::data()` method during /// application configuration. - pub fn app_data(&self) -> Option> { - if let Some(st) = self.0.config.extensions().get::>() { + pub fn app_data(&self) -> Option<&T> { + if let Some(st) = self.0.app_data.get::>() { + Some(&st) + } else { + None + } + } + + /// Get an application data stored with `App::data()` method during + /// application configuration. + pub fn get_app_data(&self) -> Option> { + if let Some(st) = self.0.app_data.get::>() { Some(st.clone()) } else { None } } - - /// Load route data. Route data could be set during - /// route configuration with `Route::data()` method. - pub fn route_data(&self) -> Option<&RouteData> { - if let Some(ref ext) = self.0.route_data { - ext.get::>() - } else { - None - } - } - - pub(crate) fn set_route_data(&mut self, data: Option>) { - Rc::get_mut(&mut self.0).unwrap().route_data = data; - } } impl HttpMessage for HttpRequest { diff --git a/src/resource.rs b/src/resource.rs index 03c614a9d..8bafc0fcd 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -2,7 +2,7 @@ use std::cell::RefCell; use std::fmt; use std::rc::Rc; -use actix_http::{Error, Response}; +use actix_http::{Error, Extensions, Response}; use actix_service::boxed::{self, BoxedNewService, BoxedService}; use actix_service::{ apply_transform, IntoNewService, IntoTransform, NewService, Service, Transform, @@ -10,6 +10,7 @@ use actix_service::{ use futures::future::{ok, Either, FutureResult}; use futures::{Async, Future, IntoFuture, Poll}; +use crate::data::Data; use crate::dev::{insert_slash, AppService, HttpServiceFactory, ResourceDef}; use crate::extract::FromRequest; use crate::guard::Guard; @@ -48,6 +49,7 @@ pub struct Resource { rdef: String, name: Option, routes: Vec, + data: Option, guards: Vec>, default: Rc>>>, factory_ref: Rc>>, @@ -64,6 +66,7 @@ impl Resource { endpoint: ResourceEndpoint::new(fref.clone()), factory_ref: fref, guards: Vec::new(), + data: None, default: Rc::new(RefCell::new(None)), } } @@ -154,7 +157,42 @@ where /// # fn delete_handler() {} /// ``` pub fn route(mut self, route: Route) -> Self { - self.routes.push(route.finish()); + self.routes.push(route); + 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. + /// + /// ```rust + /// use actix_web::{web, App, FromRequest}; + /// + /// /// extract text data from request + /// 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) + /// )); + /// } + /// ``` + pub fn data(mut self, data: U) -> Self { + if self.data.is_none() { + self.data = Some(Extensions::new()); + } + self.data.as_mut().unwrap().insert(Data::new(data)); self } @@ -260,6 +298,7 @@ where guards: self.guards, routes: self.routes, default: self.default, + data: self.data, factory_ref: self.factory_ref, } } @@ -361,6 +400,10 @@ where if let Some(ref name) = self.name { *rdef.name_mut() = name.clone(); } + // custom app data storage + if let Some(ref mut ext) = self.data { + config.set_route_data(ext); + } config.register_service(rdef, guards, self, None) } } @@ -377,6 +420,7 @@ where fn into_new_service(self) -> T { *self.factory_ref.borrow_mut() = Some(ResourceFactory { routes: self.routes, + data: self.data.map(|data| Rc::new(data)), default: self.default, }); @@ -386,6 +430,7 @@ where pub struct ResourceFactory { routes: Vec, + data: Option>, default: Rc>>>, } @@ -410,6 +455,7 @@ impl NewService for ResourceFactory { .iter() .map(|route| CreateRouteServiceItem::Future(route.new_service(&()))) .collect(), + data: self.data.clone(), default: None, default_fut, } @@ -423,6 +469,7 @@ enum CreateRouteServiceItem { pub struct CreateResourceService { fut: Vec, + data: Option>, default: Option, default_fut: Option>>, } @@ -467,6 +514,7 @@ impl Future for CreateResourceService { .collect(); Ok(Async::Ready(ResourceService { routes, + data: self.data.clone(), default: self.default.take(), })) } else { @@ -477,6 +525,7 @@ impl Future for CreateResourceService { pub struct ResourceService { routes: Vec, + data: Option>, default: Option, } @@ -496,6 +545,9 @@ impl Service for ResourceService { fn call(&mut self, mut req: ServiceRequest) -> Self::Future { for route in self.routes.iter_mut() { if route.check(&mut req) { + if let Some(ref data) = self.data { + req.set_data_container(data.clone()); + } return route.call(req); } } diff --git a/src/route.rs b/src/route.rs index 8c97d7720..62f030c79 100644 --- a/src/route.rs +++ b/src/route.rs @@ -1,12 +1,10 @@ -use std::cell::RefCell; use std::rc::Rc; -use actix_http::{http::Method, Error, Extensions}; +use actix_http::{http::Method, Error}; use actix_service::{NewService, Service}; use futures::future::{ok, Either, FutureResult}; use futures::{Async, Future, IntoFuture, Poll}; -use crate::data::RouteData; use crate::extract::FromRequest; use crate::guard::{self, Guard}; use crate::handler::{AsyncFactory, AsyncHandler, Extract, Factory, Handler}; @@ -44,30 +42,19 @@ type BoxedRouteNewService = Box< pub struct Route { service: BoxedRouteNewService, guards: Rc>>, - data: Option, - data_ref: Rc>>>, } impl Route { /// Create new route which matches any request. pub fn new() -> Route { - let data_ref = Rc::new(RefCell::new(None)); Route { - service: Box::new(RouteNewService::new(Extract::new( - data_ref.clone(), - Handler::new(|| HttpResponse::NotFound()), - ))), + service: Box::new(RouteNewService::new(Extract::new(Handler::new(|| { + HttpResponse::NotFound() + })))), guards: Rc::new(Vec::new()), - data: None, - data_ref, } } - pub(crate) fn finish(mut self) -> Self { - *self.data_ref.borrow_mut() = self.data.take().map(Rc::new); - self - } - pub(crate) fn take_guards(&mut self) -> Vec> { std::mem::replace(Rc::get_mut(&mut self.guards).unwrap(), Vec::new()) } @@ -239,10 +226,8 @@ impl Route { T: FromRequest + 'static, R: Responder + 'static, { - self.service = Box::new(RouteNewService::new(Extract::new( - self.data_ref.clone(), - Handler::new(handler), - ))); + self.service = + Box::new(RouteNewService::new(Extract::new(Handler::new(handler)))); self } @@ -281,42 +266,9 @@ impl Route { R::Item: Responder, R::Error: Into, { - self.service = Box::new(RouteNewService::new(Extract::new( - self.data_ref.clone(), - AsyncHandler::new(handler), - ))); - self - } - - /// Provide route specific data. This method allows to add extractor - /// configuration or specific state available via `RouteData` extractor. - /// - /// ```rust - /// use actix_web::{web, App, FromRequest}; - /// - /// /// extract text data from request - /// fn index(body: String) -> String { - /// format!("Body {}!", body) - /// } - /// - /// fn main() { - /// let app = App::new().service( - /// web::resource("/index.html").route( - /// web::get() - /// // limit size of the payload - /// .data(String::configure(|cfg| { - /// cfg.limit(4096) - /// })) - /// // register handler - /// .to(index) - /// )); - /// } - /// ``` - pub fn data(mut self, data: T) -> Self { - if self.data.is_none() { - self.data = Some(Extensions::new()); - } - self.data.as_mut().unwrap().insert(RouteData::new(data)); + self.service = Box::new(RouteNewService::new(Extract::new(AsyncHandler::new( + handler, + )))); self } } diff --git a/src/scope.rs b/src/scope.rs index d048d1437..ada533341 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -322,6 +322,7 @@ where default: self.default.clone(), services: Rc::new( cfg.into_services() + .1 .into_iter() .map(|(mut rdef, srv, guards, nested)| { rmap.add(&mut rdef, nested); diff --git a/src/service.rs b/src/service.rs index 7fbbf013d..f35ea89f2 100644 --- a/src/service.rs +++ b/src/service.rs @@ -1,4 +1,5 @@ use std::cell::{Ref, RefMut}; +use std::rc::Rc; use std::{fmt, net}; use actix_http::body::{Body, MessageBody, ResponseBody}; @@ -180,12 +181,18 @@ impl ServiceRequest { /// Get an application data stored with `App::data()` method during /// application configuration. pub fn app_data(&self) -> Option> { - if let Some(st) = self.req.app_config().extensions().get::>() { + if let Some(st) = self.req.0.app_data.get::>() { Some(st.clone()) } else { None } } + + #[doc(hidden)] + /// Set new app data container + pub fn set_data_container(&mut self, extensions: Rc) { + Rc::get_mut(&mut self.req.0).unwrap().app_data = extensions; + } } impl Resource for ServiceRequest { diff --git a/src/test.rs b/src/test.rs index 7d3321180..66b380e83 100644 --- a/src/test.rs +++ b/src/test.rs @@ -2,11 +2,10 @@ use std::cell::RefCell; use std::rc::Rc; -use actix_http::cookie::Cookie; use actix_http::http::header::{Header, HeaderName, IntoHeaderValue}; use actix_http::http::{HttpTryFrom, Method, StatusCode, Uri, Version}; use actix_http::test::TestRequest as HttpTestRequest; -use actix_http::{Extensions, Request}; +use actix_http::{cookie::Cookie, Extensions, Request}; use actix_router::{Path, ResourceDef, Url}; use actix_rt::Runtime; use actix_server_config::ServerConfig; @@ -20,7 +19,7 @@ use serde_json; pub use actix_http::test::TestBuffer; use crate::config::{AppConfig, AppConfigInner}; -use crate::data::{Data, RouteData}; +use crate::data::Data; use crate::dev::{Body, MessageBody, Payload}; use crate::request::HttpRequestPool; use crate::rmap::ResourceMap; @@ -363,8 +362,8 @@ pub struct TestRequest { req: HttpTestRequest, rmap: ResourceMap, config: AppConfigInner, - route_data: Extensions, path: Path, + app_data: Extensions, } impl Default for TestRequest { @@ -373,8 +372,8 @@ impl Default for TestRequest { req: HttpTestRequest::default(), rmap: ResourceMap::new(ResourceDef::new("")), config: AppConfigInner::default(), - route_data: Extensions::new(), path: Path::new(Url::new(Uri::default())), + app_data: Extensions::new(), } } } @@ -479,15 +478,8 @@ impl TestRequest { /// Set application data. This is equivalent of `App::data()` method /// for testing purpose. - pub fn app_data(self, data: T) -> Self { - self.config.extensions.borrow_mut().insert(Data::new(data)); - self - } - - /// Set route data. This is equivalent of `Route::data()` method - /// for testing purpose. - pub fn route_data(mut self, data: T) -> Self { - self.route_data.insert(RouteData::new(data)); + pub fn data(mut self, data: T) -> Self { + self.app_data.insert(Data::new(data)); self } @@ -513,6 +505,7 @@ impl TestRequest { head, Rc::new(self.rmap), AppConfig::new(self.config), + Rc::new(self.app_data), HttpRequestPool::create(), ); @@ -529,15 +522,14 @@ impl TestRequest { let (head, _) = self.req.finish().into_parts(); self.path.get_mut().update(&head.uri); - let mut req = HttpRequest::new( + HttpRequest::new( self.path, head, Rc::new(self.rmap), AppConfig::new(self.config), + Rc::new(self.app_data), HttpRequestPool::create(), - ); - req.set_route_data(Some(Rc::new(self.route_data))); - req + ) } /// Complete request creation and generate `HttpRequest` and `Payload` instances @@ -545,14 +537,15 @@ impl TestRequest { let (head, payload) = self.req.finish().into_parts(); self.path.get_mut().update(&head.uri); - let mut req = HttpRequest::new( + let req = HttpRequest::new( self.path, head, Rc::new(self.rmap), AppConfig::new(self.config), + Rc::new(self.app_data), HttpRequestPool::create(), ); - req.set_route_data(Some(Rc::new(self.route_data))); + (req, payload) } } @@ -571,15 +564,20 @@ mod tests { .version(Version::HTTP_2) .set(header::Date(SystemTime::now().into())) .param("test", "123") - .app_data(10u32) + .data(10u32) .to_http_request(); assert!(req.headers().contains_key(header::CONTENT_TYPE)); assert!(req.headers().contains_key(header::DATE)); assert_eq!(&req.match_info()["test"], "123"); assert_eq!(req.version(), Version::HTTP_2); - let data = req.app_data::().unwrap(); + let data = req.get_app_data::().unwrap(); + assert!(req.get_app_data::().is_none()); assert_eq!(*data, 10); assert_eq!(*data.get_ref(), 10); + + assert!(req.app_data::().is_none()); + let data = req.app_data::().unwrap(); + assert_eq!(*data, 10); } #[test] diff --git a/src/types/form.rs b/src/types/form.rs index e8f78c496..0bc6a0303 100644 --- a/src/types/form.rs +++ b/src/types/form.rs @@ -81,7 +81,7 @@ where fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { let req2 = req.clone(); let (limit, err) = req - .route_data::() + .app_data::() .map(|c| (c.limit, c.ehandler.clone())) .unwrap_or((16384, None)); @@ -132,12 +132,11 @@ impl fmt::Display for Form { /// fn main() { /// let app = App::new().service( /// web::resource("/index.html") -/// .route(web::get() -/// // change `Form` extractor configuration -/// .data( -/// web::Form::::configure(|cfg| cfg.limit(4097)) -/// ) -/// .to(index)) +/// // change `Form` extractor configuration +/// .data( +/// web::Form::::configure(|cfg| cfg.limit(4097)) +/// ) +/// .route(web::get().to(index)) /// ); /// } /// ``` diff --git a/src/types/json.rs b/src/types/json.rs index 3543975ae..73614d87e 100644 --- a/src/types/json.rs +++ b/src/types/json.rs @@ -176,7 +176,7 @@ where fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future { let req2 = req.clone(); let (limit, err) = req - .route_data::() + .app_data::() .map(|c| (c.limit, c.ehandler.clone())) .unwrap_or((32768, None)); @@ -220,17 +220,16 @@ where /// /// fn main() { /// let app = App::new().service( -/// web::resource("/index.html").route( -/// web::post().data( -/// // change json extractor configuration -/// web::Json::::configure(|cfg| { -/// cfg.limit(4096) -/// .error_handler(|err, req| { // <- create custom error response -/// error::InternalError::from_response( -/// err, HttpResponse::Conflict().finish()).into() -/// }) -/// })) -/// .to(index)) +/// web::resource("/index.html").data( +/// // change json extractor configuration +/// web::Json::::configure(|cfg| { +/// cfg.limit(4096) +/// .error_handler(|err, req| { // <- create custom error response +/// error::InternalError::from_response( +/// err, HttpResponse::Conflict().finish()).into() +/// }) +/// })) +/// .route(web::post().to(index)) /// ); /// } /// ``` @@ -431,7 +430,7 @@ mod tests { header::HeaderValue::from_static("16"), ) .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) - .route_data(JsonConfig::default().limit(10).error_handler(|err, _| { + .data(JsonConfig::default().limit(10).error_handler(|err, _| { let msg = MyObject { name: "invalid request".to_string(), }; @@ -483,7 +482,7 @@ mod tests { header::HeaderValue::from_static("16"), ) .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) - .route_data(JsonConfig::default().limit(10)) + .data(JsonConfig::default().limit(10)) .to_http_parts(); let s = block_on(Json::::from_request(&req, &mut pl)); @@ -500,7 +499,7 @@ mod tests { header::HeaderValue::from_static("16"), ) .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) - .route_data( + .data( JsonConfig::default() .limit(10) .error_handler(|_, _| JsonPayloadError::ContentType.into()), diff --git a/src/types/payload.rs b/src/types/payload.rs index ca4b5de6b..8e4dd7030 100644 --- a/src/types/payload.rs +++ b/src/types/payload.rs @@ -130,7 +130,7 @@ impl FromRequest for Bytes { #[inline] fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future { let mut tmp; - let cfg = if let Some(cfg) = req.route_data::() { + let cfg = if let Some(cfg) = req.app_data::() { cfg } else { tmp = PayloadConfig::default(); @@ -167,12 +167,11 @@ impl FromRequest for Bytes { /// /// fn main() { /// let app = App::new().service( -/// web::resource("/index.html").route( -/// web::get() -/// .data(String::configure(|cfg| { // <- limit size of the payload -/// cfg.limit(4096) -/// })) -/// .to(index)) // <- register handler with extractor params +/// web::resource("/index.html") +/// .data(String::configure(|cfg| { // <- limit size of the payload +/// cfg.limit(4096) +/// })) +/// .route(web::get().to(index)) // <- register handler with extractor params /// ); /// } /// ``` @@ -185,7 +184,7 @@ impl FromRequest for String { #[inline] fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future { let mut tmp; - let cfg = if let Some(cfg) = req.route_data::() { + let cfg = if let Some(cfg) = req.app_data::() { cfg } else { tmp = PayloadConfig::default(); diff --git a/src/web.rs b/src/web.rs index 1ecebe77e..5669a1e86 100644 --- a/src/web.rs +++ b/src/web.rs @@ -15,7 +15,7 @@ use crate::scope::Scope; use crate::service::WebService; pub use crate::config::ServiceConfig; -pub use crate::data::{Data, RouteData}; +pub use crate::data::Data; pub use crate::request::HttpRequest; pub use crate::types::*;