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 parser to 2.0.29 #11388

Merged
merged 12 commits into from
Feb 21, 2022
Merged

Update parser to 2.0.29 #11388

merged 12 commits into from
Feb 21, 2022

Conversation

wing328
Copy link
Member

@wing328 wing328 commented Jan 24, 2022

Update parser to 2.0.29

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    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.
  • File the PR against the correct branch: master (5.3.0), 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@wing328
Copy link
Member Author

wing328 commented Jan 24, 2022

Updating the samples resulted in
[pool-1-thread-8] Finished generating python-fastapi… [pool-1-thread-6] Generation failed for python-experimental: (RuntimeException) Could not process model 'NullableShape'.Please make sure that your schema is correct! java.lang.RuntimeException: Could not process model 'NullableShape'.Please make sure that your schema is correct! at org.openapitools.codegen.DefaultGenerator.generateModels(DefaultGenerator.java:492) at org.openapitools.codegen.DefaultGenerator.generate(DefaultGenerator.java:888) at org.openapitools.codegen.cmd.GenerateBatch$GenerationRunner.run(GenerateBatch.java:232) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:829) Caused by: java.lang.NullPointerException at org.openapitools.codegen.utils.ModelUtils.getSimpleRef(ModelUtils.java:384) at org.openapitools.codegen.DefaultCodegen.getOneOfAnyOfDescendants(DefaultCodegen.java:3089) at org.openapitools.codegen.DefaultCodegen.createDiscriminator(DefaultCodegen.java:3233) at org.openapitools.codegen.DefaultCodegen.fromModel(DefaultCodegen.java:2695) at org.openapitools.codegen.languages.PythonExperimentalClientCodegen.fromModel(PythonExperimentalClientCodegen.java:1057) at org.openapitools.codegen.DefaultGenerator.processModels(DefaultGenerator.java:1265) at org.openapitools.codegen.DefaultGenerator.generateModels(DefaultGenerator.java:487) ... 5 more [pool-1-thread-2] Generating python-legacy (outputs to /Users/williamcheng/Code/openapi-generator2/samples/client/petstore/python-legacy)… ERROR: One or more generators failed to generate. Halting ensure-up-to-date scripts.

@wing328 wing328 marked this pull request as draft January 24, 2022 08:38
@wing328
Copy link
Member Author

wing328 commented Jan 24, 2022

@spacether I wonder if you can take a look at the NPE above when you've time? Thanks.

@wing328 wing328 mentioned this pull request Jan 24, 2022
5 tasks
@wing328 wing328 modified the milestones: 5.4.0, 6.0.0 Jan 31, 2022
@wing328
Copy link
Member Author

wing328 commented Feb 7, 2022

UPDATE: I've pushed e1bffbc to better handle the NPE

@wing328 wing328 marked this pull request as ready for review February 7, 2022 08:30
@@ -94,14 +95,14 @@ def _composed_schemas(cls):
# code would be run when this module is imported, and these composed
# classes don't exist yet because their module has not finished
# loading
oneOf_2 = NoneSchema
oneOf_2 = AnyTypeSchema
Copy link
Contributor

@spacether spacether Feb 8, 2022

Choose a reason for hiding this comment

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

@wing328
Something is not working right here. The schema is:

    NullableShape:
      description: The value may be a shape or the 'null' value.
        The 'nullable' attribute was introduced in OAS schema >= 3.0
        and has been deprecated in OAS schema >= 3.1.
        For a nullable composed schema to work, one of its chosen oneOf schemas must be type null
      oneOf:
        - $ref: '#/components/schemas/Triangle'
        - $ref: '#/components/schemas/Quadrilateral'
        - type: null
      discriminator:
        propertyName: shapeType
      nullable: true

So the 3rd oneOf schema should be NoneSchema NOT AnyTypeSchema.
AnyTypeSchema is {} or true

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, could be a regression. I tried updating the parser to 2.0.30 (released several hours ago) but no luck (no change in samples).

Can you please report the issue to swagger parser directly with the details when you've time?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, it could also be our code not seeing the null type right. If it is the parser I will report it.

Copy link
Contributor

@spacether spacether Feb 13, 2022

Choose a reason for hiding this comment

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

I did some investigating here:

  1. For yaml type: null the schema is defined as an type: null AnyType schema by swagger-parser
  2. For yaml type: "null" the schema is correctly defined as type: "null" NoneSchema by swagger-parser
    So I will:
  • Update the spec to make the type be the quoted null value
  • Remove the discriminator from that schema because per the openapi spec, when discriminator exists, then the discriminator key MUST exist in the payload. a null payload value cannot meet that constraint.
  • Remove nullable: true because it had no impact on this schema and the type: "null" in the oneOf already allows null as does having an untyped composed schema.

@wing328 wing328 marked this pull request as draft February 8, 2022 03:47
@wing328 wing328 marked this pull request as ready for review February 21, 2022 10:08
@wing328 wing328 merged commit df05e6f into master Feb 21, 2022
@wing328 wing328 deleted the update-parser branch February 21, 2022 10:37
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