From b2a9ba2ee432be1b60709b4b29b1b4cd31b2512b Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 15 Jan 2021 04:54:23 +0000 Subject: [PATCH 1/3] Update PULL_REQUEST_TEMPLATE.md --- .github/PULL_REQUEST_TEMPLATE.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index b779b33fa..42deadf5a 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,21 +1,21 @@ - + ## PR Type -INSERT_PR_TYPE +PR_TYPE ## PR Checklist -Check your PR fulfills the following: - + - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] A changelog entry has been made for the appropriate packages. -- [ ] Format code with the latest stable rustfmt +- [ ] Format code with the latest stable rustfmt. +- [ ] (Team) Label with affected crates and semver status. ## Overview From 0a506bf2e9f0d07c505df725a68808c6343f7a4e Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 15 Jan 2021 05:38:50 +0000 Subject: [PATCH 2/3] cleanup top level doc comments --- actix-http/src/client/config.rs | 5 +- actix-http/src/client/h2proto.rs | 2 +- actix-http/src/config.rs | 2 +- actix-http/src/error.rs | 1 - .../src/header/common/content_disposition.rs | 14 ++--- actix-http/src/header/mod.rs | 4 +- actix-http/src/ws/mask.rs | 8 +-- actix-http/src/ws/mod.rs | 6 --- awc/tests/test_client.rs | 5 ++ awc/tests/test_rustls_client.rs | 1 + src/app_service.rs | 4 +- src/handler.rs | 2 +- src/middleware/compat.rs | 3 +- src/types/json.rs | 2 +- src/types/payload.rs | 2 +- tests/test_server.rs | 26 +--------- tests/test_weird_poll.rs | 52 +++++++++---------- 17 files changed, 57 insertions(+), 82 deletions(-) diff --git a/actix-http/src/client/config.rs b/actix-http/src/client/config.rs index c86c697a2..369732964 100644 --- a/actix-http/src/client/config.rs +++ b/actix-http/src/client/config.rs @@ -1,8 +1,7 @@ use std::time::Duration; -// These values are taken from hyper/src/proto/h2/client.rs -const DEFAULT_H2_CONN_WINDOW: u32 = 1024 * 1024 * 2; // 2mb -const DEFAULT_H2_STREAM_WINDOW: u32 = 1024 * 1024; // 1mb +const DEFAULT_H2_CONN_WINDOW: u32 = 1024 * 1024 * 2; // 2MB +const DEFAULT_H2_STREAM_WINDOW: u32 = 1024 * 1024; // 1MB /// Connector configuration #[derive(Clone)] diff --git a/actix-http/src/client/h2proto.rs b/actix-http/src/client/h2proto.rs index 4c609ef22..76915f214 100644 --- a/actix-http/src/client/h2proto.rs +++ b/actix-http/src/client/h2proto.rs @@ -171,7 +171,7 @@ async fn send_body( } } -// release SendRequest object +/// release SendRequest object fn release( io: SendRequest, pool: Option>, diff --git a/actix-http/src/config.rs b/actix-http/src/config.rs index 1cd7e4aea..f178db274 100644 --- a/actix-http/src/config.rs +++ b/actix-http/src/config.rs @@ -9,7 +9,7 @@ use bytes::BytesMut; use futures_util::{future, FutureExt}; use time::OffsetDateTime; -// "Sun, 06 Nov 1994 08:49:37 GMT".len() +/// "Sun, 06 Nov 1994 08:49:37 GMT".len() const DATE_VALUE_LENGTH: usize = 29; #[derive(Debug, PartialEq, Clone, Copy)] diff --git a/actix-http/src/error.rs b/actix-http/src/error.rs index a585962be..9ff154240 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -18,7 +18,6 @@ use serde::de::value::Error as DeError; use serde_json::error::Error as JsonError; use serde_urlencoded::ser::Error as FormError; -// re-export for convenience use crate::body::Body; pub use crate::cookie::ParseError as CookieParseError; use crate::helpers::Writer; diff --git a/actix-http/src/header/common/content_disposition.rs b/actix-http/src/header/common/content_disposition.rs index ae4a97902..ecc59aba3 100644 --- a/actix-http/src/header/common/content_disposition.rs +++ b/actix-http/src/header/common/content_disposition.rs @@ -1,10 +1,10 @@ -// # References -// -// "The Content-Disposition Header Field" https://www.ietf.org/rfc/rfc2183.txt -// "The Content-Disposition Header Field in the Hypertext Transfer Protocol (HTTP)" https://www.ietf.org/rfc/rfc6266.txt -// "Returning Values from Forms: multipart/form-data" https://www.ietf.org/rfc/rfc7578.txt -// Browser conformance tests at: http://greenbytes.de/tech/tc2231/ -// IANA assignment: http://www.iana.org/assignments/cont-disp/cont-disp.xhtml +//! # References +//! +//! "The Content-Disposition Header Field" https://www.ietf.org/rfc/rfc2183.txt +//! "The Content-Disposition Header Field in the Hypertext Transfer Protocol (HTTP)" https://www.ietf.org/rfc/rfc6266.txt +//! "Returning Values from Forms: multipart/form-data" https://www.ietf.org/rfc/rfc7578.txt +//! Browser conformance tests at: http://greenbytes.de/tech/tc2231/ +//! IANA assignment: http://www.iana.org/assignments/cont-disp/cont-disp.xhtml use lazy_static::lazy_static; use regex::Regex; diff --git a/actix-http/src/header/mod.rs b/actix-http/src/header/mod.rs index e4e15cd91..dc97bf5ff 100644 --- a/actix-http/src/header/mod.rs +++ b/actix-http/src/header/mod.rs @@ -80,8 +80,8 @@ impl From for HeaderMap { } } -// This encode set is used for HTTP header values and is defined at -// https://tools.ietf.org/html/rfc5987#section-3.2. +/// This encode set is used for HTTP header values and is defined at +/// https://tools.ietf.org/html/rfc5987#section-3.2. pub(crate) const HTTP_VALUE: &AsciiSet = &CONTROLS .add(b' ') .add(b'"') diff --git a/actix-http/src/ws/mask.rs b/actix-http/src/ws/mask.rs index d37d57eb1..79e015f79 100644 --- a/actix-http/src/ws/mask.rs +++ b/actix-http/src/ws/mask.rs @@ -3,7 +3,7 @@ use std::ptr::copy_nonoverlapping; use std::slice; -// Holds a slice guaranteed to be shorter than 8 bytes +/// Holds a slice guaranteed to be shorter than 8 bytes. struct ShortSlice<'a> { inner: &'a mut [u8], } @@ -80,8 +80,10 @@ unsafe fn cast_slice(buf: &mut [u8]) -> &mut [u64] { slice::from_raw_parts_mut(buf.as_mut_ptr() as *mut u64, buf.len() >> 3) } -// Splits a slice into three parts: an unaligned short head and tail, plus an aligned -// u64 mid section. +/// Splits a slice into three parts: +/// - an unaligned short head +/// - an aligned `u64` slice mid section +/// - an unaligned short tail #[inline] fn align_buf(buf: &mut [u8]) -> (ShortSlice<'_>, &mut [u64], ShortSlice<'_>) { let start_ptr = buf.as_ptr() as usize; diff --git a/actix-http/src/ws/mod.rs b/actix-http/src/ws/mod.rs index f8142693f..dad2646c1 100644 --- a/actix-http/src/ws/mod.rs +++ b/actix-http/src/ws/mod.rs @@ -128,18 +128,12 @@ impl ResponseError for HandshakeError { } /// Verify `WebSocket` handshake request and create handshake response. -// /// `protocols` is a sequence of known protocols. On successful handshake, -// /// the returned response headers contain the first protocol in this list -// /// which the server also knows. pub fn handshake(req: &RequestHead) -> Result { verify_handshake(req)?; Ok(handshake_response(req)) } /// Verify `WebSocket` handshake request. -// /// `protocols` is a sequence of known protocols. On successful handshake, -// /// the returned response headers contain the first protocol in this list -// /// which the server also knows. pub fn verify_handshake(req: &RequestHead) -> Result<(), HandshakeError> { // WebSocket accepts only GET if req.method != Method::GET { diff --git a/awc/tests/test_client.rs b/awc/tests/test_client.rs index c31921560..3f5bf2958 100644 --- a/awc/tests/test_client.rs +++ b/awc/tests/test_client.rs @@ -557,6 +557,7 @@ async fn test_client_brotli_encoding_large_random() { assert_eq!(bytes, Bytes::from(data)); } +// TODO: why is test ignored // #[actix_rt::test] // async fn test_client_deflate_encoding() { // let srv = test::TestServer::start(|app| { @@ -585,6 +586,7 @@ async fn test_client_brotli_encoding_large_random() { // assert_eq!(bytes, Bytes::from_static(STR.as_ref())); // } +// TODO: why is test ignored // #[actix_rt::test] // async fn test_client_deflate_encoding_large_random() { // let data = rand::thread_rng() @@ -618,6 +620,7 @@ async fn test_client_brotli_encoding_large_random() { // assert_eq!(bytes, Bytes::from(data)); // } +// TODO: why is test ignored // #[actix_rt::test] // async fn test_client_streaming_explicit() { // let srv = test::TestServer::start(|app| { @@ -645,6 +648,7 @@ async fn test_client_brotli_encoding_large_random() { // assert_eq!(bytes, Bytes::from_static(STR.as_ref())); // } +// TODO: why is test ignored // #[actix_rt::test] // async fn test_body_streaming_implicit() { // let srv = test::TestServer::start(|app| { @@ -734,6 +738,7 @@ async fn test_client_cookie_handling() { assert_eq!(c2, cookie2); } +// TODO: why is test ignored // #[actix_rt::test] // fn client_read_until_eof() { // let addr = test::TestServer::unused_addr(); diff --git a/awc/tests/test_rustls_client.rs b/awc/tests/test_rustls_client.rs index 3fa76d4a9..2f5ffeca2 100644 --- a/awc/tests/test_rustls_client.rs +++ b/awc/tests/test_rustls_client.rs @@ -50,6 +50,7 @@ mod danger { } } +// TODO: why is test ignored // #[actix_rt::test] async fn _test_connection_reuse_h2() { let num = Arc::new(AtomicUsize::new(0)); diff --git a/src/app_service.rs b/src/app_service.rs index 8169be517..4c099d4a4 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -149,7 +149,7 @@ where } } -/// Service to convert `Request` to a `ServiceRequest` +/// Service that takes a [`Request`] and delegates to a service that take a [`ServiceRequest`]. pub struct AppInitService where T: Service, Error = Error>, @@ -159,7 +159,7 @@ where app_state: Rc, } -// a collection of AppInitService state that shared between HttpRequests. +/// A collection of [`AppInitService`] state that shared across `HttpRequest`s. pub(crate) struct AppInitServiceState { rmap: Rc, config: AppConfig, diff --git a/src/handler.rs b/src/handler.rs index 47656cd84..1526f050d 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -101,7 +101,7 @@ where } } -// HandlerService is both it's ServiceFactory and Service Type. +/// HandlerService is both it's ServiceFactory and Service Type. impl Service for HandlerService where F: Handler, diff --git a/src/middleware/compat.rs b/src/middleware/compat.rs index eabd1190d..d5661fc77 100644 --- a/src/middleware/compat.rs +++ b/src/middleware/compat.rs @@ -110,8 +110,7 @@ where } } -// trait for convert ServiceResponse's ResponseBody generic type -// to ResponseBody +/// Convert `ServiceResponse`'s `ResponseBody` generic type to `ResponseBody`. pub trait MapServiceResponseBody { fn map_body(self) -> ServiceResponse; } diff --git a/src/types/json.rs b/src/types/json.rs index 7b94f66f6..41618b409 100644 --- a/src/types/json.rs +++ b/src/types/json.rs @@ -269,7 +269,7 @@ impl JsonConfig { } } -// Allow shared refs to default. +/// Allow shared refs used as default. const DEFAULT_CONFIG: JsonConfig = JsonConfig { limit: 32_768, // 2^15 bytes, (~32kB) err_handler: None, diff --git a/src/types/payload.rs b/src/types/payload.rs index bee1b695a..ad0bfd14f 100644 --- a/src/types/payload.rs +++ b/src/types/payload.rs @@ -247,7 +247,7 @@ impl PayloadConfig { } } -// Allow shared refs to default. +/// Allow shared refs used as defaults. const DEFAULT_CONFIG: PayloadConfig = PayloadConfig { limit: DEFAULT_CONFIG_LIMIT, mimetype: None, diff --git a/tests/test_server.rs b/tests/test_server.rs index e8735966f..7756d44d2 100644 --- a/tests/test_server.rs +++ b/tests/test_server.rs @@ -800,6 +800,7 @@ async fn test_reading_deflate_encoding_large_random_rustls() { assert_eq!(bytes, Bytes::from(data)); } +// TODO: why is test ignored // #[test] // fn test_server_cookies() { // use actix_web::http; @@ -889,28 +890,3 @@ async fn test_normalize() { let response = srv.get("/one/").send().await.unwrap(); assert!(response.status().is_success()); } - -// #[cfg(feature = "openssl")] -// #[actix_rt::test] -// async fn test_ssl_handshake_timeout() { -// use open_ssl::ssl::{SslAcceptor, SslFiletype, SslMethod}; -// use std::net; - -// // load ssl keys -// let mut builder = SslAcceptor::mozilla_intermediate(SslMethod::tls()).unwrap(); -// builder -// .set_private_key_file("tests/key.pem", SslFiletype::PEM) -// .unwrap(); -// builder -// .set_certificate_chain_file("tests/cert.pem") -// .unwrap(); - -// let srv = test::start_with(test::config().openssl(builder.build()), || { -// App::new().service(web::resource("/").route(web::to(|| HttpResponse::Ok()))) -// }); - -// let mut stream = net::TcpStream::connect(srv.addr()).unwrap(); -// let mut data = String::new(); -// let _ = stream.read_to_string(&mut data); -// assert!(data.is_empty()); -// } diff --git a/tests/test_weird_poll.rs b/tests/test_weird_poll.rs index 7e4300901..5844ea2c2 100644 --- a/tests/test_weird_poll.rs +++ b/tests/test_weird_poll.rs @@ -1,30 +1,30 @@ -// Regression test for #/1321 +//! Regression test for https://github.com/actix/actix-web/issues/1321 -/* -use futures::task::{noop_waker, Context}; -use futures::stream::once; -use actix_http::body::{MessageBody, BodyStream}; -use bytes::Bytes; +// use actix_http::body::{BodyStream, MessageBody}; +// use bytes::Bytes; +// use futures_channel::oneshot; +// use futures_util::{ +// stream::once, +// task::{noop_waker, Context}, +// }; -Disable weird poll until actix-web is based on actix-http 2.0.0 +// #[test] +// fn weird_poll() { +// let (sender, receiver) = oneshot::channel(); +// let mut body_stream = Ok(BodyStream::new(once(async { +// let x = Box::new(0); +// let y = &x; +// receiver.await.unwrap(); +// let _z = **y; +// Ok::<_, ()>(Bytes::new()) +// }))); -#[test] -fn weird_poll() { - let (sender, receiver) = futures::channel::oneshot::channel(); - let mut body_stream = Ok(BodyStream::new(once(async { - let x = Box::new(0); - let y = &x; - receiver.await.unwrap(); - let _z = **y; - Ok::<_, ()>(Bytes::new()) - }))); +// let waker = noop_waker(); +// let mut cx = Context::from_waker(&waker); - let waker = noop_waker(); - let mut context = Context::from_waker(&waker); - - let _ = body_stream.as_mut().unwrap().poll_next(&mut context); - sender.send(()).unwrap(); - let _ = std::mem::replace(&mut body_stream, Err([0; 32])).unwrap().poll_next(&mut context); -} - -*/ +// let _ = body_stream.as_mut().unwrap().poll_next(&mut cx); +// sender.send(()).unwrap(); +// let _ = std::mem::replace(&mut body_stream, Err([0; 32])) +// .unwrap() +// .poll_next(&mut cx); +// } From da69bb4d12c35d170f85273eeda190ae86551d35 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 15 Jan 2021 23:37:33 +0000 Subject: [PATCH 3/3] implement `App::data` as `App::app_data(Data::new(T)))` (#1906) --- CHANGES.md | 7 ++++ src/app.rs | 25 ++++---------- src/app_service.rs | 17 +++------- src/config.rs | 82 ++++++++++++++++++++++------------------------ src/data.rs | 29 ++++++++++++---- src/resource.rs | 17 +++++----- src/scope.rs | 24 +++----------- src/service.rs | 6 ++-- 8 files changed, 99 insertions(+), 108 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 12caa2df9..a6bcd56cc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,15 +12,22 @@ `ServiceRequest::from_request` would not fail and no payload would be generated [#1893] * Our `Either` type now uses `Left`/`Right` variants (instead of `A`/`B`) [#1894] +### Fixed +* Multiple calls `App::data` with the same type now keeps the latest call's data. [#1906] + ### Removed * Public field of `web::Path` has been made private. [#1894] * Public field of `web::Query` has been made private. [#1894] * `TestRequest::with_header`; use `TestRequest::default().insert_header()`. [#1869] +* `AppService::set_service_data`; for custom HTTP service factories adding application data, use the + layered data model by calling `ServiceRequest::add_data_container` when handling + requests instead. [#1906] [#1891]: https://github.com/actix/actix-web/pull/1891 [#1893]: https://github.com/actix/actix-web/pull/1893 [#1894]: https://github.com/actix/actix-web/pull/1894 [#1869]: https://github.com/actix/actix-web/pull/1869 +[#1906]: https://github.com/actix/actix-web/pull/1906 ## 4.0.0-beta.1 - 2021-01-07 diff --git a/src/app.rs b/src/app.rs index fcb491a21..9c5b806ea 100644 --- a/src/app.rs +++ b/src/app.rs @@ -34,7 +34,6 @@ pub struct App { services: Vec>, default: Option>, factory_ref: Rc>>, - data: Vec>, data_factories: Vec, external: Vec, extensions: Extensions, @@ -48,7 +47,6 @@ impl App { let fref = Rc::new(RefCell::new(None)); App { endpoint: AppEntry::new(fref.clone()), - data: Vec::new(), data_factories: Vec::new(), services: Vec::new(), default: None, @@ -101,9 +99,8 @@ where /// web::resource("/index.html").route( /// web::get().to(index))); /// ``` - pub fn data(mut self, data: U) -> Self { - self.data.push(Box::new(Data::new(data))); - self + pub fn data(self, data: U) -> Self { + self.app_data(Data::new(data)) } /// Set application data factory. This function is @@ -157,8 +154,7 @@ where /// some of the resource's configuration could be moved to different module. /// /// ```rust - /// # extern crate actix_web; - /// use actix_web::{web, middleware, App, HttpResponse}; + /// use actix_web::{web, App, HttpResponse}; /// /// // this function could be located in different module /// fn config(cfg: &mut web::ServiceConfig) { @@ -168,12 +164,9 @@ where /// ); /// } /// - /// fn main() { - /// let app = App::new() - /// .wrap(middleware::Logger::default()) - /// .configure(config) // <- register resources - /// .route("/index.html", web::get().to(|| HttpResponse::Ok())); - /// } + /// App::new() + /// .configure(config) // <- register resources + /// .route("/index.html", web::get().to(|| HttpResponse::Ok())); /// ``` pub fn configure(mut self, f: F) -> Self where @@ -181,10 +174,9 @@ where { let mut cfg = ServiceConfig::new(); f(&mut cfg); - self.data.extend(cfg.data); self.services.extend(cfg.services); self.external.extend(cfg.external); - self.extensions.extend(cfg.extensions); + self.extensions.extend(cfg.app_data); self } @@ -374,7 +366,6 @@ where { App { endpoint: apply(mw, self.endpoint), - data: self.data, data_factories: self.data_factories, services: self.services, default: self.default, @@ -436,7 +427,6 @@ where { App { endpoint: apply_fn_factory(self.endpoint, mw), - data: self.data, data_factories: self.data_factories, services: self.services, default: self.default, @@ -462,7 +452,6 @@ where { fn into_factory(self) -> AppInit { AppInit { - data_factories: self.data.into_boxed_slice().into(), async_data_factories: self.data_factories.into_boxed_slice().into(), endpoint: self.endpoint, services: Rc::new(RefCell::new(self.services)), diff --git a/src/app_service.rs b/src/app_service.rs index 4c099d4a4..8a00f59eb 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -10,7 +10,7 @@ use futures_core::future::LocalBoxFuture; use futures_util::future::join_all; use crate::config::{AppConfig, AppService}; -use crate::data::{DataFactory, FnDataFactory}; +use crate::data::FnDataFactory; use crate::error::Error; use crate::guard::Guard; use crate::request::{HttpRequest, HttpRequestPool}; @@ -35,7 +35,6 @@ where { pub(crate) endpoint: T, pub(crate) extensions: RefCell>, - pub(crate) data_factories: Rc<[Box]>, pub(crate) async_data_factories: Rc<[FnDataFactory]>, pub(crate) services: Rc>>>, pub(crate) default: Option>, @@ -71,8 +70,7 @@ where }); // App config - let mut config = - AppService::new(config, default.clone(), self.data_factories.clone()); + let mut config = AppService::new(config, default.clone()); // register services std::mem::take(&mut *self.services.borrow_mut()) @@ -119,8 +117,6 @@ where .take() .unwrap_or_else(Extensions::new); - let data_factories = self.data_factories.clone(); - Box::pin(async move { // async data factories let async_data_factories = factory_futs @@ -133,12 +129,9 @@ where let service = endpoint_fut.await?; // populate app data container from (async) data factories. - data_factories - .iter() - .chain(&async_data_factories) - .for_each(|factory| { - factory.create(&mut app_data); - }); + async_data_factories.iter().for_each(|factory| { + factory.create(&mut app_data); + }); Ok(AppInitService { service, diff --git a/src/config.rs b/src/config.rs index 2b93ae892..8e22dc90d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -5,7 +5,7 @@ use actix_http::Extensions; use actix_router::ResourceDef; use actix_service::{boxed, IntoServiceFactory, ServiceFactory}; -use crate::data::{Data, DataFactory}; +use crate::data::Data; use crate::error::Error; use crate::guard::Guard; use crate::resource::Resource; @@ -31,20 +31,14 @@ pub struct AppService { Option, Option>, )>, - service_data: Rc<[Box]>, } impl AppService { - /// Crate server settings instance - pub(crate) fn new( - config: AppConfig, - default: Rc, - service_data: Rc<[Box]>, - ) -> Self { + /// Crate server settings instance. + pub(crate) fn new(config: AppConfig, default: Rc) -> Self { AppService { config, default, - service_data, root: true, services: Vec::new(), } @@ -75,7 +69,6 @@ impl AppService { default: self.default.clone(), services: Vec::new(), root: false, - service_data: self.service_data.clone(), } } @@ -89,15 +82,7 @@ impl AppService { self.default.clone() } - /// Set global route data - pub fn set_service_data(&self, extensions: &mut Extensions) -> bool { - for f in self.service_data.iter() { - f.create(extensions); - } - !self.service_data.is_empty() - } - - /// Register http service + /// Register HTTP service. pub fn register_service( &mut self, rdef: ResourceDef, @@ -168,47 +153,60 @@ impl Default for AppConfig { } } -/// Service config is used for external configuration. -/// Part of application configuration could be offloaded -/// to set of external methods. This could help with -/// modularization of big application configuration. +/// Enables parts of app configuration to be declared separately from the app itself. Helpful for +/// modularizing large applications. +/// +/// Merge a `ServiceConfig` into an app using [`App::configure`](crate::App::configure). Scope and +/// resources services have similar methods. +/// +/// ``` +/// use actix_web::{web, App, HttpResponse}; +/// +/// // this function could be located in different module +/// fn config(cfg: &mut web::ServiceConfig) { +/// cfg.service(web::resource("/test") +/// .route(web::get().to(|| HttpResponse::Ok())) +/// .route(web::head().to(|| HttpResponse::MethodNotAllowed())) +/// ); +/// } +/// +/// // merge `/test` routes from config function to App +/// App::new().configure(config); +/// ``` pub struct ServiceConfig { pub(crate) services: Vec>, - pub(crate) data: Vec>, pub(crate) external: Vec, - pub(crate) extensions: Extensions, + pub(crate) app_data: Extensions, } impl ServiceConfig { pub(crate) fn new() -> Self { Self { services: Vec::new(), - data: Vec::new(), external: Vec::new(), - extensions: Extensions::new(), + app_data: Extensions::new(), } } - /// Set application data. Application data could be accessed - /// by using `Data` extractor where `T` is data type. + /// Add shared app data item. /// - /// This is same as `App::data()` method. - pub fn data(&mut self, data: S) -> &mut Self { - self.data.push(Box::new(Data::new(data))); + /// Counterpart to [`App::data()`](crate::App::data). + pub fn data(&mut self, data: U) -> &mut Self { + self.app_data(Data::new(data)); self } - /// Set arbitrary data item. + /// Add arbitrary app data item. /// - /// This is same as `App::data()` method. + /// Counterpart to [`App::app_data()`](crate::App::app_data). pub fn app_data(&mut self, ext: U) -> &mut Self { - self.extensions.insert(ext); + self.app_data.insert(ext); self } /// Configure route for a specific path. /// - /// This is same as `App::route()` method. + /// Counterpart to [`App::route()`](crate::App::route). pub fn route(&mut self, path: &str, mut route: Route) -> &mut Self { self.service( Resource::new(path) @@ -217,9 +215,9 @@ impl ServiceConfig { ) } - /// Register http service. + /// Register HTTP service factory. /// - /// This is same as `App::service()` method. + /// Counterpart to [`App::service()`](crate::App::service). pub fn service(&mut self, factory: F) -> &mut Self where F: HttpServiceFactory + 'static, @@ -231,11 +229,11 @@ impl ServiceConfig { /// Register an external resource. /// - /// External resources are useful for URL generation purposes only - /// and are never considered for matching at request time. Calls to - /// `HttpRequest::url_for()` will work as expected. + /// External resources are useful for URL generation purposes only and are never considered for + /// matching at request time. Calls to [`HttpRequest::url_for()`](crate::HttpRequest::url_for) + /// will work as expected. /// - /// This is same as `App::external_service()` method. + /// Counterpart to [`App::external_resource()`](crate::App::external_resource). pub fn external_resource(&mut self, name: N, url: U) -> &mut Self where N: AsRef, diff --git a/src/data.rs b/src/data.rs index 19c258ff0..dd0fbed0e 100644 --- a/src/data.rs +++ b/src/data.rs @@ -10,8 +10,9 @@ use crate::dev::Payload; use crate::extract::FromRequest; use crate::request::HttpRequest; -/// Application data factory +/// Data factory. pub(crate) trait DataFactory { + /// Return true if modifications were made to extensions map. fn create(&self, extensions: &mut Extensions) -> bool; } @@ -126,12 +127,8 @@ impl FromRequest for Data { impl DataFactory for Data { fn create(&self, extensions: &mut Extensions) -> bool { - if !extensions.contains::>() { - extensions.insert(Data(self.0.clone())); - true - } else { - false - } + extensions.insert(Data(self.0.clone())); + true } } @@ -167,6 +164,24 @@ mod tests { let req = TestRequest::default().to_request(); let resp = srv.call(req).await.unwrap(); assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); + + let mut srv = init_service( + App::new() + .data(10u32) + .data(13u32) + .app_data(12u64) + .app_data(15u64) + .default_service(web::to(|n: web::Data, req: HttpRequest| { + // in each case, the latter insertion should be preserved + assert_eq!(*req.app_data::().unwrap(), 15); + assert_eq!(*n.into_inner(), 13); + 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] diff --git a/src/resource.rs b/src/resource.rs index 843237079..3844b429d 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -203,10 +203,10 @@ where /// /// Data of different types from parent contexts will still be accessible. pub fn app_data(mut self, data: U) -> Self { - if self.app_data.is_none() { - self.app_data = Some(Extensions::new()); - } - self.app_data.as_mut().unwrap().insert(data); + self.app_data + .get_or_insert_with(Extensions::new) + .insert(data); + self } @@ -382,18 +382,16 @@ where } else { Some(std::mem::take(&mut self.guards)) }; + let mut rdef = if config.is_root() || !self.rdef.is_empty() { ResourceDef::new(insert_slash(self.rdef.clone())) } else { ResourceDef::new(self.rdef.clone()) }; + if let Some(ref name) = self.name { *rdef.name_mut() = name.clone(); } - // custom app data storage - if let Some(ref mut ext) = self.app_data { - config.set_service_data(ext); - } config.register_service(rdef, guards, self, None) } @@ -479,12 +477,15 @@ impl Service for ResourceService { if let Some(ref app_data) = self.app_data { req.add_data_container(app_data.clone()); } + return route.call(req); } } + if let Some(ref app_data) = self.app_data { req.add_data_container(app_data.clone()); } + self.default.call(req) } } diff --git a/src/scope.rs b/src/scope.rs index 2da4f5546..290a35eb1 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -155,10 +155,10 @@ where /// /// Data of different types from parent contexts will still be accessible. pub fn app_data(mut self, data: U) -> Self { - if self.app_data.is_none() { - self.app_data = Some(Extensions::new()); - } - self.app_data.as_mut().unwrap().insert(data); + self.app_data + .get_or_insert_with(Extensions::new) + .insert(data); + self } @@ -200,18 +200,9 @@ where self.services.extend(cfg.services); self.external.extend(cfg.external); - if !cfg.data.is_empty() { - let mut data = self.app_data.unwrap_or_else(Extensions::new); - - for value in cfg.data.iter() { - value.create(&mut data); - } - - self.app_data = Some(data); - } self.app_data .get_or_insert_with(Extensions::new) - .extend(cfg.extensions); + .extend(cfg.app_data); self } @@ -432,11 +423,6 @@ where rmap.add(&mut rdef, None); } - // custom app data storage - if let Some(ref mut ext) = self.app_data { - config.set_service_data(ext); - } - // complete scope pipeline creation *self.factory_ref.borrow_mut() = Some(ScopeFactory { app_data: self.app_data.take().map(Rc::new), diff --git a/src/service.rs b/src/service.rs index d4fa4acca..596eedd7f 100644 --- a/src/service.rs +++ b/src/service.rs @@ -231,8 +231,10 @@ impl ServiceRequest { self.payload = payload; } - #[doc(hidden)] - /// Add app data container to request's resolution set. + /// Add data container to request's resolution set. + /// + /// In middleware, prefer [`extensions_mut`](ServiceRequest::extensions_mut) for request-local + /// data since it is assumed that the same app data is presented for every request. pub fn add_data_container(&mut self, extensions: Rc) { Rc::get_mut(&mut (self.req).inner) .unwrap()