better application handling, fix url_for method for routes with prefix

guide/src/qs_3.md

@@ -10,7 +10,11 @@ Also it stores application specific state that is shared across all handlers
 within same application.
 
 Application acts as namespace for all routes, i.e all routes for specific application
-has same url path prefix:
+has same url path prefix. Application prefix always contains laading "/" slash. 
+If supplied prefix does not contain leading slash, it get inserted. 
+Prefix should consists of valud path segments. i.e for application with prefix `/app` 
+any request with following paths `/app`, `/app/` or `/app/test` would match,
+but path `/application` would not match.
 
 ```rust,ignore
 # extern crate actix_web;
@@ -21,14 +25,14 @@ has same url path prefix:
 # }
 # fn main() {
    let app = Application::new()
-       .prefix("/prefix")
+       .prefix("/app")
        .resource("/index.html", |r| r.method(Method::GET).f(index))
        .finish()
 # }
 ```
 
-In this example application with `/prefix` prefix and `index.html` resource
+In this example application with `/app` prefix and `index.html` resource
-get created. This resource is available as on `/prefix/index.html` url.
+get created. This resource is available as on `/app/index.html` url.
 For more information check 
 [*URL Matching*](./qs_5.html#using-a-application-prefix-to-compose-applications) section.
 
@@ -56,6 +60,10 @@ fn main() {
 ```
 
 All `/app1` requests route to first application, `/app2` to second and then all other to third.
+Applications get matched based on registration order, if application with more general
+prefix is registered before less generic, that would effectively block less generic
+application to get matched. For example if *application* with prefix "/" get registered
+as first application, it would match all incoming requests.
 
 ## State
 

guide/src/qs_5.md

@@ -445,7 +445,6 @@ fn main() {
 ## Using a Application Prefix to Compose Applications
 
 The `Applicaiton::prefix()`" method allows to set specific application prefix.
-If route_prefix is supplied to the include method, it must be a string.
 This prefix represents a resource prefix that will be prepended to all resource patterns added 
 by the resource configuration. This can be used to help mount a set of routes at a different
 location than the included callable's author intended while still maintaining the same
@@ -471,7 +470,7 @@ fn main() {
 
 In the above example, the *show_users* route will have an effective route pattern of 
 */users/show* instead of */show* because the application's prefix argument will be prepended 
-to the pattern. The route will then only match if the URL path is /users/show, 
+to the pattern. The route will then only match if the URL path is */users/show*,
 and when the `HttpRequest.url_for()` function is called with the route name show_users, 
 it will generate a URL with that same path.
 

src/application.rs

@@ -50,7 +50,13 @@ impl<S: 'static> HttpApplication<S> {
 impl<S: 'static> HttpHandler for HttpApplication<S> {
 
     fn handle(&mut self, req: HttpRequest) -> Result<Box<HttpHandlerTask>, HttpRequest> {
-        if req.path().starts_with(&self.prefix) {
+        let m = {
+            let path = req.path();
+            path.starts_with(&self.prefix) && (
+                path.len() == self.prefix.len() ||
+                    path.split_at(self.prefix.len()).1.starts_with('/'))
+        };
+        if m {
             let inner = Rc::clone(&self.inner);
             let req = req.with_state(Rc::clone(&self.state), self.router.clone());
 
@@ -126,11 +132,14 @@ impl<S> Application<S> where S: 'static {
     ///
     /// Only requests that matches application's prefix get processed by this application.
     /// Application prefix always contains laading "/" slash. If supplied prefix
-    /// does not contain leading slash, it get inserted.
+    /// does not contain leading slash, it get inserted. Prefix should
+    /// consists of valud path segments. i.e for application with
+    /// prefix `/app` any request with following paths `/app`, `/app/` or `/app/test`
+    /// would match, but path `/application` would not match.
     ///
-    /// Inthe following example only requests with "/app/" path prefix
+    /// In the following example only requests with "/app/" path prefix
-    /// get handled. Request with path "/app/test/" will be handled,
+    /// get handled. Request with path "/app/test/" would be handled,
-    /// but request with path "/other/..." will return *NOT FOUND*
+    /// but request with path "/application" or "/other/..." would return *NOT FOUND*
     ///
     /// ```rust
     /// # extern crate actix_web;
@@ -387,4 +396,23 @@ mod tests {
         let resp = app.run(req);
         assert_eq!(resp.as_response().unwrap().status(), StatusCode::OK);
     }
+
+    #[test]
+    fn test_prefix() {
+        let mut app = Application::new()
+            .prefix("/test")
+            .resource("/blah", |r| r.h(httpcodes::HTTPOk))
+            .finish();
+        let req = TestRequest::with_uri("/test").finish();
+        let resp = app.handle(req);
+        assert!(resp.is_ok());
+
+        let req = TestRequest::with_uri("/test/").finish();
+        let resp = app.handle(req);
+        assert!(resp.is_ok());
+
+        let req = TestRequest::with_uri("/testing").finish();
+        let resp = app.handle(req);
+        assert!(resp.is_err());
+    }
 }

src/httprequest.rs

@@ -823,7 +823,7 @@ mod tests {
         resource.name("index");
         let mut map = HashMap::new();
         map.insert(Pattern::new("index", "/user/{name}.{ext}"), Some(resource));
-        let (router, _) = Router::new("", ServerSettings::default(), map);
+        let (router, _) = Router::new("/", ServerSettings::default(), map);
         assert!(router.has_route("/user/test.html"));
         assert!(!router.has_route("/test/unknown"));
 
@@ -840,6 +840,27 @@ mod tests {
         assert_eq!(url.ok().unwrap().as_str(), "http://www.rust-lang.org/user/test.html");
     }
 
+    #[test]
+    fn test_url_for_with_prefix() {
+        let mut headers = HeaderMap::new();
+        headers.insert(header::HOST,
+                       header::HeaderValue::from_static("www.rust-lang.org"));
+        let req = HttpRequest::new(
+            Method::GET, Uri::from_str("/").unwrap(), Version::HTTP_11, headers, None);
+
+        let mut resource = Resource::<()>::default();
+        resource.name("index");
+        let mut map = HashMap::new();
+        map.insert(Pattern::new("index", "/user/{name}.{ext}"), Some(resource));
+        let (router, _) = Router::new("/prefix/", ServerSettings::default(), map);
+        assert!(router.has_route("/user/test.html"));
+        assert!(!router.has_route("/prefix/user/test.html"));
+
+        let req = req.with_state(Rc::new(()), router);
+        let url = req.url_for("index", &["test", "html"]);
+        assert_eq!(url.ok().unwrap().as_str(), "http://www.rust-lang.org/prefix/user/test.html");
+    }
+
     #[test]
     fn test_url_for_external() {
         let req = HttpRequest::new(

src/router.rs

@@ -16,6 +16,7 @@ pub struct Router(Rc<Inner>);
 
 struct Inner {
     prefix: String,
+    prefix_len: usize,
     regset: RegexSet,
     named: HashMap<String, (Pattern, bool)>,
     patterns: Vec<Pattern>,
@@ -47,8 +48,10 @@ impl Router {
             }
         }
 
+        let len = prefix.len();
         (Router(Rc::new(
             Inner{ prefix: prefix,
+                   prefix_len: len,
                    regset: RegexSet::new(&paths).unwrap(),
                    named: named,
                    patterns: patterns,
@@ -71,7 +74,10 @@ impl Router {
     pub fn recognize<S>(&self, req: &mut HttpRequest<S>) -> Option<usize> {
         let mut idx = None;
         {
-            let path = &req.path()[self.0.prefix.len()..];
+            if self.0.prefix_len > req.path().len() {
+                return None
+            }
+            let path = &req.path()[self.0.prefix_len..];
             if path.is_empty() {
                 if let Some(i) = self.0.regset.matches("/").into_iter().next() {
                     idx = Some(i);
@@ -82,7 +88,7 @@ impl Router {
         }
 
         if let Some(idx) = idx {
-            let path: &str = unsafe{ mem::transmute(&req.path()[self.0.prefix.len()..]) };
+            let path: &str = unsafe{ mem::transmute(&req.path()[self.0.prefix_len..]) };
             self.0.patterns[idx].update_match_info(path, req);
             return Some(idx)
         } else {
@@ -91,13 +97,17 @@ impl Router {
     }
 
     /// Check if application contains matching route.
+    ///
+    /// This method does not take `prefix` into account.
+    /// For example if prefix is `/test` and router contains route `/name`,
+    /// following path would be recognizable `/test/name` but `has_route()` call
+    /// would return `false`.
     pub fn has_route(&self, path: &str) -> bool {
-        let p = &path[self.0.prefix.len()..];
+        if path.is_empty() {
-        if p.is_empty() {
             if self.0.regset.matches("/").into_iter().next().is_some() {
                 return true
             }
-        } else if self.0.regset.matches(p).into_iter().next().is_some() {
+        } else if self.0.regset.matches(path).into_iter().next().is_some() {
             return true
         }
         false
@@ -205,12 +215,11 @@ impl Pattern {
     {
         let mut iter = elements.into_iter();
         let mut path = if let Some(prefix) = prefix {
-            let mut path = String::from(prefix);
+            format!("{}/", prefix)
-            path.push('/');
-            path
         } else {
             String::new()
         };
+        println!("TEST: {:?} {:?}", path, prefix);
         for el in &self.elements {
             match *el {
                 PatternElement::Str(ref s) => path.push_str(s),
@@ -297,11 +306,9 @@ impl Hash for Pattern {
 
 #[cfg(test)]
 mod tests {
-    use regex::Regex;
     use super::*;
-    use http::{Uri, Version, Method};
+    use regex::Regex;
-    use http::header::HeaderMap;
+    use test::TestRequest;
-    use std::str::FromStr;
 
     #[test]
     fn test_recognizer() {
@@ -314,45 +321,63 @@ mod tests {
         routes.insert(Pattern::new("", "{test}/index.html"), Some(Resource::default()));
         let (rec, _) = Router::new::<()>("", ServerSettings::default(), routes);
 
-        let mut req = HttpRequest::new(
+        let mut req = TestRequest::with_uri("/name").finish();
-            Method::GET, Uri::from_str("/name").unwrap(),
-            Version::HTTP_11, HeaderMap::new(), None);
         assert!(rec.recognize(&mut req).is_some());
         assert!(req.match_info().is_empty());
 
-        let mut req = HttpRequest::new(
+        let mut req = TestRequest::with_uri("/name/value").finish();
-            Method::GET, Uri::from_str("/name/value").unwrap(),
-            Version::HTTP_11, HeaderMap::new(), None);
         assert!(rec.recognize(&mut req).is_some());
         assert_eq!(req.match_info().get("val").unwrap(), "value");
         assert_eq!(&req.match_info()["val"], "value");
 
-        let mut req = HttpRequest::new(
+        let mut req = TestRequest::with_uri("/name/value2/index.html").finish();
-            Method::GET, Uri::from_str("/name/value2/index.html").unwrap(),
-            Version::HTTP_11, HeaderMap::new(), None);
         assert!(rec.recognize(&mut req).is_some());
         assert_eq!(req.match_info().get("val").unwrap(), "value2");
 
-        let mut req = HttpRequest::new(
+        let mut req = TestRequest::with_uri("/vtest/ttt/index.html").finish();
-            Method::GET, Uri::from_str("/vtest/ttt/index.html").unwrap(),
-            Version::HTTP_11, HeaderMap::new(), None);
         assert!(rec.recognize(&mut req).is_some());
         assert_eq!(req.match_info().get("val").unwrap(), "test");
         assert_eq!(req.match_info().get("val2").unwrap(), "ttt");
 
-        let mut req = HttpRequest::new(
+        let mut req = TestRequest::with_uri("/v/blah-blah/index.html").finish();
-            Method::GET, Uri::from_str("/v/blah-blah/index.html").unwrap(),
-            Version::HTTP_11, HeaderMap::new(), None);
         assert!(rec.recognize(&mut req).is_some());
         assert_eq!(req.match_info().get("tail").unwrap(), "blah-blah/index.html");
 
-        let mut req = HttpRequest::new(
+        let mut req = TestRequest::with_uri("/bbb/index.html").finish();
-            Method::GET, Uri::from_str("/bbb/index.html").unwrap(),
-            Version::HTTP_11, HeaderMap::new(), None);
         assert!(rec.recognize(&mut req).is_some());
         assert_eq!(req.match_info().get("test").unwrap(), "bbb");
     }
 
+    #[test]
+    fn test_recognizer_with_prefix() {
+        let mut routes = HashMap::new();
+        routes.insert(Pattern::new("", "/name"), Some(Resource::default()));
+        routes.insert(Pattern::new("", "/name/{val}"), Some(Resource::default()));
+        let (rec, _) = Router::new::<()>("/test", ServerSettings::default(), routes);
+
+        let mut req = TestRequest::with_uri("/name").finish();
+        assert!(rec.recognize(&mut req).is_none());
+
+        let mut req = TestRequest::with_uri("/test/name").finish();
+        assert!(rec.recognize(&mut req).is_some());
+
+        let mut req = TestRequest::with_uri("/test/name/value").finish();
+        assert!(rec.recognize(&mut req).is_some());
+        assert_eq!(req.match_info().get("val").unwrap(), "value");
+        assert_eq!(&req.match_info()["val"], "value");
+
+        // same patterns
+        let mut routes = HashMap::new();
+        routes.insert(Pattern::new("", "/name"), Some(Resource::default()));
+        routes.insert(Pattern::new("", "/name/{val}"), Some(Resource::default()));
+        let (rec, _) = Router::new::<()>("/test2", ServerSettings::default(), routes);
+
+        let mut req = TestRequest::with_uri("/name").finish();
+        assert!(rec.recognize(&mut req).is_none());
+        let mut req = TestRequest::with_uri("/test2/name").finish();
+        assert!(rec.recognize(&mut req).is_some());
+    }
+
     fn assert_parse(pattern: &str, expected_re: &str) -> Regex {
         let (re_str, _) = Pattern::parse(pattern);
         assert_eq!(&*re_str, expected_re);