From 879cad94226a398523f8c258e3699eb864ba006a Mon Sep 17 00:00:00 2001
From: Rob Ede <robjtede@icloud.com>
Date: Sat, 9 May 2020 00:31:26 +0100
Subject: [PATCH] allow parent data containers to be accessed from child scopes
---
CHANGES.md | 10 +++++-
Cargo.toml | 1 +
src/app_service.rs | 2 +-
src/request.rs | 85 +++++++++++++++++++++++++++++++++++++++++----
src/resource.rs | 86 +++-------------------------------------------
src/scope.rs | 8 ++---
src/service.rs | 20 ++++++-----
7 files changed, 109 insertions(+), 103 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 2ed9c20b..97b7ca63 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 82389396..b24cc89d 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 808592e5..2d64bed3 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 51a1c54f..72fea1fa 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<RequestHead>,
pub(crate) path: Path<Url>,
pub(crate) payload: Payload,
- pub(crate) app_data: Rc<Extensions>,
+ pub(crate) app_data: TinyVec<[Rc<Extensions>; 4]>,
rmap: Rc<ResourceMap>,
config: AppConfig,
pool: &'static HttpRequestPool,
@@ -38,13 +39,16 @@ impl HttpRequest {
app_data: Rc<Extensions>,
pool: &'static HttpRequestPool,
) -> HttpRequest {
+ let mut data = TinyVec::<[Rc<Extensions>; 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::<Data<T>>();
/// ```
pub fn app_data<T: 'static>(&self) -> Option<&T> {
- if let Some(st) = self.0.app_data.get::<T>() {
- Some(&st)
- } else {
- None
+ for container in self.0.app_data.iter().rev() {
+ if let Some(data) = container.get::<T>() {
+ 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::<usize>().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::<usize>().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 634294cc..477f0bfb 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<U: 'static>(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::<usize>().unwrap();
- HttpResponse::Ok().body(format!("{}", num))
- }
-
- #[allow(dead_code)]
- fn echo_u32(req: HttpRequest) -> HttpResponse {
- let num = req.app_data::<u32>().unwrap();
- HttpResponse::Ok().body(format!("{}", num))
- }
-
- #[allow(dead_code)]
- fn echo_both(req: HttpRequest) -> HttpResponse {
- let num = req.app_data::<usize>().unwrap();
- let num2 = req.app_data::<u32>().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 2016c6f1..407d4946 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<U: 'static>(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 b783fd72..8dc9fa93 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<T: 'static>(&self) -> Option<Data<T>> {
- if let Some(st) = (self.0).0.app_data.get::<Data<T>>() {
- Some(st.clone())
- } else {
- None
+ for container in (self.0).0.app_data.iter().rev() {
+ if let Some(data) = container.get::<Data<T>>() {
+ 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<Extensions>) {
- 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<Extensions>) {
+ 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()