Do not review minor descriptor updates (no revision number change)

composeApp/src/androidMain/kotlin/org/ooni/probe/ui/shared/AndroidCalculateWindowSizeClass.kt

@@ -1,10 +1,10 @@
 package org.ooni.probe.ui.shared
 
 import android.app.Activity
+import androidx.activity.compose.LocalActivity
 import androidx.compose.material3.windowsizeclass.WindowSizeClass
 import androidx.compose.material3.windowsizeclass.calculateWindowSizeClass
 import androidx.compose.runtime.Composable
-import androidx.compose.ui.platform.LocalContext
 
 @Composable
-actual fun calculateWindowSizeClass(): WindowSizeClass = calculateWindowSizeClass(LocalContext.current as Activity)
+actual fun calculateWindowSizeClass(): WindowSizeClass = calculateWindowSizeClass(LocalActivity.current as Activity)

composeApp/src/commonMain/kotlin/org/ooni/probe/data/models/DescriptorUpdatesStatus.kt

@@ -1,13 +1,10 @@
 package org.ooni.probe.data.models
 
-import org.ooni.engine.Engine
-
 data class DescriptorUpdatesStatus(
     val availableUpdates: List<InstalledTestDescriptorModel> = emptyList(),
     val autoUpdated: List<InstalledTestDescriptorModel> = emptyList(),
     val rejectedUpdates: List<InstalledTestDescriptorModel> = emptyList(),
     val reviewUpdates: List<InstalledTestDescriptorModel> = emptyList(),
-    val errors: List<Engine.MkException> = emptyList(),
     val refreshType: UpdateStatusType = UpdateStatusType.None,
 ) {
     private val availableUpdatesMap = availableUpdates.associateBy { it.id.value }
@@ -16,15 +13,17 @@ data class DescriptorUpdatesStatus(
 
     fun getStatusOf(id: InstalledTestDescriptorModel.Id): UpdateStatus {
         return when {
-            autoUpdatesMap.containsKey(id.value) -> UpdateStatus.AutoUpdated
-            rejectedUpdatesMap.containsKey(id.value) -> UpdateStatus.UpdateRejected(
-                rejectedUpdatesMap[id.value]!!,
-            )
-            availableUpdatesMap.containsKey(id.value) -> {
-                val update = availableUpdatesMap[id.value]!!
-                if (update.autoUpdate) UpdateStatus.AutoUpdated else UpdateStatus.Updatable(update)
-            }
-            else -> UpdateStatus.UpToDate
+            autoUpdatesMap.containsKey(id.value) ->
+                UpdateStatus.AutoUpdated
+
+            rejectedUpdatesMap.containsKey(id.value) ->
+                UpdateStatus.UpdateRejected(rejectedUpdatesMap[id.value]!!)
+
+            availableUpdatesMap.containsKey(id.value) ->
+                UpdateStatus.Updatable(availableUpdatesMap[id.value]!!)
+
+            else ->
+                UpdateStatus.UpToDate
         }
     }
 }

composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/ResultRepository.kt

@@ -149,8 +149,7 @@ class ResultRepository(
             descriptorKey = descriptor_runId?.let {
                 descriptor_revision?.let {
                     InstalledTestDescriptorModel.Key(
-                        // TODO: Convert descriptor_runId to TEXT
-                        id = InstalledTestDescriptorModel.Id(descriptor_runId.toString()),
+                        id = InstalledTestDescriptorModel.Id(descriptor_runId),
                         revision = descriptor_revision,
                     )
                 }

composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/TestDescriptorRepository.kt

@@ -20,14 +20,21 @@ class TestDescriptorRepository(
     private val json: Json,
     private val backgroundContext: CoroutineContext,
 ) {
-    // Warning: this list will bring duplicated descriptors of different revisions
+    /**
+     * Lists all entries in the database.
+     *
+     * Warning: this list will bring duplicated descriptors of different revisions.
+     */
     fun listAll() =
         database.testDescriptorQueries
             .selectAll()
             .asFlow()
             .mapToList(backgroundContext)
             .map { list -> list.map { it.toModel() } }
 
+    /**
+     * Lists the latest revision of every installed descriptor in the database.
+     */
     fun listLatest() =
         database.testDescriptorQueries
             .selectLatest()

composeApp/src/commonMain/kotlin/org/ooni/probe/domain/FetchDescriptorUpdate.kt

@@ -1,5 +1,6 @@
 package org.ooni.probe.domain
 
+import co.touchlab.kermit.Logger
 import kotlinx.coroutines.async
 import kotlinx.coroutines.awaitAll
 import kotlinx.coroutines.coroutineScope
@@ -8,96 +9,88 @@ import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.update
-import org.ooni.engine.Engine.MkException
+import org.ooni.engine.Engine
 import org.ooni.engine.models.Result
 import org.ooni.probe.data.models.DescriptorUpdatesStatus
 import org.ooni.probe.data.models.InstalledTestDescriptorModel
 import org.ooni.probe.data.models.UpdateStatusType
 
 class FetchDescriptorUpdate(
-    private val fetchDescriptor: suspend (descriptorId: String) -> Result<InstalledTestDescriptorModel?, MkException>,
+    private val fetchDescriptor: suspend (descriptorId: String) -> Result<InstalledTestDescriptorModel?, Engine.MkException>,
     private val saveTestDescriptors: suspend (List<InstalledTestDescriptorModel>, SaveTestDescriptors.Mode) -> Unit,
     private val listInstalledTestDescriptors: () -> Flow<List<InstalledTestDescriptorModel>>,
 ) {
     private val status = MutableStateFlow(DescriptorUpdatesStatus())
 
     fun observeStatus() = status.asStateFlow()
 
-    suspend fun invoke(
-        descriptors: List<InstalledTestDescriptorModel>,
-    ): Map<ResultStatus, MutableList<Result<InstalledTestDescriptorModel?, MkException>>> {
+    suspend fun invoke(descriptors: List<InstalledTestDescriptorModel>) {
         status.update { _ ->
             DescriptorUpdatesStatus(
                 refreshType = UpdateStatusType.FetchingUpdates,
             )
         }
-        val response = coroutineScope {
+        val fetchResults = coroutineScope {
             descriptors.map { descriptor ->
-                async {
-                    Pair(descriptor, fetchDescriptor(descriptor.id.value))
-                }
+                async { descriptor to fetchDescriptor(descriptor.id.value) }
             }.awaitAll()
         }
-        val autoUpdateItems = mutableListOf<InstalledTestDescriptorModel>()
-        val resultsMap = mutableMapOf<ResultStatus, MutableList<Result<InstalledTestDescriptorModel?, MkException>>>()
-        ResultStatus.entries.forEach { resultsMap[it] = mutableListOf() }
-        response.forEach { (descriptor, result) ->
-            val status: ResultStatus = if (descriptor.autoUpdate) {
-                var res: ResultStatus = ResultStatus.NoUpdates
-                result.map { updatedDescriptor ->
-                    updatedDescriptor?.let {
-                        if (descriptor.shouldUpdate(it)) {
-                            autoUpdateItems.add(it.copy(autoUpdate = descriptor.autoUpdate))
-                            res = ResultStatus.AutoUpdated
-                        }
-                    }
+
+        val minorUpdates = mutableListOf<InstalledTestDescriptorModel>()
+        val updatesToReview = mutableListOf<InstalledTestDescriptorModel>()
+        val autoUpdates = mutableListOf<InstalledTestDescriptorModel>()
+
+        fetchResults.forEach { (descriptor, fetchResult) ->
+            val newDescriptor = fetchResult.get()
+                ?.copy(autoUpdate = descriptor.autoUpdate)
+                ?: run {
+                    Logger.w("Failed to fetch update", fetchResult.getError())
+                    return@forEach
                 }
-                res
-            } else {
-                var res: ResultStatus = ResultStatus.NoUpdates
-                result.map { updatedDescriptor ->
-                    updatedDescriptor?.let {
-                        if (descriptor.shouldUpdate(updatedDescriptor)) {
-                            res = ResultStatus.UpdatesAvailable
-                        }
-                    }
+
+            val newUpdate = newDescriptor.dateUpdated != null && (
+                descriptor.dateUpdated == null || descriptor.dateUpdated < newDescriptor.dateUpdated
+            )
+            if (!newUpdate) return@forEach
+
+            if (newDescriptor.revision > descriptor.revision) {
+                // Major update
+                if (descriptor.autoUpdate) {
+                    autoUpdates += newDescriptor
+                } else {
+                    updatesToReview += newDescriptor
                 }
-                res
+            } else {
+                minorUpdates += newDescriptor
             }
-            resultsMap[status]?.add(result.map { it?.copy(autoUpdate = descriptor.autoUpdate) })
         }
 
-        saveTestDescriptors(autoUpdateItems, SaveTestDescriptors.Mode.CreateOrUpdate)
+        saveTestDescriptors(minorUpdates + autoUpdates, SaveTestDescriptors.Mode.CreateOrUpdate)
 
-        val updatesAvailable: List<InstalledTestDescriptorModel> = resultsMap[ResultStatus.UpdatesAvailable]?.mapNotNull { result ->
-            result.get()
-        }.orEmpty()
-        val autoUpdated: List<InstalledTestDescriptorModel> = resultsMap[ResultStatus.AutoUpdated]?.mapNotNull { result ->
-            result.get()
-        }.orEmpty()
         status.update { _ ->
             DescriptorUpdatesStatus(
-                availableUpdates = updatesAvailable,
-                autoUpdated = autoUpdated,
-                errors = response.mapNotNull { (_, result) -> result.getError() },
-                refreshType = if (updatesAvailable.isNotEmpty()) UpdateStatusType.ReviewLink else UpdateStatusType.None,
+                availableUpdates = updatesToReview,
+                autoUpdated = autoUpdates,
+                refreshType = if (updatesToReview.isNotEmpty()) {
+                    UpdateStatusType.ReviewLink
+                } else {
+                    UpdateStatusType.None
+                },
             )
         }
-        return resultsMap
     }
 
     suspend operator fun invoke() {
-        listInstalledTestDescriptors().first().let { items ->
-            if (items.isNotEmpty()) {
-                invoke(items)
-            }
+        val items = listInstalledTestDescriptors().first()
+        if (items.isNotEmpty()) {
+            invoke(items)
         }
     }
 
     fun cancelUpdates(descriptors: List<InstalledTestDescriptorModel>) {
         status.update { currentItems ->
             currentItems.copy(
-                availableUpdates = currentItems.availableUpdates - descriptors,
+                availableUpdates = currentItems.availableUpdates - descriptors.toSet(),
                 rejectedUpdates = currentItems.availableUpdates + descriptors,
                 refreshType = UpdateStatusType.None,
             )
@@ -116,16 +109,10 @@ class FetchDescriptorUpdate(
     fun markAsUpdated(items: List<InstalledTestDescriptorModel>) {
         status.update { status ->
             status.copy(
-                availableUpdates = status.availableUpdates - items,
-                rejectedUpdates = status.rejectedUpdates - items,
-                reviewUpdates = status.reviewUpdates - items,
+                availableUpdates = status.availableUpdates - items.toSet(),
+                rejectedUpdates = status.rejectedUpdates - items.toSet(),
+                reviewUpdates = status.reviewUpdates - items.toSet(),
             )
         }
     }
 }
-
-enum class ResultStatus {
-    AutoUpdated,
-    NoUpdates,
-    UpdatesAvailable,
-}

composeApp/src/commonTest/kotlin/org/ooni/probe/domain/FetchDescriptorUpdateTest.kt

@@ -63,6 +63,33 @@ class FetchDescriptorUpdateTest {
             assertEquals(listOf(newDescriptor), saveDescriptors)
         }
 
+    @Test
+    fun minorUpdate() =
+        runTest {
+            val oldDescriptor = DescriptorFactory.buildInstalledModel(
+                autoUpdate = false,
+                dateUpdated = Clock.System.now().minus(1.days).toLocalDateTime(),
+            )
+            val newDescriptor = oldDescriptor.copy(
+                dateUpdated = LocalDateTime.now(),
+            )
+            var saveDescriptors: List<InstalledTestDescriptorModel>? = null
+            val subject = FetchDescriptorUpdate(
+                fetchDescriptor = { Success(newDescriptor) },
+                saveTestDescriptors = { list, _ -> saveDescriptors = list },
+                listInstalledTestDescriptors = { flowOf(listOf(oldDescriptor)) },
+            )
+
+            subject()
+
+            val state = subject.observeStatus().value
+
+            assertEquals(0, state.availableUpdates.size)
+            assertEquals(0, state.autoUpdated.size)
+            assertEquals(UpdateStatusType.None, state.refreshType)
+            assertEquals(listOf(newDescriptor), saveDescriptors)
+        }
+
     @Test
     fun updatesAvailableAndReview() =
         runTest {

gradle/libs.versions.toml

@@ -9,7 +9,7 @@ android-targetSdk = "35"
 compose-plugin = "1.7.0"
 kotlin = "2.1.0"
 sqldelight = "2.0.2"
-dataStoreVersion = "1.1.1"
+dataStoreVersion = "1.1.2"
 junitKtx = "1.2.1"
 
 [plugins]
@@ -31,7 +31,7 @@ kotlin-serialization = { module = "org.jetbrains.kotlinx:kotlinx-serialization-j
 kotlin-datetime = { module = "org.jetbrains.kotlinx:kotlinx-datetime", version = "0.6.0" }
 
 # UI
-android-activity = { module = "androidx.activity:activity-ktx", version = "1.9.3" }
+android-activity = { module = "androidx.activity:activity-ktx", version = "1.10.0" }
 android-work = { module = "androidx.work:work-runtime-ktx", version = "2.10.0" }
 
 lifecycle-viewmodel-compose = { module = "org.jetbrains.androidx.lifecycle:lifecycle-viewmodel-compose", version = "2.8.0" }