feat: refactor GeoIP DB update handling and improve error management

aanorbel · aanorbel · commit 188221a9b1f3 · 2025-10-28T19:11:21.000+03:00
diff --git a/composeApp/src/androidMain/kotlin/org/ooni/probe/net/Http.android.kt b/composeApp/src/androidMain/kotlin/org/ooni/probe/net/Http.android.kt
@@ -2,10 +2,14 @@ package org.ooni.probe.net
 
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
+import org.ooni.engine.models.Failure
+import org.ooni.engine.models.Result
+import org.ooni.engine.models.Success
+import org.ooni.probe.data.models.GetBytesException
 import java.net.HttpURLConnection
 import java.net.URL
 
-actual suspend fun httpGetBytes(url: String): ByteArray =
+actual suspend fun httpGetBytes(url: String): Result<ByteArray, GetBytesException> =
     withContext(Dispatchers.IO) {
         val connection = (URL(url).openConnection() as HttpURLConnection)
         connection.requestMethod = "GET"
@@ -16,8 +20,13 @@ actual suspend fun httpGetBytes(url: String): ByteArray =
             val code = connection.responseCode
             val stream = if (code in 200..299) connection.inputStream else connection.errorStream
             val bytes = stream?.use { it.readBytes() } ?: ByteArray(0)
-            if (code !in 200..299) throw RuntimeException("HTTP $code while GET $url: ${String(bytes)}")
-            bytes
+            if (code !in 200..299) {
+                Failure(GetBytesException(RuntimeException("HTTP $code while GET $url: ${String(bytes)}")))
+            } else {
+                Success(bytes)
+            }
+        } catch (e: Throwable) {
+            Failure(GetBytesException(e))
         } finally {
             connection.disconnect()
         }
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/App.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/App.kt
@@ -120,8 +120,7 @@ fun App(
         // dependencies.startSingleRunInner(RunSpecification.OnlyUploadMissingResults)
     }
     LaunchedEffect(Unit) {
-        // Check for GeoIP DB updates in the background
-        runCatching { dependencies.fetchGeoIpDbUpdates() }
+        dependencies.fetchGeoIpDbUpdates()
     }
     LaunchedEffect(Unit) {
         dependencies.observeAndConfigureAutoUpdate()
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/data/models/GetBytesException.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/data/models/GetBytesException.kt
@@ -0,0 +1,5 @@
+package org.ooni.probe.data.models
+
+class GetBytesException(
+    t: Throwable,
+) : Exception(t)
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/PreferenceRepository.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/PreferenceRepository.kt
@@ -6,6 +6,7 @@ import androidx.datastore.preferences.core.Preferences
 import androidx.datastore.preferences.core.booleanPreferencesKey
 import androidx.datastore.preferences.core.edit
 import androidx.datastore.preferences.core.intPreferencesKey
+import androidx.datastore.preferences.core.longPreferencesKey
 import androidx.datastore.preferences.core.stringPreferencesKey
 import androidx.datastore.preferences.core.stringSetPreferencesKey
 import kotlinx.coroutines.flow.Flow
@@ -75,12 +76,14 @@ class PreferenceRepository(
     ): PreferenceKey<*> {
         val preferenceKey = getPreferenceKey(name = key.value, prefix = prefix, autoRun = autoRun)
         return when (key) {
-            SettingsKey.MMDB_LAST_CHECK,
             SettingsKey.MAX_RUNTIME,
             SettingsKey.LEGACY_PROXY_PORT,
             SettingsKey.DELETE_OLD_RESULTS_THRESHOLD,
             -> PreferenceKey.IntKey(intPreferencesKey(preferenceKey))
 
+            SettingsKey.MMDB_LAST_CHECK,
+            -> PreferenceKey.LongKey(longPreferencesKey(preferenceKey))
+
             SettingsKey.MMDB_VERSION,
             SettingsKey.LEGACY_PROXY_HOSTNAME,
             SettingsKey.LEGACY_PROXY_PROTOCOL,
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/DownloadFile.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/DownloadFile.kt
@@ -5,6 +5,8 @@ import okio.Path
 import okio.Path.Companion.toPath
 import okio.buffer
 import okio.use
+import org.ooni.engine.models.Result
+import org.ooni.probe.data.models.GetBytesException
 
 /**
  * Downloads binary content to a target absolute path using the provided fetcher.
@@ -13,20 +15,19 @@ import okio.use
  */
 class DownloadFile(
     private val fileSystem: FileSystem,
-    private val fetchBytes: suspend (url: String) -> ByteArray,
+    private val fetchBytes: suspend (url: String) -> Result<ByteArray, GetBytesException>,
 ) {
     suspend operator fun invoke(
         url: String,
         absoluteTargetPath: String,
-    ): Path {
+    ): Result<Path, GetBytesException> {
         val target = absoluteTargetPath.toPath()
         target.parent?.let { parent ->
             if (fileSystem.metadataOrNull(parent) == null) fileSystem.createDirectories(parent)
         }
-        val bytes = fetchBytes(url)
-        val existing = fileSystem.metadataOrNull(target)
-        if (existing?.size == bytes.size.toLong()) return target
-        fileSystem.sink(target).buffer().use { sink -> sink.write(bytes) }
-        return target
+        return fetchBytes(url).map { bytes ->
+            fileSystem.sink(target).buffer().use { sink -> sink.write(bytes) }
+            target
+        }
     }
 }
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/FetchGeoIpDbUpdates.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/FetchGeoIpDbUpdates.kt
@@ -13,41 +13,41 @@ import org.ooni.engine.models.Failure
 import org.ooni.engine.models.Result
 import org.ooni.engine.models.Success
 import org.ooni.engine.models.TaskOrigin
+import org.ooni.probe.data.models.GetBytesException
 import org.ooni.probe.data.models.SettingsKey
 import org.ooni.probe.data.repositories.PreferenceRepository
 import kotlin.time.Clock
 
 class FetchGeoIpDbUpdates(
-    private val downloadFile: suspend (url: String, absoluteTargetPath: String) -> Path,
+    private val downloadFile: suspend (url: String, absoluteTargetPath: String) -> Result<Path, GetBytesException>,
     private val cacheDir: String,
     private val engineHttpDo: suspend (method: String, url: String, taskOrigin: TaskOrigin) -> Result<String?, Engine.MkException>,
     private val preferencesRepository: PreferenceRepository,
     private val json: Json,
 ) {
     suspend operator fun invoke(): Result<Path?, Engine.MkException> =
-        try {
-            when (val versionRes = getLatestEngineVersion()) {
-                is Failure -> Failure(versionRes.reason)
-                is Success -> {
-                    val (isLatest, _, latestVersion) = isGeoIpDbLatest(versionRes.value)
-                    if (isLatest) {
-                        Success(null)
-                    } else {
-                        val versionName = latestVersion
-                        val url = buildGeoIpDbUrl(versionName)
-                        val target = "$cacheDir/$versionName.mmdb"
+        getLatestEngineVersion()
+            .onSuccess { version ->
+                val (isLatest, _, latestVersion) = isGeoIpDbLatest(version)
+                if (isLatest) {
+                    return Success(null)
+                } else {
+                    val versionName = latestVersion
+                    val url = buildGeoIpDbUrl(versionName)
+                    val target = "$cacheDir/$versionName.mmdb"
 
-                        downloadFile(url, target).let {
+                    downloadFile(url, target)
+                        .onSuccess { downloadedPath ->
                             preferencesRepository.setValueByKey(SettingsKey.MMDB_VERSION, versionName)
                             preferencesRepository.setValueByKey(SettingsKey.MMDB_LAST_CHECK, Clock.System.now().toEpochMilliseconds())
-                            Success(it)
+                            return Success(downloadedPath)
+                        }.onFailure { downloadError ->
+                            return Failure(Engine.MkException(downloadError))
                         }
-                    }
                 }
-            }
-        } catch (t: Throwable) {
-            Failure(t as? Engine.MkException ?: Engine.MkException(t))
-        }
+            }.onFailure { versionError ->
+                return Failure(versionError)
+            }.let { Failure(Engine.MkException(Throwable("Unexpected state"))) }
 
     /**
      * Compare latest and current version integers and return pair of latest state and actual version number
@@ -62,7 +62,7 @@ class FetchGeoIpDbUpdates(
     }
 
     private suspend fun getLatestEngineVersion(): Result<String, Engine.MkException> {
-        val url = "https://api.0.github.com/repos/aanorbel/oomplt-mmdb/releases/latest"
+        val url = "https://api.github.com/repos/aanorbel/oomplt-mmdb/releases/latest"
 
         return engineHttpDo("GET", url, TaskOrigin.OoniRun).map { payload ->
             val jsonStr = payload ?: throw Engine.MkException(Throwable("Empty body"))
diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/net/Http.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/net/Http.kt
@@ -1,7 +1,10 @@
 package org.ooni.probe.net
 
+import org.ooni.engine.models.Result
+import org.ooni.probe.data.models.GetBytesException
+
 /**
  * Perform a simple HTTP GET and return the raw response body bytes.
  * Implemented per-platform to ensure binary-safe downloads.
  */
-expect suspend fun httpGetBytes(url: String): ByteArray
+expect suspend fun httpGetBytes(url: String): Result<ByteArray, GetBytesException>
diff --git a/composeApp/src/desktopMain/kotlin/org/ooni/engine/DesktopOonimkallBridge.kt b/composeApp/src/desktopMain/kotlin/org/ooni/engine/DesktopOonimkallBridge.kt
@@ -66,7 +66,7 @@ class DesktopOonimkallBridge : OonimkallBridge {
             it.assetsDir = assetsDir
             // geoipDB may or may not exist in this binding; set via reflection when available
             geoIpDB?.let { path ->
-                // it.geoipDB = path
+                it.geoipDB = path
             }
             it.stateDir = stateDir
             it.tempDir = tempDir
diff --git a/composeApp/src/desktopMain/kotlin/org/ooni/probe/net/Http.desktop.kt b/composeApp/src/desktopMain/kotlin/org/ooni/probe/net/Http.desktop.kt
@@ -2,11 +2,15 @@ package org.ooni.probe.net
 
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
+import org.ooni.engine.models.Failure
+import org.ooni.engine.models.Result
+import org.ooni.engine.models.Success
+import org.ooni.probe.data.models.GetBytesException
 import java.io.BufferedInputStream
 import java.net.HttpURLConnection
 import java.net.URL
 
-actual suspend fun httpGetBytes(url: String): ByteArray =
+actual suspend fun httpGetBytes(url: String): Result<ByteArray, GetBytesException> =
     withContext(Dispatchers.IO) {
         val connection = (URL(url).openConnection() as HttpURLConnection)
         connection.requestMethod = "GET"
@@ -17,8 +21,13 @@ actual suspend fun httpGetBytes(url: String): ByteArray =
             val code = connection.responseCode
             val stream = if (code in 200..299) connection.inputStream else connection.errorStream
             val bytes = stream?.let { BufferedInputStream(it).use { bis -> bis.readBytes() } } ?: ByteArray(0)
-            if (code !in 200..299) throw RuntimeException("HTTP $code while GET $url: ${bytes.decodeToString()}")
-            bytes
+            if (code !in 200..299) {
+                Failure(GetBytesException(RuntimeException("HTTP $code while GET $url: ${bytes.decodeToString()}")))
+            } else {
+                Success(bytes)
+            }
+        } catch (e: Throwable) {
+            Failure(GetBytesException(e))
         } finally {
             connection.disconnect()
         }
diff --git a/composeApp/src/iosMain/kotlin/org/ooni/probe/net/Http.ios.kt b/composeApp/src/iosMain/kotlin/org/ooni/probe/net/Http.ios.kt
@@ -3,31 +3,34 @@ package org.ooni.probe.net
 import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlinx.cinterop.addressOf
 import kotlinx.cinterop.usePinned
+import org.ooni.engine.models.Failure
+import org.ooni.engine.models.Result
+import org.ooni.engine.models.Success
+import org.ooni.probe.data.models.GetBytesException
 import platform.Foundation.NSData
 import platform.Foundation.NSURL
 import platform.Foundation.NSURLSession
 import platform.Foundation.dataTaskWithURL
 import platform.posix.memcpy
 import kotlin.coroutines.resume
-import kotlin.coroutines.resumeWithException
 
-actual suspend fun httpGetBytes(url: String): ByteArray =
+actual suspend fun httpGetBytes(url: String): Result<ByteArray, GetBytesException> =
     suspendCancellableCoroutine { cont ->
         val nsurl = NSURL.URLWithString(url)!!
         val task = NSURLSession.sharedSession.dataTaskWithURL(nsurl) { data, response, error ->
             when {
-                error != null -> cont.resumeWithException(RuntimeException(error.localizedDescription))
+                error != null -> cont.resume(Failure(GetBytesException(RuntimeException(error.localizedDescription))))
                 data != null -> {
                     // If we have an HTTP response, check status code
                     val http = response as? platform.Foundation.NSHTTPURLResponse
                     val status = http?.statusCode?.toInt() ?: 200
                     if (status in 200..299) {
-                        cont.resume((data as NSData).toByteArray())
+                        cont.resume(Success((data as NSData).toByteArray()))
                     } else {
-                        cont.resumeWithException(RuntimeException("HTTP $status while GET $url"))
+                        cont.resume(Failure(GetBytesException(RuntimeException("HTTP $status while GET $url"))))
                     }
                 }
-                else -> cont.resume(ByteArray(0))
+                else -> cont.resume(Success(ByteArray(0)))
             }
         }
         cont.invokeOnCancellation { task.cancel() }

Original file line number	Diff line number	Diff line change
`@@ -120,8 +120,7 @@ fun App(`
`120`	`120`	`// dependencies.startSingleRunInner(RunSpecification.OnlyUploadMissingResults)`
`121`	`121`	`}`
`122`	`122`	`LaunchedEffect(Unit) {`
`123`		`- // Check for GeoIP DB updates in the background`
`124`		`- runCatching { dependencies.fetchGeoIpDbUpdates() }`
	`123`	`+ dependencies.fetchGeoIpDbUpdates()`
`125`	`124`	`}`
`126`	`125`	`LaunchedEffect(Unit) {`
`127`	`126`	`dependencies.observeAndConfigureAutoUpdate()`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,7 @@ class DesktopOonimkallBridge : OonimkallBridge {`
`66`	`66`	`it.assetsDir = assetsDir`
`67`	`67`	`// geoipDB may or may not exist in this binding; set via reflection when available`
`68`	`68`	`geoIpDB?.let { path ->`
`69`		`- // it.geoipDB = path`
	`69`	`+ it.geoipDB = path`
`70`	`70`	`}`
`71`	`71`	`it.stateDir = stateDir`
`72`	`72`	`it.tempDir = tempDir`