Conversation
ZhongpinWang
left a comment
ZhongpinWang
left a comment
There was a problem hiding this comment.
Some clean code ideas. Bear my first Java PR review with some old JS habits. 😁
| "gzip".equals(headerParams.get("Content-Encoding")) | ||
| ? serializeGzip(body, contentTypeObj) | ||
| : serialize(body, formParams, contentTypeObj)); |
There was a problem hiding this comment.
[pp] Maybe this part can be extracted into a separate function and do "switch case" for different encodings? Just for better code structure since gzip is only one of the many possible values of content encoding.
| } | ||
| } | ||
|
|
||
| private HttpEntity serializeGzip( final Object body, final ContentType contentType ) |
There was a problem hiding this comment.
[pp] Some personal preference (we use pp in js team) 🙂 It could be better if this method can be merged with the already existing serialize() method by passing an additional param contentEncoding since it is the responsibility of the serialize() method to consider if data needs to be encoded / compressed.
Then maybe refactor the serialize() method as it now has 55 lines with a big if-else.
| } | ||
| } | ||
|
|
||
| private HttpEntity serializeGzip( final Object body, final ContentType contentType ) |
There was a problem hiding this comment.
[pp/req] (req for request changes, but up to you to decide) Also the current serializeGzip() cannot handle form data?
What about using the current serialize() method first, after getting the HttpEntity, check the Content-Encoding and call HttpEntity::writeTo() to write the content to OutputStream and then compress? Makes code much cleaner IMO.
The ApiClient is straight from OpenAPI Generator. We shouldn't put any effort towards generated code or modified code from OpenAPI. |
I now get your point. But aren't you then editing a generated file? How would you make |
2 participants
Context
SAP/cloud-sdk-java-backlog#363.
Smaller payload.
Corresponding AI SDK PR:
Feature scope:
Content-Encodingis set togzip-> compress the request payloadDefinition of Done
Error handling created / updated & covered by the tests aboveDocumentation updatedRelease notes updated