[Ktor] Allow building a Ktor client (#1810)

* Add Ktor dependency

* Add buildKtor method

* Add test and deprecation notice

* KDoc

app/build.gradle.kts

@@ -190,6 +190,8 @@ dependencies {
     implementation(libs.conscrypt)
     implementation(libs.dnsjava)
     implementation(libs.guava)
+    implementation(libs.ktor.client.core)
+    implementation(libs.ktor.client.okhttp)
     implementation(libs.mikepenz.aboutLibraries.m3)
     implementation(libs.okhttp.base)
     implementation(libs.okhttp.brotli)

app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientBuilderTest.kt

@@ -7,7 +7,9 @@ package at.bitfire.davdroid.network
 import android.security.NetworkSecurityPolicy
 import dagger.hilt.android.testing.HiltAndroidRule
 import dagger.hilt.android.testing.HiltAndroidTest
-import okhttp3.OkHttpClient
+import io.ktor.client.request.get
+import io.ktor.client.statement.bodyAsText
+import kotlinx.coroutines.test.runTest
 import okhttp3.Request
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
@@ -20,25 +22,23 @@ import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import javax.inject.Inject
+import javax.inject.Provider
 
 @HiltAndroidTest
-class HttpClientTest {
+class HttpClientBuilderTest {
 
     @get:Rule
     var hiltRule = HiltAndroidRule(this)
 
     @Inject
-    lateinit var httpClientBuilder: HttpClientBuilder
+    lateinit var httpClientBuilder: Provider<HttpClientBuilder>
 
-    lateinit var httpClient: OkHttpClient
     lateinit var server: MockWebServer
 
     @Before
     fun setUp() {
         hiltRule.inject()
 
-        httpClient = httpClientBuilder.build()
-
         server = MockWebServer()
         server.start(30000)
     }
@@ -49,6 +49,18 @@ class HttpClientTest {
     }
 
 
+    @Test
+    fun testBuildKtor_CreatesWorkingClient() = runTest {
+        server.enqueue(MockResponse()
+            .setResponseCode(200)
+            .setBody("Some Content"))
+
+        val client = httpClientBuilder.get().buildKtor()
+        val response = client.get(server.url("/").toString())
+        assertEquals(200, response.status.value)
+        assertEquals("Some Content", response.bodyAsText())
+    }
+
     @Test
     fun testCookies() {
         Assume.assumeTrue(NetworkSecurityPolicy.getInstance().isCleartextTrafficPermitted)
@@ -60,6 +72,8 @@ class HttpClientTest {
                 .addHeader("Set-Cookie", "cookie1=1; path=/")
                 .addHeader("Set-Cookie", "cookie2=2")
                 .setBody("Cookie set"))
+
+        val httpClient = httpClientBuilder.get().build()
         httpClient.newCall(Request.Builder()
                 .get().url(url)
                 .build()).execute()

app/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt

@@ -19,6 +19,8 @@ import at.bitfire.davdroid.settings.SettingsManager
 import at.bitfire.davdroid.ui.ForegroundTracker
 import com.google.common.net.HttpHeaders
 import dagger.hilt.android.qualifiers.ApplicationContext
+import io.ktor.client.HttpClient
+import io.ktor.client.engine.okhttp.OkHttp
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.withContext
 import net.openid.appauth.AuthState
@@ -181,17 +183,17 @@ class HttpClientBuilder @Inject constructor(
     }
 
 
-    // actual builder
+    // okhttp builder
 
     /**
-     * Builds the [OkHttpClient].
+     * Builds an [OkHttpClient] with the configured settings.
      *
-     * Must be called only once because multiple calls indicate this wrong usage pattern:
+     * [build] or [buildKtor] must be called only once because multiple calls indicate this wrong usage pattern:
      *
      * ```
      * val builder = HttpClientBuilder(/*injected*/)
-     * val client1 = builder.configure().builder()
-     * val client2 = builder.configureOtherwise().builder()
+     * val client1 = builder.configure().build()
+     * val client2 = builder.configureOtherwise().build()
      * ```
      *
      * However in this case the configuration of `client1` is still in `builder` and would be reused for `client2`,
@@ -199,42 +201,39 @@ class HttpClientBuilder @Inject constructor(
      *
      * @throws IllegalStateException    on second and later calls
      */
+    @Deprecated("Use buildKtor instead", replaceWith = ReplaceWith("buildKtor()"))
     fun build(): OkHttpClient {
         if (alreadyBuilt)
             throw IllegalStateException("build() must only be called once; use Provider<HttpClientBuilder>")
 
-        val okBuilder = OkHttpClient.Builder()
-            // Set timeouts. According to [AbstractThreadedSyncAdapter], when there is no network
-            // traffic within a minute, a sync will be cancelled.
-            .connectTimeout(15, TimeUnit.SECONDS)
-            .writeTimeout(30, TimeUnit.SECONDS)
-            .readTimeout(120, TimeUnit.SECONDS)
-            .pingInterval(45, TimeUnit.SECONDS)     // avoid cancellation because of missing traffic; only works for HTTP/2
+        val builder = OkHttpClient.Builder()
+        configureOkHttp(builder)
+
+        alreadyBuilt = true
+        return builder.build()
+    }
+
+    private fun configureOkHttp(builder: OkHttpClient.Builder) {
+        buildTimeouts(builder)
 
         // don't allow redirects by default because it would break PROPFIND handling
-            .followRedirects(followRedirects)
+        builder.followRedirects(followRedirects)
 
         // add User-Agent to every request
-            .addInterceptor(UserAgentInterceptor)
+        builder.addInterceptor(UserAgentInterceptor)
 
         // connection-private cookie store
-            .cookieJar(cookieStore)
-
-            // allow cleartext and TLS 1.2+
-            .connectionSpecs(listOf(
-                ConnectionSpec.Companion.CLEARTEXT,
-                ConnectionSpec.Companion.MODERN_TLS
-            ))
+        builder.cookieJar(cookieStore)
 
         // offer Brotli and gzip compression (can be disabled per request with `Accept-Encoding: identity`)
-            .addInterceptor(BrotliInterceptor)
+        builder.addInterceptor(BrotliInterceptor)
 
         // app-wide custom proxy support
-        buildProxy(okBuilder)
+        buildProxy(builder)
 
         // add connection security (including client certificates) and authentication
-        buildConnectionSecurity(okBuilder)
-        buildAuthentication(okBuilder)
+        buildConnectionSecurity(builder)
+        buildAuthentication(builder)
 
         // add network logging, if requested
         if (logger.isLoggable(Level.FINEST)) {
@@ -244,11 +243,8 @@ class HttpClientBuilder @Inject constructor(
             loggingInterceptor.redactHeader(HttpHeaders.SET_COOKIE)
             loggingInterceptor.redactHeader(HttpHeaders.SET_COOKIE2)
             loggingInterceptor.level = loggerInterceptorLevel
-            okBuilder.addNetworkInterceptor(loggingInterceptor)
+            builder.addNetworkInterceptor(loggingInterceptor)
         }
-
-        alreadyBuilt = true
-        return okBuilder.build()
     }
 
     private fun buildAuthentication(okBuilder: OkHttpClient.Builder) {
@@ -258,6 +254,12 @@ class HttpClientBuilder @Inject constructor(
     }
 
     private fun buildConnectionSecurity(okBuilder: OkHttpClient.Builder) {
+        // allow cleartext and TLS 1.2+
+        okBuilder.connectionSpecs(listOf(
+            ConnectionSpec.CLEARTEXT,
+            ConnectionSpec.MODERN_TLS
+        ))
+
         // client certificate
         val clientKeyManager: KeyManager? = certificateAlias?.let { alias ->
             try {
@@ -346,4 +348,55 @@ class HttpClientBuilder @Inject constructor(
         }
     }
 
+    /**
+     * Set timeouts for the connection.
+     *
+     * **Note:** According to [android.content.AbstractThreadedSyncAdapter], when there is no network
+     * traffic within a minute, a sync will be cancelled.
+     */
+    private fun buildTimeouts(builder: OkHttpClient.Builder) {
+        builder.connectTimeout(15, TimeUnit.SECONDS)
+            .writeTimeout(30, TimeUnit.SECONDS)
+            .readTimeout(120, TimeUnit.SECONDS)
+            .pingInterval(45, TimeUnit.SECONDS)     // avoid cancellation because of missing traffic; only works for HTTP/2
+    }
+
+
+    // Ktor builder
+
+    /**
+     * Builds a Ktor [HttpClient] with the configured settings.
+     *
+     * [buildKtor] or [build] must be called only once because multiple calls indicate this wrong usage pattern:
+     *
+     * ```
+     * val builder = HttpClientBuilder(/*injected*/)
+     * val client1 = builder.configure().buildKtor()
+     * val client2 = builder.configureOtherwise().buildKtor()
+     * ```
+     *
+     * However in this case the configuration of `client1` is still in `builder` and would be reused for `client2`,
+     * which is usually not desired.
+     */
+    fun buildKtor(): HttpClient {
+        if (alreadyBuilt)
+            throw IllegalStateException("build() must only be called once; use Provider<HttpClientBuilder>")
+
+        val client = HttpClient(OkHttp) {
+            // Ktor-level configuration here
+
+            engine {
+                // okhttp engine configuration here
+
+                config {
+                    // OkHttpClient.Builder configuration here
+                    configureOkHttp(this)
+                }
+            }
+        }
+
+        alreadyBuilt = true
+        return client
+    }
+
 }

gradle/libs.versions.toml

@@ -32,6 +32,7 @@ hilt = "2.57.2"
 kotlin = "2.2.21"
 kotlinx-coroutines = "1.10.2"
 ksp = "2.3.2"
+ktor = "3.3.2"
 mikepenz-aboutLibraries = "13.1.0"
 mockk = "1.14.5"
 okhttp = "5.3.0"
@@ -93,6 +94,8 @@ junit = { module = "junit:junit", version = "4.13.2" }
 kotlin-stdlib = { module = "org.jetbrains.kotlin:kotlin-stdlib", version.ref = "kotlin" }
 kotlinx-coroutines = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "kotlinx-coroutines" }
 kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "kotlinx-coroutines" }
+ktor-client-core = { module = "io.ktor:ktor-client-core", version.ref = "ktor" }
+ktor-client-okhttp = { module = "io.ktor:ktor-client-okhttp", version.ref = "ktor" }
 mikepenz-aboutLibraries-m3 = { module = "com.mikepenz:aboutlibraries-compose-m3", version.ref = "mikepenz-aboutLibraries" }
 mockk = { module = "io.mockk:mockk", version.ref = "mockk" }
 mockk-android = { module = "io.mockk:mockk-android", version.ref = "mockk" }