From 4b49cd81bf24f8e01f987b97b64b146e43bf95e2 Mon Sep 17 00:00:00 2001 From: Kenneth Murerwa Date: Fri, 19 Jul 2024 12:09:02 +0300 Subject: [PATCH] Fixes 5445: Add checks for feature flags (#5464) ## 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 ## Essential Checklist - [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 --- .github/workflows/static_checks.yml | 4 + scripts/assets/todo_open_exemptions.textproto | 2 +- scripts/feature_flags_check.sh | 147 ++++++++++++++++++ scripts/static_checks.sh | 4 + .../platformparameter/FeatureFlagConstants.kt | 7 - 5 files changed, 156 insertions(+), 8 deletions(-) create mode 100644 scripts/feature_flags_check.sh diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml index b312a43f067..7aaa1ee11ab 100644 --- a/.github/workflows/static_checks.yml +++ b/.github/workflows/static_checks.yml @@ -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 diff --git a/scripts/assets/todo_open_exemptions.textproto b/scripts/assets/todo_open_exemptions.textproto index 715fdf0bf32..8dd53a51ce9 100644 --- a/scripts/assets/todo_open_exemptions.textproto +++ b/scripts/assets/todo_open_exemptions.textproto @@ -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" diff --git a/scripts/feature_flags_check.sh b/scripts/feature_flags_check.sh new file mode 100644 index 00000000000..48e61153a70 --- /dev/null +++ b/scripts/feature_flags_check.sh @@ -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> = 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> = 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 diff --git a/scripts/static_checks.sh b/scripts/static_checks.sh index f7ad2a39d8e..0a0d08206cb 100644 --- a/scripts/static_checks.sh +++ b/scripts/static_checks.sh @@ -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 "" diff --git a/utility/src/main/java/org/oppia/android/util/platformparameter/FeatureFlagConstants.kt b/utility/src/main/java/org/oppia/android/util/platformparameter/FeatureFlagConstants.kt index e125d2a1fc6..c30ae97206c 100644 --- a/utility/src/main/java/org/oppia/android/util/platformparameter/FeatureFlagConstants.kt +++ b/utility/src/main/java/org/oppia/android/util/platformparameter/FeatureFlagConstants.kt @@ -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. */