Regression: Option<> fields are labeled as required in the generated OpenAPI spec.

[ryzhyk]

Given the following Rust definition:

#[derive(Deserialize, ToSchema)]
pub struct FileInputConfig {
    path: String,
    buffer_size_bytes: Option<usize>,
    #[serde(default)]
    follow: bool,
}

utoipa generates the following OpenAPI schema that (I think) incorrectly labels buffer_size_bytes as required:

      "FileInputConfig": {
        "properties": {
          "buffer_size_bytes": {
            "minimum": 0,
            "nullable": true,
            "type": "integer"
          },
          "follow": {
            "type": "boolean"
          },
          "path": {
            "type": "string"
          }
        },
        "required": [
          "path",
          "buffer_size_bytes"
        ],
        "type": "object"
      },

I believe this behavior was introduced recently. It works correctly in v3.0.3.

[juhaku]

This is by the desing, since Option::None will serialize to null with serde (See issue #314). While this might seem incovenient to some it actually follows more strictly how serde will serialize types.

Also this change allows client side generators to generate the correct client types which was not possible previously because it handled the types incorrectly. Basically Option in OpenAPI means nullable type that is required by desing unless it has either

• serde's double option,
• default value
• or skip_serializing_if attribute set.

See more explanation here: #479 (comment)

[ryzhyk]

@juhaku , thanks for this really useful crate.

If the goal is to be consistent with serde, then I don't think this is correct behavior, since serde will happily deserialize optional fields that are missing in JSON (as opposed to being set to null).

As for the cliend-side generator compatibility, I actually ran into this problem because my code using openapi-python-client-generated bindings started to crash because of missing required fields. So it might be technically more correct (although it's coutner-intuitive to me that Option<> should mean nullable rather than, well, "optional"), but it also seems more error-prone.

[juhaku]

thanks for this really useful crate.

It's my pleasure ❤️

It seems like it does both. In example below both will deserialize to correct rust type. It deserializes null and empty value as None.

#[derive(Serialize, Deserialize, Debug)]
struct Foobar {
    value: Option<String>
}

let value: Foobar = serde_json::from_str(r###"{"value": null}"###).unwrap();
let value2: Foobar = serde_json::from_str(r###"{}"###).unwrap();

// will deserialize to
let value = Foobar { value: None };

I think we could get some middle ground here with making it not required as previously, but it anyway will still be nullable. Other option I guess would be to add strict feature flag to the crate, that when enabled would behave as it does now and if disabled then it would consider Option types as non required as before.

[ryzhyk]

It seems like it does both.

Yes, that's what I meant, apologies about lack of clarity.

I think we could get some middle ground here with making it not required as previously, but it anyway will still be nullable.

This combination would make the most sense to me. Would this work in the use case brought up in #314?

Other option I guess would be to add strict feature flag to the crate, that when enabled would behave as it does now and if disabled then it would consider Option types as non required as before.

Wouldn't it be cleaner to make it an attribute that can be attached to a specific field?

[juhaku]

This combination would make the most sense to me. Would this work in the use case brought up in #314?

I believe it would at least partly. Speaking of TypeScript, now with the current implementation Option<T> would produce type like T | null. But if we make it non required as well then it would produce type like T | null | undefined. Which is technically correct but not a strict interpretation. Still I reckon that would be good enough.

Wouldn't it be cleaner to make it an attribute that can be attached to a specific field?

You mean adding a required attribute?. That could be also done. Though there already is nullable attribute which does not have much use now since the nullability is interpreted automatically by now. Probably it would be a good idea to have support for declaring required attribute explicitly if needed.

[ryzhyk]

This combination would make the most sense to me. Would this work in the use case brought up in #314?

I believe it would at least partly. Speaking of TypeScript, now with the current implementation Option<T> would produce type like T | null. But if we make it non required as well then it would produce type like T | null | undefined. Which is technically correct but not a strict interpretation. Still I reckon that would be good enough.

I don't know TypeScript, so can't comment on this.

Wouldn't it be cleaner to make it an attribute that can be attached to a specific field?

You mean adding a required attribute?.

Yep, or the other way around -- add optional attribute, and make required the default behavior. Although I prefer your suggestion.

That could be also done. Though there already is nullable attribute which does not have much use now since the nullability is interpreted automatically by now. Probably it would be a good idea to have support for declaring required attribute explicitly if needed.

[juhaku]

Yeah, I reckon I'll add required attribute that can be declared optionally above the fields and by default considering Option fields as non-required as before. That would make the best of both worlds.

[ryzhyk]

Sounds great, thanks!

[juhaku]

@ryzhyk There is a new release available in crates for grab.