Skip to content
Prev Previous commit
Next Next commit
Disallow non-positive default max, and update tests
  • Loading branch information
peckb1 committed Apr 25, 2024
commit 5724e9d6bb891f097a189de754ec3e14b3e187eb
8 changes: 7 additions & 1 deletion okhttp/src/main/kotlin/okhttp3/ConnectionPool.kt
Original file line number Diff line number Diff line change
Expand Up @@ -178,5 +178,11 @@ class ConnectionPool internal constructor(
* Set this value to 1 to disable HTTP/2 connection coalescing
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure "connection coalescing" is the right term here.

From https://daniel.haxx.se/blog/2016/08/18/http2-connection-coalescing/

Connection coalescing means that the browser tries to determine which of the remote hosts that it can reach over the same TCP connection.

*/
@JvmField val maximumConcurrentCallsPerConnection: Int = Int.MAX_VALUE,
)
) {
init {
require(maximumConcurrentCallsPerConnection > 0) {
"maximumConcurrentCallsPerConnection is not allowed to be less than one (1)."
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import okhttp3.internal.http2.MockHttp2Peer
import okhttp3.internal.http2.Settings
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows

class ConnectionPoolTest {
private val routePlanner = FakeRoutePlanner()
Expand Down Expand Up @@ -333,36 +334,10 @@ class ConnectionPoolTest {
assertThat(pool.connectionCount()).isEqualTo(3)
}

@Test fun testSettingDefaultPolicyMinConnectionsHttp1() {
taskFaker.advanceUntil(System.nanoTime())
val expireTime = taskFaker.nanoTime + 1_000_000_000_000

routePlanner.autoGeneratePlans = true
routePlanner.defaultConnectionIdleAtNanos = expireTime
val address = routePlanner.address
val pool = routePlanner.pool

// set a default policy that has no bearing on HTTP/1 connections!
setDefaultPolicy(pool, ConnectionPool.ConnectionPoolPolicy(0))

// Connections are created as soon as a policy is set
setPolicy(pool, address, ConnectionPool.AddressPolicy(2))
assertThat(pool.connectionCount()).isEqualTo(2)

// Connections are replaced if they idle out or are evicted from the pool
evictAllConnections(pool)
assertThat(pool.connectionCount()).isEqualTo(2)
forceConnectionsToExpire(pool, expireTime)
assertThat(pool.connectionCount()).isEqualTo(2)

// Excess connections aren't removed until they idle out, even if no longer needed
setPolicy(pool, address, ConnectionPool.AddressPolicy(1))
assertThat(pool.connectionCount()).isEqualTo(2)
forceConnectionsToExpire(pool, expireTime)
assertThat(pool.connectionCount()).isEqualTo(1)

setPolicy(pool, address, ConnectionPool.AddressPolicy(3))
assertThat(pool.connectionCount()).isEqualTo(3)
@Test fun testDefaultConnectionPoolPoliciesOnlyAllowPositiveValues() {
assertThrows<IllegalArgumentException> {
ConnectionPool.ConnectionPoolPolicy(0)
}
}

@Test fun testSettingDefaultPolicyMaxConcurrentCallsHttp2() {
Expand Down