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

Avoid generating uncompilable response body in Spring's API template #8691

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

smasset
Copy link

@smasset smasset commented Sep 10, 2018

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: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

If definitions start to get complicated, example response bodies can exceed Java compiler's limit for constant strings.

This PR addresses this issue by introducing and using two new lambdas to remove any unnecessary whitespace and (if still needed) to split the constant string into smaller compilable parts using a StringBuilder to merge them back again.

Fixes #9055

@smasset smasset force-pushed the long-example-string branch from e5bd6a1 to e202250 Compare April 17, 2019 04:40
@smasset
Copy link
Author

smasset commented Apr 17, 2019

Rebased against current master. Should fix #9055

Not sure why continuous-integration/appveyor/branch check is not running automatically

@smasset smasset force-pushed the long-example-string branch from e202250 to ceb93f9 Compare April 20, 2019 03:23
@smasset
Copy link
Author

smasset commented Apr 20, 2019

Rebased against current master

@smasset
Copy link
Author

smasset commented Apr 20, 2019

I can confirm this fixes #9055.

With provided example, only removing duplicate white space in generated example string made the class compilable.

Duplicating the billingAccounts property in CustomerAccount definition to a total of 10 instances illustrates the use of StringBuilder.

@smasset
Copy link
Author

smasset commented Apr 20, 2019

Attaching ZIP archive with API interfaces generated by version 2.3.1 of swagger-codegen and by a snapshot with my proposed fix (with vanilla sample provided in #9055 and with extra array properties to demonstrate the use of StringBuilder)

CustomerAccountsApi.zip

@smasset smasset force-pushed the long-example-string branch from ceb93f9 to 3a262c3 Compare April 29, 2019 21:02
@smasset
Copy link
Author

smasset commented Apr 29, 2019

@frantuma, rebased this against latest master, is there anything missing in this PR ?

@drej1, can you confirm this fixes #9055 ?

@drej1
Copy link

drej1 commented Jan 15, 2021

Hi @smasset , I appologize, I missed the notification. We checked your solution out and it's working... is it possible to merge it into the upcoming version?
Thanks

@smasset smasset force-pushed the long-example-string branch from 3a262c3 to b32c46a Compare January 18, 2021 08:54
@smasset
Copy link
Author

smasset commented Jan 18, 2021

Hey @drej1. I've just rebased against latest master and didn't get any conflict.
Let's hope all checks pass and if so feel free to merge it for the next release.

@smasset
Copy link
Author

smasset commented Jan 18, 2021

All checks passed. @frantuma can you include this PR in the next release ?

@drej1
Copy link

drej1 commented Jan 18, 2021

@smasset thank you very much!
@frantuma any idea when this will be released? thanks :)

@drej1
Copy link

drej1 commented Feb 18, 2021

Hi @diyfr, can you or somebody from the team check this PR and include it into the next release of 2.x (and also 3.x)?
Thanks

@diyfr
Copy link
Contributor

diyfr commented Feb 18, 2021

Hi @diyfr, can you or somebody from the team check this PR and include it into the next release of 2.x (and also 3.x)?
Thanks
-> @cbornet @wing328

@drej1
Copy link

drej1 commented Jul 16, 2021

@cbornet @wing328 any updates on this???

@drej1
Copy link

drej1 commented Feb 2, 2022

@cbornet @wing328 @diyfr or anybody... this is a pull request from 4 years ago...

@Xerocry
Copy link

Xerocry commented Feb 16, 2023

@cbornet @wing328 @diyfr
Will this ever be available? Been 5 years already..

@Xerocry
Copy link

Xerocry commented Apr 28, 2023

@cbornet @wing328 @diyfr @frantuma @smasset
Guys, at least give some comment, please..

@diyfr
Copy link
Contributor

diyfr commented Apr 28, 2023

@Xerocry This pull request 2903 doesn't solved this issue ?
Included in V4.2.2

Difference between Swagger Codegen and OpenAPI Generator

@Xerocry
Copy link

Xerocry commented Apr 28, 2023

@diyfr Maybe I misunderstood something but that is a fix for OpenApi generator, not swagger codegen, no?
Sadly I can't switch cause of complex structure openapi plugin is running out of space and it would be great to have fix in swagger-codegen too :)

@diyfr
Copy link
Contributor

diyfr commented Apr 28, 2023

@diyfr Maybe I misunderstood something but that is a fix for OpenApi generator, not swagger codegen, no? Sadly I can't switch cause of complex structure openapi plugin is running out of space and it would be great to have fix in swagger-codegen too :)

Check, it's present on master branch

@diyfr
Copy link
Contributor

diyfr commented Apr 28, 2023

@wing328 @frantuma you can close this pull request

@Xerocry
Copy link

Xerocry commented Apr 28, 2023

Check, it's present on master branch

Correct me if I'm wrong but there are only lambdaEscapeDoubleQuote and lambdaRemoveLineBreak in master, no lambdaTrimWhitespace*lambdaSplitString*

@diyfr
Copy link
Contributor

diyfr commented Apr 28, 2023

@Xerocry I suggest you propose a new PR. this one is based on an obsolete branch

@smasset
Copy link
Author

smasset commented May 2, 2023

@diyfr I've just created a new PR #12136 cherry-picking all commits from this one without any conflict.
Can you approve the workflows there ? I can also rebase this PR's branch against latest master if you prefer.
Let me know what is more convenient for you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[JAVA SPRING] error: constant string too long
4 participants