Skip to content

Commit 6b161f5

Browse files
adhiamboperesadhiamboperes
authored
Merge branch 'develop' into redesign-pin-input
2 parents 1a03d92 + d4d99d4 commit 6b161f5

File tree

11 files changed

+249
-79
lines changed

11 files changed

+249
-79
lines changed

‎app/src/main/res/values-sw/strings.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@
9494
<string name="math_expression_error_multiple_redundant_parentheses">Tafadhali ondoa mabano ya ziada karibu na \'(%1$s)\', kwa mfano: \'%1$s\'.</string>
9595
<string name="math_expression_error_redundant_parentheses_individual_term">Tafadhali ondoa mabano ya ziada karibu na \'(%1$s)\', kwa mfano: \'%1$s\'.</string>
9696
<string name="math_expression_error_unnecessary_symbols">Kuna\'%s\' isiyo halali katika jibu. Tafadhali ondoa.</string>
97-
<string name="math_expression_error_number_after_var_term">Tafadhali panga upya utaratibu wa % %1$s &amp; %2$s. Kwa mfano:% $ 2% $ s.</string>
97+
<string name="math_expression_error_number_after_var_term">Tafadhali panga upya utaratibu wa %1$s &amp; %2$s. Kwa mfano: %1$s %2$s.</string>
9898
<string name="math_expression_error_consecutive_binary_operators">%1$s na % $%2$s inapaswa kutengwa na nambari au kitu tofauti.</string>
9999
<string name="math_expression_error_consecutive_unary_operators">Tafadhali ondoa alama za ziada katika jibu lako.</string>
100100
<string name="math_expression_error_missing_lhs_for_addition_operator">Je! Kuna idadi au kitu tofauti inayokosa kabla au baada ya ishara ya kuongeza \'%1$s\'? Ikiwa sivyo, tafadhali ondoa \'%1$s\' ya ziada.</string>

‎domain/src/main/java/org/oppia/android/domain/platformparameter/PlatformParameterControllerDebugImpl.kt‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class PlatformParameterControllerDebugImpl @Inject constructor(
4444
// Note that the 'by lazy' here guarantees thread-safe and singleton initialization.
4545
private val initializationDeferred by lazy { loadParametersInternalAsync() }
4646
private val parametersAreLoadedFlow by lazy { MutableStateFlow(false) }
47+
private val downloadRemoteParametersProvider by lazy { downloadRemoteParametersInternal() }
4748
private var ongoingDownloadTask: Deferred<Unit>? = null
4849

4950
init {
@@ -73,18 +74,7 @@ class PlatformParameterControllerDebugImpl @Inject constructor(
7374
}
7475
}
7576

76-
override fun downloadRemoteParameters(): DataProvider<Unit> {
77-
oppiaLogger.d("PlatformParameterController", "Calling Force Download of remote parameters")
78-
return dataProviders.createInMemoryDataProviderAsync(DOWNLOAD_REMOTE_PARAMETERS_PROVIDER_ID) {
79-
ongoingDownloadTask?.cancel()
80-
ongoingDownloadTask = CoroutineScope(backgroundCoroutineDispatcher).async {
81-
// TODO(#5950): Replace with actual logic to force-download remote parameters.
82-
}
83-
ongoingDownloadTask?.await()
84-
oppiaLogger.d("PlatformParameterController", "Download finished successfully.")
85-
return@createInMemoryDataProviderAsync AsyncResult.Success(Unit)
86-
}
87-
}
77+
override fun downloadRemoteParameters() = downloadRemoteParametersProvider
8878

8979
/** Cancels any ongoing remote parameter download. */
9080
fun cancelRemoteParameterDownload(): Boolean {
@@ -258,6 +248,20 @@ class PlatformParameterControllerDebugImpl @Inject constructor(
258248
}
259249
}
260250

251+
private fun downloadRemoteParametersInternal(): DataProvider<Unit> {
252+
return dataProviders.createInMemoryDataProviderAsync(DOWNLOAD_REMOTE_PARAMETERS_PROVIDER_ID) {
253+
if (ongoingDownloadTask == null) {
254+
oppiaLogger.d("PlatformParameterController", "Calling Force Download of remote parameters")
255+
ongoingDownloadTask = CoroutineScope(backgroundCoroutineDispatcher).async {
256+
// TODO(#5950): Replace with actual logic to force-download remote parameters.
257+
oppiaLogger.d("PlatformParameterController", "Download finished successfully.")
258+
}
259+
}
260+
ongoingDownloadTask?.await()
261+
return@createInMemoryDataProviderAsync AsyncResult.Success(Unit)
262+
}
263+
}
264+
261265
/**
262266
* Updates the local override database with the provided list of overridden feature flags.
263267
*

‎domain/src/test/java/org/oppia/android/domain/devoptions/ForceDownloadRemoteParametersControllerTest.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,18 @@ class ForceDownloadRemoteParametersControllerTest {
6767
assertThat(result).isEqualTo(true)
6868
}
6969

70+
@Test
71+
fun testDownloadRemoteParameters_secondInvocation_returnsSameInstance() {
72+
setUpTestApplicationComponent()
73+
74+
val firstProvider = forceDownloadRemoteParametersController.downloadRemoteParameters()
75+
val secondProvider = forceDownloadRemoteParametersController.downloadRemoteParameters()
76+
testCoroutineDispatchers.runCurrent()
77+
78+
// Multiple calls to downloadRemoteParameters() should yield the same DataProvider instance.
79+
assertThat(secondProvider).isEqualTo(firstProvider)
80+
}
81+
7082
private fun setUpTestApplicationComponent() {
7183
ApplicationProvider.getApplicationContext<TestApplication>().inject(this)
7284
}

‎domain/src/test/java/org/oppia/android/domain/platformparameter/PlatformParameterControllerDebugImplTest.kt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,6 +1138,19 @@ class PlatformParameterControllerDebugImplTest {
11381138
assertThat(ephemeralNewUiFlag?.currentValue)
11391139
.isEqualTo(TEST_REMOTE_MULTIPLE_CLASSROOMS)
11401140
}
1141+
1142+
@Test
1143+
fun testDownloadRemoteParameters_secondInvocation_returnsSameInstance() {
1144+
setUpTestApplicationComponent()
1145+
1146+
val firstProvider = platformParameterControllerDebugImpl.downloadRemoteParameters()
1147+
val secondProvider = platformParameterControllerDebugImpl.downloadRemoteParameters()
1148+
testCoroutineDispatchers.runCurrent()
1149+
1150+
// Multiple calls to downloadRemoteParameters() should yield the same DataProvider instance.
1151+
assertThat(secondProvider).isEqualTo(firstProvider)
1152+
}
1153+
11411154
// Populates the remote DB with test feature flag for MULTIPLE_CLASSROOMS.
11421155
private fun addTestRemoteFeatureFlagToDatabase(
11431156
component: TestApplicationComponent,

‎scripts/assets/android_lint_exemptions.textproto‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ android_lint_exemption: {
22
exempted_file_path: "app/src/main/AndroidManifest.xml"
33
lint_issue_id: LOCKED_ORIENTATION_ACTIVITY
44
lint_issue_id: MISSING_VERSION
5-
lint_issue_id: OLD_TARGET_API
65
lint_issue_id: REDUNDANT_LABEL
76
}
87
android_lint_exemption: {
@@ -320,30 +319,12 @@ android_lint_exemption: {
320319
}
321320
android_lint_exemption: {
322321
exempted_file_path: "app/src/main/res/values-sw/strings.xml"
323-
lint_issue_id: STRING_FORMAT_COUNT
324322
lint_issue_id: TYPOGRAPHY_QUOTES
325323
}
326324
android_lint_exemption: {
327325
exempted_file_path: "app/src/main/res/values/strings.xml"
328-
lint_issue_id: STRING_FORMAT_COUNT
329326
lint_issue_id: TYPOGRAPHY_QUOTES
330327
}
331-
android_lint_exemption: {
332-
exempted_file_path: "data/src/main/AndroidManifest.xml"
333-
lint_issue_id: OLD_TARGET_API
334-
}
335-
android_lint_exemption: {
336-
exempted_file_path: "domain/src/main/AndroidManifest.xml"
337-
lint_issue_id: OLD_TARGET_API
338-
}
339-
android_lint_exemption: {
340-
exempted_file_path: "testing/src/main/AndroidManifest.xml"
341-
lint_issue_id: OLD_TARGET_API
342-
}
343-
android_lint_exemption: {
344-
exempted_file_path: "utility/src/main/AndroidManifest.xml"
345-
lint_issue_id: OLD_TARGET_API
346-
}
347328
android_lint_exemption: {
348329
exempted_file_path: "utility/src/main/java/org/oppia/android/util/locale/DisplayLocaleImpl.kt"
349330
lint_issue_id: APP_BUNDLE_LOCALE_CHANGES

‎scripts/src/java/org/oppia/android/scripts/lint/AndroidLintRunner.kt‎

Lines changed: 98 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@ import java.lang.Module
1616
import java.lang.ModuleLayer
1717
import java.nio.file.Files
1818
import java.util.concurrent.TimeUnit
19+
import kotlin.concurrent.thread
20+
import kotlin.system.exitProcess
1921
import com.android.tools.lint.Main as LintCli
2022

2123
/** The default timeout duration for executing external processes. */
22-
private const val DEFAULT_PROCESS_TIMEOUT_MINUTES = 10L
24+
private const val DEFAULT_PROCESS_TIMEOUT_MINUTES = 15L
2325
/** Default path to the exemption .pb file. */
2426
private const val DEFAULT_PROTO_BINARY_PATH = "scripts/assets/android_lint_exemptions.pb"
2527

@@ -123,10 +125,23 @@ private fun Long.toFormattedDuration(): String {
123125
* Examples:
124126
* bazel run //scripts:android_lint_check -- $(pwd)
125127
* bazel run //scripts:android_lint_check -- $(pwd) --group_by_severity
126-
* bazel run //scripts:android_lint_check -- $(pwd) --processTimeout=15
128+
* bazel run //scripts:android_lint_check -- $(pwd) --processTimeout=20
127129
* bazel run //scripts:android_lint_check -- $(pwd) --timer
128130
*/
129131
fun main(vararg args: String) {
132+
var exitCode = 0
133+
try {
134+
executeAndroidLintAnalysis(*args)
135+
} catch (e: Exception) {
136+
e.printStackTrace()
137+
exitCode = 1
138+
} finally {
139+
exitProcess(exitCode)
140+
}
141+
}
142+
143+
/** Executes Android Lint analysis with given arguments and handles setup, and execution. */
144+
fun executeAndroidLintAnalysis(vararg args: String) {
130145
ScriptBackgroundCoroutineDispatcher().use { scriptBgDispatcher ->
131146
require(args.isNotEmpty()) {
132147
"<path_to_repository_root argument> is required: \$(pwd)"
@@ -136,13 +151,15 @@ fun main(vararg args: String) {
136151
require(repoRoot.exists()) {
137152
"Repository root path does not exist: ${args[0]}"
138153
}
154+
139155
val exemptionProtoPath = args.find { it.startsWith("--proto=") }?.let { option ->
140156
val path = option.substringAfter("=")
141157
require(path.endsWith(".pb")) {
142158
"Invalid exemption file: $path. The file must have a .pb extension."
143159
}
144160
path
145161
} ?: DEFAULT_PROTO_BINARY_PATH
162+
146163
val groupByIssueSeverity = args.contains("--group_by_severity")
147164
val showTimer = args.contains("--timer")
148165
val processTimeout = args.find { it.startsWith("--processTimeout=") }
@@ -225,7 +242,8 @@ class AndroidLintAnalyzer(
225242
"UnusedResources",
226243
"UnusedAttribute",
227244
"UnknownNullness",
228-
"MergeRootFrame"
245+
"MergeRootFrame",
246+
"OldTargetApi"
229247
)
230248
}
231249

@@ -279,6 +297,75 @@ class AndroidLintAnalyzer(
279297
}
280298
}
281299

300+
/**
301+
* Wrapper class to run lint with timeout protection.
302+
*
303+
* The lint tool sometimes hangs on certain systems after writing the report
304+
* (likely due to lingering non-daemon threads in the native Java process).
305+
* Since lint runs natively inside this script as a Java application,
306+
* it may continue running indefinitely unless explicitly stopped.
307+
*
308+
* This utility ensures that lint execution terminates within a bounded time,
309+
* preventing hangs by enforcing a timeout and interrupting the process if
310+
* it fails to complete.
311+
*/
312+
class LintTimeoutWrapper(
313+
private val cliArgs: Array<String>,
314+
private val timeoutMinutes: Long
315+
) {
316+
@Volatile
317+
private var exitCode: Int = -1
318+
319+
@Volatile
320+
private var completed = false
321+
322+
@Volatile
323+
private var timedOut = false
324+
325+
/**
326+
* Runs lint analysis with a timeout.
327+
*
328+
* @return the exit code from the lint process
329+
* @throws IllegalStateException if lint doesn't finish within the specified duration
330+
*/
331+
fun runWithTimeout(): Int {
332+
val lintThread = thread(start = true, name = "lint-runner") {
333+
try {
334+
exitCode = LintCli().run(cliArgs)
335+
completed = true
336+
} catch (e: Exception) {
337+
e.printStackTrace()
338+
exitCode = 1
339+
completed = true
340+
}
341+
}
342+
343+
val timeoutMillis = TimeUnit.MINUTES.toMillis(timeoutMinutes)
344+
val startTime = System.currentTimeMillis()
345+
346+
while (!completed && (System.currentTimeMillis() - startTime) < timeoutMillis) {
347+
Thread.sleep(100)
348+
}
349+
350+
if (!completed) {
351+
timedOut = true
352+
// Attempt to interrupt the lint thread
353+
lintThread.interrupt()
354+
355+
// Short time to respond to interruption
356+
Thread.sleep(1000)
357+
358+
throw IllegalStateException(
359+
"Lint analysis timed out after $timeoutMinutes minutes. " +
360+
"This can happen if the Lint tool hanged or is taking excess execution time. " +
361+
"Consider increasing the timeout via --processTimeout=<minutes> if needed."
362+
)
363+
}
364+
365+
return exitCode
366+
}
367+
}
368+
282369
/**
283370
* Runs the Android Lint tool and reports issues.
284371
*
@@ -322,7 +409,14 @@ class AndroidLintRunner(
322409
* @param cliArgs the command-line arguments to pass to the Lint CLI
323410
*/
324411
fun runLint(cliArgs: Array<String>) {
325-
val exitCode = LintCli().run(cliArgs)
412+
println("Starting lint analysis with $DEFAULT_PROCESS_TIMEOUT_MINUTES minute timeout.")
413+
414+
val exitCode = try {
415+
val wrapper = LintTimeoutWrapper(cliArgs, DEFAULT_PROCESS_TIMEOUT_MINUTES)
416+
wrapper.runWithTimeout()
417+
} catch (e: IllegalStateException) {
418+
exitProcess(1)
419+
}
326420

327421
// Allow exit code 1(ISSUES_FOUND) since it indicates issues with
328422
// severity Error which is being handled by LintAnalysisReporter.

‎scripts/src/java/org/oppia/android/scripts/lint/LintAnalysisReporter.kt‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,9 @@ class LintAnalysisReporter(private val repoRoot: File) {
153153
"LockedOrientationActivity" to LintIssueId.LOCKED_ORIENTATION_ACTIVITY,
154154
"MissingVersion" to LintIssueId.MISSING_VERSION,
155155
"NotifyDataSetChanged" to LintIssueId.NOTIFY_DATA_SET_CHANGED,
156-
"OldTargetApi" to LintIssueId.OLD_TARGET_API,
157156
"Overdraw" to LintIssueId.OVERDRAW,
158157
"RedundantLabel" to LintIssueId.REDUNDANT_LABEL,
159158
"Registered" to LintIssueId.REGISTERED,
160-
"StringFormatCount" to LintIssueId.STRING_FORMAT_COUNT,
161159
"SwitchIntDef" to LintIssueId.SWITCH_INT_DEF,
162160
"TypographyDashes" to LintIssueId.TYPOGRAPHY_DASHES,
163161
"TypographyQuotes" to LintIssueId.TYPOGRAPHY_QUOTES,
@@ -293,7 +291,7 @@ class LintAnalysisReporter(private val repoRoot: File) {
293291
// Unknown issues cannot be exempted, so they should appear in the report
294292
val issueIdEnum = getLintIssueIdFromString(issue.id) ?: return false
295293

296-
return issue.locations.any { location ->
294+
return issue.locations.all { location ->
297295
val relativePath = File(location.file).toRelativeString(repoRoot)
298296
val exemptedIssues = exemptionMap[relativePath]
299297
exemptedIssues?.contains(issueIdEnum) == true
@@ -459,11 +457,12 @@ class LintAnalysisReporter(private val repoRoot: File) {
459457

460458
printSeveritySummary(filteredIssues, redundantExemptionsCount)
461459
println()
462-
463-
println(
464-
"If you need additional help to resolve an issue," +
465-
" see https://googlesamples.github.io/android-custom-lint-rules/checks/severity.md.html"
466-
)
460+
if (filteredIssues.isNotEmpty()) {
461+
println(
462+
"If you need additional help to resolve an issue," +
463+
" see https://googlesamples.github.io/android-custom-lint-rules/checks/severity.md.html"
464+
)
465+
}
467466
println()
468467

469468
if (redundantExemptions.isNotEmpty()) {
@@ -544,7 +543,10 @@ class LintAnalysisReporter(private val repoRoot: File) {
544543
println(
545544
wrapText(
546545
"In $filePath the ${toUpperSnakeCase(issueId)} exemption is redundant" +
547-
" and can be removed since there are no corresponding lint issues."
546+
" and can be removed since there are no corresponding lint issues." +
547+
"\nRefer to the Android Lint Check Wiki for more information:" +
548+
" https://github.com/oppia/oppia-android/wiki/" +
549+
"Android-Lint-Check#exemption-file-maintenance-prompts"
548550
)
549551
)
550552

@@ -748,7 +750,10 @@ class LintAnalysisReporter(private val repoRoot: File) {
748750
println("${YELLOW}UNUSED ENUM MAPPINGS DETECTED:$RESET")
749751
println(
750752
"The following issue IDs are defined in issueIdMapping " +
751-
"but no corresponding lint issues were found."
753+
"but no corresponding lint issues were found." +
754+
"\nRefer to the Android Lint Check Wiki for more" +
755+
" information: https://github.com/oppia/oppia-android/wiki/" +
756+
"Android-Lint-Check#unused-enum-mappings-detection"
752757
)
753758
println("Please remove them from the LintIssueId enum and issueIdMapping:")
754759
println()

‎scripts/src/java/org/oppia/android/scripts/proto/android_lint.proto‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,13 @@ enum LintIssueId {
3636
LOCKED_ORIENTATION_ACTIVITY = 8;
3737
MISSING_VERSION = 9;
3838
NOTIFY_DATA_SET_CHANGED = 10;
39-
OLD_TARGET_API = 11;
40-
OVERDRAW = 12;
41-
REDUNDANT_LABEL = 13;
42-
REGISTERED = 14;
43-
STRING_FORMAT_COUNT = 15;
44-
SWITCH_INT_DEF = 16;
45-
TYPOGRAPHY_DASHES = 17;
46-
TYPOGRAPHY_QUOTES = 18;
47-
USE_COMPOUND_DRAWABLES = 19;
48-
VECTOR_PATH = 20;
49-
VECTOR_RASTER = 21;
39+
OVERDRAW = 11;
40+
REDUNDANT_LABEL = 12;
41+
REGISTERED = 13;
42+
SWITCH_INT_DEF = 14;
43+
TYPOGRAPHY_DASHES = 15;
44+
TYPOGRAPHY_QUOTES = 16;
45+
USE_COMPOUND_DRAWABLES = 17;
46+
VECTOR_PATH = 18;
47+
VECTOR_RASTER = 19;
5048
}

0 commit comments

Comments
 (0)