[Java][okhttp] Fix gson deserialize format byte#7473
[Java][okhttp] Fix gson deserialize format byte#7473wing328 merged 10 commits intoswagger-api:masterfrom
Conversation
…n) out of base64 string. Add disableHtmlEscaping() to gsonbuilder.
…t of byte array serialization and deserialization.
| import java.time.LocalDate; | ||
| import java.time.OffsetDateTime; | ||
| import java.time.format.DateTimeFormatter; | ||
| import java.util.Base64; |
There was a problem hiding this comment.
Okhttp has a Base64 class that can be used instead of the Java8 one. This way android users can enjoy the fix.
| "sold" | ||
| ] | ||
| }, | ||
| "profilePhoto": { |
There was a problem hiding this comment.
This will impact all generators. So all examples shall be regenerated
There was a problem hiding this comment.
We want to keep petstore.json, petstore.yaml the same as the Petstore server (https://petstore.swagger.io) so please undo the change in these files.
I noticed that you already added the additional test case in petstore-with-fake-endpoints-models-for-testing.yaml (there's an similar test case here: https://github.com/benrobot/swagger-codegen/blob/53018c475ad6bbc097f29d47b11a2bc55b5142ea/modules/swagger-codegen/src/test/resources/2_0/petstore-with-fake-endpoints-models-for-testing.yaml#L1202)
There was a problem hiding this comment.
Thank you. I'll undo the addition of Pet.profilePhoto and use the existing format_test.byte.
There was a problem hiding this comment.
Just to be clear. I am not regenerating all the clients because I've undone the changes instead.
| private LocalDateTypeAdapter localDateTypeAdapter = new LocalDateTypeAdapter(); | ||
| {{/jsr310}} | ||
| {{#java8}} | ||
| private LocalByteArrayAdapter localByteArrayAdapter = new LocalByteArrayAdapter(); |
There was a problem hiding this comment.
Why not just ByteArrayAdapter ?
There was a problem hiding this comment.
Agreed. I thought I was following the naming convention of the other type adapters but only the DateAdapter was prepended with Local.
| .registerTypeAdapter(LocalDate.class, localDateTypeAdapter) | ||
| {{/jsr310}} | ||
| {{#java8}} | ||
| .disableHtmlEscaping(){{! Added to prevent base64 = (equal sign) from being escaped }} |
There was a problem hiding this comment.
The gson default is to escape html (it is the contrary for Jackson) so this should be a choice left to the SDK user.
There was a problem hiding this comment.
Removed disableHtmlEscaping. Changed test to compensate.
| */ | ||
| @Test | ||
| public void testApiClient() { | ||
| // the default api client is used |
There was a problem hiding this comment.
Something is wrong. The tests shouldn’t be deleted
There was a problem hiding this comment.
I will put these back.
There was a problem hiding this comment.
The deleted tests have been undeleted
…for base64 conversions instead of java 8 lib since the okhttp-gson client already includes okio. Remove setting to disableHtmlEscaping. Rename LocalByteArrayAdapter to ByteArrayAdapter.
…eady available under the format_test definition.
…erify exception since the server now returns a 500 for this request.
|
I see that CircleCI tests against a local instance of http://petstore.swagger.io. I will spin up my own instance so my tests run against that instead of the one published on the internet. |
…gerapi/petstore (heads up, this test fails if calling out to the internet published version of http://petstore.swagger.io)
|
@benrobot thanks for the PR, which has been merged into master. |
wing328
changed the title
|
@wing328 we're using the 3.0.0 branch but I'd like to have this fix. Is there a good way to do that? Can I propose a cherry-pick of this PR back into the Thanks! |
|
@HugoMario ping |
|
@wing328 Looks like this fix is still not in 3.0.11? |
7 participants
The java client based off of okhttp-gson libraries failed to serialized and deserialize
type=string,format=byteThis PR aims to add byte[] serialization/deserialization functionality by adding a new Gson type adapter which employs the Base64 class available starting with Java 8
Unit testing showed that the default gson setup was also escaping the = (equal signs) commonly found at the end of a base64 string therefore, the disableHtmlEscaping option has been set via the gson builder
Fix #4824
cc: @bbdouglas, @JFCote, @sreeshas, @jfiala, @lukoyanov, @cbornet, @jeff9finger