router: represent methods as http::Method

Currently, the `Router` stores HTTP methods for an endpoint as
`String`s. This is a bit unfortunate, as it means the already-parsed
`http::Method` for each request has to be un-parsed back into a `String`
to look it up in the map, allocating a new `String` each time and
converting it to uppercases to perform the lookup. Switching to the
typed `http::Method` representation should make things a bit more
efficient (especially as `Method` also implements inline storage for
extension methods up to a certain length) and provide additional type
safety. This does require switching from a `BTreeMap` of strings to
handlers to a `HashMap` of `Method`s to handlers, as `Method` does not
implement `Ord`/`PartialOrd`, but ordering shouldn't matter here.

Author: hawkw

dropshot/src/api_description.rs

@@ -670,15 +670,15 @@ impl<Context: ServerContext> ApiDescription<Context> {
                 _ => panic!("reference not expected"),
             };
 
-            let method_ref = match &method[..] {
-                "GET" => &mut pathitem.get,
-                "PUT" => &mut pathitem.put,
-                "POST" => &mut pathitem.post,
-                "DELETE" => &mut pathitem.delete,
-                "OPTIONS" => &mut pathitem.options,
-                "HEAD" => &mut pathitem.head,
-                "PATCH" => &mut pathitem.patch,
-                "TRACE" => &mut pathitem.trace,
+            let method_ref = match method {
+                http::Method::GET => &mut pathitem.get,
+                http::Method::PUT => &mut pathitem.put,
+                http::Method::POST => &mut pathitem.post,
+                http::Method::DELETE => &mut pathitem.delete,
+                http::Method::OPTIONS => &mut pathitem.options,
+                http::Method::HEAD => &mut pathitem.head,
+                http::Method::PATCH => &mut pathitem.patch,
+                http::Method::TRACE => &mut pathitem.trace,
                 other => panic!("unexpected method `{}`", other),
             };
             let mut operation = openapiv3::Operation::default();

dropshot/src/router.rs

@@ -14,6 +14,7 @@ use http::StatusCode;
 use percent_encoding::percent_decode_str;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
+use std::collections::HashMap;
 use std::sync::Arc;
 
 /// `HttpRouter` is a simple data structure for routing incoming HTTP requests to
@@ -81,7 +82,7 @@ pub struct HttpRouter<Context: ServerContext> {
 #[derive(Debug)]
 struct HttpRouterNode<Context: ServerContext> {
     /// Handlers, etc. for each of the HTTP methods defined for this node.
-    methods: BTreeMap<String, ApiEndpoint<Context>>,
+    methods: HashMap<Method, ApiEndpoint<Context>>,
     /// Edges linking to child nodes.
     edges: Option<HttpRouterEdges<Context>>,
 }
@@ -217,7 +218,7 @@ pub struct RouterLookupResult<Context: ServerContext> {
 
 impl<Context: ServerContext> HttpRouterNode<Context> {
     pub fn new() -> Self {
-        HttpRouterNode { methods: BTreeMap::new(), edges: None }
+        HttpRouterNode { methods: HashMap::new(), edges: None }
     }
 }
 
@@ -385,16 +386,15 @@ impl<Context: ServerContext> HttpRouter<Context> {
             };
         }
 
-        let methodname = method.as_str().to_uppercase();
-        if node.methods.contains_key(&methodname) {
+        if node.methods.contains_key(&method) {
             panic!(
                 "URI path \"{}\": attempted to create duplicate route for \
                  method \"{}\"",
                 path, method,
             );
         }
 
-        node.methods.insert(methodname, endpoint);
+        node.methods.insert(method, endpoint);
     }
 
     /// Look up the route handler for an HTTP request having method `method` and
@@ -478,9 +478,8 @@ impl<Context: ServerContext> HttpRouter<Context> {
             ));
         }
 
-        let methodname = method.as_str().to_uppercase();
         node.methods
-            .get(&methodname)
+            .get(&method)
             .map(|handler| RouterLookupResult {
                 handler: Arc::clone(&handler.handler),
                 operation_id: handler.operation_id.clone(),
@@ -512,7 +511,7 @@ fn insert_var(
 }
 
 impl<'a, Context: ServerContext> IntoIterator for &'a HttpRouter<Context> {
-    type Item = (String, String, &'a ApiEndpoint<Context>);
+    type Item = (String, Method, &'a ApiEndpoint<Context>);
     type IntoIter = HttpRouterIter<'a, Context>;
     fn into_iter(self) -> Self::IntoIter {
         HttpRouterIter::new(self)
@@ -529,7 +528,7 @@ impl<'a, Context: ServerContext> IntoIterator for &'a HttpRouter<Context> {
 /// blank string and an iterator over the root node's children.
 pub struct HttpRouterIter<'a, Context: ServerContext> {
     method:
-        Box<dyn Iterator<Item = (&'a String, &'a ApiEndpoint<Context>)> + 'a>,
+        Box<dyn Iterator<Item = (&'a Method, &'a ApiEndpoint<Context>)> + 'a>,
     path: Vec<(PathSegment, Box<PathIter<'a, Context>>)>,
 }
 type PathIter<'a, Context> =
@@ -592,7 +591,7 @@ impl<'a, Context: ServerContext> HttpRouterIter<'a, Context> {
 }
 
 impl<'a, Context: ServerContext> Iterator for HttpRouterIter<'a, Context> {
-    type Item = (String, String, &'a ApiEndpoint<Context>);
+    type Item = (String, Method, &'a ApiEndpoint<Context>);
 
     fn next(&mut self) -> Option<Self::Item> {
         // If there are no path components left then we've reached the end of
@@ -1309,10 +1308,10 @@ mod test {
         assert_eq!(
             ret,
             vec![
-                ("/".to_string(), "GET".to_string(),),
+                ("/".to_string(), http::Method::GET,),
                 (
                     "/projects/{project_id}/instances".to_string(),
-                    "GET".to_string(),
+                    http::Method::GET,
                 ),
             ]
         );
@@ -1335,8 +1334,8 @@ mod test {
         assert_eq!(
             ret,
             vec![
-                ("/".to_string(), "GET".to_string(),),
-                ("/".to_string(), "POST".to_string(),),
+                ("/".to_string(), http::Method::GET,),
+                ("/".to_string(), http::Method::POST),
             ]
         );
     }

dropshot/src/server.rs

@@ -206,7 +206,7 @@ impl<C: ServerContext> HttpServerStarter<C> {
 
         for (path, method, _) in &app_state.router {
             debug!(&log, "registered endpoint";
-                "method" => &method,
+                "method" => %method,
                 "path" => &path
             );
         }