Skip to content

move to union type #2

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

Merged
ahl merged 3 commits into from
Mar 24, 2020
Merged

move to union type #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e535f2
edge
ahl Mar 17, 2020
b1d7745
move to union type
ahl Mar 18, 2020
11d1f37
turn router edges into enum type
ahl Mar 18, 2020
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
2 changes: 1 addition & 1 deletion rustfmt.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
186 changes: 95 additions & 91 deletions src/httpapi/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,16 @@ pub struct HttpRouter {
struct HttpRouterNode {
/** Handlers for each of the HTTP methods defined for this node. */
method_handlers: BTreeMap<String, Box<dyn RouteHandler>>,
/** Outgoing edges for different literal paths. */
edges_literals: BTreeMap<String, Box<HttpRouterNode>>,
/** Edges linking to child nodes. */
edges: Option<HttpRouterEdges>,
}

#[derive(Debug)]
enum HttpRouterEdges {
/** Outgoing edges for literal paths. */
Literals(BTreeMap<String, Box<HttpRouterNode>>),
/** Outgoing edges for variable-named paths. */
edge_varname: Option<HttpRouterEdgeVariable>,
Variable(String, Box<HttpRouterNode>),
}

/**
Expand Down Expand Up @@ -220,17 +226,22 @@ pub struct RouterLookupResult<'a> {
pub variables: BTreeMap<String, String>,
}

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()),
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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 \"{}\"",
Expand Down Expand Up @@ -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))?;
}

/*
Expand All @@ -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))
}
}
}

Expand Down