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

[Java][RestTemplate] Use class level RestTemplate for uri encoding #11606

Merged
merged 2 commits into from
Feb 15, 2022

Conversation

Laurens-W
Copy link
Contributor

@Laurens-W Laurens-W commented Feb 14, 2022

Moved static logic to initialization method for when no RestTemplate is provided.
Otherwise, use the settings from the RestTemplate that was provided. (Wired by spring)

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master (5.3.0), 6.0.x
    Help me on this one: this needs to go into 5.3.2, 5.4.1 and 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@lwlee2608 @Zomzog

#11274
#10958

Westerlaken, H.L. (Laurens) added 2 commits February 14, 2022 13:42
…ovided.

Otherwise, use the settings from the RestTemplate that was provided.
Run required scripts
@Laurens-W Laurens-W changed the title [Java][RestTemplate] Use class level RestTemplate for url encoding [Java][RestTemplate] Use class level RestTemplate for uri encoding Feb 14, 2022
@wing328
Copy link
Member

wing328 commented Feb 15, 2022

@Laurens-W thanks for the PR. Is it correct to say that this change is transparent to the users of the API client? In other words, it's not a breaking change, right?

@Laurens-W
Copy link
Contributor Author

@Laurens-W thanks for the PR. Is it correct to say that this change is transparent to the users of the API client? In other words, it's not a breaking change, right?

In case they are providing their own RestTemplate they will now experience the same behaviour as they did before 5.3.1.
If they do not provide a RestTemplate one is created which now uses the default @berlincho introduced in #10958.

So it shouldn't be breaking :)

@wing328 wing328 merged commit 52e3265 into OpenAPITools:master Feb 15, 2022
@wing328
Copy link
Member

wing328 commented Feb 15, 2022

👌 thanks for the explanation. PR merged.

@drej1
Copy link

drej1 commented Mar 7, 2022

Will this be released in 5.4.1 and when? This is a major issue for us now...

Thanks

@wing328
Copy link
Member

wing328 commented Mar 7, 2022

It will be included in the v6.0.0 release.

You can use the snapshot version of v6.0.0 for the time being (or build the JAR directly from the latest master)

@drej1
Copy link

drej1 commented Mar 7, 2022

Hi @wing328 isn't it possible to create a hotfix version for 5.4.x? The encoding of path variables were fully broken in 5.3.1 and 5.4.0.
6.0.0 is a major release and I assume there will be breaking changes and the upgrade will be not trivial.

Thanks

PS: or maybe some advice how to create an effective workaround?

@wing328
Copy link
Member

wing328 commented Mar 7, 2022

6.0.0 is a major release and I assume there will be breaking changes and the upgrade will be not trivial.

My opinion can be biased - the upgrade should be straightforward and there are not many breaking changes in the v6.0.0 release.

Please give the snapshot version a try. You may like other improvements as well.

@drej1
Copy link

drej1 commented Mar 7, 2022

@wing328 ok understood and thanks. :)

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.

None yet

3 participants