Skip to content

Commit

Permalink
update based on review
Browse files Browse the repository at this point in the history
  • Loading branch information
aanorbel committed Dec 3, 2024
1 parent b246b77 commit 8c0c5d8
Show file tree
Hide file tree
Showing 18 changed files with 31 additions and 256 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,15 @@ 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.key
}
get() = source.key

val allTests get() = netTests + longRunningTests

Expand All @@ -46,23 +37,14 @@ data class Descriptor(
.sumOf { it.test.runtime(it.inputs).inWholeSeconds }
.seconds

val settingsPrefix: String?
get() = when (isDefaultDescriptor()) {
true -> null
else -> (source as Source.Installed).value.id.value.toString()
}
val settingsPrefix: String
get() = source.id.value.toString()

fun isDefaultDescriptor(): Boolean {
return when (source) {
is Source.Default -> true
is Source.Installed -> source.value.isDefaultTestDescriptor
}
return source.isDefaultTestDescriptor
}

fun isInstalledNonDefaultDescriptor(): Boolean {
return when (source) {
is Source.Installed -> !source.value.isDefaultTestDescriptor
else -> false
}
return !source.isDefaultTestDescriptor
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ fun InstalledTestDescriptorModel.toDescriptor(updateStatus: UpdateStatus = Updat
dataUsage = { if (isDefaultTestDescriptor) stringResource(getDataUsage()) else null },
expirationDate = expirationDate,
netTests = netTests.orEmpty(),
source = Descriptor.Source.Installed(this),
source = this,
updateStatus = updateStatus,
)

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,
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
}
}
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,6 @@ class Dependencies(
@VisibleForTesting
val getTestDescriptors by lazy {
GetTestDescriptors(
getDefaultTestDescriptors = { emptyList() },
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"),
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
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,11 +127,7 @@ fun DescriptorScreen(
modifier = Modifier.padding(horizontal = 16.dp, vertical = 16.dp),
)

if (descriptor.isInstalledNonDefaultDescriptor()) {
(descriptor.source as Descriptor.Source.Installed?)?.let {
ConfigureUpdates(onEvent, it.value.autoUpdate)
}
}
ConfigureUpdates(onEvent, descriptor.source.autoUpdate)
Text(
stringResource(Res.string.AddDescriptor_Settings),
style = MaterialTheme.typography.titleMedium,
Expand Down Expand Up @@ -164,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
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,7 @@ class DescriptorViewModel(
events.filterIsInstance<Event.AutoUpdateChanged>()
.onEach {
val descriptor = state.value.descriptor ?: return@onEach
if (descriptor.source !is Descriptor.Source.Installed) return@onEach
setAutoUpdate(descriptor.source.value.id, it.value)
setAutoUpdate(descriptor.source.id, it.value)
}
.launchIn(viewModelScope)

Expand All @@ -176,23 +175,21 @@ class DescriptorViewModel(
if (state.value.isRefreshing) return@onEach
val descriptor = state.value.descriptor ?: return@onEach

if (descriptor.source !is Descriptor.Source.Installed) return@onEach
_state.update {
it.copy(refreshType = UpdateStatusType.UpdateLink, updatedDescriptor = null)
}

fetchDescriptorUpdate(listOf(descriptor.source.value))
fetchDescriptorUpdate(listOf(descriptor.source))
}
.launchIn(viewModelScope)

events.filterIsInstance<Event.UpdateDescriptor>()
.onEach {
val descriptor = state.value.updatedDescriptor ?: return@onEach
if (descriptor.source !is Descriptor.Source.Installed) return@onEach
_state.update {
it.copy(refreshType = UpdateStatusType.None, updatedDescriptor = null)
}
reviewUpdates(listOf(descriptor.source.value))
reviewUpdates(listOf(descriptor.source))
goToReviewDescriptorUpdates()
}
.launchIn(viewModelScope)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class ResultViewModel(
return RunSpecification.Full(
tests = listOf(
RunSpecification.Test(
source = RunSpecification.Test.Source.fromDescriptor(item.descriptor),
source = RunSpecification.Test.fromDescriptor(item.descriptor),
netTests = listOf(
NetTest(
test = TestType.WebConnectivity,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,7 @@ class RunViewModel(
tests =
selectedTests.map { (descriptor, tests) ->
RunSpecification.Test(
source =
when (descriptor.source) {
is Descriptor.Source.Default ->
RunSpecification.Test.Source.Default(descriptor.name)

is Descriptor.Source.Installed ->
RunSpecification.Test.Source.Installed(descriptor.source.value.id)
},
source = descriptor.source.id,
netTests = tests,
)
},
Expand Down
Loading

0 comments on commit 8c0c5d8

Please sign in to comment.