From 0093b7ea5ae5f6e23a247adce968ce1694c4e745 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 21 Jun 2018 11:47:01 +0600 Subject: [PATCH] refactor extractor configuration #331 --- MIGRATION.md | 32 +++++++++++++ src/extractor.rs | 9 ++-- src/lib.rs | 1 - src/route.rs | 95 +++++++++++++++++++++++++++++++++---- src/with.rs | 119 +++++------------------------------------------ 5 files changed, 134 insertions(+), 122 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 175d82b3c..73e2d5653 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -23,6 +23,38 @@ * Renamed `client::ClientConnectorError::Connector` to `client::ClientConnectorError::Resolver` +* `Route::with()` does not return `ExtractorConfig`, to configure + extractor use `Route::with_config()` + + instead of + + ```rust + fn main() { + let app = App::new().resource("/index.html", |r| { + r.method(http::Method::GET) + .with(index) + .limit(4096); // <- limit size of the payload + }); + } + ``` + + use + + ```rust + + fn main() { + let app = App::new().resource("/index.html", |r| { + r.method(http::Method::GET) + .with_config(index, |cfg| { // <- register handler + cfg.limit(4096); // <- limit size of the payload + }) + }); + } + ``` + +* `Route::with_async()` does not return `ExtractorConfig`, to configure + extractor use `Route::with_async_config()` + ## 0.6 diff --git a/src/extractor.rs b/src/extractor.rs index 0cdcb3afb..425bc7852 100644 --- a/src/extractor.rs +++ b/src/extractor.rs @@ -1,7 +1,7 @@ use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; -use std::{fmt, str}; use std::rc::Rc; +use std::{fmt, str}; use bytes::Bytes; use encoding::all::UTF_8; @@ -328,7 +328,7 @@ impl FormConfig { self.limit = limit; self } - + /// Set custom error handler pub fn error_handler(&mut self, f: F) -> &mut Self where @@ -408,8 +408,9 @@ impl FromRequest for Bytes { /// fn main() { /// let app = App::new().resource("/index.html", |r| { /// r.method(http::Method::GET) -/// .with(index) // <- register handler with extractor params -/// .limit(4096); // <- limit size of the payload +/// .with_config(index, |cfg| { // <- register handler with extractor params +/// cfg.limit(4096); // <- limit size of the payload +/// }) /// }); /// } /// ``` diff --git a/src/lib.rs b/src/lib.rs index 95f5a4ee0..90b743810 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -246,7 +246,6 @@ pub mod dev { pub use resource::ResourceHandler; pub use route::Route; pub use router::{Resource, ResourceType, Router}; - pub use with::ExtractorConfig; } pub mod http { diff --git a/src/route.rs b/src/route.rs index 524b66ef8..7ce1104c7 100644 --- a/src/route.rs +++ b/src/route.rs @@ -17,7 +17,7 @@ use middleware::{ Started as MiddlewareStarted, }; use pred::Predicate; -use with::{ExtractorConfig, With, WithAsync}; +use with::{With, WithAsync}; /// Resource route definition /// @@ -164,15 +164,49 @@ impl Route { /// ); // <- use `with` extractor /// } /// ``` - pub fn with(&mut self, handler: F) -> ExtractorConfig + pub fn with(&mut self, handler: F) where F: Fn(T) -> R + 'static, R: Responder + 'static, T: FromRequest + 'static, { - let cfg = ExtractorConfig::::default(); - self.h(With::new(handler, cfg.clone())); - cfg + self.h(With::new(handler, ::default())); + } + + /// Set handler function. Same as `.with()` but it allows to configure + /// extractor. + /// + /// ```rust + /// # extern crate bytes; + /// # extern crate actix_web; + /// # extern crate futures; + /// #[macro_use] extern crate serde_derive; + /// use actix_web::{http, App, Path, Result}; + /// + /// /// extract text data from request + /// fn index(body: String) -> Result { + /// Ok(format!("Body {}!", body)) + /// } + /// + /// fn main() { + /// let app = App::new().resource("/index.html", |r| { + /// r.method(http::Method::GET) + /// .with_config(index, |cfg| { // <- register handler + /// cfg.limit(4096); // <- limit size of the payload + /// }) + /// }); + /// } + /// ``` + pub fn with_config(&mut self, handler: F, cfg_f: C) + where + F: Fn(T) -> R + 'static, + R: Responder + 'static, + T: FromRequest + 'static, + C: FnOnce(&mut T::Config), + { + let mut cfg = ::default(); + cfg_f(&mut cfg); + self.h(With::new(handler, cfg)); } /// Set async handler function, use request extractor for parameters. @@ -204,7 +238,7 @@ impl Route { /// ); // <- use `with` extractor /// } /// ``` - pub fn with_async(&mut self, handler: F) -> ExtractorConfig + pub fn with_async(&mut self, handler: F) where F: Fn(T) -> R + 'static, R: Future + 'static, @@ -212,9 +246,52 @@ impl Route { E: Into + 'static, T: FromRequest + 'static, { - let cfg = ExtractorConfig::::default(); - self.h(WithAsync::new(handler, cfg.clone())); - cfg + self.h(WithAsync::new(handler, ::default())); + } + + /// Set async handler function, use request extractor for parameters. + /// This method allows to configure extractor. + /// + /// ```rust + /// # extern crate bytes; + /// # extern crate actix_web; + /// # extern crate futures; + /// #[macro_use] extern crate serde_derive; + /// use actix_web::{http, App, Error, Path}; + /// use futures::Future; + /// + /// #[derive(Deserialize)] + /// struct Info { + /// username: String, + /// } + /// + /// /// extract path info using serde + /// fn index(info: Form) -> Box> { + /// unimplemented!() + /// } + /// + /// fn main() { + /// let app = App::new().resource( + /// "/{username}/index.html", // <- define path parameters + /// |r| r.method(http::Method::GET) + /// .with_async_config(index, |cfg| { + /// cfg.limit(4096); + /// }), + /// ); // <- use `with` extractor + /// } + /// ``` + pub fn with_async_config(&mut self, handler: F, cfg: C) + where + F: Fn(T) -> R + 'static, + R: Future + 'static, + I: Responder + 'static, + E: Into + 'static, + T: FromRequest + 'static, + C: FnOnce(&mut T::Config), + { + let mut extractor_cfg = ::default(); + cfg(&mut extractor_cfg); + self.h(WithAsync::new(handler, extractor_cfg)); } } diff --git a/src/with.rs b/src/with.rs index 4f53a316d..126958b50 100644 --- a/src/with.rs +++ b/src/with.rs @@ -1,7 +1,6 @@ use futures::{Async, Future, Poll}; use std::cell::UnsafeCell; use std::marker::PhantomData; -use std::ops::{Deref, DerefMut}; use std::rc::Rc; use error::Error; @@ -9,110 +8,14 @@ use handler::{AsyncResult, AsyncResultItem, FromRequest, Handler, Responder}; use httprequest::HttpRequest; use httpresponse::HttpResponse; -/// Extractor configuration -/// -/// `Route::with()` and `Route::with_async()` returns instance -/// of the `ExtractorConfig` type. It could be used for extractor configuration. -/// -/// In this example `Form` configured. -/// -/// ```rust -/// # extern crate actix_web; -/// #[macro_use] extern crate serde_derive; -/// use actix_web::{http, App, Form, Result}; -/// -/// #[derive(Deserialize)] -/// struct FormData { -/// username: String, -/// } -/// -/// fn index(form: Form) -> Result { -/// Ok(format!("Welcome {}!", form.username)) -/// } -/// -/// fn main() { -/// let app = App::new().resource( -/// "/index.html", -/// |r| { -/// r.method(http::Method::GET).with(index).limit(4096); -/// }, // <- change form extractor configuration -/// ); -/// } -/// ``` -/// -/// Same could be donce with multiple extractors -/// -/// ```rust -/// # extern crate actix_web; -/// #[macro_use] extern crate serde_derive; -/// use actix_web::{http, App, Form, Path, Result}; -/// -/// #[derive(Deserialize)] -/// struct FormData { -/// username: String, -/// } -/// -/// fn index(data: (Path<(String,)>, Form)) -> Result { -/// Ok(format!("Welcome {}!", data.1.username)) -/// } -/// -/// fn main() { -/// let app = App::new().resource( -/// "/index.html", -/// |r| { -/// r.method(http::Method::GET).with(index).1.limit(4096); -/// }, // <- change form extractor configuration -/// ); -/// } -/// ``` -pub struct ExtractorConfig> { - cfg: Rc>, -} - -impl> Default for ExtractorConfig { - fn default() -> Self { - ExtractorConfig { - cfg: Rc::new(UnsafeCell::new(T::Config::default())), - } - } -} - -impl> ExtractorConfig { - pub(crate) fn clone(&self) -> Self { - ExtractorConfig { - cfg: Rc::clone(&self.cfg), - } - } -} - -impl> AsRef for ExtractorConfig { - fn as_ref(&self) -> &T::Config { - unsafe { &*self.cfg.get() } - } -} - -impl> Deref for ExtractorConfig { - type Target = T::Config; - - fn deref(&self) -> &T::Config { - unsafe { &*self.cfg.get() } - } -} - -impl> DerefMut for ExtractorConfig { - fn deref_mut(&mut self) -> &mut T::Config { - unsafe { &mut *self.cfg.get() } - } -} - -pub struct With +pub(crate) struct With where F: Fn(T) -> R, T: FromRequest, S: 'static, { hnd: Rc>, - cfg: ExtractorConfig, + cfg: Rc, } pub struct WithHnd @@ -132,9 +35,9 @@ where T: FromRequest, S: 'static, { - pub fn new(f: F, cfg: ExtractorConfig) -> Self { + pub fn new(f: F, cfg: T::Config) -> Self { With { - cfg, + cfg: Rc::new(cfg), hnd: Rc::new(WithHnd { hnd: Rc::new(UnsafeCell::new(f)), _t: PhantomData, @@ -180,7 +83,7 @@ where { started: bool, hnd: Rc>, - cfg: ExtractorConfig, + cfg: Rc, req: HttpRequest, fut1: Option>>, fut2: Option>>, @@ -244,7 +147,7 @@ where } } -pub struct WithAsync +pub(crate) struct WithAsync where F: Fn(T) -> R, R: Future, @@ -254,7 +157,7 @@ where S: 'static, { hnd: Rc>, - cfg: ExtractorConfig, + cfg: Rc, } impl WithAsync @@ -266,9 +169,9 @@ where T: FromRequest, S: 'static, { - pub fn new(f: F, cfg: ExtractorConfig) -> Self { + pub fn new(f: F, cfg: T::Config) -> Self { WithAsync { - cfg, + cfg: Rc::new(cfg), hnd: Rc::new(WithHnd { hnd: Rc::new(UnsafeCell::new(f)), _s: PhantomData, @@ -294,7 +197,7 @@ where req, started: false, hnd: Rc::clone(&self.hnd), - cfg: self.cfg.clone(), + cfg: Rc::clone(&self.cfg), fut1: None, fut2: None, fut3: None, @@ -319,7 +222,7 @@ where { started: bool, hnd: Rc>, - cfg: ExtractorConfig, + cfg: Rc, req: HttpRequest, fut1: Option>>, fut2: Option,