refactor: reuse okhttp instance (#2065)

* refactor: reuse the same OkHttp client instance across sessions * bump okHttp to 4.11.0 * detekt * move comment to its correct place
wireapp · Sep 18, 2023 · 8cf16e0 · 8cf16e0
1 parent 9e1f194
commit 8cf16e0
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 46 deletions.
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -13,7 +13,7 @@ cryptobox-android = "1.1.5"
 android-security = "1.1.0-alpha06"
 ktor = "2.3.4"
 okio = "3.4.0"
-ok-http = "4.10.0"
+ok-http = "4.11.0"
 mockative = "1.4.1"
 android-work = "2.8.1"
 android-test-runner = "1.5.0"

diff --git a/network/src/appleMain/kotlin/com/wire/kalium/network/defaultHttpEngine.kt b/network/src/appleMain/kotlin/com/wire/kalium/network/defaultHttpEngine.kt
@@ -31,6 +31,13 @@ actual fun defaultHttpEngine(
     ignoreSSLCertificates: Boolean,
     certificatePinning: CertificatePinning
 ): HttpClientEngine {
+    if (serverConfigDTOApiProxy != null) {
+        throw IllegalArgumentException("Proxy is not supported on iOS")
+    }
+    if (proxyCredentials != null) {
+        throw IllegalArgumentException("Proxy is not supported on iOS")
+    }
+
     return Darwin.create {
         pipelining = true
 

diff --git a/network/src/commonJvmAndroid/kotlin/com/wire/kalium/network/HttpEngine.kt b/network/src/commonJvmAndroid/kotlin/com/wire/kalium/network/HttpEngine.kt
@@ -37,67 +37,71 @@ import javax.net.ssl.SSLContext
 import javax.net.ssl.TrustManager
 import javax.net.ssl.X509TrustManager
 
+private object OkHttpSingleton {
+    private val sharedClient = OkHttpClient.Builder().apply {
+
+        // OkHttp doesn't support configuring ping intervals dynamically,
+        // so they must be set when creating the Engine
+        // See https://youtrack.jetbrains.com/issue/KTOR-4752
+        pingInterval(WEBSOCKET_PING_INTERVAL_MILLIS, TimeUnit.MILLISECONDS)
+            .connectTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
+            .readTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
+            .writeTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
+    }.build()
+
+    fun createNew(block: OkHttpClient.Builder.() -> Unit): OkHttpClient {
+        return sharedClient.newBuilder().apply(block).build()
+    }
+}
+
 actual fun defaultHttpEngine(
     serverConfigDTOApiProxy: ServerConfigDTO.ApiProxy?,
     proxyCredentials: ProxyCredentialsDTO?,
     ignoreSSLCertificates: Boolean,
     certificatePinning: CertificatePinning
 ): HttpClientEngine = OkHttp.create {
+    OkHttpSingleton.createNew {
+        if (certificatePinning.isNotEmpty()) {
+            val certPinner = CertificatePinner.Builder().apply {
+                certificatePinning.forEach { (cert, hosts) ->
+                    hosts.forEach { host ->
+                        add(host, cert)
+                    }
+                }
+            }.build()
+            certificatePinner(certPinner)
+        }
 
-    val okHttpClient = OkHttpClient.Builder()
+        if (ignoreSSLCertificates) ignoreAllSSLErrors()
 
-    if (certificatePinning.isNotEmpty()) {
-        val certPinner = CertificatePinner.Builder().apply {
-            certificatePinning.forEach { (cert, hosts) ->
-                hosts.forEach { host ->
-                    add(host, cert)
+        if (isProxyRequired(serverConfigDTOApiProxy)) {
+            if (serverConfigDTOApiProxy?.needsAuthentication == true) {
+                if (proxyCredentials == null) error("Credentials does not exist")
+                with(proxyCredentials) {
+                    Authenticator.setDefault(object : Authenticator() {
+                        override fun getPasswordAuthentication(): PasswordAuthentication {
+                            return PasswordAuthentication(username, password?.toCharArray())
+                        }
+                    })
                 }
             }
-        }.build()
-        okHttpClient.certificatePinner(certPinner)
-    }
 
-    if (ignoreSSLCertificates) okHttpClient.ignoreAllSSLErrors()
+            val proxy = Proxy(
+                Proxy.Type.SOCKS,
+                InetSocketAddress.createUnresolved(serverConfigDTOApiProxy?.host, serverConfigDTOApiProxy!!.port)
+            )
 
-    okHttpClient.pingInterval(WEBSOCKET_PING_INTERVAL_MILLIS, TimeUnit.MILLISECONDS)
-        .connectTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
-        .readTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
-        .writeTimeout(WEBSOCKET_TIMEOUT, TimeUnit.MILLISECONDS)
-
-    // OkHttp doesn't support configuring ping intervals dynamically,
-    // so they must be set when creating the Engine
-    // See https://youtrack.jetbrains.com/issue/KTOR-4752
-    if (isProxyRequired(serverConfigDTOApiProxy)) {
-        if (serverConfigDTOApiProxy?.needsAuthentication == true) {
-            if (proxyCredentials == null) error("Credentials does not exist")
-            with(proxyCredentials) {
-                Authenticator.setDefault(object : Authenticator() {
-                    override fun getPasswordAuthentication(): PasswordAuthentication {
-                        return PasswordAuthentication(username, password?.toCharArray())
-                    }
-                })
-            }
+            proxy(proxy)
         }
 
-        val proxy = Proxy(
-            Proxy.Type.SOCKS,
-            InetSocketAddress.createUnresolved(serverConfigDTOApiProxy?.host, serverConfigDTOApiProxy!!.port)
-        )
-
-        val clientWithProxy = okHttpClient.proxy(proxy).build()
-        preconfigured = clientWithProxy
-        webSocketFactory = KaliumWebSocketFactory(clientWithProxy)
-
-    } else {
-        val client = okHttpClient.build()
-        preconfigured = client
-        webSocketFactory = KaliumWebSocketFactory(client)
+    }.also {
+        preconfigured = it
+        webSocketFactory = KaliumWebSocketFactory(it)
     }
 }
 
-fun OkHttpClient.Builder.ignoreAllSSLErrors(): OkHttpClient.Builder {
-    val naiveTrustManager = @Suppress("CustomX509TrustManager")
-    object : X509TrustManager {
+private fun OkHttpClient.Builder.ignoreAllSSLErrors() {
+    val naiveTrustManager = object : X509TrustManager {
         override fun getAcceptedIssuers(): Array<X509Certificate> = arrayOf()
         override fun checkClientTrusted(certs: Array<X509Certificate>, authType: String) = Unit
         override fun checkServerTrusted(certs: Array<X509Certificate>, authType: String) = Unit
@@ -110,5 +114,4 @@ fun OkHttpClient.Builder.ignoreAllSSLErrors(): OkHttpClient.Builder {
 
     sslSocketFactory(insecureSocketFactory, naiveTrustManager)
     hostnameVerifier { _, _ -> true }
-    return this
 }