-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add ooni test descriptors. #275
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,16 +6,20 @@ import kotlinx.datetime.format | |
| import kotlinx.datetime.format.MonthNames | ||
| import kotlinx.datetime.format.char | ||
| import kotlinx.serialization.Serializable | ||
| import kotlinx.serialization.encodeToString | ||
| 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.engine.models.SummaryType | ||
| import org.ooni.probe.data.TestDescriptor | ||
|
|
@@ -24,6 +28,26 @@ import org.ooni.probe.shared.hexToColor | |
| import org.ooni.probe.shared.stringMonthArrayResource | ||
| import org.ooni.probe.shared.toEpoch | ||
|
|
||
| enum class OoniTest( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be in a separate file, there's no need to be here. |
||
| val id: Long, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This |
||
| val key: String, | ||
| ) { | ||
| WEBSITES(10470L, "websites"), | ||
| INSTANT_MESSAGING(10471L, "instant_messaging"), | ||
| CIRCUMVENTION(10472L, "circumvention"), | ||
| PERFORMANCE(10473L, "performance"), | ||
| EXPERIMENTAL(10474L, "experimental"), | ||
|
Comment on lines
+35
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're using regular capitalization for enum and sealed classes in this project: WEBSITES -> Websites. |
||
| ; | ||
|
|
||
| companion object { | ||
| private val map = entries.associateBy(OoniTest::id) | ||
|
|
||
| fun fromId(id: Long) = map[id] | ||
|
|
||
| fun isValidId(id: Long): Boolean = entries.any { it.id == id } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we're already caching the entries as a map, this could be |
||
| } | ||
| } | ||
|
|
||
| @Serializable | ||
| data class InstalledTestDescriptorModel( | ||
| val id: Id, | ||
|
|
@@ -56,6 +80,8 @@ data class InstalledTestDescriptorModel( | |
| val revision: Long, | ||
| ) | ||
|
|
||
| val isDefaultTestDescriptor get() = OoniTest.isValidId(id.value.toLongOrNull() ?: -1L) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if we should keep with the default concept or not. It's confusing with the NewsMediaScan descriptors, which are default, but not OONI tests. Maybe split this between |
||
|
|
||
| val key get() = Key(id, revision) | ||
|
|
||
| val previousRevisions | ||
|
|
@@ -85,15 +111,31 @@ 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, | ||
| // In the future, this will become a DB field with a value provided by the back-end | ||
| summaryType = SummaryType.Anomaly, | ||
| ) | ||
|
|
||
| fun InstalledTestDescriptorModel.getDataUsage(): StringResource = | ||
| when ( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's an enum class, so you can just do: |
||
| OoniTest | ||
| .fromId( | ||
| this.key.id.value | ||
| .toLong(), | ||
| )?.key | ||
| ) { | ||
| OoniTest.WEBSITES.key -> Res.string.websites_datausage | ||
| OoniTest.INSTANT_MESSAGING.key -> Res.string.small_datausage | ||
| OoniTest.CIRCUMVENTION.key -> Res.string.small_datausage | ||
| OoniTest.PERFORMANCE.key -> Res.string.performance_datausage | ||
| OoniTest.EXPERIMENTAL.key -> 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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,7 +38,7 @@ fun List<Descriptor>.forResult(result: ResultModel): Descriptor? = | |
| result.descriptorKey | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can now stop using the concept of |
||
| ?.let { key -> | ||
| firstOrNull { | ||
| it.source is Descriptor.Source.Installed && it.source.value.key == key | ||
| it.source?.key?.toString() == key.toString() | ||
| } | ||
| } | ||
| ?: firstOrNull { it.name == result.descriptorName } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had mentioned this before, but keeping both the
Descriptorand theInstalledTestDescriptorModelis just a big duplication of fields now. If we are to keep both because it helps withUpdateStatus, then we should remove the duplication:Descriptorclass should just have thesourceandUpdateStatusInstalledTestDescriptorModelcould be renamed toDescriptorandDescriptorcould become aDescriptorItemorDescriptorWithUpdateStatus