From 115e74ad6f81308b0c413d78d8390424848fe81b Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 2 Oct 2024 13:31:55 -0700 Subject: [PATCH 1/3] allow overriding operation_id --- dropshot/tests/test_openapi.json | 45 ++++ dropshot/tests/test_openapi.rs | 33 ++- dropshot/tests/test_openapi_fuller.json | 45 ++++ dropshot_endpoint/src/api_trait.rs | 37 ++++ dropshot_endpoint/src/channel.rs | 25 +++ dropshot_endpoint/src/endpoint.rs | 24 +++ dropshot_endpoint/src/metadata.rs | 17 +- .../tests/output/api_trait_operation_id.rs | 193 ++++++++++++++++++ .../tests/output/channel_operation_id.rs | 63 ++++++ .../tests/output/endpoint_operation_id.rs | 64 ++++++ 10 files changed, 541 insertions(+), 5 deletions(-) create mode 100644 dropshot_endpoint/tests/output/api_trait_operation_id.rs create mode 100644 dropshot_endpoint/tests/output/channel_operation_id.rs create mode 100644 dropshot_endpoint/tests/output/endpoint_operation_id.rs diff --git a/dropshot/tests/test_openapi.json b/dropshot/tests/test_openapi.json index 9ce08e20..79ff0c49 100644 --- a/dropshot/tests/test_openapi.json +++ b/dropshot/tests/test_openapi.json @@ -200,6 +200,32 @@ } } }, + "/first_thing": { + "get": { + "tags": [ + "it" + ], + "operationId": "vzeroupper", + "responses": { + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Response" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/impairment": { "get": { "tags": [ @@ -269,6 +295,25 @@ } } }, + "/other_thing": { + "get": { + "tags": [ + "it" + ], + "operationId": "vzerolower", + "responses": { + "default": { + "description": "", + "content": { + "*/*": { + "schema": {} + } + } + } + }, + "x-dropshot-websocket": {} + } + }, "/playing/a/bit/nicer": { "get": { "tags": [ diff --git a/dropshot/tests/test_openapi.rs b/dropshot/tests/test_openapi.rs index 039fd7e3..7fbec5ac 100644 --- a/dropshot/tests/test_openapi.rs +++ b/dropshot/tests/test_openapi.rs @@ -1,8 +1,7 @@ // Copyright 2023 Oxide Computer Company -use dropshot::Body; use dropshot::{ - endpoint, http_response_found, http_response_see_other, + channel, endpoint, http_response_found, http_response_see_other, http_response_temporary_redirect, ApiDescription, ApiDescriptionRegisterError, FreeformBody, HttpError, HttpResponseAccepted, HttpResponseCreated, HttpResponseDeleted, HttpResponseFound, @@ -11,6 +10,7 @@ use dropshot::{ PaginationParams, Path, Query, RequestContext, ResultsPage, TagConfig, TagDetails, TypedBody, UntypedBody, }; +use dropshot::{Body, WebsocketConnection}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::{collections::HashMap, io::Cursor, str::from_utf8}; @@ -473,6 +473,33 @@ async fn handler25( Ok(HttpResponseCreated(Response {})) } +// test: Overridden operation id +#[endpoint { + operation_id = "vzeroupper", + method = GET, + path = "/first_thing", + tags = ["it"] +}] +async fn handler26( + _rqctx: RequestContext<()>, +) -> Result, HttpError> { + Ok(HttpResponseCreated(Response {})) +} + +// test: websocket using overriden operation id +#[channel { + protocol = WEBSOCKETS, + operation_id = "vzerolower", + path = "/other_thing", + tags = ["it"] +}] +async fn handler27( + _rqctx: RequestContext<()>, + _: WebsocketConnection, +) -> dropshot::WebsocketChannelResult { + Ok(()) +} + fn make_api( maybe_tag_config: Option, ) -> Result, ApiDescriptionRegisterError> { @@ -507,6 +534,8 @@ fn make_api( api.register(handler23)?; api.register(handler24)?; api.register(handler25)?; + api.register(handler26)?; + api.register(handler27)?; Ok(api) } diff --git a/dropshot/tests/test_openapi_fuller.json b/dropshot/tests/test_openapi_fuller.json index eabb5bd5..3a4f3124 100644 --- a/dropshot/tests/test_openapi_fuller.json +++ b/dropshot/tests/test_openapi_fuller.json @@ -208,6 +208,32 @@ } } }, + "/first_thing": { + "get": { + "tags": [ + "it" + ], + "operationId": "vzeroupper", + "responses": { + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Response" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/impairment": { "get": { "tags": [ @@ -277,6 +303,25 @@ } } }, + "/other_thing": { + "get": { + "tags": [ + "it" + ], + "operationId": "vzerolower", + "responses": { + "default": { + "description": "", + "content": { + "*/*": { + "schema": {} + } + } + } + }, + "x-dropshot-websocket": {} + } + }, "/playing/a/bit/nicer": { "get": { "tags": [ diff --git a/dropshot_endpoint/src/api_trait.rs b/dropshot_endpoint/src/api_trait.rs index 2a87e4b7..1f09db5b 100644 --- a/dropshot_endpoint/src/api_trait.rs +++ b/dropshot_endpoint/src/api_trait.rs @@ -1759,4 +1759,41 @@ mod tests { &prettyplease::unparse(&parse_quote! { #item }), ); } + + #[test] + fn test_api_trait_operation_id() { + let (item, errors) = do_trait( + quote! {}, + quote! { + trait MyTrait { + type Context; + + #[endpoint { + operation_id = "vzerolower", + method = GET, + path = "/xyz" + }] + async fn handler_xyz( + rqctx: RequestContext, + ) -> Result, HttpError>; + + #[channel { + protocol = WEBSOCKETS, + path = "/ws", + operation_id = "vzeroupper", + }] + async fn handler_ws( + rqctx: RequestContext, + upgraded: WebsocketConnection, + ) -> WebsocketChannelResult; + } + }, + ); + + assert!(errors.is_empty()); + assert_contents( + "tests/output/api_trait_operation_id.rs", + &prettyplease::unparse(&parse_quote! { #item }), + ); + } } diff --git a/dropshot_endpoint/src/channel.rs b/dropshot_endpoint/src/channel.rs index d3a199ce..d6440cf6 100644 --- a/dropshot_endpoint/src/channel.rs +++ b/dropshot_endpoint/src/channel.rs @@ -613,4 +613,29 @@ mod tests { &prettyplease::unparse(&parse_quote! { #item }), ); } + + #[test] + fn test_channel_operation_id() { + let (item, errors) = do_channel( + quote! { + protocol = WEBSOCKETS, + path = "/my/ws/channel", + operation_id = "vzeroupper" + }, + quote! { + async fn handler_xyz( + _rqctx: RequestContext<()>, + _ws: WebsocketConnection, + ) -> Result, HttpError> { + Ok(()) + } + }, + ); + + assert!(errors.is_empty()); + assert_contents( + "tests/output/channel_operation_id.rs", + &prettyplease::unparse(&parse_quote! { #item }), + ); + } } diff --git a/dropshot_endpoint/src/endpoint.rs b/dropshot_endpoint/src/endpoint.rs index 31361909..d2601085 100644 --- a/dropshot_endpoint/src/endpoint.rs +++ b/dropshot_endpoint/src/endpoint.rs @@ -920,4 +920,28 @@ mod tests { Some("endpoint `handler_xyz` must have at least one RequestContext argument".to_string()) ); } + + #[test] + fn test_operation_id() { + let (item, errors) = do_endpoint( + quote! { + method = GET, + path = "/a/b/c", + operation_id = "vzeroupper" + }, + quote! { + pub async fn handler_xyz( + _rqctx: RequestContext<()>, + ) -> Result, HttpError> { + Ok(()) + } + }, + ); + + assert!(errors.is_empty()); + assert_contents( + "tests/output/endpoint_operation_id.rs", + &prettyplease::unparse(&parse_quote! { #item }), + ); + } } diff --git a/dropshot_endpoint/src/metadata.rs b/dropshot_endpoint/src/metadata.rs index 09614fe8..ff8663c5 100644 --- a/dropshot_endpoint/src/metadata.rs +++ b/dropshot_endpoint/src/metadata.rs @@ -41,6 +41,8 @@ impl MethodType { #[derive(Deserialize, Debug)] pub(crate) struct EndpointMetadata { + #[serde(default)] + pub(crate) operation_id: Option, pub(crate) method: MethodType, pub(crate) path: String, #[serde(default)] @@ -59,7 +61,7 @@ impl EndpointMetadata { get_crate(self._dropshot_crate.as_deref()) } - /// Validates metadata, returning an `EndpointMetadata` if valid. + /// Validates metadata, returning a `ValidatedEndpointMetadata` if valid. /// /// Note: the only reason we pass in attr here is to provide a span for /// error reporting. As of Rust 1.76, just passing in `attr.span()` produces @@ -74,6 +76,7 @@ impl EndpointMetadata { let errors = errors.new(); let EndpointMetadata { + operation_id, method, path, tags, @@ -130,6 +133,7 @@ impl EndpointMetadata { None } else if let Some(content_type) = content_type { Some(ValidatedEndpointMetadata { + operation_id, method, path, tags, @@ -145,6 +149,7 @@ impl EndpointMetadata { /// A validated form of endpoint metadata. pub(crate) struct ValidatedEndpointMetadata { + operation_id: Option, method: MethodType, path: String, tags: Vec, @@ -163,6 +168,8 @@ impl ValidatedEndpointMetadata { ) -> TokenStream { let path = &self.path; let content_type = self.content_type; + let operation_id = + self.operation_id.as_deref().unwrap_or(endpoint_name); let method_ident = format_ident!("{}", self.method.as_str()); let summary = doc.summary.as_ref().map(|summary| { @@ -192,7 +199,7 @@ impl ValidatedEndpointMetadata { ApiEndpointKind::Regular(endpoint_fn) => { quote_spanned! {endpoint_fn.span()=> #dropshot::ApiEndpoint::new( - #endpoint_name.to_string(), + #operation_id.to_string(), #endpoint_fn, #dropshot::Method::#method_ident, #content_type, @@ -212,7 +219,7 @@ impl ValidatedEndpointMetadata { // Seems pretty unobjectionable. quote_spanned! {attr.pound_token.span()=> #dropshot::ApiEndpoint::new_for_types::<(#(#extractor_types,)*), #ret_ty>( - #endpoint_name.to_string(), + #operation_id.to_string(), #dropshot::Method::#method_ident, #content_type, #path, @@ -241,6 +248,8 @@ pub(crate) enum ChannelProtocol { #[derive(Deserialize, Debug)] pub(crate) struct ChannelMetadata { pub(crate) protocol: ChannelProtocol, + #[serde(default)] + pub(crate) operation_id: Option, pub(crate) path: String, #[serde(default)] pub(crate) tags: Vec, @@ -273,6 +282,7 @@ impl ChannelMetadata { let ChannelMetadata { protocol: ChannelProtocol::WEBSOCKETS, + operation_id, path, tags, unpublished, @@ -307,6 +317,7 @@ impl ChannelMetadata { // Validating channel metadata also validates the corresponding // endpoint metadata. let inner = ValidatedEndpointMetadata { + operation_id, method: MethodType::GET, path, tags, diff --git a/dropshot_endpoint/tests/output/api_trait_operation_id.rs b/dropshot_endpoint/tests/output/api_trait_operation_id.rs new file mode 100644 index 00000000..52a2fc87 --- /dev/null +++ b/dropshot_endpoint/tests/output/api_trait_operation_id.rs @@ -0,0 +1,193 @@ +trait MyTrait: 'static { + type Context: dropshot::ServerContext; + fn handler_xyz( + rqctx: RequestContext, + ) -> impl ::core::future::Future< + Output = Result, HttpError>, + > + Send + 'static; + fn handler_ws( + rqctx: RequestContext, + upgraded: WebsocketConnection, + ) -> impl ::core::future::Future + Send + 'static; +} +/// Support module for the Dropshot API trait [`MyTrait`](MyTrait). +#[automatically_derived] +mod my_trait_mod { + use super::*; + const _: fn() = || { + trait TypeEq { + type This: ?Sized; + } + impl TypeEq for T { + type This = Self; + } + fn validate_websocket_connection_type() + where + T: ?Sized + TypeEq, + {} + validate_websocket_connection_type::(); + }; + /// Generate a _stub_ API description for [`MyTrait`], meant for OpenAPI + /// generation. + /// + /// Unlike [`api_description`], this function does not require an implementation + /// of [`MyTrait`] to be available, instead generating handlers that panic. + /// The return value is of type [`ApiDescription`]`<`[`StubContext`]`>`. + /// + /// The main use of this function is in cases where [`MyTrait`] is defined + /// in a separate crate from its implementation. The OpenAPI spec can then be + /// generated directly from the stub API description. + /// + /// ## Example + /// + /// A function that prints the OpenAPI spec to standard output: + /// + /// ```rust,ignore + /// fn print_openapi_spec() { + /// let stub = my_trait_mod::stub_api_description().unwrap(); + /// + /// // Generate OpenAPI spec from `stub`. + /// let spec = stub.openapi("MyTrait", "0.1.0"); + /// spec.write(&mut std::io::stdout()).unwrap(); + /// } + /// ``` + /// + /// [`MyTrait`]: MyTrait + /// [`api_description`]: my_trait_mod::api_description + /// [`ApiDescription`]: dropshot::ApiDescription + /// [`StubContext`]: dropshot::StubContext + #[automatically_derived] + pub fn stub_api_description() -> ::std::result::Result< + dropshot::ApiDescription, + dropshot::ApiDescriptionBuildErrors, + > { + let mut dropshot_api = dropshot::ApiDescription::new(); + let mut dropshot_errors: Vec = Vec::new(); + { + let endpoint_handler_xyz = dropshot::ApiEndpoint::new_for_types::< + (), + Result, HttpError>, + >( + "vzerolower".to_string(), + dropshot::Method::GET, + "application/json", + "/xyz", + ); + if let Err(error) = dropshot_api.register(endpoint_handler_xyz) { + dropshot_errors.push(error); + } + } + { + let endpoint_handler_ws = dropshot::ApiEndpoint::new_for_types::< + (dropshot::WebsocketUpgrade,), + dropshot::WebsocketEndpointResult, + >( + "vzeroupper".to_string(), + dropshot::Method::GET, + "application/json", + "/ws", + ); + if let Err(error) = dropshot_api.register(endpoint_handler_ws) { + dropshot_errors.push(error); + } + } + if !dropshot_errors.is_empty() { + Err(dropshot::ApiDescriptionBuildErrors::new(dropshot_errors)) + } else { + Ok(dropshot_api) + } + } + /// Given an implementation of [`MyTrait`], generate an API description. + /// + /// This function accepts a single type argument `ServerImpl`, turning it into a + /// Dropshot [`ApiDescription`]``. + /// The returned `ApiDescription` can then be turned into a Dropshot server that + /// accepts a concrete `Context`. + /// + /// ## Example + /// + /// ```rust,ignore + /// /// A type used to define the concrete implementation for `MyTrait`. + /// /// + /// /// This type is never constructed -- it is just a place to define your + /// /// implementation of `MyTrait`. + /// enum MyTraitImpl {} + /// + /// impl MyTrait for MyTraitImpl { + /// type Context = /* context type */; + /// + /// // ... trait methods + /// } + /// + /// #[tokio::main] + /// async fn main() { + /// // Generate the description for `MyTraitImpl`. + /// let description = my_trait_mod::api_description::().unwrap(); + /// + /// // Create a value of the concrete context type. + /// let context = /* some value of type `MyTraitImpl::Context` */; + /// + /// // Create a Dropshot server from the description. + /// let log = /* ... */; + /// let server = dropshot::ServerBuilder::new(description, context, log) + /// .start() + /// .unwrap(); + /// + /// // Run the server. + /// server.await + /// } + /// ``` + /// + /// [`ApiDescription`]: dropshot::ApiDescription + /// [`MyTrait`]: MyTrait + /// [`Context`]: MyTrait::Context + #[automatically_derived] + pub fn api_description() -> ::std::result::Result< + dropshot::ApiDescription<::Context>, + dropshot::ApiDescriptionBuildErrors, + > { + let mut dropshot_api = dropshot::ApiDescription::new(); + let mut dropshot_errors: Vec = Vec::new(); + { + let endpoint_handler_xyz = dropshot::ApiEndpoint::new( + "vzerolower".to_string(), + ::handler_xyz, + dropshot::Method::GET, + "application/json", + "/xyz", + ); + if let Err(error) = dropshot_api.register(endpoint_handler_xyz) { + dropshot_errors.push(error); + } + } + { + async fn handler_ws_adapter( + arg0: RequestContext<::Context>, + __dropshot_websocket: dropshot::WebsocketUpgrade, + ) -> dropshot::WebsocketEndpointResult { + __dropshot_websocket + .handle(move |__dropshot_websocket: WebsocketConnection| async move { + ::handler_ws(arg0, __dropshot_websocket) + .await + }) + } + { + let endpoint_handler_ws = dropshot::ApiEndpoint::new( + "vzeroupper".to_string(), + handler_ws_adapter::, + dropshot::Method::GET, + "application/json", + "/ws", + ); + if let Err(error) = dropshot_api.register(endpoint_handler_ws) { + dropshot_errors.push(error); + } + } + } + if !dropshot_errors.is_empty() { + Err(dropshot::ApiDescriptionBuildErrors::new(dropshot_errors)) + } else { + Ok(dropshot_api) + } + } +} diff --git a/dropshot_endpoint/tests/output/channel_operation_id.rs b/dropshot_endpoint/tests/output/channel_operation_id.rs new file mode 100644 index 00000000..16b95b6e --- /dev/null +++ b/dropshot_endpoint/tests/output/channel_operation_id.rs @@ -0,0 +1,63 @@ +const _: fn() = || { + struct NeedRequestContext( + as dropshot::RequestContextArgument>::Context, + ); +}; +const _: fn() = || { + trait TypeEq { + type This: ?Sized; + } + impl TypeEq for T { + type This = Self; + } + fn validate_websocket_connection_type() + where + T: ?Sized + TypeEq, + {} + validate_websocket_connection_type::(); +}; +#[allow(non_camel_case_types, missing_docs)] +///API Endpoint: handler_xyz +struct handler_xyz {} +#[allow(non_upper_case_globals, missing_docs)] +///API Endpoint: handler_xyz +const handler_xyz: handler_xyz = handler_xyz {}; +impl From +for dropshot::ApiEndpoint< + as dropshot::RequestContextArgument>::Context, +> { + fn from(_: handler_xyz) -> Self { + #[allow(clippy::unused_async)] + async fn handler_xyz( + _rqctx: RequestContext<()>, + _ws: WebsocketConnection, + ) -> Result, HttpError> { + Ok(()) + } + const _: fn() = || { + fn future_endpoint_must_be_send(_t: T) {} + fn check_future_bounds( + arg0: RequestContext<()>, + __dropshot_websocket: WebsocketConnection, + ) { + future_endpoint_must_be_send(handler_xyz(arg0, __dropshot_websocket)); + } + }; + async fn handler_xyz_adapter( + arg0: RequestContext<()>, + __dropshot_websocket: dropshot::WebsocketUpgrade, + ) -> dropshot::WebsocketEndpointResult { + __dropshot_websocket + .handle(move |__dropshot_websocket: WebsocketConnection| async move { + handler_xyz(arg0, __dropshot_websocket).await + }) + } + dropshot::ApiEndpoint::new( + "vzeroupper".to_string(), + handler_xyz_adapter, + dropshot::Method::GET, + "application/json", + "/my/ws/channel", + ) + } +} diff --git a/dropshot_endpoint/tests/output/endpoint_operation_id.rs b/dropshot_endpoint/tests/output/endpoint_operation_id.rs new file mode 100644 index 00000000..586eb3b3 --- /dev/null +++ b/dropshot_endpoint/tests/output/endpoint_operation_id.rs @@ -0,0 +1,64 @@ +const _: fn() = || { + struct NeedRequestContext( + as dropshot::RequestContextArgument>::Context, + ); +}; +const _: fn() = || { + trait ResultTrait { + type T; + type E; + } + impl ResultTrait for Result + where + TT: dropshot::HttpResponse, + { + type T = TT; + type E = EE; + } + struct NeedHttpResponse(, HttpError> as ResultTrait>::T); + trait TypeEq { + type This: ?Sized; + } + impl TypeEq for T { + type This = Self; + } + fn validate_result_error_type() + where + T: ?Sized + TypeEq, + {} + validate_result_error_type::< + , HttpError> as ResultTrait>::E, + >(); +}; +#[allow(non_camel_case_types, missing_docs)] +///API Endpoint: handler_xyz +pub struct handler_xyz {} +#[allow(non_upper_case_globals, missing_docs)] +///API Endpoint: handler_xyz +pub const handler_xyz: handler_xyz = handler_xyz {}; +impl From +for dropshot::ApiEndpoint< + as dropshot::RequestContextArgument>::Context, +> { + fn from(_: handler_xyz) -> Self { + #[allow(clippy::unused_async)] + pub async fn handler_xyz( + _rqctx: RequestContext<()>, + ) -> Result, HttpError> { + Ok(()) + } + const _: fn() = || { + fn future_endpoint_must_be_send(_t: T) {} + fn check_future_bounds(arg0: RequestContext<()>) { + future_endpoint_must_be_send(handler_xyz(arg0)); + } + }; + dropshot::ApiEndpoint::new( + "vzeroupper".to_string(), + handler_xyz, + dropshot::Method::GET, + "application/json", + "/a/b/c", + ) + } +} From d30fc789116e5510ae253dbf7f6eb22322bb34e0 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 2 Oct 2024 13:35:57 -0700 Subject: [PATCH 2/3] mention in the docs --- dropshot/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/dropshot/src/lib.rs b/dropshot/src/lib.rs index 10f8d967..96aa20d9 100644 --- a/dropshot/src/lib.rs +++ b/dropshot/src/lib.rs @@ -304,6 +304,7 @@ //! path = "/path/name/with/{named}/{variables}", //! //! // Optional fields +//! operation_id = "my_operation" // (default: name of the function) //! tags = [ "all", "your", "OpenAPI", "tags" ], //! }] //! ``` From 7a8683df1ac5c9cd936114f83f8bb33a80e0f477 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 2 Oct 2024 16:42:14 -0700 Subject: [PATCH 3/3] fix whitespace --- dropshot_endpoint/src/api_trait.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dropshot_endpoint/src/api_trait.rs b/dropshot_endpoint/src/api_trait.rs index 1f09db5b..9fffac94 100644 --- a/dropshot_endpoint/src/api_trait.rs +++ b/dropshot_endpoint/src/api_trait.rs @@ -1780,7 +1780,7 @@ mod tests { #[channel { protocol = WEBSOCKETS, path = "/ws", - operation_id = "vzeroupper", + operation_id = "vzeroupper", }] async fn handler_ws( rqctx: RequestContext,