From 46f95c4f22a0a1fcadde60ce1e18f63720082262 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Thu, 23 Jul 2026 12:41:47 -0400 Subject: [PATCH] Rename DnsCallStateMachine to StateMachineDnsCall 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