From 48e05a2d872b37b5463d9a78c9b3e487813a16c1 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Mon, 30 Apr 2018 22:04:24 -0700 Subject: [PATCH] add nested scope support --- src/application.rs | 11 +-- src/httprequest.rs | 46 ++++++++----- src/param.rs | 16 +++++ src/router.rs | 1 + src/scope.rs | 146 ++++++++++++++++++++++++++++++++-------- src/server/h1decoder.rs | 8 ++- tests/test_client.rs | 1 + 7 files changed, 176 insertions(+), 53 deletions(-) diff --git a/src/application.rs b/src/application.rs index 0b1e0ec1a..bd6c4d007 100644 --- a/src/application.rs +++ b/src/application.rs @@ -69,9 +69,11 @@ impl HttpApplication { }; if m { - let path: &'static str = unsafe { - &*(&req.path()[inner.prefix + prefix.len()..] as *const _) - }; + let prefix_len = inner.prefix + prefix.len(); + let path: &'static str = + unsafe { &*(&req.path()[prefix_len..] as *const _) }; + + req.set_prefix_len(prefix_len as u16); if path.is_empty() { req.match_info_mut().add("tail", "/"); } else { @@ -881,8 +883,9 @@ mod tests { ); let req = TestRequest::with_uri("/app/test").finish(); - let resp = app.run(req); + let resp = app.run(req.clone()); assert_eq!(resp.as_response().unwrap().status(), StatusCode::OK); + assert_eq!(req.prefix_len(), 9); let req = TestRequest::with_uri("/app/test/").finish(); let resp = app.run(req); diff --git a/src/httprequest.rs b/src/httprequest.rs index ad7864a7c..5b3c6619d 100644 --- a/src/httprequest.rs +++ b/src/httprequest.rs @@ -25,20 +25,27 @@ use router::{Resource, Router}; use server::helpers::SharedHttpInnerMessage; use uri::Url as InnerUrl; +bitflags! { + pub(crate) struct MessageFlags: u8 { + const QUERY = 0b0000_0001; + const KEEPALIVE = 0b0000_0010; + } +} + pub struct HttpInnerMessage { pub version: Version, pub method: Method, pub(crate) url: InnerUrl, + pub(crate) flags: MessageFlags, pub headers: HeaderMap, pub extensions: Extensions, pub params: Params<'static>, pub cookies: Option>>, pub query: Params<'static>, - pub query_loaded: bool, pub addr: Option, pub payload: Option, pub info: Option>, - pub keep_alive: bool, + pub prefix: u16, resource: RouterResource, } @@ -55,15 +62,15 @@ impl Default for HttpInnerMessage { url: InnerUrl::default(), version: Version::HTTP_11, headers: HeaderMap::with_capacity(16), + flags: MessageFlags::empty(), params: Params::new(), query: Params::new(), - query_loaded: false, addr: None, cookies: None, payload: None, extensions: Extensions::new(), info: None, - keep_alive: true, + prefix: 0, resource: RouterResource::Notset, } } @@ -73,7 +80,7 @@ impl HttpInnerMessage { /// Checks if a connection should be kept alive. #[inline] pub fn keep_alive(&self) -> bool { - self.keep_alive + self.flags.contains(MessageFlags::KEEPALIVE) } #[inline] @@ -83,10 +90,10 @@ impl HttpInnerMessage { self.params.clear(); self.addr = None; self.info = None; - self.query_loaded = false; + self.flags = MessageFlags::empty(); self.cookies = None; self.payload = None; - self.keep_alive = true; + self.prefix = 0; self.resource = RouterResource::Notset; } } @@ -115,12 +122,12 @@ impl HttpRequest<()> { payload, params: Params::new(), query: Params::new(), - query_loaded: false, extensions: Extensions::new(), cookies: None, addr: None, info: None, - keep_alive: true, + prefix: 0, + flags: MessageFlags::empty(), resource: RouterResource::Notset, }), None, @@ -234,12 +241,13 @@ impl HttpRequest { } #[doc(hidden)] - pub fn prefix_len(&self) -> usize { - if let Some(router) = self.router() { - router.prefix().len() - } else { - 0 - } + pub fn prefix_len(&self) -> u16 { + self.as_ref().prefix as u16 + } + + #[doc(hidden)] + pub fn set_prefix_len(&mut self, len: u16) { + self.as_mut().prefix = len; } /// Read the Request Uri. @@ -367,14 +375,16 @@ impl HttpRequest { self.as_mut().addr = addr; } + #[doc(hidden)] + #[deprecated(since = "0.6.0", note = "please use `Query` extractor")] /// Get a reference to the Params object. /// Params is a container for url query parameters. pub fn query(&self) -> &Params { - if !self.as_ref().query_loaded { + if !self.as_ref().flags.contains(MessageFlags::QUERY) { + self.as_mut().flags.insert(MessageFlags::QUERY); let params: &mut Params = unsafe { mem::transmute(&mut self.as_mut().query) }; params.clear(); - self.as_mut().query_loaded = true; for (key, val) in form_urlencoded::parse(self.query_string().as_ref()) { params.add(key, val); } @@ -443,7 +453,7 @@ impl HttpRequest { /// Checks if a connection should be kept alive. pub fn keep_alive(&self) -> bool { - self.as_ref().keep_alive() + self.as_ref().flags.contains(MessageFlags::KEEPALIVE) } /// Check if request requires connection upgrade diff --git a/src/param.rs b/src/param.rs index 41100763d..386c0c338 100644 --- a/src/param.rs +++ b/src/param.rs @@ -42,6 +42,22 @@ impl<'a> Params<'a> { self.0.push((name.into(), value.into())); } + pub(crate) fn set(&mut self, name: N, value: V) + where + N: Into>, + V: Into>, + { + let name = name.into(); + let value = value.into(); + for item in &mut self.0 { + if item.0 == name { + item.1 = value; + return; + } + } + self.0.push((name, value)); + } + /// Check if there are any matched patterns pub fn is_empty(&self) -> bool { self.0.is_empty() diff --git a/src/router.rs b/src/router.rs index dd29e4bcc..6c5a7da6c 100644 --- a/src/router.rs +++ b/src/router.rs @@ -84,6 +84,7 @@ impl Router { for (idx, pattern) in self.0.patterns.iter().enumerate() { if pattern.match_with_params(route_path, req.match_info_mut()) { req.set_resource(idx); + req.set_prefix_len(self.0.prefix_len as u16); return Some(idx); } } diff --git a/src/scope.rs b/src/scope.rs index 6f730e91a..ff630e680 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -14,6 +14,7 @@ use middleware::{Finished as MiddlewareFinished, Middleware, use resource::ResourceHandler; use router::Resource; +type Route = UnsafeCell>>; type ScopeResources = Rc>>)>>; /// Resources scope @@ -42,7 +43,7 @@ type ScopeResources = Rc>>)>> /// * /app/path3 - `HEAD` requests /// pub struct Scope { - handler: Option>>>, + nested: Vec<(String, Route)>, middlewares: Rc>>>, default: Rc>>, resources: ScopeResources, @@ -57,17 +58,14 @@ impl Default for Scope { impl Scope { pub fn new() -> Scope { Scope { - handler: None, + nested: Vec::new(), resources: Rc::new(Vec::new()), middlewares: Rc::new(Vec::new()), default: Rc::new(UnsafeCell::new(ResourceHandler::default_not_found())), } } - /// Create scope with new state. - /// - /// Scope can have only one nested scope with new state. Every call - /// destroys previously created scope with state. + /// Create nested scope with new state. /// /// ```rust /// # extern crate actix_web; @@ -82,28 +80,78 @@ impl Scope { /// fn main() { /// let app = App::new() /// .scope("/app", |scope| { - /// scope.with_state(AppState, |scope| { + /// scope.with_state("/state2", AppState, |scope| { /// scope.resource("/test1", |r| r.f(index)) /// }) /// }); /// } /// ``` - pub fn with_state(mut self, state: T, f: F) -> Scope + pub fn with_state(mut self, path: &str, state: T, f: F) -> Scope where F: FnOnce(Scope) -> Scope, { let scope = Scope { - handler: None, + nested: Vec::new(), resources: Rc::new(Vec::new()), middlewares: Rc::new(Vec::new()), default: Rc::new(UnsafeCell::new(ResourceHandler::default_not_found())), }; let scope = f(scope); - self.handler = Some(UnsafeCell::new(Box::new(Wrapper { + let mut path = path.trim().trim_right_matches('/').to_owned(); + if !path.is_empty() && !path.starts_with('/') { + path.insert(0, '/') + } + + let handler = UnsafeCell::new(Box::new(Wrapper { scope, state: Rc::new(state), - }))); + })); + self.nested.push((path, handler)); + + self + } + + /// Create nested scope. + /// + /// ```rust + /// # extern crate actix_web; + /// use actix_web::{App, HttpRequest}; + /// + /// struct AppState; + /// + /// fn index(req: HttpRequest) -> &'static str { + /// "Welcome!" + /// } + /// + /// fn main() { + /// let app = App::with_state(AppState) + /// .scope("/app", |scope| { + /// scope.nested("/v1", |scope| { + /// scope.resource("/test1", |r| r.f(index)) + /// }) + /// }); + /// } + /// ``` + pub fn nested(mut self, path: &str, f: F) -> Scope + where + F: FnOnce(Scope) -> Scope, + { + let scope = Scope { + nested: Vec::new(), + resources: Rc::new(Vec::new()), + middlewares: Rc::new(Vec::new()), + default: Rc::new(UnsafeCell::new(ResourceHandler::default_not_found())), + }; + let scope = f(scope); + + let mut path = path.trim().trim_right_matches('/').to_owned(); + if !path.is_empty() && !path.starts_with('/') { + path.insert(0, '/') + } + + self.nested + .push((path, UnsafeCell::new(Box::new(scope)))); self } @@ -249,24 +297,46 @@ impl RouteHandler for Scope { } } - // nested scope - if let Some(ref handler) = self.handler { - let hnd: &mut RouteHandler<_> = unsafe { (&mut *(handler.get())).as_mut() }; - hnd.handle(req) - } else { - // default handler - let default = unsafe { &mut *self.default.as_ref().get() }; - if self.middlewares.is_empty() { - default.handle(req, None) - } else { - Reply::async(Compose::new( - req, - Rc::clone(&self.middlewares), - Rc::clone(&self.default), - None, - )) + // nested scopes + for &(ref prefix, ref handler) in &self.nested { + let len = req.prefix_len() as usize; + let m = { + let path = &req.path()[len..]; + path.starts_with(prefix) + && (path.len() == prefix.len() + || path.split_at(prefix.len()).1.starts_with('/')) + }; + + if m { + let prefix_len = len + prefix.len(); + let path: &'static str = + unsafe { &*(&req.path()[prefix_len..] as *const _) }; + + req.set_prefix_len(prefix_len as u16); + if path.is_empty() { + req.match_info_mut().set("tail", "/"); + } else { + req.match_info_mut().set("tail", path); + } + + let hnd: &mut RouteHandler<_> = + unsafe { (&mut *(handler.get())).as_mut() }; + return hnd.handle(req); } } + + // default handler + let default = unsafe { &mut *self.default.as_ref().get() }; + if self.middlewares.is_empty() { + default.handle(req, None) + } else { + Reply::async(Compose::new( + req, + Rc::clone(&self.middlewares), + Rc::clone(&self.default), + None, + )) + } } } @@ -646,13 +716,31 @@ mod tests { let mut app = App::new() .scope("/app", |scope| { - scope.with_state(State, |scope| { + scope.with_state("/t1", State, |scope| { scope.resource("/path1", |r| r.f(|_| HttpResponse::Created())) }) }) .finish(); - let req = TestRequest::with_uri("/app/path1").finish(); + let req = TestRequest::with_uri("/app/t1/path1").finish(); + let resp = app.run(req); + assert_eq!( + resp.as_response().unwrap().status(), + StatusCode::CREATED + ); + } + + #[test] + fn test_nested_scope() { + let mut app = App::new() + .scope("/app", |scope| { + scope.nested("/t1", |scope| { + scope.resource("/path1", |r| r.f(|_| HttpResponse::Created())) + }) + }) + .finish(); + + let req = TestRequest::with_uri("/app/t1/path1").finish(); let resp = app.run(req); assert_eq!( resp.as_response().unwrap().status(), diff --git a/src/server/h1decoder.rs b/src/server/h1decoder.rs index 7ff6e8b92..375923d06 100644 --- a/src/server/h1decoder.rs +++ b/src/server/h1decoder.rs @@ -9,6 +9,7 @@ use super::settings::WorkerSettings; use error::ParseError; use http::header::{HeaderName, HeaderValue}; use http::{header, HttpTryFrom, Method, Uri, Version}; +use httprequest::MessageFlags; use uri::Url; const MAX_BUFFER_SIZE: usize = 131_072; @@ -128,7 +129,9 @@ impl H1Decoder { let msg = settings.get_http_message(); { let msg_mut = msg.get_mut(); - msg_mut.keep_alive = version != Version::HTTP_10; + msg_mut + .flags + .set(MessageFlags::KEEPALIVE, version != Version::HTTP_10); for header in headers[..headers_len].iter() { if let Ok(name) = HeaderName::from_bytes(header.name.as_bytes()) { @@ -165,7 +168,7 @@ impl H1Decoder { } // connection keep-alive state header::CONNECTION => { - msg_mut.keep_alive = if let Ok(conn) = value.to_str() { + let ka = if let Ok(conn) = value.to_str() { if version == Version::HTTP_10 && conn.contains("keep-alive") { @@ -178,6 +181,7 @@ impl H1Decoder { } else { false }; + msg_mut.flags.set(MessageFlags::KEEPALIVE, ka); } _ => (), } diff --git a/tests/test_client.rs b/tests/test_client.rs index 3c4c85861..094656840 100644 --- a/tests/test_client.rs +++ b/tests/test_client.rs @@ -1,3 +1,4 @@ +#![allow(deprecated)] extern crate actix; extern crate actix_web; extern crate bytes;