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 multi-part binary array form parameters for Spring Boot and Spring Cloud templates #4616

Merged
merged 8 commits into from
Dec 9, 2019

Conversation

rbrincke
Copy link
Contributor

@rbrincke rbrincke commented Nov 26, 2019

Multi-part binary array strings as outlined in #3139 resulted in MultipartFile rather than List<MultipartFile>. This PR attempts to resolve this by adjusting the templates for Spring Boot and Spring Cloud. Changes to these templates include:

  • wrap contents in List where appropriate
  • use defined description rather than a hard-coded one
  • use defined RequestPart name rather than hard-coded one

Two tests have been added: one for spring-boot and one for spring-cloud.

Resolve #3139

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608

Multi-part binary array strings as outlined in OpenAPITools#3139 resulted in `MultipartFile` rather than `List<MultipartFile>`. This PR attempts to resolve this by adjusting the templates.

Two tests have been added: one for `spring-boot` and one for `spring-cloud`.

Resolve OpenAPITools#3139
@rbrincke rbrincke changed the title Fix multi-part binary array strings Fix multi-part binary array form parameters for Spring Boot and Spring Cloud Nov 27, 2019
@rbrincke
Copy link
Contributor Author

Please note that the runner logs a warning when using the defined .yml snippet ("Could not compute datatypeWithEnum from ...", org.openapitools.codegen.DefaultCodeGen#4684). From the history of commits it is unclear to me what is going on there, and the generated code seems to be fine. I am completely new to this code base, so it would be helpful if someone else could provide further explanation as to whether this is harmful, and if so, what needs to be fixed to make it go away.

@rbrincke rbrincke changed the title Fix multi-part binary array form parameters for Spring Boot and Spring Cloud Fix multi-part binary array form parameters for Spring Boot and Spring Cloud templates Nov 27, 2019
@sun9999
Copy link

sun9999 commented Nov 28, 2019

I think the related changes should also apply to the following file:
modules/openapi-generator/src/main/resources/JavaSpring/apiDelegate.mustache line 53.

@rbrincke
Copy link
Contributor Author

rbrincke commented Nov 29, 2019

I've run the code generator against the specification file locally to check, and you are right. I've fixed that one as well, and have added test coverage for it.

Because the description is now picked up, a lot of the samples have changed as a result of having run bin/spring-all-petstore. This makes the PR look much bigger than it actually is.

Copy link
Contributor

@lwlee2608 lwlee2608 left a comment

Choose a reason for hiding this comment

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

Looks good to me

@wing328 wing328 merged commit 42f685f into OpenAPITools:master Dec 9, 2019
@wing328
Copy link
Member

wing328 commented Dec 9, 2019

Merge conflicts resolved via 69cb1ce. All tests passed.

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.

[BUG] [Spring] Generated API interface treats multipart arrays as a single file
4 participants