From 5e51d6bd947fccf63469cfe761b187a3679d12fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Santos?= Date: Wed, 15 Jan 2025 18:01:35 +0000 Subject: [PATCH 1/2] Minor descriptor updates don't need review --- .../data/models/DescriptorUpdatesStatus.kt | 23 ++-- .../data/repositories/ResultRepository.kt | 3 +- .../repositories/TestDescriptorRepository.kt | 9 +- .../probe/domain/FetchDescriptorUpdate.kt | 105 ++++++++---------- .../probe/domain/FetchDescriptorUpdateTest.kt | 27 +++++ 5 files changed, 93 insertions(+), 74 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/data/models/DescriptorUpdatesStatus.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/data/models/DescriptorUpdatesStatus.kt index 0825dbf8..975a6d4f 100644 --- a/composeApp/src/commonMain/kotlin/org/ooni/probe/data/models/DescriptorUpdatesStatus.kt +++ b/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 = emptyList(), val autoUpdated: List = emptyList(), val rejectedUpdates: List = emptyList(), val reviewUpdates: List = emptyList(), - val errors: List = 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 } } } diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/ResultRepository.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/ResultRepository.kt index d121b858..e75afed4 100644 --- a/composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/ResultRepository.kt +++ b/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, ) } diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/TestDescriptorRepository.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/TestDescriptorRepository.kt index 27d58144..9dd4a09e 100644 --- a/composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/TestDescriptorRepository.kt +++ b/composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/TestDescriptorRepository.kt @@ -20,7 +20,11 @@ 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() @@ -28,6 +32,9 @@ class TestDescriptorRepository( .mapToList(backgroundContext) .map { list -> list.map { it.toModel() } } + /** + * Lists the latest revision of every installed descriptor in the database. + */ fun listLatest() = database.testDescriptorQueries .selectLatest() diff --git a/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/FetchDescriptorUpdate.kt b/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/FetchDescriptorUpdate.kt index aa7abc7c..672dacf3 100644 --- a/composeApp/src/commonMain/kotlin/org/ooni/probe/domain/FetchDescriptorUpdate.kt +++ b/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,14 +9,14 @@ 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, + private val fetchDescriptor: suspend (descriptorId: String) -> Result, private val saveTestDescriptors: suspend (List, SaveTestDescriptors.Mode) -> Unit, private val listInstalledTestDescriptors: () -> Flow>, ) { @@ -23,81 +24,73 @@ class FetchDescriptorUpdate( fun observeStatus() = status.asStateFlow() - suspend fun invoke( - descriptors: List, - ): Map>> { + suspend fun invoke(descriptors: List) { 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() - val resultsMap = mutableMapOf>>() - 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() + val updatesToReview = mutableListOf() + val autoUpdates = mutableListOf() + + 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 = resultsMap[ResultStatus.UpdatesAvailable]?.mapNotNull { result -> - result.get() - }.orEmpty() - val autoUpdated: List = 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) { 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) { 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, -} diff --git a/composeApp/src/commonTest/kotlin/org/ooni/probe/domain/FetchDescriptorUpdateTest.kt b/composeApp/src/commonTest/kotlin/org/ooni/probe/domain/FetchDescriptorUpdateTest.kt index 8cdbac83..2264560e 100644 --- a/composeApp/src/commonTest/kotlin/org/ooni/probe/domain/FetchDescriptorUpdateTest.kt +++ b/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? = 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 { From cc7801eb8229b201536394864bee746addad62d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Santos?= Date: Mon, 20 Jan 2025 15:14:29 +0000 Subject: [PATCH 2/2] Address android lint issues --- .../ooni/probe/ui/shared/AndroidCalculateWindowSizeClass.kt | 4 ++-- gradle/libs.versions.toml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/composeApp/src/androidMain/kotlin/org/ooni/probe/ui/shared/AndroidCalculateWindowSizeClass.kt b/composeApp/src/androidMain/kotlin/org/ooni/probe/ui/shared/AndroidCalculateWindowSizeClass.kt index 28dc9041..fcd51a66 100644 --- a/composeApp/src/androidMain/kotlin/org/ooni/probe/ui/shared/AndroidCalculateWindowSizeClass.kt +++ b/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) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index e15da002..0a07cfa2 100644 --- a/gradle/libs.versions.toml +++ b/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" }