From d008ff464664a43206bc5775f19df243eb90a294 Mon Sep 17 00:00:00 2001 From: Juha Kukkonen Date: Thu, 6 Jul 2023 00:26:53 +0300 Subject: [PATCH] Add automatic query parameter recognition (#666) Add automatic query parameter recognition. And improve existing code related to path and query parameters recognition and elsewhere. --- utoipa-gen/Cargo.toml | 5 +- utoipa-gen/src/component/features.rs | 2 +- utoipa-gen/src/ext.rs | 31 ++++---- utoipa-gen/src/ext/actix.rs | 13 ++-- utoipa-gen/src/ext/axum.rs | 2 +- utoipa-gen/src/ext/rocket.rs | 13 ++-- utoipa-gen/src/lib.rs | 9 ++- utoipa-gen/src/path.rs | 104 ++++--------------------- utoipa-gen/src/path/parameter.rs | 93 +++++++++++++++------- utoipa-gen/tests/path_derive_actix.rs | 106 +++++++++++++++++++++++++- utoipa/Cargo.toml | 2 +- utoipa/src/openapi.rs | 6 +- 12 files changed, 224 insertions(+), 162 deletions(-) diff --git a/utoipa-gen/Cargo.toml b/utoipa-gen/Cargo.toml index 86bc9780..4948e48e 100644 --- a/utoipa-gen/Cargo.toml +++ b/utoipa-gen/Cargo.toml @@ -18,7 +18,6 @@ syn = { version = "2.0", features = ["full", "extra-traits"] } quote = "1.0" proc-macro-error = "1.0" regex = { version = "1.7", optional = true } -lazy_static = { version = "1.4", optional = true } uuid = { version = "1", optional = true } [dev-dependencies] @@ -39,11 +38,11 @@ serde_with = "3.0" [features] # See README.md for list and explanations of features debug = ["syn/extra-traits"] -actix_extras = ["regex", "lazy_static", "syn/extra-traits"] +actix_extras = ["regex", "syn/extra-traits"] chrono = [] yaml = [] decimal = [] -rocket_extras = ["regex", "lazy_static", "syn/extra-traits"] +rocket_extras = ["regex", "syn/extra-traits"] non_strict_integers = [] uuid = ["dep:uuid", "utoipa/uuid"] axum_extras = ["syn/extra-traits"] diff --git a/utoipa-gen/src/component/features.rs b/utoipa-gen/src/component/features.rs index ec7f1cbb..8b7eb77c 100644 --- a/utoipa-gen/src/component/features.rs +++ b/utoipa-gen/src/component/features.rs @@ -1444,7 +1444,7 @@ pub struct Required(pub bool); impl Required { pub fn is_true(&self) -> bool { - self.0 == true + self.0 } } diff --git a/utoipa-gen/src/ext.rs b/utoipa-gen/src/ext.rs index d3b36577..125a853c 100644 --- a/utoipa-gen/src/ext.rs +++ b/utoipa-gen/src/ext.rs @@ -61,6 +61,15 @@ pub struct IntoParamsType<'a> { pub type_path: Option>, } +impl<'i> From<(Option>, TokenStream)> for IntoParamsType<'i> { + fn from((type_path, parameter_in_provider): (Option>, TokenStream)) -> Self { + IntoParamsType { + parameter_in_provider, + type_path, + } + } +} + #[cfg(any( feature = "actix_extras", feature = "rocket_extras", @@ -276,9 +285,7 @@ impl PathOperationResolver for PathOperations {} ))] pub mod fn_arg { - use std::borrow::Cow; - - use proc_macro2::{Ident, TokenStream}; + use proc_macro2::Ident; use proc_macro_error::abort; #[cfg(any(feature = "actix_extras", feature = "axum_extras"))] use quote::quote; @@ -289,8 +296,6 @@ pub mod fn_arg { #[cfg(any(feature = "actix_extras", feature = "axum_extras"))] use crate::component::ValueType; - use super::IntoParamsType; - /// Http operation handler functions fn argument. #[cfg_attr(feature = "debug", derive(Debug))] pub struct FnArg<'a> { @@ -410,11 +415,14 @@ pub mod fn_arg { #[cfg(any(feature = "actix_extras", feature = "axum_extras"))] pub(super) fn with_parameter_in( arg: FnArg<'_>, - ) -> Option<(Option>, TokenStream)> { + ) -> Option<( + Option>, + proc_macro2::TokenStream, + )> { let parameter_in_provider = if arg.ty.is("Path") { quote! { || Some (utoipa::openapi::path::ParameterIn::Path) } } else if arg.ty.is("Query") { - quote! { || Some( utoipa::openapi::path::ParameterIn::Query) } + quote! { || Some(utoipa::openapi::path::ParameterIn::Query) } } else { quote! { || None } }; @@ -431,15 +439,6 @@ pub mod fn_arg { Some((type_path, parameter_in_provider)) } - pub(super) fn into_into_params_type( - (type_path, parameter_in_provider): (Option>, TokenStream), - ) -> IntoParamsType<'_> { - IntoParamsType { - parameter_in_provider, - type_path, - } - } - // if type is either Path or Query with direct children as Object types without generics #[cfg(any(feature = "actix_extras", feature = "axum_extras"))] pub(super) fn is_into_params(fn_arg: &FnArg) -> bool { diff --git a/utoipa-gen/src/ext/actix.rs b/utoipa-gen/src/ext/actix.rs index a7145c9e..881c06d5 100644 --- a/utoipa-gen/src/ext/actix.rs +++ b/utoipa-gen/src/ext/actix.rs @@ -1,6 +1,5 @@ use std::borrow::Cow; -use lazy_static::lazy_static; use proc_macro2::Ident; use proc_macro_error::abort; use regex::{Captures, Regex}; @@ -45,7 +44,7 @@ impl ArgumentResolver for PathOperations { into_params_args .into_iter() .flat_map(fn_arg::with_parameter_in) - .map(fn_arg::into_into_params_type) + .map(Into::into) .collect(), ), body.into_iter().next().map(Into::into), @@ -58,7 +57,7 @@ impl ArgumentResolver for PathOperations { into_params_args .into_iter() .flat_map(fn_arg::with_parameter_in) - .map(fn_arg::into_into_params_type) + .map(Into::into) .collect(), ), body.into_iter().next().map(Into::into), @@ -158,13 +157,11 @@ impl Parse for Path { impl PathResolver for PathOperations { fn resolve_path(path: &Option) -> Option { path.as_ref().map(|path| { - lazy_static! { - static ref RE: Regex = Regex::new(r"\{[a-zA-Z0-9_][^{}]*}").unwrap(); - } + let regex = Regex::new(r"\{[a-zA-Z0-9_][^{}]*}").unwrap(); - let mut args = Vec::::with_capacity(RE.find_iter(path).count()); + let mut args = Vec::::with_capacity(regex.find_iter(path).count()); MacroPath { - path: RE + path: regex .replace_all(path, |captures: &Captures| { let mut capture = &captures[0]; let original_name = String::from(capture); diff --git a/utoipa-gen/src/ext/axum.rs b/utoipa-gen/src/ext/axum.rs index aaf4c5ee..0b729baf 100644 --- a/utoipa-gen/src/ext/axum.rs +++ b/utoipa-gen/src/ext/axum.rs @@ -30,7 +30,7 @@ impl ArgumentResolver for PathOperations { into_params_args .into_iter() .flat_map(fn_arg::with_parameter_in) - .map(fn_arg::into_into_params_type) + .map(Into::into) .collect(), ), None, diff --git a/utoipa-gen/src/ext/rocket.rs b/utoipa-gen/src/ext/rocket.rs index 3220b5ff..65324b55 100644 --- a/utoipa-gen/src/ext/rocket.rs +++ b/utoipa-gen/src/ext/rocket.rs @@ -1,6 +1,5 @@ use std::{borrow::Cow, str::FromStr}; -use lazy_static::lazy_static; use proc_macro2::{Ident, TokenStream}; use proc_macro_error::abort; use quote::quote; @@ -54,7 +53,7 @@ impl ArgumentResolver for PathOperations { into_params_args .into_iter() .flat_map(with_parameter_in(&named_args)) - .map(fn_arg::into_into_params_type) + .map(Into::into) .collect(), ), None, @@ -220,16 +219,14 @@ fn is_valid_route_type(ident: Option<&Ident>) -> bool { impl PathResolver for PathOperations { fn resolve_path(path: &Option) -> Option { path.as_ref().map(|whole_path| { - lazy_static! { - static ref RE: Regex = Regex::new(r"<[a-zA-Z0-9_][^<>]*>").unwrap(); - } + let regex = Regex::new(r"<[a-zA-Z0-9_][^<>]*>").unwrap(); whole_path .split_once('?') .or(Some((whole_path, ""))) .map(|(path, query)| { let mut names = - Vec::::with_capacity(RE.find_iter(whole_path).count()); + Vec::::with_capacity(regex.find_iter(whole_path).count()); let mut underscore_count = 0; let mut format_arg = @@ -259,12 +256,12 @@ impl PathResolver for PathOperations { arg }; - let path = RE.replace_all(path, |captures: &Captures| { + let path = regex.replace_all(path, |captures: &Captures| { format_arg(captures, MacroArg::Path) }); if !query.is_empty() { - RE.replace_all(query, |captures: &Captures| { + regex.replace_all(query, |captures: &Captures| { format_arg(captures, MacroArg::Query) }); } diff --git a/utoipa-gen/src/lib.rs b/utoipa-gen/src/lib.rs index 79777b9b..711d974b 100644 --- a/utoipa-gen/src/lib.rs +++ b/utoipa-gen/src/lib.rs @@ -1333,12 +1333,17 @@ pub fn path(attr: TokenStream, item: TokenStream) -> TokenStream { ))] { use ext::ArgumentResolver; + use path::parameter::Parameter; let args = resolved_path.as_mut().map(|path| mem::take(&mut path.args)); let (arguments, into_params_types, body) = PathOperations::resolve_arguments(&ast_fn.sig.inputs, args); - path_attribute.update_parameters(arguments); - path_attribute.update_parameters_parameter_in(into_params_types); + let parameters = arguments + .into_iter() + .flatten() + .map(Parameter::from) + .chain(into_params_types.into_iter().flatten().map(Parameter::from)); + path_attribute.update_parameters_ext(parameters); path_attribute.update_request_body(body); } diff --git a/utoipa-gen/src/path.rs b/utoipa-gen/src/path.rs index 42b5f79e..16768337 100644 --- a/utoipa-gen/src/path.rs +++ b/utoipa-gen/src/path.rs @@ -19,20 +19,6 @@ use crate::{schema_type::SchemaType, security_requirement::SecurityRequirementAt use self::response::Response; use self::{parameter::Parameter, request_body::RequestBodyAttr, response::Responses}; -#[cfg(any( - feature = "actix_extras", - feature = "rocket_extras", - feature = "axum_extras" -))] -use self::parameter::ValueParameter; - -#[cfg(any( - feature = "actix_extras", - feature = "rocket_extras", - feature = "axum_extras" -))] -use crate::ext::{IntoParamsType, ValueArgument}; - pub mod example; pub mod parameter; mod request_body; @@ -56,55 +42,6 @@ pub struct PathAttr<'p> { } impl<'p> PathAttr<'p> { - #[cfg(any( - feature = "actix_extras", - feature = "rocket_extras", - feature = "axum_extras" - ))] - pub fn update_parameters<'a>(&mut self, arguments: Option>>) - where - 'a: 'p, - { - if let Some(arguments) = arguments { - if !self.params.is_empty() { - let mut value_parameters: Vec<&mut ValueParameter> = self - .params - .iter_mut() - .filter_map(|parameter| match parameter { - Parameter::Value(value) => Some(value), - Parameter::Struct(_) => None, - }) - .collect::>(); - let (existing_arguments, new_arguments): (Vec, Vec) = - arguments.into_iter().partition(|argument| { - value_parameters.iter().any(|parameter| { - Some(parameter.name.as_ref()) == argument.name.as_deref() - }) - }); - - for argument in existing_arguments { - if let Some(parameter) = value_parameters - .iter_mut() - .find(|parameter| Some(parameter.name.as_ref()) == argument.name.as_deref()) - { - parameter.update_parameter_type(argument.type_tree); - } - } - self.params - .extend(new_arguments.into_iter().map(Parameter::from)); - } else { - // no parameters at all, add arguments to the parameters - let mut parameters = Vec::with_capacity(arguments.len()); - - arguments - .into_iter() - .map(Parameter::from) - .for_each(|parameter| parameters.push(parameter)); - self.params = parameters; - } - } - } - #[cfg(feature = "auto_into_responses")] pub fn responses_from_into_responses(&mut self, ty: &'p syn::TypePath) { self.responses @@ -124,40 +61,31 @@ impl<'p> PathAttr<'p> { .or(mem::take(&mut self.request_body)); } + /// Update path with external parameters from extensions. #[cfg(any( feature = "actix_extras", feature = "rocket_extras", feature = "axum_extras" ))] - pub fn update_parameters_parameter_in( + pub fn update_parameters_ext>>( &mut self, - into_params_types: Option>, + ext_parameters: I, ) { - fn path_segments(path: &syn::Path) -> Vec<&'_ Ident> { - path.segments.iter().map(|segment| &segment.ident).collect() - } + let ext_params = ext_parameters.into_iter(); - if !self.params.is_empty() { - if let Some(mut into_params_types) = into_params_types { - self.params - .iter_mut() - .filter_map(|parameter| match parameter { - Parameter::Value(_) => None, - Parameter::Struct(parameter) => Some(parameter), - }) - .for_each(|parameter| { - if let Some(into_params_argument) = - into_params_types - .iter_mut() - .find(|argument| matches!(&argument.type_path, Some(path) if path_segments(path.as_ref()) == path_segments(¶meter.path.path)) -) - { - parameter.update_parameter_in( - &mut into_params_argument.parameter_in_provider, - ); - } - }) + if self.params.is_empty() { + self.params = ext_params.collect(); + } else { + let (existing_params, new_params): (Vec, Vec) = + ext_params.partition(|param| self.params.iter().any(|p| p == param)); + + for existing in existing_params { + if let Some(param) = self.params.iter_mut().find(|p| **p == existing) { + param.merge(existing); + } } + + self.params.extend(new_params.into_iter()); } } } diff --git a/utoipa-gen/src/path/parameter.rs b/utoipa-gen/src/path/parameter.rs index 1a01674a..680ef7b9 100644 --- a/utoipa-gen/src/path/parameter.rs +++ b/utoipa-gen/src/path/parameter.rs @@ -6,7 +6,7 @@ use quote::{quote, quote_spanned, ToTokens}; use syn::{ parenthesized, parse::{Parse, ParseBuffer, ParseStream}, - Error, ExprPath, LitStr, Token, + Error, LitStr, Token, TypePath, }; use crate::{ @@ -37,17 +37,37 @@ use super::InlineType; /// /// The `= String` type statement is optional if automatic resolution is supported. #[cfg_attr(feature = "debug", derive(Debug))] +#[derive(PartialEq, Eq)] pub enum Parameter<'a> { Value(ValueParameter<'a>), /// Identifier for a struct that implements `IntoParams` trait. - Struct(StructParameter), + IntoParamsIdent(IntoParamsIdentParameter<'a>), +} + +#[cfg(any( + feature = "actix_extras", + feature = "rocket_extras", + feature = "axum_extras" +))] +impl<'p> Parameter<'p> { + pub fn merge(&mut self, other: Parameter<'p>) { + match (self, other) { + (Self::Value(value), Parameter::Value(other)) => { + value.parameter_schema = other.parameter_schema; + } + (Self::IntoParamsIdent(into_params), Parameter::IntoParamsIdent(other)) => { + *into_params = other; + } + _ => (), + } + } } impl Parse for Parameter<'_> { fn parse(input: ParseStream) -> syn::Result { - if input.fork().parse::().is_ok() { - Ok(Self::Struct(StructParameter { - path: input.parse()?, + if input.fork().parse::().is_ok() { + Ok(Self::IntoParamsIdent(IntoParamsIdentParameter { + path: Cow::Owned(input.parse::()?.path), parameter_in_fn: None, })) } else { @@ -60,11 +80,11 @@ impl ToTokens for Parameter<'_> { fn to_tokens(&self, tokens: &mut TokenStream) { match self { Parameter::Value(parameter) => tokens.extend(quote! { .parameter(#parameter) }), - Parameter::Struct(StructParameter { + Parameter::IntoParamsIdent(IntoParamsIdentParameter { path, parameter_in_fn, }) => { - let last_ident = &path.path.segments.last().unwrap().ident; + let last_ident = &path.segments.last().unwrap().ident; let default_parameter_in_provider = "e! { || None }; let parameter_in_provider = parameter_in_fn @@ -103,6 +123,20 @@ impl<'a> From> for Parameter<'a> { } } +#[cfg(any( + feature = "actix_extras", + feature = "rocket_extras", + feature = "axum_extras" +))] +impl<'a> From> for Parameter<'a> { + fn from(value: crate::ext::IntoParamsType<'a>) -> Self { + Self::IntoParamsIdent(IntoParamsIdentParameter { + path: value.type_path.expect("IntoParams type must have a path"), + parameter_in_fn: Some(value.parameter_in_provider), + }) + } +} + #[cfg_attr(feature = "debug", derive(Debug))] struct ParameterSchema<'p> { parameter_type: ParameterType<'p>, @@ -177,20 +211,14 @@ pub struct ValueParameter<'a> { features: (Vec, Vec), } -impl<'p> ValueParameter<'p> { - #[cfg(any( - feature = "actix_extras", - feature = "rocket_extras", - feature = "axum_extras" - ))] - pub fn update_parameter_type(&mut self, type_path: Option>) { - self.parameter_schema = type_path.map(|type_tree| ParameterSchema { - parameter_type: ParameterType::External(type_tree), - features: self.features.0.clone(), // clone possible features for the parameter schema - }) +impl PartialEq for ValueParameter<'_> { + fn eq(&self, other: &Self) -> bool { + self.name == other.name && self.parameter_in == other.parameter_in } } +impl Eq for ValueParameter<'_> {} + impl Parse for ValueParameter<'_> { fn parse(input_with_parens: ParseStream) -> syn::Result { let input: ParseBuffer; @@ -343,24 +371,31 @@ impl ToTokens for ValueParameter<'_> { } #[cfg_attr(feature = "debug", derive(Debug))] -pub struct StructParameter { - pub path: ExprPath, +pub struct IntoParamsIdentParameter<'i> { + pub path: Cow<'i, syn::Path>, /// quote!{ ... } of function which should implement `parameter_in_provider` for [`utoipa::IntoParams::into_param`] parameter_in_fn: Option, } -impl StructParameter { - #[cfg(any( - feature = "actix_extras", - feature = "rocket_extras", - feature = "axum_extras" - ))] - pub fn update_parameter_in(&mut self, parameter_in_provider: &mut TokenStream) { - use std::mem; - self.parameter_in_fn = Some(mem::take(parameter_in_provider)); +// Compare paths loosly only by segment idents ignoring possible generics +impl PartialEq for IntoParamsIdentParameter<'_> { + fn eq(&self, other: &Self) -> bool { + self.path + .segments + .iter() + .map(|segment| &segment.ident) + .collect::>() + == other + .path + .segments + .iter() + .map(|segment| &segment.ident) + .collect::>() } } +impl Eq for IntoParamsIdentParameter<'_> {} + #[cfg_attr(feature = "debug", derive(Debug))] #[derive(PartialEq, Eq, Clone, Copy)] pub enum ParameterIn { diff --git a/utoipa-gen/tests/path_derive_actix.rs b/utoipa-gen/tests/path_derive_actix.rs index e54db19d..c7fc3445 100644 --- a/utoipa-gen/tests/path_derive_actix.rs +++ b/utoipa-gen/tests/path_derive_actix.rs @@ -1,8 +1,15 @@ #![cfg(feature = "actix_extras")] -use actix_web::web::{Path, Query}; +use std::fmt::Display; + +use actix_web::{ + get, post, + web::{Json, Path, Query}, + ResponseError, +}; +use assert_json_diff::assert_json_eq; use serde::{Deserialize, Serialize}; -use serde_json::Value; +use serde_json::{json, Value}; use utoipa::{ openapi::{ path::{Parameter, ParameterBuilder, ParameterIn}, @@ -761,6 +768,101 @@ fn derive_into_params_in_another_module() { } } +#[test] +fn path_with_all_args() { + #[derive(utoipa::ToSchema, serde::Serialize, serde::Deserialize)] + struct Item(String); + + /// Error + #[derive(Debug)] + struct Error; + + impl Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Error") + } + } + + impl ResponseError for Error {} + + #[derive(serde::Serialize, serde::Deserialize, IntoParams)] + struct Filter { + age: i32, + status: String, + } + + #[utoipa::path] + #[post("/item/{id}/{name}")] + async fn post_item( + _path: Path<(i32, String)>, + _query: Query, + _body: Json, + ) -> Result, Error> { + Ok(Json(Item(String::new()))) + } + + #[derive(utoipa::OpenApi)] + #[openapi(paths(post_item))] + struct Doc; + + let doc = serde_json::to_value(Doc::openapi()).unwrap(); + let operation = doc.pointer("/paths/~1item~1{id}~1{name}/post").unwrap(); + + assert_json_eq!( + &operation.pointer("/parameters").unwrap(), + json!([ + { + "in": "path", + "name": "id", + "required": true, + "schema": { + "format": "int32", + "type": "integer" + } + }, + { + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "in": "query", + "name": "age", + "required": true, + "schema": { + "format": "int32", + "type": "integer" + } + }, + { + "in": "query", + "name": "status", + "required": true, + "schema": { + "type": "string" + } + } + ]) + ); + assert_json_eq!( + &operation.pointer("/requestBody"), + json!({ + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Item" + } + } + }, + "required": true, + }) + ) +} + macro_rules! test_derive_path_operations { ( $( $name:ident, $mod:ident: $operation:ident)* ) => { $( diff --git a/utoipa/Cargo.toml b/utoipa/Cargo.toml index 40b2c663..acc940ef 100644 --- a/utoipa/Cargo.toml +++ b/utoipa/Cargo.toml @@ -46,7 +46,7 @@ serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0" } serde_yaml = { version = "0.9", optional = true } utoipa-gen = { version = "3.3.0", path = "../utoipa-gen" } -indexmap = { version = "1", features = ["serde"] } +indexmap = { version = "2", features = ["serde"] } [dev-dependencies] assert-json-diff = "2" diff --git a/utoipa/src/openapi.rs b/utoipa/src/openapi.rs index 40ce9963..97b82230 100644 --- a/utoipa/src/openapi.rs +++ b/utoipa/src/openapi.rs @@ -301,15 +301,15 @@ impl<'de> Deserialize<'de> for OpenApiVersion { E: Error, { let parts = v.split('.').collect::>(); - if parts.len() > 3 || parts.len() < 1 { + if parts.len() > 3 || parts.is_empty() { return Err(E::custom(format!( "Invalid format of OpenAPI version: {}", v, ))); } - Ok(match (parts[0], parts.get(1).map(|p| *p).unwrap_or("0")) { - ("3", "0") => Self::Value::Version3, + Ok(match (parts[0], parts.get(1).copied().unwrap_or("0")) { + ("3", "0") => OpenApiVersion::Version3, _ => return Err(E::custom(format!("Unsupported version: {}", &v))), }) }