From 9d9707cc8f52ac9bd68b325c1154c096b895bfdc Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Thu, 23 Jul 2026 11:40:47 -0400 Subject: [PATCH 1/3] More tests for the new DNS cache --- .../internal/dns/DnsCallStateMachineTest.kt | 454 +++++++++++++++++- .../internal/dns/DnsCallStateMachineTester.kt | 40 +- 2 files changed, 480 insertions(+), 14 deletions(-) diff --git a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt index 6a5b162afd81..c4d6bf379ff7 100644 --- a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt +++ b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt @@ -18,8 +18,11 @@ package okhttp3.internal.dns import app.cash.burst.Burst +import assertk.assertThat +import assertk.assertions.isEqualTo import java.net.InetAddress import kotlin.test.Test +import kotlin.time.Duration.Companion.seconds import okhttp3.Dns import okhttp3.Protocol import okhttp3.internal.OkHttpInternalApi @@ -29,6 +32,8 @@ class DnsCallStateMachineTest { /** Arbitrary sample values. */ private val blueIpv6s = listOf(InetAddress.getByName("1:2::3:4")) private val blueIpv4s = listOf(InetAddress.getByName("10.20.30.40")) + private val greenIpv6s = listOf(InetAddress.getByName("5:6::7:8")) + private val greenIpv4s = listOf(InetAddress.getByName("50.60.70.80")) @Test fun `happy path`(caching: Boolean = true) { @@ -68,6 +73,65 @@ class DnsCallStateMachineTest { } } + @Test + fun `caches are independent per hostname`() { + testDnsCallStateMachine { + val lysineCall0 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + lysineCall0.enqueue() + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + addresses = blueIpv4s, + ) + assertThat(lysineCall0.takeAllRecords().addresses()) + .isEqualTo(blueIpv4s) + + val commonhausCall0 = + newCall( + request = Dns.Request(hostname = "commonhaus.org"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + commonhausCall0.enqueue() + transport.respondToQuery( + hostname = "commonhaus.org", + type = TYPE_A, + addresses = greenIpv4s, + ) + assertThat(commonhausCall0.takeAllRecords().addresses()) + .isEqualTo(greenIpv4s) + + val lysineCall1 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + lysineCall1.enqueue() + assertThat(lysineCall1.takeAllRecords().addresses()) + .isEqualTo(blueIpv4s) + + val commonhausCall1 = + newCall( + request = Dns.Request(hostname = "commonhaus.org"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + commonhausCall1.enqueue() + assertThat(commonhausCall1.takeAllRecords().addresses()) + .isEqualTo(greenIpv4s) + } + } + @Test fun `cache already completed values`() = testDnsCallStateMachine { @@ -85,33 +149,190 @@ class DnsCallStateMachineTest { call0QueryIpv6.respondIpAddresses( addresses = blueIpv6s, ) - call0.takeOnRecordsIpAddresses( - addresses = blueIpv6s, + call0QueryIpv4.respondIpAddresses( + addresses = blueIpv4s, ) + assertThat(call0.takeAllRecords().addresses()) + .isEqualTo(blueIpv6s + blueIpv4s) - call0QueryIpv4.respondIpAddresses( + val call1 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeServiceMetadata = false, + caching = true, + ) + call1.enqueue() + assertThat(call1.takeAllRecords().addresses()) + .isEqualTo(blueIpv6s + blueIpv4s) + } + + @Test + fun `server time to live is honored`() = + testDnsCallStateMachine { + val call0 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + call0.enqueue() + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + timeToLive = 30.seconds, addresses = blueIpv4s, ) - call0.takeOnRecordsIpAddresses( - last = true, + assertThat(call0.takeAllRecords().addresses()) + .isEqualTo(blueIpv4s) + + sleep(27.seconds) + val call1 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + call1.enqueue() + assertThat(call1.takeAllRecords().addresses()) + .isEqualTo(blueIpv4s) + + // The blueIp4s response is expired after 30 seconds. + sleep(3.seconds) + val call2 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + call2.enqueue() + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + addresses = greenIpv4s, + ) + assertThat(call2.takeAllRecords().addresses()) + .isEqualTo(greenIpv4s) + } + + /** + * We compute expiration time from when the request is made, not from when it is received. This is + * the most conservative policy, but moderated by the minimum time to live configuration. + */ + @Test + fun `time to live is measured from call send time`() = + testDnsCallStateMachine { + val call0 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + call0.enqueue() + sleep(30.seconds) + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + timeToLive = 30.seconds, addresses = blueIpv4s, ) + assertThat(call0.takeAllRecords().addresses()) + .isEqualTo(blueIpv4s) + // The first call's cache is already expired because it took 30 seconds to be returned. val call1 = newCall( request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, includeServiceMetadata = false, caching = true, ) call1.enqueue() + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + timeToLive = 30.seconds, + addresses = greenIpv4s, + ) + assertThat(call1.takeAllRecords().addresses()) + .isEqualTo(greenIpv4s) + } - call1.takeOnRecordsIpAddresses( - addresses = blueIpv6s, + @Test + fun `server time to live is clamped to at least configured minimum`() = + testDnsCallStateMachine { + val call0 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + call0.enqueue() + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + timeToLive = 1.seconds, + addresses = blueIpv4s, ) - call1.takeOnRecordsIpAddresses( - last = true, + assertThat(call0.takeAllRecords().addresses()) + .isEqualTo(blueIpv4s) + + // The test cache's configured minimum TTL is 10 seconds, so the first response is served. + sleep(2.seconds) + val call1 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + call1.enqueue() + assertThat(call1.takeAllRecords().addresses()) + .isEqualTo(blueIpv4s) + } + + @Test + fun `server time to live is clamped to at most configured maximum`() = + testDnsCallStateMachine { + val call0 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + call0.enqueue() + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + timeToLive = 100.seconds, addresses = blueIpv4s, ) + assertThat(call0.takeAllRecords().addresses()) + .isEqualTo(blueIpv4s) + + // The test cache's configured maximum TTL is 60 seconds, so the first response is not served. + sleep(62.seconds) + val call1 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeIPv6 = false, + includeServiceMetadata = false, + caching = true, + ) + call1.enqueue() + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + timeToLive = 1.seconds, + addresses = greenIpv4s, + ) + assertThat(call1.takeAllRecords().addresses()) + .isEqualTo(greenIpv4s) } /** Confirm that two queries to the cache yield a single query to the underlying transport. */ @@ -160,6 +381,140 @@ class DnsCallStateMachineTest { ) } + @Test + fun `cache revalidate returns cached result and also makes request`() = + testDnsCallStateMachine { + // Seed the cache. + val call0 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeServiceMetadata = false, + caching = true, + ) + call0.enqueue() + + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_AAAA, + timeToLive = 10.seconds, + addresses = blueIpv6s, + ) + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + timeToLive = 10.seconds, + addresses = blueIpv4s, + ) + assertThat(call0.takeAllRecords().addresses()) + .isEqualTo(blueIpv6s + blueIpv4s) + + // After 8 seconds, the cached response is returned immediately and a revalidating call is + // also made. (10 seconds minus 2 seconds for revalidateBeforeExpire.) + sleep(8.seconds) + val call1 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeServiceMetadata = false, + caching = true, + ) + call1.enqueue() + assertThat(call1.takeAllRecords().addresses()) + .isEqualTo(blueIpv6s + blueIpv4s) + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_AAAA, + addresses = greenIpv6s, + ) + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + addresses = greenIpv4s, + ) + + // After the revalidating queries return, new queries return that data immediately. + val call2 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeServiceMetadata = false, + caching = true, + ) + call2.enqueue() + assertThat(call2.takeAllRecords().addresses()) + .isEqualTo(greenIpv6s + greenIpv4s) + } + + @Test + fun `new call joins incomplete revalidate call`() = + testDnsCallStateMachine { + // Seed the cache. + val call0 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeServiceMetadata = false, + caching = true, + ) + call0.enqueue() + + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_AAAA, + timeToLive = 10.seconds, + addresses = blueIpv6s, + ) + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + timeToLive = 10.seconds, + addresses = blueIpv4s, + ) + assertThat(call0.takeAllRecords().addresses()) + .isEqualTo(blueIpv6s + blueIpv4s) + + // After 8 seconds, the cached response is returned immediately and a revalidating call is + // also made. (10 seconds minus 2 seconds for revalidateBeforeExpire.) + sleep(8.seconds) + val call1 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeServiceMetadata = false, + caching = true, + ) + call1.enqueue() + assertThat(call1.takeAllRecords().addresses()) + .isEqualTo(blueIpv6s + blueIpv4s) + // Note this doesn't respond to TYPE_AAAA yet. + val revalidateQuery0 = transport.takeQuery( + hostname = "lysine.dev", + type = TYPE_AAAA + ) + val revalidateQuery1 = transport.takeQuery( + hostname = "lysine.dev", + type = TYPE_A + ) + revalidateQuery1.respondIpAddresses(addresses = greenIpv4s) + + // A later query can use the revalidated IPv4 records, but must wait for the revalidated + // IPv6 records. + sleep(2.seconds) + val call2 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + includeServiceMetadata = false, + caching = true, + ) + call2.enqueue() + call2.takeOnRecordsIpAddresses( + addresses = greenIpv4s, + ) + revalidateQuery0.respondIpAddresses( + addresses = greenIpv6s, + ) + call2.takeOnRecordsIpAddresses( + last = true, + addresses = greenIpv6s, + ) + } + @Test fun `failure returned last`(caching: Boolean = true) = testDnsCallStateMachine { @@ -194,7 +549,7 @@ class DnsCallStateMachineTest { } @Test - fun `failure is cached`() = + fun `partial failure is cached`() = testDnsCallStateMachine { val call0 = newCall( @@ -231,6 +586,85 @@ class DnsCallStateMachineTest { call1.takeOnFailure("boom!") } + @Test + fun `failure expires`() = + testDnsCallStateMachine { + val call0 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + caching = true, + includeIPv6 = false, + includeServiceMetadata = false, + ) + call0.enqueue() + transport.takeQuery("lysine.dev", TYPE_A) + .respondFailure("boom!") + call0.takeOnFailure("boom!") + + // The test cache expires failures after 5 seconds. + sleep(5.seconds) + val call1 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + caching = true, + includeIPv6 = false, + includeServiceMetadata = false, + ) + call1.enqueue() + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + addresses = greenIpv4s, + ) + assertThat(call1.takeAllRecords().addresses()) + .isEqualTo(greenIpv4s) + } + + @Test + fun `failure is revalidated`() = + testDnsCallStateMachine { + val call0 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + caching = true, + includeIPv6 = false, + includeServiceMetadata = false, + ) + call0.enqueue() + transport.takeQuery("lysine.dev", TYPE_A) + .respondFailure("boom!") + call0.takeOnFailure("boom!") + + // The failure expires after 5 seconds, but we start revalidating it 2 seconds before that. + sleep(3.seconds) + val call1 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + caching = true, + includeIPv6 = false, + includeServiceMetadata = false, + ) + call1.enqueue() + call1.takeOnFailure("boom!") + transport.respondToQuery( + hostname = "lysine.dev", + type = TYPE_A, + addresses = blueIpv4s, + ) + + // After the revalidation returns, that result is used. + val call2 = + newCall( + request = Dns.Request(hostname = "lysine.dev"), + caching = true, + includeIPv6 = false, + includeServiceMetadata = false, + ) + call2.enqueue() + assertThat(call2.takeAllRecords().addresses()) + .isEqualTo(blueIpv4s) + } + /** * Confirm that the state machine calls doesn't call any [Dns.Callback] methods until the previous * call to a [Dns.Callback] method has returned. diff --git a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt index 0d122444c4a7..8c1d144f0d79 100644 --- a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt +++ b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt @@ -9,6 +9,8 @@ import assertk.assertions.isNull import java.io.IOException import java.net.InetAddress import java.util.concurrent.LinkedBlockingDeque +import kotlin.time.Duration +import kotlin.time.Duration.Companion.seconds import kotlin.time.ExperimentalTime import okhttp3.Dns import okhttp3.Protocol @@ -52,6 +54,10 @@ class DnsCallStateMachineTester internal constructor() { taskRunner = taskFaker.taskRunner, delegate = transport, timeSource = taskFaker.timeSource, + minimumTimeToLive = 10.seconds, + maximumTimeToLive = 60.seconds, + failureTimeToLive = 5.seconds, + revalidateBeforeExpire = 2.seconds, ) fun newCall( @@ -67,6 +73,10 @@ class DnsCallStateMachineTester internal constructor() { caching = caching, ) + fun sleep(duration: Duration) { + taskFaker.advanceUntil(taskFaker.nanoTime + duration.inWholeNanoseconds) + } + inner class Transport : DnsCallStateMachine.Transport { val events = LinkedBlockingDeque() @@ -95,6 +105,18 @@ class DnsCallStateMachineTester internal constructor() { return event } + /** Combines [takeQuery] and [QueryEnqueued.respondIpAddresses]. */ + fun respondToQuery( + hostname: String, + type: Int, + timeToLive: Duration = 300.seconds, + addresses: List = listOf(), + ): QueryEnqueued { + val event = takeQuery(hostname, type) + event.respondIpAddresses(timeToLive, addresses) + return event + } + /** Asserts that the next-posted event is a query cancel. */ fun takeCancel( hostname: String, @@ -236,6 +258,13 @@ class DnsCallStateMachineTester internal constructor() { return event } + fun takeAllRecords(): List = buildList { + do { + val event = takeEvent() as OnRecords + addAll(event.records) + } while (!event.last) + } + /** Asserts that the next-posted event is a call to [Dns.Callback.onFailure]. */ fun takeOnFailure(message: String): OnFailure { val event = takeEvent() as OnFailure @@ -260,7 +289,7 @@ class DnsCallStateMachineTester internal constructor() { /** Respond to a [TYPE_HTTPS] query with service metadata. */ fun respondServiceMetadata( - timeToLive: Int = 300, + timeToLive: Duration = 300.seconds, alpnIds: List? = null, echConfigList: ByteString? = null, ) { @@ -270,7 +299,7 @@ class DnsCallStateMachineTester internal constructor() { listOf( ResourceRecord.Https( name = query.question.name, - timeToLive = timeToLive, + timeToLive = timeToLive.inWholeSeconds.toInt(), alpnIds = alpnIds, echConfigList = echConfigList, ), @@ -286,7 +315,7 @@ class DnsCallStateMachineTester internal constructor() { /** Respond to a [TYPE_A] or [TYPE_AAAA] query with a (possibly-empty) list of IP addresses. */ fun respondIpAddresses( - timeToLive: Int = 300, + timeToLive: Duration = 300.seconds, addresses: List = listOf(), ) { callback.onResponse( @@ -295,7 +324,7 @@ class DnsCallStateMachineTester internal constructor() { addresses.map { address -> ResourceRecord.IpAddress( name = query.question.name, - timeToLive = timeToLive, + timeToLive = timeToLive.inWholeSeconds.toInt(), address = address, ) }, @@ -325,3 +354,6 @@ class DnsCallStateMachineTester internal constructor() { ) : CallEvent } } + +fun List.addresses(): List = + map { (it as Dns.Record.IpAddress).address } From dfdf1bb10abe3214d0681d576a3c20f3e13d8f9a Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Thu, 23 Jul 2026 12:06:41 -0400 Subject: [PATCH 2/3] Spotless --- .../internal/dns/DnsCallStateMachineTest.kt | 24 +++++++++++-------- .../internal/dns/DnsCallStateMachineTester.kt | 16 ++++++------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt index c4d6bf379ff7..a73d191814b9 100644 --- a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt +++ b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt @@ -483,14 +483,16 @@ class DnsCallStateMachineTest { assertThat(call1.takeAllRecords().addresses()) .isEqualTo(blueIpv6s + blueIpv4s) // Note this doesn't respond to TYPE_AAAA yet. - val revalidateQuery0 = transport.takeQuery( - hostname = "lysine.dev", - type = TYPE_AAAA - ) - val revalidateQuery1 = transport.takeQuery( - hostname = "lysine.dev", - type = TYPE_A - ) + val revalidateQuery0 = + transport.takeQuery( + hostname = "lysine.dev", + type = TYPE_AAAA, + ) + val revalidateQuery1 = + transport.takeQuery( + hostname = "lysine.dev", + type = TYPE_A, + ) revalidateQuery1.respondIpAddresses(addresses = greenIpv4s) // A later query can use the revalidated IPv4 records, but must wait for the revalidated @@ -597,7 +599,8 @@ class DnsCallStateMachineTest { includeServiceMetadata = false, ) call0.enqueue() - transport.takeQuery("lysine.dev", TYPE_A) + transport + .takeQuery("lysine.dev", TYPE_A) .respondFailure("boom!") call0.takeOnFailure("boom!") @@ -631,7 +634,8 @@ class DnsCallStateMachineTest { includeServiceMetadata = false, ) call0.enqueue() - transport.takeQuery("lysine.dev", TYPE_A) + transport + .takeQuery("lysine.dev", TYPE_A) .respondFailure("boom!") call0.takeOnFailure("boom!") diff --git a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt index 8c1d144f0d79..e5420a0d8eae 100644 --- a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt +++ b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt @@ -258,12 +258,13 @@ class DnsCallStateMachineTester internal constructor() { return event } - fun takeAllRecords(): List = buildList { - do { - val event = takeEvent() as OnRecords - addAll(event.records) - } while (!event.last) - } + fun takeAllRecords(): List = + buildList { + do { + val event = takeEvent() as OnRecords + addAll(event.records) + } while (!event.last) + } /** Asserts that the next-posted event is a call to [Dns.Callback.onFailure]. */ fun takeOnFailure(message: String): OnFailure { @@ -355,5 +356,4 @@ class DnsCallStateMachineTester internal constructor() { } } -fun List.addresses(): List = - map { (it as Dns.Record.IpAddress).address } +fun List.addresses(): List = map { (it as Dns.Record.IpAddress).address } From 9a76bb5c5addb5d07255e4613ff4fea2fed23d9f Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Thu, 23 Jul 2026 14:51:29 -0400 Subject: [PATCH 3/3] Rename DnsCallStateMachine to StateMachineDnsCall (#9582) It's no longer necessary to have both the call and the state machine, the state machine can implement Dns.Call directly. --- .../okhttp3/dnsoverhttps/DnsOverHttps.kt | 18 +++--- ...rHttpsCall.kt => DnsOverHttpsTransport.kt} | 44 ++----------- ...chingTransport.kt => -CachingTransport.kt} | 4 +- ...tateMachine.kt => -StateMachineDnsCall.kt} | 31 +++++----- ...hineTest.kt => StateMachineDnsCallTest.kt} | 40 ++++++------ ...Tester.kt => StateMachineDnsCallTester.kt} | 61 +++++++++---------- 6 files changed, 84 insertions(+), 114 deletions(-) rename okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/internal/{-DnsOverHttpsCall.kt => DnsOverHttpsTransport.kt} (79%) rename okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/{CachingTransport.kt => -CachingTransport.kt} (98%) rename okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/{-DnsCallStateMachine.kt => -StateMachineDnsCall.kt} (93%) rename okhttp/src/commonTest/kotlin/okhttp3/internal/dns/{DnsCallStateMachineTest.kt => StateMachineDnsCallTest.kt} (97%) rename okhttp/src/commonTest/kotlin/okhttp3/internal/dns/{DnsCallStateMachineTester.kt => StateMachineDnsCallTester.kt} (88%) diff --git a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt index 4f22803daa90..ed543bf18c55 100644 --- a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt +++ b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt @@ -22,7 +22,8 @@ import okhttp3.HttpUrl import okhttp3.MediaType import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient -import okhttp3.dnsoverhttps.internal.DnsOverHttpsCall +import okhttp3.dnsoverhttps.internal.DnsOverHttpsTransport +import okhttp3.internal.dns.StateMachineDnsCall import okhttp3.internal.dns.execute import okhttp3.internal.publicsuffix.PublicSuffixDatabase @@ -45,18 +46,21 @@ class DnsOverHttps internal constructor( @get:JvmName("resolvePrivateAddresses") val resolvePrivateAddresses: Boolean, @get:JvmName("resolvePublicAddresses") val resolvePublicAddresses: Boolean, ) : Dns { - override fun newCall(request: Dns.Request): Dns.Call { - val canceledException = validate(request.hostname) - return DnsOverHttpsCall( - request = request, + private val transport = + DnsOverHttpsTransport( client = client, dnsUrl = url, post = post, + ) + + override fun newCall(request: Dns.Request): Dns.Call = + StateMachineDnsCall( + request = request, + transport = transport, + canceledException = validate(request.hostname), includeIPv6 = includeIPv6, includeServiceMetadata = includeServiceMetadata, - canceledException = canceledException, ) - } /** * Returns an exception if [hostname] should not be resolved. diff --git a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/internal/-DnsOverHttpsCall.kt b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/internal/DnsOverHttpsTransport.kt similarity index 79% rename from okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/internal/-DnsOverHttpsCall.kt rename to okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/internal/DnsOverHttpsTransport.kt index c2924423001a..e735af3b4a30 100644 --- a/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/internal/-DnsOverHttpsCall.kt +++ b/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/internal/DnsOverHttpsTransport.kt @@ -13,66 +13,44 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -@file:Suppress("ktlint:standard:filename") - package okhttp3.dnsoverhttps.internal import java.io.IOException import java.net.ProtocolException import okhttp3.Call import okhttp3.Callback -import okhttp3.Dns import okhttp3.HttpUrl import okhttp3.OkHttpClient import okhttp3.Protocol import okhttp3.Request import okhttp3.RequestBody import okhttp3.Response +import okhttp3.dnsoverhttps.DnsOverHttps import okhttp3.dnsoverhttps.DnsOverHttps.Companion.DNS_MESSAGE import okhttp3.dnsoverhttps.DnsOverHttps.Companion.MAX_RESPONSE_SIZE import okhttp3.internal.OkHttpInternalApi -import okhttp3.internal.dns.DnsCallStateMachine import okhttp3.internal.dns.DnsMessage import okhttp3.internal.dns.DnsMessageReader import okhttp3.internal.dns.DnsMessageWriter import okhttp3.internal.dns.Question +import okhttp3.internal.dns.StateMachineDnsCall import okhttp3.internal.platform.Platform import okio.Buffer import okio.BufferedSink -// TODO: in-memory caching that uses timeToLive. -// TODO: honor Https.priority and Https.targetName. Create new calls! - -/** - * Implements [Dns.Call] by making multiple HTTPS calls. - */ @OkHttpInternalApi -internal class DnsOverHttpsCall( - override val request: Dns.Request, +internal class DnsOverHttpsTransport( private val client: OkHttpClient, private val dnsUrl: HttpUrl, private val post: Boolean, - includeIPv6: Boolean, - includeServiceMetadata: Boolean, - canceledException: IOException?, -) : Dns.Call, - DnsCallStateMachine.Transport { - private val stateMachine = - DnsCallStateMachine( - transport = this, - call = this, - canceledException = canceledException, - includeIPv6 = includeIPv6, - includeServiceMetadata = includeServiceMetadata, - ) - +) : StateMachineDnsCall.Transport { override fun newQuery(question: Question): Call { val dnsMessage = DnsMessage.query(question) return client.newCall( request = Request .Builder() - .header("Accept", DNS_MESSAGE.toString()) + .header("Accept", DnsOverHttps.DNS_MESSAGE.toString()) .apply { if (post) { url(dnsUrl) @@ -98,7 +76,7 @@ internal class DnsOverHttpsCall( override fun enqueue( query: Call, - callback: DnsCallStateMachine.Transport.Callback, + callback: StateMachineDnsCall.Transport.Callback, ) { query.enqueue( object : Callback { @@ -129,16 +107,6 @@ internal class DnsOverHttpsCall( override fun cancel(query: Call) { query.cancel() } - - override fun enqueue(callback: Dns.Callback) { - stateMachine.start(callback) - } - - override fun cancel() { - stateMachine.cancel() - } - - override fun isCanceled() = stateMachine.canceled } internal fun DnsMessage.asQueryParameter(): String { diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/CachingTransport.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-CachingTransport.kt similarity index 98% rename from okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/CachingTransport.kt rename to okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-CachingTransport.kt index 1ad0c04f3d5a..c0c7c222d152 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/CachingTransport.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-CachingTransport.kt @@ -13,6 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +@file:Suppress("ktlint:standard:filename") + package okhttp3.internal.dns import java.io.IOException @@ -25,7 +27,7 @@ import kotlin.time.ExperimentalTime import kotlin.time.TimeSource import okhttp3.internal.OkHttpInternalApi import okhttp3.internal.concurrent.TaskRunner -import okhttp3.internal.dns.DnsCallStateMachine.Transport +import okhttp3.internal.dns.StateMachineDnsCall.Transport // TODO: evict old entries from cache using State.lastRequestedAt diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-DnsCallStateMachine.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-StateMachineDnsCall.kt similarity index 93% rename from okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-DnsCallStateMachine.kt rename to okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-StateMachineDnsCall.kt index 267f73585fe1..8a8990de1bb6 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-DnsCallStateMachine.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/dns/-StateMachineDnsCall.kt @@ -26,8 +26,8 @@ import okhttp3.Protocol import okhttp3.internal.OkHttpInternalApi /** - * State machine for DNS calls. This is intended for use with any transport for the queries, such - * as UDP or DNS over HTTPS. + * An application-layer DNS call that performs multiple transport-layer DNS queries in parallel. + * This delegates to an arbitrary transport like UDP or DNS over HTTPS. * * Concurrency * ----------- @@ -53,28 +53,27 @@ import okhttp3.internal.OkHttpInternalApi * that call is executing. */ @OkHttpInternalApi -class DnsCallStateMachine( +class StateMachineDnsCall( + override val request: Dns.Request, private val transport: Transport, - private val call: Dns.Call, private val canceledException: IOException?, private val includeIPv6: Boolean, private val includeServiceMetadata: Boolean, -) { +) : Dns.Call { private val state = AtomicReference>(State.Idle()) - val canceled: Boolean - get() = state.get().canceled + override fun isCanceled() = state.get().canceled - fun start(callback: Dns.Callback) { + override fun enqueue(callback: Dns.Callback) { val questions = buildList { if (includeServiceMetadata) { - add(Question(call.request.hostname, TYPE_HTTPS)) + add(Question(request.hostname, TYPE_HTTPS)) } if (includeIPv6) { - add(Question(call.request.hostname, TYPE_AAAA)) + add(Question(request.hostname, TYPE_AAAA)) } - add(Question(call.request.hostname, TYPE_A)) + add(Question(request.hostname, TYPE_A)) } val queries = @@ -126,7 +125,7 @@ class DnsCallStateMachine( } } - fun cancel() { + override fun cancel() { while (true) { val previous = state.get() val next = previous.cancel() @@ -164,7 +163,7 @@ class DnsCallStateMachine( when (resourceRecord) { is ResourceRecord.Https -> { Dns.Record.ServiceMetadata( - hostname = resourceRecord.targetName.takeIf { it != "" } ?: call.request.hostname, + hostname = resourceRecord.targetName.takeIf { it != "" } ?: request.hostname, alpnIds = resourceRecord.alpnIds?.mapNotNull { alpnId -> try { @@ -181,7 +180,7 @@ class DnsCallStateMachine( is ResourceRecord.IpAddress -> { Dns.Record.IpAddress( - hostname = call.request.hostname, + hostname = request.hostname, address = resourceRecord.address, ) } @@ -268,7 +267,7 @@ class DnsCallStateMachine( val lastAndNoExceptions = last && allExceptions.isEmpty() if (allRecords.isNotEmpty() || lastAndNoExceptions) { previous.callback.onRecords( - call = call, + call = this, last = lastAndNoExceptions, records = allRecords, ) @@ -276,7 +275,7 @@ class DnsCallStateMachine( if (last && allExceptions.isNotEmpty()) { previous.callback.onFailure( - call = call, + call = this, exceptions = allExceptions, ) } diff --git a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/StateMachineDnsCallTest.kt similarity index 97% rename from okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt rename to okhttp/src/commonTest/kotlin/okhttp3/internal/dns/StateMachineDnsCallTest.kt index a73d191814b9..d136dc5ec546 100644 --- a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTest.kt +++ b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/StateMachineDnsCallTest.kt @@ -28,7 +28,7 @@ import okhttp3.Protocol import okhttp3.internal.OkHttpInternalApi @Burst -class DnsCallStateMachineTest { +class StateMachineDnsCallTest { /** Arbitrary sample values. */ private val blueIpv6s = listOf(InetAddress.getByName("1:2::3:4")) private val blueIpv4s = listOf(InetAddress.getByName("10.20.30.40")) @@ -37,7 +37,7 @@ class DnsCallStateMachineTest { @Test fun `happy path`(caching: Boolean = true) { - testDnsCallStateMachine { + testStateMachineDnsCall { val call = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -75,7 +75,7 @@ class DnsCallStateMachineTest { @Test fun `caches are independent per hostname`() { - testDnsCallStateMachine { + testStateMachineDnsCall { val lysineCall0 = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -134,7 +134,7 @@ class DnsCallStateMachineTest { @Test fun `cache already completed values`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call0 = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -168,7 +168,7 @@ class DnsCallStateMachineTest { @Test fun `server time to live is honored`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call0 = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -223,7 +223,7 @@ class DnsCallStateMachineTest { */ @Test fun `time to live is measured from call send time`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call0 = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -263,7 +263,7 @@ class DnsCallStateMachineTest { @Test fun `server time to live is clamped to at least configured minimum`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call0 = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -297,7 +297,7 @@ class DnsCallStateMachineTest { @Test fun `server time to live is clamped to at most configured maximum`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call0 = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -338,7 +338,7 @@ class DnsCallStateMachineTest { /** Confirm that two queries to the cache yield a single query to the underlying transport. */ @Test fun `cache in flight calls`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call0 = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -383,7 +383,7 @@ class DnsCallStateMachineTest { @Test fun `cache revalidate returns cached result and also makes request`() = - testDnsCallStateMachine { + testStateMachineDnsCall { // Seed the cache. val call0 = newCall( @@ -445,7 +445,7 @@ class DnsCallStateMachineTest { @Test fun `new call joins incomplete revalidate call`() = - testDnsCallStateMachine { + testStateMachineDnsCall { // Seed the cache. val call0 = newCall( @@ -519,7 +519,7 @@ class DnsCallStateMachineTest { @Test fun `failure returned last`(caching: Boolean = true) = - testDnsCallStateMachine { + testStateMachineDnsCall { val call = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -552,7 +552,7 @@ class DnsCallStateMachineTest { @Test fun `partial failure is cached`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call0 = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -590,7 +590,7 @@ class DnsCallStateMachineTest { @Test fun `failure expires`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call0 = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -625,7 +625,7 @@ class DnsCallStateMachineTest { @Test fun `failure is revalidated`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call0 = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -678,7 +678,7 @@ class DnsCallStateMachineTest { */ @Test fun `calls to onRecords are serialized`(caching: Boolean = true) = - testDnsCallStateMachine { + testStateMachineDnsCall { val call = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -720,7 +720,7 @@ class DnsCallStateMachineTest { */ @Test fun `cancel before enqueue`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -745,7 +745,7 @@ class DnsCallStateMachineTest { @Test fun `cancel before enqueue with caching`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -768,7 +768,7 @@ class DnsCallStateMachineTest { /** Cancels are asynchronous and if the canceled query completes anyway, that's fine. */ @Test fun `cancel ignored if canceled query completes`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call = newCall( request = Dns.Request(hostname = "lysine.dev"), @@ -811,7 +811,7 @@ class DnsCallStateMachineTest { /** When caching, cancels aren't applied to the transport. */ @Test fun `cancel ignored if canceled query completes with caching`() = - testDnsCallStateMachine { + testStateMachineDnsCall { val call = newCall( request = Dns.Request(hostname = "lysine.dev"), diff --git a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/StateMachineDnsCallTester.kt similarity index 88% rename from okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt rename to okhttp/src/commonTest/kotlin/okhttp3/internal/dns/StateMachineDnsCallTester.kt index e5420a0d8eae..b185420737d9 100644 --- a/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/DnsCallStateMachineTester.kt +++ b/okhttp/src/commonTest/kotlin/okhttp3/internal/dns/StateMachineDnsCallTester.kt @@ -17,29 +17,29 @@ import okhttp3.Protocol import okhttp3.dnsResponse import okhttp3.internal.OkHttpInternalApi import okhttp3.internal.concurrent.TaskFaker -import okhttp3.internal.dns.DnsCallStateMachine.Transport -import okhttp3.internal.dns.DnsCallStateMachineTester.CallEvent.OnFailure -import okhttp3.internal.dns.DnsCallStateMachineTester.CallEvent.OnRecords -import okhttp3.internal.dns.DnsCallStateMachineTester.TransportEvent.QueryCanceled -import okhttp3.internal.dns.DnsCallStateMachineTester.TransportEvent.QueryEnqueued import okhttp3.internal.dns.DnsMessage.Companion.query +import okhttp3.internal.dns.StateMachineDnsCall.Transport +import okhttp3.internal.dns.StateMachineDnsCallTester.CallEvent.OnFailure +import okhttp3.internal.dns.StateMachineDnsCallTester.CallEvent.OnRecords +import okhttp3.internal.dns.StateMachineDnsCallTester.TransportEvent.QueryCanceled +import okhttp3.internal.dns.StateMachineDnsCallTester.TransportEvent.QueryEnqueued import okio.ByteString /** - * Test the DNS state machine. + * Test [StateMachineDnsCall]. * * This has helpers to operate on the state machine: * * This tracks all effects from the state machine as events: creating queries, canceling queries, * calling callbacks. */ -fun testDnsCallStateMachine(block: DnsCallStateMachineTester.() -> Unit) { - val tester = DnsCallStateMachineTester() +fun testStateMachineDnsCall(block: StateMachineDnsCallTester.() -> Unit) { + val tester = StateMachineDnsCallTester() tester.block() assertThat(tester.transport.events.poll(), "unexpected transport event").isNull() } -class DnsCallStateMachineTester internal constructor() { +class StateMachineDnsCallTester internal constructor() { var onNextEvent: (() -> Unit)? = null /** Defend against re-entrant calls. */ @@ -65,19 +65,19 @@ class DnsCallStateMachineTester internal constructor() { includeIPv6: Boolean = true, includeServiceMetadata: Boolean = true, caching: Boolean = false, - ): Call = - Call( - request = request, - includeIPv6 = includeIPv6, - includeServiceMetadata = includeServiceMetadata, - caching = caching, - ) + ) = CallTester( + request = request, + includeIPv6 = includeIPv6, + includeServiceMetadata = includeServiceMetadata, + caching = caching, + ) fun sleep(duration: Duration) { taskFaker.advanceUntil(taskFaker.nanoTime + duration.inWholeNanoseconds) } - inner class Transport : DnsCallStateMachine.Transport { + /** Scriptable transport for testing. */ + inner class Transport : StateMachineDnsCall.Transport { val events = LinkedBlockingDeque() override fun newQuery(question: Question) = Query(question) @@ -141,23 +141,22 @@ class DnsCallStateMachineTester internal constructor() { } /** A DNS call for the fake state machine. */ - inner class Call( - override val request: Dns.Request, + inner class CallTester( + request: Dns.Request, includeIPv6: Boolean = true, includeServiceMetadata: Boolean = true, caching: Boolean = false, - ) : Dns.Call, - Dns.Callback { + ) : Dns.Callback { private val events = LinkedBlockingDeque() - val stateMachine = - DnsCallStateMachine( + val call = + StateMachineDnsCall( + request = request, transport = when { caching -> cachingTransport else -> transport }, - call = this, canceledException = null, includeIPv6 = includeIPv6, includeServiceMetadata = includeServiceMetadata, @@ -173,16 +172,14 @@ class DnsCallStateMachineTester internal constructor() { } } - override fun enqueue(callback: Dns.Callback) { - stateMachine.start(callback) + fun enqueue(callback: Dns.Callback) { + call.enqueue(callback) } - override fun cancel() { - stateMachine.cancel() + fun cancel() { + call.cancel() } - override fun isCanceled() = stateMachine.canceled - private fun postEvent(e: CallEvent) { events.put(e) @@ -200,7 +197,7 @@ class DnsCallStateMachineTester internal constructor() { last: Boolean, records: List, ) { - check(call == this) + check(call == this.call) check(acceptCallbacks) { "unexpected callback" } acceptCallbacks = false @@ -215,7 +212,7 @@ class DnsCallStateMachineTester internal constructor() { call: Dns.Call, e: IOException, ) { - check(call == this) + check(call == this.call) check(acceptCallbacks) { "unexpected callback" } acceptCallbacks = false