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

Update swagger-parser-version to 2.0.13-OpenAPITools.org-1 #2775

Conversation

jmini
Copy link
Member

@jmini jmini commented May 1, 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\.
  • 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

Update the Swagger-Parser version to 2.0.13-OpenAPITools.org-1

@jmini jmini added Swagger-Parser WIP Work in Progress labels May 1, 2019
pom.xml Outdated
@@ -262,6 +262,7 @@
</requireMavenVersion>
<requireReleaseDeps>
<message>No Snapshots Allowed!</message>
<onlyWhenRelease>true</onlyWhenRelease>
Copy link
Member Author

Choose a reason for hiding this comment

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

This line must be removed when we switch to a release version!

@jmini
Copy link
Member Author

jmini commented May 1, 2019

WIP:
For the moment trying a 2.0.13-SNAPSHOT version.
If everything goes well I will publish a 2.0.13-OpenAPITools.org-1 release.

@jmini
Copy link
Member Author

jmini commented May 2, 2019

I have fixed 2 tests in InlineModelResolverTest in e4b5ece

The Parser behave differently:

  • arbitraryObjectResponseWithAdditionalProperty:
    • mediaType.getSchema() returns a MapSchema instead of a ObjectSchema => this is correct.
  • resolveInlineObjectResponseWithAdditionalProperties:
    • mediaType.getSchema().getAdditionalProperties() returns a Schema with a $ref instead of the ObjectSchema
    • ModelUtils.getReferencedSchema can be used.
    • The referenced schema is a Schema and not an ObjectSchema

If our CI Tests do not detect more regression, I propose to continue with our release of Swagger-Parser.

@jmini
Copy link
Member Author

jmini commented May 2, 2019

The samples were no longer up-to-date. I have pushed 3c18263 to fix this

We see some changes with Integer that becomes Object in enums.

Example

--- a/samples/server/petstore/jaxrs-jersey/src/gen/java/org/openapitools/model/EnumTest.java
+++ b/samples/server/petstore/jaxrs-jersey/src/gen/java/org/openapitools/model/EnumTest.java
@@ -107,13 +107,13 @@ public class EnumTest   {
    * Gets or Sets enumInteger
    */
   public enum EnumIntegerEnum {
-    NUMBER_1(1),
+    _1("1"),
     
-    NUMBER_MINUS_1(-1);
+    _1("-1");
 
-    private Integer value;
+    private Object value;
 
-    EnumIntegerEnum(Integer value) {
+    EnumIntegerEnum(Object value) {
       this.value = value;
     }
 
@@ -124,7 +124,7 @@ public class EnumTest   {
     }
 
     @JsonCreator
-    public static EnumIntegerEnum fromValue(Integer value) {
+    public static EnumIntegerEnum fromValue(Object value) {
       for (EnumIntegerEnum b : EnumIntegerEnum.values()) {
         if (b.value.equals(value)) {
           return b;

We need to investigate, because I think that this is not OK.
(the required fix might be in Swagger-Parser or in OpenAPI Generator)

@jmini
Copy link
Member Author

jmini commented May 3, 2019

I found the reason for the regression we observe:
swagger-api/swagger-parser#1090

This needs to be fixed before we can update Swagger-Parser

cc: @cvgaviao

@jmini jmini mentioned this pull request May 3, 2019
4 tasks
@jmini jmini removed the WIP Work in Progress label May 6, 2019
@jmini
Copy link
Member Author

jmini commented May 6, 2019

Now that 2.0.13-OpenAPITools.org-1 is released to Maven Central, this PR can use this version and is no longer WIP.

Copy link
Member

@wing328 wing328 left a comment

Choose a reason for hiding this comment

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

LGTM

@jmini jmini merged commit 16f94e7 into OpenAPITools:master May 6, 2019
@wing328 wing328 added this to the 4.0.0 milestone May 9, 2019
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.

2 participants