Skip to content

Commit

Permalink
rename App::register_data to App::app_data and HttpRequest::app_data …
Browse files Browse the repository at this point in the history
…returns Option<&T> instead of Option<&Data<T>>
  • Loading branch information
fafhrd91 committed Dec 20, 2019
1 parent 20248da commit c877840
Show file tree
Hide file tree
Showing 15 changed files with 114 additions and 84 deletions.
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@

* Allow to set `peer_addr` for TestRequest #1074

* Rename `App::register_data()` to `App::app_data()`

* `HttpRequest::app_data<T>()` returns `Option<&T>` instead of `Option<&Data<T>>`

### Fixed

* Fix `AppConfig::secure()` is always false. #1202
Expand Down
5 changes: 5 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
33 changes: 29 additions & 4 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -37,6 +38,7 @@ pub struct App<T, B> {
data: Vec<Box<dyn DataFactory>>,
data_factories: Vec<FnDataFactory>,
external: Vec<ResourceDef>,
extensions: Extensions,
_t: PhantomData<B>,
}

Expand All @@ -52,6 +54,7 @@ impl App<AppEntry, Body> {
default: None,
factory_ref: fref,
external: Vec::new(),
extensions: Extensions::new(),
_t: PhantomData,
}
}
Expand Down Expand Up @@ -133,10 +136,15 @@ where
self
}

/// Set application data. Application data could be accessed
/// by using `Data<T>` extractor where `T` is data type.
pub fn register_data<U: 'static>(mut self, data: Data<U>) -> 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<T>` as well, in that case
/// data could be accessed by using `Data<T>` extractor.
pub fn app_data<U: 'static>(mut self, ext: U) -> Self {
self.extensions.insert(ext);
self
}

Expand Down Expand Up @@ -370,6 +378,7 @@ where
default: self.default,
factory_ref: self.factory_ref,
external: self.external,
extensions: self.extensions,
_t: PhantomData,
}
}
Expand Down Expand Up @@ -431,6 +440,7 @@ where
default: self.default,
factory_ref: self.factory_ref,
external: self.external,
extensions: self.extensions,
_t: PhantomData,
}
}
Expand All @@ -456,6 +466,7 @@ where
external: RefCell::new(self.external),
default: self.default,
factory_ref: self.factory_ref,
extensions: RefCell::new(Some(self.extensions)),
}
}
}
Expand Down Expand Up @@ -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::<usize>().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(
Expand Down
10 changes: 9 additions & 1 deletion src/app_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ where
>,
{
pub(crate) endpoint: T,
pub(crate) extensions: RefCell<Option<Extensions>>,
pub(crate) data: Rc<Vec<Box<dyn DataFactory>>>,
pub(crate) data_factories: Rc<Vec<FnDataFactory>>,
pub(crate) services: Rc<RefCell<Vec<Box<dyn AppServiceFactory>>>>,
Expand Down Expand Up @@ -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,
Expand All @@ -134,6 +141,7 @@ where
data: Rc<Vec<Box<dyn DataFactory>>>,
data_factories: Vec<Box<dyn DataFactory>>,
data_factories_fut: Vec<LocalBoxFuture<'static, Result<Box<dyn DataFactory>, ()>>>,
extensions: Option<Extensions>,
_t: PhantomData<B>,
}

Expand Down Expand Up @@ -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);
}
Expand Down
12 changes: 6 additions & 6 deletions src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down Expand Up @@ -107,8 +107,8 @@ impl<T: 'static> FromRequest for Data<T> {

#[inline]
fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future {
if let Some(st) = req.get_app_data::<T>() {
ok(st)
if let Some(st) = req.app_data::<Data<T>>() {
ok(st.clone())
} else {
log::debug!(
"Failed to construct App-level Data extractor. \
Expand Down Expand Up @@ -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<usize>| HttpResponse::Ok()),
))
.await;
Expand All @@ -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<usize>| HttpResponse::Ok()),
))
.await;
Expand Down
21 changes: 5 additions & 16 deletions src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<T: 'static>(&self) -> Option<&T> {
if let Some(st) = self.0.app_data.get::<Data<T>>() {
if let Some(st) = self.0.app_data.get::<T>() {
Some(&st)
} else {
None
}
}

/// Get an application data stored with `App::data()` method during
/// application configuration.
pub fn get_app_data<T: 'static>(&self) -> Option<Data<T>> {
if let Some(st) = self.0.app_data.get::<Data<T>>() {
Some(st.clone())
} else {
None
}
}
}

impl HttpMessage for HttpRequest {
Expand Down Expand Up @@ -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::<usize>().is_some() {
HttpResponse::Ok()
Expand All @@ -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::<usize>().is_some() {
HttpResponse::Ok()
Expand Down
13 changes: 5 additions & 8 deletions src/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,13 @@ where
/// }
/// ```
pub fn data<U: 'static>(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<T>`. Use a `Data<T>` extractor to retrieve its
/// value.
pub fn register_data<U: 'static>(mut self, data: Data<U>) -> Self {
/// This method overrides data stored with [`App::app_data()`](#method.app_data)
pub fn app_data<U: 'static>(mut self, data: U) -> Self {
if self.data.is_none() {
self.data = Some(Extensions::new());
}
Expand Down Expand Up @@ -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<usize>,
Expand Down
32 changes: 13 additions & 19 deletions src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,13 @@ where
/// }
/// ```
pub fn data<U: 'static>(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<T>`. Use a `Data<T>` extractor to retrieve its value.
pub fn register_data<U: 'static>(mut self, data: Data<U>) -> Self {
/// This method overrides data stored with [`App::app_data()`](#method.app_data)
pub fn app_data<U: 'static>(mut self, data: U) -> Self {
if self.data.is_none() {
self.data = Some(Extensions::new());
}
Expand Down Expand Up @@ -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<usize>| {
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<usize>| {
assert_eq!(*data, 10);
let _ = data.clone();
HttpResponse::Ok()
}),
),
)
))
.await;

let req = TestRequest::with_uri("/app/t").to_request();
Expand Down
19 changes: 13 additions & 6 deletions src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: 'static>(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 {
Expand Down Expand Up @@ -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));
Expand All @@ -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::<u32>().unwrap();
assert!(req.get_app_data::<u64>().is_none());
assert_eq!(*data, 10);
let data = req.app_data::<Data<u32>>().unwrap();
assert!(req.app_data::<Data<u64>>().is_none());
assert_eq!(*data.get_ref(), 10);

assert!(req.app_data::<u64>().is_none());
let data = req.app_data::<u32>().unwrap();
assert_eq!(*data, 10);
assert!(req.app_data::<u32>().is_none());
let data = req.app_data::<u64>().unwrap();
assert_eq!(*data, 20);
}

#[actix_rt::test]
Expand Down
2 changes: 1 addition & 1 deletion src/types/form.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl<T: Serialize> Responder for Form<T> {
/// let app = App::new().service(
/// web::resource("/index.html")
/// // change `Form` extractor configuration
/// .data(
/// .app_data(
/// web::Form::<FormData>::configure(|cfg| cfg.limit(4097))
/// )
/// .route(web::get().to(index))
Expand Down
Loading

0 comments on commit c877840

Please sign in to comment.