Skip to content

Commit

Permalink
Fixes 5445: Add checks for feature flags (#5464)
Browse files Browse the repository at this point in the history
<!-- 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]>
  • Loading branch information
kkmurerwa and kennethmurerwa authored Jul 19, 2024
1 parent 436cf9e commit 4b49cd8
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 8 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ jobs:
run: |
bash /home/runner/work/oppia-android/oppia-android/scripts/ktlint_lint_check.sh $HOME
- name: Feature flag checks
run: |
bash /home/runner/work/oppia-android/oppia-android/scripts/feature_flags_check.sh $HOME
- name: Protobuf lint check
run: |
bash /home/runner/work/oppia-android/oppia-android/scripts/buf_lint_check.sh $HOME
Expand Down
2 changes: 1 addition & 1 deletion scripts/assets/todo_open_exemptions.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ todo_open_exemption {
}
todo_open_exemption {
exempted_file_path: "scripts/static_checks.sh"
line_number: 110
line_number: 114
}
todo_open_exemption {
exempted_file_path: "wiki/Coding-style-guide.md"
Expand Down
147 changes: 147 additions & 0 deletions scripts/feature_flags_check.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
#!/bin/bash

echo "********************************"
echo "Running feature flag checks"
echo "********************************"

failed_checks=0

function item_in_array() {
local item="$1"
shift
local array=("$@")

for element in "${array[@]}"; do
if [[ "$element" == "$item" ]]; then
echo 1
return
fi
done

echo 0
}

function get_classes_from_constants_file() {
# Get the file path of the FeatureFlagConstants File
file_path=$(find . -name FeatureFlagConstants.kt)

# Get a list of feature flag annotation classes
annotation_classes=$(grep -E '[\s\w]*annotation\s*class' "$file_path")

# Convert the string into an array, splitting by newline
IFS=$'\n' read -rd '' -a array <<<"$annotation_classes"

# Create an empty array to hold individual class names
class_names=()

# Iterate through each line and take the last word of the list
for line in "${array[@]}"; do
# Convert each line into an array of words, splitting by space
IFS=' ' read -ra words <<<"$line"

# Get last element of the list
last_element="${words[${#words[@]}-1]}"

# Add the element into class_names
class_names+=("$last_element")
done

echo "${class_names[@]}"
}

function get_imported_classes_in_logger() {
# File path containing the block of text
file_path=$(find . -name FeatureFlagsLogger.kt)

# Use sed to extract the block based on a starting pattern and an ending pattern
extracted_block=$(sed -n '/class FeatureFlagsLogger @Inject constructor(/,/^)/p' "$file_path")

# Get annotation classes
logged_classes=$(grep -E '@Enable[A-Za-z0-9_]+' "$file_path")

# Replace the @ symbol and spaces within each element of the list
for i in "${!logged_classes[@]}"; do
logged_classes[$i]=$(echo "${logged_classes[$i]}" | tr -d '@' | tr -d ' ')
done

# Print the logged classes
echo "$logged_classes"
}

function get_flags_added_into_the_logging_map() {
# File path containing the block of text
file_path=$(find . -name FeatureFlagsLogger.kt)

# Use sed to extract the block based on a starting pattern and an ending pattern
extracted_block=$(sed -n '/Map<String, PlatformParameterValue<Boolean>> = mapOf(/,/)$/p' "$file_path")

# Get any word beginning with enable from the extracted block
enable_flags=$(grep -E 'enable[A-Za-z0-9_]+' <<<"$extracted_block")

added_flags=()

# Convert the string into an array, splitting by newline
IFS=$'\n' read -rd '' -a added_flags <<<"$enable_flags"

# Create an empty array to hold individual class names
class_names=()

# Iterate through each line and take the last word of the list
for line in "${added_flags[@]}"; do
# Remove the comma at the end of the line
line=$(echo "$line" | awk '{sub(/,$/, ""); print}')

# Convert each line into an array of words, splitting by space
IFS=' ' read -ra words <<<"$line"

# Get last element of the list
last_element="${words[${#words[@]}-1]}"

# Capitalize last element using sed
last_element=$(echo "$last_element" | awk '{for(i=1;i<=NF;i++) $i=toupper(substr($i,1,1)) substr($i,2)}1')

# Add the element into class_names
class_names+=("$last_element")
done

echo "${class_names[@]}"
}

function perform_checks_on_feature_flags() {
feature_flags=($(get_classes_from_constants_file))
imported_classes=($(get_imported_classes_in_logger))
flags_added_to_map=($(get_flags_added_into_the_logging_map))

file_path=$(find . -name FeatureFlagsLogger.kt)
imports_line_number=$(grep -n 'class FeatureFlagsLogger @Inject constructor(' "$file_path" | head -n 1 | awk -F: '{print $1}')
flags_map_line_number=$(grep -n 'Map<String, PlatformParameterValue<Boolean>> = mapOf(' "$file_path" | head -n 1 | awk -F: '{print $1}')

for element in "${feature_flags[@]}"; do
in_array=$(item_in_array "$element" "${imported_classes[@]}")
if [[ $in_array -ne 1 ]]; then
failed_checks=$((failed_checks + 1))
echo "$element is not imported in the constructor argument in $file_path at line $imports_line_number"
fi
done

for element in "${feature_flags[@]}"; do
in_array=$(item_in_array "$element" "${flags_added_to_map[@]}")
if [[ $in_array -ne 1 ]]; then
failed_checks=$((failed_checks + 1))
echo "$element is not added to the logging map in $file_path at line $flags_map_line_number"
fi
done

if [[ $failed_checks -eq 0 ]]; then
echo "Feature flag checks completed successfully"
exit 0
else
echo "********************************"
echo "Feature flag issues found."
echo "Please fix the above issues."
echo "********************************"
exit 1
fi
}

perform_checks_on_feature_flags
4 changes: 4 additions & 0 deletions scripts/static_checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ echo ""
bash scripts/ktlint_lint_check.sh
echo ""

# Run feature flag checks
bash scripts/feature_flags_check.sh
echo ""

# Run protobuf lint checks
bash scripts/buf_lint_check.sh
echo ""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,6 @@ const val DOWNLOADS_SUPPORT = "android_enable_downloads_support"
/** Default value for feature flag corresponding to [EnableDownloadsSupport]. */
const val ENABLE_DOWNLOADS_SUPPORT_DEFAULT_VALUE = false

/** Qualifier for the feature flag corresponding to enabling the language selection UI. */
@Qualifier
annotation class EnableLanguageSelectionUi

/** Default value for the feature flag corresponding to [EnableLanguageSelectionUi]. */
const val ENABLE_LANGUAGE_SELECTION_UI_DEFAULT_VALUE = true

/**
* Qualifier for the feature flag corresponding to enabling the extra topic tabs: practice and info.
*/
Expand Down

0 comments on commit 4b49cd8

Please sign in to comment.