From a25a3c69c6ce5cf147fcc7848082eef04904e59c Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Thu, 13 Apr 2017 02:36:51 -0700 Subject: [PATCH] Cache parsed ContentType and Accept headers. This is a breaking change. `Request::content_type` now returns a borrow to `ContentType`. `FromRequest` for `ContentType` is no longer implemented. Instead, `FromRequest` for `&ContentType` is implemented. --- lib/Cargo.toml | 2 +- lib/src/data/from_data.rs | 2 +- lib/src/request/from_request.rs | 4 +-- lib/src/request/request.rs | 56 ++++++++++++++++++++------------- 4 files changed, 39 insertions(+), 25 deletions(-) diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 2f998332..a322a97d 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -25,7 +25,7 @@ url = "1" hyper = { version = "0.10.4", default-features = false } toml = { version = "0.2", default-features = false } num_cpus = "1" -state = "0.2" +state = "0.2.1" time = "0.1" memchr = "1" base64 = "0.4" diff --git a/lib/src/data/from_data.rs b/lib/src/data/from_data.rs index 74afe33a..57ad8203 100644 --- a/lib/src/data/from_data.rs +++ b/lib/src/data/from_data.rs @@ -104,7 +104,7 @@ impl<'a, S, E> IntoOutcome for Result { /// fn from_data(req: &Request, data: Data) -> data::Outcome { /// // Ensure the content type is correct before opening the data. /// let person_ct = ContentType::new("application", "x-person"); -/// if req.content_type() != Some(person_ct) { +/// if req.content_type() != Some(&person_ct) { /// return Outcome::Forward(data); /// } /// diff --git a/lib/src/request/from_request.rs b/lib/src/request/from_request.rs index 9d4b416b..0f1dc395 100644 --- a/lib/src/request/from_request.rs +++ b/lib/src/request/from_request.rs @@ -220,7 +220,7 @@ impl<'a, 'r> FromRequest<'a, 'r> for Session<'a> { } } -impl<'a, 'r> FromRequest<'a, 'r> for Accept { +impl<'a, 'r> FromRequest<'a, 'r> for &'a Accept { type Error = (); fn from_request(request: &'a Request<'r>) -> Outcome { @@ -231,7 +231,7 @@ impl<'a, 'r> FromRequest<'a, 'r> for Accept { } } -impl<'a, 'r> FromRequest<'a, 'r> for ContentType { +impl<'a, 'r> FromRequest<'a, 'r> for &'a ContentType { type Error = (); fn from_request(request: &'a Request<'r>) -> Outcome { diff --git a/lib/src/request/request.rs b/lib/src/request/request.rs index 07ce0101..d5df43ad 100644 --- a/lib/src/request/request.rs +++ b/lib/src/request/request.rs @@ -6,7 +6,7 @@ use std::str; use term_painter::Color::*; use term_painter::ToStyle; -use state::Container; +use state::{Container, Storage}; use error::Error; use super::{FromParam, FromSegments}; @@ -15,7 +15,6 @@ use router::Route; use http::uri::{URI, Segments}; use http::{Method, Header, HeaderMap, Cookies, Session, CookieJar, Key}; use http::{RawStr, ContentType, Accept, MediaType}; -use http::parse::media_type; use http::hyper; struct PresetState<'r> { @@ -28,6 +27,8 @@ struct RequestState<'r> { params: RefCell>, cookies: RefCell, session: RefCell, + accept: Storage>, + content_type: Storage>, } /// The type of an incoming web request. @@ -71,6 +72,8 @@ impl<'r> Request<'r> { params: RefCell::new(Vec::new()), cookies: RefCell::new(CookieJar::new()), session: RefCell::new(CookieJar::new()), + accept: Storage::new(), + content_type: Storage::new(), } } } @@ -238,11 +241,11 @@ impl<'r> Request<'r> { /// let mut request = Request::new(Method::Get, "/uri"); /// assert!(request.headers().is_empty()); /// - /// request.add_header(ContentType::HTML); - /// assert_eq!(request.content_type(), Some(ContentType::HTML)); + /// request.add_header(ContentType::Any); + /// assert_eq!(request.headers().get_one("Content-Type"), Some("*/*")); /// - /// request.replace_header(ContentType::JSON); - /// assert_eq!(request.content_type(), Some(ContentType::JSON)); + /// request.replace_header(ContentType::PNG); + /// assert_eq!(request.headers().get_one("Content-Type"), Some("image/png")); /// ``` #[inline(always)] pub fn replace_header>>(&mut self, header: H) { @@ -307,7 +310,9 @@ impl<'r> Request<'r> { } /// Returns `Some` of the Content-Type header of `self`. If the header is - /// not present, returns `None`. + /// not present, returns `None`. The Content-Type header is cached after the + /// first call to this function. As a result, subsequent calls will always + /// return the same value. /// /// # Example /// @@ -317,38 +322,47 @@ impl<'r> Request<'r> { /// /// let mut request = Request::new(Method::Get, "/uri"); /// assert_eq!(request.content_type(), None); + /// ``` /// - /// request.replace_header(ContentType::JSON); - /// assert_eq!(request.content_type(), Some(ContentType::JSON)); + /// ```rust + /// use rocket::Request; + /// use rocket::http::{Method, ContentType}; + /// + /// let mut request = Request::new(Method::Get, "/uri"); + /// request.add_header(ContentType::JSON); + /// assert_eq!(request.content_type(), Some(&ContentType::JSON)); /// ``` #[inline(always)] - pub fn content_type(&self) -> Option { - // FIXME: Don't reparse each time! Use RC? Smarter than that? - // Use state::Storage! - self.headers().get_one("Content-Type").and_then(|value| value.parse().ok()) + pub fn content_type(&self) -> Option<&ContentType> { + self.extra.content_type.get_or_set(|| { + self.headers().get_one("Content-Type").and_then(|v| v.parse().ok()) + }).as_ref() } #[inline(always)] - pub fn accept(&self) -> Option { - self.headers().get_one("Accept").and_then(|v| v.parse().ok()) + pub fn accept(&self) -> Option<&Accept> { + self.extra.accept.get_or_set(|| { + self.headers().get_one("Accept").and_then(|v| v.parse().ok()) + }).as_ref() } #[inline(always)] - pub fn accept_first(&self) -> Option { - self.headers().get_one("Accept").and_then(|mut v| media_type(&mut v).ok()) + pub fn accept_first(&self) -> Option<&MediaType> { + self.accept().and_then(|accept| accept.first()).map(|wmt| wmt.media_type()) } #[inline(always)] - pub fn format(&self) -> Option { + pub fn format(&self) -> Option<&MediaType> { + static ANY: MediaType = MediaType::Any; if self.method.supports_payload() { - self.content_type().map(|ct| ct.into_media_type()) + self.content_type().map(|ct| ct.media_type()) } else { // FIXME: Should we be using `accept_first` or `preferred`? Or // should we be checking neither and instead pass things through // where the client accepts the thing at all? self.accept() - .map(|accept| accept.preferred().media_type().clone()) - .or(Some(MediaType::Any)) + .map(|accept| accept.preferred().media_type()) + .or(Some(&ANY)) } }