Output all matching routes, not just first ranked.

Cargo.toml

@@ -14,4 +14,5 @@ members = [
   "examples/static_files",
   "examples/todo",
   "examples/content_types",
+  "examples/hello_ranks",
 ]

examples/hello_ranks/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "hello_ranks"
+version = "0.0.1"
+authors = ["Sergio Benitez <sb@sergio.bz>"]
+workspace = "../../"
+
+[dependencies]
+rocket = { path = "../../lib" }
+rocket_macros = { path = "../../macros" }

examples/hello_ranks/src/main.rs

@@ -0,0 +1,20 @@
+#![feature(plugin)]
+#![plugin(rocket_macros)]
+
+extern crate rocket;
+use rocket::Rocket;
+
+#[GET(path = "/hello/<name>/<age>")]
+fn hello(name: &str, age: i8) -> String {
+    format!("Hello, {} year old named {}!", age, name)
+}
+
+// FIXME: Add 'rank = 2'.
+#[GET(path = "/hello/<name>/<age>")]
+fn hi(name: &str, age: &str) -> String {
+    format!("Hi {}! You age ({}) is kind of funky.", name, age)
+}
+
+fn main() {
+    Rocket::new("localhost", 8000).mount_and_launch("/", routes![hello, hi]);
+}

lib/src/rocket.rs

@@ -1,5 +1,5 @@
 use super::*;
-use response::FreshHyperResponse;
+use response::{FreshHyperResponse, Outcome};
 use request::HyperRequest;
 use catcher;
 
@@ -29,7 +29,7 @@ impl HyperHandler for Rocket {
 
 impl Rocket {
     fn dispatch<'h, 'k>(&self, hyp_req: HyperRequest<'h, 'k>,
-                        res: FreshHyperResponse<'h>) {
+                        mut res: FreshHyperResponse<'h>) {
         // Get a copy of the URI for later use.
         let uri = hyp_req.uri.to_string();
 
@@ -43,27 +43,25 @@ impl Rocket {
         };
 
         info!("{}:", request);
-        let route = self.router.route(&request);
-        if let Some(ref route) = route {
+        let matches = self.router.route(&request);
+        trace_!("Found {} matches.", matches.len());
+        for route in matches {
             // Retrieve and set the requests parameters.
+            info_!("Matched: {}", route);
             request.set_params(route);
 
             // Here's the magic: dispatch the request to the handler.
             let outcome = (route.handler)(&request).respond(res);
             info_!("{} {}", White.paint("Outcome:"), outcome);
 
-            // TODO: keep trying lower ranked routes before dispatching a not
-            // found error.
-            outcome.map_forward(|res| {
-                error_!("No further matching routes.");
-                // TODO: Have some way to know why this was failed forward. Use that
-                // instead of always using an unchained error.
-                self.handle_not_found(&request, res);
-            });
-        } else {
-            error_!("No matching routes.");
-            self.handle_not_found(&request, res);
+            res = match outcome {
+                Outcome::Complete => return,
+                Outcome::FailStop => return,
+                Outcome::FailForward(r) => r
+            };
         }
+
+        self.handle_not_found(&request, res);
     }
 
     // Call on internal server error.
@@ -95,6 +93,7 @@ impl Rocket {
 
     pub fn mount(&mut self, base: &'static str, routes: Vec<Route>)
             -> &mut Self {
+        self.enable_normal_logging_if_disabled();
         info!("🛰  {} '{}':", Magenta.paint("Mounting"), base);
         for mut route in routes {
             let path = format!("{}/{}", base, route.path.as_str());
@@ -108,12 +107,13 @@ impl Rocket {
     }
 
     pub fn catch(&mut self, catchers: Vec<Catcher>) -> &mut Self {
+        self.enable_normal_logging_if_disabled();
         info!("👾  {}:", Magenta.paint("Catchers"));
         for c in catchers {
             if self.catchers.contains_key(&c.code) &&
                     !self.catchers.get(&c.code).unwrap().is_default() {
                 let msg = format!("warning: overrides {} catcher!", c.code);
-                info_!("{} ({})", c, Yellow.paint(msg.as_str()));
+                warn!("{} ({})", c, Yellow.paint(msg.as_str()));
             } else {
                 info_!("{}", c);
             }
@@ -124,6 +124,13 @@ impl Rocket {
         self
     }
 
+    fn enable_normal_logging_if_disabled(&mut self) {
+        if !self.log_set {
+            logger::init(LoggingLevel::Normal);
+            self.log_set = true;
+        }
+    }
+
     pub fn log(&mut self, level: LoggingLevel) {
         if self.log_set {
             warn!("Log level already set! Not overriding.");
@@ -134,14 +141,11 @@ impl Rocket {
     }
 
     pub fn launch(mut self) {
+        self.enable_normal_logging_if_disabled();
         if self.router.has_collisions() {
             warn!("Route collisions detected!");
         }
 
-        if !self.log_set {
-            self.log(LoggingLevel::Normal)
-        }
-
         let full_addr = format!("{}:{}", self.address, self.port);
         info!("🚀  {} {}...", White.paint("Rocket has launched from"),
             White.bold().paint(&full_addr));

lib/src/router/collider.rs

@@ -45,7 +45,7 @@ mod tests {
     use Method;
     use Method::*;
     use {Request, Response};
-    use content_type::{ContentType, TopLevel, SubLevel};
+    use content_type::ContentType;
     use std::str::FromStr;
 
     type SimpleRoute = (Method, &'static str);

lib/src/router/mod.rs

@@ -32,27 +32,16 @@ impl Router {
     // `Route` structure is inflexible. Have it be an associated type.
     // FIXME: Figure out a way to get more than one route, i.e., to correctly
     // handle ranking.
-    // TODO: Should the Selector include the content-type? If it does, can't
-    // warn the user that a match was found for the wrong content-type. It
-    // doesn't, can, but this method is slower.
-    pub fn route<'b>(&'b self, req: &Request) -> Option<&'b Route> {
+    pub fn route<'b>(&'b self, req: &Request) -> Vec<&'b Route> {
         let num_segments = req.uri.segment_count();
+        self.routes.get(&(req.method, num_segments)).map_or(vec![], |routes| {
+            let mut matches: Vec<&'b Route> = routes.iter().filter(|r| {
+                r.collides_with(req)
+            }).collect();
 
-        let mut matched_route: Option<&Route> = None;
-        if let Some(routes) = self.routes.get(&(req.method, num_segments)) {
-            for route in routes.iter().filter(|r| r.collides_with(req)) {
-                info_!("Matched: {}", route);
-                if let Some(existing_route) = matched_route {
-                    if route.rank < existing_route.rank {
-                        matched_route = Some(route);
-                    }
-                } else {
-                    matched_route = Some(route);
-                }
-            }
-        }
-
-        matched_route
+            matches.sort_by(|a, b| a.rank.cmp(&b.rank));
+            matches
+        })
     }
 
     pub fn has_collisions(&self) -> bool {
@@ -94,6 +83,16 @@ mod test {
         router
     }
 
+    fn router_with_ranked_routes(routes: &[(isize, &'static str)]) -> Router {
+        let mut router = Router::new();
+        for &(rank, route) in routes {
+            let route = Route::ranked(rank, Get, route.to_string(), dummy_handler);
+            router.add(route);
+        }
+
+        router
+    }
+
     fn router_with_unranked_routes(routes: &[&'static str]) -> Router {
         let mut router = Router::new();
         for route in routes {
@@ -104,42 +103,41 @@ mod test {
         router
     }
 
+    fn unranked_route_collisions(routes: &[&'static str]) -> bool {
+        let router = router_with_unranked_routes(routes);
+        router.has_collisions()
+    }
+
+    fn default_rank_route_collisions(routes: &[&'static str]) -> bool {
+        let router = router_with_routes(routes);
+        router.has_collisions()
+    }
+
     #[test]
     fn test_collisions() {
-        let router = router_with_unranked_routes(&["/hello", "/hello"]);
-        assert!(router.has_collisions());
-
-        let router = router_with_unranked_routes(&["/<a>", "/hello"]);
-        assert!(router.has_collisions());
-
-        let router = router_with_unranked_routes(&["/<a>", "/<b>"]);
-        assert!(router.has_collisions());
-
-        let router = router_with_unranked_routes(&["/hello/bob", "/hello/<b>"]);
-        assert!(router.has_collisions());
-
-        let router = router_with_routes(&["/a/b/<c>/d", "/<a>/<b>/c/d"]);
-        assert!(router.has_collisions());
+        assert!(unranked_route_collisions(&["/hello", "/hello"]));
+        assert!(unranked_route_collisions(&["/<a>", "/hello"]));
+        assert!(unranked_route_collisions(&["/<a>", "/<b>"]));
+        assert!(unranked_route_collisions(&["/hello/bob", "/hello/<b>"]));
+        assert!(unranked_route_collisions(&["/a/b/<c>/d", "/<a>/<b>/c/d"]));
     }
 
     #[test]
     fn test_none_collisions_when_ranked() {
-        let router = router_with_routes(&["/<a>", "/hello"]);
-        assert!(!router.has_collisions());
-
-        let router = router_with_routes(&["/hello/bob", "/hello/<b>"]);
-        assert!(!router.has_collisions());
-
-        let router = router_with_routes(&["/a/b/c/d", "/<a>/<b>/c/d"]);
-        assert!(!router.has_collisions());
-
-        let router = router_with_routes(&["/hi", "/<hi>"]);
-        assert!(!router.has_collisions());
+        assert!(!default_rank_route_collisions(&["/<a>", "/hello"]));
+        assert!(!default_rank_route_collisions(&["/hello/bob", "/hello/<b>"]));
+        assert!(!default_rank_route_collisions(&["/a/b/c/d", "/<a>/<b>/c/d"]));
+        assert!(!default_rank_route_collisions(&["/hi", "/<hi>"]));
     }
 
     fn route<'a>(router: &'a Router, method: Method, uri: &str) -> Option<&'a Route> {
         let request = Request::mock(method, uri);
-        router.route(&request)
+        let matches = router.route(&request);
+        if matches.len() > 0 {
+            Some(matches[0])
+        } else {
+            None
+        }
     }
 
     #[test]
@@ -215,11 +213,47 @@ mod test {
         assert_ranked_routes!(&["/hi/a", "/hi/<c>"], "/hi/c", "/hi/<c>");
     }
 
+    fn ranked_collisions(routes: &[(isize, &'static str)]) -> bool {
+        let router = router_with_ranked_routes(routes);
+        router.has_collisions()
+    }
+
     #[test]
-    fn test_ranking() {
-        let a = Route::ranked(1, Get, "a/<b>", dummy_handler);
-        let b = Route::ranked(2, Get, "a/<b>", dummy_handler);
-        // FIXME: Add tests for non-default ranks.
+    fn test_no_manual_ranked_collisions() {
+        assert!(!ranked_collisions(&[(1, "a/<b>"), (2, "a/<b>")]));
+        assert!(!ranked_collisions(&[(0, "a/<b>"), (2, "a/<b>")]));
+        assert!(!ranked_collisions(&[(5, "a/<b>"), (2, "a/<b>")]));
+        assert!(!ranked_collisions(&[(1, "a/<b>"), (1, "b/<b>")]));
+    }
+
+    macro_rules! assert_ranked_routing {
+        (to: $to:expr, with: $routes:expr, expect: $want:expr) => ({
+            let router = router_with_ranked_routes(&$routes);
+            let routed_to = route(&router, Get, $to).unwrap();
+            assert_eq!(routed_to.path.as_str() as &str, $want.1);
+            assert_eq!(routed_to.rank, $want.0);
+        })
+    }
+
+    #[test]
+    fn test_ranked_routing() {
+        assert_ranked_routing!(
+            to: "a/b",
+            with: [(1, "a/<b>"), (2, "a/<b>")],
+            expect: (1, "a/<b>")
+        );
+
+        assert_ranked_routing!(
+            to: "b/b",
+            with: [(1, "a/<b>"), (2, "b/<b>"), (3, "b/b")],
+            expect: (2, "b/<b>")
+        );
+
+        assert_ranked_routing!(
+            to: "b/b",
+            with: [(1, "a/<b>"), (2, "b/<b>"), (0, "b/b")],
+            expect: (0, "b/b")
+        );
     }
 
     fn match_params(router: &Router, path: &str, expected: &[&str]) -> bool {

lib/src/router/route.rs

@@ -69,10 +69,10 @@ impl Route {
 
 impl fmt::Display for Route {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{} {} ", Green.paint(&self.method), Blue.paint(&self.path))?;
+        write!(f, "{} {}", Green.paint(&self.method), Blue.paint(&self.path))?;
 
         if !self.content_type.is_any() {
-            write!(f, "{}", Yellow.paint(&self.content_type))
+            write!(f, " {}", Yellow.paint(&self.content_type))
         } else {
             Ok(())
         }