utopia-gen: Adjust params code to not set nullable on Option for …

…`Query` params (#1248)
juhaku · Dec 19, 2024 · de536e4 · de536e4
1 parent 66b80a4
commit de536e4
Show file tree

Hide file tree

Showing 9 changed files with 132 additions and 104 deletions.
diff --git a/utoipa-gen/CHANGELOG.md b/utoipa-gen/CHANGELOG.md
@@ -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)

diff --git a/utoipa-gen/src/component.rs b/utoipa-gen/src/component.rs
@@ -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),
@@ -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> {
@@ -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()

diff --git a/utoipa-gen/src/component/features/attributes.rs b/utoipa-gen/src/component/features/attributes.rs
@@ -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))

diff --git a/utoipa-gen/src/component/into_params.rs b/utoipa-gen/src/component/into_params.rs
@@ -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() });

diff --git a/utoipa-gen/src/path/parameter.rs b/utoipa-gen/src/path/parameter.rs
@@ -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()
         })
@@ -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<'_> {
@@ -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,
                 );
@@ -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,
                 );
@@ -267,6 +279,7 @@ impl Parse for ValueParameter<'_> {
                         })
                     })?),
                     features: Vec::new(),
+                    option_is_nullable: true,
                 });
             }
         } else {
@@ -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)