diff --git a/rustfmt.toml b/rustfmt.toml index e48aca7c1ce..7c364819a87 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -74,7 +74,7 @@ trailing_comma = "Vertical" match_block_trailing_comma = false blank_lines_upper_bound = 1 blank_lines_lower_bound = 0 -edition = "2015" +edition = "2018" version = "One" inline_attribute_width = 0 merge_derives = true diff --git a/src/httpapi/router.rs b/src/httpapi/router.rs index 8a5284356f4..a949a0aab48 100644 --- a/src/httpapi/router.rs +++ b/src/httpapi/router.rs @@ -148,10 +148,16 @@ pub struct HttpRouter { struct HttpRouterNode { /** Handlers for each of the HTTP methods defined for this node. */ method_handlers: BTreeMap>, - /** Outgoing edges for different literal paths. */ - edges_literals: BTreeMap>, + /** Edges linking to child nodes. */ + edges: Option, +} + +#[derive(Debug)] +enum HttpRouterEdges { + /** Outgoing edges for literal paths. */ + Literals(BTreeMap>), /** Outgoing edges for variable-named paths. */ - edge_varname: Option, + Variable(String, Box), } /** @@ -220,17 +226,22 @@ pub struct RouterLookupResult<'a> { pub variables: BTreeMap, } +impl HttpRouterNode { + pub fn new() -> Self { + HttpRouterNode { + method_handlers: BTreeMap::new(), + edges: None, + } + } +} + impl HttpRouter { /** * Returns a new `HttpRouter` with no routes configured. */ pub fn new() -> Self { HttpRouter { - root: Box::new(HttpRouterNode { - method_handlers: BTreeMap::new(), - edges_literals: BTreeMap::new(), - edge_varname: None, - }), + root: Box::new(HttpRouterNode::new()), } } @@ -265,7 +276,7 @@ impl HttpRouter { * that does not apply here. We could certainly make one up (e.g., * "http://127.0.0.1") and construct a URL whose path matches the path * we were given. However, while it seems natural that our internal - * representaiton would not be percent-encoded, the "url" crate + * representation would not be percent-encoded, the "url" crate * percent-encodes any path that it's given. Further, we probably want * to treat consecutive "/" characters as equivalent to a single "/", * but that crate treats them separately (which is not unreasonable, @@ -300,51 +311,32 @@ impl HttpRouter { node = match segment { PathSegment::Literal(lit) => { - /* - * We do not allow both literal and variable edges from the - * same node. This could be supported (with some caveats - * about how matching would work), but it seems more likely - * to be a mistake. - */ - if let Some(HttpRouterEdgeVariable(varname, _)) = - &node.edge_varname - { - panic!( - "URI path \"{}\": attempted to register route for \ - literal path segment \"{}\" when a route exists \ - for variable path segment (variable name: \"{}\")", - path, lit, varname - ); - } - - if !node.edges_literals.contains_key(&lit) { - let newnode = Box::new(HttpRouterNode { - method_handlers: BTreeMap::new(), - edges_literals: BTreeMap::new(), - edge_varname: None, - }); - - node.edges_literals.insert(lit.clone(), newnode); + let edges = node.edges.get_or_insert( + HttpRouterEdges::Literals(BTreeMap::new()), + ); + match edges { + /* + * We do not allow both literal and variable edges from the + * same node. This could be supported (with some caveats + * about how matching would work), but it seems more likely + * to be a mistake. + */ + HttpRouterEdges::Variable(varname, _) => { + panic!( + "URI path \"{}\": attempted to register route \ + for literal path segment \"{}\" when a route \ + exists for variable path segment (variable \ + name: \"{}\")", + path, lit, varname + ); + } + HttpRouterEdges::Literals(ref mut literals) => literals + .entry(lit) + .or_insert_with(|| Box::new(HttpRouterNode::new())), } - - node.edges_literals.get_mut(&lit).unwrap() } PathSegment::Varname(new_varname) => { - /* - * See the analogous check above about combining literal and - * variable path segments from the same resource. - */ - if !node.edges_literals.is_empty() { - panic!( - "URI path \"{}\": attempted to register route for \ - variable path segment (variable name: \"{}\") \ - when a route already exists for a literal path \ - segment", - path, new_varname - ); - } - /* * Do not allow the same variable name to be used more than * once in the path. Again, this could be supported (with @@ -359,43 +351,50 @@ impl HttpRouter { } varnames.insert(new_varname.clone()); - if node.edge_varname.is_none() { - let newnode = Box::new(HttpRouterNode { - method_handlers: BTreeMap::new(), - edges_literals: BTreeMap::new(), - edge_varname: None, - }); - - node.edge_varname = Some(HttpRouterEdgeVariable( + let edges = + node.edges.get_or_insert(HttpRouterEdges::Variable( new_varname.clone(), - newnode, + Box::new(HttpRouterNode::new()), )); - } else if *new_varname - != *node.edge_varname.as_ref().unwrap().0 - { + match edges { /* - * Don't allow people to use different names for the - * same part of the path. Again, this could be - * supported, but it seems likely to be confusing and - * probably a mistake. + * See the analogous check above about combining literal and + * variable path segments from the same resource. */ - panic!( - "URI path \"{}\": attempted to use variable name \ - \"{}\", but a different name (\"{}\") has \ - already been used for this", - path, - new_varname, - node.edge_varname.as_ref().unwrap().0 - ); + HttpRouterEdges::Literals(_) => panic!( + "URI path \"{}\": attempted to register route for \ + variable path segment (variable name: \"{}\") \ + when a route already exists for a literal path \ + segment", + path, new_varname + ), + + HttpRouterEdges::Variable(varname, ref mut node) => { + if *new_varname != *varname { + /* + * Don't allow people to use different names for the + * same part of the path. Again, this could be + * supported, but it seems likely to be confusing and + * probably a mistake. + */ + panic!( + "URI path \"{}\": attempted to use \ + variable name \"{}\", but a different \ + name (\"{}\") has already been used for \ + this", + path, new_varname, varname + ); + } + + node + } } - - &mut node.edge_varname.as_mut().unwrap().1 } }; } let methodname = method.as_str().to_uppercase(); - if let Some(_) = node.method_handlers.get(&methodname) { + if node.method_handlers.get(&methodname).is_some() { panic!( "URI path \"{}\": attempted to create duplicate route for \ method \"{}\"", @@ -428,14 +427,18 @@ impl HttpRouter { for segment in all_segments { let segment_string = segment.to_string(); - if let Some(n) = node.edges_literals.get(&segment_string) { - node = n; - } else if let Some(edge) = &node.edge_varname { - variables.insert(edge.0.clone(), segment_string); - node = &edge.1 - } else { - return Err(HttpError::for_status(StatusCode::NOT_FOUND)); + + node = match &node.edges { + None => None, + Some(HttpRouterEdges::Literals(edges)) => { + edges.get(&segment_string) + } + Some(HttpRouterEdges::Variable(varname, ref node)) => { + variables.insert(varname.clone(), segment_string); + Some(node) + } } + .ok_or_else(|| HttpError::for_status(StatusCode::NOT_FOUND))?; } /* @@ -447,14 +450,15 @@ impl HttpRouter { } let methodname = method.as_str().to_uppercase(); - if let Some(handler) = node.method_handlers.get(&methodname) { - Ok(RouterLookupResult { - handler: handler, - variables: variables, + node.method_handlers + .get(&methodname) + .map(|handler| RouterLookupResult { + handler, + variables, + }) + .ok_or_else(|| { + HttpError::for_status(StatusCode::METHOD_NOT_ALLOWED) }) - } else { - Err(HttpError::for_status(StatusCode::METHOD_NOT_ALLOWED)) - } } }