fix: Use UTF-8 as default charset for HttpJson requests #1477
Conversation
gcf-owl-bot
bot
removed
the
owlbot:run
|
Thanks Lawrence! This is a perfect example that showcase can help us identify bugs in our implementation, cc @burkedavison @mpeddada1 |
| @@ -172,7 +172,7 @@ HttpRequest createHttpRequest() throws IOException { | |||
| jsonFactory.createJsonParser(requestBody).parse(tokenRequest); | |||
| jsonHttpContent = | |||
| new JsonHttpContent(jsonFactory, tokenRequest) | |||
| .setMediaType((new HttpMediaType("application/json"))); | |||
| .setMediaType((new HttpMediaType("application/json; charset=utf-8"))); | |||
There was a problem hiding this comment.
The fix looks good to me! This makes me think though, we are currently hardcoding all content type to application/json, which is fine for now, but it will not work for services like storage in the future. It might be one of the reasons that storage has been a handwritten library. CC @meltsufin
There was a problem hiding this comment.
For my curiosity: What content-type does Storage use? Wondering if it's as easy as setting the charset or having to create/ modify the HttpMediaType class.
There was a problem hiding this comment.
It depends on the use case, in their example, it is set to text/plain. We probably need to make it a parameter in the future and set it differently based on the scenario.
There was a problem hiding this comment.
Sure, I'll create an issue to track this. Might be something that a future GAPIC library needs. Also, don't the handwritten libraries use java-core instead of gax?
There was a problem hiding this comment.
They use both, also we are thinking about reducing the need of handwritten libraries(including java-core), so this could be one of places. I wouldn't create an issue yet as we don't have the need yet, but something to keep in mind.
gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpRequestRunnableTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
…ase and gax modules (#1430) * chore: add aggregate test coverage collection for showcase and gax
* feat: install clirr check * install an independent action * format changes * change name * change action name
product-auto-label
bot
added
size: l
|
[gapic-generator-java-root] Kudos, SonarCloud Quality Gate passed! |
lqiu96
added
the
automerge
gcf-merge-on-green
bot
removed
the
automerge
) Thank you for opening a Pull Request! For general contributing guidelines, please refer to [contributing guide](https://togithub.com/googleapis/gapic-generator-java/blob/main/CONTRIBUTING.md) Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [x] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/gapic-generator-java/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) Fixes googleapis#1437
) Thank you for opening a Pull Request! For general contributing guidelines, please refer to [contributing guide](https://togithub.com/googleapis/gapic-generator-java/blob/main/CONTRIBUTING.md) Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [x] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/gapic-generator-java/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) Fixes googleapis#1437
Thank you for opening a Pull Request! For general contributing guidelines, please refer to contributing guide
Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #1437