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

Support for discriminator.mapping #536

Merged
merged 8 commits into from
Jul 18, 2018
Merged

Support for discriminator.mapping #536

merged 8 commits into from
Jul 18, 2018

Conversation

0v1se
Copy link
Contributor

@0v1se 0v1se commented Jul 11, 2018

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 and ./bin/security/{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\.
  • Filed the PR against the correct branch: master, 3.1.x, 4.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

See #417
Basic support for discriminator.mapping and support for jackson generator for polymorphic types with custom mapping.

@jmini
Copy link
Member

jmini commented Jul 11, 2018

Thank you a lot!


Have you the possibility to update the samples/ folder?
You need to:

  • run mvn verify locally
  • run the scripts (bin/*.sh) to update the java clients.
  • add the changes to this PR.

I will perform some tests locally, but on the first look, it looks very good to me.

@0v1se
Copy link
Contributor Author

0v1se commented Jul 12, 2018

@jmini, ok
found some bugs, will fix and update

@0v1se
Copy link
Contributor Author

0v1se commented Jul 12, 2018

@0v1se
Copy link
Contributor Author

0v1se commented Jul 16, 2018

@jmini did you check the code?

@jmini
Copy link
Member

jmini commented Jul 16, 2018

Yes I did, it looks good to me. I was wondering if the change can be added to the 3.1.x stream (master branch) or if we need to do it for the 3.2.x stream (3.2.x branch).

The replacement of io.swagger.v3.oas.models.media.Discriminator with DiscriminatorCodegen used in CodegenModel and in CodegenOperation is not an API break from a template point of view.

Technically speaking, if people are extending the Codegen classes (to support their own generator for example) then they might get compile error...
I am not sure if we consider this as a breaking change or not.

What do you think? Would 3.2.x branch be OK for you?

@0v1se
Copy link
Contributor Author

0v1se commented Jul 17, 2018

@jmini 3.2.x looks good to me

also, did you notice I changed CodegenDiscriminator?
I removed CodegenDiscriminator.MappedModel. It currently doesn't hold CodegenModel, but only its name.
I think, it's enough for code generation (may be I'm wrong). At least, it's enough for Java.

@jmini jmini changed the title Support for discriminator.mapping https://github.com/OpenAPITools/openapi-generator/issues/417 Support for discriminator.mapping Jul 18, 2018
@jmini jmini added this to the 3.2.0 milestone Jul 18, 2018
@jmini jmini changed the base branch from master to 3.2.x July 18, 2018 04:13
@jmini jmini merged commit 0a52f56 into OpenAPITools:3.2.x Jul 18, 2018
@jmini
Copy link
Member

jmini commented Jul 18, 2018

Follow up: I have created a small PR for the documentation of this feature: #587

@jmini
Copy link
Member

jmini commented Jul 18, 2018

I forgot to say: thank you a lot for this PR!

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
kevinoid added a commit to kevinoid/openapi-generator that referenced this pull request Jun 29, 2019
The `mapping` property of the [Discriminator Object] is "An object to
hold mappings between payload values and schema names or references."
The subsequent examples in the spec have `mapping`s of both types.
The `mapping` support introduced in OpenAPITools#536 only supports references.

Update the code to support names (identified by lack of `/`) or
references and change a test mapping to cover this case.

[Discriminator Object]: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#discriminatorObject

Signed-off-by: Kevin Locke <[email protected]>
wing328 pushed a commit that referenced this pull request Jul 4, 2019
The `mapping` property of the [Discriminator Object] is "An object to
hold mappings between payload values and schema names or references."
The subsequent examples in the spec have `mapping`s of both types.
The `mapping` support introduced in #536 only supports references.

Update the code to support names (identified by lack of `/`) or
references and change a test mapping to cover this case.

[Discriminator Object]: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#discriminatorObject

Signed-off-by: Kevin Locke <[email protected]>
nilskuhn pushed a commit to nilskuhn/openapi-generator that referenced this pull request Apr 6, 2023
…commitlint-monorepo

chore(deps): update commitlint monorepo (major)
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.

None yet

2 participants