Keep old descriptor revisions

composeApp/src/androidMain/kotlin/org/ooni/probe/background/DescriptorUpdateWorker.kt

@@ -62,7 +62,7 @@ class DescriptorUpdateWorker(
             try {
                 val ids =
                     json.decodeFromString<List<InstalledTestDescriptorModel.Id>>(descriptorsJson)
-                return testDescriptorRepository.selectByRunIds(ids).first()
+                return testDescriptorRepository.listLatestByRunIds(ids).first()
             } catch (e: SerializationException) {
                 Logger.w("Could not start update worker: invalid configuration", e)
                 return null
@@ -71,7 +71,7 @@ class DescriptorUpdateWorker(
                 return null
             }
         }
-        return testDescriptorRepository.list().first()
+        return testDescriptorRepository.listLatest().first()
     }
 
     private suspend fun buildNotificationChannelIfNeeded() {

composeApp/src/commonMain/kotlin/org/ooni/engine/models/OONIRunDescriptor.kt

@@ -14,7 +14,7 @@ import org.ooni.probe.shared.toLocalDateTime
  */
 @Serializable
 data class OONIRunDescriptor(
-    @SerialName("oonirun_link_id") val oonirunLinkId: Long,
+    @SerialName("oonirun_link_id") val oonirunLinkId: String,
     @SerialName("name") val name: String,
     @SerialName("short_description") val shortDescription: String,
     @SerialName("description") val description: String,
@@ -30,6 +30,7 @@ data class OONIRunDescriptor(
     @SerialName("date_created") val dateCreated: Instant,
     @SerialName("date_updated") val dateUpdated: Instant,
     @SerialName("is_expired") val isExpired: Boolean,
+    @SerialName("revision") val revision: Long,
 )
 
 @Serializable
@@ -40,6 +41,7 @@ class OONIRunRevisions(
 fun OONIRunDescriptor.toModel() =
     InstalledTestDescriptorModel(
         id = InstalledTestDescriptorModel.Id(oonirunLinkId),
+        revision = revision,
         name = name,
         shortDescription = shortDescription,
         description = description,

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

@@ -50,6 +50,4 @@ data class Descriptor(
             .seconds
 }
 
-fun List<Descriptor>.runnableDescriptors(): List<Descriptor> {
-    return filter { !it.isExpired }
-}
+fun List<Descriptor>.notExpired() = filter { !it.isExpired }

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

@@ -22,13 +22,13 @@ import org.ooni.engine.models.SummaryType
 import org.ooni.probe.data.TestDescriptor
 import org.ooni.probe.shared.InstalledDescriptorIcons
 import org.ooni.probe.shared.hexToColor
-import org.ooni.probe.shared.now
 import org.ooni.probe.shared.stringMonthArrayResource
 import org.ooni.probe.shared.toEpoch
 
 @Serializable
 data class InstalledTestDescriptorModel(
     val id: Id,
+    val revision: Long,
     val name: String,
     val shortDescription: String?,
     val description: String?,
@@ -48,10 +48,16 @@ data class InstalledTestDescriptorModel(
 ) {
     @Serializable
     data class Id(
-        val value: Long,
+        val value: String,
     )
 
-    val isExpired get() = expirationDate != null && expirationDate < LocalDateTime.now()
+    @Serializable
+    data class Key(
+        val id: Id,
+        val revision: Long,
+    )
+
+    val key get() = Key(id, revision)
 
     fun shouldUpdate(other: InstalledTestDescriptorModel): Boolean {
         return dateUpdated != null && other.dateUpdated != null && other.dateUpdated > dateUpdated
@@ -72,9 +78,10 @@ fun InstalledTestDescriptorModel.toDescriptor(updateStatus: UpdateStatus = Updat
                     Res.string.Dashboard_Runv2_Overview_Description,
                     author.orEmpty(),
                     formattedDateCreated,
-                ) + ". " + formattedDate(dateUpdated)?.let {
-                    stringResource(Res.string.Dashboard_Runv2_Overview_LastUpdated, it)
-                }
+                ) + ". " +
+                    formattedDate(dateUpdated)?.let {
+                        stringResource(Res.string.Dashboard_Runv2_Overview_LastUpdated, it)
+                    }
             }
         },
         icon = icon?.let(InstalledDescriptorIcons::getIconFromValue),
@@ -104,13 +111,14 @@ private fun dateTimeFormat(monthNames: List<String>) =
         monthName(MonthNames(monthNames))
         char(' ')
         dayOfMonth()
-        chars(" , ")
+        chars(", ")
         year()
     }
 
-fun InstalledTestDescriptorModel.toDb(json: Json): TestDescriptor {
-    return TestDescriptor(
+fun InstalledTestDescriptorModel.toDb(json: Json) =
+    TestDescriptor(
         runId = id.value,
+        revision = revision,
         name = name,
         short_description = shortDescription,
         description = description,
@@ -127,14 +135,11 @@ fun InstalledTestDescriptorModel.toDb(json: Json): TestDescriptor {
         expiration_date = expirationDate?.toEpoch(),
         date_created = dateCreated?.toEpoch(),
         date_updated = dateUpdated?.toEpoch(),
-        revision = try {
+        auto_update = if (autoUpdate) 1 else 0,
+        revisions = try {
             json.encodeToString(revisions)
         } catch (e: Exception) {
             Logger.e(e) { "Failed to encode revisions" }
             null
         },
-        previous_revision = null,
-        is_expired = if (isExpired) 1 else 0,
-        auto_update = if (autoUpdate) 1 else 0,
     )
-}

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

@@ -6,7 +6,6 @@ import org.ooni.probe.shared.now
 
 data class ResultModel(
     val id: Id? = null,
-    val testGroupName: String?,
     val startTime: LocalDateTime = LocalDateTime.now(),
     val isViewed: Boolean = false,
     val isDone: Boolean = false,
@@ -15,7 +14,8 @@ data class ResultModel(
     val failureMessage: String? = null,
     val taskOrigin: TaskOrigin,
     val networkId: NetworkModel.Id? = null,
-    val testDescriptorId: InstalledTestDescriptorModel.Id?,
+    val descriptorName: String?,
+    val descriptorKey: InstalledTestDescriptorModel.Key?,
 ) {
     data class Id(
         val value: Long,

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

@@ -25,7 +25,7 @@ data class TestKeysWithResultId(
     val testName: String?,
     val testKeys: TestKeys?,
     val resultId: ResultModel.Id,
-    val testGroupName: String?,
+    val descriptorName: String?,
     val descriptorRunId: InstalledTestDescriptorModel.Id?,
 )
 

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

@@ -166,7 +166,7 @@ class MeasurementRepository(
             resultId = result_id?.let(ResultModel::Id) ?: return null,
             testName = test_name,
             testKeys = test_keys?.let { json.decodeFromString<TestKeys>(it) },
-            testGroupName = test_group_name,
+            descriptorName = descriptor_name,
             descriptorRunId = descriptor_runId?.let(InstalledTestDescriptorModel::Id),
         )
     }

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

@@ -76,7 +76,7 @@ class ResultRepository(
             database.transactionWithResult {
                 database.resultQueries.insertOrReplace(
                     id = model.id?.value,
-                    test_group_name = model.testGroupName,
+                    descriptor_name = model.descriptorName,
                     start_time = model.startTime.toEpoch(),
                     is_viewed = if (model.isViewed) 1 else 0,
                     is_done = if (model.isDone) 1 else 0,
@@ -85,7 +85,8 @@ class ResultRepository(
                     failure_msg = model.failureMessage,
                     task_origin = model.taskOrigin.value,
                     network_id = model.networkId?.value,
-                    descriptor_runId = model.testDescriptorId?.value,
+                    descriptor_runId = model.descriptorKey?.id?.value,
+                    descriptor_revision = model.descriptorKey?.revision,
                 )
                 model.id
                     ?: ResultModel.Id(
@@ -118,9 +119,9 @@ class ResultRepository(
             database.resultQueries.markAllAsDone()
         }
 
-    suspend fun deleteByRunId(resultId: InstalledTestDescriptorModel.Id) =
+    suspend fun deleteByRunId(descriptorId: InstalledTestDescriptorModel.Id) =
         withContext(backgroundContext) {
-            database.resultQueries.deleteByRunId(resultId.value)
+            database.resultQueries.deleteByRunId(descriptorId.value)
         }
 
     suspend fun deleteAll() {
@@ -136,7 +137,7 @@ class ResultRepository(
     private fun Result.toModel(): ResultModel? {
         return ResultModel(
             id = ResultModel.Id(id),
-            testGroupName = test_group_name,
+            descriptorName = descriptor_name,
             startTime = start_time?.toLocalDateTime() ?: return null,
             isViewed = is_viewed == 1L,
             isDone = is_done == 1L,
@@ -145,15 +146,23 @@ class ResultRepository(
             failureMessage = failure_msg,
             taskOrigin = TaskOrigin.fromValue(task_origin),
             networkId = network_id?.let(NetworkModel::Id),
-            testDescriptorId = descriptor_runId?.let(InstalledTestDescriptorModel::Id),
+            descriptorKey = descriptor_runId?.let {
+                descriptor_revision?.let {
+                    InstalledTestDescriptorModel.Key(
+                        // TODO: Convert descriptor_runId to TEXT
+                        id = InstalledTestDescriptorModel.Id(descriptor_runId.toString()),
+                        revision = descriptor_revision,
+                    )
+                }
+            },
         )
     }
 
     private fun SelectAllWithNetwork.toModel(): ResultWithNetworkAndAggregates? {
         return ResultWithNetworkAndAggregates(
             result = Result(
                 id = id ?: return null,
-                test_group_name = test_group_name,
+                descriptor_name = descriptor_name,
                 start_time = start_time,
                 is_viewed = is_viewed,
                 is_done = is_done,
@@ -163,6 +172,7 @@ class ResultRepository(
                 task_origin = task_origin,
                 network_id = network_id,
                 descriptor_runId = descriptor_runId,
+                descriptor_revision = descriptor_revision,
             ).toModel() ?: return null,
             network = network_id_inner?.let { networkId ->
                 Network(
@@ -187,7 +197,7 @@ class ResultRepository(
         return Pair(
             Result(
                 id = id,
-                test_group_name = test_group_name,
+                descriptor_name = descriptor_name,
                 start_time = start_time,
                 is_viewed = is_viewed,
                 is_done = is_done,
@@ -197,6 +207,7 @@ class ResultRepository(
                 task_origin = task_origin,
                 network_id = network_id,
                 descriptor_runId = descriptor_runId,
+                descriptor_revision = descriptor_revision,
             ).toModel() ?: return null,
             id_?.let { networkId ->
                 Network(

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

@@ -2,6 +2,7 @@ package org.ooni.probe.data.repositories
 
 import app.cash.sqldelight.coroutines.asFlow
 import app.cash.sqldelight.coroutines.mapToList
+import co.touchlab.kermit.Logger
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.withContext
 import kotlinx.serialization.json.Json
@@ -19,19 +20,27 @@ class TestDescriptorRepository(
     private val json: Json,
     private val backgroundContext: CoroutineContext,
 ) {
-    fun list() =
+    // Warning: this list will bring duplicated descriptors of different revisions
+    fun listAll() =
         database.testDescriptorQueries
             .selectAll()
             .asFlow()
             .mapToList(backgroundContext)
-            .map { list -> list.mapNotNull { it.toModel() } }
+            .map { list -> list.map { it.toModel() } }
 
-    fun selectByRunIds(ids: List<InstalledTestDescriptorModel.Id>) =
+    fun listLatest() =
         database.testDescriptorQueries
-            .selectByRunIds(ids.map { it.value })
+            .selectLatest()
             .asFlow()
             .mapToList(backgroundContext)
-            .map { list -> list.mapNotNull { it.toModel() } }
+            .map { list -> list.map { it.toModel() } }
+
+    fun listLatestByRunIds(ids: List<InstalledTestDescriptorModel.Id>) =
+        database.testDescriptorQueries
+            .selectLatestByRunIds(ids.map { it.value })
+            .asFlow()
+            .mapToList(backgroundContext)
+            .map { list -> list.map { it.toModel() } }
 
     suspend fun createOrIgnore(models: List<InstalledTestDescriptorModel>) {
         withContext(backgroundContext) {
@@ -40,6 +49,7 @@ class TestDescriptorRepository(
                     val installedModel = model.toDb(json = json)
                     database.testDescriptorQueries.insertOrIgnore(
                         runId = installedModel.runId,
+                        revision = installedModel.revision,
                         name = installedModel.name,
                         short_description = installedModel.short_description,
                         description = installedModel.description,
@@ -54,10 +64,8 @@ class TestDescriptorRepository(
                         expiration_date = installedModel.expiration_date,
                         date_created = installedModel.date_created,
                         date_updated = installedModel.date_updated,
-                        revision = installedModel.revision,
-                        previous_revision = null,
-                        is_expired = installedModel.is_expired,
                         auto_update = installedModel.auto_update,
+                        revisions = installedModel.revisions,
                     )
                 }
             }
@@ -71,6 +79,7 @@ class TestDescriptorRepository(
                     val installedModel = model.toDb(json = json)
                     database.testDescriptorQueries.createOrUpdate(
                         runId = installedModel.runId,
+                        revision = installedModel.revision,
                         name = installedModel.name,
                         short_description = installedModel.short_description,
                         description = installedModel.description,
@@ -85,10 +94,8 @@ class TestDescriptorRepository(
                         expiration_date = installedModel.expiration_date,
                         date_created = installedModel.date_created,
                         date_updated = installedModel.date_updated,
-                        revision = installedModel.revision,
-                        previous_revision = null,
-                        is_expired = installedModel.is_expired,
                         auto_update = installedModel.auto_update,
+                        revisions = installedModel.revisions,
                     )
                 }
             }
@@ -100,7 +107,10 @@ class TestDescriptorRepository(
         autoUpdate: Boolean,
     ) {
         withContext(backgroundContext) {
-            database.testDescriptorQueries.setAutoUpdate(auto_update = if (autoUpdate) 1 else 0, runId = runId.value)
+            database.testDescriptorQueries.setAutoUpdate(
+                auto_update = if (autoUpdate) 1 else 0,
+                runId = runId.value,
+            )
         }
     }
 
@@ -110,9 +120,10 @@ class TestDescriptorRepository(
         }
     }
 
-    private fun TestDescriptor.toModel(): InstalledTestDescriptorModel? {
-        return InstalledTestDescriptorModel(
-            id = runId?.let(InstalledTestDescriptorModel::Id) ?: return null,
+    private fun TestDescriptor.toModel() =
+        InstalledTestDescriptorModel(
+            id = InstalledTestDescriptorModel.Id(runId),
+            revision = revision,
             name = name.orEmpty(),
             shortDescription = short_description,
             description = description,
@@ -129,15 +140,14 @@ class TestDescriptorRepository(
             expirationDate = expiration_date?.toLocalDateTime(),
             dateCreated = date_created?.toLocalDateTime(),
             dateUpdated = date_updated?.toLocalDateTime(),
-            revisions = revision?.let {
+            autoUpdate = auto_update == 1L,
+            revisions = revisions?.let {
                 try {
                     json.decodeFromString<List<String>>(it)
                 } catch (e: Exception) {
-                    // Handle the exception, e.g., log it or return a default value
+                    Logger.e(e) { "Failed to decode revisions" }
                     emptyList()
                 }
             },
-            autoUpdate = auto_update == 1L,
         )
-    }
 }