Skip to content

Commit 4b49cd8

Browse files
authored
Fixes 5445: Add checks for feature flags (#5464)
<!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation Fixes #5445: When merged, this PR will; - Introduce a check to ensure new feature flags are added into the FeatureFlagsLogger - Add any existing flags that are currently not being logged - Add the feature_flags_check.sh file to static_checks.sh so that it can be run locally <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> ## 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". - [x] 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: Kenneth Murerwa <[email protected]>
1 parent 436cf9e commit 4b49cd8

File tree

5 files changed

+156
-8
lines changed

5 files changed

+156
-8
lines changed

.github/workflows/static_checks.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ jobs:
8484
run: |
8585
bash /home/runner/work/oppia-android/oppia-android/scripts/ktlint_lint_check.sh $HOME
8686
87+
- name: Feature flag checks
88+
run: |
89+
bash /home/runner/work/oppia-android/oppia-android/scripts/feature_flags_check.sh $HOME
90+
8791
- name: Protobuf lint check
8892
run: |
8993
bash /home/runner/work/oppia-android/oppia-android/scripts/buf_lint_check.sh $HOME

scripts/assets/todo_open_exemptions.textproto

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ todo_open_exemption {
336336
}
337337
todo_open_exemption {
338338
exempted_file_path: "scripts/static_checks.sh"
339-
line_number: 110
339+
line_number: 114
340340
}
341341
todo_open_exemption {
342342
exempted_file_path: "wiki/Coding-style-guide.md"

scripts/feature_flags_check.sh

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
#!/bin/bash
2+
3+
echo "********************************"
4+
echo "Running feature flag checks"
5+
echo "********************************"
6+
7+
failed_checks=0
8+
9+
function item_in_array() {
10+
local item="$1"
11+
shift
12+
local array=("$@")
13+
14+
for element in "${array[@]}"; do
15+
if [[ "$element" == "$item" ]]; then
16+
echo 1
17+
return
18+
fi
19+
done
20+
21+
echo 0
22+
}
23+
24+
function get_classes_from_constants_file() {
25+
# Get the file path of the FeatureFlagConstants File
26+
file_path=$(find . -name FeatureFlagConstants.kt)
27+
28+
# Get a list of feature flag annotation classes
29+
annotation_classes=$(grep -E '[\s\w]*annotation\s*class' "$file_path")
30+
31+
# Convert the string into an array, splitting by newline
32+
IFS=$'\n' read -rd '' -a array <<<"$annotation_classes"
33+
34+
# Create an empty array to hold individual class names
35+
class_names=()
36+
37+
# Iterate through each line and take the last word of the list
38+
for line in "${array[@]}"; do
39+
# Convert each line into an array of words, splitting by space
40+
IFS=' ' read -ra words <<<"$line"
41+
42+
# Get last element of the list
43+
last_element="${words[${#words[@]}-1]}"
44+
45+
# Add the element into class_names
46+
class_names+=("$last_element")
47+
done
48+
49+
echo "${class_names[@]}"
50+
}
51+
52+
function get_imported_classes_in_logger() {
53+
# File path containing the block of text
54+
file_path=$(find . -name FeatureFlagsLogger.kt)
55+
56+
# Use sed to extract the block based on a starting pattern and an ending pattern
57+
extracted_block=$(sed -n '/class FeatureFlagsLogger @Inject constructor(/,/^)/p' "$file_path")
58+
59+
# Get annotation classes
60+
logged_classes=$(grep -E '@Enable[A-Za-z0-9_]+' "$file_path")
61+
62+
# Replace the @ symbol and spaces within each element of the list
63+
for i in "${!logged_classes[@]}"; do
64+
logged_classes[$i]=$(echo "${logged_classes[$i]}" | tr -d '@' | tr -d ' ')
65+
done
66+
67+
# Print the logged classes
68+
echo "$logged_classes"
69+
}
70+
71+
function get_flags_added_into_the_logging_map() {
72+
# File path containing the block of text
73+
file_path=$(find . -name FeatureFlagsLogger.kt)
74+
75+
# Use sed to extract the block based on a starting pattern and an ending pattern
76+
extracted_block=$(sed -n '/Map<String, PlatformParameterValue<Boolean>> = mapOf(/,/)$/p' "$file_path")
77+
78+
# Get any word beginning with enable from the extracted block
79+
enable_flags=$(grep -E 'enable[A-Za-z0-9_]+' <<<"$extracted_block")
80+
81+
added_flags=()
82+
83+
# Convert the string into an array, splitting by newline
84+
IFS=$'\n' read -rd '' -a added_flags <<<"$enable_flags"
85+
86+
# Create an empty array to hold individual class names
87+
class_names=()
88+
89+
# Iterate through each line and take the last word of the list
90+
for line in "${added_flags[@]}"; do
91+
# Remove the comma at the end of the line
92+
line=$(echo "$line" | awk '{sub(/,$/, ""); print}')
93+
94+
# Convert each line into an array of words, splitting by space
95+
IFS=' ' read -ra words <<<"$line"
96+
97+
# Get last element of the list
98+
last_element="${words[${#words[@]}-1]}"
99+
100+
# Capitalize last element using sed
101+
last_element=$(echo "$last_element" | awk '{for(i=1;i<=NF;i++) $i=toupper(substr($i,1,1)) substr($i,2)}1')
102+
103+
# Add the element into class_names
104+
class_names+=("$last_element")
105+
done
106+
107+
echo "${class_names[@]}"
108+
}
109+
110+
function perform_checks_on_feature_flags() {
111+
feature_flags=($(get_classes_from_constants_file))
112+
imported_classes=($(get_imported_classes_in_logger))
113+
flags_added_to_map=($(get_flags_added_into_the_logging_map))
114+
115+
file_path=$(find . -name FeatureFlagsLogger.kt)
116+
imports_line_number=$(grep -n 'class FeatureFlagsLogger @Inject constructor(' "$file_path" | head -n 1 | awk -F: '{print $1}')
117+
flags_map_line_number=$(grep -n 'Map<String, PlatformParameterValue<Boolean>> = mapOf(' "$file_path" | head -n 1 | awk -F: '{print $1}')
118+
119+
for element in "${feature_flags[@]}"; do
120+
in_array=$(item_in_array "$element" "${imported_classes[@]}")
121+
if [[ $in_array -ne 1 ]]; then
122+
failed_checks=$((failed_checks + 1))
123+
echo "$element is not imported in the constructor argument in $file_path at line $imports_line_number"
124+
fi
125+
done
126+
127+
for element in "${feature_flags[@]}"; do
128+
in_array=$(item_in_array "$element" "${flags_added_to_map[@]}")
129+
if [[ $in_array -ne 1 ]]; then
130+
failed_checks=$((failed_checks + 1))
131+
echo "$element is not added to the logging map in $file_path at line $flags_map_line_number"
132+
fi
133+
done
134+
135+
if [[ $failed_checks -eq 0 ]]; then
136+
echo "Feature flag checks completed successfully"
137+
exit 0
138+
else
139+
echo "********************************"
140+
echo "Feature flag issues found."
141+
echo "Please fix the above issues."
142+
echo "********************************"
143+
exit 1
144+
fi
145+
}
146+
147+
perform_checks_on_feature_flags

scripts/static_checks.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ echo ""
1919
bash scripts/ktlint_lint_check.sh
2020
echo ""
2121

22+
# Run feature flag checks
23+
bash scripts/feature_flags_check.sh
24+
echo ""
25+
2226
# Run protobuf lint checks
2327
bash scripts/buf_lint_check.sh
2428
echo ""

utility/src/main/java/org/oppia/android/util/platformparameter/FeatureFlagConstants.kt

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,6 @@ const val DOWNLOADS_SUPPORT = "android_enable_downloads_support"
2424
/** Default value for feature flag corresponding to [EnableDownloadsSupport]. */
2525
const val ENABLE_DOWNLOADS_SUPPORT_DEFAULT_VALUE = false
2626

27-
/** Qualifier for the feature flag corresponding to enabling the language selection UI. */
28-
@Qualifier
29-
annotation class EnableLanguageSelectionUi
30-
31-
/** Default value for the feature flag corresponding to [EnableLanguageSelectionUi]. */
32-
const val ENABLE_LANGUAGE_SELECTION_UI_DEFAULT_VALUE = true
33-
3427
/**
3528
* Qualifier for the feature flag corresponding to enabling the extra topic tabs: practice and info.
3629
*/

0 commit comments

Comments
 (0)