Skip to content

Commit a6f01eb

Browse files
aanorbelaanorbel
committed
chore: update based on review
1 parent 29a7903 commit a6f01eb

12 files changed

Lines changed: 142 additions & 95 deletions

File tree

‎androidApp/src/main/kotlin/org/ooni/probe/AndroidApplication.kt‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ import org.ooni.engine.AndroidNetworkTypeFinder
3535
import org.ooni.engine.AndroidOonimkallBridge
3636
import org.ooni.engine.AndroidResolverTypeFinder
3737
import org.ooni.engine.AndroidSecureStorage
38+
import org.ooni.engine.NetworkTypeFinder
39+
import org.ooni.engine.ResolverTypeMapper
3840
import org.ooni.passport.AndroidPassportBridge
3941
import org.ooni.probe.background.AppWorkerManager
4042
import org.ooni.probe.config.AndroidBatteryOptimization
@@ -63,7 +65,7 @@ class AndroidApplication : Application() {
6365
cacheDir = cacheDir.absolutePath,
6466
databaseDriverFactory = ::buildDatabaseDriver,
6567
networkTypeFinder = AndroidNetworkTypeFinder(connectivityManager),
66-
resolverTypeFinder = AndroidResolverTypeFinder(connectivityManager),
68+
buildResolverTypeFinder = ::buildResolverTypeFinder,
6769
secureStorage = AndroidSecureStorage(this, OrganizationConfig.baseSoftwareName),
6870
buildDataStore = ::buildDataStore,
6971
getBatteryState = ::getBatteryState,
@@ -149,6 +151,10 @@ class AndroidApplication : Application() {
149151
),
150152
)
151153

154+
private fun buildResolverTypeFinder(
155+
networkTypeFinder: NetworkTypeFinder, resolverTypeMapper: ResolverTypeMapper
156+
) = AndroidResolverTypeFinder(connectivityManager=connectivityManager, networkTypeFinder = networkTypeFinder)
157+
152158
private fun getBatteryState(): BatteryState {
153159
// From https://developer.android.com/training/monitoring-device-state/battery-monitoring#DetermineChargeState
154160
val batteryStatus = registerReceiver(null, IntentFilter(Intent.ACTION_BATTERY_CHANGED))

‎composeApp/src/androidMain/kotlin/org/ooni/engine/AndroidResolverTypeFinder.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import org.ooni.engine.models.ResolverType
1212
*/
1313
class AndroidResolverTypeFinder(
1414
private val connectivityManager: ConnectivityManager?,
15-
private val networkTypeFinder: NetworkTypeFinder = AndroidNetworkTypeFinder(connectivityManager),
15+
private val networkTypeFinder: NetworkTypeFinder,
1616
) : ResolverTypeFinder {
1717
override fun invoke(): ResolverType {
1818
val networkType = networkTypeFinder()

composeApp/src/commonMain/kotlin/org/ooni/engine/ResolverTypeDetector.kt renamed to composeApp/src/commonMain/kotlin/org/ooni/engine/ResolverTypeMapper.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import org.ooni.engine.models.ResolverType
88
*
99
* Holds one instance per [ResolverTypeFinder] so the last fresh verdict is scoped to that finder.
1010
*/
11-
class ResolverTypeDetector {
11+
class ResolverTypeMapper {
1212
/** Last fresh (non-cache) Private DNS verdict, reused for cache hits. */
1313
private var lastIsPrivateDnsActive: Boolean? = null
1414

‎composeApp/src/commonMain/kotlin/org/ooni/probe/di/Dependencies.kt‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ import kotlinx.serialization.json.Json
1313
import okio.FileSystem
1414
import okio.Path.Companion.toPath
1515
import okio.SYSTEM
16-
import org.ooni.engine.DefaultResolverTypeFinder
1716
import org.ooni.engine.Engine
1817
import org.ooni.engine.NetworkTypeFinder
1918
import org.ooni.engine.OonimkallBridge
2019
import org.ooni.engine.ResolverTypeFinder
20+
import org.ooni.engine.ResolverTypeMapper
2121
import org.ooni.engine.SecureStorage
2222
import org.ooni.engine.TaskEventMapper
2323
import org.ooni.passport.PassportBridge
@@ -173,7 +173,7 @@ class Dependencies(
173173
val cacheDir: String,
174174
private val databaseDriverFactory: () -> SqlDriver,
175175
private val networkTypeFinder: NetworkTypeFinder,
176-
private val resolverTypeFinder: ResolverTypeFinder = DefaultResolverTypeFinder(networkTypeFinder),
176+
val buildResolverTypeFinder: ((NetworkTypeFinder, ResolverTypeMapper) -> ResolverTypeFinder),
177177
val secureStorage: SecureStorage,
178178
@get:VisibleForTesting
179179
val buildDataStore: () -> DataStore<Preferences>,
@@ -198,6 +198,12 @@ class Dependencies(
198198
@get:VisibleForTesting
199199
var databaseContext: CoroutineContext = Dispatchers.IO,
200200
) {
201+
private val resolverTypeMapper: ResolverTypeMapper by lazy { ResolverTypeMapper() }
202+
203+
private val resolverTypeFinder: ResolverTypeFinder by lazy {
204+
buildResolverTypeFinder(networkTypeFinder, resolverTypeMapper)
205+
}
206+
201207
// Common
202208

203209
@VisibleForTesting

‎composeApp/src/commonTest/kotlin/org/ooni/engine/ResolverTypeDetectorTest.kt‎

Lines changed: 0 additions & 63 deletions
This file was deleted.

‎composeApp/src/commonTest/kotlin/org/ooni/engine/ResolverTypeMapperTest.kt‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package org.ooni.engine
2+
3+
import org.ooni.engine.models.NetworkType
4+
import org.ooni.engine.models.ResolverType
5+
import kotlin.test.Test
6+
import kotlin.test.assertEquals
7+
8+
class ResolverTypeMapperTest {
9+
@Test
10+
fun encryptedDnsIsPrivateDns() {
11+
assertEquals(ResolverType.PrivateDns, ResolverTypeMapper().resolverType(NetworkType.Wifi, listOf("https")))
12+
assertEquals(ResolverType.PrivateDns, ResolverTypeMapper().resolverType(NetworkType.Wifi, listOf("tls")))
13+
assertEquals(ResolverType.PrivateDns, ResolverTypeMapper().resolverType(NetworkType.Wifi, listOf("udp", "https")))
14+
}
15+
16+
@Test
17+
fun plaintextDnsIsSystem() {
18+
assertEquals(ResolverType.System, ResolverTypeMapper().resolverType(NetworkType.Wifi, listOf("udp")))
19+
assertEquals(ResolverType.System, ResolverTypeMapper().resolverType(NetworkType.Mobile, listOf("tcp")))
20+
}
21+
22+
@Test
23+
fun vpnWithPlaintextDnsIsVpn() {
24+
assertEquals(ResolverType.VPN, ResolverTypeMapper().resolverType(NetworkType.VPN, listOf("udp")))
25+
}
26+
27+
@Test
28+
fun vpnWithEncryptedDnsIsPrivateDns() {
29+
assertEquals(ResolverType.PrivateDns, ResolverTypeMapper().resolverType(NetworkType.VPN, listOf("https")))
30+
}
31+
32+
@Test
33+
fun noInternetIsUnknown() {
34+
assertEquals(ResolverType.Unknown, ResolverTypeMapper().resolverType(NetworkType.NoInternet, null))
35+
assertEquals(ResolverType.Unknown, ResolverTypeMapper().resolverType(NetworkType.NoInternet, listOf("udp")))
36+
}
37+
38+
@Test
39+
fun noObservationIsUnknown() {
40+
assertEquals(ResolverType.Unknown, ResolverTypeMapper().resolverType(NetworkType.Wifi, null))
41+
assertEquals(ResolverType.Unknown, ResolverTypeMapper().resolverType(NetworkType.Wifi, emptyList()))
42+
assertEquals(ResolverType.Unknown, ResolverTypeMapper().resolverType(NetworkType.Wifi, listOf("unknown")))
43+
}
44+
45+
@Test
46+
fun cachedAnswerWithoutPreviousResultIsSystem() {
47+
assertEquals(ResolverType.System, ResolverTypeMapper().resolverType(NetworkType.Wifi, listOf("cache")))
48+
}
49+
50+
@Test
51+
fun cachedAnswerReusesLastFreshResult() {
52+
val detector = ResolverTypeMapper()
53+
assertEquals(ResolverType.PrivateDns, detector.resolverType(NetworkType.Wifi, listOf("https")))
54+
assertEquals(ResolverType.PrivateDns, detector.resolverType(NetworkType.Wifi, listOf("cache")))
55+
}
56+
57+
@Test
58+
fun cachedAnswerReusesLastPlaintextResult() {
59+
val detector = ResolverTypeMapper()
60+
assertEquals(ResolverType.System, detector.resolverType(NetworkType.Wifi, listOf("udp")))
61+
assertEquals(ResolverType.System, detector.resolverType(NetworkType.Wifi, listOf("cache")))
62+
}
63+
}

‎composeApp/src/desktopMain/kotlin/org/ooni/engine/DesktopResolverTypeFinder.kt‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,16 @@ import org.ooni.shared.DesktopBridgeLoader
77

88
/**
99
* Resolves the active network's [ResolverType] on macOS. Probes DNS transports for
10-
* [probeDomain] via Network.framework and maps them with the shared [ResolverTypeDetector].
10+
* [probeDomain] via Network.framework and maps them with the shared [ResolverTypeMapper].
1111
*/
1212
class DesktopResolverTypeFinder(
1313
private val networkTypeFinder: NetworkTypeFinder,
1414
private val probeDomain: String?,
1515
private val timeoutMillis: Long = 3000,
1616
private val probeDnsProtocols: ((host: String, timeoutMillis: Long) -> List<String>?)? = null,
17+
private val mapper: ResolverTypeMapper,
1718
) : ResolverTypeFinder {
18-
private val detector = ResolverTypeDetector()
19+
private val logger = Logger.withTag("DesktopResolverTypeFinder")
1920

2021
/** Native probe returning observed DNS protocols, or null on failure/timeout. */
2122
private external fun nativeProbeDnsProtocols(
@@ -26,37 +27,37 @@ class DesktopResolverTypeFinder(
2627

2728
override fun invoke(): ResolverType {
2829
val networkType = networkTypeFinder()
29-
Logger.d("[DesktopResolverTypeFinder] networkType: ${networkType::class.simpleName}")
30+
logger.d("networkType: ${networkType::class.simpleName}")
3031

3132
if (networkType is NetworkType.NoInternet) {
32-
Logger.d("[DesktopResolverTypeFinder] No internet connection; skipping Private DNS probe")
33-
return detector.resolverType(networkType, null)
33+
logger.d("No internet connection; skipping Private DNS probe")
34+
return mapper.resolverType(networkType, null)
3435
}
3536

3637
return try {
3738
val dnsProtocols = readDnsProtocols()
38-
val resolverType = detector.resolverType(networkType, dnsProtocols)
39-
Logger.d("[DesktopResolverTypeFinder] dnsProtocols: $dnsProtocols -> resolverType: ${resolverType.value}")
39+
val resolverType = mapper.resolverType(networkType, dnsProtocols)
40+
logger.d("dnsProtocols: $dnsProtocols -> resolverType: ${resolverType.value}")
4041
resolverType
4142
} catch (e: Throwable) {
42-
Logger.w("Error reading resolver type: ${e.message}")
43-
detector.resolverType(networkType, null)
43+
logger.w("Error reading resolver type: ${e.message}")
44+
mapper.resolverType(networkType, null)
4445
}
4546
}
4647

4748
/** Resolves [probeDomain] and returns observed DNS protocol/source names, or null on failure/timeout. */
4849
private fun readDnsProtocols(): List<String>? {
4950
val domain = probeDomain
5051
if (domain == null) {
51-
Logger.d("[DesktopResolverTypeFinder] No probe domain configured; cannot determine Private DNS state")
52+
logger.d("No probe domain configured; cannot determine Private DNS state")
5253
return null
5354
}
54-
Logger.d("[DesktopResolverTypeFinder] Probing Private DNS using domain: $domain")
55+
logger.d("Probing Private DNS using domain: $domain")
5556
return (probeDnsProtocols ?: ::loadAndProbeDnsProtocols)(domain, timeoutMillis)
5657
}
5758

5859
private fun loadAndProbeDnsProtocols(
5960
host: String,
6061
timeoutMillis: Long,
61-
): List<String>? = if (DesktopBridgeLoader.ensureLoaded()) nativeProbeDnsProtocols(host, timeoutMillis, Logger::d)?.toList() else null
62+
): List<String>? = if (DesktopBridgeLoader.ensureLoaded()) nativeProbeDnsProtocols(host, timeoutMillis, logger::d)?.toList() else null
6263
}

‎composeApp/src/desktopMain/kotlin/org/ooni/probe/BuildDependencies.kt‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import okio.Path.Companion.toPath
1010
import org.ooni.engine.DefaultResolverTypeFinder
1111
import org.ooni.engine.DesktopNetworkTypeFinder
1212
import org.ooni.engine.DesktopResolverTypeFinder
13+
import org.ooni.engine.ResolverTypeMapper
1314
import org.ooni.engine.NetworkTypeFinder
1415
import org.ooni.engine.ResolverTypeFinder
1516
import org.ooni.engine.OonimkallBridge
@@ -98,7 +99,7 @@ internal fun buildDependencies(
9899
platformInfo: PlatformInfo = buildPlatformInfo(),
99100
oonimkallBridge: OonimkallBridge = DesktopOonimkallBridge(),
100101
networkTypeFinder: NetworkTypeFinder = DesktopNetworkTypeFinder(),
101-
resolverTypeFinder: ResolverTypeFinder = buildResolverTypeFinder(networkTypeFinder),
102+
buildResolverTypeFinder: (NetworkTypeFinder, ResolverTypeMapper) -> ResolverTypeFinder = ::buildResolverTypeFinder,
102103
secureStorageAppId: String = DesktopOrganizationConfig.appId,
103104
dataStoreFile: File = File(dataDir).resolve("probe.preferences_pb"),
104105
batteryState: BatteryState = BatteryState.Unknown,
@@ -117,7 +118,7 @@ internal fun buildDependencies(
117118
cacheDir = cacheDir,
118119
databaseDriverFactory = { buildDatabaseDriver(dataDir) },
119120
networkTypeFinder = networkTypeFinder,
120-
resolverTypeFinder = resolverTypeFinder,
121+
buildResolverTypeFinder = buildResolverTypeFinder,
121122
secureStorage = createDesktopSecureStorage(platform.os, secureStorageAppId, DesktopOrganizationConfig.baseSoftwareName),
122123
buildDataStore = { PreferenceDataStoreFactory.create { dataStoreFile } },
123124
getBatteryState = { batteryState },
@@ -184,11 +185,15 @@ internal fun buildPlatformInfo(): PlatformInfo {
184185
* On macOS Private DNS is detected from the DNS transport observed by Network.framework.
185186
* Other desktop platforms fall back to [DefaultResolverTypeFinder].
186187
*/
187-
private fun buildResolverTypeFinder(networkTypeFinder: NetworkTypeFinder): ResolverTypeFinder =
188+
private fun buildResolverTypeFinder(
189+
networkTypeFinder: NetworkTypeFinder,
190+
resolverTypeMapper: ResolverTypeMapper,
191+
): ResolverTypeFinder =
188192
if (platform.os == DesktopOS.Mac) {
189193
DesktopResolverTypeFinder(
190194
networkTypeFinder = networkTypeFinder,
191195
probeDomain = DesktopOrganizationConfig.resolverProbeDomain,
196+
mapper = resolverTypeMapper,
192197
)
193198
} else {
194199
DefaultResolverTypeFinder(networkTypeFinder)

‎composeApp/src/desktopTest/kotlin/org/ooni/engine/DesktopResolverTypeFinderTest.kt‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class DesktopResolverTypeFinderTest {
3737
networkTypeFinder = { NetworkType.Wifi },
3838
probeDomain = "probe.example.org",
3939
probeDnsProtocols = { _, _ -> throw IllegalStateException("boom") },
40+
mapper = ResolverTypeMapper(),
4041
)
4142
assertEquals(ResolverType.Unknown, finder())
4243
}
@@ -48,8 +49,14 @@ class DesktopResolverTypeFinderTest {
4849
probes++
4950
listOf("udp")
5051
}
51-
val noInternet = DesktopResolverTypeFinder({ NetworkType.NoInternet }, "probe.example.org", probeDnsProtocols = probe)
52-
val noDomain = DesktopResolverTypeFinder({ NetworkType.Wifi }, null, probeDnsProtocols = probe)
52+
val noInternet =
53+
DesktopResolverTypeFinder(
54+
{ NetworkType.NoInternet },
55+
"probe.example.org",
56+
probeDnsProtocols = probe,
57+
mapper = ResolverTypeMapper(),
58+
)
59+
val noDomain = DesktopResolverTypeFinder({ NetworkType.Wifi }, null, probeDnsProtocols = probe, mapper = ResolverTypeMapper())
5360

5461
assertEquals(ResolverType.Unknown, noInternet())
5562
assertEquals(ResolverType.Unknown, noDomain())
@@ -66,6 +73,7 @@ class DesktopResolverTypeFinderTest {
6673
hosts += host
6774
listOf("udp")
6875
},
76+
mapper = ResolverTypeMapper(),
6977
)
7078
finder()
7179

@@ -79,6 +87,7 @@ class DesktopResolverTypeFinderTest {
7987
networkTypeFinder = { NetworkType.Wifi },
8088
probeDomain = "ooni.org",
8189
probeDnsProtocols = { _, _ -> protocols },
90+
mapper = ResolverTypeMapper(),
8291
)
8392
assertEquals(ResolverType.PrivateDns, finder())
8493

@@ -91,12 +100,26 @@ class DesktopResolverTypeFinderTest {
91100
assertEquals(ResolverType.System, buildFinder(protocols = listOf("cache"))())
92101
}
93102

103+
@Test
104+
fun delegatesToCustomMapper() {
105+
val customMapper = ResolverTypeMapper()
106+
val finder = DesktopResolverTypeFinder(
107+
networkTypeFinder = { NetworkType.Wifi },
108+
probeDomain = "ooni.org",
109+
probeDnsProtocols = { _, _ -> listOf("https") },
110+
mapper = customMapper,
111+
)
112+
assertEquals(ResolverType.PrivateDns, finder())
113+
}
114+
94115
private fun buildFinder(
95116
networkType: NetworkType = NetworkType.Wifi,
96117
protocols: List<String>?,
118+
mapper: ResolverTypeMapper = ResolverTypeMapper(),
97119
) = DesktopResolverTypeFinder(
98120
networkTypeFinder = { networkType },
99121
probeDomain = "probe.example.org",
100122
probeDnsProtocols = { _, _ -> protocols },
123+
mapper = mapper,
101124
)
102125
}

0 commit comments

Comments
 (0)