Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add implicit optional format param to routes #91

Closed
wants to merge 4 commits into from
Closed

Add implicit optional format param to routes #91

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b36528
chore(routeresult): improve error message for bad param call
Ryman Oct 4, 2014
5d1fe6a
feat(routes): add implicit optional format param to routes
Ryman Oct 4, 2014
130b415
style(router): shorten extremely long lines
Ryman Oct 4, 2014
6c4d723
fix(routes): allow custom :format variable in route
Ryman Oct 8, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 89 additions & 36 deletions src/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,15 @@ pub trait HttpRouter {
///
/// // with simple wildcard
/// fn wild_handler(request: &Request, response: &mut Response) {
/// response.send("This matches /user/list/4711 but not /user/extended/list/4711");
/// response.send("This matches /user/list/4711");
/// response.send("NOT /user/extended/list/4711");
/// };
/// server.get("/user/*/:userid", wild_handler);
///
/// // with double wildcard
/// fn very_wild_handler(request: &Request, response: &mut Response) {
/// response.send("This matches /user/list/4711 and also /user/extended/list/4711");
/// response.send("This matches /user/list/4711");
/// response.send("AND and also /user/extended/list/4711");
/// };
/// server.get("/user/**/:userid", very_wild_handler);
/// ```
Expand Down Expand Up @@ -202,7 +204,13 @@ pub struct RouteResult<'a> {

impl<'a> RouteResult<'a> {
pub fn param(&self, key: &str) -> &str {
let idx = self.route.variables.find_equiv(&key).unwrap();
let idx = match self.route.variables.find_equiv(&key) {
Some(idx) => idx,
None => {
fail!("Unknown param '{}' for route '{}'", key, self.route.path)
}
};

self.params[*idx].as_slice()
}
}
Expand All @@ -222,16 +230,18 @@ mod path_utils {
static REGEX_START:&'static str = "^";
static REGEX_END:&'static str = "$";
pub fn create_regex (route_path: &str) -> Regex {

let updated_path = route_path.to_string()
// first mark all double wildcards for replacement. We can't directly replace them
// since the replacement does contain the * symbol as well, which would get overwritten
// with the next replace call
.replace("**", "___DOUBLE_WILDCARD___")
// then replace the regular wildcard symbols (*) with the appropriate regex
.replace("*", VAR_SEQ)
// now replace the previously marked double wild cards (**)
.replace("___DOUBLE_WILDCARD___", VAR_SEQ_WITH_SLASH);
let updated_path =
route_path.to_string()
// first mark all double wildcards for replacement.
// We can't directly replace them since the replacement
// does contain the * symbol as well, which would get
// overwritten with the next replace call
.replace("**", "___DOUBLE_WILDCARD___")
// then replace the regular wildcard symbols (*) with the
// appropriate regex
.replace("*", VAR_SEQ)
// now replace the previously marked double wild cards (**)
.replace("___DOUBLE_WILDCARD___", VAR_SEQ_WITH_SLASH);

// then replace the variable symbols (:variable) with the appropriate regex
let result = [REGEX_START,
Expand Down Expand Up @@ -266,31 +276,43 @@ impl<'a> Router {
}
}

pub fn match_route(&'a self, method: &Method, path: &str) -> Option<RouteResult<'a>> {
self.routes.iter().find(|item| item.method == *method && item.matcher.is_match(path))
.map(|route| {
let vec = match route.matcher.captures(path) {
Some(captures) => {
range(0, route.variables.len()).map(|pos|
captures.at(pos + 1).to_string()
).collect()
},
None => vec![],
};
RouteResult {
route: route,
params: vec
}
})
pub fn match_route(&'a self, method: &Method, path: &str)
-> Option<RouteResult<'a>> {
self.routes.iter().find(|item| {
item.method == *method
&& item.matcher.is_match(path)
}).map(|route| {
let vec = match route.matcher.captures(path) {
Some(captures) => {
range(0, route.variables.len()).map(|pos|
captures.at(pos + 1).to_string()
).collect()
},
None => vec![],
};

RouteResult {
route: route,
params: vec
}
})
}
}

impl HttpRouter for Router {
fn add_route(&mut self, method: Method, path: &str, handler: RequestHandler) {
let matcher = path_utils::create_regex(path);
let variable_infos = path_utils::get_variable_info(path);
static FORMAT_VAR: &'static str = ":format";

let with_format = if path.contains(FORMAT_VAR) {
path.to_string()
} else {
format!("{}(\\.{})?", path, FORMAT_VAR)
};

let matcher = path_utils::create_regex(with_format[]);
let variable_infos = path_utils::get_variable_info(with_format[]);
let route = Route {
path: path.to_string(),
path: with_format,
method: method,
matcher: matcher,
handler: handler,
Expand Down Expand Up @@ -392,33 +414,64 @@ fn can_match_var_routes () {

route_store.add_route(method::Get, "/foo/:userid", handler);
route_store.add_route(method::Get, "/bar", handler);
route_store.add_route(method::Get, "/file/:format/:file", handler);

let route_result = route_store.match_route(&method::Get, "/foo/4711").unwrap();
let route = route_result.route;

assert_eq!(route_result.param("userid"), "4711");

//assert the route has identified the variable
assert_eq!(route.variables.len(), 1);
// assert the route has identified the variable
assert_eq!(route.variables.len(), 2);
assert_eq!(route.variables["userid".to_string()], 0);
// routes have an implicit format variable
assert_eq!(route.variables["format".to_string()], 1);

let route_result = route_store.match_route(&method::Get, "/bar/4711");
assert!(route_result.is_none());

let route_result = route_store.match_route(&method::Get, "/foo");
assert!(route_result.is_none());

//ensure that this will work with commas too
// ensure that this will work with commas too
let route_result = route_store.match_route(&method::Get, "/foo/123,456");
assert!(route_result.is_some());

let route_result = route_result.unwrap();
assert_eq!(route_result.param("userid"), "123,456");

//ensure that this will work with spacing too
// ensure that this will work with spacing too
let route_result = route_store.match_route(&method::Get, "/foo/John%20Doe");
assert!(route_result.is_some());

let route_result = route_result.unwrap();
assert_eq!(route_result.param("userid"), "John%20Doe");

// check for optional format param
let route_result = route_store.match_route(&method::Get, "/foo/John%20Doe.json");
assert!(route_result.is_some());

let route_result = route_result.unwrap();
assert_eq!(route_result.param("userid"), "John%20Doe");
assert_eq!(route_result.param("format"), ".json");

// ensure format works with queries
let route_result = route_store.match_route(&method::Get,
"/foo/5490,1234.csv?foo=true&bar=false");
assert!(route_result.is_some());

let route_result = route_result.unwrap();
// NOTE: `.param` doesn't cover query params currently
assert_eq!(route_result.param("userid"), "5490,1234");
assert_eq!(route_result.param("format"), ".csv");

// ensure format works if defined by user
let route_result = route_store.match_route(&method::Get,
"/file/markdown/something?foo=true");
assert!(route_result.is_some());

let route_result = route_result.unwrap();
// NOTE: `.param` doesn't cover query params currently
assert_eq!(route_result.param("file"), "something");
assert_eq!(route_result.param("format"), "markdown");
}