-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
[rust] basic oneOf support #13970
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] basic oneOf support #13970
Conversation
|
@nshaaban-cPacket thanks for the PR. Can you please PM me via Slack when you've time? https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g |
|
kk after talking to @wing328 :
|
|
@nshaaban-cPacket is this PR ready for another review? |
yup! May need to re-run CI, error appears unrelated to code changes |
|
right, the circleci node1 failure can be ignore. |
|
Is there anything still blocking this being merged? Excited to have this in :) |
Just waiting on review from some of the rust technical community. Feel free to give a try yourself in the meantime! |
|
@nshaaban-cPacket thanks for the PR. Is it correct to say that the oneOf implementation uses Rust's enum? |
Correct, this PR uses an enum for oneOf. Current version doesn't use a struct (similar to a class in other languages) with optional fields |
|
@nshaaban-cPacket can it handle the following? |
temp.yaml
Outdated
| @@ -0,0 +1,34 @@ | |||
| openapi: 3.0.1 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to commit this file?
TL;DR; no due to an existing limitation on master, however this PR does do it's part in switching the type of Color to an enum. Given a config of And a openapi of openapi: 3.0.0
servers:
- url: 'http://localhost'
info:
description: >-
This is a simple sample for oneOf support
version: 1.0.0
title: OpenAPI Color
license:
name: Apache-2.0
url: 'https://www.apache.org/licenses/LICENSE-2.0.html'
paths:
/color:
get:
summary: Get the current color
description: ''
operationId: getColor
responses:
'200':
description: successful operation
content:
application/json:
schema:
$ref: '#/components/schemas/Color'
components:
schemas:
RgbColor:
description: RGB three element array with values 0-255.
type: array
items:
type: integer
minimum: 0
maximum: 255
minItems: 3
maxItems: 3
RgbaColor:
description: RGBA four element array with values 0-255.
type: array
items:
type: integer
minimum: 0
maximum: 255
minItems: 4
maxItems: 4
HexColor:
description: 'Hex color string, such as #00FF00.'
type: string
pattern: ^#(?:[0-9a-fA-F]{3}){1,2}$
minLength: 7
maxLength: 7
Color:
description: RGB array, RGBA array, or hex string.
oneOf:
- $ref: '#/components/schemas/RgbColor'
- $ref: '#/components/schemas/RgbaColor'
- $ref: '#/components/schemas/HexColor'It correctly generates the Color model as the following #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum Color {
/// RGB array, RGBA array, or hex string.
RgbColor(Box<RgbColor>),
/// RGB array, RGBA array, or hex string.
RgbaColor(Box<RgbaColor>),
/// RGB array, RGBA array, or hex string.
String(Box<String>),
}However the current state of the rust generator (as it exists both on master and this PR) fails to generate the correct model for the aliases. This issue exists on master as well, and is out of scope of the PR to fix. A separate PR could be created to fix this, and in combination with this PR would result in complete support for the given spec. #[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize)]
pub struct RgbColor {
} |
|
@ctrlaltf24 can you please resolve the merge conflicts when you've time? there's been change to the rust client generator so not sure if it has addressed the existing limitations you mentioned above. |
|
@ctrlaltf24 I made a PR with rebased master to your fork, hope it can help ctrlaltf24#1 |
c9fecdd to
d0af4ad
Compare
d0af4ad to
c9fecdd
Compare
|
@wing328 - @ctrlaltf24 merged rebase onto master branch in his fork |
c9fecdd to
6a90a83
Compare
Suport oneOf as a rust struct enum. Details: Enum without a discriminator is untagged being "untagged" simply means serde won't attempt to store the name of the enum inside the serialized object. See https://serde.rs/enum-representations.html#untagged for more Also check to make sure the mapping values are not an empty object (aka null). Co-authored-by: Nikita Puzankov <[email protected]>
6a90a83 to
f65ed95
Compare
No longer needed as of reqwest 0.10, it now takes the response as owned instead of mut ref. Is not empty is more clear
Will show as a struct enum when there are additional fields, otherwise will be a tuple enum. not sure the purpose of x-mapped-models, perhaps legacy code? mappedModels appears to do the same thing. Also add default implementation for quality of life
|
This will be super helpful! Thanks. @wing328 |
|
I also did a test to manually trigger a build failure and the workflow caught the issue as expected: https://github.com/OpenAPITools/openapi-generator/actions/runs/7885693679/job/21517363148?pr=17854 let's give this PR a try. thanks again for the contribution. |
* [rust] basic oneOf support Suport oneOf as a rust struct enum. Details: Enum without a discriminator is untagged being "untagged" simply means serde won't attempt to store the name of the enum inside the serialized object. See https://serde.rs/enum-representations.html#untagged for more Also check to make sure the mapping values are not an empty object (aka null). Co-authored-by: Nikita Puzankov <[email protected]> * refactor: fix clippy lints No longer needed as of reqwest 0.10, it now takes the response as owned instead of mut ref. Is not empty is more clear * fix: discriminator and oneof case Will show as a struct enum when there are additional fields, otherwise will be a tuple enum. not sure the purpose of x-mapped-models, perhaps legacy code? mappedModels appears to do the same thing. Also add default implementation for quality of life * chore: update samples --------- Co-authored-by: Nikita Puzankov <[email protected]>
Generates oneOf enums.
Expected behavior: Able to fill in struct fields in such a way that creates a valid request
Actual behavior (on master): Impossible to fill the struct Foo to make it a valid schema (as BOTH required fields (field_one and field_three) are required to be filled in rust, yet the openapi schema only allows one or the other to be set.
Actual behavior (on this PR): able to fill fields by using the corresponding variant on the enum Foo.
Known Limitations
Related issue
#9497
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.For Windows users, please run the script in Git BASH.
master(6.3.0) (minor release - breaking changes with fallbacks),7.0.x(breaking changes without fallbacks)@frol @farcaller @richardwhiuk @paladinzh @jacob-pro