mirror of
https://github.com/f-droid/fdroidclient.git
synced 2026-04-17 21:39:49 -04:00
DNS cache feature 2.0 refactor + fix
This commit is contained in:
committed by
Torsten Grote
parent
f976c588f8
commit
206ffbb26f
@@ -132,6 +132,8 @@ dependencies {
|
||||
implementation("com.journeyapps:zxing-android-embedded:4.3.0") { isTransitive = false }
|
||||
implementation(libs.zxing.core)
|
||||
|
||||
implementation(libs.okhttp)
|
||||
|
||||
implementation(libs.hilt.android)
|
||||
implementation(libs.androidx.hilt.navigation.compose)
|
||||
ksp(libs.hilt.android.compiler)
|
||||
|
||||
103
app/src/androidTest/java/org/fdroid/download/DnsCacheTest.kt
Normal file
103
app/src/androidTest/java/org/fdroid/download/DnsCacheTest.kt
Normal file
@@ -0,0 +1,103 @@
|
||||
package org.fdroid.download
|
||||
|
||||
import android.content.Context
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import org.fdroid.settings.SettingsManager
|
||||
import org.junit.runner.RunWith
|
||||
import java.net.InetAddress
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNotNull
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
@RunWith(AndroidJUnit4::class)
|
||||
class DnsCacheTest {
|
||||
|
||||
private val context = ApplicationProvider.getApplicationContext<Context>()
|
||||
private val settings = SettingsManager(context)
|
||||
|
||||
private val url1 = "locaihost"
|
||||
private val url2 = "fdroid.org"
|
||||
private val url3 = "fdroid.net"
|
||||
|
||||
private val ip1 = InetAddress.getByName("127.0.0.1")
|
||||
private val ip2 = InetAddress.getByName("127.0.0.2")
|
||||
private val ip3 = InetAddress.getByName("127.0.0.3")
|
||||
|
||||
private val list1 = listOf(ip1, ip2, ip3)
|
||||
private val list2 = listOf(ip2)
|
||||
private val list3 = listOf(ip3)
|
||||
|
||||
@Test
|
||||
fun basicCacheTest() {
|
||||
// test setup
|
||||
settings.useDnsCache = true
|
||||
val testObject = DnsCache(settings)
|
||||
|
||||
// populate cache
|
||||
testObject.insert(url1, list1)
|
||||
testObject.insert(url2, list2)
|
||||
testObject.insert(url3, list3)
|
||||
|
||||
// check for cached lookup results
|
||||
val testList1 = testObject.lookup(url1)
|
||||
assertNotNull(testList1)
|
||||
assertEquals(3, testList1.size.toLong())
|
||||
assertEquals(ip1.hostAddress, testList1[0].hostAddress)
|
||||
assertEquals(ip2.hostAddress, testList1[1].hostAddress)
|
||||
assertEquals(ip3.hostAddress, testList1[2].hostAddress)
|
||||
|
||||
// toggle preference (false)
|
||||
settings.useDnsCache = false
|
||||
|
||||
// attempt non-cached lookup
|
||||
val testList2 = testObject.lookup(url1)
|
||||
assertNull(testList2)
|
||||
|
||||
// toggle preference (true)
|
||||
settings.useDnsCache = true
|
||||
|
||||
// confirm lookup results remain in cache
|
||||
val testList3 = testObject.lookup(url2)
|
||||
assertNotNull(testList3)
|
||||
assertEquals(1, testList3.size.toLong())
|
||||
assertEquals(ip2.hostAddress, testList3[0].hostAddress)
|
||||
|
||||
// test removal
|
||||
testObject.remove(url2)
|
||||
|
||||
// confirm result was removed from cache
|
||||
val testList4 = testObject.lookup(url2)
|
||||
assertNull(testList4)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun dnsRetryTest() {
|
||||
// test setup
|
||||
settings.useDnsCache = true
|
||||
val testCache = DnsCache(settings)
|
||||
val testObject = DnsWithCache(settings, testCache)
|
||||
|
||||
// insert dummy value into cache
|
||||
testCache.insert(url2, list2)
|
||||
|
||||
// check initial status
|
||||
val testList1 = testObject.lookup(url2)
|
||||
assertEquals(1, testList1.size.toLong())
|
||||
assertEquals(ip2.hostAddress, testList1[0].hostAddress)
|
||||
|
||||
// mismatch with dummy value should require retry and clear cache
|
||||
val testFlag = testObject.shouldRetryRequest(url2)
|
||||
assertTrue(testFlag)
|
||||
val testList2 = testCache.lookup(url2)
|
||||
assertNull(testList2)
|
||||
|
||||
// subsequent lookup should cache actual dns result (not testing actual values)
|
||||
val testList3 = testObject.lookup(url2)
|
||||
assertNotNull(testList3)
|
||||
val testList4 = testCache.lookup(url2)
|
||||
assertNotNull(testList4)
|
||||
}
|
||||
}
|
||||
105
app/src/main/kotlin/org/fdroid/download/DnsCache.kt
Normal file
105
app/src/main/kotlin/org/fdroid/download/DnsCache.kt
Normal file
@@ -0,0 +1,105 @@
|
||||
package org.fdroid.download
|
||||
|
||||
import kotlinx.coroutines.MainScope
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.launch
|
||||
import mu.KotlinLogging
|
||||
import org.fdroid.settings.SettingsManager
|
||||
import java.net.InetAddress
|
||||
import java.net.UnknownHostException
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
import kotlin.concurrent.atomics.AtomicBoolean
|
||||
import kotlin.concurrent.atomics.ExperimentalAtomicApi
|
||||
|
||||
@OptIn(ExperimentalAtomicApi::class)
|
||||
@Singleton
|
||||
class DnsCache @Inject constructor(
|
||||
private val settingsManager: SettingsManager
|
||||
) {
|
||||
|
||||
private val log = KotlinLogging.logger {}
|
||||
|
||||
private var cache: MutableMap<String, List<InetAddress>>
|
||||
private var writeScheduled: AtomicBoolean = AtomicBoolean(false)
|
||||
|
||||
init {
|
||||
cache = stringToIpMap(settingsManager.dnsCache).toMutableMap()
|
||||
}
|
||||
|
||||
fun insert(hostname: String, ipList: List<InetAddress>) {
|
||||
cache[hostname] = ipList
|
||||
cacheWrite()
|
||||
}
|
||||
|
||||
fun remove(hostname: String) {
|
||||
val ipList = cache.remove(hostname)
|
||||
if (ipList != null) {
|
||||
cacheWrite()
|
||||
}
|
||||
}
|
||||
|
||||
fun lookup(hostname: String): List<InetAddress>? {
|
||||
return if (settingsManager.useDnsCache) {
|
||||
cache[hostname]
|
||||
} else {
|
||||
null
|
||||
}
|
||||
}
|
||||
|
||||
private fun cacheWrite() {
|
||||
if (writeScheduled.compareAndSet(expectedValue = false, newValue = true)) {
|
||||
MainScope().launch {
|
||||
delay(1000L)
|
||||
settingsManager.dnsCache = ipMapToString(cache)
|
||||
writeScheduled.store(false)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun ipMapToString(ipMap: Map<String, List<InetAddress>>): String {
|
||||
try {
|
||||
var output = ""
|
||||
ipMap.forEach { (key, addresses) ->
|
||||
if (!output.isEmpty()) {
|
||||
output += "\n"
|
||||
}
|
||||
output += key
|
||||
for (item in addresses) {
|
||||
if (!output.isEmpty()) {
|
||||
output += " "
|
||||
}
|
||||
output += item.hostAddress
|
||||
}
|
||||
}
|
||||
return output
|
||||
} catch (e: Exception) {
|
||||
log.error(e) { "Error converting IP map to string, returning empty string: " }
|
||||
return ""
|
||||
}
|
||||
}
|
||||
|
||||
private fun stringToIpMap(string: String): Map<String, List<InetAddress>> {
|
||||
try {
|
||||
val output = mutableMapOf<String, List<InetAddress>>()
|
||||
for (line in string.split("\n")) {
|
||||
val items = line.split(" ").toMutableList()
|
||||
val key = items.removeAt(0)
|
||||
val ipList = mutableListOf<InetAddress>()
|
||||
for (ip in items) {
|
||||
try {
|
||||
ipList.add(InetAddress.getByName(ip))
|
||||
} catch (e: UnknownHostException) {
|
||||
// should not occur, if an ip address is supplied only the format is checked
|
||||
log.error(e) { "Error parsing IP address, moving on to next item: " }
|
||||
}
|
||||
}
|
||||
output[key] = ipList
|
||||
}
|
||||
return output
|
||||
} catch (e: Exception) {
|
||||
log.error(e) { "Error converting string to IP map, returning empty map: " }
|
||||
return emptyMap()
|
||||
}
|
||||
}
|
||||
}
|
||||
59
app/src/main/kotlin/org/fdroid/download/DnsWithCache.kt
Normal file
59
app/src/main/kotlin/org/fdroid/download/DnsWithCache.kt
Normal file
@@ -0,0 +1,59 @@
|
||||
package org.fdroid.download
|
||||
|
||||
import okhttp3.Dns
|
||||
import org.fdroid.settings.SettingsManager
|
||||
import java.net.InetAddress
|
||||
import java.net.UnknownHostException
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
@Singleton
|
||||
class DnsWithCache @Inject constructor(
|
||||
private val settingsManager: SettingsManager,
|
||||
private val cache: DnsCache
|
||||
) : Dns {
|
||||
|
||||
override fun lookup(hostname: String): List<InetAddress> {
|
||||
if (!settingsManager.useDnsCache) {
|
||||
return Dns.SYSTEM.lookup(hostname)
|
||||
}
|
||||
var ipList = cache.lookup(hostname)
|
||||
if (ipList == null) {
|
||||
ipList = Dns.SYSTEM.lookup(hostname)
|
||||
cache.insert(hostname, ipList)
|
||||
}
|
||||
return ipList
|
||||
}
|
||||
|
||||
/**
|
||||
* in case a host is unreachable, check whether the cached dns result is different from
|
||||
* the current result. if the cached result is different, remove that result from the
|
||||
* cache. returns true if a cached result was removed, indicating that the connection
|
||||
* should be retried, otherwise returns false.
|
||||
*/
|
||||
fun shouldRetryRequest(hostname: String): Boolean {
|
||||
if (!settingsManager.useDnsCache) {
|
||||
// the cache feature was not enabled, so a cached result didn't cause the failure
|
||||
return false
|
||||
}
|
||||
// if no cached result was found, a cached result didn't cause the failure
|
||||
val ipList = cache.lookup(hostname) ?: return false
|
||||
try {
|
||||
val dnsList = Dns.SYSTEM.lookup(hostname)
|
||||
for (address in dnsList) {
|
||||
if (!ipList.contains(address)) {
|
||||
// the cached result doesn't match the current dns result,
|
||||
// so the connection should be retried
|
||||
cache.remove(hostname)
|
||||
return true
|
||||
}
|
||||
}
|
||||
// the cached result matches the current dns result,
|
||||
// so a cached result didn't cause the failure
|
||||
return false
|
||||
} catch (_: UnknownHostException) {
|
||||
// the url returned an unknown host exception, so there's no point in retrying
|
||||
return false
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -18,12 +18,14 @@ object DownloadModule {
|
||||
@Singleton
|
||||
fun provideHttpManager(
|
||||
settingsManager: SettingsManager,
|
||||
mirrorParameters: MirrorParameters,
|
||||
dns: DnsWithCache,
|
||||
manager: FDroidMirrorParameterManager
|
||||
): HttpManager {
|
||||
return HttpManager(
|
||||
userAgent = USER_AGENT,
|
||||
proxyConfig = settingsManager.proxyConfig,
|
||||
mirrorParameterManager = mirrorParameters,
|
||||
customDns = dns,
|
||||
mirrorParameterManager = manager
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -8,13 +8,24 @@ import org.fdroid.settings.SettingsConstants
|
||||
import org.fdroid.settings.SettingsManager
|
||||
import java.util.Locale
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
import kotlin.concurrent.atomics.ExperimentalAtomicApi
|
||||
|
||||
class MirrorParameters @Inject constructor(
|
||||
@OptIn(ExperimentalAtomicApi::class)
|
||||
@Singleton
|
||||
class FDroidMirrorParameterManager @Inject constructor(
|
||||
@param:ApplicationContext private val context: Context,
|
||||
private val settingsManager: SettingsManager
|
||||
private val settingsManager: SettingsManager,
|
||||
private val dnsWithCache: DnsWithCache
|
||||
) : MirrorParameterManager {
|
||||
|
||||
override fun shouldRetryRequest(mirrorUrl: String): Boolean {
|
||||
return dnsWithCache.shouldRetryRequest(mirrorUrl)
|
||||
}
|
||||
|
||||
// TODO overhaul default MirrorChooser
|
||||
override fun incrementMirrorErrorCount(mirrorUrl: String) {}
|
||||
|
||||
override fun getMirrorErrorCount(mirrorUrl: String): Int = 0
|
||||
|
||||
override fun preferForeignMirrors(): Boolean {
|
||||
@@ -43,6 +43,12 @@ object SettingsConstants {
|
||||
const val PREF_KEY_PROXY = "proxy"
|
||||
const val PREF_DEFAULT_PROXY = ""
|
||||
|
||||
const val PREF_USE_DNS_CACHE = "useDnsCache"
|
||||
const val PREF_USE_DNS_CACHE_DEFAULT = false
|
||||
|
||||
const val PREF_DNS_CACHE = "dnsCache"
|
||||
const val PREF_DNS_CACHE_DEFAULT = ""
|
||||
|
||||
const val PREF_KEY_PREVENT_SCREENSHOTS = "preventScreenshots"
|
||||
const val PREF_DEFAULT_PREVENT_SCREENSHOTS = false
|
||||
|
||||
|
||||
@@ -28,6 +28,8 @@ import org.fdroid.settings.SettingsConstants.PREF_DEFAULT_PROXY
|
||||
import org.fdroid.settings.SettingsConstants.PREF_DEFAULT_REPO_UPDATES
|
||||
import org.fdroid.settings.SettingsConstants.PREF_DEFAULT_SHOW_INCOMPATIBLE
|
||||
import org.fdroid.settings.SettingsConstants.PREF_DEFAULT_THEME
|
||||
import org.fdroid.settings.SettingsConstants.PREF_DNS_CACHE
|
||||
import org.fdroid.settings.SettingsConstants.PREF_DNS_CACHE_DEFAULT
|
||||
import org.fdroid.settings.SettingsConstants.PREF_KEY_APP_LIST_SORT_ORDER
|
||||
import org.fdroid.settings.SettingsConstants.PREF_KEY_AUTO_UPDATES
|
||||
import org.fdroid.settings.SettingsConstants.PREF_KEY_DYNAMIC_COLORS
|
||||
@@ -41,6 +43,8 @@ import org.fdroid.settings.SettingsConstants.PREF_KEY_PROXY
|
||||
import org.fdroid.settings.SettingsConstants.PREF_KEY_REPO_UPDATES
|
||||
import org.fdroid.settings.SettingsConstants.PREF_KEY_SHOW_INCOMPATIBLE
|
||||
import org.fdroid.settings.SettingsConstants.PREF_KEY_THEME
|
||||
import org.fdroid.settings.SettingsConstants.PREF_USE_DNS_CACHE
|
||||
import org.fdroid.settings.SettingsConstants.PREF_USE_DNS_CACHE_DEFAULT
|
||||
import org.fdroid.settings.SettingsConstants.getAppListSortOrder
|
||||
import org.fdroid.settings.SettingsConstants.toSettings
|
||||
import java.net.InetSocketAddress
|
||||
@@ -151,6 +155,22 @@ class SettingsManager @Inject constructor(
|
||||
it.get<Boolean>(PREF_KEY_PREVENT_SCREENSHOTS) ?: PREF_DEFAULT_PREVENT_SCREENSHOTS
|
||||
}.distinctUntilChanged()
|
||||
|
||||
var useDnsCache: Boolean
|
||||
get() {
|
||||
return prefs.getBoolean(PREF_USE_DNS_CACHE, PREF_USE_DNS_CACHE_DEFAULT)
|
||||
}
|
||||
set(value) {
|
||||
return prefs.edit { putBoolean(PREF_USE_DNS_CACHE, value) }
|
||||
}
|
||||
|
||||
var dnsCache: String
|
||||
get() {
|
||||
return prefs.getString(PREF_DNS_CACHE, null) ?: PREF_DNS_CACHE_DEFAULT
|
||||
}
|
||||
set(value) {
|
||||
return prefs.edit { putString(PREF_DNS_CACHE, value) }
|
||||
}
|
||||
|
||||
val filterIncompatible: Boolean
|
||||
get() = !prefs.getBoolean(PREF_KEY_SHOW_INCOMPATIBLE, PREF_DEFAULT_SHOW_INCOMPATIBLE)
|
||||
val appListSortOrder: AppListSortOrder
|
||||
|
||||
@@ -25,6 +25,7 @@ import kotlinx.coroutines.withContext
|
||||
import mu.KotlinLogging
|
||||
import org.fdroid.database.FDroidDatabase
|
||||
import org.fdroid.database.Repository
|
||||
import org.fdroid.download.DnsCache
|
||||
import org.fdroid.download.Mirror
|
||||
import org.fdroid.download.NetworkMonitor
|
||||
import org.fdroid.index.RepoManager
|
||||
@@ -46,6 +47,7 @@ class RepoDetailsViewModel @AssistedInject constructor(
|
||||
private val updateManager: UpdatesManager,
|
||||
repoUpdateManager: RepoUpdateManager,
|
||||
private val settingsManager: SettingsManager,
|
||||
private val dnsCache: DnsCache,
|
||||
private val onboardingManager: OnboardingManager,
|
||||
@param:IoDispatcher private val ioScope: CoroutineScope,
|
||||
) : AndroidViewModel(app), RepoDetailsActions {
|
||||
@@ -96,6 +98,14 @@ class RepoDetailsViewModel @AssistedInject constructor(
|
||||
|
||||
override fun deleteRepository() {
|
||||
ioScope.launch {
|
||||
// before deleting a repo, clear the related entries from the dns cache
|
||||
val repo = repoManager.getRepository(repoId)
|
||||
if (repo != null) {
|
||||
val mirrors = repo.getMirrors()
|
||||
for (mirror in mirrors) {
|
||||
dnsCache.remove(mirror.url.host)
|
||||
}
|
||||
}
|
||||
repoManager.deleteRepository(repoId)
|
||||
updateManager.loadUpdates()
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ import androidx.compose.foundation.lazy.LazyColumn
|
||||
import androidx.compose.material.icons.Icons
|
||||
import androidx.compose.material.icons.filled.BrightnessMedium
|
||||
import androidx.compose.material.icons.filled.ColorLens
|
||||
import androidx.compose.material.icons.filled.Dns
|
||||
import androidx.compose.material.icons.filled.Lan
|
||||
import androidx.compose.material.icons.filled.Notifications
|
||||
import androidx.compose.material.icons.filled.Save
|
||||
@@ -74,6 +75,8 @@ import org.fdroid.settings.SettingsConstants.PREF_KEY_PREVENT_SCREENSHOTS
|
||||
import org.fdroid.settings.SettingsConstants.PREF_KEY_PROXY
|
||||
import org.fdroid.settings.SettingsConstants.PREF_KEY_REPO_UPDATES
|
||||
import org.fdroid.settings.SettingsConstants.PREF_KEY_THEME
|
||||
import org.fdroid.settings.SettingsConstants.PREF_USE_DNS_CACHE
|
||||
import org.fdroid.settings.SettingsConstants.PREF_USE_DNS_CACHE_DEFAULT
|
||||
import org.fdroid.settings.toAutoUpdateValue
|
||||
import org.fdroid.settings.toMirrorChooserValue
|
||||
import org.fdroid.ui.FDroidContent
|
||||
@@ -353,6 +356,23 @@ fun Settings(
|
||||
key = "pref_category_privacy",
|
||||
title = { Text(stringResource(R.string.privacy)) },
|
||||
)
|
||||
switchPreference(
|
||||
key = PREF_USE_DNS_CACHE,
|
||||
defaultValue = PREF_USE_DNS_CACHE_DEFAULT,
|
||||
title = {
|
||||
Text(stringResource(R.string.useDnsCache))
|
||||
},
|
||||
icon = {
|
||||
Icon(
|
||||
imageVector = Icons.Default.Dns,
|
||||
contentDescription = null,
|
||||
modifier = Modifier.semantics { hideFromAccessibility() },
|
||||
)
|
||||
},
|
||||
summary = {
|
||||
Text(stringResource(R.string.useDnsCacheSummary))
|
||||
},
|
||||
)
|
||||
switchPreference(
|
||||
key = PREF_KEY_PREVENT_SCREENSHOTS,
|
||||
defaultValue = PREF_DEFAULT_PREVENT_SCREENSHOTS,
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
package org.fdroid.fdroid.net
|
||||
|
||||
import org.fdroid.fdroid.Preferences
|
||||
import org.junit.Assert
|
||||
import java.net.InetAddress
|
||||
import java.util.Arrays
|
||||
import kotlin.test.Test
|
||||
|
||||
class DnsCacheTest {
|
||||
|
||||
private val url1: String = "locaihost"
|
||||
private val url2: String = "fdroid.org"
|
||||
private val url3: String = "fdroid.net"
|
||||
|
||||
private val ip1: InetAddress = InetAddress.getByName("127.0.0.1")
|
||||
private val ip2: InetAddress = InetAddress.getByName("127.0.0.2")
|
||||
private val ip3: InetAddress = InetAddress.getByName("127.0.0.3")
|
||||
|
||||
private val list1: MutableList<InetAddress> = Arrays.asList<InetAddress>(ip1, ip2, ip3)
|
||||
private val list2: MutableList<InetAddress> = Arrays.asList<InetAddress>(ip2)
|
||||
private val list3: MutableList<InetAddress> = Arrays.asList<InetAddress>(ip3)
|
||||
|
||||
@Test
|
||||
fun basicCacheTest() {
|
||||
// test setup
|
||||
val prefs = Preferences.get()
|
||||
prefs.setDnsCacheEnabledValue(true)
|
||||
val testObject = DnsCache.get()
|
||||
|
||||
// populate cache
|
||||
testObject.insert(url1, list1)
|
||||
testObject.insert(url2, list2)
|
||||
testObject.insert(url3, list3)
|
||||
|
||||
// check for cached lookup results
|
||||
val testList1 = testObject.lookup(url1)
|
||||
Assert.assertEquals(3, testList1.size.toLong())
|
||||
Assert.assertEquals(ip1.hostAddress, testList1[0]!!.hostAddress)
|
||||
Assert.assertEquals(ip2.hostAddress, testList1[1]!!.hostAddress)
|
||||
Assert.assertEquals(ip3.hostAddress, testList1[2]!!.hostAddress)
|
||||
|
||||
// toggle preference (false)
|
||||
prefs.setDnsCacheEnabledValue(false)
|
||||
|
||||
// attempt non-cached lookup
|
||||
val testList2 = testObject.lookup(url1)
|
||||
Assert.assertNull(testList2)
|
||||
|
||||
// toggle preference (true)
|
||||
prefs.setDnsCacheEnabledValue(true)
|
||||
|
||||
// confirm lookup results remain in cache
|
||||
val testList3 = testObject.lookup(url2)
|
||||
Assert.assertEquals(1, testList3.size.toLong())
|
||||
Assert.assertEquals(ip2.hostAddress, testList3[0].hostAddress)
|
||||
|
||||
// test removal
|
||||
val testList4 = testObject.remove(url2)
|
||||
Assert.assertEquals(1, testList4.size.toLong())
|
||||
Assert.assertEquals(ip2.hostAddress, testList4[0].hostAddress)
|
||||
val testList5 = testObject.lookup(url2)
|
||||
Assert.assertNull(testList5)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun dnsRetryTest() {
|
||||
// test setup
|
||||
val prefs = Preferences.get()
|
||||
prefs.setDnsCacheEnabledValue(true)
|
||||
val testObject = DnsWithCache.get()
|
||||
val testCache = DnsCache.get()
|
||||
|
||||
// insert dummy value into cache
|
||||
testCache.insert(url2, list2)
|
||||
|
||||
// check initial status
|
||||
val testList1 = testObject.lookup(url2)
|
||||
Assert.assertEquals(1, testList1.size.toLong())
|
||||
Assert.assertEquals(ip2.hostAddress, testList1[0].hostAddress)
|
||||
|
||||
// mismatch with dummy value should require retry and clear cache
|
||||
val testFlag = testObject.shouldRetryRequest(url2)
|
||||
Assert.assertTrue(testFlag)
|
||||
val testList2 = testCache.lookup(url2)
|
||||
Assert.assertNull(testList2)
|
||||
|
||||
// subsequent lookup should cache actual dns result (not testing actual values)
|
||||
val testList3 = testObject.lookup(url2)
|
||||
Assert.assertNotNull(testList3)
|
||||
val testList4 = testCache.lookup(url2)
|
||||
Assert.assertNotNull(testList4)
|
||||
}
|
||||
}
|
||||
@@ -1,80 +0,0 @@
|
||||
package org.fdroid.fdroid.net;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNull;
|
||||
|
||||
import org.fdroid.fdroid.Preferences;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.net.InetAddress;
|
||||
import java.net.UnknownHostException;
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
|
||||
public class DnsWithCacheTest {
|
||||
|
||||
private static final String URL_1 = "locaihost";
|
||||
private static final String URL_2 = "fdroid.org";
|
||||
private static final String URL_3 = "fdroid.net";
|
||||
|
||||
private static final InetAddress IP_1;
|
||||
private static final InetAddress IP_2;
|
||||
private static final InetAddress IP_3;
|
||||
|
||||
static {
|
||||
try {
|
||||
IP_1 = InetAddress.getByName("127.0.0.1");
|
||||
IP_2 = InetAddress.getByName("127.0.0.2");
|
||||
IP_3 = InetAddress.getByName("127.0.0.3");
|
||||
} catch (UnknownHostException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
}
|
||||
|
||||
private static final List<InetAddress> LIST_1 = Arrays.asList(IP_1, IP_2, IP_3);
|
||||
private static final List<InetAddress> LIST_2 = Arrays.asList(IP_2);
|
||||
private static final List<InetAddress> LIST_3 = Arrays.asList(IP_3);
|
||||
|
||||
@Test
|
||||
public void basicCacheTest() throws IOException, InterruptedException {
|
||||
// test setup
|
||||
Preferences prefs = Preferences.get();
|
||||
prefs.setDnsCacheEnabledValue(true);
|
||||
DnsCache testObject = DnsCache.get();
|
||||
|
||||
// populate cache
|
||||
testObject.insert(URL_1, LIST_1);
|
||||
testObject.insert(URL_2, LIST_2);
|
||||
testObject.insert(URL_3, LIST_3);
|
||||
|
||||
// check for cached lookup results
|
||||
List<InetAddress> testList = testObject.lookup(URL_1);
|
||||
assertEquals(3, testList.size());
|
||||
assertEquals(IP_1.getHostAddress(), testList.get(0).getHostAddress());
|
||||
assertEquals(IP_2.getHostAddress(), testList.get(1).getHostAddress());
|
||||
assertEquals(IP_3.getHostAddress(), testList.get(2).getHostAddress());
|
||||
|
||||
// toggle preference (false)
|
||||
prefs.setDnsCacheEnabledValue(false);
|
||||
|
||||
// attempt non-cached lookup
|
||||
testList = testObject.lookup(URL_1);
|
||||
assertNull(testList);
|
||||
|
||||
// toggle preference (true)
|
||||
prefs.setDnsCacheEnabledValue(true);
|
||||
|
||||
// confirm lookup results remain in cache
|
||||
testList = testObject.lookup(URL_2);
|
||||
assertEquals(1, testList.size());
|
||||
assertEquals(IP_2.getHostAddress(), testList.get(0).getHostAddress());
|
||||
|
||||
// test removal
|
||||
testList = testObject.remove(URL_2);
|
||||
assertEquals(1, testList.size());
|
||||
assertEquals(IP_2.getHostAddress(), testList.get(0).getHostAddress());
|
||||
testList = testObject.lookup(URL_2);
|
||||
assertNull(testList);
|
||||
}
|
||||
}
|
||||
@@ -71,7 +71,6 @@ import org.fdroid.fdroid.nearby.PublicSourceDirProvider;
|
||||
import org.fdroid.fdroid.nearby.SDCardScannerService;
|
||||
import org.fdroid.fdroid.nearby.WifiStateChangeService;
|
||||
import org.fdroid.fdroid.net.ConnectivityMonitorService;
|
||||
import org.fdroid.fdroid.net.DnsCache;
|
||||
import org.fdroid.fdroid.net.DownloaderFactory;
|
||||
import org.fdroid.fdroid.panic.HidingManager;
|
||||
import org.fdroid.fdroid.receiver.DeviceStorageReceiver;
|
||||
@@ -302,7 +301,6 @@ public class FDroidApp extends Application implements androidx.work.Configuratio
|
||||
.build());
|
||||
}
|
||||
Preferences.setup(this);
|
||||
DnsCache.setup();
|
||||
Languages.setLanguage(this);
|
||||
Preferences preferences = Preferences.get();
|
||||
|
||||
|
||||
@@ -1,7 +1,5 @@
|
||||
package org.fdroid.fdroid.net;
|
||||
|
||||
import android.util.Log;
|
||||
|
||||
import androidx.annotation.NonNull;
|
||||
|
||||
import org.fdroid.fdroid.Preferences;
|
||||
@@ -15,8 +13,6 @@ import java.util.concurrent.TimeUnit;
|
||||
|
||||
public final class DnsCache {
|
||||
|
||||
private static final String TAG = "DnsCache";
|
||||
|
||||
private volatile HashMap<String, List<InetAddress>> cache;
|
||||
private static final int DELAY_TIME = 1;
|
||||
private static final TimeUnit DELAY_UNIT = TimeUnit.SECONDS;
|
||||
@@ -40,20 +36,9 @@ public final class DnsCache {
|
||||
}
|
||||
}
|
||||
|
||||
public static void setup() {
|
||||
if (instance != null) {
|
||||
final String error = "DnsCache can only be initialized once";
|
||||
Log.e(TAG, error);
|
||||
throw new RuntimeException(error);
|
||||
}
|
||||
instance = new DnsCache();
|
||||
}
|
||||
|
||||
public static DnsCache get() {
|
||||
if (instance == null) {
|
||||
final String error = "DnsCache must be initialized first";
|
||||
Log.e(TAG, error);
|
||||
throw new RuntimeException(error);
|
||||
instance = new DnsCache();
|
||||
}
|
||||
return instance;
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ package org.fdroid.fdroid.net;
|
||||
import androidx.annotation.NonNull;
|
||||
|
||||
import org.fdroid.fdroid.Preferences;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
|
||||
import java.net.InetAddress;
|
||||
import java.net.UnknownHostException;
|
||||
@@ -10,7 +11,22 @@ import java.util.List;
|
||||
|
||||
import okhttp3.Dns;
|
||||
|
||||
public class DnsWithCache implements Dns {
|
||||
public final class DnsWithCache implements Dns {
|
||||
|
||||
private final DnsCache cache;
|
||||
|
||||
private static DnsWithCache instance;
|
||||
|
||||
private DnsWithCache() {
|
||||
cache = DnsCache.get();
|
||||
}
|
||||
|
||||
public static DnsWithCache get() {
|
||||
if (instance == null) {
|
||||
instance = new DnsWithCache();
|
||||
}
|
||||
return instance;
|
||||
}
|
||||
|
||||
@NonNull
|
||||
@Override
|
||||
@@ -19,7 +35,6 @@ public class DnsWithCache implements Dns {
|
||||
if (!prefs.isDnsCacheEnabled()) {
|
||||
return Dns.SYSTEM.lookup(url);
|
||||
}
|
||||
DnsCache cache = DnsCache.get();
|
||||
List<InetAddress> list = cache.lookup(url);
|
||||
if (list == null) {
|
||||
list = Dns.SYSTEM.lookup(url);
|
||||
@@ -27,4 +42,39 @@ public class DnsWithCache implements Dns {
|
||||
}
|
||||
return list;
|
||||
}
|
||||
|
||||
/**
|
||||
* in case a host is unreachable, check whether the cached dns result is different from
|
||||
* the current result. if the cached result is different, remove that result from the
|
||||
* cache. returns true if a cached result was removed, indicating that the connection
|
||||
* should be retried, otherwise returns false.
|
||||
*/
|
||||
public boolean shouldRetryRequest(@NotNull String url) {
|
||||
Preferences prefs = Preferences.get();
|
||||
if (!prefs.isDnsCacheEnabled()) {
|
||||
// the cache feature was not enabled, so a cached result didn't cause the failure
|
||||
return false;
|
||||
}
|
||||
List<InetAddress> list = cache.lookup(url);
|
||||
if (list == null) {
|
||||
// no cached result was found, so a cached result didn't cause the failure
|
||||
return false;
|
||||
} else {
|
||||
try {
|
||||
List<InetAddress> lookupList = Dns.SYSTEM.lookup(url);
|
||||
for (InetAddress address : lookupList) {
|
||||
if (!list.contains(address)) {
|
||||
// the cached result doesn't match the current dns result, so the connection should be retried
|
||||
cache.remove(url);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
// the cached result matches the current dns result, so a cached result didn't cause the failure
|
||||
return false;
|
||||
} catch (UnknownHostException e) {
|
||||
// the url returned an unknown host exception, so there's no point in retrying
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -37,7 +37,7 @@ public class DownloaderFactory extends org.fdroid.download.DownloaderFactory {
|
||||
Utils.getUserAgent(),
|
||||
FDroidApp.queryString,
|
||||
NetCipher.getProxy(),
|
||||
new DnsWithCache(),
|
||||
DnsWithCache.get(),
|
||||
new FDroidMirrorParameterManager());
|
||||
|
||||
@NonNull
|
||||
|
||||
@@ -10,6 +10,7 @@ import org.fdroid.download.MirrorParameterManager;
|
||||
import org.fdroid.fdroid.FDroidApp;
|
||||
import org.fdroid.fdroid.Preferences;
|
||||
import org.fdroid.fdroid.data.App;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
@@ -66,6 +67,15 @@ public class FDroidMirrorParameterManager implements MirrorParameterManager {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true or false depending on whether a particular mirror should be retried before
|
||||
* moving on to the next one (based on checking dns results)
|
||||
*/
|
||||
@Override
|
||||
public boolean shouldRetryRequest(@NotNull String mirrorUrl) {
|
||||
return DnsWithCache.get().shouldRetryRequest(mirrorUrl);
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean preferForeignMirrors() {
|
||||
Preferences prefs = Preferences.get();
|
||||
|
||||
@@ -163,6 +163,7 @@ public abstract interface class org/fdroid/download/MirrorParameterManager {
|
||||
public abstract fun getMirrorErrorCount (Ljava/lang/String;)I
|
||||
public abstract fun incrementMirrorErrorCount (Ljava/lang/String;)V
|
||||
public abstract fun preferForeignMirrors ()Z
|
||||
public abstract fun shouldRetryRequest (Ljava/lang/String;)Z
|
||||
}
|
||||
|
||||
public final class org/fdroid/download/NoResumeException : java/lang/Exception {
|
||||
|
||||
@@ -59,7 +59,7 @@ internal abstract class MirrorChooserImpl : MirrorChooser {
|
||||
mirror.getUrl(downloadRequest.indexFile.name)
|
||||
}
|
||||
try {
|
||||
return request(mirror, url)
|
||||
return executeRequest(mirror, url, request)
|
||||
} catch (e: ResponseException) {
|
||||
// don't try other mirrors if we got Forbidden response, but supplied credentials
|
||||
if (downloadRequest.hasCredentials && e.response.status == Forbidden) throw e
|
||||
@@ -77,6 +77,14 @@ internal abstract class MirrorChooserImpl : MirrorChooser {
|
||||
error("Reached code that was thought to be unreachable.")
|
||||
}
|
||||
|
||||
open suspend fun <T> executeRequest(
|
||||
mirror: Mirror,
|
||||
url: Url,
|
||||
request: suspend (mirror: Mirror, url: Url) -> T,
|
||||
): T {
|
||||
return request(mirror, url)
|
||||
}
|
||||
|
||||
open fun handleException(e: Exception, mirror: Mirror, mirrorIndex: Int, mirrorCount: Int) {
|
||||
val wasLastMirror = mirrorIndex == mirrorCount - 1
|
||||
log.info {
|
||||
@@ -186,10 +194,28 @@ internal class MirrorChooserWithParameters(
|
||||
mirrorList.addAll(unknownList)
|
||||
mirrorList.addAll(foreignList)
|
||||
}
|
||||
|
||||
return mirrorList
|
||||
}
|
||||
|
||||
override suspend fun <T> executeRequest(
|
||||
mirror: Mirror,
|
||||
url: Url,
|
||||
request: suspend (mirror: Mirror, url: Url) -> T,
|
||||
): T {
|
||||
return try {
|
||||
request(mirror, url)
|
||||
} catch (e: Exception) {
|
||||
// in case of an exception, potentially attempt a single retry
|
||||
if (mirrorParameterManager != null &&
|
||||
mirrorParameterManager.shouldRetryRequest(url.host)
|
||||
) {
|
||||
request(mirror, url)
|
||||
} else {
|
||||
throw e
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
override fun handleException(e: Exception, mirror: Mirror, mirrorIndex: Int, mirrorCount: Int) {
|
||||
if (e is ResponseException || e is IOException) {
|
||||
mirrorParameterManager?.incrementMirrorErrorCount(mirror.baseUrl)
|
||||
|
||||
@@ -18,6 +18,12 @@ public interface MirrorParameterManager {
|
||||
|
||||
public fun getMirrorErrorCount(mirrorUrl: String): Int
|
||||
|
||||
/**
|
||||
* Returns true or false depending on whether a particular mirror should be retried before
|
||||
* moving on to the next one (typically based on checking dns results)
|
||||
*/
|
||||
public fun shouldRetryRequest(mirrorUrl: String): Boolean
|
||||
|
||||
/**
|
||||
* Returns true or false depending on whether the location preference has been enabled. This
|
||||
* preference reflects whether mirrors matching your location should get priority.
|
||||
|
||||
Reference in New Issue
Block a user