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

[BUG] [jaxrs-jersey] [java] [jersey] Generated API interface treats multipart arrays as a single file #7330

Closed
5 of 6 tasks
itaru2622 opened this issue Sep 2, 2020 · 3 comments · Fixed by #7363
Closed
5 of 6 tasks

Comments

@itaru2622
Copy link
Contributor

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issuue still exists?
  • Have you search for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

Generated API interface for multipart arrays contains a single of InputStream and FormDataContentDisposition, instead of a list of those.

openapi-generator version

version 4.3.1 and official docker image openapitools/openapi-generator:cli-4.3.x pulled from https://hub.docker.com/r/openapitools/openapi-generator/

OpenAPI declaration file content or url

https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator/src/test/resources/3_0/form-multipart-binary-array.yaml

Generation Details
wget -O /tmp/form-multipart-binary-array.yaml https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator/src/test/resources/3_0/form-multipart-binary-array.yaml

docker run -it -v /tmp:/tmp openapitools/openapi-generator:cli-4.3.x generate -g jaxrs-jersey -i /tmp/form-multipart-binary-array.yaml -o /tmp/out

cat /tmp/out/src/main/java/org/openapitools/api/impl/MultipartArrayApiServiceImpl.java | grep multipartArray
Steps to reproduce

Generate MultipartApi interface as listed above.
Both multipartArray and multipartSingle methods contains a single of { InputStream, FormDataContentDisposition }

Related issues/PRs

Issue #3139 and PR #4616 (already merged).

Issue #3139 is occurred in spring and this is almost same issue occurred in jaxrs-jersey.

Suggest a fix

The following template files have no array, as described bellow snippets:

JavaJaxRS/formParams.mustache
JavaJaxRS/serviceFormParams.mustache

  @FormDataParam("{{baseName}}") InputStream {{paramName}}InputStream,
  @FormDataParam("{{baseName}}") FormDataContentDisposition {{paramName}}Detail

There are following three choices to fix, according to stackoverflow.com and other sites.
cf. https://stackoverflow.com/questions/56954122/multiple-files-upload-in-a-rest-service-using-jersey

Choice B and C could fix this issue but 'Choice A' may not.
Some may prefer 'Choice C' but other may prefer 'Choice B' !?

choice A) simplest but FAILED in communication test.

  • as described following snippets, just appending 'List' in multipartArray case.
  • when I tested this choice in real communication, results are:
    • multipartSingle case: passed.
    • multipartArray case: failed. ApiServiceImpl cannot get file content because instance of List<InputStream> is always null . It needs some further investigation to find out reason.
{{^isListContainer}}
   @FormDataParam("{{baseName}}") InputStream {{paramName}}InputStream,
   @FormDataParam("{{baseName}}") FormDataContentDisposition {{paramName}}Detail
{{/isListContainer}}
{{#isListContainer}}
   @FormDataParam("{{baseName}}") List<InputStream> {{paramName}}InputStream,
   @FormDataParam("{{baseName}}") List<FormDataContentDisposition> {{paramName}}Detail
{{/isListContainer}}

choice B) keeping backward compatibility, but ugly in coding.

  • Keep original template in multipartSingle case, and involve List<FormDataBodyPart> in multipartArray case as described in following snippets.
  • This choice enables users to keep using their ApiServiceImpl.java without any modification as far as multipartSingle case.
{{^isListContainer}}
   @FormDataParam("{{baseName}}") InputStream {{paramName}}InputStream,
   @FormDataParam("{{baseName}}") FormDataContentDisposition {{paramName}}Detail
{{/isListContainer}}
{{#isListContainer}}
   @FormDataParam("{{baseName}}") List<FormDataBodyPart> {{paramName}}Bodypart
{{/isListContainer}}
  • FormDataBodyPart is org.glassfish.jersey.media.multipart.FormDataBodyPart

choice C) breaking backward compatibility, but smarter.

  • Always using FormDataBodyPart, and appending List in multipartArray case.
  • Users may need little change in their ApiServiceImpl.java .
{{^isListContainer}}
   @FormDataParam("{{baseName}}") FormDataBodyPart {{paramName}}Bodypart
{{/isListContainer}}
{{#isListContainer}}
   @FormDataParam("{{baseName}}") List<FormDataBodyPart> {{paramName}}Bodypart
{{/isListContainer}}
@auto-labeler
Copy link

auto-labeler bot commented Sep 2, 2020

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@itaru2622
Copy link
Contributor Author

I prefer 'choice C' , for fixing this issue :-)

@itaru2622 itaru2622 changed the title [BUG] [jaxrs-jersey] Generated API interface treats multipart arrays as a single file [BUG] [jaxrs-jersey] [java] [jersey] Generated API interface treats multipart arrays as a single file Sep 2, 2020
@itaru2622
Copy link
Contributor Author

I tested with latest version (*) and found this issue is still exists.

*: docker image named openapitools/openapi-generator-cli:latest at https://hub.docker.com/r/openapitools/openapi-generator-cli/

itaru2622 added a commit to itaru2622/openapi-generator that referenced this issue Sep 7, 2020
… on generating jaxrs-jersey.

This commit aims to fix OpenAPITools#7330 with described solution 'choice C'.
itaru2622 added a commit to itaru2622/openapi-generator that referenced this issue Oct 14, 2020
… on generating jaxrs-jersey.

This commit aims to fix OpenAPITools#7330 with described solution 'choice C'.
wing328 pushed a commit that referenced this issue Oct 25, 2020
* Fix [jaxrs-jersey][java][jersey] multipart/form-data file array issue on generating jaxrs-jersey.

This commit aims to fix #7330 with described solution 'choice C'.

* add test code [jaxrs-jersey][java][jersey] for multipart/form-data file on generating

* update test code [jaxrs-jersey][java][jersey] for multipart/form-data file on generating

followed latest master branch of OpenAPITools/openapi-generator

* update samples/* by executing ./bin/generate-samples.sh

* update mustaches for JavaJaxxRS/libraries/jersey1/* and samples/* to resolve failure of CI test on PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant