From c877840c07d657fd7379d4bc8381f7c38c29edd2 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Fri, 20 Dec 2019 17:13:09 +0600 Subject: [PATCH] rename App::register_data to App::app_data and HttpRequest::app_data returns Option<&T> instead of Option<&Data> --- CHANGES.md | 4 ++++ MIGRATION.md | 5 +++++ src/app.rs | 33 +++++++++++++++++++++++++++++---- src/app_service.rs | 10 +++++++++- src/data.rs | 12 ++++++------ src/request.rs | 21 +++++---------------- src/resource.rs | 13 +++++-------- src/scope.rs | 32 +++++++++++++------------------- src/test.rs | 19 +++++++++++++------ src/types/form.rs | 2 +- src/types/json.rs | 35 ++++++++++++++++++----------------- src/types/path.rs | 4 ++-- src/types/payload.rs | 2 +- src/types/query.rs | 4 ++-- tests/test_server.rs | 2 +- 15 files changed, 114 insertions(+), 84 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index e669ae38c..54c5bb06c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,6 +8,10 @@ * Allow to set `peer_addr` for TestRequest #1074 +* Rename `App::register_data()` to `App::app_data()` + +* `HttpRequest::app_data()` returns `Option<&T>` instead of `Option<&Data>` + ### Fixed * Fix `AppConfig::secure()` is always false. #1202 diff --git a/MIGRATION.md b/MIGRATION.md index dd3a1b043..357a4e4ca 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1,5 +1,10 @@ ## 2.0.0 +* `App::register_data()` renamed to `App::app_data()` and accepts any type `T: 'static`. + Stored data is available via `HttpRequest::app_data()` method at runtime. + +* Extractor configuration must be registered with `App::app_data()` instead of `App::data()` + * Sync handlers has been removed. `.to_async()` method has been renamed to `.to()` replace `fn` with `async fn` to convert sync handler to async diff --git a/src/app.rs b/src/app.rs index ccf2e88e0..962ff4b47 100644 --- a/src/app.rs +++ b/src/app.rs @@ -5,6 +5,7 @@ use std::marker::PhantomData; use std::rc::Rc; use actix_http::body::{Body, MessageBody}; +use actix_http::Extensions; use actix_service::boxed::{self, BoxServiceFactory}; use actix_service::{ apply, apply_fn_factory, IntoServiceFactory, ServiceFactory, Transform, @@ -37,6 +38,7 @@ pub struct App { data: Vec>, data_factories: Vec, external: Vec, + extensions: Extensions, _t: PhantomData, } @@ -52,6 +54,7 @@ impl App { default: None, factory_ref: fref, external: Vec::new(), + extensions: Extensions::new(), _t: PhantomData, } } @@ -133,10 +136,15 @@ where self } - /// Set application data. Application data could be accessed - /// by using `Data` extractor where `T` is data type. - pub fn register_data(mut self, data: Data) -> Self { - self.data.push(Box::new(data)); + /// 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 } @@ -370,6 +378,7 @@ where default: self.default, factory_ref: self.factory_ref, external: self.external, + extensions: self.extensions, _t: PhantomData, } } @@ -431,6 +440,7 @@ where default: self.default, factory_ref: self.factory_ref, external: self.external, + extensions: self.extensions, _t: PhantomData, } } @@ -456,6 +466,7 @@ where external: RefCell::new(self.external), default: self.default, factory_ref: self.factory_ref, + extensions: RefCell::new(Some(self.extensions)), } } } @@ -539,6 +550,20 @@ mod tests { assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } + #[actix_rt::test] + async fn test_extension() { + let mut srv = init_service(App::new().app_data(10usize).service( + web::resource("/").to(|req: HttpRequest| { + assert_eq!(*req.app_data::().unwrap(), 10); + HttpResponse::Ok() + }), + )) + .await; + let req = TestRequest::default().to_request(); + let resp = srv.call(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } + #[actix_rt::test] async fn test_wrap() { let mut srv = init_service( diff --git a/src/app_service.rs b/src/app_service.rs index 28e245228..ccfefbc68 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -39,6 +39,7 @@ where >, { pub(crate) endpoint: T, + pub(crate) extensions: RefCell>, pub(crate) data: Rc>>, pub(crate) data_factories: Rc>, pub(crate) services: Rc>>>, @@ -114,6 +115,12 @@ where data: self.data.clone(), data_factories: Vec::new(), data_factories_fut: self.data_factories.iter().map(|f| f()).collect(), + extensions: Some( + self.extensions + .borrow_mut() + .take() + .unwrap_or_else(Extensions::new), + ), config, rmap, _t: PhantomData, @@ -134,6 +141,7 @@ where data: Rc>>, data_factories: Vec>, data_factories_fut: Vec, ()>>>, + extensions: Option, _t: PhantomData, } @@ -172,7 +180,7 @@ where if this.endpoint.is_some() && this.data_factories_fut.is_empty() { // create app data container - let mut data = Extensions::new(); + let mut data = this.extensions.take().unwrap(); for f in this.data.iter() { f.create(&mut data); } diff --git a/src/data.rs b/src/data.rs index e8928188f..eaa2db388 100644 --- a/src/data.rs +++ b/src/data.rs @@ -56,7 +56,7 @@ pub(crate) trait DataFactory { /// /// let app = App::new() /// // Store `MyData` in application storage. -/// .register_data(data.clone()) +/// .app_data(data.clone()) /// .service( /// web::resource("/index.html").route( /// web::get().to(index))); @@ -107,8 +107,8 @@ impl FromRequest for Data { #[inline] fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { - if let Some(st) = req.get_app_data::() { - ok(st) + if let Some(st) = req.app_data::>() { + ok(st.clone()) } else { log::debug!( "Failed to construct App-level Data extractor. \ @@ -165,9 +165,9 @@ mod tests { } #[actix_rt::test] - async fn test_register_data_extractor() { + async fn test_app_data_extractor() { let mut srv = - init_service(App::new().register_data(Data::new(10usize)).service( + init_service(App::new().app_data(Data::new(10usize)).service( web::resource("/").to(|_: web::Data| HttpResponse::Ok()), )) .await; @@ -177,7 +177,7 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); let mut srv = - init_service(App::new().register_data(Data::new(10u32)).service( + init_service(App::new().app_data(Data::new(10u32)).service( web::resource("/").to(|_: web::Data| HttpResponse::Ok()), )) .await; diff --git a/src/request.rs b/src/request.rs index 46b8fe387..b51438950 100644 --- a/src/request.rs +++ b/src/request.rs @@ -8,7 +8,6 @@ use actix_router::{Path, Url}; use futures::future::{ok, Ready}; use crate::config::AppConfig; -use crate::data::Data; use crate::error::UrlGenerationError; use crate::extract::FromRequest; use crate::info::ConnectionInfo; @@ -207,25 +206,15 @@ impl HttpRequest { &self.0.config } - /// Get an application data stored with `App::data()` method during + /// Get an application data stored with `App::extension()` method during /// application configuration. pub fn app_data(&self) -> Option<&T> { - if let Some(st) = self.0.app_data.get::>() { + 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 - } - } } impl HttpMessage for HttpRequest { @@ -467,8 +456,8 @@ mod tests { } #[actix_rt::test] - async fn test_app_data() { - let mut srv = init_service(App::new().data(10usize).service( + async fn test_data() { + let mut srv = init_service(App::new().app_data(10usize).service( web::resource("/").to(|req: HttpRequest| { if req.app_data::().is_some() { HttpResponse::Ok() @@ -483,7 +472,7 @@ mod tests { let resp = call_service(&mut srv, req).await; assert_eq!(resp.status(), StatusCode::OK); - let mut srv = init_service(App::new().data(10u32).service( + let mut srv = init_service(App::new().app_data(10u32).service( web::resource("/").to(|req: HttpRequest| { if req.app_data::().is_some() { HttpResponse::Ok() diff --git a/src/resource.rs b/src/resource.rs index 41d663d3d..8fc5973e0 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -192,16 +192,13 @@ where /// } /// ``` pub fn data(self, data: U) -> Self { - self.register_data(Data::new(data)) + self.app_data(Data::new(data)) } /// Set or override application data. /// - /// This method has the same effect as [`Resource::data`](#method.data), - /// except that instead of taking a value of some type `T`, it expects a - /// value of type `Data`. Use a `Data` extractor to retrieve its - /// value. - pub fn register_data(mut self, data: Data) -> Self { + /// This method overrides data stored with [`App::app_data()`](#method.app_data) + pub fn app_data(mut self, data: U) -> Self { if self.data.is_none() { self.data = Some(Extensions::new()); } @@ -754,11 +751,11 @@ mod tests { App::new() .data(1.0f64) .data(1usize) - .register_data(web::Data::new('-')) + .app_data(web::Data::new('-')) .service( web::resource("/test") .data(10usize) - .register_data(web::Data::new('*')) + .app_data(web::Data::new('*')) .guard(guard::Get()) .to( |data1: web::Data, diff --git a/src/scope.rs b/src/scope.rs index 26ace6892..5a965292b 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -148,15 +148,13 @@ where /// } /// ``` pub fn data(self, data: U) -> Self { - self.register_data(Data::new(data)) + self.app_data(Data::new(data)) } /// Set or override application data. /// - /// This method has the same effect as [`Scope::data`](#method.data), except - /// that instead of taking a value of some type `T`, it expects a value of - /// type `Data`. Use a `Data` extractor to retrieve its value. - pub fn register_data(mut self, data: Data) -> Self { + /// This method overrides data stored with [`App::app_data()`](#method.app_data) + pub fn app_data(mut self, data: U) -> Self { if self.data.is_none() { self.data = Some(Extensions::new()); } @@ -1122,21 +1120,17 @@ mod tests { } #[actix_rt::test] - async fn test_override_register_data() { - let mut srv = init_service( - App::new().register_data(web::Data::new(1usize)).service( - web::scope("app") - .register_data(web::Data::new(10usize)) - .route( - "/t", - web::get().to(|data: web::Data| { - assert_eq!(*data, 10); - let _ = data.clone(); - HttpResponse::Ok() - }), - ), + async fn test_override_app_data() { + let mut srv = init_service(App::new().app_data(web::Data::new(1usize)).service( + web::scope("app").app_data(web::Data::new(10usize)).route( + "/t", + web::get().to(|data: web::Data| { + assert_eq!(*data, 10); + let _ = data.clone(); + HttpResponse::Ok() + }), ), - ) + )) .await; let req = TestRequest::with_uri("/app/t").to_request(); diff --git a/src/test.rs b/src/test.rs index 4113a7c15..a10490fd5 100644 --- a/src/test.rs +++ b/src/test.rs @@ -451,6 +451,13 @@ impl TestRequest { self } + /// Set application data. This is equivalent of `App::app_data()` method + /// for testing purpose. + pub fn app_data(mut self, data: T) -> Self { + self.app_data.insert(data); + self + } + #[cfg(test)] /// Set request config pub(crate) fn rmap(mut self, rmap: ResourceMap) -> Self { @@ -964,6 +971,7 @@ mod tests { .set(header::Date(SystemTime::now().into())) .param("test", "123") .data(10u32) + .app_data(20u64) .peer_addr("127.0.0.1:8081".parse().unwrap()) .to_http_request(); assert!(req.headers().contains_key(header::CONTENT_TYPE)); @@ -974,14 +982,13 @@ mod tests { ); assert_eq!(&req.match_info()["test"], "123"); assert_eq!(req.version(), Version::HTTP_2); - let data = req.get_app_data::().unwrap(); - assert!(req.get_app_data::().is_none()); - assert_eq!(*data, 10); + let data = req.app_data::>().unwrap(); + assert!(req.app_data::>().is_none()); assert_eq!(*data.get_ref(), 10); - assert!(req.app_data::().is_none()); - let data = req.app_data::().unwrap(); - assert_eq!(*data, 10); + assert!(req.app_data::().is_none()); + let data = req.app_data::().unwrap(); + assert_eq!(*data, 20); } #[actix_rt::test] diff --git a/src/types/form.rs b/src/types/form.rs index 756d5fcc9..333ecbd64 100644 --- a/src/types/form.rs +++ b/src/types/form.rs @@ -190,7 +190,7 @@ impl Responder for Form { /// let app = App::new().service( /// web::resource("/index.html") /// // change `Form` extractor configuration -/// .data( +/// .app_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 03c4a2db6..adb425cd9 100644 --- a/src/types/json.rs +++ b/src/types/json.rs @@ -224,17 +224,18 @@ where /// /// fn main() { /// let app = App::new().service( -/// web::resource("/index.html").data( -/// // change json extractor configuration -/// web::Json::::configure(|cfg| { -/// cfg.limit(4096) -/// .content_type(|mime| { // <- accept text/plain content type -/// mime.type_() == mime::TEXT && mime.subtype() == mime::PLAIN -/// }) -/// .error_handler(|err, req| { // <- create custom error response -/// error::InternalError::from_response( -/// err, HttpResponse::Conflict().finish()).into() -/// }) +/// web::resource("/index.html") +/// .app_data( +/// // change json extractor configuration +/// web::Json::::configure(|cfg| { +/// cfg.limit(4096) +/// .content_type(|mime| { // <- accept text/plain content type +/// mime.type_() == mime::TEXT && mime.subtype() == mime::PLAIN +/// }) +/// .error_handler(|err, req| { // <- create custom error response +/// error::InternalError::from_response( +/// err, HttpResponse::Conflict().finish()).into() +/// }) /// })) /// .route(web::post().to(index)) /// ); @@ -462,7 +463,7 @@ mod tests { header::HeaderValue::from_static("16"), ) .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) - .data(JsonConfig::default().limit(10).error_handler(|err, _| { + .app_data(JsonConfig::default().limit(10).error_handler(|err, _| { let msg = MyObject { name: "invalid request".to_string(), }; @@ -514,7 +515,7 @@ mod tests { header::HeaderValue::from_static("16"), ) .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) - .data(JsonConfig::default().limit(10)) + .app_data(JsonConfig::default().limit(10)) .to_http_parts(); let s = Json::::from_request(&req, &mut pl).await; @@ -531,7 +532,7 @@ mod tests { header::HeaderValue::from_static("16"), ) .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) - .data( + .app_data( JsonConfig::default() .limit(10) .error_handler(|_, _| JsonPayloadError::ContentType.into()), @@ -604,7 +605,7 @@ mod tests { header::HeaderValue::from_static("16"), ) .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) - .data(JsonConfig::default().limit(4096)) + .app_data(JsonConfig::default().limit(4096)) .to_http_parts(); let s = Json::::from_request(&req, &mut pl).await; @@ -622,7 +623,7 @@ mod tests { header::HeaderValue::from_static("16"), ) .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) - .data(JsonConfig::default().content_type(|mime: mime::Mime| { + .app_data(JsonConfig::default().content_type(|mime: mime::Mime| { mime.type_() == mime::TEXT && mime.subtype() == mime::PLAIN })) .to_http_parts(); @@ -642,7 +643,7 @@ mod tests { header::HeaderValue::from_static("16"), ) .set_payload(Bytes::from_static(b"{\"name\": \"test\"}")) - .data(JsonConfig::default().content_type(|mime: mime::Mime| { + .app_data(JsonConfig::default().content_type(|mime: mime::Mime| { mime.type_() == mime::TEXT && mime.subtype() == mime::PLAIN })) .to_http_parts(); diff --git a/src/types/path.rs b/src/types/path.rs index d1a5f1fb9..9af5a0b9a 100644 --- a/src/types/path.rs +++ b/src/types/path.rs @@ -212,7 +212,7 @@ where /// fn main() { /// let app = App::new().service( /// web::resource("/messages/{folder}") -/// .data(PathConfig::default().error_handler(|err, req| { +/// .app_data(PathConfig::default().error_handler(|err, req| { /// error::InternalError::from_response( /// err, /// HttpResponse::Conflict().finish(), @@ -358,7 +358,7 @@ mod tests { #[actix_rt::test] async fn test_custom_err_handler() { let (req, mut pl) = TestRequest::with_uri("/name/user1/") - .data(PathConfig::default().error_handler(|err, _| { + .app_data(PathConfig::default().error_handler(|err, _| { error::InternalError::from_response( err, HttpResponse::Conflict().finish(), diff --git a/src/types/payload.rs b/src/types/payload.rs index 7cb714a87..dd7a84f32 100644 --- a/src/types/payload.rs +++ b/src/types/payload.rs @@ -176,7 +176,7 @@ impl FromRequest for Bytes { /// fn main() { /// let app = App::new().service( /// web::resource("/index.html") -/// .data(String::configure(|cfg| { // <- limit size of the payload +/// .app_data(String::configure(|cfg| { // <- limit size of the payload /// cfg.limit(4096) /// })) /// .route(web::get().to(index)) // <- register handler with extractor params diff --git a/src/types/query.rs b/src/types/query.rs index 9d62f31c6..696e10b94 100644 --- a/src/types/query.rs +++ b/src/types/query.rs @@ -185,7 +185,7 @@ where /// /// fn main() { /// let app = App::new().service( -/// web::resource("/index.html").data( +/// web::resource("/index.html").app_data( /// // change query extractor configuration /// web::Query::::configure(|cfg| { /// cfg.error_handler(|err, req| { // <- create custom error response @@ -273,7 +273,7 @@ mod tests { #[actix_rt::test] async fn test_custom_error_responder() { let req = TestRequest::with_uri("/name/user1/") - .data(QueryConfig::default().error_handler(|e, _| { + .app_data(QueryConfig::default().error_handler(|e, _| { let resp = HttpResponse::UnprocessableEntity().finish(); InternalError::from_response(e, resp).into() })) diff --git a/tests/test_server.rs b/tests/test_server.rs index 5019157e2..1916b372c 100644 --- a/tests/test_server.rs +++ b/tests/test_server.rs @@ -675,7 +675,7 @@ async fn test_brotli_encoding_large() { let srv = test::start_with(test::config().h1(), || { App::new().service( web::resource("/") - .data(web::PayloadConfig::new(320_000)) + .app_data(web::PayloadConfig::new(320_000)) .route(web::to(move |body: Bytes| { HttpResponse::Ok().streaming(TestBody::new(body, 10240)) })),