rust serialize error: UiNodeInputAttributesValue about sdk HOT 19 CLOSED

Linked error for reference
OpenAPITools/openapi-generator#9497

from sdk.

I found some issues in our OpenAPI Spec which have been resolved in Ory Kratos master and cargo test now passes for the newest spec!

from sdk.

This looks like a bug in the rust generator, please report the bug over there - the bug looks similar to the one I found in dotnet: OpenAPITools/openapi-generator#9442

from sdk.

@aeneasr Thanks for your reply. I'll open a issue there as well, since ory_kratos_client is on crates.io now, I think this bug should be kept as a reference.

from sdk.

Nice :) Could you link the issue here so we can keep track of upstreams? :)

from sdk.

I might have found a solution - check out: #75 (comment)

The updated Rust client can be found here: https://github.com/ory/sdk/tree/openapi-fixy/clients/kratos/rust

Does that fix the issue?

from sdk.

@aeneasr Just got back from work, haven't got time to file the issue on the generator repo. I'll update it this weekend.

Sorry it still produce empty rust struct.

This is the generated rust struct which shoudn't be empty

This is the golang equivalent

Where the spec says it's gonna be:

    uiNodeInputAttributesValue:
      oneOf:
        - type: string
        - type: number
        - type: boolean

from sdk.

No problem! I believe that the linked Rust struct is no longer used in the SDK. Check my comment over here: #75 (comment)

Could you test the linked commit in the comment in Rust to see if kt resolves the issue as it does for Java? Thanks!

from sdk.

@aeneasr

Thanks again for helping out, I've tried with the 0.7.0 rust client again.

And it hit me with a different error this time
I'm hit with the this path "/self-service/login/api",
the response body contains ui.nodes.attributes which should be the following according to the spec:

    uiNodeAttributes:
      oneOf:
        - $ref: '#/components/schemas/uiNodeInputAttributes'
        - $ref: '#/components/schemas/uiNodeTextAttributes'
        - $ref: '#/components/schemas/uiNodeImageAttributes'
        - $ref: '#/components/schemas/uiNodeAnchorAttributes'

But the generated rust struct is a combined of all variants:

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Default)]
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,
    /// The input's value.
    #[serde(rename = "value", skip_serializing_if = "Option::is_none")]
    pub value: Option<serde_json::Value>,
    #[serde(rename = "text", default)]
    pub text: Box<crate::models::UiText>,
    /// The image's source URL.  format: uri
    #[serde(rename = "src", skip_serializing_if = "Option::is_none")]
    pub src: Option<String>,
    /// The link's href (destination) URL.  format: uri
    #[serde(rename = "href", default)]
    pub href: String,
    #[serde(rename = "title", default)]
    pub title: Box<crate::models::UiText>,
}

Which will failed to work with serde_json if either one of the fields are non-existing.
I would expect the openapi-generator to generate a enum rather than a mixed struct here.
Currently a workaround can be applied, that is to use serde(default) to tell the serializer to use default value when field is missing.
Which might break semantics of the client but would work if null or empty fields are not look at.

Are there any option to separate the api and models spec so that the api options are all valid, while the respond body can fail if parsed wrongly, but will not fail when the client have already completed the request calling kratos? In rust semantics:

async fn execute() -> Result<RawPayload, RawErrorWithoutDeserialzied>;

from sdk.

Hard to say, you'd probably have to dive into the openapi-generator. Unfortunately, I don't know any rust :/

from sdk.

@aeneasr

Scrolling through all the oneOf usage in the generated client, only UiNodeAttributes didn't generate correct results:
All the other 5 out of 6 usage of the oneOf keyword results in correct rust enum.

I think it'll be easy hand roll a rust enum to replace that struct!!
Did a pr against this repo allowed, or are there any ideal repo the rust client sdk? I saw the golang-sdk separated with this repo.

from sdk.

#90 created a pr to fix this issue

from sdk.

Thank you for the PR - I am reopening this issue. The problem with the PR is that the code is fully autogenerated, so the next push will overwrite this.

I think we could try and figure out why all other uses work, while this use doesn't work. Is it maybe because we are lacking a discriminator definition?

from sdk.

cargo test may now pass, but UiNodeAttributes is still generated as a concatenation of the four structs. Adding the following discriminator seems to help.

"discriminator": {
          "mapping": {
            "input": "#/components/schemas/uiNodeInputAttributes",
            "text": "#/components/schemas/uiNodeTextAttributes",
            "image": "#/components/schemas/uiNodeImageAttributes",
            "anchor": "#/components/schemas/uiNodeAnchorAttributes"
          },
          "propertyName": "nodetype"
},

UiNodeAttributes generated from the above:

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

But this would require adding a new field to the JSON payload. There is precedent for this, as there are already five discriminators in the spec. @aeneasr what do you think about adding another one here to help with the generation of this client?

from sdk.

I see, yeah unfortunately I was unaware of how discriminators work when I worked on the structure. For some reason it appears not to be possible to habe the discriminator in the outer object…

But yeah adding a discriminator makes sense here. Have you come to the same conclusion as I here regarding where the discriminator can be?

from sdk.

It seems to me that each of the four structs need to have an extra property with the same name but different values that acts as the discriminator.

from sdk.

Damn :/ It looks like the current approach is valid polymorphism in OpenAPI spec (see Polymorphism section in https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/) but apparently the Rust generator is not dealing with it properly (see OpenAPITools/openapi-generator#9497).

from sdk.

Hm the only alternative would be to have the oneOf not for attributes but for the whole node and then use the discriminator in the „type“. Theoretically we could also use that pattern but it would definitely be a larger breaking change in the generated SDK code. I think the easiest would be indeed to add node_type to the payload and then use it as a discriminator. Would you be open to contribute this? :)

from sdk.

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers a year. That does not imply that the issue has no merit! If you feel strongly about this issue

• open a PR referencing and resolving the issue;
• leave a comment on it and discuss ideas how you could contribute towards resolving it;
• leave a comment and describe in detail why this issue is critical for your use case;
• open a new issue with updated details and a plan on resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneous you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

from sdk.