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

Better OpenAPI spec v3 support: allOf, anyOf, oneOf #1360

Merged
merged 28 commits into from Dec 6, 2018

Conversation

@wing328
Member

wing328 commented Nov 1, 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.4.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

  • fix composition
  • fix inheritance support
  • add mustache tags allOf, anyOf, oneOf
  • add tests for discriminator, allOf, anyOf, etc in Rub client generator
  • update Ruby templates to support inheritance, anyOf, oneOf, etc

Related issues/PRs

cc @OpenAPITools/generator-core-team

wing328 added some commits Nov 2, 2018

@wing328 wing328 removed the WIP label Nov 2, 2018

@wing328 wing328 changed the title from [WIP] Better OpenAPI spec v3 support: allOf, anyOf, oneOf to Better OpenAPI spec v3 support: allOf, anyOf, oneOf Nov 2, 2018

@etherealjoy

This comment has been minimized.

Contributor

etherealjoy commented Nov 2, 2018

also this one #537?

@wing328

This comment has been minimized.

Member

wing328 commented Nov 2, 2018

@etherealjoy 👍 updated the list

@ybelenko

Fix {{{#vars}{{{/vars}}} codegen array, please.

@hellboy81

This comment has been minimized.

hellboy81 commented Nov 30, 2018

Waiting for anyOf, oneOf for Java as well.

@jvans Have you Gitter or Telegram?

@wing328 wing328 added this to the 4.0.0 milestone Dec 3, 2018

@jonschoning

This comment has been minimized.

Contributor

jonschoning commented Dec 3, 2018

regarding haskell-http-client generator

I've pushed an update to the branch which i think will fix it.

I was using {{#requiredVars}} in my template.
It appears there were two behavior changes regarding {{#requiredVars}}

  1. the vendorExtensions i had been previously using were not present
  2. the {{name}} property of the {{#requiredVars}} was different than the the {{name}} property of {{#vars}}{{#required}}

I have switched the code in this PR to avoid using {{#requiredVars}} and switched to only use {{#vars}}{{#required}}

// primitive type or model
names.add(getAlias(getPrimitiveType(s)));
}
return "oneOf<" + String.join(",", names) + ">";

This comment has been minimized.

@jonschoning

jonschoning Dec 3, 2018

Contributor

I'm guessing non-java langs will have to override this

This comment has been minimized.

@wing328

wing328 Dec 6, 2018

Member

I think one way is to override getSchemaType in the generator and then parse the string, e.g.

String result = super.getSchemaType(schema);
// parse the string here e.g. oneOf<Cat,Dog>

If there are demands to add a function to be overridden in the generator to customize the return value, we can definitely add that later.

@mm-matthias

This comment has been minimized.

mm-matthias commented Dec 5, 2018

FWIW, I tested with some simple specs using allOf using the python generator. It works really well so far. I ran into the limitation of "allOf"/ComposedSchemas not begin supported, but this is clearly logged.
I'm also writing my own codegen against the oas3-spec-support1 branch and things have worked out very nice as well.
Thanks for adding this sorely needed feature!

@wing328

This comment has been minimized.

Member

wing328 commented Dec 6, 2018

@mm-matthias thanks for reviewing the change. Can you ping me via https://gitter.im (ID: wing328) or Google hangout (email address in my Github profile page) for a quick chat?

@wing328 wing328 changed the title from [WIP] Better OpenAPI spec v3 support: allOf, anyOf, oneOf to Better OpenAPI spec v3 support: allOf, anyOf, oneOf Dec 6, 2018

@wing328

This comment has been minimized.

Member

wing328 commented Dec 6, 2018

Let's go with this PR to start with. If anyone encounters issues related to allOf/anyOf/oneOf, please open an issue with the details.

@wing328 wing328 merged commit 774013c into master Dec 6, 2018

5 checks passed

Shippable Run 4548 status is SUCCESS.
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hellboy81

This comment has been minimized.

hellboy81 commented Dec 6, 2018

Waiting for anyOf, oneOf for Java as well.

+1000

@wing328 wing328 deleted the oas3-spec-support1 branch Dec 10, 2018

@tomghyselinck

This comment has been minimized.

tomghyselinck commented Dec 10, 2018

I tried the latest build, but it looks like allOf is not supported for Python either yet.
We definitely need this too.

We get warnings like:

[main] INFO  o.o.codegen.DefaultCodegen - Composed schema not yet supported: class ComposedSchema {
...
}
@wing328

This comment has been minimized.

Member

wing328 commented Dec 10, 2018

@tomghyselinck do you mind opening an issue and share the spec so that we can reproduce the issue more easily?

@tomghyselinck

This comment has been minimized.

tomghyselinck commented Dec 10, 2018

@wing328 Thank you very much for your quick response!
I stripped down my pretty huge yaml file and file BUG report #1657

@marcelstoer

This comment has been minimized.

marcelstoer commented Dec 12, 2018

I would love to give this a try. Is there a binary somewhere that includes this fix? Specifically I want to find out if this addresses the v2 Java multi-level hierarchy bug (not sure it was even reported) where it would not generate the correct Jackson @JsonSubTypes for

C allOf B allOf A

on A. That's the only reason I'm currently still stuck on Swagger gen 2.4; would love to migrate.

@jmini

This comment has been minimized.

Member

jmini commented Dec 12, 2018

I would love to give this a try. Is there a binary somewhere that includes this fix?

We push SNAPSHOT releases on sonatype snapshot repository (sort of maven central for snapshots).
4.0.0-SNAPSHOT is available.

3 options:

  1. You can use gradle/maven to get them, indicate that you want to consume binaries for:
https://oss.sonatype.org/content/repositories/snapshots/

With gradle/maven you can download the CLI jar and then use it from the command line or directly use the gradle/maven plugin.

  1. If you want to download the all-in-one jar that we are producing manually, download the newest jar from this folder:
    https://oss.sonatype.org/content/repositories/snapshots/org/openapitools/openapi-generator-cli/4.0.0-SNAPSHOT/

  2. @jimschubert has proposed a script that do the download for you:
    https://gist.github.com/jimschubert/ce241b0c78140e364f46914ef8ec4103
    Set OPENAPI_GENERATOR_VERSION to 4.0.0-SNAPSHOT and it should work.


Specifically I want to find out if this addresses the v2 Java multi-level hierarchy bug (not sure it was even reported)

Is it this #827 ?

If not please create a separated issue with a minimal spec to reproduce your issue...

@wing328

This comment has been minimized.

Member

wing328 commented Dec 12, 2018

@marcelstoer we only added the mustache tags for oneOf, anyOf and allOf but have not yet updated the templates for all the languages/generators to take advantage of these new features in OAS v3.

For Java client, please open a new issue with a spec and the current vs expected output for tracking.

@wing328

This comment has been minimized.

Member

wing328 commented Dec 12, 2018

@YeTingGe are you free for a quick chat via https://gitter.im (ID: wing328) about your use case?

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