From aff43cc8b84b172b6937ea314c5e84a2746eb9d5 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Thu, 22 Feb 2018 05:48:18 -0800 Subject: [PATCH] fix routes registration order --- src/application.rs | 14 +++---- src/httprequest.rs | 22 +++++------ src/router.rs | 91 ++++++++++++++++++++++++++++------------------ src/test.rs | 3 +- 4 files changed, 72 insertions(+), 58 deletions(-) diff --git a/src/application.rs b/src/application.rs index b12d87ee1..c7c1bcacb 100644 --- a/src/application.rs +++ b/src/application.rs @@ -100,7 +100,7 @@ struct ApplicationParts { prefix: String, settings: ServerSettings, default: Resource, - resources: HashMap>>, + resources: Vec<(Pattern, Option>)>, handlers: Vec<(String, Box>)>, external: HashMap, encoding: ContentEncoding, @@ -123,7 +123,7 @@ impl Application<()> { prefix: "/".to_owned(), settings: ServerSettings::default(), default: Resource::default_not_found(), - resources: HashMap::new(), + resources: Vec::new(), handlers: Vec::new(), external: HashMap::new(), encoding: ContentEncoding::Auto, @@ -153,7 +153,7 @@ impl Application where S: 'static { prefix: "/".to_owned(), settings: ServerSettings::default(), default: Resource::default_not_found(), - resources: HashMap::new(), + resources: Vec::new(), handlers: Vec::new(), external: HashMap::new(), middlewares: Vec::new(), @@ -242,11 +242,7 @@ impl Application where S: 'static { f(&mut resource); let pattern = Pattern::new(resource.get_name(), path); - if parts.resources.contains_key(&pattern) { - panic!("Resource {:?} is registered.", path); - } - - parts.resources.insert(pattern, Some(resource)); + parts.resources.push((pattern, Some(resource))); } self } @@ -354,7 +350,7 @@ impl Application where S: 'static { let mut resources = parts.resources; for (_, pattern) in parts.external { - resources.insert(pattern, None); + resources.push((pattern, None)); } let (router, resources) = Router::new(prefix, parts.settings, resources); diff --git a/src/httprequest.rs b/src/httprequest.rs index eb08b7220..aa0eb9f0e 100644 --- a/src/httprequest.rs +++ b/src/httprequest.rs @@ -883,9 +883,9 @@ mod tests { let mut resource = Resource::<()>::default(); resource.name("index"); - let mut map = HashMap::new(); - map.insert(Pattern::new("index", "/{key}/"), Some(resource)); - let (router, _) = Router::new("", ServerSettings::default(), map); + let mut routes = Vec::new(); + routes.push((Pattern::new("index", "/{key}/"), Some(resource))); + let (router, _) = Router::new("", ServerSettings::default(), routes); assert!(router.recognize(&mut req).is_some()); assert_eq!(req.match_info().get("key"), Some("value")); @@ -994,9 +994,8 @@ mod tests { 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("/", ServerSettings::default(), map); + let routes = vec!((Pattern::new("index", "/user/{name}.{ext}"), Some(resource))); + let (router, _) = Router::new("/", ServerSettings::default(), routes); assert!(router.has_route("/user/test.html")); assert!(!router.has_route("/test/unknown")); @@ -1019,9 +1018,8 @@ mod tests { 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); + let routes = vec![(Pattern::new("index", "/user/{name}.{ext}"), Some(resource))]; + let (router, _) = Router::new("/prefix/", ServerSettings::default(), routes); assert!(router.has_route("/user/test.html")); assert!(!router.has_route("/prefix/user/test.html")); @@ -1036,9 +1034,9 @@ mod tests { let mut resource = Resource::<()>::default(); resource.name("index"); - let mut map = HashMap::new(); - map.insert(Pattern::new("youtube", "https://youtube.com/watch/{video_id}"), None); - let (router, _) = Router::new::<()>("", ServerSettings::default(), map); + let routes = vec![ + (Pattern::new("youtube", "https://youtube.com/watch/{video_id}"), None)]; + let (router, _) = Router::new::<()>("", ServerSettings::default(), routes); assert!(!router.has_route("https://youtube.com/watch/unknown")); let req = req.with_state(Rc::new(()), router); diff --git a/src/router.rs b/src/router.rs index 765b56e36..9f1d93b05 100644 --- a/src/router.rs +++ b/src/router.rs @@ -27,7 +27,7 @@ impl Router { /// Create new router pub fn new(prefix: &str, settings: ServerSettings, - map: HashMap>>) -> (Router, Vec>) + map: Vec<(Pattern, Option>)>) -> (Router, Vec>) { let prefix = prefix.trim().trim_right_matches('/').to_owned(); let mut named = HashMap::new(); @@ -133,7 +133,7 @@ enum PatternElement { Var(String), } -#[derive(Clone)] +#[derive(Clone, Debug)] enum PatternType { Static(String), Dynamic(Regex, Vec), @@ -243,7 +243,8 @@ impl Pattern { fn parse(pattern: &str) -> (String, Vec, bool) { const DEFAULT_PATTERN: &str = "[^/]+"; - let mut re = String::from("/"); + let mut re1 = String::from("^/"); + let mut re2 = String::from("/"); let mut el = String::new(); let mut in_param = false; let mut in_param_pattern = false; @@ -262,7 +263,7 @@ impl Pattern { // In parameter segment: `{....}` if ch == '}' { elems.push(PatternElement::Var(param_name.clone())); - re.push_str(&format!(r"(?P<{}>{})", ¶m_name, ¶m_pattern)); + re1.push_str(&format!(r"(?P<{}>{})", ¶m_name, ¶m_pattern)); param_name.clear(); param_pattern = String::from(DEFAULT_PATTERN); @@ -287,15 +288,18 @@ impl Pattern { elems.push(PatternElement::Str(el.clone())); el.clear(); } else { - re.push_str(escape(&ch.to_string()).as_str()); + re1.push_str(escape(&ch.to_string()).as_str()); + re2.push(ch); el.push(ch); } } - if is_dynamic { - re.insert(0, '^'); - re.push('$'); - } + let re = if is_dynamic { + re1.push('$'); + re1 + } else { + re2 + }; (re, elems, is_dynamic) } } @@ -321,78 +325,95 @@ mod tests { #[test] fn test_recognizer() { - let mut routes = HashMap::new(); - routes.insert(Pattern::new("", "/name"), Some(Resource::default())); - routes.insert(Pattern::new("", "/name/{val}"), Some(Resource::default())); - routes.insert(Pattern::new("", "/name/{val}/index.html"), - Some(Resource::default())); - routes.insert(Pattern::new("", "/file/{file}.{ext}"), Some(Resource::default())); - routes.insert(Pattern::new("", "/v{val}/{val2}/index.html"), - Some(Resource::default())); - routes.insert(Pattern::new("", "/v/{tail:.*}"), Some(Resource::default())); - routes.insert(Pattern::new("", "{test}/index.html"), Some(Resource::default())); + let routes = vec![ + (Pattern::new("", "/name"), Some(Resource::default())), + (Pattern::new("", "/name/{val}"), Some(Resource::default())), + (Pattern::new("", "/name/{val}/index.html"), Some(Resource::default())), + (Pattern::new("", "/file/{file}.{ext}"), Some(Resource::default())), + (Pattern::new("", "/v{val}/{val2}/index.html"), Some(Resource::default())), + (Pattern::new("", "/v/{tail:.*}"), Some(Resource::default())), + (Pattern::new("", "{test}/index.html"), Some(Resource::default()))]; let (rec, _) = Router::new::<()>("", ServerSettings::default(), routes); let mut req = TestRequest::with_uri("/name").finish(); - assert!(rec.recognize(&mut req).is_some()); + assert_eq!(rec.recognize(&mut req), Some(0)); assert!(req.match_info().is_empty()); let mut req = TestRequest::with_uri("/name/value").finish(); - assert!(rec.recognize(&mut req).is_some()); + assert_eq!(rec.recognize(&mut req), Some(1)); assert_eq!(req.match_info().get("val").unwrap(), "value"); assert_eq!(&req.match_info()["val"], "value"); let mut req = TestRequest::with_uri("/name/value2/index.html").finish(); - assert!(rec.recognize(&mut req).is_some()); + assert_eq!(rec.recognize(&mut req), Some(2)); assert_eq!(req.match_info().get("val").unwrap(), "value2"); let mut req = TestRequest::with_uri("/file/file.gz").finish(); - assert!(rec.recognize(&mut req).is_some()); + assert_eq!(rec.recognize(&mut req), Some(3)); assert_eq!(req.match_info().get("file").unwrap(), "file"); assert_eq!(req.match_info().get("ext").unwrap(), "gz"); let mut req = TestRequest::with_uri("/vtest/ttt/index.html").finish(); - assert!(rec.recognize(&mut req).is_some()); + assert_eq!(rec.recognize(&mut req), Some(4)); assert_eq!(req.match_info().get("val").unwrap(), "test"); assert_eq!(req.match_info().get("val2").unwrap(), "ttt"); let mut req = TestRequest::with_uri("/v/blah-blah/index.html").finish(); - assert!(rec.recognize(&mut req).is_some()); + assert_eq!(rec.recognize(&mut req), Some(5)); assert_eq!(req.match_info().get("tail").unwrap(), "blah-blah/index.html"); let mut req = TestRequest::with_uri("/bbb/index.html").finish(); - assert!(rec.recognize(&mut req).is_some()); + assert_eq!(rec.recognize(&mut req), Some(6)); assert_eq!(req.match_info().get("test").unwrap(), "bbb"); } + #[test] + fn test_recognizer_2() { + let routes = vec![ + (Pattern::new("", "/index.json"), Some(Resource::default())), + (Pattern::new("", "/{source}.json"), Some(Resource::default()))]; + let (rec, _) = Router::new::<()>("", ServerSettings::default(), routes); + + let mut req = TestRequest::with_uri("/index.json").finish(); + assert_eq!(rec.recognize(&mut req), Some(0)); + + let mut req = TestRequest::with_uri("/test.json").finish(); + assert_eq!(rec.recognize(&mut req), Some(1)); + } + #[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 routes = vec![ + (Pattern::new("", "/name"), Some(Resource::default())), + (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()); + assert_eq!(rec.recognize(&mut req), Some(0)); let mut req = TestRequest::with_uri("/test/name/value").finish(); - assert!(rec.recognize(&mut req).is_some()); + assert_eq!(rec.recognize(&mut req), Some(1)); 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 routes = vec![ + (Pattern::new("", "/name"), Some(Resource::default())), + (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()); + assert_eq!(rec.recognize(&mut req), Some(0)); + let mut req = TestRequest::with_uri("/test2/name-test").finish(); + assert!(rec.recognize(&mut req).is_none()); + let mut req = TestRequest::with_uri("/test2/name/ttt").finish(); + assert_eq!(rec.recognize(&mut req), Some(1)); + assert_eq!(&req.match_info()["val"], "ttt"); } #[test] diff --git a/src/test.rs b/src/test.rs index 2fbb7c8f0..faad063f2 100644 --- a/src/test.rs +++ b/src/test.rs @@ -4,7 +4,6 @@ use std::{net, thread}; use std::rc::Rc; use std::sync::mpsc; use std::str::FromStr; -use std::collections::HashMap; use actix::{Arbiter, Addr, Syn, System, SystemRunner, msgs}; use cookie::Cookie; @@ -411,7 +410,7 @@ impl TestRequest { let req = HttpRequest::new(method, uri, version, headers, payload); req.as_mut().cookies = cookies; req.as_mut().params = params; - let (router, _) = Router::new::("/", ServerSettings::default(), HashMap::new()); + let (router, _) = Router::new::("/", ServerSettings::default(), Vec::new()); req.with_state(Rc::new(state), router) }