oxidecomputer · sunshowers · Nov 16, 2024 · Nov 12, 2024 · Nov 12, 2024 · Nov 15, 2024
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
@@ -21,6 +21,11 @@ https://github.com/oxidecomputer/dropshot/compare/v0.13.0\...HEAD[Full list of c
 +
 Defining the old config option will produce an error, guiding you to perform the rename.
 
+* Within `RequestContext`, endpoint-specific metadata has been moved to an `endpoint` field:
+** `rqctx.operation_id` is now `rqctx.endpoint.operation_id`.
+** `rqctx.path_variables` is now `rqctx.endpoint.variables`.
+** `rqctx.body_content_type` is now `rqctx.endpoint.body_content_type`.
+
 == 0.13.0 (released 2024-11-13)
 
 https://github.com/oxidecomputer/dropshot/compare/v0.12.0\...v0.13.0[Full list of commits]

diff --git a/dropshot/src/extractor/body.rs b/dropshot/src/extractor/body.rs
@@ -153,7 +153,7 @@ where
     let body_content_type =
         ApiEndpointBodyContentType::from_mime_type(&mime_type)
             .map_err(|e| HttpError::for_bad_request(None, e))?;
-    let expected_content_type = rqctx.body_content_type.clone();
+    let expected_content_type = rqctx.endpoint.body_content_type.clone();
 
     use ApiEndpointBodyContentType::*;
 

diff --git a/dropshot/src/extractor/path.rs b/dropshot/src/extractor/path.rs
@@ -43,7 +43,8 @@ where
     async fn from_request<Context: ServerContext>(
         rqctx: &RequestContext<Context>,
     ) -> Result<Path<PathType>, HttpError> {
-        let params: PathType = http_extract_path_params(&rqctx.path_variables)?;
+        let params: PathType =
+            http_extract_path_params(&rqctx.endpoint.variables)?;
         Ok(Path { inner: params })
     }
 

diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs
@@ -73,19 +73,16 @@ pub type HttpHandlerResult = Result<Response<Body>, HttpError>;
 
 /// Handle for various interfaces useful during request processing.
 #[derive(Debug)]
+#[non_exhaustive]
 pub struct RequestContext<Context: ServerContext> {
     /// shared server state
     pub server: Arc<DropshotState<Context>>,
-    /// HTTP request routing variables
-    pub path_variables: VariableSet,
-    /// expected request body mime type
-    pub body_content_type: ApiEndpointBodyContentType,
+    /// Endpoint-specific information.
+    pub endpoint: RequestEndpointMetadata,
     /// unique id assigned to this request
     pub request_id: String,
     /// logger for this specific request
     pub log: Logger,
-    /// The operation ID for the endpoint handler method
-    pub operation_id: String,
     /// basic request information (method, URI, etc.)
     pub request: RequestInfo,
 }
@@ -208,6 +205,21 @@ impl<Context: ServerContext> RequestContext<Context> {
     }
 }
 
+/// Endpoint-specific information for a request.
+///
+/// This is part of [`RequestContext`].
+#[derive(Debug)]
+pub struct RequestEndpointMetadata {
+    /// The operation ID for the endpoint handler method.
+    pub operation_id: String,
+
+    /// HTTP request routing variables.
+    pub variables: VariableSet,
+
+    /// The expected request body MIME type.
+    pub body_content_type: ApiEndpointBodyContentType,
+}
+
 /// Helper trait for extracting the underlying Context type from the
 /// first argument to an endpoint. This trait exists to help the
 /// endpoint macro parse this argument.

diff --git a/dropshot/src/lib.rs b/dropshot/src/lib.rs
@@ -914,6 +914,7 @@ pub use handler::HttpResponseTemporaryRedirect;
 pub use handler::HttpResponseUpdatedNoContent;
 pub use handler::NoHeaders;
 pub use handler::RequestContext;
+pub use handler::RequestEndpointMetadata;
 pub use handler::RequestInfo;
 pub use http_util::CONTENT_TYPE_JSON;
 pub use http_util::CONTENT_TYPE_MULTIPART_FORM_DATA;

diff --git a/dropshot/src/router.rs b/dropshot/src/router.rs
@@ -9,7 +9,7 @@ use crate::from_map::MapError;
 use crate::from_map::MapValue;
 use crate::server::ServerContext;
 use crate::ApiEndpoint;
-use crate::ApiEndpointBodyContentType;
+use crate::RequestEndpointMetadata;
 use http::Method;
 use http::StatusCode;
 use percent_encoding::percent_decode_str;
@@ -208,17 +208,13 @@ impl MapValue for VariableValue {
     }
 }
 
-/// `RouterLookupResult` represents the result of invoking
-/// `HttpRouter::lookup_route()`.  A successful route lookup includes
-/// the handler, a mapping of variables in the configured path to the
-/// corresponding values in the actual path, and the expected body
-/// content type.
+/// The result of invoking `HttpRouter::lookup_route()`.
+///
+/// A successful route lookup includes the handler and endpoint-related metadata.
 #[derive(Debug)]
 pub struct RouterLookupResult<Context: ServerContext> {
     pub handler: Arc<dyn RouteHandler<Context>>,
-    pub operation_id: String,
-    pub variables: VariableSet,
-    pub body_content_type: ApiEndpointBodyContentType,
+    pub endpoint: RequestEndpointMetadata,
 }
 
 impl<Context: ServerContext> HttpRouterNode<Context> {
@@ -520,9 +516,11 @@ impl<Context: ServerContext> HttpRouter<Context> {
         ) {
             return Ok(RouterLookupResult {
                 handler: Arc::clone(&handler.handler),
-                operation_id: handler.operation_id.clone(),
-                variables,
-                body_content_type: handler.body_content_type.clone(),
+                endpoint: RequestEndpointMetadata {
+                    operation_id: handler.operation_id.clone(),
+                    variables,
+                    body_content_type: handler.body_content_type.clone(),
+                },
             });
         }
 
@@ -1227,16 +1225,16 @@ mod test {
         let result =
             router.lookup_route_unversioned(&Method::GET, "/".into()).unwrap();
         assert_eq!(result.handler.label(), "h1");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
         let result =
             router.lookup_route_unversioned(&Method::GET, "//".into()).unwrap();
         assert_eq!(result.handler.label(), "h1");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
         let result = router
             .lookup_route_unversioned(&Method::GET, "///".into())
             .unwrap();
         assert_eq!(result.handler.label(), "h1");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
 
         // Now insert a handler for a different method at the root.  Verify that
         // we get both this handler and the previous one if we ask for the
@@ -1249,14 +1247,14 @@ mod test {
         let result =
             router.lookup_route_unversioned(&Method::PUT, "/".into()).unwrap();
         assert_eq!(result.handler.label(), "h2");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
         let result =
             router.lookup_route_unversioned(&Method::GET, "/".into()).unwrap();
         assert_eq!(result.handler.label(), "h1");
+        assert!(result.endpoint.variables.is_empty());
         assert!(router
             .lookup_route_unversioned(&Method::DELETE, "/".into())
             .is_err());
-        assert!(result.variables.is_empty());
 
         // Now insert a handler one level deeper.  Verify that all the previous
         // handlers behave as we expect, and that we have one handler at the new
@@ -1272,31 +1270,31 @@ mod test {
         let result =
             router.lookup_route_unversioned(&Method::PUT, "/".into()).unwrap();
         assert_eq!(result.handler.label(), "h2");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
         let result =
             router.lookup_route_unversioned(&Method::GET, "/".into()).unwrap();
         assert_eq!(result.handler.label(), "h1");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
         let result = router
             .lookup_route_unversioned(&Method::GET, "/foo".into())
             .unwrap();
         assert_eq!(result.handler.label(), "h3");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
         let result = router
             .lookup_route_unversioned(&Method::GET, "/foo/".into())
             .unwrap();
         assert_eq!(result.handler.label(), "h3");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
         let result = router
             .lookup_route_unversioned(&Method::GET, "//foo//".into())
             .unwrap();
         assert_eq!(result.handler.label(), "h3");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
         let result = router
             .lookup_route_unversioned(&Method::GET, "/foo//".into())
             .unwrap();
         assert_eq!(result.handler.label(), "h3");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
         assert!(router
             .lookup_route_unversioned(&Method::PUT, "/foo".into())
             .is_err());
@@ -1404,7 +1402,7 @@ mod test {
             .lookup_route_unversioned(&Method::GET, "/not{a}variable".into())
             .unwrap();
         assert_eq!(result.handler.label(), "h4");
-        assert!(result.variables.is_empty());
+        assert!(result.endpoint.variables.is_empty());
         assert!(router
             .lookup_route_unversioned(&Method::GET, "/not{b}variable".into())
             .is_err());
@@ -1433,11 +1431,11 @@ mod test {
             .unwrap();
         assert_eq!(result.handler.label(), "h5");
         assert_eq!(
-            result.variables.keys().collect::<Vec<&String>>(),
+            result.endpoint.variables.keys().collect::<Vec<&String>>(),
             vec!["project_id"]
         );
         assert_eq!(
-            *result.variables.get("project_id").unwrap(),
+            *result.endpoint.variables.get("project_id").unwrap(),
             VariableValue::String("p12345".to_string())
         );
         assert!(router
@@ -1451,7 +1449,7 @@ mod test {
             .unwrap();
         assert_eq!(result.handler.label(), "h5");
         assert_eq!(
-            *result.variables.get("project_id").unwrap(),
+            *result.endpoint.variables.get("project_id").unwrap(),
             VariableValue::String("p12345".to_string())
         );
         let result = router
@@ -1462,7 +1460,7 @@ mod test {
             .unwrap();
         assert_eq!(result.handler.label(), "h5");
         assert_eq!(
-            *result.variables.get("project_id").unwrap(),
+            *result.endpoint.variables.get("project_id").unwrap(),
             VariableValue::String("p12345".to_string())
         );
         // Trick question!
@@ -1474,7 +1472,7 @@ mod test {
             .unwrap();
         assert_eq!(result.handler.label(), "h5");
         assert_eq!(
-            *result.variables.get("project_id").unwrap(),
+            *result.endpoint.variables.get("project_id").unwrap(),
             VariableValue::String("{project_id}".to_string())
         );
     }
@@ -1497,19 +1495,19 @@ mod test {
             .unwrap();
         assert_eq!(result.handler.label(), "h6");
         assert_eq!(
-            result.variables.keys().collect::<Vec<&String>>(),
+            result.endpoint.variables.keys().collect::<Vec<&String>>(),
             vec!["fwrule_id", "instance_id", "project_id"]
         );
         assert_eq!(
-            *result.variables.get("project_id").unwrap(),
+            *result.endpoint.variables.get("project_id").unwrap(),
             VariableValue::String("p1".to_string())
         );
         assert_eq!(
-            *result.variables.get("instance_id").unwrap(),
+            *result.endpoint.variables.get("instance_id").unwrap(),
             VariableValue::String("i2".to_string())
         );
         assert_eq!(
-            *result.variables.get("fwrule_id").unwrap(),
+            *result.endpoint.variables.get("fwrule_id").unwrap(),
             VariableValue::String("fw3".to_string())
         );
     }
@@ -1568,7 +1566,7 @@ mod test {
             .unwrap();
 
         assert_eq!(
-            result.variables.get("path"),
+            result.endpoint.variables.get("path"),
             Some(&VariableValue::Components(vec![
                 "missiles".to_string(),
                 "launch".to_string()
@@ -1604,7 +1602,8 @@ mod test {
             .unwrap();
 
         let path =
-            from_map::<MyPath, VariableValue>(&result.variables).unwrap();
+            from_map::<MyPath, VariableValue>(&result.endpoint.variables)
+                .unwrap();
 
         assert_eq!(path.t, "console");
         assert_eq!(path.r, "missiles");

diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs
@@ -910,9 +910,7 @@ async fn http_request_handle<C: ServerContext>(
     let rqctx = RequestContext {
         server: Arc::clone(&server),
         request: RequestInfo::new(&request, remote_addr),
-        path_variables: lookup_result.variables,
-        body_content_type: lookup_result.body_content_type,
-        operation_id: lookup_result.operation_id,
+        endpoint: lookup_result.endpoint,
         request_id: request_id.to_string(),
         log: request_log,
     };

diff --git a/dropshot/src/websocket.rs b/dropshot/src/websocket.rs
@@ -353,8 +353,8 @@ mod tests {
     use crate::router::HttpRouter;
     use crate::server::{DropshotState, ServerConfig};
     use crate::{
-        ExclusiveExtractor, HttpError, RequestContext, RequestInfo,
-        VersionPolicy, WebsocketUpgrade,
+        ExclusiveExtractor, HttpError, RequestContext, RequestEndpointMetadata,
+        RequestInfo, VersionPolicy, WebsocketUpgrade,
     };
     use debug_ignore::DebugIgnore;
     use http::Request;
@@ -399,9 +399,11 @@ mod tests {
                 version_policy: VersionPolicy::Unversioned,
             }),
             request: RequestInfo::new(&request, remote_addr),
-            path_variables: Default::default(),
-            body_content_type: Default::default(),
-            operation_id: "".to_string(),
+            endpoint: RequestEndpointMetadata {
+                operation_id: "".to_string(),
+                variables: Default::default(),
+                body_content_type: Default::default(),
+            },
             request_id: "".to_string(),
             log: log.clone(),
         };