From 8aeb371656578cafe6abe8a2047f37ccce38ff44 Mon Sep 17 00:00:00 2001 From: msrd0 Date: Tue, 14 May 2024 17:27:18 +0200 Subject: [PATCH] Use function name as operation verb if operation id is missing (#483) --- derive/src/endpoint.rs | 27 ++++++++++++++++++++------- src/endpoint.rs | 6 ++++-- src/lib.rs | 2 +- src/openapi/operation.rs | 32 +++++++++++++++++++++++++++----- tests/openapi_specification.json | 4 +++- 5 files changed, 55 insertions(+), 16 deletions(-) diff --git a/derive/src/endpoint.rs b/derive/src/endpoint.rs index 4c3f33c754..27796dcdf4 100644 --- a/derive/src/endpoint.rs +++ b/derive/src/endpoint.rs @@ -372,18 +372,31 @@ fn expand_operation_verb(_: TokenStream) -> Option { } #[cfg(feature = "openapi")] -fn expand_operation_id(operation_id: Option) -> Option { - operation_id.map(|operation_id| { - quote! { - fn operation_id() -> ::core::option::Option<::std::string::String> { - ::core::option::Option::Some(::std::string::String::from(#operation_id)) +fn expand_operation_id(fun_ident: &Ident, operation_id: Option) -> Option { + let op_id = match operation_id { + Some(operation_id) => quote! { + ::gotham_restful::OperationId::Manual( + ::std::string::String::from(#operation_id) + ) + }, + None => { + let verb = fun_ident.to_string(); + quote! { + ::gotham_restful::OperationId::SemiAuto( + ::std::borrow::Cow::Borrowed(#verb) + ) } } + }; + Some(quote! { + fn operation_id() -> ::gotham_restful::OperationId { + #op_id + } }) } #[cfg(not(feature = "openapi"))] -fn expand_operation_id(_: Option) -> Option { +fn expand_operation_id(_: &Ident, _: Option) -> Option { None } @@ -736,7 +749,7 @@ fn expand_endpoint_type( quote!(::gotham_restful::Endpoint) }; let operation_verb = expand_operation_verb(ty.operation_verb()); - let operation_id = expand_operation_id(operation_id); + let operation_id = expand_operation_id(fun_ident, operation_id); let wants_auth = expand_wants_auth(wants_auth, args.iter().any(|arg| arg.ty.is_auth_status())); let code = quote! { #[doc(hidden)] diff --git a/src/endpoint.rs b/src/endpoint.rs index acd2c797dd..91b4a75235 100644 --- a/src/endpoint.rs +++ b/src/endpoint.rs @@ -1,3 +1,5 @@ +#[cfg(feature = "openapi")] +use crate::openapi::operation::OperationId; use crate::{IntoResponse, RequestBody}; use futures_util::future::BoxFuture; use gotham::{ @@ -89,8 +91,8 @@ pub trait Endpoint { /// Replace the automatically generated operation id with a custom one. Only relevant for the /// OpenAPI Specification. #[openapi_only] - fn operation_id() -> Option { - None + fn operation_id() -> OperationId { + OperationId::FullAuto } /// Add a description to the openapi specification. Usually taken from the rustdoc comment diff --git a/src/lib.rs b/src/lib.rs index 00f286607f..cd1a05b1a8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -564,7 +564,7 @@ pub use cors::{handle_cors, CorsConfig, CorsRoute}; #[cfg(feature = "openapi")] mod openapi; #[cfg(feature = "openapi")] -pub use openapi::{builder::OpenapiInfo, router::GetOpenapi}; +pub use openapi::{builder::OpenapiInfo, operation::OperationId, router::GetOpenapi}; mod endpoint; #[cfg(feature = "openapi")] diff --git a/src/openapi/operation.rs b/src/openapi/operation.rs index bf2006fc75..f95406daed 100644 --- a/src/openapi/operation.rs +++ b/src/openapi/operation.rs @@ -10,7 +10,7 @@ use openapi_type::{ }, OpenapiSchema }; -use std::collections::HashMap; +use std::{borrow::Cow, collections::HashMap}; fn new_parameter_data( name: String, @@ -93,6 +93,17 @@ impl OperationParams { } } +#[derive(Debug, Default)] +pub enum OperationId { + /// Automatically generate the operation id based on path and operation verb. + #[default] + FullAuto, + /// Automatically generate the operation id based on path and the provided string. + SemiAuto(Cow<'static, str>), + /// Use the provided operation id. + Manual(String) +} + pub(crate) struct OperationDescription { operation_id: Option, description: Option, @@ -112,10 +123,21 @@ impl OperationDescription { responses: HashMap>, path: &str ) -> Self { - let operation_id = E::operation_id().or_else(|| { - E::operation_verb() - .map(|verb| format!("{verb}_{}", path.replace("/", "_").trim_start_matches('_'))) - }); + let (mut operation_id, op_id_verb) = match E::operation_id() { + OperationId::FullAuto => (None, E::operation_verb().map(Cow::Borrowed)), + OperationId::SemiAuto(verb) => (None, Some(verb)), + OperationId::Manual(id) => (Some(id), None) + }; + if let Some(verb) = op_id_verb { + let op_path = path.replace('/', "_"); + let op_path = op_path.trim_start_matches('_'); + if verb.starts_with(op_path) || verb.ends_with(op_path) { + operation_id = Some(verb.into_owned()); + } else { + operation_id = Some(format!("{verb}_{op_path}")); + } + } + Self { operation_id, description: E::description(), diff --git a/tests/openapi_specification.json b/tests/openapi_specification.json index 7da3c0f243..705ee118e8 100644 --- a/tests/openapi_specification.json +++ b/tests/openapi_specification.json @@ -46,7 +46,7 @@ "paths": { "/coffee": { "get": { - "operationId": "read_all_coffee", + "operationId": "coffee_read_all", "responses": { "418": { "content": { @@ -64,6 +64,7 @@ }, "/custom": { "patch": { + "operationId": "custom_patch", "requestBody": { "content": { "application/json": { @@ -83,6 +84,7 @@ }, "/custom/read/{from}/with/{id}": { "get": { + "operationId": "custom_read_with", "parameters": [ { "in": "path",