add variable segments support for scope prefix

src/application.rs

@@ -29,7 +29,12 @@ pub(crate) struct Inner<S> {
     default: ResourceHandler<S>,
     encoding: ContentEncoding,
     resources: Vec<ResourceHandler<S>>,
-    handlers: Vec<(String, Box<RouteHandler<S>>)>,
+    handlers: Vec<PrefixHandlerType<S>>,
+}
+
+enum PrefixHandlerType<S> {
+    Handler(String, Box<RouteHandler<S>>),
+    Scope(Resource, Box<RouteHandler<S>>),
 }
 
 impl<S: 'static> PipelineHandler<S> for Inner<S> {
@@ -44,7 +49,10 @@ impl<S: 'static> PipelineHandler<S> for Inner<S> {
             HandlerType::Normal(idx) => {
                 self.resources[idx].handle(req, Some(&mut self.default))
             }
-            HandlerType::Handler(idx) => self.handlers[idx].1.handle(req),
+            HandlerType::Handler(idx) => match self.handlers[idx] {
+                PrefixHandlerType::Handler(_, ref mut hnd) => hnd.handle(req),
+                PrefixHandlerType::Scope(_, ref mut hnd) => hnd.handle(req),
+            },
             HandlerType::Default => self.default.handle(req, None),
         }
     }
@@ -62,12 +70,15 @@ impl<S: 'static> HttpApplication<S> {
             HandlerType::Normal(idx)
         } else {
             let inner = self.as_ref();
+            let path: &'static str =
+                unsafe { &*(&req.path()[inner.prefix..] as *const _) };
+            let path_len = path.len();
             for idx in 0..inner.handlers.len() {
-                let &(ref prefix, _) = &inner.handlers[idx];
+                match &inner.handlers[idx] {
+                    PrefixHandlerType::Handler(ref prefix, _) => {
                         let m = {
-                    let path = &req.path()[inner.prefix..];
                             path.starts_with(prefix)
-                        && (path.len() == prefix.len()
+                                && (path_len == prefix.len()
                                     || path.split_at(prefix.len()).1.starts_with('/'))
                         };
 
@@ -85,6 +96,25 @@ impl<S: 'static> HttpApplication<S> {
                             return HandlerType::Handler(idx);
                         }
                     }
+                    PrefixHandlerType::Scope(ref pattern, _) => {
+                        if let Some(prefix_len) =
+                            pattern.match_prefix_with_params(path, req.match_info_mut())
+                        {
+                            let prefix_len = inner.prefix + prefix_len - 1;
+                            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);
+                            }
+                            return HandlerType::Handler(idx);
+                        }
+                    }
+                }
+            }
             HandlerType::Default
         }
     }
@@ -131,7 +161,7 @@ struct ApplicationParts<S> {
     settings: ServerSettings,
     default: ResourceHandler<S>,
     resources: Vec<(Resource, Option<ResourceHandler<S>>)>,
-    handlers: Vec<(String, Box<RouteHandler<S>>)>,
+    handlers: Vec<PrefixHandlerType<S>>,
     external: HashMap<String, Resource>,
     encoding: ContentEncoding,
     middlewares: Vec<Box<Middleware<S>>>,
@@ -305,7 +335,7 @@ where
     /// Configure scope for common root path.
     ///
     /// Scopes collect multiple paths under a common path prefix.
-    /// Scope path can not contain variable path segments as resources.
+    /// Scope path can contain variable path segments as resources.
     ///
     /// ```rust
     /// # extern crate actix_web;
@@ -313,7 +343,7 @@ where
     ///
     /// fn main() {
     ///     let app = App::new()
-    ///         .scope("/app", |scope| {
+    ///         .scope("/{project_id}", |scope| {
     ///              scope.resource("/path1", |r| r.f(|_| HttpResponse::Ok()))
     ///                .resource("/path2", |r| r.f(|_| HttpResponse::Ok()))
     ///                .resource("/path3", |r| r.f(|_| HttpResponse::MethodNotAllowed()))
@@ -322,9 +352,9 @@ where
     /// ```
     ///
     /// In the above example, three routes get added:
-    ///  * /app/path1
+    ///  * /{project_id}/path1
-    ///  * /app/path2
+    ///  * /{project_id}/path2
-    ///  * /app/path3
+    ///  * /{project_id}/path3
     ///
     pub fn scope<F>(mut self, path: &str, f: F) -> App<S>
     where
@@ -337,9 +367,15 @@ where
             if !path.is_empty() && !path.starts_with('/') {
                 path.insert(0, '/')
             }
+            if !path.ends_with('/') {
+                path.push('/');
+            }
             let parts = self.parts.as_mut().expect("Use after finish");
 
-            parts.handlers.push((path, scope));
+            parts.handlers.push(PrefixHandlerType::Scope(
+                Resource::prefix("", &path),
+                scope,
+            ));
         }
         self
     }
@@ -496,11 +532,15 @@ where
             if !path.is_empty() && !path.starts_with('/') {
                 path.insert(0, '/')
             }
+            if path.len() > 1 && path.ends_with('/') {
+                path.pop();
+            }
             let parts = self.parts.as_mut().expect("Use after finish");
 
-            parts
+            parts.handlers.push(PrefixHandlerType::Handler(
-                .handlers
+                path,
-                .push((path, Box::new(WrapHandler::new(handler))));
+                Box::new(WrapHandler::new(handler)),
+            ));
         }
         self
     }

src/router.rs

@@ -142,7 +142,8 @@ enum PatternElement {
 #[derive(Clone, Debug)]
 enum PatternType {
     Static(String),
-    Dynamic(Regex, Vec<String>),
+    Prefix(String),
+    Dynamic(Regex, Vec<String>, usize),
 }
 
 #[derive(Debug, Copy, Clone, PartialEq)]
@@ -173,14 +174,23 @@ impl Resource {
     ///
     /// Panics if path pattern is wrong.
     pub fn new(name: &str, path: &str) -> Self {
-        Resource::with_prefix(name, path, "/")
+        Resource::with_prefix(name, path, "/", false)
+    }
+
+    /// Parse path pattern and create new `Resource` instance.
+    ///
+    /// Use `prefix` type instead of `static`.
+    ///
+    /// Panics if path regex pattern is wrong.
+    pub fn prefix(name: &str, path: &str) -> Self {
+        Resource::with_prefix(name, path, "/", true)
     }
 
     /// Construct external resource
     ///
     /// Panics if path pattern is wrong.
     pub fn external(name: &str, path: &str) -> Self {
-        let mut resource = Resource::with_prefix(name, path, "/");
+        let mut resource = Resource::with_prefix(name, path, "/", false);
         resource.rtp = ResourceType::External;
         resource
     }
@@ -197,8 +207,9 @@ impl Resource {
     }
 
     /// Parse path pattern and create new `Resource` instance with custom prefix
-    pub fn with_prefix(name: &str, path: &str, prefix: &str) -> Self {
+    pub fn with_prefix(name: &str, path: &str, prefix: &str, for_prefix: bool) -> Self {
-        let (pattern, elements, is_dynamic) = Resource::parse(path, prefix);
+        let (pattern, elements, is_dynamic, len) =
+            Resource::parse(path, prefix, for_prefix);
 
         let tp = if is_dynamic {
             let re = match Regex::new(&pattern) {
@@ -208,7 +219,9 @@ impl Resource {
             let names = re.capture_names()
                 .filter_map(|name| name.map(|name| name.to_owned()))
                 .collect();
-            PatternType::Dynamic(re, names)
+            PatternType::Dynamic(re, names, len)
+        } else if for_prefix {
+            PatternType::Prefix(pattern.clone())
         } else {
             PatternType::Static(pattern.clone())
         };
@@ -240,7 +253,8 @@ impl Resource {
     pub fn is_match(&self, path: &str) -> bool {
         match self.tp {
             PatternType::Static(ref s) => s == path,
-            PatternType::Dynamic(ref re, _) => re.is_match(path),
+            PatternType::Dynamic(ref re, _, _) => re.is_match(path),
+            PatternType::Prefix(ref s) => path.starts_with(s),
         }
     }
 
@@ -249,7 +263,7 @@ impl Resource {
     ) -> bool {
         match self.tp {
             PatternType::Static(ref s) => s == path,
-            PatternType::Dynamic(ref re, ref names) => {
+            PatternType::Dynamic(ref re, ref names, _) => {
                 if let Some(captures) = re.captures(path) {
                     let mut idx = 0;
                     for capture in captures.iter() {
@@ -265,6 +279,42 @@ impl Resource {
                     false
                 }
             }
+            PatternType::Prefix(ref s) => path.starts_with(s),
+        }
+    }
+
+    pub fn match_prefix_with_params<'a>(
+        &'a self, path: &'a str, params: &'a mut Params<'a>,
+    ) -> Option<usize> {
+        match self.tp {
+            PatternType::Static(ref s) => if s == path {
+                Some(s.len())
+            } else {
+                None
+            },
+            PatternType::Dynamic(ref re, ref names, len) => {
+                if let Some(captures) = re.captures(path) {
+                    let mut idx = 0;
+                    let mut pos = 0;
+                    for capture in captures.iter() {
+                        if let Some(ref m) = capture {
+                            if idx != 0 {
+                                params.add(names[idx - 1].as_str(), m.as_str());
+                            }
+                            idx += 1;
+                            pos = m.end();
+                        }
+                    }
+                    Some(pos + len)
+                } else {
+                    None
+                }
+            }
+            PatternType::Prefix(ref s) => if path.starts_with(s) {
+                Some(s.len())
+            } else {
+                None
+            },
         }
     }
 
@@ -297,7 +347,9 @@ impl Resource {
         Ok(path)
     }
 
-    fn parse(pattern: &str, prefix: &str) -> (String, Vec<PatternElement>, bool) {
+    fn parse(
+        pattern: &str, prefix: &str, for_prefix: bool,
+    ) -> (String, Vec<PatternElement>, bool, usize) {
         const DEFAULT_PATTERN: &str = "[^/]+";
 
         let mut re1 = String::from("^") + prefix;
@@ -309,6 +361,7 @@ impl Resource {
         let mut param_pattern = String::from(DEFAULT_PATTERN);
         let mut is_dynamic = false;
         let mut elems = Vec::new();
+        let mut len = 0;
 
         for (index, ch) in pattern.chars().enumerate() {
             // All routes must have a leading slash so its optional to have one
@@ -325,6 +378,7 @@ impl Resource {
                     param_name.clear();
                     param_pattern = String::from(DEFAULT_PATTERN);
 
+                    len = 0;
                     in_param_pattern = false;
                     in_param = false;
                 } else if ch == ':' {
@@ -348,16 +402,19 @@ impl Resource {
                 re1.push_str(escape(&ch.to_string()).as_str());
                 re2.push(ch);
                 el.push(ch);
+                len += 1;
             }
         }
 
         let re = if is_dynamic {
+            if !for_prefix {
                 re1.push('$');
+            }
             re1
         } else {
             re2
         };
-        (re, elems, is_dynamic)
+        (re, elems, is_dynamic, len)
     }
 }
 
@@ -570,6 +627,41 @@ mod tests {
         assert_eq!(req.match_info().get("id").unwrap(), "adahg32");
     }
 
+    #[test]
+    fn test_resource_prefix() {
+        let re = Resource::prefix("test", "/name");
+        assert!(re.is_match("/name"));
+        assert!(re.is_match("/name/"));
+        assert!(re.is_match("/name/test/test"));
+        assert!(re.is_match("/name1"));
+        assert!(re.is_match("/name~"));
+
+        let re = Resource::prefix("test", "/name/");
+        assert!(re.is_match("/name/"));
+        assert!(re.is_match("/name/gs"));
+        assert!(!re.is_match("/name"));
+    }
+
+    #[test]
+    fn test_reousrce_prefix_dynamic() {
+        let re = Resource::prefix("test", "/{name}/");
+        assert!(re.is_match("/name/"));
+        assert!(re.is_match("/name/gs"));
+        assert!(!re.is_match("/name"));
+
+        let mut req = TestRequest::with_uri("/test2/").finish();
+        assert!(re.match_with_params("/test2/", req.match_info_mut()));
+        assert_eq!(&req.match_info()["name"], "test2");
+
+        let mut req =
+            TestRequest::with_uri("/test2/subpath1/subpath2/index.html").finish();
+        assert!(re.match_with_params(
+            "/test2/subpath1/subpath2/index.html",
+            req.match_info_mut()
+        ));
+        assert_eq!(&req.match_info()["name"], "test2");
+    }
+
     #[test]
     fn test_request_resource() {
         let routes = vec![

src/scope.rs

@@ -21,7 +21,12 @@ type ScopeResources<S> = Rc<Vec<(Resource, Rc<UnsafeCell<ResourceHandler<S>>>)>>
 ///
 /// Scope is a set of resources with common root path.
 /// Scopes collect multiple paths under a common path prefix.
-/// Scope path can not contain variable path segments as resources.
+/// Scope path can contain variable path segments as resources.
+/// Scope prefix is always complete path segment, i.e `/app` would
+/// be converted to a `/app/` and it would not match `/app` path.
+///
+/// You can use variable path segments with `Path` extractor, also variable
+/// segments are available in `HttpRequest::match_info()`.
 ///
 /// ```rust
 /// # extern crate actix_web;
@@ -29,7 +34,7 @@ type ScopeResources<S> = Rc<Vec<(Resource, Rc<UnsafeCell<ResourceHandler<S>>>)>>
 ///
 /// fn main() {
 ///     let app = App::new()
-///         .scope("/app", |scope| {
+///         .scope("/{project_id}/", |scope| {
 ///              scope.resource("/path1", |r| r.f(|_| HttpResponse::Ok()))
 ///                .resource("/path2", |r| r.f(|_| HttpResponse::Ok()))
 ///                .resource("/path3", |r| r.f(|_| HttpResponse::MethodNotAllowed()))
@@ -38,12 +43,12 @@ type ScopeResources<S> = Rc<Vec<(Resource, Rc<UnsafeCell<ResourceHandler<S>>>)>>
 /// ```
 ///
 /// In the above example three routes get registered:
-///  * /app/path1 - reponds to all http method
+///  * /{project_id}/path1 - reponds to all http method
-///  * /app/path2 - `GET` requests
+///  * /{project_id}/path2 - `GET` requests
-///  * /app/path3 - `HEAD` requests
+///  * /{project_id}/path3 - `HEAD` requests
 ///
 pub struct Scope<S: 'static> {
-    nested: Vec<(String, Route<S>)>,
+    nested: Vec<(Resource, Route<S>)>,
     middlewares: Rc<Vec<Box<Middleware<S>>>>,
     default: Rc<UnsafeCell<ResourceHandler<S>>>,
     resources: ScopeResources<S>,
@@ -102,12 +107,16 @@ impl<S: 'static> Scope<S> {
         if !path.is_empty() && !path.starts_with('/') {
             path.insert(0, '/')
         }
+        if !path.ends_with('/') {
+            path.push('/');
+        }
 
         let handler = UnsafeCell::new(Box::new(Wrapper {
             scope,
             state: Rc::new(state),
         }));
-        self.nested.push((path, handler));
+        self.nested
+            .push((Resource::prefix("", &path), handler));
 
         self
     }
@@ -149,9 +158,14 @@ impl<S: 'static> Scope<S> {
         if !path.is_empty() && !path.starts_with('/') {
             path.insert(0, '/')
         }
+        if !path.ends_with('/') {
+            path.push('/');
+        }
 
-        self.nested
+        self.nested.push((
-            .push((path, UnsafeCell::new(Box::new(scope))));
+            Resource::prefix("", &path),
+            UnsafeCell::new(Box::new(scope)),
+        ));
 
         self
     }
@@ -298,17 +312,14 @@ impl<S: 'static> RouteHandler<S> for Scope<S> {
         }
 
         // nested scopes
-        for &(ref prefix, ref handler) in &self.nested {
         let len = req.prefix_len() as usize;
-            let m = {
+        let path: &'static str = unsafe { &*(&req.path()[len..] as *const _) };
-                let path = &req.path()[len..];
-                path.starts_with(prefix)
-                    && (path.len() == prefix.len()
-                        || path.split_at(prefix.len()).1.starts_with('/'))
-            };
 
-            if m {
+        for &(ref prefix, ref handler) in &self.nested {
-                let prefix_len = len + prefix.len();
+            if let Some(prefix_len) =
+                prefix.match_prefix_with_params(path, req.match_info_mut())
+            {
+                let prefix_len = len + prefix_len - 1;
                 let path: &'static str =
                     unsafe { &*(&req.path()[prefix_len..] as *const _) };
 
@@ -698,7 +709,10 @@ impl<S: 'static> Response<S> {
 
 #[cfg(test)]
 mod tests {
+    use bytes::Bytes;
+
     use application::App;
+    use body::Body;
     use http::StatusCode;
     use httpresponse::HttpResponse;
     use test::TestRequest;
@@ -716,6 +730,36 @@ mod tests {
         assert_eq!(resp.as_msg().status(), StatusCode::OK);
     }
 
+    #[test]
+    fn test_scope_variable_segment() {
+        let mut app = App::new()
+            .scope("/ab-{project}", |scope| {
+                scope.resource("/path1", |r| {
+                    r.f(|r| {
+                        HttpResponse::Ok()
+                            .body(format!("project: {}", &r.match_info()["project"]))
+                    })
+                })
+            })
+            .finish();
+
+        let req = TestRequest::with_uri("/ab-project1/path1").finish();
+        let resp = app.run(req);
+        assert_eq!(resp.as_msg().status(), StatusCode::OK);
+
+        match resp.as_msg().body() {
+            Body::Binary(ref b) => {
+                let bytes: Bytes = b.clone().into();
+                assert_eq!(bytes, Bytes::from_static(b"project: project1"));
+            }
+            _ => panic!(),
+        }
+
+        let req = TestRequest::with_uri("/aa-project1/path1").finish();
+        let resp = app.run(req);
+        assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND);
+    }
+
     #[test]
     fn test_scope_with_state() {
         struct State;
@@ -748,6 +792,73 @@ mod tests {
         assert_eq!(resp.as_msg().status(), StatusCode::CREATED);
     }
 
+    #[test]
+    fn test_nested_scope_with_variable_segment() {
+        let mut app = App::new()
+            .scope("/app", |scope| {
+                scope.nested("/{project_id}", |scope| {
+                    scope.resource("/path1", |r| {
+                        r.f(|r| {
+                            HttpResponse::Created().body(format!(
+                                "project: {}",
+                                &r.match_info()["project_id"]
+                            ))
+                        })
+                    })
+                })
+            })
+            .finish();
+
+        let req = TestRequest::with_uri("/app/project_1/path1").finish();
+        let resp = app.run(req);
+        assert_eq!(resp.as_msg().status(), StatusCode::CREATED);
+
+        match resp.as_msg().body() {
+            Body::Binary(ref b) => {
+                let bytes: Bytes = b.clone().into();
+                assert_eq!(bytes, Bytes::from_static(b"project: project_1"));
+            }
+            _ => panic!(),
+        }
+    }
+
+    #[test]
+    fn test_nested2_scope_with_variable_segment() {
+        let mut app = App::new()
+            .scope("/app", |scope| {
+                scope.nested("/{project}", |scope| {
+                    scope.nested("/{id}", |scope| {
+                        scope.resource("/path1", |r| {
+                            r.f(|r| {
+                                HttpResponse::Created().body(format!(
+                                    "project: {} - {}",
+                                    &r.match_info()["project"],
+                                    &r.match_info()["id"],
+                                ))
+                            })
+                        })
+                    })
+                })
+            })
+            .finish();
+
+        let req = TestRequest::with_uri("/app/test/1/path1").finish();
+        let resp = app.run(req);
+        assert_eq!(resp.as_msg().status(), StatusCode::CREATED);
+
+        match resp.as_msg().body() {
+            Body::Binary(ref b) => {
+                let bytes: Bytes = b.clone().into();
+                assert_eq!(bytes, Bytes::from_static(b"project: test - 1"));
+            }
+            _ => panic!(),
+        }
+
+        let req = TestRequest::with_uri("/app/test/1/path2").finish();
+        let resp = app.run(req);
+        assert_eq!(resp.as_msg().status(), StatusCode::NOT_FOUND);
+    }
+
     #[test]
     fn test_default_resource() {
         let mut app = App::new()

src/ws/context.rs

@@ -13,9 +13,9 @@ use context::{ActorHttpContext, Drain, Frame as ContextFrame};
 use error::{Error, ErrorInternalServerError};
 use httprequest::HttpRequest;
 
-use ws::WsWriter;
 use ws::frame::Frame;
 use ws::proto::{CloseReason, OpCode};
+use ws::WsWriter;
 
 /// Execution context for `WebSockets` actors
 pub struct WebsocketContext<A, S = ()>

tests/test_middleware.rs

@@ -551,7 +551,7 @@ fn test_async_middleware_multiple() {
     assert_eq!(num1.load(Ordering::Relaxed), 2);
     assert_eq!(num2.load(Ordering::Relaxed), 2);
 
-    thread::sleep(Duration::from_millis(30));
+    thread::sleep(Duration::from_millis(50));
     assert_eq!(num3.load(Ordering::Relaxed), 2);
 }