allow endpoints and channels to specify the operation_id (#1127)

Author: davepacheco

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" ],
 //! }]
 //! ```

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": [

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<HttpResponseCreated<Response>, 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<TagConfig>,
 ) -> Result<ApiDescription<()>, 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)
 }
 

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": [

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<Self::Context>,
+                    ) -> Result<HttpResponseOk<()>, HttpError>;
+
+                    #[channel {
+                        protocol = WEBSOCKETS,
+                        path = "/ws",
+                        operation_id = "vzeroupper",
+                    }]
+                    async fn handler_ws(
+                        rqctx: RequestContext<Self::Context>,
+                        upgraded: WebsocketConnection,
+                    ) -> WebsocketChannelResult;
+                }
+            },
+        );
+
+        assert!(errors.is_empty());
+        assert_contents(
+            "tests/output/api_trait_operation_id.rs",
+            &prettyplease::unparse(&parse_quote! { #item }),
+        );
+    }
 }

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<HttpResponseOk<()>, HttpError> {
+                    Ok(())
+                }
+            },
+        );
+
+        assert!(errors.is_empty());
+        assert_contents(
+            "tests/output/channel_operation_id.rs",
+            &prettyplease::unparse(&parse_quote! { #item }),
+        );
+    }
 }

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<HttpResponseOk<()>, HttpError> {
+                    Ok(())
+                }
+            },
+        );
+
+        assert!(errors.is_empty());
+        assert_contents(
+            "tests/output/endpoint_operation_id.rs",
+            &prettyplease::unparse(&parse_quote! { #item }),
+        );
+    }
 }

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<String>,
     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<String>,
     method: MethodType,
     path: String,
     tags: Vec<String>,
@@ -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<String>,
     pub(crate) path: String,
     #[serde(default)]
     pub(crate) tags: Vec<String>,
@@ -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,