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

Fix NPEs in Java generator #154

Merged
merged 1 commit into from
May 26, 2018
Merged

Conversation

delenius
Copy link
Contributor

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: Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

After switching from swagger-codegen to openapi-generator, I got NPEs from a couple of places in DefaultCodegen.java. This PR adds defensive programming null checks in two places, which fixes the problem for me.

My spec is very large, and it would be time-consuming to isolate the exact problem; perhaps the experts around here can figure it out from looking at the code.

@jmini
Copy link
Member

jmini commented May 26, 2018

About codegenParameter.datatypeWithEnum:

The line 4100 in DefaultCodegen introduced with commit 13f084e) is causing some NullPointerException when enumName is null.

When we have introduced this line, we were aware that this will be reworked when we try to share code between CodegenProperty and CodegenParameter. (See issue #20).

I did not investigate it in details, but I agree with your proposition to guard this line with a null-check. This will still help cases where enumName is not null and for cases where this is null, we will fall back to the state as it was before 13f084e.

@jmini
Copy link
Member

jmini commented May 26, 2018

For the other change allDefinitions == null:

In DefaultCodegen.fromModel(String, Schema, Map<String, Schema>) the allDefinitions parameter can be null.
There is also an on going on effort (see #83) to rework this method (instead of this allDefinition instance, the current openAPI instance should be provided).
The method DefaultCodegen.fromModel(String, Schema) (that creates the case where allDefinitions is null) should be removed (see #90)

@wing328 wing328 merged commit 7db0201 into OpenAPITools:master May 26, 2018
@wing328 wing328 added this to the 3.0.0 milestone May 26, 2018
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.

3 participants