Skip to content

Commit

Permalink
Fix #1903 added regex check to ensure analytics/crashlytics is explic…
Browse files Browse the repository at this point in the history
…itly disab… (#4995)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fix #1903 

Adds a regex check in file content validation to ensure Firebase
analytics are disabled in the codebase. Both regex are tested in
https://regex101.com/r/l14O5J/1, https://regex101.com/r/QVHGxi/1 for
Firebase analytics, Crashlytics, respectively.
- "Fixes ##1903:" Adds a regex check that ensures that firebase
analytics and crashlytics are explicitly disabled in
AndroidManifest.xml, new pull requests based on
/pull/4944

Manual testing:
Case 1: firebase_analytis_collection_deactivated = false in
AndroidManifest.xml; want = true (line 19)

![image](https://github.com/oppia/oppia-android/assets/44930615/0da2895b-55ea-4d17-acb9-00cf0e3dbfbe)

Case 2: firebase_analytis_collection_deactivated = true not found in
AndroidManifest.xml; want = explicit line (line 19)

![image](https://github.com/oppia/oppia-android/assets/44930615/99ebcd4e-bb10-4e49-a1a1-141d8e2e7ae9)

Case 3: firebase_crashlytics_collection_enabled = true in
AndroidManifest.xml; want = false (line 20)

![image](https://github.com/oppia/oppia-android/assets/44930615/0f459eb0-4c34-46b9-baff-3a5efd99e320)

Case 4: firebase_crashlytics_collection_enabled = false not found in
AndroidManifest.xml; want = explicit line (line 20)

![image](https://github.com/oppia/oppia-android/assets/44930615/92f5d167-dc12-4cb1-a940-9d836cf7e7db)

Case 5: happy case

![image](https://github.com/oppia/oppia-android/assets/44930615/f38d4460-9d0a-4b5f-af7e-b2b8df31c3a8)

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [X] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

---------

Co-authored-by: Ben Henning <[email protected]>
  • Loading branch information
chrislee115 and BenHenning authored May 31, 2023
1 parent ef01e73 commit 324023b
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 7 deletions.
8 changes: 2 additions & 6 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,8 @@
android:supportsRtl="true"
android:theme="@style/OppiaTheme">
<!-- NOTE TO DEVELOPER: You can enable debug logs for fast upload & better inspection in Firebase console using https://support.google.com/analytics/answer/7201382. -->
<meta-data
android:name="firebase_analytics_collection_deactivated"
android:value="true" />
<meta-data
android:name="firebase_crashlytics_collection_enabled"
android:value="false" />
<meta-data android:name="firebase_analytics_collection_deactivated" android:value="true" />
<meta-data android:name="firebase_crashlytics_collection_enabled" android:value="false" />
<meta-data
android:name="automatic_app_expiration_enabled"
android:value="false" />
Expand Down
10 changes: 10 additions & 0 deletions scripts/assets/file_content_validation_checks.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -440,3 +440,13 @@ file_content_checks {
exempted_file_name: "scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt"
exempted_file_name: "utility/src/main/java/org/oppia/android/util/parser/image/UrlImageParser.kt"
}
file_content_checks {
file_path_regex: "app/src/main/AndroidManifest.xml"
required_content_regex: "\\s*<meta-data\\s*android:name\\s*=\\s*\"firebase_analytics_collection_deactivated\"\\s*android:value\\s*=\\s*\"true\"\\s*/>\\s*"
failure_message: "Firebase analytics collection should always be explicitly deactivated in develop."
}
file_content_checks {
file_path_regex: "app/src/main/AndroidManifest.xml"
required_content_regex: "\\s*<meta-data\\s*android:name\\s*=\\s*\"firebase_crashlytics_collection_enabled\"\\s*android:value\\s*=\\s*\"false\"\\s*/>\\s*"
failure_message: "Firebase crashlytics collection should always be explicitly deactivated in develop."
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ class RegexPatternValidationCheckTest {
"Never explicitly handle configuration changes. Instead, use saved instance states for" +
" retaining state across rotations. For other types of configuration changes, follow up" +
" with the developer mailing list with how to proceed if you think this is a legitimate case."
private val androidManifestFirebaseAnalyticsEnabledErrorMessage =
"Firebase analytics collection should always be explicitly deactivated in develop."
private val androidManifestFirebaseCrashlyticsEnabledErrorMessage =
"Firebase crashlytics collection should always be explicitly deactivated in develop."
private val nonCompatDrawableUsedErrorMessage =
"Drawable start/end/top/bottom & image source should use the compat versions, instead, e.g.:" +
" app:drawableStartCompat or app:srcCompat, to ensure that vector drawables can load" +
Expand Down Expand Up @@ -1480,6 +1484,8 @@ class RegexPatternValidationCheckTest {
<?xml version="1.0" encoding="utf-8"?>
<manifest package="org.oppia.android">
<application android:name=".app.application.OppiaApplication">
<meta-data android:name="firebase_crashlytics_collection_enabled" android:value="false" />
<meta-data android:name="firebase_analytics_collection_deactivated" android:value="true" />
<activity
android:name=".app.ExampleActivity"
android:configChanges="orientation" />
Expand All @@ -1498,7 +1504,67 @@ class RegexPatternValidationCheckTest {
assertThat(outContent.toString().trim())
.isEqualTo(
"""
$stringFilePath:6: $androidActivityConfigChangesErrorMessage
$stringFilePath:8: $androidActivityConfigChangesErrorMessage
$wikiReferenceNote
""".trimIndent()
)
}

@Test
fun testFileContent_manifestWithFirebaseCrashlyticsEnabled_fileContentIsNotCorrect() {
val prohibitedContent =
"""
<?xml version="1.0" encoding="utf-8"?>
<manifest package="org.oppia.android">
<application android:name=".app.application.OppiaApplication">
<meta-data android:name="firebase_crashlytics_collection_enabled" android:value="true" />
<meta-data android:name="firebase_analytics_collection_deactivated" android:value="true" />
</application>
</manifest>
""".trimIndent()
tempFolder.newFolder("testfiles", "app", "src", "main")
val stringFilePath = "app/src/main/AndroidManifest.xml"
tempFolder.newFile("testfiles/$stringFilePath").writeText(prohibitedContent)

val exception = assertThrows(Exception::class) {
runScript()
}

assertThat(exception).hasMessageThat().contains(REGEX_CHECK_FAILED_OUTPUT_INDICATOR)
assertThat(outContent.toString().trim())
.isEqualTo(
"""
$stringFilePath: $androidManifestFirebaseCrashlyticsEnabledErrorMessage
$wikiReferenceNote
""".trimIndent()
)
}

@Test
fun testFileContent_manifestWithFirebaseAnalyticsEnabled_fileContentIsNotCorrect() {
val prohibitedContent =
"""
<?xml version="1.0" encoding="utf-8"?>
<manifest package="org.oppia.android">
<application android:name=".app.application.OppiaApplication">
<meta-data android:name="firebase_crashlytics_collection_enabled" android:value="false" />
<meta-data android:name="firebase_analytics_collection_deactivated" android:value="false" />
</application>
</manifest>
""".trimIndent()
tempFolder.newFolder("testfiles", "app", "src", "main")
val stringFilePath = "app/src/main/AndroidManifest.xml"
tempFolder.newFile("testfiles/$stringFilePath").writeText(prohibitedContent)

val exception = assertThrows(Exception::class) {
runScript()
}

assertThat(exception).hasMessageThat().contains(REGEX_CHECK_FAILED_OUTPUT_INDICATOR)
assertThat(outContent.toString().trim())
.isEqualTo(
"""
$stringFilePath: $androidManifestFirebaseAnalyticsEnabledErrorMessage
$wikiReferenceNote
""".trimIndent()
)
Expand Down

0 comments on commit 324023b

Please sign in to comment.