From 8fff2c7595ff6d768d4985792ade2c198404fa21 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Mon, 26 Mar 2018 18:18:38 -0700 Subject: [PATCH] remove Path and Query from public api --- CHANGES.md | 2 ++ guide/src/qs_4_5.md | 6 ++-- src/error.rs | 8 ++++++ src/extractor.rs | 26 ++++++++---------- src/httprequest.rs | 67 +++++++++++++++++++++++++++++++++++++++------ src/lib.rs | 3 +- 6 files changed, 84 insertions(+), 28 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 9043d3fd8..15ed9aead 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,8 @@ ## 0.4.11 +* Added `HttpReuqest::extract_xxx()`, type safe path/query information extractor. + * Fix long client urls #129 * Fix panic on invalid URL characters #130 diff --git a/guide/src/qs_4_5.md b/guide/src/qs_4_5.md index f46042712..0cbfc7de7 100644 --- a/guide/src/qs_4_5.md +++ b/guide/src/qs_4_5.md @@ -5,7 +5,7 @@ and [`ResponseError` trait](../actix_web/error/trait.ResponseError.html) for handling handler's errors. Any error that implements `ResponseError` trait can be returned as error value. *Handler* can return *Result* object, actix by default provides -`Responder` implementation for compatible result object. Here is implementation +`Responder` implementation for compatible result types. Here is implementation definition: ```rust,ignore @@ -64,7 +64,7 @@ fn index(req: HttpRequest) -> Result<&'static str, MyError> { # } ``` -In this example *index* handler will always return *500* response. But it is easy +In this example *index* handler always returns *500* response. But it is easy to return different responses for different type of errors. ```rust @@ -109,7 +109,7 @@ fn index(req: HttpRequest) -> Result<&'static str, MyError> { ## Error helpers Actix provides set of error helper types. It is possible to use them to generate -specific error responses. We can use helper types for first example with custom error. +specific error responses. We can use helper types for the first example with custom error. ```rust # extern crate actix_web; diff --git a/src/error.rs b/src/error.rs index 72ec56604..861669bfa 100644 --- a/src/error.rs +++ b/src/error.rs @@ -13,6 +13,7 @@ use http2::Error as Http2Error; use http::{header, StatusCode, Error as HttpError}; use http::uri::InvalidUri; use http_range::HttpRangeParseError; +use serde::de::value::Error as DeError; use serde_json::error::Error as JsonError; pub use url::ParseError as UrlParseError; @@ -109,6 +110,13 @@ impl ResponseError for JsonError {} /// `InternalServerError` for `UrlParseError` impl ResponseError for UrlParseError {} +/// Return `BAD_REQUEST` for `de::value::Error` +impl ResponseError for DeError { + fn error_response(&self) -> HttpResponse { + HttpResponse::new(StatusCode::BAD_REQUEST, Body::Empty) + } +} + /// Return `InternalServerError` for `HttpError`, /// Response generation can return `HttpError`, so it is internal error impl ResponseError for HttpError {} diff --git a/src/extractor.rs b/src/extractor.rs index 1dcc2cdf7..c2d2ebba9 100644 --- a/src/extractor.rs +++ b/src/extractor.rs @@ -1,11 +1,10 @@ use serde_urlencoded; use serde::de::{self, Deserializer, Visitor, Error as DeError}; -use error::{Error, ErrorBadRequest}; use httprequest::HttpRequest; pub trait HttpRequestExtractor<'de> { - fn extract(&self, req: &'de HttpRequest) -> Result + fn extract(&self, req: &'de HttpRequest) -> Result where T: de::Deserialize<'de>, S: 'static; } @@ -19,6 +18,7 @@ pub trait HttpRequestExtractor<'de> { /// # extern crate futures; /// #[macro_use] extern crate serde_derive; /// use actix_web::*; +/// use actix_web::dev::{Path, HttpRequestExtractor}; /// /// #[derive(Deserialize)] /// struct Info { @@ -26,7 +26,7 @@ pub trait HttpRequestExtractor<'de> { /// } /// /// fn index(mut req: HttpRequest) -> Result { -/// let info: Info = req.extract(Path)?; // <- extract path info using serde +/// let info: Info = Path.extract(&req)?; // <- extract path info using serde /// Ok(format!("Welcome {}!", info.username)) /// } /// @@ -40,11 +40,10 @@ pub struct Path; impl<'de> HttpRequestExtractor<'de> for Path { #[inline] - fn extract(&self, req: &'de HttpRequest) -> Result + fn extract(&self, req: &'de HttpRequest) -> Result where T: de::Deserialize<'de>, S: 'static, { - Ok(de::Deserialize::deserialize(PathExtractor{req: req}) - .map_err(ErrorBadRequest)?) + de::Deserialize::deserialize(PathExtractor{req: req}) } } @@ -58,6 +57,7 @@ impl<'de> HttpRequestExtractor<'de> for Path { /// # extern crate futures; /// #[macro_use] extern crate serde_derive; /// use actix_web::*; +/// use actix_web::dev::{Query, HttpRequestExtractor}; /// /// #[derive(Deserialize)] /// struct Info { @@ -65,7 +65,7 @@ impl<'de> HttpRequestExtractor<'de> for Path { /// } /// /// fn index(mut req: HttpRequest) -> Result { -/// let info: Info = req.extract(Query)?; // <- extract query info using serde +/// let info: Info = Query.extract(&req)?; // <- extract query info using serde /// Ok(format!("Welcome {}!", info.username)) /// } /// @@ -75,11 +75,10 @@ pub struct Query; impl<'de> HttpRequestExtractor<'de> for Query { #[inline] - fn extract(&self, req: &'de HttpRequest) -> Result + fn extract(&self, req: &'de HttpRequest) -> Result where T: de::Deserialize<'de>, S: 'static, { - Ok(serde_urlencoded::from_str::(req.query_string()) - .map_err(ErrorBadRequest)?) + serde_urlencoded::from_str::(req.query_string()) } } @@ -189,7 +188,6 @@ impl<'de, S: 'static> Deserializer<'de> for PathExtractor<'de, S> #[cfg(test)] mod tests { - use super::*; use router::{Router, Pattern}; use resource::Resource; use test::TestRequest; @@ -217,15 +215,15 @@ mod tests { let (router, _) = Router::new("", ServerSettings::default(), routes); assert!(router.recognize(&mut req).is_some()); - let s: MyStruct = req.extract(Path).unwrap(); + let s: MyStruct = req.extract_path().unwrap(); assert_eq!(s.key, "name"); assert_eq!(s.value, "user1"); - let s: (String, String) = req.extract(Path).unwrap(); + let s: (String, String) = req.extract_path().unwrap(); assert_eq!(s.0, "name"); assert_eq!(s.1, "user1"); - let s: Id = req.extract(Query).unwrap(); + let s: Id = req.extract_query().unwrap(); assert_eq!(s.id, "test"); } } diff --git a/src/httprequest.rs b/src/httprequest.rs index 3584ec52d..53f4e68fa 100644 --- a/src/httprequest.rs +++ b/src/httprequest.rs @@ -20,7 +20,7 @@ use payload::Payload; use httpmessage::HttpMessage; use httpresponse::{HttpResponse, HttpResponseBuilder}; use helpers::SharedHttpInnerMessage; -use extractor::HttpRequestExtractor; +use extractor::{Path, Query, HttpRequestExtractor}; use error::{Error, UrlGenerationError, CookieParseError, PayloadError}; @@ -383,6 +383,7 @@ impl HttpRequest { } /// Get a reference to the Params object. + /// /// Params is a container for url parameters. /// Route supports glob patterns: * for a single wildcard segment and :param /// for matching storing that segment of the request url in the Params object. @@ -397,14 +398,15 @@ impl HttpRequest { unsafe{ mem::transmute(&mut self.as_mut().params) } } - /// Extract typed information from path. + /// Extract typed information from request's path. + /// + /// By default, in case of error `BAD_REQUEST` response get returned to peer. + /// If you need to return different response use `map_err()` method. /// /// ## Example /// /// ```rust - /// # extern crate bytes; /// # extern crate actix_web; - /// # extern crate futures; /// #[macro_use] extern crate serde_derive; /// use actix_web::*; /// @@ -414,8 +416,7 @@ impl HttpRequest { /// } /// /// fn index(mut req: HttpRequest) -> Result { - /// let info: Info = req.extract(Path)?; // <- extract path info using serde - /// let info: Info = req.extract(Query)?; // <- extract query info + /// let info: Info = req.extract_path()?; // <- extract path info using serde /// Ok(format!("Welcome {}!", info.username)) /// } /// @@ -425,12 +426,60 @@ impl HttpRequest { /// |r| r.method(Method::GET).f(index)); /// } /// ``` - pub fn extract<'a, T, D>(&'a self, ds: D) -> Result + pub fn extract_path<'a, T>(&'a self) -> Result where S: 'static, T: de::Deserialize<'a>, - D: HttpRequestExtractor<'a> { - ds.extract(self) + Ok(Path.extract(self)?) + } + + /// Extract typed information from request's query string. + /// + /// ## Example + /// + /// ```rust + /// # extern crate actix_web; + /// #[macro_use] extern crate serde_derive; + /// use actix_web::{HttpRequest, Result}; + /// + /// #[derive(Deserialize)] + /// struct Info { + /// username: String, + /// } + /// + /// fn index(mut req: HttpRequest) -> Result { + /// let info: Info = req.extract_query()?; // <- extract query info, i.e: /?id=username + /// Ok(format!("Welcome {}!", info.username)) + /// } + /// # fn main() {} + /// ``` + /// + /// By default, in case of error, `BAD_REQUEST` response get returned to peer. + /// If you need to return different response use `map_err()` method. + /// + /// ```rust + /// # extern crate actix_web; + /// #[macro_use] extern crate serde_derive; + /// use actix_web::{HttpRequest, Result, error}; + /// + /// #[derive(Deserialize)] + /// struct Info { + /// username: String, + /// } + /// + /// fn index(mut req: HttpRequest) -> Result { + /// let info: Info = req.extract_query() // <- extract query information + /// .map_err(error::ErrorInternalServerError)?; // <- return 500 in case of error + /// Ok(format!("Welcome {}!", info.username)) + /// } + /// # fn main() {} + /// ``` + /// + pub fn extract_query<'a, T>(&'a self) -> Result + where S: 'static, + T: de::Deserialize<'a>, + { + Ok(Query.extract(self)?) } /// Checks if a connection should be kept alive. diff --git a/src/lib.rs b/src/lib.rs index 30428aed4..2e92c44fd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -144,7 +144,6 @@ pub use route::Route; pub use resource::Resource; pub use context::HttpContext; pub use server::HttpServer; -pub use extractor::{Path, Query}; // re-exports pub use http::{Method, StatusCode, Version}; @@ -190,5 +189,5 @@ pub mod dev { pub use param::{FromParam, Params}; pub use httpmessage::{UrlEncoded, MessageBody}; pub use httpresponse::HttpResponseBuilder; - pub use extractor::HttpRequestExtractor; + pub use extractor::{Path, Query, HttpRequestExtractor}; }