Skip to content

Commit

Permalink
Merge pull request #390 from ooni/do-not-review-minor-updates
Browse files Browse the repository at this point in the history 
Do not review minor descriptor updates (no revision number change)
  • Loading branch information
@sdsantos
sdsantos authored Jan 20, 2025
2 parents 336fe3b + cc7801e commit d9c13cd
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 78 deletions.
4 changes: 2 additions & 2 deletions ...oseApp/src/androidMain/kotlin/org/ooni/probe/ui/shared/AndroidCalculateWindowSizeClass.kt
View file
Original file line number Diff line number Diff line change
@@ -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)
23 changes: 11 additions & 12 deletions composeApp/src/commonMain/kotlin/org/ooni/probe/data/models/DescriptorUpdatesStatus.kt
View file
Original file line number Diff line number Diff line change
@@ -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 }
Expand All @@ -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
}
}
}
3 changes: 1 addition & 2 deletions composeApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/ResultRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}
Expand Down
9 changes: 8 additions & 1 deletion ...oseApp/src/commonMain/kotlin/org/ooni/probe/data/repositories/TestDescriptorRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
105 changes: 46 additions & 59 deletions composeApp/src/commonMain/kotlin/org/ooni/probe/domain/FetchDescriptorUpdate.kt
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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,
)
Expand All @@ -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,
}
27 changes: 27 additions & 0 deletions composeApp/src/commonTest/kotlin/org/ooni/probe/domain/FetchDescriptorUpdateTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions gradle/libs.versions.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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" }
Expand Down

0 comments on commit d9c13cd

Please sign in to comment.