aconfig: modify and filter flags before passing into java codegen

Before this change java codegen filter flags for exported mode in the
template. This change move the filter process to commands as other
codegen. Thus the codegen code will only generate code based on the
passed in flags.

Bug: 311152507
Test: atest aconfig.test aconfig.test.java AconfigJavaHostTest
Change-Id: I74045709cde19e6c687c3eb0d94050ea40cf5042

6 files changed, 28 insertions, 142 deletions

diff --git a/tools/aconfig/src/codegen/java.rs b/tools/aconfig/src/codegen/java.rs
index 6a7d7c1e27..a02a7e2d90 100644
--- a/tools/aconfig/src/codegen/java.rs
+++ b/tools/aconfig/src/codegen/java.rs
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-use anyhow::{anyhow, Result};
+use anyhow::Result;
 use serde::Serialize;
 use std::collections::{BTreeMap, BTreeSet};
 use std::path::PathBuf;
@@ -35,8 +35,6 @@ where
 {
     let flag_elements: Vec<FlagElement> =
         parsed_flags_iter.map(|pf| create_flag_element(package, &pf)).collect();
-    let exported_flag_elements: Vec<FlagElement> =
-        flag_elements.iter().filter(|elem| elem.exported).cloned().collect();
     let namespace_flags = gen_flags_by_namespace(&flag_elements);
     let properties_set: BTreeSet<String> =
         flag_elements.iter().map(|fe| format_property_name(&fe.device_config_namespace)).collect();
@@ -45,13 +43,8 @@ where
     let runtime_lookup_required =
         flag_elements.iter().any(|elem| elem.is_read_write) || library_exported;
 
-    if library_exported && exported_flag_elements.is_empty() {
-        return Err(anyhow!("exported library contains no exported flags"));
-    }
-
     let context = Context {
         flag_elements,
-        exported_flag_elements,
         namespace_flags,
         is_test_mode,
         runtime_lookup_required,
@@ -110,7 +103,6 @@ fn gen_flags_by_namespace(flags: &[FlagElement]) -> Vec<NamespaceFlags> {
 #[derive(Serialize)]
 struct Context {
     pub flag_elements: Vec<FlagElement>,
-    pub exported_flag_elements: Vec<FlagElement>,
     pub namespace_flags: Vec<NamespaceFlags>,
     pub is_test_mode: bool,
     pub runtime_lookup_required: bool,
@@ -134,7 +126,6 @@ struct FlagElement {
     pub is_read_write: bool,
     pub method_name: String,
     pub properties: String,
-    pub exported: bool,
 }
 
 fn create_flag_element(package: &str, pf: &ProtoParsedFlag) -> FlagElement {
@@ -148,7 +139,6 @@ fn create_flag_element(package: &str, pf: &ProtoParsedFlag) -> FlagElement {
         is_read_write: pf.permission() == ProtoFlagPermission::READ_WRITE,
         method_name: format_java_method_name(pf.name()),
         properties: format_property_name(pf.namespace()),
-        exported: pf.is_exported.unwrap_or(false),
     }
 }
 
@@ -376,12 +366,12 @@ mod tests {
     #[test]
     fn test_generate_java_code_production() {
         let parsed_flags = crate::test::parse_test_flags();
-        let generated_files = generate_java_code(
-            crate::test::TEST_PACKAGE,
-            parsed_flags.parsed_flag.into_iter(),
-            CodegenMode::Production,
-        )
-        .unwrap();
+        let mode = CodegenMode::Production;
+        let modified_parsed_flags =
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
+        let generated_files =
+            generate_java_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode)
+                .unwrap();
         let expect_flags_content = EXPECTED_FLAG_COMMON_CONTENT.to_string()
             + r#"
             private static FeatureFlags FEATURE_FLAGS = new FeatureFlagsImpl();
@@ -534,12 +524,12 @@ mod tests {
     #[test]
     fn test_generate_java_code_exported() {
         let parsed_flags = crate::test::parse_test_flags();
-        let generated_files = generate_java_code(
-            crate::test::TEST_PACKAGE,
-            parsed_flags.parsed_flag.into_iter(),
-            CodegenMode::Exported,
-        )
-        .unwrap();
+        let mode = CodegenMode::Exported;
+        let modified_parsed_flags =
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
+        let generated_files =
+            generate_java_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode)
+                .unwrap();
 
         let expect_flags_content = r#"
         package com.android.aconfig.test;
@@ -594,7 +584,6 @@ mod tests {
         /** @hide */
         public final class FeatureFlagsImpl implements FeatureFlags {
             private static boolean aconfig_test_is_cached = false;
-            private static boolean other_namespace_is_cached = false;
             private static boolean disabledRwExported = false;
             private static boolean enabledFixedRoExported = false;
             private static boolean enabledRoExported = false;
@@ -622,22 +611,6 @@ mod tests {
                 aconfig_test_is_cached = true;
             }
 
-            private void load_overrides_other_namespace() {
-                try {
-                    Properties properties = DeviceConfig.getProperties("other_namespace");
-                } catch (NullPointerException e) {
-                    throw new RuntimeException(
-                        "Cannot read value from namespace other_namespace "
-                        + "from DeviceConfig. It could be that the code using flag "
-                        + "executed before SettingsProvider initialization. Please use "
-                        + "fixed read-only flag by adding is_fixed_read_only: true in "
-                        + "flag declaration.",
-                        e
-                    );
-                }
-                other_namespace_is_cached = true;
-            }
-
             @Override
             @UnsupportedAppUsage
             public boolean disabledRwExported() {
@@ -751,12 +724,12 @@ mod tests {
     #[test]
     fn test_generate_java_code_test() {
         let parsed_flags = crate::test::parse_test_flags();
-        let generated_files = generate_java_code(
-            crate::test::TEST_PACKAGE,
-            parsed_flags.parsed_flag.into_iter(),
-            CodegenMode::Test,
-        )
-        .unwrap();
+        let mode = CodegenMode::Test;
+        let modified_parsed_flags =
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
+        let generated_files =
+            generate_java_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode)
+                .unwrap();
 
         let expect_flags_content = EXPECTED_FLAG_COMMON_CONTENT.to_string()
             + r#"
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs
index 7d9dd69354..cd53371eae 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -190,12 +190,12 @@ pub fn parse_flags(
 
 pub fn create_java_lib(mut input: Input, codegen_mode: CodegenMode) -> Result<Vec<OutputFile>> {
     let parsed_flags = input.try_parse_flags()?;
-    let filtered_parsed_flags = filter_parsed_flags(parsed_flags, codegen_mode);
-    let Some(package) = find_unique_package(&filtered_parsed_flags) else {
+    let modified_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, codegen_mode)?;
+    let Some(package) = find_unique_package(&modified_parsed_flags) else {
         bail!("no parsed flags, or the parsed flags use different packages");
     };
     let package = package.to_string();
-    generate_java_code(&package, filtered_parsed_flags.into_iter(), codegen_mode)
+    generate_java_code(&package, modified_parsed_flags.into_iter(), codegen_mode)
 }
 
 pub fn create_cpp_lib(mut input: Input, codegen_mode: CodegenMode) -> Result<Vec<OutputFile>> {
@@ -316,18 +316,6 @@ fn find_unique_container(parsed_flags: &ProtoParsedFlags) -> Option<&str> {
     Some(container)
 }
 
-fn filter_parsed_flags(
-    parsed_flags: ProtoParsedFlags,
-    codegen_mode: CodegenMode,
-) -> Vec<ProtoParsedFlag> {
-    match codegen_mode {
-        CodegenMode::Exported => {
-            parsed_flags.parsed_flag.into_iter().filter(|pf| pf.is_exported()).collect()
-        }
-        _ => parsed_flags.parsed_flag,
-    }
-}
-
 pub fn modify_parsed_flags_based_on_mode(
     parsed_flags: ProtoParsedFlags,
     codegen_mode: CodegenMode,
@@ -628,23 +616,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn test_filter_parsed_flags() {
-        let mut input = parse_test_flags_as_input();
-        let parsed_flags = input.try_parse_flags().unwrap();
-
-        let filtered_parsed_flags =
-            filter_parsed_flags(parsed_flags.clone(), CodegenMode::Exported);
-        assert_eq!(3, filtered_parsed_flags.len());
-
-        let filtered_parsed_flags =
-            filter_parsed_flags(parsed_flags.clone(), CodegenMode::Production);
-        assert_eq!(9, filtered_parsed_flags.len());
-
-        let filtered_parsed_flags = filter_parsed_flags(parsed_flags.clone(), CodegenMode::Test);
-        assert_eq!(9, filtered_parsed_flags.len());
-    }
-
     fn parse_test_flags_as_input() -> Input {
         let parsed_flags = crate::test::parse_test_flags();
         let binary_proto = parsed_flags.write_to_bytes().unwrap();
diff --git a/tools/aconfig/templates/FakeFeatureFlagsImpl.java.template b/tools/aconfig/templates/FakeFeatureFlagsImpl.java.template
index 8010b884c4..933d6a77cb 100644
--- a/tools/aconfig/templates/FakeFeatureFlagsImpl.java.template
+++ b/tools/aconfig/templates/FakeFeatureFlagsImpl.java.template
@@ -12,23 +12,11 @@ public class FakeFeatureFlagsImpl implements FeatureFlags \{
     }
 
 {{ for item in flag_elements}}
-{{ if library_exported }}
-
-{{ if item.exported }}
-    @Override
-    @UnsupportedAppUsage
-    public boolean {item.method_name}() \{
-        return getValue(Flags.FLAG_{item.flag_name_constant_suffix});
-    }
-{{ endif }}
-
-{{ else }}
     @Override
     @UnsupportedAppUsage
     public boolean {item.method_name}() \{
         return getValue(Flags.FLAG_{item.flag_name_constant_suffix});
     }
-{{ endif }}
 {{ endfor}}
     public void setFlag(String flagName, boolean value) \{
         if (!this.mFlagMap.containsKey(flagName)) \{
@@ -52,20 +40,11 @@ public class FakeFeatureFlagsImpl implements FeatureFlags \{
     }
 
     private Map<String, Boolean> mFlagMap = new HashMap<>(
-        {{ if library_exported }}
-        Map.ofEntries(
-            {{-for item in exported_flag_elements}}
-            Map.entry(Flags.FLAG_{item.flag_name_constant_suffix}, false)
-            {{ -if not @last }},{{ endif }}
-            {{ -endfor }}
-        )
-        {{ else }}
         Map.ofEntries(
             {{-for item in flag_elements}}
             Map.entry(Flags.FLAG_{item.flag_name_constant_suffix}, false)
             {{ -if not @last }},{{ endif }}
             {{ -endfor }}
         )
-        {{ endif }}
     );
 }
diff --git a/tools/aconfig/templates/FeatureFlags.java.template b/tools/aconfig/templates/FeatureFlags.java.template
index 180f8828d2..d6af62cca8 100644
--- a/tools/aconfig/templates/FeatureFlags.java.template
+++ b/tools/aconfig/templates/FeatureFlags.java.template
@@ -6,14 +6,9 @@ import android.compat.annotation.UnsupportedAppUsage;
 public interface FeatureFlags \{
 {{ for item in flag_elements }}
 {{ if library_exported }}
-
-{{ if item.exported }}
     @UnsupportedAppUsage
     boolean {item.method_name}();
-{{ endif }}
-
 {{ else }}
-
 {{ -if not item.is_read_write }}
 {{ -if item.default_value }}
     @com.android.aconfig.annotations.AssumeTrueForR8
@@ -23,7 +18,6 @@ public interface FeatureFlags \{
 {{ endif }}
     @UnsupportedAppUsage
     boolean {item.method_name}();
-
 {{ endif }}
 {{ endfor }}
 }
diff --git a/tools/aconfig/templates/FeatureFlagsImpl.java.template b/tools/aconfig/templates/FeatureFlagsImpl.java.template
index 7a52ceb4ee..cf49c17298 100644
--- a/tools/aconfig/templates/FeatureFlagsImpl.java.template
+++ b/tools/aconfig/templates/FeatureFlagsImpl.java.template
@@ -15,12 +15,8 @@ public final class FeatureFlagsImpl implements FeatureFlags \{
 
 {{ for flag in flag_elements }}
 {{ if library_exported }}
-{{ if flag.exported }}
     private static boolean {flag.method_name} = false;
-{{ endif }}
-
 {{ else }}
-
 {{- if flag.is_read_write }}
     private static boolean {flag.method_name} = {flag.default_value};
 {{- endif- }}
@@ -34,19 +30,13 @@ public final class FeatureFlagsImpl implements FeatureFlags \{
 
             {{- for flag in namespace_with_flags.flags }}
             {{ if library_exported }}
-
-            {{ if flag.exported }}
             {flag.method_name} =
                 properties.getBoolean("{flag.device_config_flag}", false);
-            {{ endif }}
-
             {{ else }}
-
             {{ if flag.is_read_write }}
             {flag.method_name} =
                 properties.getBoolean("{flag.device_config_flag}", {flag.default_value});
             {{ endif }}
-
             {{ endif }}
             {{ endfor }}
         } catch (NullPointerException e) \{
@@ -65,23 +55,15 @@ public final class FeatureFlagsImpl implements FeatureFlags \{
 {{ endif- }}
 
 {{ for flag in flag_elements }}
-{{ if library_exported }}
-
-{{ if flag.exported }}
     @Override
     @UnsupportedAppUsage
     public boolean {flag.method_name}() \{
+    {{ -if library_exported }}
         if (!{flag.device_config_namespace}_is_cached) \{
             load_overrides_{flag.device_config_namespace}();
         }
         return {flag.method_name};
-    }
-{{ endif }}
-
-{{ else }}
-    @Override
-    @UnsupportedAppUsage
-    public boolean {flag.method_name}() \{
+    {{ else }}
     {{ -if flag.is_read_write }}
         if (!{flag.device_config_namespace}_is_cached) \{
             load_overrides_{flag.device_config_namespace}();
@@ -89,10 +71,9 @@ public final class FeatureFlagsImpl implements FeatureFlags \{
         return {flag.method_name};
     {{ else }}
         return {flag.default_value};
-    {{ endif- }}
+    {{ -endif- }}
+    {{ -endif }}
     }
-{{ endif }}
-
 {{ endfor }}
 }
 {{ else }}
diff --git a/tools/aconfig/templates/Flags.java.template b/tools/aconfig/templates/Flags.java.template
index 9f4c52f978..ff942e5bfc 100644
--- a/tools/aconfig/templates/Flags.java.template
+++ b/tools/aconfig/templates/Flags.java.template
@@ -6,28 +6,16 @@ import android.compat.annotation.UnsupportedAppUsage;
 /** @hide */
 public final class Flags \{
 {{- for item in flag_elements}}
-    {{ if library_exported }}
-    {{ if item.exported }}
     /** @hide */
     public static final String FLAG_{item.flag_name_constant_suffix} = "{item.device_config_flag}";
-    {{ endif }}
-    {{ else }}
-    /** @hide */
-    public static final String FLAG_{item.flag_name_constant_suffix} = "{item.device_config_flag}";
-    {{ endif }}
 {{- endfor }}
 {{ for item in flag_elements}}
-{{ if library_exported }}
-
-{{ if item.exported }}
+{{ -if library_exported }}
     @UnsupportedAppUsage
     public static boolean {item.method_name}() \{
         return FEATURE_FLAGS.{item.method_name}();
     }
-{{ endif }}
-
-{{ else }}
-
+{{ -else }}
 {{ -if not item.is_read_write }}
 {{ -if item.default_value }}
     @com.android.aconfig.annotations.AssumeTrueForR8