utoipa icon indicating copy to clipboard operation
utoipa copied to clipboard

IntoParams marks Options as required

Open FloppyDisck opened this issue 1 year ago • 5 comments

Hello, the below snippet shows all of the values as required in the Swagger doc

#[derive(Deserialize, IntoParams)]
struct Request {
    ids: Option<String>,
    with_price_ids: Option<String>,
    without_flags: Option<String>,
    with_flags: Option<String>,
}

This is the generated Open API spec

"parameters": [
          {
            "name": "ids",
            "in": "path",
            "required": true,
            "schema": {
              "type": "string",
              "nullable": true
            }
          },
          {
            "name": "with_price_ids",
            "in": "path",
            "required": true,
            "schema": {
              "type": "string",
              "nullable": true
            }
          },
          {
            "name": "without_flags",
            "in": "path",
            "required": true,
            "schema": {
              "type": "string",
              "nullable": true
            }
          },
          {
            "name": "with_flags",
            "in": "path",
            "required": true,
            "schema": {
              "type": "string",
              "nullable": true
            }
          }
        ],

My expectation is that any optional value is not set as not required by default, or at least have some sort of param to explicitly set as not required, such as #[param(not-required)]

FloppyDisck avatar May 02 '23 07:05 FloppyDisck

This seems like a duplicate of https://github.com/juhaku/utoipa/issues/529

carueda avatar Jul 03 '23 02:07 carueda

@carueda @FloppyDisck

This seems like a duplicate of https://github.com/juhaku/utoipa/issues/529

Yes.

The field nullability and requirability is defined here https://docs.rs/utoipa/latest/utoipa/derive.ToSchema.html#field-nullability-and-required-rules. Any other behavior should be considered a bug.

But in short just defining a Option<Type> does not make it non-required to better support the code generation libraries and based on ideology that null serializes to None and vise versa. If a parameter need to be non-required then it must have a default value.

Related #479 #314

juhaku avatar Jul 11 '23 04:07 juhaku

Thanks @juhaku .

In case it helps @FloppyDisck, I faced some similar issue:

#[derive(Deserialize, IntoParams, Debug)]
#[serde(rename_all = "camelCase")]
pub struct PositionsQuery {
    /// Maximum number of last positions to report
    last_number_of_fixes: Option<u32>,
    /// Lower limit for time range restriction
    start_date: Option<String>,
    /// Upper limit for time range restriction
    end_date: Option<String>,
}

/// Get latest platform positions.
#[utoipa::path(
    get,
    path = "/trackdb/platforms/{platform_id}/positions",
    params(
        ("platform_id" = String, Path, description = "Platform ID"),
        PositionsQuery,
    ),
    responses(
       ...
    )
)]
pub async fn get_platform_positions(
    Path(platform_id): Path<String>,
    query: Query<PositionsQuery>,
) -> impl IntoResponse {
...

image

But then found out that with #[into_params(parameter_in = Query)] those Option params started being rendered as non-required:

#[derive(Deserialize, IntoParams, Debug)]
#[into_params(parameter_in = Query)]
#[serde(rename_all = "camelCase")]
pub(super) struct PositionsQuery {
...

image

EDIT: The above using utoipa = "3.3.0".

carueda avatar Jul 11 '23 16:07 carueda

@juhaku - just noted your recent https://github.com/juhaku/utoipa/issues/622#issuecomment-1630142681:

...need to provide explicitly the #[into_params(parameter_in = Query)] attribute. If I remember this should not be necessary...

so, I shall be on the lookout for when this becomes unnecessary.

(btw, thanks a lot for the super useful library!)

carueda avatar Jul 11 '23 16:07 carueda

Sorry folks, I somehow overlooked this part of the documentation regarding features:

axum_extras Enhances axum framework integration allowing users to use IntoParams without defining the parameter_in attribute.

I just enabled that feature and it indeed removes the need for `#[into_params(parameter_in = Query)].

carueda avatar Jul 23 '23 21:07 carueda