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] Enum Varients Merging Incorrectly, breaking change #17869

Closed
5 of 6 tasks
RobbieMcKinstry opened this issue Feb 14, 2024 · 0 comments · Fixed by #17915
Closed
5 of 6 tasks

[BUG] [RUST] Enum Varients Merging Incorrectly, breaking change #17869

RobbieMcKinstry opened this issue Feb 14, 2024 · 0 comments · Fixed by #17915

Comments

@RobbieMcKinstry
Copy link

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

I'm receiving a new compilation error when using an email with two tuple variants that reduce to the same type. Unfortunately, I suspect the error is related to #13970 .

I'm using the newtype pattern to validate the string with RAII. I have two types

// My email type.
struct Email(String)
// My account name type
struct AccountName(String)

Because I want my API route to access either an email or an account name using login, I have a third type:

#[derive(ToSchema, Debug, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
#[serde(untagged)]
enum AccountOrEmail {
    EmailAddress(Email),
    AccountName(AccountName),
}

(These enum varients may look identical but they're actually mutually exclusive and disambiguated during serde::deserialize via a TryFrom impl.)

I'm using Utoipa to generate an openapi.json file for my Rust server, and I generate my client using the Docker image for openapi-generator.

Using the Docker image tagged at v7.2.0, the AccountOrEmail type would generate as...

/*
 * {{Omitted from GitHub Issue}}
 *
 * {{Omitted from GitHub Issue}}
 *
 * The version of the OpenAPI document: 0.1.0
 * 
 * Generated by: https://openapi-generator.tech
 */

/// AccountOrEmail : When logging in, users can identify their account by either account name or email address, since both are unique to their account.



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

impl AccountOrEmail {
    /// When logging in, users can identify their account by either account name or email address, since both are unique to their account.
    pub fn new() -> AccountOrEmail {
        AccountOrEmail {
        }
    }
}

This type is completely useless, but it did compile. #13970 sought to make the generated code actually useful (and I really benefit from the PR!). However, using the latest docker image, which I confirmed was published after the PR landed, the following code is generated:

/*
 * 
 * 
 * Generated by: https://openapi-generator.tech
 */

use super::String;



#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum AccountOrEmail {
    String(Box<String>),
}

impl Default for AccountOrEmail {
    fn default() -> Self {
        Self::String(Box::default())
    }
}

Note the spurious import of use super::String; no such file exists, and the name conspicuously conflicts with std::string::String in the default namespace. This code no longer compiles. Here's the console output:

   Compiling serde v1.0.196
   Compiling proc-macro2 v1.0.78
   Compiling quote v1.0.35
   Compiling syn v2.0.48
    Checking serde_json v1.0.113
    Checking serde_urlencoded v0.7.1
    Checking uuid v1.7.0
   Compiling serde_derive v1.0.196
    Checking reqwest v0.11.24
    Checking wack-http-client v1.0.0 (/Users/robbiemckinstry/workspace/{{omitted from GitHub issue}})
error[E0432]: unresolved import `super::String`
 --> crates/wack-http-client/src/models/account_or_email.rs:7:5
  |
7 | use super::String;
  |     ^^^^^^^^^^^^^ no `String` in `models`
  |
help: consider importing one of these items instead
  |
7 | use crate::models::AccountOrEmail::String;
  |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
7 | use serde::__private::de::Content::String;
  |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
7 | use serde_json::Value::String;
  |     ~~~~~~~~~~~~~~~~~~~~~~~~~
7 | use std::string::String;
  |     ~~~~~~~~~~~~~~~~~~~

For more information about this error, try `rustc --explain E0432`.
error: could not compile `wack-http-client` (lib) due to 1 previous error
openapi-generator version

I'm using the latest Docker image (accessed on 2/14/24). This appears to be a regression; using the v7.2.0 image, the code will compile (but it won't be useful without #13790).

OpenAPI declaration file content or url

I paired down the generated specification to be the smaller possible valid spec that reproduces the issue:

{
  "openapi": "3.0.3",
  "info": {
    "title": "Omitted",
    "description": "Omitted",
    "license": {
      "name": ""
    },
    "version": "0.1.0"
  },
  "servers": [
    {
      "url": "http://localhost:8080",
      "description": "Omitted"
    }
  ],
  "paths": {},
  "components": {
    "schemas": {
      "AccountName": {
        "type": "string",
        "description": "An account name."
      },
      "AccountOrEmail": {
        "oneOf": [
          {
            "$ref": "#/components/schemas/Email"
          },
          {
            "$ref": "#/components/schemas/AccountName"
          }
        ],
        "description": "Either an account name or an email."
      },
      "Email": {
        "type": "string",
        "description": "An email address."
      }
    },
    "responses": {}
  }
}
Generation Details
docker run --rm -v "$PWD:/local" \
  openapitools/openapi-generator-cli:latest generate \
  --skip-validate-spec \
  -i /local/target/debug/openapi.json \  # NOTE: Replace with the file provided above
  -g rust \
  --package-name "wack-http-client" \
  -o /local/crates/wack-http-client
Steps to reproduce
  1. Copy the openapi.json file I included above into $PWD/openapi.json.
  2. Run the following:
docker run --rm -v "$PWD:/local" \
  openapitools/openapi-generator-cli:latest generate \
  --skip-validate-spec \
  -i /local/openapi.json \  # NOTE: Replace with the file provided above
  -g rust \
  --package-name "foobar" \
  -o /local/foobar
Related issues/PRs

#13970

I really hope this bug can be patched without reversing the above PR, because it's a much needed addition to the project 😅

Suggest a fix

It looks like the two newtypes are getting incorrectly merged; instead, I believe the models for AccountName and Email should be generated and referenced in the enum.

DDtKey added a commit to DDtKey/openapi-generator that referenced this issue Feb 20, 2024
Partially solves OpenAPITools#17869
There is a downside - variants names doesn't reflect model name of reference

It also doesn't solve issue of import, will be solved separately
DDtKey added a commit to DDtKey/openapi-generator that referenced this issue Feb 20, 2024
Partially solves OpenAPITools#17869
There is a downside - variants names doesn't reflect model name of reference

It also doesn't solve issue of import, will be solved separately
DDtKey added a commit to DDtKey/openapi-generator that referenced this issue Feb 21, 2024
Solves OpenAPITools#17869 and OpenAPITools#17896 and also includes unmerged $17898

Unfortunately it affects quite a lot of code, but we can see that only client-side models were affected by re-generation.
I tried to split this PR to several, but they're really coupled and hard to create a chain of PRs.
wing328 pushed a commit that referenced this issue Feb 24, 2024
* fix(rust): discriminator mapping to serde rename

Discriminator mapping has been ignored in some cases.
Even existing samples had wrong definition in some cases

This PR addresses this

* fix(rust): `oneOf` generation for client

Solves #17869 and #17896 and also includes unmerged $17898

Unfortunately it affects quite a lot of code, but we can see that only client-side models were affected by re-generation.
I tried to split this PR to several, but they're really coupled and hard to create a chain of PRs.

* fix: indentation in `impl Default`

* missing fixes

* fix: correct typeDeclaration with unaliased schema

* style: improve indentation for models

* fix: user toModelName for aliases of oneOf

* refactor: unify `getTypeDeclaration` for rust

* cover the case when `mapping` has the same `ref` for different mapping names

* test: add test for previous change

* style: remove extra qualified path to models

* add some comments

* fix(build): use method of `List` instead of specific for `LinkedList`
oscarr-reyes pushed a commit to oscarr-reyes/openapi-generator that referenced this issue Feb 25, 2024
* fix(rust): discriminator mapping to serde rename

Discriminator mapping has been ignored in some cases.
Even existing samples had wrong definition in some cases

This PR addresses this

* fix(rust): `oneOf` generation for client

Solves OpenAPITools#17869 and OpenAPITools#17896 and also includes unmerged $17898

Unfortunately it affects quite a lot of code, but we can see that only client-side models were affected by re-generation.
I tried to split this PR to several, but they're really coupled and hard to create a chain of PRs.

* fix: indentation in `impl Default`

* missing fixes

* fix: correct typeDeclaration with unaliased schema

* style: improve indentation for models

* fix: user toModelName for aliases of oneOf

* refactor: unify `getTypeDeclaration` for rust

* cover the case when `mapping` has the same `ref` for different mapping names

* test: add test for previous change

* style: remove extra qualified path to models

* add some comments

* fix(build): use method of `List` instead of specific for `LinkedList`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant