From 6240e771f86ea676368fda1521914b5e85bf728f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 20:48:12 +0000 Subject: [PATCH] refactor(okhttp): inject AmethystDns instead of using a singleton MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the AmethystDns.shared lazy companion. Construct one amethystDns in AppModules and thread it through: - DualHttpClientManager / OkHttpClientFactory - DualHttpClientManagerForRelays / OkHttpClientFactoryForRelays - MediaCallEventListenerFactory / MediaCallEventListener - DnsInvalidatingEventListener.Factory - AmethystDnsStore This matches the dependency-injection style the rest of AppModules uses and lets tests inject a mock Dns where needed. Behavior is unchanged — every consumer still shares the same single instance, just by construction rather than by a static singleton. --- .../com/vitorpamplona/amethyst/AppModules.kt | 16 ++++++++++++---- .../amethyst/service/okhttp/AmethystDns.kt | 6 ------ .../amethyst/service/okhttp/AmethystDnsStore.kt | 2 +- .../okhttp/DnsInvalidatingEventListener.kt | 15 ++++++++++----- .../service/okhttp/DualHttpClientManager.kt | 3 ++- .../okhttp/DualHttpClientManagerForRelays.kt | 3 ++- .../service/okhttp/MediaCallEventListener.kt | 6 ++++-- .../service/okhttp/OkHttpClientFactory.kt | 5 +++-- .../okhttp/OkHttpClientFactoryForRelays.kt | 5 +++-- 9 files changed, 37 insertions(+), 24 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index af11cf8115..7555e6c5b4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -53,6 +53,7 @@ import com.vitorpamplona.amethyst.service.location.LocationState import com.vitorpamplona.amethyst.service.notifications.AlwaysOnNotificationServiceManager import com.vitorpamplona.amethyst.service.notifications.NotificationDispatcher import com.vitorpamplona.amethyst.service.notifications.PokeyReceiver +import com.vitorpamplona.amethyst.service.okhttp.AmethystDns import com.vitorpamplona.amethyst.service.okhttp.AmethystDnsStore import com.vitorpamplona.amethyst.service.okhttp.DualHttpClientManager import com.vitorpamplona.amethyst.service.okhttp.DualHttpClientManagerForRelays @@ -202,10 +203,15 @@ class AppModules( // Key cache service to download and decrypt encrypted files before caching them. val keyCache = EncryptionKeyCache() - // Persists the shared DNS resolver's positive cache across process restarts so cold starts - // don't pay ~700 sync getaddrinfo calls. Restored entries fall through to the - // stale-while-revalidate path on first lookup. - val dnsStore = AmethystDnsStore(appContext) + // Concurrent, caching DNS resolver shared by every OkHttp client built below — a host + // resolved for an image fetch is reused when a relay handshake or NIP-05 lookup hits the + // same host. + val amethystDns = AmethystDns() + + // Persists [amethystDns]'s positive cache across process restarts so cold starts don't pay + // ~700 sync getaddrinfo calls. Restored entries fall through to the stale-while-revalidate + // path on first lookup. + val dnsStore = AmethystDnsStore(appContext, amethystDns) // manages all the other connections separately from relays. val okHttpClients = @@ -215,6 +221,7 @@ class AppModules( isMobileDataProvider = connManager.isMobileOrNull, keyCache = keyCache, scope = applicationIOScope, + dns = amethystDns, ) // Offers easy methods to know when connections are happening through Tor or not @@ -296,6 +303,7 @@ class AppModules( proxyPortProvider = torManager.activePortOrNull, isMobileDataProvider = connManager.isMobileOrNull, scope = applicationIOScope, + dns = amethystDns, ) // Connects the INostrClient class with okHttp diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt index b42a45061f..754a33a2ac 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDns.kt @@ -273,12 +273,6 @@ class AmethystDns( Executors.newFixedThreadPool(8) { r -> Thread(r, "amethyst-dns-refresh").apply { isDaemon = true } } - - /** - * Process-wide instance shared by every OkHttp client built in the app, so a host resolved - * for an image fetch is reused when a relay handshake or NIP-05 lookup hits the same host. - */ - val shared: AmethystDns by lazy { AmethystDns() } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt index b9424b2ed3..0ae538d52d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/AmethystDnsStore.kt @@ -40,7 +40,7 @@ import com.vitorpamplona.quartz.utils.Log */ class AmethystDnsStore( private val context: Context, - private val dns: AmethystDns = AmethystDns.shared, + private val dns: AmethystDns, ) { private val prefs by lazy { context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt index 9feb9884ef..3da60a7aca 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DnsInvalidatingEventListener.kt @@ -34,17 +34,22 @@ import java.io.IOException * Used by the relay client. The media path uses [MediaCallEventListener], which folds the same * invalidation into its `finish` method alongside its existing timing logging. */ -class DnsInvalidatingEventListener private constructor() : EventListener() { +class DnsInvalidatingEventListener( + private val dns: AmethystDns, +) : EventListener() { override fun callFailed( call: Call, ioe: IOException, ) { - AmethystDns.shared.invalidate(call.request().url.host) + dns.invalidate(call.request().url.host) } - object Factory : EventListener.Factory { - private val INSTANCE = DnsInvalidatingEventListener() + /** Per-client factory. The listener is stateless, so the same instance serves every call. */ + class Factory( + dns: AmethystDns, + ) : EventListener.Factory { + private val listener = DnsInvalidatingEventListener(dns) - override fun create(call: Call): EventListener = INSTANCE + override fun create(call: Call): EventListener = listener } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt index b0c4d0ed22..460f90250a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManager.kt @@ -38,8 +38,9 @@ class DualHttpClientManager( isMobileDataProvider: StateFlow, keyCache: EncryptionKeyCache, scope: CoroutineScope, + dns: AmethystDns, ) : IHttpClientManager { - val factory = OkHttpClientFactory(keyCache, userAgent) + val factory = OkHttpClientFactory(keyCache, userAgent, dns) val defaultHttpClient: StateFlow = combine(proxyPortProvider, isMobileDataProvider) { proxy, mobile -> diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt index 76413155f7..891ccfae9c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/DualHttpClientManagerForRelays.kt @@ -35,8 +35,9 @@ class DualHttpClientManagerForRelays( proxyPortProvider: StateFlow, isMobileDataProvider: StateFlow, scope: CoroutineScope, + dns: AmethystDns, ) : IHttpClientManager { - val factory = OkHttpClientFactoryForRelays(userAgent) + val factory = OkHttpClientFactoryForRelays(userAgent, dns) val defaultHttpClient: StateFlow = combine(proxyPortProvider, isMobileDataProvider) { proxy, mobile -> diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt index 4397112a89..b7b399d42b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/MediaCallEventListener.kt @@ -44,6 +44,7 @@ import java.net.Proxy class MediaCallEventListener( private val dispatcher: Dispatcher, private val connectionPool: ConnectionPool, + private val dns: AmethystDns, ) : EventListener() { private var callStartNanos = 0L private var dnsStartNanos = 0L @@ -129,7 +130,7 @@ class MediaCallEventListener( // dead IPs for up to 24h. Per-attempt connectFailed isn't enough — a multi-A-record // host can have one bad IP and OkHttp will recover by trying the next one. if (error != null) { - AmethystDns.shared.invalidate(host) + dns.invalidate(host) } val totalMs = (System.nanoTime() - callStartNanos) / 1_000_000 @@ -178,6 +179,7 @@ class MediaCallEventListener( class MediaCallEventListenerFactory( private val dispatcher: Dispatcher, private val connectionPool: ConnectionPool, + private val dns: AmethystDns, ) : EventListener.Factory { - override fun create(call: Call): EventListener = MediaCallEventListener(dispatcher, connectionPool) + override fun create(call: Call): EventListener = MediaCallEventListener(dispatcher, connectionPool, dns) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt index 6150ad33ff..de24b6a864 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactory.kt @@ -35,6 +35,7 @@ import java.util.concurrent.TimeUnit class OkHttpClientFactory( keyCache: EncryptionKeyCache, val userAgent: String, + private val dns: AmethystDns, ) { // val logging = LoggingInterceptor() val keyDecryptor = EncryptedBlobInterceptor(keyCache) @@ -63,8 +64,8 @@ class OkHttpClientFactory( .Builder() .dispatcher(dispatcher) .connectionPool(connectionPool) - .dns(AmethystDns.shared) - .eventListenerFactory(MediaCallEventListenerFactory(dispatcher, connectionPool)) + .dns(dns) + .eventListenerFactory(MediaCallEventListenerFactory(dispatcher, connectionPool, dns)) .followRedirects(true) .followSslRedirects(true) .addInterceptor(DefaultContentTypeInterceptor(userAgent)) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt index b5dc62b039..10983a6d4d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/okhttp/OkHttpClientFactoryForRelays.kt @@ -29,6 +29,7 @@ import java.time.Duration class OkHttpClientFactoryForRelays( userAgent: String, + private val dns: AmethystDns, ) { companion object { // by picking a random proxy port, the connection will fail as it should. @@ -55,8 +56,8 @@ class OkHttpClientFactoryForRelays( OkHttpClient .Builder() .dispatcher(myDispatcher) - .dns(AmethystDns.shared) - .eventListenerFactory(DnsInvalidatingEventListener.Factory) + .dns(dns) + .eventListenerFactory(DnsInvalidatingEventListener.Factory(dns)) .followRedirects(true) .followSslRedirects(true) .addInterceptor(DefaultContentTypeInterceptor(userAgent))