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

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,11 +366,11 @@ mod tests {
     #[test]
     fn test_generate_java_code_production() {
         let parsed_flags = crate::test::parse_test_flags();
-        let generated_files = generate_java_code(
+        let mode = CodegenMode::Production;
-            crate::test::TEST_PACKAGE,
+        let modified_parsed_flags =
-            parsed_flags.parsed_flag.into_iter(),
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
-            CodegenMode::Production,
+        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#"
@@ -534,11 +524,11 @@ mod tests {
     #[test]
     fn test_generate_java_code_exported() {
         let parsed_flags = crate::test::parse_test_flags();
-        let generated_files = generate_java_code(
+        let mode = CodegenMode::Exported;
-            crate::test::TEST_PACKAGE,
+        let modified_parsed_flags =
-            parsed_flags.parsed_flag.into_iter(),
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
-            CodegenMode::Exported,
+        let generated_files =
-        )
+            generate_java_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode)
                 .unwrap();
 
         let expect_flags_content = r#"
@@ -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,11 +724,11 @@ mod tests {
     #[test]
     fn test_generate_java_code_test() {
         let parsed_flags = crate::test::parse_test_flags();
-        let generated_files = generate_java_code(
+        let mode = CodegenMode::Test;
-            crate::test::TEST_PACKAGE,
+        let modified_parsed_flags =
-            parsed_flags.parsed_flag.into_iter(),
+            crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap();
-            CodegenMode::Test,
+        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()

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 modified_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, codegen_mode)?;
-    let Some(package) = find_unique_package(&filtered_parsed_flags) else {
+    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();

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 }}
     );
 }

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 }}
 }

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}() \{
     {{ -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 }}

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 library_exported }}
-
-{{ if item.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