diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientBuilderTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientBuilderTest.kt index 323d046668..68fe25d1f3 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientBuilderTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientBuilderTest.kt @@ -49,6 +49,14 @@ class HttpClientBuilderTest { } + @Test + fun testBuild_SharesConnectionPoolAndDispatcher() { + val client1 = httpClientBuilder.get().build() + val client2 = httpClientBuilder.get().build() + assertEquals(client1.connectionPool, client2.connectionPool) + assertEquals(client1.dispatcher, client2.dispatcher) + } + @Test fun testBuildKtor_CreatesWorkingClient() = runTest { server.enqueue(MockResponse() diff --git a/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt b/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt index dcbe2bf80c..4f5dff5bb5 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClientBuilder.kt @@ -73,6 +73,23 @@ class HttpClientBuilder @Inject constructor( // make sure Conscrypt is available when the HttpClientBuilder class is loaded the first time ConscryptIntegration().initialize() } + + /** + * According to [OkHttpClient] documentation, [OkHttpClient]s should be shared, which allows it to use a + * shared connection and thread pool. + * + * We need custom settings for each actual client, but we can use a shared client as a base. This also + * enables sharing resources like connection and thread pool. + * + * The shared client is available for the lifetime of the application and must not be shut down or + * closed (which is not necessary, according to its documentation). + */ + val sharedOkHttpClient = OkHttpClient.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 + .build() } /** @@ -206,12 +223,14 @@ class HttpClientBuilder @Inject constructor( * * However in this case the configuration of `client1` is still in `builder` and would be reused for `client2`, * which is usually not desired. + * + * Closing/shutting down the client is not necessary. */ fun build(): OkHttpClient { if (alreadyBuilt) logger.warning("build() should only be called once; use Provider instead") - val builder = OkHttpClient.Builder() + val builder = sharedOkHttpClient.newBuilder() configureOkHttp(builder) alreadyBuilt = true @@ -219,8 +238,6 @@ class HttpClientBuilder @Inject constructor( } private fun configureOkHttp(builder: OkHttpClient.Builder) { - buildTimeouts(builder) - // don't allow redirects by default because it would break PROPFIND handling builder.followRedirects(followRedirects) @@ -353,19 +370,6 @@ 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