Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

utopia-gen: Adjust params code to not set nullable on Option for Query params #1248

Merged
merged 3 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions utoipa-gen/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

### Changed

* Adjust params code to not set `nullable` on `Option` for `Query` params (https://github.com/juhaku/utoipa/pull/1248)
* Use `insta` for snapshot testing (https://github.com/juhaku/utoipa/pull/1247)
* Make `parse_named_attributes` a method of `MediaTypeAttr` (https://github.com/juhaku/utoipa/pull/1236)
* Use a re-exported `serde_json` dependency in macros instead of implicitly requiring it as dependency in end projects (https://github.com/juhaku/utoipa/pull/1243)
Expand Down
45 changes: 36 additions & 9 deletions utoipa-gen/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,18 @@ pub struct ComponentSchemaProps<'c> {
pub description: Option<&'c ComponentDescription<'c>>,
}

impl ComponentSchemaProps<'_> {
fn set_nullable(&mut self) {
if !self
.features
.iter()
.any(|feature| matches!(feature, Feature::Nullable(_)))
{
self.features.push(Nullable::new().into());
}
}
}

#[cfg_attr(feature = "debug", derive(Debug))]
pub enum ComponentDescription<'c> {
CommentAttributes(&'c CommentAttributes),
Expand Down Expand Up @@ -750,11 +762,33 @@ pub struct ComponentSchema {
}

impl ComponentSchema {
pub fn new(
pub fn for_params(
mut schema_props: ComponentSchemaProps,
option_is_nullable: bool,
) -> Result<Self, Diagnostics> {
// Add nullable feature if not already exists.
// Option is always nullable, except when used in query parameters.
if schema_props.type_tree.is_option() && option_is_nullable {
schema_props.set_nullable()
}

Self::new_inner(schema_props)
}

pub fn new(mut schema_props: ComponentSchemaProps) -> Result<Self, Diagnostics> {
// Add nullable feature if not already exists. Option is always nullable
if schema_props.type_tree.is_option() {
schema_props.set_nullable();
}

Self::new_inner(schema_props)
}

fn new_inner(
ComponentSchemaProps {
container,
type_tree,
mut features,
features,
description,
}: ComponentSchemaProps,
) -> Result<Self, Diagnostics> {
Expand Down Expand Up @@ -791,13 +825,6 @@ impl ComponentSchema {
description,
)?,
Some(GenericType::Option) => {
// Add nullable feature if not already exists. Option is always nullable
if !features
.iter()
.any(|feature| matches!(feature, Feature::Nullable(_)))
{
features.push(Nullable::new().into());
}
let child = type_tree
.children
.as_ref()
Expand Down
6 changes: 6 additions & 0 deletions utoipa-gen/src/component/features/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,12 @@ impl_feature! {
pub struct ParameterIn(parameter::ParameterIn);
}

impl ParameterIn {
pub fn is_query(&self) -> bool {
matches!(self.0, parameter::ParameterIn::Query)
}
}

impl Parse for ParameterIn {
fn parse(input: syn::parse::ParseStream, _: Ident) -> syn::Result<Self> {
parse_utils::parse_next(input, || input.parse::<parameter::ParameterIn>().map(Self))
Expand Down
18 changes: 12 additions & 6 deletions utoipa-gen/src/component/into_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,12 +407,18 @@ impl Param {
});
tokens.extend(param_features.to_token_stream()?);

let schema = ComponentSchema::new(component::ComponentSchemaProps {
type_tree: component,
features: schema_features,
description: None,
container: &Container { generics },
})?;
let is_query = matches!(container_attributes.parameter_in, Some(Feature::ParameterIn(p)) if p.is_query());
let option_is_nullable = !is_query;

let schema = ComponentSchema::for_params(
component::ComponentSchemaProps {
type_tree: component,
features: schema_features,
description: None,
container: &Container { generics },
},
option_is_nullable,
)?;
let schema_tokens = schema.to_token_stream();

tokens.extend(quote! { .schema(Some(#schema_tokens)).build() });
Expand Down
55 changes: 36 additions & 19 deletions utoipa-gen/src/path/parameter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,16 +126,21 @@ impl ToTokensDiagnostics for Parameter<'_> {
))]
impl<'a> From<crate::ext::ValueArgument<'a>> for Parameter<'a> {
fn from(argument: crate::ext::ValueArgument<'a>) -> Self {
let parameter_in = if argument.argument_in == crate::ext::ArgumentIn::Path {
ParameterIn::Path
} else {
ParameterIn::Query
};

let option_is_nullable = parameter_in != ParameterIn::Query;

Self::Value(ValueParameter {
name: argument.name.unwrap_or_else(|| Cow::Owned(String::new())),
parameter_in: if argument.argument_in == crate::ext::ArgumentIn::Path {
ParameterIn::Path
} else {
ParameterIn::Query
},
parameter_in,
parameter_schema: argument.type_tree.map(|type_tree| ParameterSchema {
parameter_type: ParameterType::External(type_tree),
features: Vec::new(),
option_is_nullable,
}),
..Default::default()
})
Expand All @@ -160,6 +165,7 @@ impl<'a> From<crate::ext::IntoParamsType<'a>> for Parameter<'a> {
struct ParameterSchema<'p> {
parameter_type: ParameterType<'p>,
features: Vec<Feature>,
option_is_nullable: bool,
}

impl ToTokensDiagnostics for ParameterSchema<'_> {
Expand All @@ -178,14 +184,17 @@ impl ToTokensDiagnostics for ParameterSchema<'_> {
let required: Required = (!type_tree.is_option()).into();

to_tokens(
ComponentSchema::new(component::ComponentSchemaProps {
type_tree,
features: self.features.clone(),
description: None,
container: &Container {
generics: &Generics::default(),
ComponentSchema::for_params(
component::ComponentSchemaProps {
type_tree,
features: self.features.clone(),
description: None,
container: &Container {
generics: &Generics::default(),
},
},
})?
self.option_is_nullable,
)?
.to_token_stream(),
required,
);
Expand All @@ -199,14 +208,17 @@ impl ToTokensDiagnostics for ParameterSchema<'_> {
schema_features.push(Feature::Inline(inline_type.is_inline.into()));

to_tokens(
ComponentSchema::new(component::ComponentSchemaProps {
type_tree: &type_tree,
features: schema_features,
description: None,
container: &Container {
generics: &Generics::default(),
ComponentSchema::for_params(
component::ComponentSchemaProps {
type_tree: &type_tree,
features: schema_features,
description: None,
container: &Container {
generics: &Generics::default(),
},
},
})?
self.option_is_nullable,
)?
.to_token_stream(),
required,
);
Expand Down Expand Up @@ -267,6 +279,7 @@ impl Parse for ValueParameter<'_> {
})
})?),
features: Vec::new(),
option_is_nullable: true,
});
}
} else {
Expand All @@ -289,6 +302,10 @@ impl Parse for ValueParameter<'_> {
parameter.features = (schema_features.clone(), parameter_features);
if let Some(parameter_schema) = &mut parameter.parameter_schema {
parameter_schema.features = schema_features;

if parameter.parameter_in == ParameterIn::Query {
parameter_schema.option_is_nullable = false;
}
}

Ok(parameter)
Expand Down
Loading
Loading