Skip to content

Commit

Permalink
Remove redundant required argument
Browse files Browse the repository at this point in the history
* Remove redundant required argument from RequestBody and Parameter.

The required is defined via Rust's own Option<> type now thus eliminating need for required attribute
  • Loading branch information
juhaku committed Jan 30, 2022
1 parent c9ac0b2 commit d8ee7fe
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 92 deletions.
2 changes: 1 addition & 1 deletion tests/path_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ fn derive_path_with_defaults_success() {
],
params = [
("id" = u64, description = "Foo database id"),
("since" = String, query, description = "Datetime since foo is updated")
("since" = Option<String>, query, description = "Datetime since foo is updated")
]
)]
#[allow(unused)]
Expand Down
12 changes: 5 additions & 7 deletions tests/path_parameter_derive_test.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
#![cfg(not(feature = "actix_extras"))]
use std::println;

use serde_json::Value;
use utoipa::OpenApi;

mod common;
Expand All @@ -17,7 +15,7 @@ mod derive_params_all_options {
(status = 200, description = "success"),
],
params = [
("id" = i32, path, required, deprecated, description = "Search foos by ids"),
("id" = i32, path, deprecated, description = "Search foos by ids"),
]
)]
#[allow(unused)]
Expand Down Expand Up @@ -150,10 +148,10 @@ mod mod_derive_parameters_all_types {
],
params = [
("id" = i32, path, description = "Foo id"),
("since" = String, deprecated, required, query, description = "Datetime since"),
("numbers" = [u64], query, description = "Foo numbers list"),
("token" = String, header, deprecated, required, description = "Token of foo"),
("cookieval" = String, cookie, deprecated, required, description = "Foo cookie"),
("since" = String, deprecated, query, description = "Datetime since"),
("numbers" = Option<[u64]>, query, description = "Foo numbers list"),
("token" = String, header, deprecated, description = "Token of foo"),
("cookieval" = String, cookie, deprecated, description = "Foo cookie"),
]
)]
#[allow(unused)]
Expand Down
40 changes: 31 additions & 9 deletions tests/request_body_derive_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use utoipa::OpenApi;
mod common;

macro_rules! test_fn {
( module: $name:ident, body: $body:expr ) => {
( module: $name:ident, body: $($body:tt)* ) => {
#[allow(unused)]
mod $name {

Expand All @@ -13,7 +13,7 @@ macro_rules! test_fn {
#[utoipa::path(
post,
path = "/foo",
request_body = $body,
request_body = $($body)*,
responses = [
(status = 200, description = "success response")
]
Expand All @@ -39,7 +39,7 @@ fn derive_path_request_body_simple_success() {
assert_value! {doc=>
"paths./foo.post.requestBody.content.application/json.schema.$ref" = r###""#/components/schemas/Foo""###, "Request body content object type"
"paths./foo.post.requestBody.content.text/plain" = r###"null"###, "Request body content object type not text/plain"
"paths./foo.post.requestBody.required" = r###"null"###, "Request body required"
"paths./foo.post.requestBody.required" = r###"true"###, "Request body required"
"paths./foo.post.requestBody.description" = r###"null"###, "Request body description"
}
}
Expand All @@ -62,11 +62,33 @@ fn derive_path_request_body_simple_array_success() {
"paths./foo.post.requestBody.content.application/json.schema.items.$ref" = r###""#/components/schemas/Foo""###, "Request body content items object type"
"paths./foo.post.requestBody.content.application/json.schema.type" = r###""array""###, "Request body content items type"
"paths./foo.post.requestBody.content.text/plain" = r###"null"###, "Request body content object type not text/plain"
"paths./foo.post.requestBody.required" = r###"null"###, "Request body required"
"paths./foo.post.requestBody.required" = r###"true"###, "Request body required"
"paths./foo.post.requestBody.description" = r###"null"###, "Request body description"
}
}

test_fn! {
module: derive_request_body_option_array,
body: Option<[Foo]>
}

#[test]
fn derive_request_body_option_array_success() {
#[derive(OpenApi, Default)]
#[openapi(handlers = [derive_request_body_option_array::post_foo])]
struct ApiDoc;

let doc = serde_json::to_value(&ApiDoc::openapi()).unwrap();

assert_value! {doc=>
"paths./foo.post.requestBody.content.application/json.schema.$ref" = r###"null"###, "Request body content object type"
"paths./foo.post.requestBody.content.application/json.schema.items.$ref" = r###""#/components/schemas/Foo""###, "Request body content items object type"
"paths./foo.post.requestBody.content.application/json.schema.type" = r###""array""###, "Request body content items type"
"paths./foo.post.requestBody.content.text/plain" = r###"null"###, "Request body content object type not text/plain"
"paths./foo.post.requestBody.required" = r###"false"###, "Request body required"
"paths./foo.post.requestBody.description" = r###"null"###, "Request body description"
}
}
test_fn! {
module: derive_request_body_primitive_simple,
body: String
Expand All @@ -85,7 +107,7 @@ fn derive_request_body_primitive_simple_success() {
"paths./foo.post.requestBody.content.application/json.schema.items.$ref" = r###"null"###, "Request body content items object type"
"paths./foo.post.requestBody.content.application/json.schema.type" = r###"null"###, "Request body content items type"
"paths./foo.post.requestBody.content.text/plain.schema.type" = r###""string""###, "Request body content object type"
"paths./foo.post.requestBody.required" = r###"null"###, "Request body required"
"paths./foo.post.requestBody.required" = r###"true"###, "Request body required"
"paths./foo.post.requestBody.description" = r###"null"###, "Request body description"
}
}
Expand All @@ -108,14 +130,14 @@ fn derive_request_body_primitive_array_success() {
"paths./foo.post.requestBody.content.text/plain.schema.type" = r###""array""###, "Request body content object item type"
"paths./foo.post.requestBody.content.text/plain.schema.items.type" = r###""integer""###, "Request body content items object type"
"paths./foo.post.requestBody.content.text/plain.schema.items.format" = r###""int64""###, "Request body content items object format"
"paths./foo.post.requestBody.required" = r###"null"###, "Request body required"
"paths./foo.post.requestBody.required" = r###"true"###, "Request body required"
"paths./foo.post.requestBody.description" = r###"null"###, "Request body description"
}
}

test_fn! {
module: derive_request_body_complex,
body: (content = Foo, required, description = "Create new Foo", content_type = "text/xml")
body: (content = Foo, description = "Create new Foo", content_type = "text/xml")
}

#[test]
Expand All @@ -138,7 +160,7 @@ fn derive_request_body_complex_success() {

test_fn! {
module: derive_request_body_complex_required_explisit,
body: (content = Foo, required = false, description = "Create new Foo", content_type = "text/xml")
body: (content = Option<Foo>, description = "Create new Foo", content_type = "text/xml")
}

#[test]
Expand Down Expand Up @@ -177,7 +199,7 @@ fn derive_request_body_complex_primitive_array_success() {
"paths./foo.post.requestBody.content.text/plain.schema.type" = r###""array""###, "Request body content object item type"
"paths./foo.post.requestBody.content.text/plain.schema.items.type" = r###""integer""###, "Request body content items object type"
"paths./foo.post.requestBody.content.text/plain.schema.items.format" = r###""int32""###, "Request body content items object format"
"paths./foo.post.requestBody.required" = r###"null"###, "Request body required"
"paths./foo.post.requestBody.required" = r###"true"###, "Request body required"
"paths./foo.post.requestBody.description" = r###""Create new foo references""###, "Request body description"
}
}
65 changes: 43 additions & 22 deletions utoipa-gen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ use syn::{
bracketed,
parse::{Parse, ParseBuffer, ParseStream},
punctuated::Punctuated,
DeriveInput, Error, ItemFn,
token::Bracket,
DeriveInput, ItemFn, Token,
};

mod component;
Expand Down Expand Up @@ -190,50 +191,70 @@ impl ToTokens for Required {
}
}

/// Media type is wrapper around type and information is type an array
// #[derive(Default)]
/// Parses a type information in uotapi macro parameters.
///
/// Supports formats:
/// * `type` type is just a simple type identifier
/// * `[type]` type is an array of types
/// * `Option<type>` type is option of type
/// * `Option<[type]>` type is an option of array of types
#[cfg_attr(feature = "debug", derive(Debug))]
struct MediaType {
struct Type {
ty: Ident,
is_array: bool,
is_option: bool,
}

impl MediaType {
impl Type {
pub fn new(ident: Ident) -> Self {
Self {
ty: ident,
is_array: false,
is_option: false,
}
}
}

impl Parse for MediaType {
impl Parse for Type {
fn parse(input: ParseStream) -> syn::Result<Self> {
let mut is_array = false;
let mut is_option = false;

let parse_ident = |group: &ParseBuffer, error_msg: &str| {
if group.peek(syn::Ident) {
group.parse::<Ident>()
} else {
Err(Error::new(input.span(), error_msg))
}
let mut parse_array = |input: &ParseBuffer| {
is_array = true;
let group;
bracketed!(group in input);
group.parse::<Ident>()
};

let ty = if input.peek(syn::Ident) {
parse_ident(input, "unparseable MediaType, expected identifer")
} else {
is_array = true;
let mut ident: Ident = input.parse().unwrap();

let group;
bracketed!(group in input);
// is option of type or [type]
if (ident == "Option" && input.peek(Token![<]))
&& (input.peek2(syn::Ident) || input.peek2(Bracket))
{
is_option = true;

input.parse::<Token![<]>().unwrap();

parse_ident(
&group,
"unparseable MediaType, expected identifer within brackets",
)
if input.peek(syn::Ident) {
ident = input.parse::<Ident>().unwrap();
} else {
ident = parse_array(input)?;
}
input.parse::<Token![>]>()?;
}
Ok(ident)
} else {
parse_array(input)
}?;

Ok(MediaType { ty, is_array })
Ok(Type {
ty,
is_array,
is_option,
})
}
}

Expand Down
28 changes: 9 additions & 19 deletions utoipa-gen/src/path/parameter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use syn::{
Error, LitStr, Token,
};

use crate::{parse_utils, Deprecated, MediaType, Required};
use crate::{parse_utils, Deprecated, Required, Type};

use super::property::Property;

Expand All @@ -19,36 +19,32 @@ use super::property::Property;
///
/// Parse is executed for following formats:
///
/// * ("id" = String, path, required, deprecated, description = "Users database id"),
/// * ("id", path, required, deprecated, description = "Users database id"),
/// * ("id" = String, path, deprecated, description = "Users database id"),
/// * ("id", path, deprecated, description = "Users database id"),
///
/// The `= String` type statement is optional if automatic resolvation is supported.
#[derive(Default)]
#[cfg_attr(feature = "debug", derive(Debug))]
pub struct Parameter {
pub name: String,
parameter_in: ParameterIn,
required: bool,
deprecated: bool,
description: Option<String>,
parameter_type: Option<MediaType>,
parameter_type: Option<Type>,
}

impl Parameter {
pub fn new<S: AsRef<str>>(name: S, parameter_type: &Ident, parameter_in: ParameterIn) -> Self {
let required = parameter_in == ParameterIn::Path;

Self {
name: name.as_ref().to_string(),
parameter_type: Some(MediaType::new(parameter_type.clone())),
parameter_type: Some(Type::new(parameter_type.clone())),
parameter_in,
required,
..Default::default()
}
}

pub fn update_parameter_type(&mut self, ident: &Ident) {
self.parameter_type = Some(MediaType::new(ident.clone()));
self.parameter_type = Some(Type::new(ident.clone()));
}
}

Expand Down Expand Up @@ -85,11 +81,7 @@ impl Parse for Parameter {
match name {
"path" | "query" | "header" | "cookie" => {
parameter.parameter_in = name.parse::<ParameterIn>().unwrap_or_abort();
if parameter.parameter_in == ParameterIn::Path {
parameter.required = true; // all path parameters are required by default
}
}
"required" => parameter.required = parse_utils::parse_bool_or_true(input),
"deprecated" => parameter.deprecated = parse_utils::parse_bool_or_true(input),
"description" => {
parameter.description = parse_utils::parse_next(input, || {
Expand All @@ -105,7 +97,7 @@ impl Parse for Parameter {
return Err(Error::new(
ident.span(),
format!(
"unexpected identifier: {}, expected any of: path, query, header, cookie, required, deprecated, description",
"unexpected identifier: {}, expected any of: path, query, header, cookie, deprecated, description",
name
),
))
Expand All @@ -130,9 +122,6 @@ impl ToTokens for Parameter {
let parameter_in = &self.parameter_in;
tokens.extend(quote! { .with_in(#parameter_in) });

let required: Required = self.required.into();
tokens.extend(quote! { .with_required(#required) });

let deprecated: Deprecated = self.deprecated.into();
tokens.extend(quote! { .with_deprecated(#deprecated) });

Expand All @@ -142,8 +131,9 @@ impl ToTokens for Parameter {

if let Some(ref parameter_type) = self.parameter_type {
let property = Property::new(parameter_type.is_array, &parameter_type.ty);
let required: Required = (!parameter_type.is_option).into();

tokens.extend(quote! { .with_schema(#property) });
tokens.extend(quote! { .with_schema(#property).with_required(#required) });
}
}
}
Expand Down
Loading

0 comments on commit d8ee7fe

Please sign in to comment.