From f2a7143fd92767046abc9433daedc15938afe1a2 Mon Sep 17 00:00:00 2001 From: Juha Kukkonen Date: Sat, 24 Aug 2024 22:22:07 +0300 Subject: [PATCH] Fix default tag logic for paths (#1002) The default tag for paths is resolved from the module path of the handler or `OpenApi` if the type in question is a nested `OpenApi` document. This was supposed to be only the case when no additional tags are provided for the paths. However it was resolved in all cases and there was no way to "not to use" module path as a tag. This commit fixes this and the module path will be only used if there are no other tags defined. This commit also changes the old behavior where _`crate`_ was used as a tag in case there was no module path available. From now on if there is no module path there will be no default tag added to any of the paths. This will result the paths to be rendered under _`default`_ tag in a Swagger UI. Fixes #978 --- utoipa-gen/src/lib.rs | 31 +++-- utoipa-gen/src/openapi.rs | 11 +- utoipa-gen/src/path.rs | 8 +- utoipa-gen/tests/openapi_derive.rs | 28 ++--- utoipa-gen/tests/path_derive.rs | 184 ++++++++++++++++++++++++++++- utoipa/src/lib.rs | 24 +--- 6 files changed, 220 insertions(+), 66 deletions(-) diff --git a/utoipa-gen/src/lib.rs b/utoipa-gen/src/lib.rs index 7284c6ca..6a12dcca 100644 --- a/utoipa-gen/src/lib.rs +++ b/utoipa-gen/src/lib.rs @@ -274,7 +274,7 @@ use self::{ /// This attribute requires that a `tag` is present, otherwise serde will trigger a compile-time /// failure. /// * `untagged` Supported at the container level. Allows [untagged -/// enum representation](https://serde.rs/enum-representations.html#untagged). +/// enum representation](https://serde.rs/enum-representations.html#untagged). /// * `default` Supported at the container level and field level according to [serde attributes]. /// * `deny_unknown_fields` Supported at the container level. /// * `flatten` Supported at the field level. @@ -725,11 +725,11 @@ pub fn derive_to_schema(input: TokenStream) -> TokenStream { /// **context_path** can become handy to alter the path. /// /// * `tag = "..."` Can be used to group operations. Operations with same tag are grouped together. By default -/// this is derived from the handler that is given to [`OpenApi`][openapi]. If derive results empty str -/// then default value _`crate`_ is used instead. +/// this is derived from the module path of the handler that is given to [`OpenApi`][openapi]. /// /// * `tags = ["tag1", ...]` Can be used to group operations. Operations with same tag are grouped -/// toghether. Tags attribute can be used to add addtional _tags_ for the operation. +/// toghether. Tags attribute can be used to add addtional _tags_ for the operation. If both +/// _`tag`_ and _`tags`_ are provided then they will be combined to a single _`tags`_ array. /// /// * `request_body = ... | request_body(...)` Defining request body indicates that the request is expecting request body within /// the performed request. @@ -806,9 +806,9 @@ pub fn derive_to_schema(input: TokenStream) -> TokenStream { /// [primitive Rust types][primitive], `application/octet-stream` for _`[u8]`_ and /// _`application/json`_ for struct and complex enum types. /// Content type can also be slice of **content_type** values if the endpoint support returning multiple -/// response content types. E.g _`["application/json", "text/xml"]`_ would indicate that endpoint can return both -/// _`json`_ and _`xml`_ formats. **The order** of the content types define the default example show first in -/// the Swagger UI. Swagger UI will use the first _`content_type`_ value as a default example. +/// response content types. E.g _`["application/json", "text/xml"]`_ would indicate that endpoint can return both +/// _`json`_ and _`xml`_ formats. **The order** of the content types define the default example show first in +/// the Swagger UI. Swagger UI will use the first _`content_type`_ value as a default example. /// /// * `headers(...)` Slice of response headers that are returned back to a caller. /// @@ -1141,7 +1141,7 @@ pub fn derive_to_schema(input: TokenStream) -> TokenStream { /// 1. It allows users to use tuple style path parameters e.g. _`Path((id, name)): Path<(i32, String)>`_ and resolves /// parameter names and types from it. /// 2. It enhances [`IntoParams` derive][into_params_derive] functionality by automatically resolving _`parameter_in`_ from -/// _`Path<...>`_ or _`Query<...>`_ handler function arguments. +/// _`Path<...>`_ or _`Query<...>`_ handler function arguments. /// /// _**Resole path argument types from tuple style handler arguments.**_ /// ```rust @@ -1470,8 +1470,7 @@ pub fn path(attr: TokenStream, item: TokenStream) -> TokenStream { /// * `components(schemas(...), responses(...))` Takes available _`component`_ configurations. Currently only /// _`schema`_ and _`response`_ components are supported. /// * `schemas(...)` List of [`ToSchema`][to_schema]s in OpenAPI schema. -/// * `responses(...)` List of types that implement -/// [`ToResponse`][to_response_trait]. +/// * `responses(...)` List of types that implement [`ToResponse`][to_response_trait]. /// * `modifiers(...)` List of items implementing [`Modify`][modify] trait for runtime OpenApi modification. /// See the [trait documentation][modify] for more details. /// * `security(...)` List of [`SecurityRequirement`][security]s global to all operations. @@ -2182,9 +2181,9 @@ pub fn into_params(input: TokenStream) -> TokenStream { /// [primitive Rust types][primitive], `application/octet-stream` for _`[u8]`_ and /// _`application/json`_ for struct and complex enum types. /// Content type can also be slice of **content_type** values if the endpoint support returning multiple -/// response content types. E.g _`["application/json", "text/xml"]`_ would indicate that endpoint can return both -/// _`json`_ and _`xml`_ formats. **The order** of the content types define the default example show first in -/// the Swagger UI. Swagger UI will use the first _`content_type`_ value as a default example. +/// response content types. E.g _`["application/json", "text/xml"]`_ would indicate that endpoint can return both +/// _`json`_ and _`xml`_ formats. **The order** of the content types define the default example show first in +/// the Swagger UI. Swagger UI will use the first _`content_type`_ value as a default example. /// /// * `headers(...)` Slice of response headers that are returned back to a caller. /// @@ -2349,9 +2348,9 @@ pub fn to_response(input: TokenStream) -> TokenStream { /// [primitive Rust types][primitive], `application/octet-stream` for _`[u8]`_ and /// _`application/json`_ for struct and complex enum types. /// Content type can also be slice of **content_type** values if the endpoint support returning multiple -/// response content types. E.g _`["application/json", "text/xml"]`_ would indicate that endpoint can return both -/// _`json`_ and _`xml`_ formats. **The order** of the content types define the default example show first in -/// the Swagger UI. Swagger UI will use the first _`content_type`_ value as a default example. +/// response content types. E.g _`["application/json", "text/xml"]`_ would indicate that endpoint can return both +/// _`json`_ and _`xml`_ formats. **The order** of the content types define the default example show first in +/// the Swagger UI. Swagger UI will use the first _`content_type`_ value as a default example. /// /// * `headers(...)` Slice of response headers that are returned back to a caller. /// diff --git a/utoipa-gen/src/openapi.rs b/utoipa-gen/src/openapi.rs index 61aa54d6..7d114119 100644 --- a/utoipa-gen/src/openapi.rs +++ b/utoipa-gen/src/openapi.rs @@ -429,16 +429,13 @@ impl OpenApi<'_> { let span = nest_api.span(); quote_spanned! {span=> .nest(#path, { + #[allow(non_camel_case_types)] struct #nest_api_config; impl utoipa::__dev::NestedApiConfig for #nest_api_config { - fn config() -> (utoipa::openapi::OpenApi, Vec<&'static str>) { + fn config() -> (utoipa::openapi::OpenApi, Vec<&'static str>, &'static str) { let api = <#nest_api as utoipa::OpenApi>::openapi(); - let mut tags: Vec<_> = #tags.into(); - if !#module_path.is_empty() { - tags.push(#module_path); - } - (api, tags) + (api, #tags.into(), #module_path) } } <#nest_api_config as utoipa::OpenApi>::openapi() @@ -641,7 +638,7 @@ fn impl_paths(handler_paths: &Punctuated) -> TokenStream { let item = #usage::path_item(); let path = #usage::path(); let mut tags = <#usage as utoipa::__dev::Tags>::tags(); - if !#tag.is_empty() { + if !#tag.is_empty() && tags.is_empty() { tags.push(#tag); } diff --git a/utoipa-gen/src/path.rs b/utoipa-gen/src/path.rs index 9802d203..0e5e10c6 100644 --- a/utoipa-gen/src/path.rs +++ b/utoipa-gen/src/path.rs @@ -445,11 +445,9 @@ impl<'p> ToTokensDiagnostics for Path<'p> { let operation = as_tokens_or_diagnostics!(&operation); let mut tags = self.path_attr.tags.clone(); - match self.path_attr.tag.as_ref() { - Some(tag) if tags.is_empty() => { - tags.push(tag.clone()); - } - _ => (), + if let Some(tag) = self.path_attr.tag.as_ref() { + // if defined tag is the first before the additional tags + tags.insert(0, tag.clone()); } let tags_list = tags.into_iter().collect::>(); diff --git a/utoipa-gen/tests/openapi_derive.rs b/utoipa-gen/tests/openapi_derive.rs index a4e1d4aa..e8177dab 100644 --- a/utoipa-gen/tests/openapi_derive.rs +++ b/utoipa-gen/tests/openapi_derive.rs @@ -22,7 +22,7 @@ fn derive_openapi_with_security_requirement() { ))] struct ApiDoc; - let doc_value = serde_json::to_value(&ApiDoc::openapi()).unwrap(); + let doc_value = serde_json::to_value(ApiDoc::openapi()).unwrap(); assert_value! {doc_value=> "security.[0]" = "{}", "Optional security requirement" @@ -45,7 +45,7 @@ fn derive_openapi_tags() { ))] struct ApiDoc; - let doc = serde_json::to_value(&ApiDoc::openapi()).unwrap(); + let doc = serde_json::to_value(ApiDoc::openapi()).unwrap(); assert_value! {doc=> "tags.[0].name" = r###""random::api""###, "Tags random_api name" @@ -66,7 +66,7 @@ fn derive_openapi_tags_include_str() { ))] struct ApiDoc; - let doc = serde_json::to_value(&ApiDoc::openapi()).unwrap(); + let doc = serde_json::to_value(ApiDoc::openapi()).unwrap(); assert_value! {doc=> "tags.[0].name" = r###""random::api""###, "Tags random_api name" @@ -83,7 +83,7 @@ fn derive_openapi_with_external_docs() { ))] struct ApiDoc; - let doc = serde_json::to_value(&ApiDoc::openapi()).unwrap(); + let doc = serde_json::to_value(ApiDoc::openapi()).unwrap(); assert_value! {doc=> "externalDocs.url" = r###""http://localhost.more.about.api""###, "External docs url" @@ -97,7 +97,7 @@ fn derive_openapi_with_external_docs_only_url() { #[openapi(external_docs(url = "http://localhost.more.about.api"))] struct ApiDoc; - let doc = serde_json::to_value(&ApiDoc::openapi()).unwrap(); + let doc = serde_json::to_value(ApiDoc::openapi()).unwrap(); assert_value! {doc=> "externalDocs.url" = r###""http://localhost.more.about.api""###, "External docs url" @@ -121,7 +121,7 @@ fn derive_openapi_with_components_in_different_module() { #[openapi(components(schemas(custom::Todo)))] struct ApiDoc; - let doc = serde_json::to_value(&ApiDoc::openapi()).unwrap(); + let doc = serde_json::to_value(ApiDoc::openapi()).unwrap(); let todo = doc.pointer("/components/schemas/Todo").unwrap(); assert_ne!( @@ -149,7 +149,7 @@ fn derive_openapi_with_responses() { #[openapi(components(responses(MyResponse)))] struct ApiDoc; - let doc = serde_json::to_value(&ApiDoc::openapi()).unwrap(); + let doc = serde_json::to_value(ApiDoc::openapi()).unwrap(); let responses = doc.pointer("/components/responses").unwrap(); assert_json_eq!( @@ -178,7 +178,7 @@ fn derive_openapi_with_servers() { )] struct ApiDoc; - let value = serde_json::to_value(&ApiDoc::openapi()).unwrap(); + let value = serde_json::to_value(ApiDoc::openapi()).unwrap(); let servers = value.pointer("/servers"); assert_json_eq!( @@ -222,7 +222,7 @@ fn derive_openapi_with_custom_info() { ))] struct ApiDoc; - let value = serde_json::to_value(&ApiDoc::openapi()).unwrap(); + let value = serde_json::to_value(ApiDoc::openapi()).unwrap(); let info = value.pointer("/info"); assert_json_include!( @@ -254,7 +254,7 @@ fn derive_openapi_with_include_str_description() { ))] struct ApiDoc; - let value = serde_json::to_value(&ApiDoc::openapi()).unwrap(); + let value = serde_json::to_value(ApiDoc::openapi()).unwrap(); let info = value.pointer("/info"); assert_json_include!( @@ -438,7 +438,7 @@ fn derive_nest_openapi_with_tags() { )] struct ApiDoc; - let api = serde_json::to_value(&ApiDoc::openapi()).expect("should serialize to value"); + let api = serde_json::to_value(ApiDoc::openapi()).expect("should serialize to value"); let paths = api.pointer("/paths"); assert_json_eq!( @@ -448,7 +448,7 @@ fn derive_nest_openapi_with_tags() { "get": { "operationId": "foobar", "responses": {}, - "tags": [ "yeah", "wowow", "foobarapi" ] + "tags": [ "mytag", "yeah", "wowow", "foobarapi" ] } }, "/api/v1/foobar/another": { @@ -469,14 +469,14 @@ fn derive_nest_openapi_with_tags() { "get": { "operationId": "test_path_status", "responses": {}, - "tags": [ "crate" ] + "tags": [] } }, "/api/v1/user/test": { "get": { "operationId": "user_test_path", "responses": {}, - "tags": [ "user", TAG, "user_api" ] + "tags": [ "user", TAG ] } }, "/random": { diff --git a/utoipa-gen/tests/path_derive.rs b/utoipa-gen/tests/path_derive.rs index ff305ca1..6efd9b20 100644 --- a/utoipa-gen/tests/path_derive.rs +++ b/utoipa-gen/tests/path_derive.rs @@ -233,7 +233,7 @@ fn derive_path_with_extra_attributes_without_nested_module() { "description" = r#""This is long description for test operation""#, "Api operation description" "operationId" = r#""get_foos_by_id_since""#, "Api operation operation_id" "summary" = r#""This is test operation""#, "Api operation summary" - "tags.[0]" = r#""crate""#, "Api operation tag" + "tags.[0]" = r#"null"#, "Api operation tag" "parameters.[0].deprecated" = r#"false"#, "Parameter 0 deprecated" "parameters.[0].description" = r#""Foo database id""#, "Parameter 0 description" @@ -2072,7 +2072,7 @@ fn derive_path_with_multiple_tags() { "description": "success response", }, }, - "tags": ["one", "two","another"] + "tags": ["mytag", "one", "two","another"] }) ); } @@ -2118,7 +2118,7 @@ split to multiple lines"; }, }, "summary": "This is summary override that is\nsplit to multiple lines", - "tags": ["crate"] + "tags": [] }) ); } @@ -2155,7 +2155,183 @@ fn derive_path_include_str_description() { "description": "success response", }, }, - "tags": ["crate"] + "tags": [] + }) + ); +} + +#[test] +fn path_and_nest_with_default_tags_from_path() { + mod test_path { + #[allow(dead_code)] + #[utoipa::path(get, path = "/test")] + #[allow(unused)] + fn test_path() -> &'static str { + "" + } + } + + mod test_nest { + #[derive(utoipa::OpenApi)] + #[openapi(paths(test_path_nested))] + pub struct NestApi; + + #[allow(dead_code)] + #[utoipa::path(get, path = "/test")] + #[allow(unused)] + fn test_path_nested() -> &'static str { + "" + } + } + + #[derive(utoipa::OpenApi)] + #[openapi( + paths(test_path::test_path), + nest( + (path = "/api/nest", api = test_nest::NestApi) + ) + )] + struct ApiDoc; + let value = serde_json::to_value(ApiDoc::openapi()).expect("should be able to serialize json"); + let paths = value + .pointer("/paths") + .expect("should find /paths from the OpenAPI spec"); + + assert_json_eq!( + &paths, + json!({ + "/api/nest/test": { + "get": { + "operationId": "test_path_nested", + "responses": {}, + "tags": ["test_nest"] + } + }, + "/test": { + "get": { + "operationId": "test_path", + "responses": {}, + "tags": ["test_path"] + } + } + }) + ); +} + +#[test] +fn path_and_nest_with_addtional_tags() { + mod test_path { + #[allow(dead_code)] + #[utoipa::path(get, path = "/test", tag = "this_is_tag", tags = ["additional"])] + #[allow(unused)] + fn test_path() -> &'static str { + "" + } + } + + mod test_nest { + #[derive(utoipa::OpenApi)] + #[openapi(paths(test_path_nested))] + pub struct NestApi; + + #[allow(dead_code)] + #[utoipa::path(get, path = "/test", tag = "this_is_tag:nest", tags = ["additional:nest"])] + #[allow(unused)] + fn test_path_nested() -> &'static str { + "" + } + } + + #[derive(utoipa::OpenApi)] + #[openapi( + paths(test_path::test_path), + nest( + (path = "/api/nest", api = test_nest::NestApi) + ) + )] + struct ApiDoc; + let value = serde_json::to_value(ApiDoc::openapi()).expect("should be able to serialize json"); + let paths = value + .pointer("/paths") + .expect("should find /paths from the OpenAPI spec"); + + assert_json_eq!( + &paths, + json!({ + "/api/nest/test": { + "get": { + "operationId": "test_path_nested", + "responses": {}, + "tags": ["this_is_tag:nest", "additional:nest"] + }, + }, + "/test": { + "get": { + "operationId": "test_path", + "responses": {}, + "tags": ["this_is_tag", "additional"] + }, + } + }) + ); +} + +#[test] +fn path_nest_without_any_tags() { + mod test_path { + #[allow(dead_code)] + #[utoipa::path(get, path = "/test")] + #[allow(unused)] + pub fn test_path() -> &'static str { + "" + } + } + + mod test_nest { + #[derive(utoipa::OpenApi)] + #[openapi(paths(test_path_nested))] + pub struct NestApi; + + #[allow(dead_code)] + #[utoipa::path(get, path = "/test")] + #[allow(unused)] + fn test_path_nested() -> &'static str { + "" + } + } + + use test_nest::NestApi; + use test_path::__path_test_path; + #[derive(utoipa::OpenApi)] + #[openapi( + paths(test_path), + nest( + (path = "/api/nest", api = NestApi) + ) + )] + struct ApiDoc; + let value = serde_json::to_value(ApiDoc::openapi()).expect("should be able to serialize json"); + let paths = value + .pointer("/paths") + .expect("should find /paths from the OpenAPI spec"); + + assert_json_eq!( + &paths, + json!({ + "/api/nest/test": { + "get": { + "operationId": "test_path_nested", + "responses": {}, + "tags": [] + }, + }, + "/test": { + "get": { + "operationId": "test_path", + "responses": {}, + "tags": [] + }, + } }) ); } diff --git a/utoipa/src/lib.rs b/utoipa/src/lib.rs index c5749da5..3d530660 100644 --- a/utoipa/src/lib.rs +++ b/utoipa/src/lib.rs @@ -950,8 +950,6 @@ pub mod __dev { fn tags() -> Vec<&'t str>; } - const DEFAULT_TAG: &str = "crate"; - impl utoipa::Path for T { fn path() -> String { ::config().0.to_string() @@ -963,10 +961,6 @@ pub mod __dev { for (_, operation) in item.operations.iter_mut() { let operation_tags = operation.tags.get_or_insert(Vec::new()); operation_tags.extend(tags.iter().map(ToString::to_string)); - // add deault tag if no tags is defined - if operation_tags.is_empty() { - operation_tags.push(DEFAULT_TAG.to_string()); - } } item @@ -974,12 +968,12 @@ pub mod __dev { } pub trait NestedApiConfig { - fn config() -> (utoipa::openapi::OpenApi, Vec<&'static str>); + fn config() -> (utoipa::openapi::OpenApi, Vec<&'static str>, &'static str); } impl OpenApi for T { fn openapi() -> crate::openapi::OpenApi { - let (mut api, tags) = T::config(); + let (mut api, tags, module_path) = T::config(); api.paths .paths @@ -988,18 +982,8 @@ pub mod __dev { .for_each(|(_, operation)| { let operation_tags = operation.tags.get_or_insert(Vec::new()); operation_tags.extend(tags.iter().map(ToString::to_string)); - // remove defualt tag if found and there are more than 1 tag - if operation_tags.len() > 1 { - let index = operation_tags.iter().enumerate().find_map(|(index, tag)| { - if tag == DEFAULT_TAG { - Some(index) - } else { - None - } - }); - if let Some(index) = index { - operation_tags.remove(index); - }; + if operation_tags.is_empty() && !module_path.is_empty() { + operation_tags.push(module_path.to_string()); } });