Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

/**
Expand Down Expand Up @@ -206,21 +223,21 @@ 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<HttpClientBuilder> instead")

val builder = OkHttpClient.Builder()
val builder = sharedOkHttpClient.newBuilder()
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
builder.followRedirects(followRedirects)

Expand Down Expand Up @@ -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

Expand Down