Skip to content

Move gzip() from RequestBody to Request.Builder#8838

Merged
squarejesse merged 1 commit into
masterfrom
jwilson.0603.gzip
Jun 4, 2025
Merged

Move gzip() from RequestBody to Request.Builder#8838
squarejesse merged 1 commit into
masterfrom
jwilson.0603.gzip

Conversation

@squarejesse

Copy link
Copy Markdown
Contributor

On Request.Builder the one function can add the header and apply compression. Otherwise the caller needs to keep those calls in sync.

One severe drawback of this approach is the calls to Request.Builder are now ordered. If you call gzip() before you call post(), it'll crash. Worse, if you call post() and then gzip() and then post() again
with a different body, it'll silently not compress the new body. I don't love this drawback but I think the mitigations aren't worth the effort.

On Request.Builder the one function can add the header
and apply compression. Otherwise the caller needs to
keep those calls in sync.

One severe drawback of this approach is the calls to
Request.Builder are now ordered. If you call gzip()
before you call post(), it'll crash. Worse, if you
call post() and then gzip() and then post() again
with a different body, it'll silently not compress
the new body. I don't love this drawback but I think
the mitigations aren't worth the effort.
* ```
*/
@JvmStatic
@ExperimentalOkHttpApi

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dealing with this annotation is what brought me here

@JakeWharton

Copy link
Copy Markdown
Collaborator

Should it be on Request maybe? That way you have to do something to toBuilder() to mess it up?

@squarejesse

Copy link
Copy Markdown
Contributor Author

Yeah, Request.gzip() is tempting. But I would like that as an additional API, not as a replacement for this...

I think it’s analogous to MutableList and List. We’ve got MutableList.sort() which is in-place, and List.sorted() which returns a copy.

We could live without MutableList.sort(), but it would be less efficient.

@squarejesse
squarejesse merged commit 71a759f into master Jun 4, 2025
22 checks passed
@squarejesse
squarejesse deleted the jwilson.0603.gzip branch June 4, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants