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

[BUG] [rust-client] oneOf is mapped into a struct instead of enum #9497

Open
5 of 6 tasks
cortopy opened this issue May 16, 2021 · 5 comments
Open
5 of 6 tasks

[BUG] [rust-client] oneOf is mapped into a struct instead of enum #9497

cortopy opened this issue May 16, 2021 · 5 comments

Comments

@cortopy
Copy link

cortopy commented May 16, 2021

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

oneOf support for models is broken for rust-client. Serde will always fail when parsing responses that contain a model with oneOf

openapi-generator version

5.1.1 and also tested with master

OpenAPI declaration file content or url

https://raw.githubusercontent.com/ory/sdk/master/spec/kratos/v0.6.2-alpha.1.json

There are two problematic cases of oneOf in that spec

The first one is a case in which all possible values are json scalar values. For example:

"uiNodeInputAttributesValue": {
        "oneOf": [
          { "type": "string" },
          { "type": "number" },
          { "type": "boolean" }
        ]
      },

gets trasnformed into

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct UiNodeInputAttributesValue {
}

impl UiNodeInputAttributesValue {
    pub fn new() -> UiNodeInputAttributesValue {
        UiNodeInputAttributesValue {
        }
    }
}

but this is wrong because UiNodeInputAttributesValue is not a struct.

My hack for this particular case is to use:

pub type UiNodeInputAttributesValue = serde_json::Value;

However, this does not solve all cases, since oneOf can also be used with references to other schemas. Indeed, there is another problematic case in this spec:

"uiNodeAttributes": {
        "oneOf": [
          { "$ref": "#/components/schemas/uiNodeInputAttributes" },
          { "$ref": "#/components/schemas/uiNodeTextAttributes" },
          { "$ref": "#/components/schemas/uiNodeImageAttributes" },
          { "$ref": "#/components/schemas/uiNodeAnchorAttributes" }
        ],
        "title": "Attributes represents a list of attributes (e.g. `href=\"foo\"` for links)."
      },

Since this is a valid spec and all the models are present, the files for each type of uiNodeAttributes is created ok and accurately. However, the generated code for uiNodeAttributes is incorrect. The output result merges all properties like:

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct UiNodeAttributes {
    /// Sets the input's disabled field to true or false.
    #[serde(rename = "disabled")]
    pub disabled: bool,
    #[serde(rename = "label", skip_serializing_if = "Option::is_none")]
    pub label: Option<Box<crate::models::UiText>>,
    /// The input's element name.
    #[serde(rename = "name")]
    pub name: String,
    /// The input's pattern.
    #[serde(rename = "pattern", skip_serializing_if = "Option::is_none")]
    pub pattern: Option<String>,
    /// Mark this input field as required.
    #[serde(rename = "required", skip_serializing_if = "Option::is_none")]
    pub required: Option<bool>,
    #[serde(rename = "type")]
    pub _type: String,
    #[serde(rename = "value", skip_serializing_if = "Option::is_none")]
    pub value: Option<Box<crate::models::UiNodeInputAttributesValue>>,
    #[serde(rename = "text")]
    pub text: Box<crate::models::UiText>,
    /// The image's source URL.  format: uri
    #[serde(rename = "src")]
    pub src: String,
    /// The link's href (destination) URL.  format: uri
    #[serde(rename = "href")]
    pub href: String,
    #[serde(rename = "title")]
    pub title: Box<crate::models::UiText>,
}

impl UiNodeAttributes {
    pub fn new(disabled: bool, name: String, _type: String, text: crate::models::UiText, src: String, href: String, title: crate::models::UiText) -> UiNodeAttributes {
        UiNodeAttributes {
            disabled,
            label: None,
            name,
            pattern: None,
            required: None,
            _type,
            value: None,
            text: Box::new(text),
            src,
            href,
            title: Box::new(title),
        }
    }
}

But this is extremely problematic and wrong, as that struct will never exist. UiNodeAttributes is assumed to have ALL properties from the polymorphic types. Since each of those types have different required properties, the UiNodeAttributes assumes that ALL required properties will always exist, which will never happen.

Generation Details
docker run --rm -v "${PWD}:/local" \
	--user 1001:1001 \
	openapitools/openapi-generator-cli generate \
    -i https://raw.githubusercontent.com/ory/sdk/master/spec/kratos/v0.6.2-alpha.1.json \
    -c /local/options.yaml \
    -g rust \
    -o /local/packages/kratos
Steps to reproduce

Run the command above

Related issues/PRs

#2244 added support for multiple response types, but not for oneOf

Suggest a fix

Ideally, the generated code should generate Rust enums in the same way that polymorphism is treated with response errors

Another solution could be to continue merging all possible properties but make them all Option

The accepted answer in https://stackoverflow.com/questions/37561593/how-can-i-use-serde-with-a-json-array-with-different-objects-for-successes-and-e summarises the two possibilities

@cortopy cortopy changed the title [BUG] oneOf is mapped into a struct instead of enum [BUG] [rust-client] oneOf is mapped into a struct instead of enum May 16, 2021
@richardwhiuk
Copy link
Contributor

In case it's useful, the client generated by the rust-server generator should support this usecase.

@cortopy
Copy link
Author

cortopy commented May 17, 2021

thanks for that @richardwhiuk but I've just tried and issues are the same. Models generated are not correct

@extraymond
Copy link

@cortopy I encounter the same error.

However in the openapi.json format, only this one that uses oneOf generated a combined struct instead of enum.
I'm guessing there might be a limit for the parser for oneOf to generate untag enum.

@erikh
Copy link

erikh commented Apr 26, 2022

ran into this problem tonight: here's the rundown from my perspective: zerotier/zeronsd#126 (comment)

juhaku pushed a commit to juhaku/utoipa that referenced this issue Jul 24, 2022
Add support for `title` property ([OpenAPI Schema title](https://swagger.io/specification/#schema-object)) for struct and enum `Component` properties.

The title is useful for enums because currently the generator for rust 
clients created a struct for each variant and name them like:

    my_enum_one_of_1
    my_enum_one_of_2
    etc

So with the title you can at least control the name of each variant structure. 
There is an open issue to actually produce an enum but it's not been worked 
on for a while OpenAPITools/openapi-generator#9497.
@DDtKey
Copy link
Contributor

DDtKey commented Feb 24, 2024

Should be closed after #13970 and subsequent #17915.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants