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

Remove setting to skip auto-run after certain limit #330

Merged
merged 1 commit into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,6 @@
<string name="Settings_ShareLogs_Error">Error sharing logs</string>
<string name="Settings_FilterLogs">Filter Logs</string>
<string name="Settings_DisableVpnInstructions">Go to Settings > General > VPN and disconnect from your VPN.</string>
<string name="Settings_AutoTest_NotUploadedLimit">Skip after this amount of results failed to upload</string>
<string name="Settings_Sharing_UploadResults_Description">Results are automatically uploaded to OONI explorer</string>
<string name="Settings_Websites_MaxRuntimeEnabled_New">Limit Websites test duration</string>
<string name="Settings_Websites_MaxRuntime_New">Maximum Websites test duration</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ enum class SettingsKey(val value: String) {
AUTOMATED_TESTING_ENABLED("automated_testing_enabled"),
AUTOMATED_TESTING_WIFIONLY("automated_testing_wifionly"),
AUTOMATED_TESTING_CHARGING("automated_testing_charging"),
AUTOMATED_TESTING_NOT_UPLOADED_LIMIT("automated_testing_not_uploaded_limit"),
MAX_RUNTIME_ENABLED("max_runtime_enabled"),
MAX_RUNTIME("max_runtime"),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ class PreferenceRepository(
return when (key) {
SettingsKey.MAX_RUNTIME,
SettingsKey.PROXY_PORT,
SettingsKey.AUTOMATED_TESTING_NOT_UPLOADED_LIMIT,
-> PreferenceKey.IntKey(intPreferencesKey(preferenceKey))

SettingsKey.PROXY_HOSTNAME,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ class Dependencies(
private val checkSkipAutoRunNotUploadedLimit by lazy {
CheckSkipAutoRunNotUploadedLimit(
resultRepository::countMissingUpload,
preferenceRepository::getValueByKey,
)
}
private val deleteAllResults by lazy {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class BootstrapPreferences(
SettingsKey.UPLOAD_RESULTS to true,
SettingsKey.AUTOMATED_TESTING_WIFIONLY to true,
SettingsKey.AUTOMATED_TESTING_CHARGING to true,
SettingsKey.AUTOMATED_TESTING_NOT_UPLOADED_LIMIT to NOT_UPLOADED_LIMIT_DEFAULT,
) +
organizationPreferenceDefaults(),
)
Expand All @@ -43,8 +42,4 @@ class BootstrapPreferences(
.flatMap { descriptor ->
descriptor.netTests.map { test -> descriptor to test }
}

companion object {
const val NOT_UPLOADED_LIMIT_DEFAULT = 10
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,25 @@ package org.ooni.probe.domain
import co.touchlab.kermit.Logger
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.first
import org.ooni.probe.data.models.SettingsKey

class CheckSkipAutoRunNotUploadedLimit(
private val countResultsMissingUpload: () -> Flow<Long>,
private val getPreferenceValueByKey: (SettingsKey) -> Flow<Any?>,
) {
suspend operator fun invoke(): Boolean {
val limitPreference =
getPreferenceValueByKey(SettingsKey.AUTOMATED_TESTING_NOT_UPLOADED_LIMIT).first()
val count = countResultsMissingUpload().first()

val notUploadedLimit = (limitPreference as? Int)
?.coerceAtLeast(1)
?: BootstrapPreferences.NOT_UPLOADED_LIMIT_DEFAULT
val shouldSkip = count >= notUploadedLimit

val shouldSkip = count >= NOT_UPLOADED_LIMIT
if (shouldSkip) {
Logger.w(
"Skipping auto-run due to not uploaded limit",
SkipAutoRunException("Results missing upload: $count (limit=$notUploadedLimit)"),
SkipAutoRunException("Results missing upload: $count (limit=$NOT_UPLOADED_LIMIT)"),
)
}

return shouldSkip
}

companion object {
private const val NOT_UPLOADED_LIMIT = 10
}
}

class SkipAutoRunException(message: String) : Exception(message)
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import ooniprobe.composeapp.generated.resources.Settings_About_Label
import ooniprobe.composeapp.generated.resources.Settings_Advanced_DebugLogs
import ooniprobe.composeapp.generated.resources.Settings_Advanced_Label
import ooniprobe.composeapp.generated.resources.Settings_Advanced_RecentLogs
import ooniprobe.composeapp.generated.resources.Settings_AutoTest_NotUploadedLimit
import ooniprobe.composeapp.generated.resources.Settings_AutomatedTesting_RunAutomatically
import ooniprobe.composeapp.generated.resources.Settings_AutomatedTesting_RunAutomatically_ChargingOnly
import ooniprobe.composeapp.generated.resources.Settings_AutomatedTesting_RunAutomatically_Description
Expand Down Expand Up @@ -82,7 +81,6 @@ class GetSettings(
WebConnectivityCategory.entries.mapNotNull { it.settingsKey } + listOf(
SettingsKey.UPLOAD_RESULTS,
SettingsKey.AUTOMATED_TESTING_ENABLED,
SettingsKey.AUTOMATED_TESTING_NOT_UPLOADED_LIMIT,
SettingsKey.MAX_RUNTIME_ENABLED,
SettingsKey.MAX_RUNTIME,
),
Expand All @@ -95,7 +93,6 @@ class GetSettings(
hasWebsitesDescriptor = OrganizationConfig.hasWebsitesDescriptor,
uploadResultsEnabled = preferences[SettingsKey.UPLOAD_RESULTS] == true,
autoRunEnabled = preferences[SettingsKey.AUTOMATED_TESTING_ENABLED] == true,
autoRunNotUploadedLimit = preferences[SettingsKey.AUTOMATED_TESTING_NOT_UPLOADED_LIMIT] as? Int,
enabledCategoriesCount = enabledCategoriesCount,
maxRuntimeEnabled = preferences[SettingsKey.MAX_RUNTIME_ENABLED] == true,
maxRuntime = preferences[SettingsKey.MAX_RUNTIME] as? Int,
Expand All @@ -109,7 +106,6 @@ class GetSettings(
hasWebsitesDescriptor: Boolean,
uploadResultsEnabled: Boolean,
autoRunEnabled: Boolean,
autoRunNotUploadedLimit: Int?,
enabledCategoriesCount: Int,
maxRuntimeEnabled: Boolean,
maxRuntime: Int?,
Expand Down Expand Up @@ -178,20 +174,6 @@ class GetSettings(
enabled = autoRunEnabled && uploadResultsEnabled,
indentation = 1,
),
SettingsItem(
title = Res.string.Settings_AutoTest_NotUploadedLimit,
key = SettingsKey.AUTOMATED_TESTING_NOT_UPLOADED_LIMIT,
type = PreferenceItemType.INT,
enabled = autoRunEnabled && uploadResultsEnabled,
supportingContent = {
val value = (
autoRunNotUploadedLimit
?: BootstrapPreferences.NOT_UPLOADED_LIMIT_DEFAULT
).coerceAtLeast(1)
Text(value.toString())
},
indentation = 1,
),
)
} else {
emptyList()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,16 @@ class CheckSkipAutoRunNotUploadedLimitTest {
suspend fun assertCheck(
expected: Boolean,
count: Long,
limit: Any?,
) {
assertEquals(
expected,
CheckSkipAutoRunNotUploadedLimit(
countResultsMissingUpload = { flowOf(count) },
getPreferenceValueByKey = { flowOf(limit) },
)(),
)
}

assertCheck(expected = true, count = 20, limit = null)
assertCheck(expected = true, count = 5, limit = 5)
assertCheck(expected = false, count = 0, limit = null)
assertCheck(expected = false, count = 100, limit = 101)
assertCheck(expected = true, count = 20)
assertCheck(expected = false, count = 5)
}
}