Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ooni test descriptors. #275

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,31 @@ data class Descriptor(
val expirationDate: LocalDateTime?,
val netTests: List<NetTest>,
val longRunningTests: List<NetTest> = emptyList(),
val source: Source,
val source: InstalledTestDescriptorModel,
val updateStatus: UpdateStatus,
) {
sealed interface Source {
data class Default(val value: DefaultTestDescriptor) : Source

data class Installed(val value: InstalledTestDescriptorModel) : Source
}

val isExpired get() = expirationDate != null && expirationDate < LocalDateTime.now()

val updatable get() = updateStatus is UpdateStatus.UpdateRejected

val key: String
get() = when (source) {
is Source.Default -> name
is Source.Installed -> source.value.id.value.toString()
}
get() = source.key

val allTests get() = netTests + longRunningTests

val estimatedDuration
get() = allTests
.sumOf { it.test.runtime(it.inputs).inWholeSeconds }
.seconds

val settingsPrefix: String
get() = source.id.value.toString()

fun isDefaultDescriptor(): Boolean {
return source.isDefaultTestDescriptor
}

fun isInstalledNonDefaultDescriptor(): Boolean {
return !source.isDefaultTestDescriptor
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@ import kotlinx.serialization.json.Json
import ooniprobe.composeapp.generated.resources.Dashboard_Runv2_Overview_Description
import ooniprobe.composeapp.generated.resources.Dashboard_Runv2_Overview_LastUpdated
import ooniprobe.composeapp.generated.resources.Res
import ooniprobe.composeapp.generated.resources.TestResults_NotAvailable
import ooniprobe.composeapp.generated.resources.performance_datausage
import ooniprobe.composeapp.generated.resources.small_datausage
import ooniprobe.composeapp.generated.resources.test_circumvention
import ooniprobe.composeapp.generated.resources.test_experimental
import ooniprobe.composeapp.generated.resources.test_instant_messaging
import ooniprobe.composeapp.generated.resources.test_performance
import ooniprobe.composeapp.generated.resources.test_websites
import ooniprobe.composeapp.generated.resources.websites_datausage
import org.jetbrains.compose.resources.StringResource
import org.jetbrains.compose.resources.stringResource
import org.ooni.probe.data.TestDescriptor
import org.ooni.probe.shared.InstalledDescriptorIcons
Expand Down Expand Up @@ -52,6 +57,21 @@ data class InstalledTestDescriptorModel(

val isExpired get() = expirationDate != null && expirationDate < LocalDateTime.now()

val isDefaultTestDescriptor get() = id.value in 10470..10474 // TODO(aanorbel): switch to OONI reserved namespace
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also check if they are in OoniTest now.


val key get() = if (isDefaultTestDescriptor) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more like a settings key now no? Just to make it clear why we still need this. Because for all other purposes now we can use the ID.

when (id.value) {
10470L -> "websites"
10471L -> "instant_messaging"
10472L -> "circumvention"
10473L -> "performance"
10474L -> "experimental"
else -> id.value.toString()
}
} else {
id.value.toString()
}

fun shouldUpdate(other: InstalledTestDescriptorModel): Boolean {
return dateUpdated != null && other.dateUpdated != null && other.dateUpdated > dateUpdated
}
Expand Down Expand Up @@ -79,13 +99,24 @@ fun InstalledTestDescriptorModel.toDescriptor(updateStatus: UpdateStatus = Updat
icon = icon?.let(InstalledDescriptorIcons::getIconFromValue),
color = color?.hexToColor(),
animation = icon?.let { determineAnimation(it) } ?: animation?.let(Animation::fromFileName),
dataUsage = { null },
dataUsage = { if (isDefaultTestDescriptor) stringResource(getDataUsage()) else null },
expirationDate = expirationDate,
netTests = netTests.orEmpty(),
source = Descriptor.Source.Installed(this),
source = this,
updateStatus = updateStatus,
)

fun InstalledTestDescriptorModel.getDataUsage(): StringResource {
return when (this.key) {
"websites" -> Res.string.websites_datausage
"instant_messaging" -> Res.string.small_datausage
"circumvention" -> Res.string.small_datausage
"performance" -> Res.string.performance_datausage
"experimental" -> Res.string.TestResults_NotAvailable
else -> Res.string.TestResults_NotAvailable
}
}

private val iconAnimationMap = mapOf(
Res.drawable.test_websites to Animation.Websites,
Res.drawable.test_instant_messaging to Animation.InstantMessaging,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,11 @@ sealed interface RunSpecification {

@Serializable
data class Test(
val source: Source,
val source: InstalledTestDescriptorModel.Id? = null,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call this sourceId or descriptorId to avoid confusion?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this should never be null right?

val netTests: List<NetTest>,
) {
@Serializable
sealed interface Source {
@Serializable
data class Default(val name: String) : Source

@Serializable
data class Installed(val id: InstalledTestDescriptorModel.Id) : Source

companion object {
fun fromDescriptor(descriptor: Descriptor) =
when (descriptor.source) {
is Descriptor.Source.Default -> Default(descriptor.name)
is Descriptor.Source.Installed -> Installed(descriptor.source.value.id)
}
}
companion object {
fun fromDescriptor(descriptor: Descriptor) = descriptor.source.id
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This companion object isn't needed, it should return an object of the type Test, or it should be called something like sourceFromDescriptor. It's weird that a static factory method only returns a specific field inside its class.

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ class PreferenceRepository(
): Flow<Boolean> {
val key = getPreferenceKey(
name = descriptor.name,
prefix = (descriptor.source as? Descriptor.Source.Installed)
?.value?.id?.value?.toString(),
prefix = descriptor.source.id.value.toString(),
autoRun = isAutoRun,
)
return dataStore.data.map {
Expand Down Expand Up @@ -181,8 +180,7 @@ class PreferenceRepository(
isAutoRun: Boolean,
) = getPreferenceKey(
name = netTest.test.name,
prefix = (descriptor.source as? Descriptor.Source.Installed)
?.value?.id?.value?.toString(),
prefix = descriptor.settingsPrefix,
autoRun = isAutoRun,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import org.ooni.probe.domain.FinishInProgressData
import org.ooni.probe.domain.GetAutoRunSettings
import org.ooni.probe.domain.GetAutoRunSpecification
import org.ooni.probe.domain.GetBootstrapTestDescriptors
import org.ooni.probe.domain.GetDefaultTestDescriptors
import org.ooni.probe.domain.GetEnginePreferences
import org.ooni.probe.domain.GetFirstRun
import org.ooni.probe.domain.GetProxySettings
Expand Down Expand Up @@ -244,7 +243,6 @@ class Dependencies(
private val getBootstrapTestDescriptors by lazy {
GetBootstrapTestDescriptors(readAssetFile, json, backgroundContext)
}
private val getDefaultTestDescriptors by lazy { GetDefaultTestDescriptors() }
private val getProxySettings by lazy { GetProxySettings(preferenceRepository) }
private val getEnginePreferences by lazy {
GetEnginePreferences(
Expand Down Expand Up @@ -288,7 +286,6 @@ class Dependencies(
@VisibleForTesting
val getTestDescriptors by lazy {
GetTestDescriptors(
getDefaultTestDescriptors = getDefaultTestDescriptors::invoke,
listInstalledTestDescriptors = testDescriptorRepository::list,
descriptorUpdates = getDescriptorUpdate::observeAvailableUpdatesState,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class GetAutoRunSpecification(
return RunSpecification.Full(
tests = descriptors.map { descriptor ->
RunSpecification.Test(
source = RunSpecification.Test.Source.fromDescriptor(descriptor),
source = RunSpecification.Test.fromDescriptor(descriptor),
netTests = descriptor.netTests,
)
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ fun List<Descriptor>.forResult(result: ResultModel): Descriptor? =
result.testDescriptorId
?.let { descriptorId ->
firstOrNull {
it.source is Descriptor.Source.Installed && it.source.value.id == descriptorId
it.source.id == descriptorId
}
}
?: firstOrNull { it.name == result.testGroupName }
Original file line number Diff line number Diff line change
Expand Up @@ -3,60 +3,30 @@ package org.ooni.probe.domain
import androidx.compose.runtime.Composable
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.map
import ooniprobe.composeapp.generated.resources.Res
import ooniprobe.composeapp.generated.resources.Settings_TestOptions_LongRunningTest
import org.jetbrains.compose.resources.stringResource
import org.ooni.probe.data.models.DefaultTestDescriptor
import org.ooni.probe.data.models.Descriptor
import org.ooni.probe.data.models.DescriptorUpdatesStatus
import org.ooni.probe.data.models.InstalledTestDescriptorModel
import org.ooni.probe.data.models.UpdateStatus
import org.ooni.probe.data.models.toDescriptor

class GetTestDescriptors(
private val getDefaultTestDescriptors: () -> List<DefaultTestDescriptor>,
private val listInstalledTestDescriptors: () -> Flow<List<InstalledTestDescriptorModel>>,
private val descriptorUpdates: () -> Flow<DescriptorUpdatesStatus>,
) {
operator fun invoke(): Flow<List<Descriptor>> {
return combine(
listInstalledTestDescriptors(),
descriptorUpdates(),
flowOf(getDefaultTestDescriptors()),
) { installedDescriptors, descriptorUpdates, defaultDescriptors ->
) { installedDescriptors, descriptorUpdates ->
val updatedDescriptors = installedDescriptors.map { item ->
item.toDescriptor(updateStatus = descriptorUpdates.getStatusOf(item.id))
}
return@combine defaultDescriptors
.map { it.toDescriptor() } + updatedDescriptors
return@combine updatedDescriptors
}
}

private fun DefaultTestDescriptor.toDescriptor() =
Descriptor(
name = label,
title = { stringResource(title) },
shortDescription = { stringResource(shortDescription) },
description = {
if (label == "experimental") {
stringResource(description, experimentalLinks())
} else {
stringResource(description)
}
},
icon = icon,
color = color,
animation = animation,
dataUsage = { stringResource(dataUsage) },
expirationDate = null,
netTests = netTests,
longRunningTests = longRunningTests,
source = Descriptor.Source.Default(this),
updateStatus = UpdateStatus.NotApplicable,
)

@Composable
private fun experimentalLinks() =
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,6 @@ class GetTestDescriptorsBySpec(
// Is this descriptor contained in the RunSpecification's list of tests
private fun RunSpecification.Full.forDescriptor(descriptor: Descriptor) =
tests.firstOrNull { specTest ->
when (descriptor.source) {
is Descriptor.Source.Default -> {
specTest.source is RunSpecification.Test.Source.Default &&
specTest.source.name == descriptor.name
}

is Descriptor.Source.Installed -> {
specTest.source is RunSpecification.Test.Source.Installed &&
specTest.source.id == descriptor.source.value.id
}
}
specTest.source == descriptor.source.id
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class RunDescriptors(
) {
val result = ResultModel(
testGroupName = descriptor.name,
testDescriptorId = (descriptor.source as? Descriptor.Source.Installed)?.value?.id,
testDescriptorId = descriptor.source.id,
taskOrigin = taskOrigin,
)
val resultId = storeResult(result)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ class RunNetTest(
testTotal = spec.testTotal,
)
}
val installedDescriptorId =
(spec.descriptor.source as? Descriptor.Source.Installed)?.value?.id
val installedDescriptorId = spec.descriptor.source.id

startTest(
spec.netTest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ class ChooseWebsitesViewModel(
RunSpecification.Full(
tests = listOf(
RunSpecification.Test(
source = RunSpecification.Test.Source.Default("websites"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should fetch it from the DB, or hardcode the websites descriptor ID somewhere.

netTests = listOf(
NetTest(
test = TestType.WebConnectivity,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class DashboardViewModel(
.filterIsInstance<Event.FetchUpdatedDescriptors>()
.onEach {
state.value.descriptors[DescriptorType.Installed]
?.map { (it.source as Descriptor.Source.Installed).value }
?.map { it.source }
?.let { descriptors ->
fetchDescriptorUpdate(descriptors)
}
Expand Down Expand Up @@ -152,8 +152,8 @@ class DashboardViewModel(

private fun List<Descriptor>.groupByType() =
mapOf(
DescriptorType.Default to filter { it.source is Descriptor.Source.Default },
DescriptorType.Installed to filter { it.source is Descriptor.Source.Installed },
DescriptorType.Default to filter { it.isDefaultDescriptor() },
DescriptorType.Installed to filter { it.isInstalledNonDefaultDescriptor() },
)

data class State(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ fun DescriptorScreen(
isRefreshing = state.isRefreshing,
onRefresh = { onEvent(DescriptorViewModel.Event.FetchUpdatedDescriptor) },
state = pullRefreshState,
enabled = state.descriptor?.source is Descriptor.Source.Installed,
enabled = true,
)
.background(MaterialTheme.colorScheme.background),
) {
Expand Down Expand Up @@ -127,9 +127,7 @@ fun DescriptorScreen(
modifier = Modifier.padding(horizontal = 16.dp, vertical = 16.dp),
)

if (descriptor.source is Descriptor.Source.Installed) {
ConfigureUpdates(onEvent, descriptor.source.value.autoUpdate)
}
ConfigureUpdates(onEvent, descriptor.source.autoUpdate)
Text(
stringResource(Res.string.AddDescriptor_Settings),
style = MaterialTheme.typography.titleMedium,
Expand Down Expand Up @@ -162,13 +160,11 @@ fun DescriptorScreen(
TestDisplayMode.WebsitesOnly -> WebsiteItems(state.tests)
}

if (descriptor.source is Descriptor.Source.Installed) {
InstalledDescriptorActionsView(
descriptor = descriptor.source.value,
onEvent = onEvent,
modifier = Modifier.padding(horizontal = 16.dp),
)
}
InstalledDescriptorActionsView(
descriptor = descriptor.source,
onEvent = onEvent,
modifier = Modifier.padding(horizontal = 16.dp),
)
}
}
if (state.isRefreshing) {
Expand Down
Loading