diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 3cac092f7..c75e7f85b 100644 --- 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 index 07f902d89..144e7415b 100644 --- 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 index 24fb25044..fe3da2bc8 100644 --- a/dropshot/src/extractor/path.rs +++ b/dropshot/src/extractor/path.rs @@ -43,7 +43,8 @@ where async fn from_request( rqctx: &RequestContext, ) -> Result, 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 index c04056e34..5e2bde4dc 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -73,19 +73,16 @@ pub type HttpHandlerResult = Result, HttpError>; /// Handle for various interfaces useful during request processing. #[derive(Debug)] +#[non_exhaustive] pub struct RequestContext { /// shared server state pub server: Arc>, - /// 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 RequestContext { } } +/// 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 index 265e226cd..319cdde18 100644 --- 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 index 8f43e2bb0..4d1f756fe 100644 --- 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 { pub handler: Arc>, - pub operation_id: String, - pub variables: VariableSet, - pub body_content_type: ApiEndpointBodyContentType, + pub endpoint: RequestEndpointMetadata, } impl HttpRouterNode { @@ -520,9 +516,11 @@ impl HttpRouter { ) { 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::>(), + result.endpoint.variables.keys().collect::>(), 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::>(), + result.endpoint.variables.keys().collect::>(), 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::(&result.variables).unwrap(); + from_map::(&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 index 461e12160..6e789e727 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -910,9 +910,7 @@ async fn http_request_handle( 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 index 67cb07460..f546c5d4c 100644 --- 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(), };