diff --git a/CHANGES.md b/CHANGES.md index 2ed9c20b7..97b7ca63a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,16 +1,24 @@ # Changes +## [Unreleased] + +### Changed + +* Resources and Scopes can now access non-overridden data types set on App (or containing scopes) when setting their own data. [#1486] + ## [3.0.0-alpha.2] - 2020-05-08 ### Changed * `{Resource,Scope}::default_service(f)` handlers now support app data extraction. [#1452] * Implement `std::error::Error` for our custom errors [#1422] -* NormalizePath middleware now appends trailing / so that routes of form /example/ respond to /example requests. +* NormalizePath middleware now appends trailing / so that routes of form /example/ respond to /example requests. [#1433] * Remove the `failure` feature and support. [#1422]: https://github.com/actix/actix-web/pull/1422 +[#1433]: https://github.com/actix/actix-web/pull/1433 [#1452]: https://github.com/actix/actix-web/pull/1452 +[#1486]: https://github.com/actix/actix-web/pull/1486 ## [3.0.0-alpha.1] - 2020-03-11 diff --git a/Cargo.toml b/Cargo.toml index 823893962..b24cc89d2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -101,6 +101,7 @@ time = { version = "0.2.7", default-features = false, features = ["std"] } url = "2.1" open-ssl = { version="0.10", package = "openssl", optional = true } rust-tls = { version = "0.17.0", package = "rustls", optional = true } +tinyvec = { version = "0.3", features = ["alloc"] } [dev-dependencies] actix = "0.10.0-alpha.1" diff --git a/src/app_service.rs b/src/app_service.rs index 808592e58..2d64bed3e 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -245,7 +245,7 @@ where inner.path.reset(); inner.head = head; inner.payload = payload; - inner.app_data = self.data.clone(); + inner.app_data.push(self.data.clone()); req } else { HttpRequest::new( diff --git a/src/request.rs b/src/request.rs index 51a1c54ff..72fea1faf 100644 --- a/src/request.rs +++ b/src/request.rs @@ -6,6 +6,7 @@ use actix_http::http::{HeaderMap, Method, Uri, Version}; use actix_http::{Error, Extensions, HttpMessage, Message, Payload, RequestHead}; use actix_router::{Path, Url}; use futures::future::{ok, Ready}; +use tinyvec::TinyVec; use crate::config::AppConfig; use crate::error::UrlGenerationError; @@ -21,7 +22,7 @@ pub(crate) struct HttpRequestInner { pub(crate) head: Message, pub(crate) path: Path, pub(crate) payload: Payload, - pub(crate) app_data: Rc, + pub(crate) app_data: TinyVec<[Rc; 4]>, rmap: Rc, config: AppConfig, pool: &'static HttpRequestPool, @@ -38,13 +39,16 @@ impl HttpRequest { app_data: Rc, pool: &'static HttpRequestPool, ) -> HttpRequest { + let mut data = TinyVec::<[Rc; 4]>::new(); + data.push(app_data); + HttpRequest(Rc::new(HttpRequestInner { head, path, payload, rmap, config, - app_data, + app_data: data, pool, })) } @@ -215,11 +219,13 @@ impl HttpRequest { /// let opt_t = req.app_data::>(); /// ``` pub fn app_data(&self) -> Option<&T> { - if let Some(st) = self.0.app_data.get::() { - Some(&st) - } else { - None + for container in self.0.app_data.iter().rev() { + if let Some(data) = container.get::() { + return Some(data); + } } + + None } } @@ -342,10 +348,13 @@ impl HttpRequestPool { #[cfg(test)] mod tests { + use actix_service::Service; + use bytes::Bytes; + use super::*; use crate::dev::{ResourceDef, ResourceMap}; use crate::http::{header, StatusCode}; - use crate::test::{call_service, init_service, TestRequest}; + use crate::test::{call_service, init_service, read_body, TestRequest}; use crate::{web, App, HttpResponse}; #[test] @@ -494,6 +503,68 @@ mod tests { assert_eq!(resp.status(), StatusCode::BAD_REQUEST); } + #[actix_rt::test] + async fn test_cascading_data() { + #[allow(dead_code)] + fn echo_usize(req: HttpRequest) -> HttpResponse { + let num = req.app_data::().unwrap(); + HttpResponse::Ok().body(num.to_string()) + } + + let mut srv = init_service( + App::new() + .app_data(88usize) + .service(web::resource("/").route(web::get().to(echo_usize))) + .service( + web::resource("/one") + .app_data(1u32) + .route(web::get().to(echo_usize)), + ), + ) + .await; + + let req = TestRequest::get().uri("/").to_request(); + let resp = srv.call(req).await.unwrap(); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"88")); + + let req = TestRequest::get().uri("/one").to_request(); + let resp = srv.call(req).await.unwrap(); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"88")); + } + + #[actix_rt::test] + async fn test_overwrite_data() { + #[allow(dead_code)] + fn echo_usize(req: HttpRequest) -> HttpResponse { + let num = req.app_data::().unwrap(); + HttpResponse::Ok().body(num.to_string()) + } + + let mut srv = init_service( + App::new() + .app_data(88usize) + .service(web::resource("/").route(web::get().to(echo_usize))) + .service( + web::resource("/one") + .app_data(1usize) + .route(web::get().to(echo_usize)), + ), + ) + .await; + + let req = TestRequest::get().uri("/").to_request(); + let resp = srv.call(req).await.unwrap(); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"88")); + + let req = TestRequest::get().uri("/one").to_request(); + let resp = srv.call(req).await.unwrap(); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"1")); + } + #[actix_rt::test] async fn test_extensions_dropped() { struct Tracker { diff --git a/src/resource.rs b/src/resource.rs index 634294cc2..477f0bfba 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -198,9 +198,7 @@ where /// Add resource data. /// - /// If used, this method will create a new data context used for extracting - /// from requests. Data added here is *not* merged with data added on App - /// or containing scopes. + /// Data of different types from parent contexts will still be accessible. pub fn app_data(mut self, data: U) -> Self { if self.data.is_none() { self.data = Some(Extensions::new()); @@ -539,14 +537,14 @@ impl Service for ResourceService { 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()); + req.add_data_container(data.clone()); } return Either::Right(route.call(req)); } } if let Some(ref mut default) = self.default { if let Some(ref data) = self.data { - req.set_data_container(data.clone()); + req.add_data_container(data.clone()); } Either::Right(default.call(req)) } else { @@ -590,14 +588,13 @@ mod tests { use actix_rt::time::delay_for; use actix_service::Service; - use bytes::Bytes; use futures::future::ok; use crate::http::{header, HeaderValue, Method, StatusCode}; use crate::middleware::DefaultHeaders; use crate::service::ServiceRequest; - use crate::test::{call_service, init_service, read_body, TestRequest}; - use crate::{guard, web, App, Error, HttpRequest, HttpResponse}; + use crate::test::{call_service, init_service, TestRequest}; + use crate::{guard, web, App, Error, HttpResponse}; #[actix_rt::test] async fn test_middleware() { @@ -623,79 +620,6 @@ mod tests { ); } - #[actix_rt::test] - async fn test_overwritten_data() { - #[allow(dead_code)] - fn echo_usize(req: HttpRequest) -> HttpResponse { - let num = req.app_data::().unwrap(); - HttpResponse::Ok().body(format!("{}", num)) - } - - #[allow(dead_code)] - fn echo_u32(req: HttpRequest) -> HttpResponse { - let num = req.app_data::().unwrap(); - HttpResponse::Ok().body(format!("{}", num)) - } - - #[allow(dead_code)] - fn echo_both(req: HttpRequest) -> HttpResponse { - let num = req.app_data::().unwrap(); - let num2 = req.app_data::().unwrap(); - HttpResponse::Ok().body(format!("{}-{}", num, num2)) - } - - let mut srv = init_service( - App::new() - .app_data(88usize) - .service(web::resource("/").route(web::get().to(echo_usize))) - .service( - web::resource("/one") - .app_data(1usize) - .route(web::get().to(echo_usize)), - ) - .service( - web::resource("/two") - .app_data(2usize) - .route(web::get().to(echo_usize)), - ) - .service( - web::resource("/three") - .app_data(3u32) - // this doesnt work because app_data "overrides" the - // entire data field potentially passed down - // .route(web::get().to(echo_both)), - .route(web::get().to(echo_u32)), - ) - .service(web::resource("/eight").route(web::get().to(echo_usize))), - ) - .await; - - let req = TestRequest::get().uri("/").to_request(); - let resp = srv.call(req).await.unwrap(); - let body = read_body(resp).await; - assert_eq!(body, Bytes::from_static(b"88")); - - let req = TestRequest::get().uri("/one").to_request(); - let resp = srv.call(req).await.unwrap(); - let body = read_body(resp).await; - assert_eq!(body, Bytes::from_static(b"1")); - - let req = TestRequest::get().uri("/two").to_request(); - let resp = srv.call(req).await.unwrap(); - let body = read_body(resp).await; - assert_eq!(body, Bytes::from_static(b"2")); - - // let req = TestRequest::get().uri("/three").to_request(); - // let resp = srv.call(req).await.unwrap(); - // let body = read_body(resp).await; - // assert_eq!(body, Bytes::from_static(b"88-3")); - - let req = TestRequest::get().uri("/eight").to_request(); - let resp = srv.call(req).await.unwrap(); - let body = read_body(resp).await; - assert_eq!(body, Bytes::from_static(b"88")); - } - #[actix_rt::test] async fn test_middleware_fn() { let mut srv = init_service( diff --git a/src/scope.rs b/src/scope.rs index 2016c6f1c..407d4946d 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -153,9 +153,7 @@ where /// Add scope data. /// - /// If used, this method will create a new data context used for extracting - /// from requests. Data added here is *not* merged with data added on App - /// or containing scopes. + /// Data of different types from parent contexts will still be accessible. pub fn app_data(mut self, data: U) -> Self { if self.data.is_none() { self.data = Some(Extensions::new()); @@ -624,12 +622,12 @@ impl Service for ScopeService { if let Some((srv, _info)) = res { if let Some(ref data) = self.data { - req.set_data_container(data.clone()); + req.add_data_container(data.clone()); } Either::Left(srv.call(req)) } else if let Some(ref mut default) = self.default { if let Some(ref data) = self.data { - req.set_data_container(data.clone()); + req.add_data_container(data.clone()); } Either::Left(default.call(req)) } else { diff --git a/src/service.rs b/src/service.rs index b783fd720..8dc9fa93d 100644 --- a/src/service.rs +++ b/src/service.rs @@ -217,11 +217,13 @@ 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.0).0.app_data.get::>() { - Some(st.clone()) - } else { - None + for container in (self.0).0.app_data.iter().rev() { + if let Some(data) = container.get::>() { + return Some(Data::clone(&data)); + } } + + None } /// Set request payload. @@ -230,9 +232,12 @@ impl ServiceRequest { } #[doc(hidden)] - /// Set new app data container - pub fn set_data_container(&mut self, extensions: Rc) { - Rc::get_mut(&mut (self.0).0).unwrap().app_data = extensions; + /// Add app data container to request's resolution set. + pub fn add_data_container(&mut self, extensions: Rc) { + Rc::get_mut(&mut (self.0).0) + .unwrap() + .app_data + .push(extensions); } } @@ -578,7 +583,6 @@ mod tests { let resp = srv.call(req).await.unwrap(); assert_eq!(resp.status(), http::StatusCode::NOT_FOUND); } - #[test] fn test_fmt_debug() { let req = TestRequest::get()