Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade OkHttp 3 to Kotlin and call it OkHttp 4 #4723

Open
swankjesse opened this issue Mar 14, 2019 · 31 comments

Comments

@swankjesse
Copy link
Member

commented Mar 14, 2019

Inspired by Okio 2 (blog post, presentation), OkHttp 4 is almost exactly like OkHttp 3, except the implementation language is Kotlin instead of Java. We punt breaking API changes to a hypothetical OkHttp 5 that remains in our icebox.

Goals

Implemented in Kotlin with a dependency on the Kotlin standard library. We want the option to support coroutines and multiplatform including Kotlin/Native .

Compatible with OkHttp 3.x. We want 100% binary compatibility so .jar files compiled against OkHttp 3.x will run against OkHttp 4.x without modification. We’ll need japicmp to enforce this. We also want 100% source compatibility for Java callers. Kotlin callers will not be source-compatible but we’ll offer automated upgrades via the language’s rich deprecation facilities. One surprising consequence of this approach is that the Maven coordinates and package name of OkHttp 4 will be okhttp3.

No performance regressions. We must write careful Kotlin, paying attention to abstractions that have a runtime cost.

Tasks

  • Migrate from Maven to Gradle.
  • Integrate japicmp.
  • Get a japicmp release with this fix and delete all the suppressed methods.
  • Migrate public API types
  • Adopt Kotlin idioms everywhere appropriate (examples)

FAQ

Why Kotlin? It’s a great language for both applications and libraries. It gives us options to execute on more platforms including iOS. Coroutines are powerful concurrency abstractions that may allow us to implement nonblocking I/O without hating ourselves.

Isn’t Kotlin just a trend? It’s backed by the best tools maker and the default language of the most popular mobile platform. It has a simple upgrade path from Java.

Java already has var and I heard Java 13 will have string literals. The Java language is moving faster than ever and adding many great features! But Java has deep layers of technical baggage (checked exceptions! null! JavaBeans™) and I don’t want it for my children. Also, Oracle wants to copyright APIs and I think that’s gross!

Did you just say iOS?! It should be possible to use OkHttp’s requests, responses, and interceptors with a NSURLSession backend. If that works it’d be a pretty awesome way to write networking code for mobile apps.

When will Retrofit/Moshi/Okio/Wire be migrated to Kotlin? Okio is already there. We’re doing OkHttp and Wire right now. No firm plans for Retrofit and Moshi.

Can I stay on OkHttp 3.x forever? We’re hoping that you won’t have to. It’s our job to make OkHttp 4 compelling enough that the cost of this upgrade is justified.

@swankjesse swankjesse changed the title Upgrade OkHttp 3 to Kotlin and Call it OkHttp 4 Upgrade OkHttp 3 to Kotlin and call it OkHttp 4 Mar 14, 2019

@swankjesse swankjesse added this to the 4.0 milestone Mar 15, 2019

@yschimke

This comment has been minimized.

Copy link
Collaborator

commented Mar 15, 2019

"We want the option to support coroutines" - I hope you will consider making this a fundamental feature of the design for 4.0. It feels like a big part of the win for users moving from 3 to 4. While the win without is for the sanity of OkHttp developers.

... without breaking compatibility :)

@swankjesse

This comment has been minimized.

Copy link
Member Author

commented Mar 15, 2019

Yeah, staying compatible is the trick. If everything were on the table we’d make our interceptors suspending. But that’s too disruptive for Java.

I’d be happy to add a Call extension that suspends:

suspend fun Call.execute2(): Response

What’s the right name for this?

@alosdev

This comment has been minimized.

Copy link

commented Mar 15, 2019

how about executeSuspended?

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

commented Mar 15, 2019

It's await in Retrofit. I wouldn't use any name with execute in it since it's actually using enqueue under the hood.

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

commented Mar 15, 2019

Maybe awaitResponse()

@yschimke

This comment has been minimized.

Copy link
Collaborator

commented Mar 17, 2019

What is the experience so far with kotlin upgrades? When I have had issues I think it's strictly been with RC versions etc, and they are not compatible. Has Okio gone through a kotlin upgrade?

@swankjesse

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2019

We’ve only seen Kotlin 1.2 to 1.3. It was low risk but we've avoided anything experimental.

@bsideup

This comment has been minimized.

Copy link

commented Mar 19, 2019

Too bad that one of the best http libraries for the JVM is no longer a good choice.
Sad to see that the world does not learn from previous mistakes (Java libraries in Groovy / Scala).

One of the best things about OkHttp/Okio was the fact that it was zero-dependency library and it was easy to shade it. This is no longer true 🤷‍♂️

I will leave this comment here, please come back and tick "+1" next time you fix "OkHttp doesn't work with Kotlin version X.Y.Z"-like issues 😅

@VISTALL

This comment has been minimized.

Copy link

commented Mar 19, 2019

I agree with @bsideup.

There no way for exclude kotlin libraries, and that will be painful when more than one kotlin runtime exists in classloader with different versions.

As java developer, I don't saw any positive news. iOS ? Nope. Kotlin? (I'm not mobile developer) Also - nope.

Can I stay on OkHttp 3.x forever? We’re hoping that you won’t have to. It’s our job to make OkHttp 4 compelling enough that the cost of this upgrade is justified.

It's my way 🚶 . Later i think need find other http library.

Thanks for okhttp3 👍

@rafal-glowinski

This comment has been minimized.

Copy link

commented Mar 19, 2019

@VISTALL there are plenty of companies that use Kotlin for their backend services.

@VISTALL

This comment has been minimized.

Copy link

commented Mar 19, 2019

@rafal-glowinski but not all (and not even 50% of them). It's a holy war ⚔️

@swankjesse

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

We are going to be very conservative with which Kotlin standard library features we use. It's extremely important to me and to this project that we make it easy to stay up-to-date and also secure.

Please give us a chance to be your preferred HTTP library for the JVM. Despite the fact that OkHttp uses Kotlin, we are still very concerned with usability from Java. Square maintains a huge number of Java services that use OkHttp and we need to keep those working smoothly!

@bsideup

This comment has been minimized.

Copy link

commented Mar 19, 2019

@swankjesse let me explain our use of the library:

We ( http://github.com/testcontainers/testcontainers-java/ project ) have migrated from Netty to OkHttp because we wanted to have a zero dependency http library (and shade-able).

We care about the amount of dependencies because the library is popular, being used in many different projects, with different dependencies on its own, and every new dependency we add may lead to a conflict (as seen many times in the past). Even the most stable / popular ones caused us some headache.

This is not just a panic.
Most probably, we will stay with okhttp3 as long as it works, but also start looking for another library in parallel.
Why? Because we're not ready to depend on an language ecosystem just because one of our dependencies have decided to cut some boilerplate. I wish Kotlin adds "stdlib-less" mode.

Thanks for a great library, we enjoyed it so far, I respect your decision because obviously the project is very Android-focused, but wanted to provide some details why we regret about this decision and how it affects another, slightly less yet popular Java library.

@bsideup

This comment has been minimized.

Copy link

commented Mar 19, 2019

I wonder what @fabric8io folks think about it, I guess it will also affect their Java-only libraries

@VISTALL

This comment has been minimized.

Copy link

commented Mar 19, 2019

@swankjesse Also. For example android projects. Kotlin is prefer for Android dev, but - nobody restrict java usage. That why - many projects wrote in java, due more easy support it(or just don't want add kotlin).

and here an example:

Java android project, use okhttp, and after update - kotlin runtime required (which will be affect apk file size)

https://developer.android.com/kotlin/faq

What's Kotlin's impact on APK size / method count?

The Kotlin runtime adds about 7,000 methods and ~1MB to your debug APK. The net impact might be less if you use Kotlin to replace another library in your project, such as Guava or RxJava. This size also reduces when you optimize the APK for release with Proguard, just like other app code and libraries.

(android not provide kotlin runtime)

You forced for use Kotlin runtime, or use old versions.

@swankjesse

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

For Okio 2 the shaded + pruned cost of the stdlib is 7 KiB.

@yschimke

This comment has been minimized.

Copy link
Collaborator

commented Mar 19, 2019

@swankjesse will this be part of the release process? e.g. publish a shaded artifact with a distinct classifier (?) that is not noticeably kotlin?

@swankjesse

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

No, that’s annoying for anyone who depends on two libraries that mutually depend on OkHttp. My recommendation is for library owners to shade OkHttp + Kotlin if doing so solves problems for them. I hope it is never necessary though.

@yschimke

This comment has been minimized.

Copy link
Collaborator

commented Mar 19, 2019

@swankjesse yeah, I guess it composes badly across libraries like okio as well.

@LouisCAD

This comment has been minimized.

Copy link

commented Mar 19, 2019

@VISTALL You should always use Proguard/R8 for release apks. When doing so, you can even end up with an apk lighter than with Java thanks in part to inlining.

@galderz

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

I'd like to help with this. I've not done Kotlin before but did Scala for a few years, so as I learn about the language some aspects feel relatively familiar. Any particular classes you'd recommend I could focus on?

@swankjesse

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2019

@galderz if you're new to Kotlin I think it's the wrong effort to help with. We're migrating a ton of code and doing so quickly; this environment isn't great for learning!

@bnorm

This comment has been minimized.

Copy link

commented Apr 3, 2019

Say I have Kotlin API suggestions: is this the best place to bring them up, should I open an API suggestion issue, or open a separate issue for each one?

@swankjesse

This comment has been minimized.

Copy link
Member Author

commented Apr 4, 2019

There are four kinds we’re going to do that don't warrant much tracking:

  • val instead of accessor
  • extension function instead of static
  • operator function where appropriate
  • top level function where appropriate

For each the most important thing is whether we can do it without breaking any compatibility, including Java source, Kotlin source, and binary compatibility.

I'm reluctant to expose higher-level Kotlin APIs like DSLs. And I'm similarly not ready to adopt coroutines.

With that all said, if you have API ideas please list em!

@bnorm

This comment has been minimized.

Copy link

commented Apr 5, 2019

First suggestion: Call.timeout() feels like a val to me. I wish I and thought of this during Okio 2.0 as well.

Second is more of a thought for potential coroutine work. It's a pretty big break to Kotlin source compatibility but something like this in the Call class might set it up nicely for better coroutine naming in the future.

@JvmName("execute")
fun executeBlocking() { ... }
@swankjesse

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2019

Unfortunately we can’t have the name execute for coroutines. Our users might skip 4.0.0 and go right from 3.14 to the release that has coroutines.

@yschimke

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

On the Java Tests (#4914)

Moving tests like these is premature. These currently function as Java source compatibility regression tests.

We need specific Public API coverage for both Kotlin and Java before we refactor these (Java unit tests). Also there is less priority for moving existing tests and it introduces some risk when trying to ensure we don't have regressions between 3.14 and 4.0.

@yschimke

This comment has been minimized.

Copy link
Collaborator

commented Apr 14, 2019

No performance regressions. We must write careful Kotlin, paying attention to abstractions that have a runtime cost.

To me the requirements in the original description and especially this imply an RC-1 that we can test for source, binary compatiblity and performance regressions. Just want to confirm this is the plan?

@swankjesse

This comment has been minimized.

Copy link
Member Author

commented Apr 14, 2019

Yes! I'm ready to cut that almost immediately too. Only hazard is it'll be something worse than a release candidate because we haven't yet added nice Kotlin APIs.

Maybe 4.0.0-ALPHA1 ?

@swankjesse

This comment has been minimized.

Copy link
Member Author

commented May 12, 2019

Idiomatic Kotlin

The Okio changelog includes a table of changes to adopt Kotlin idioms. For Okio we made these 4 types of changes:

  • val/var: change Buffer.size() to Buffer.size
  • operator function: change Buffer.getByte() to operator fun Buffer.get()
  • extension function: change ByteString.decodeBase64(String) to String.decodeBase64()
  • top level function: change Okio.blackhole() to fun blackholeSink()

We should do similar for OkHttp! This includes offering @ReplaceWith migrations for the current APIs as we did with Okio’s Utf8 and Upgrade classes.

Our migration helpers will have binary names like -DeprecatedUtf8. The KotlinSourceCompatibilityTest will point at these helpers, and executing the ReplaceWith operation should adopt the Kotlin idioms.

Checklist

These are the public APIs that need Kotlin idioms.

mockwebserver

  • Dispatcher
  • MockResponse
  • MockWebServer
  • MwsDuplexAccess
  • PushPromise
  • QueueDispatcher
  • RecordedRequest
  • SocketPolicy

okhttp

  • Address
  • Authenticator
  • Cache
  • CacheControl
  • CacheControl.Builder
  • Call
  • Call.Factory
  • Callback
  • CertificatePinner
  • CertificatePinner.Builder
  • Challenge
  • CipherSuite
  • Connection
  • ConnectionPool
  • ConnectionSpec
  • ConnectionSpec.Builder
  • Cookie
  • Cookie.Builder
  • CookieJar
  • Credentials
  • Dispatcher
  • Dns
  • EventListener
  • EventListener.Factory
  • FormBody
  • FormBody.Builder
  • Handshake
  • Headers
  • Headers.Builder
  • HttpUrl
  • HttpUrl.Builder
  • Interceptor
  • Interceptor.Chain
  • MediaType
  • MultipartBody
  • MultipartBody.Builder
  • MultipartBody.Part
  • OkHttpClient
  • OkHttpClient.Builder
  • Protocol
  • Request
  • Request.Builder
  • RequestBody
  • Response
  • Response.Builder
  • ResponseBody
  • Route
  • TlsVersion
  • WebSocket
  • WebSocket.Factory
  • WebSocketListener

okhttp-dnsoverhttps

  • DnsOverHttps
  • DnsOverHttps.Builder

okhttp-logging-interceptor

  • HttpLoggingInterceptor
  • HttpLoggingInterceptor.Level
  • HttpLoggingInterceptor.Logger
  • LoggingEventListener
  • LoggingEventListener.Factory

okhttp-sse

  • EventSource
  • EventSource.Factory
  • EventSourceListener
  • EventSources

okhttp-tls

  • HandshakeCertificates
  • HandshakeCertificates.Builder
  • HeldCertificate
  • HeldCertificate.Builder

okhttp-urlconnection

  • JavaNetAuthenticator
  • JavaNetCookieJar
@shenliuyang

This comment has been minimized.

Copy link

commented May 16, 2019

Support nonblocking and coroutine will be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.