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

[Rust] Add support for discriminator #3895

Merged
merged 7 commits into from
Oct 6, 2019

Conversation

gferon
Copy link
Contributor

@gferon gferon commented Sep 16, 2019

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Example

A schema that looks like:

components:
  schemas:
    Pet:
      type: object
      required:
      - pet_type
      properties:
        pet_type:
          type: string
      discriminator:
        propertyName: pet_type
        mapping:
          cachorro: Dog
          reptilian: Lizard
    Cat:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        # all other properties specific to a `Cat`
        properties:
          name:
            type: string
    Dog:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        # all other properties specific to a `Dog`
        properties:
          bark:
            type: string
    Lizard:
      allOf:
      - $ref: '#/components/schemas/Pet'
      - type: object
        # all other properties specific to a `Lizard`
        properties:
          lovesRocks:
            type: boolean

would generate a Rust enum like:

#[derive(Debug, PartialEq, Serialize, Deserialize)]
#[serde(tag = "pet_type")]
pub enum Pet {
    #[serde(rename="cachorro")]
    Dog {
        #[serde(rename = "bark", skip_serializing_if = "Option::is_none")]
        bark: Option<String>,
    },
    #[serde(rename="reptilian")]
    Lizard {
        #[serde(rename = "lovesRocks", skip_serializing_if = "Option::is_none")]
        loves_rocks: Option<bool>,
    },
}

@wing328
Copy link
Member

wing328 commented Sep 16, 2019

Errors reported by the Travis CI:

   Checking tokio-threadpool v0.1.15
    Checking tokio-udp v0.1.5
    Checking tokio-uds v0.2.5
    Checking tokio-tcp v0.1.3
    Checking tokio-fs v0.1.6
    Checking tokio v0.1.22
    Checking tokio-core v0.1.17
   Compiling serde_derive v1.0.101
    Checking tokio-proto v0.1.1
    Checking hyper v0.11.27
    Checking petstore_client v1.0.0 (/home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/rust)
error[E0252]: the name `Status` is defined multiple times
  --> src/models/mod.rs:10:9
   |
7  | pub use self::order::Status;
   |         ------------------- previous import of the type `Status` here
...
10 | pub use self::pet::Status;
   |         ^^^^^^^^^^^^^^^^^ `Status` reimported here
   |
   = note: `Status` must be defined only once in the type namespace of this module
help: you can use `as` to change the binding name of the import
   |
10 | pub use self::pet::Status as OtherStatus;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: unreachable expression
   --> src/apis/pet_api.rs:107:9
    |
107 | /         __internal_request::Request::new(hyper::Method::Post, "/pet/{petId}/uploadImage".to_string())
108 | |             .with_auth(__internal_request::Auth::Oauth)
109 | |             .with_path_param("petId".to_string(), pet_id.to_string())
110 | |             .with_form_param("additionalMetadata".to_string(), additional_metadata.to_string())
111 | |             .with_form_param("file".to_string(), unimplemented!())
    | |__________________________________________________________________^
    |
    = note: #[warn(unreachable_code)] on by default
error: aborting due to previous error
For more information about this error, try `rustc --explain E0252`.
error: Could not compile `petstore_client`.

ref: https://travis-ci.org/OpenAPITools/openapi-generator/builds/585652775

We can't re-export those types because their name is derived from the property name, which might
be duplicated.
@gferon gferon force-pushed the add-rust-discriminator branch from e3a81e4 to 3c1d778 Compare September 16, 2019 18:40
@gferon
Copy link
Contributor Author

gferon commented Sep 16, 2019

Errors reported by the Travis CI:

   Checking tokio-threadpool v0.1.15
    Checking tokio-udp v0.1.5
    Checking tokio-uds v0.2.5
    Checking tokio-tcp v0.1.3
    Checking tokio-fs v0.1.6
    Checking tokio v0.1.22
    Checking tokio-core v0.1.17
   Compiling serde_derive v1.0.101
    Checking tokio-proto v0.1.1
    Checking hyper v0.11.27
    Checking petstore_client v1.0.0 (/home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/rust)
error[E0252]: the name `Status` is defined multiple times
  --> src/models/mod.rs:10:9
   |
7  | pub use self::order::Status;
   |         ------------------- previous import of the type `Status` here
...
10 | pub use self::pet::Status;
   |         ^^^^^^^^^^^^^^^^^ `Status` reimported here
   |
   = note: `Status` must be defined only once in the type namespace of this module
help: you can use `as` to change the binding name of the import
   |
10 | pub use self::pet::Status as OtherStatus;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: unreachable expression
   --> src/apis/pet_api.rs:107:9
    |
107 | /         __internal_request::Request::new(hyper::Method::Post, "/pet/{petId}/uploadImage".to_string())
108 | |             .with_auth(__internal_request::Auth::Oauth)
109 | |             .with_path_param("petId".to_string(), pet_id.to_string())
110 | |             .with_form_param("additionalMetadata".to_string(), additional_metadata.to_string())
111 | |             .with_form_param("file".to_string(), unimplemented!())
    | |__________________________________________________________________^
    |
    = note: #[warn(unreachable_code)] on by default
error: aborting due to previous error
For more information about this error, try `rustc --explain E0252`.
error: Could not compile `petstore_client`.

ref: https://travis-ci.org/OpenAPITools/openapi-generator/builds/585652775

good catch, sorry for that. I've adapted my changes.

@gferon
Copy link
Contributor Author

gferon commented Sep 16, 2019

Pinging @bjgill and @richardwhiuk for a review

@bjgill
Copy link
Contributor

bjgill commented Sep 17, 2019

Thanks. I'm quite busy this week, so will take a look next week unless this is urgent.

Copy link
Contributor

@bjgill bjgill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a sensible change. Just a couple of questions.

FYI - I'm about to merge #3543, which will conflict with this. You should just need to regenerate the samples to sort out the conflicts.

@bjgill
Copy link
Contributor

bjgill commented Sep 27, 2019

I've just merged the conflicting PR. You should just need to rebase onto master and re-generate the samples again.

@gferon
Copy link
Contributor Author

gferon commented Oct 1, 2019

I've just merged the conflicting PR. You should just need to rebase onto master and re-generate the samples again.

Done, I added a small change to the template files to remove some clippy warnings as well.

@bjgill
Copy link
Contributor

bjgill commented Oct 6, 2019

Thanks. I'll merge this, which should fix #4079

@bjgill bjgill merged commit f96ed69 into OpenAPITools:master Oct 6, 2019
@gferon gferon deleted the add-rust-discriminator branch October 7, 2019 15:55
borsboom added a commit to borsboom/ynab-api that referenced this pull request Oct 9, 2019
This patch includes:
- Add all missing explicit dyn to trait types to remove warnings
  OpenAPITools/openapi-generator#3895
- Add missing re-export for properties that are enum
  OpenAPITools/openapi-generator#3895
- Handle optional parameters
  OpenAPITools/openapi-generator#4016
- Derive more traits
  borsboom/openapi-generator@4e3bc31
borsboom added a commit to borsboom/ynab-api that referenced this pull request Oct 9, 2019
Built using
borsboom/openapi-generator@4e3bc31

This patch includes:
- Add all missing explicit dyn to trait types to remove warnings
  OpenAPITools/openapi-generator#3895
- Add missing re-export for properties that are enum
  OpenAPITools/openapi-generator#3895
- Handle optional parameters
  OpenAPITools/openapi-generator#4016
- Derive more traits
  borsboom/openapi-generator@4e3bc31
@wing328 wing328 added this to the 4.2.0 milestone Oct 14, 2019
@wing328 wing328 changed the title Add rust discriminator [Rust] Add support for discriminator Oct 30, 2019
@wing328
Copy link
Member

wing328 commented Oct 31, 2019

@gferon thanks for the PR, which has been included in v4.2.0 release: https://twitter.com/oas_generator/status/1189824932345069569

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

Successfully merging this pull request may close these issues.

3 participants