Add a Jackson 3 converter

[sschuberth]

---

• CHANGELOG.md's "Unreleased" section has been updated, if applicable.

[JakeWharton]

Thanks! This mostly looks good from a quick check.

I want to familiarize myself with this major release, but otherwise I hope to get this merged this month and released by EOY.

[sschuberth]

I ran ./gradlew spotlessApply to address the linter issues and squashed the formatting fixes into the last commit.

[sschuberth]

Mind approving the workflow once more to verify that tests now pass, @JakeWharton?

[sschuberth]

Mind approving the workflow once more to verify that tests now pass, @JakeWharton?

I've rebased this to catch-up with the new required final-status check, but now running checks needs an approval again.

[sschuberth]

I hope to get this merged this month and released by EOY.

Is there still a chance to get this merged before the end of the year?

[sschuberth]

I hope to get this merged this month and released by EOY.

Is there still a chance to get this merged before the end of the year?

Happy new year ping 😉

[JakeWharton]

I have quit this company a few months ago so any work I do here is entirely volunteer work. I appreciate the contribution, but it'll still probably be a few weeks until I look at this project again.

[patschl]

Hi! I'm also quite interested in this PR being merged. Is @JakeWharton the only maintainer of this project?

[JakeWharton]

The design of Retrofit is such that you do not need this to be merged to be unblocked. You can have your own Jackson 3 converter living in your codebase for the time being.

And yes, it's basically just me.

[danrebati-fujitsu]

Looks good to me, though I am not a maintainer of this project. I'm just a passer-by 🙂

[sschuberth]

Looks good to me, though I am not a maintainer of this project. I'm just a passer-by 🙂

Thanks anyway for the review, you had some good findings there!

[joanvr]

when is this planned to get merged and released?

[jloisel]

Any update on this?

[pikmin86]

sorry to bother you @JakeWharton it seems (but i can be wrong) it only miss your approval to continue

[sschuberth]

sorry to bother you @JakeWharton it seems (but i can be wrong) it only miss your approval to continue

Maybe this will move forward in the context of https://jakewharton.com/the-lysine-contingency/ 🙏🏻

[MukjepScarlet]

In my opinion we can use a Buffer here?

[sschuberth]

You mean like

ByteBuffer bytes = ByteBuffer.wrap(adapter.writeValueAsBytes(value));
return RequestBody.create(bytes.array(), mediaType);

Why, what's the advantage?

[MukjepScarlet]

I mean okio.Buffer which reuses internal byte[]

[sschuberth]

Well, I could do

ByteString bytes = ByteString.of(adapter.writeValueAsBytes(value));
return RequestBody.create(bytes, mediaType);

but I still don't see the benefit, esp. as I believe ByteString.of() copies the data underneath.

If you have a different proposal, please share the exact code.

[MukjepScarlet]

I don't have a dev env of this repo so I can only provide following pseudo code.

var buffer = new okio.Buffer();
adapter.writeValue(buffer.outputStream(), value);
return RequestBody.create....

Like this: https://github.com/square/retrofit/blob/8e83a242eac1afc22a13c52098eddb27421353ff/retrofit-converters/gson/src/main/java/retrofit2/converter/gson/GsonRequestBodyConverter.java#L51-L53

But don't call readByteString at the end of it.

Use this (copied from my code):

private fun Buffer.asRequestBody(mediaType: MediaType) = object : RequestBody() {
    override fun contentType() = mediaType
    override fun contentLength(): Long = size
    override fun writeTo(sink: BufferedSink) {
        sink.writeAll(this@asRequestBody.copy()) // <- This is a shallow copy
    }
}

Well, if you want it to be simple you can ignore my comment.

[JakeWharton]

No features will be merged until the repo is moved. Yes it's taking forever to move.

[SinhaAmit]

Hi @JakeWharton, any timelines about this?

[JakeWharton]

No