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

[core] Fix NPE for endpoints without responses #1617

Merged
merged 1 commit into from
Dec 7, 2018

Conversation

tomekc
Copy link
Contributor

@tomekc tomekc commented Dec 5, 2018

When generating Java client, generator was crashing with cryptic NPE. After debugging, it turned out, that one of the enpoints did not have 'responses' field defined,
and there was no clue, what operation it was. The code looked like was supposed to deal with that situation, but by simple mistake it moved on and caused NPE.

Additional logging helped isolate the problem.

BTW: IntelliJ IDEA IDE suggested that change, so it is easy to spot it using static code analysis.

(details of the change, additional tests that have been done, reference to the issue for tracking, etc)

@jmini jmini changed the title - [ ] Read the [contribution guidelines](https://github.com/openapitools/openapi-generator/blob/master/CONTRIBUTING.md). - [ ] 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](https://github.com/OpenAPITools/openapi-generator/wiki/Git-Branches): master~~, 3.4.x, 4.0.x~~. Default: master. - [ ] Copied the [technical committee](https://github.com/openapitools/openapi-generator/#62---openapi-generator-technical-committee) to review the pull request if your PR is targeting a particular programming language. [core] Fix NPE for endpoints without responses Dec 5, 2018
@jmini
Copy link
Member

jmini commented Dec 5, 2018

Even if not having responses defined is not a valid OpenAPI spec (it will be reported as such by editors like https://editor.swagger.io/) I think that it make sense to support this.

cc @OpenAPITools/generator-core-team

@tomekc
Copy link
Contributor Author

tomekc commented Dec 5, 2018

Um, sorry for the long title: I used command line tool (hub) to generate PR.

By the way, the validator could issue a warning in such case?

@jmini
Copy link
Member

jmini commented Dec 5, 2018

By the way, the validator could issue a warning in such case?

Yes. Good idea!
I am not sure if we have the notion of warnings yet (to be verify), but it could also be use-full for this #997

@jmini jmini merged commit 922532d into OpenAPITools:master Dec 7, 2018
@jmini jmini added this to the 4.0.0 milestone Dec 7, 2018
@jmini
Copy link
Member

jmini commented Dec 7, 2018

@tomekc I have merged your change. Thank you a lot!

For your idea with warnings, can you open a separated issue for that?

@wing328
Copy link
Member

wing328 commented Jan 2, 2019

@tomekc thanks for the contribution, which has been included in the 4.0.0-beta release: https://twitter.com/oas_generator/status/1079727020374806529.

Happy New Year and looking forward to more collaboration and contributions in 2019!

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 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.

4 participants