diff --git a/tools/aconfig/src/codegen/cpp.rs b/tools/aconfig/src/codegen/cpp.rs index 06e5ccab75..d6bebbabf9 100644 --- a/tools/aconfig/src/codegen/cpp.rs +++ b/tools/aconfig/src/codegen/cpp.rs @@ -1028,7 +1028,7 @@ bool com_android_aconfig_test_enabled_ro() { expected_src: &str, ) { let modified_parsed_flags = - crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode); + crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap(); let generated = generate_cpp_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode) .unwrap(); 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 = parsed_flags_iter.map(|pf| create_flag_element(package, &pf)).collect(); - let exported_flag_elements: Vec = - flag_elements.iter().filter(|elem| elem.exported).cloned().collect(); let namespace_flags = gen_flags_by_namespace(&flag_elements); let properties_set: BTreeSet = 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 { #[derive(Serialize)] struct Context { pub flag_elements: Vec, - pub exported_flag_elements: Vec, pub namespace_flags: Vec, 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/codegen/mod.rs b/tools/aconfig/src/codegen/mod.rs index fc61b7b27b..476d2b39b1 100644 --- a/tools/aconfig/src/codegen/mod.rs +++ b/tools/aconfig/src/codegen/mod.rs @@ -55,9 +55,19 @@ pub fn create_device_config_ident(package: &str, flag_name: &str) -> Result std::fmt::Result { + match self { + CodegenMode::Exported => write!(f, "exported"), + CodegenMode::Production => write!(f, "production"), + CodegenMode::Test => write!(f, "test"), + } + } } #[cfg(test)] diff --git a/tools/aconfig/src/codegen/rust.rs b/tools/aconfig/src/codegen/rust.rs index 6cf0b3208e..56cb311754 100644 --- a/tools/aconfig/src/codegen/rust.rs +++ b/tools/aconfig/src/codegen/rust.rs @@ -557,7 +557,7 @@ pub fn enabled_ro_exported() -> bool { fn test_generate_rust_code(mode: CodegenMode) { let parsed_flags = crate::test::parse_test_flags(); let modified_parsed_flags = - crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode); + crate::commands::modify_parsed_flags_based_on_mode(parsed_flags, mode).unwrap(); let generated = generate_rust_code(crate::test::TEST_PACKAGE, modified_parsed_flags.into_iter(), mode) .unwrap(); diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs index 23667bb9df..cd53371eae 100644 --- a/tools/aconfig/src/commands.rs +++ b/tools/aconfig/src/commands.rs @@ -190,17 +190,17 @@ pub fn parse_flags( pub fn create_java_lib(mut input: Input, codegen_mode: CodegenMode) -> Result> { 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> { let parsed_flags = input.try_parse_flags()?; - let modified_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, codegen_mode); + 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"); }; @@ -210,7 +210,7 @@ pub fn create_cpp_lib(mut input: Input, codegen_mode: CodegenMode) -> Result Result { let parsed_flags = input.try_parse_flags()?; - let modified_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, codegen_mode); + 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"); }; @@ -316,22 +316,10 @@ fn find_unique_container(parsed_flags: &ProtoParsedFlags) -> Option<&str> { Some(container) } -fn filter_parsed_flags( - parsed_flags: ProtoParsedFlags, - codegen_mode: CodegenMode, -) -> Vec { - 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, -) -> Vec { +) -> Result> { fn exported_mode_flag_modifier(mut parsed_flag: ProtoParsedFlag) -> ProtoParsedFlag { parsed_flag.set_state(ProtoFlagState::DISABLED); parsed_flag.set_permission(ProtoFlagPermission::READ_WRITE); @@ -339,7 +327,7 @@ pub fn modify_parsed_flags_based_on_mode( parsed_flag } - match codegen_mode { + let modified_parsed_flags: Vec<_> = match codegen_mode { CodegenMode::Exported => parsed_flags .parsed_flag .into_iter() @@ -349,7 +337,12 @@ pub fn modify_parsed_flags_based_on_mode( CodegenMode::Production | CodegenMode::Test => { parsed_flags.parsed_flag.into_iter().collect() } + }; + if modified_parsed_flags.is_empty() { + bail!("{} library contains no exported flags.", codegen_mode); } + + Ok(modified_parsed_flags) } #[cfg(test)] @@ -623,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(); @@ -652,7 +628,8 @@ mod tests { fn test_modify_parsed_flags_based_on_mode_prod() { let parsed_flags = crate::test::parse_test_flags(); let p_parsed_flags = - modify_parsed_flags_based_on_mode(parsed_flags.clone(), CodegenMode::Production); + modify_parsed_flags_based_on_mode(parsed_flags.clone(), CodegenMode::Production) + .unwrap(); assert_eq!(parsed_flags.parsed_flag.len(), p_parsed_flags.len()); for (i, item) in p_parsed_flags.iter().enumerate() { assert!(parsed_flags.parsed_flag[i].eq(item)); @@ -662,7 +639,8 @@ mod tests { #[test] fn test_modify_parsed_flags_based_on_mode_exported() { let parsed_flags = crate::test::parse_test_flags(); - let p_parsed_flags = modify_parsed_flags_based_on_mode(parsed_flags, CodegenMode::Exported); + let p_parsed_flags = + modify_parsed_flags_based_on_mode(parsed_flags, CodegenMode::Exported).unwrap(); assert_eq!(3, p_parsed_flags.len()); for flag in p_parsed_flags.iter() { assert_eq!(ProtoFlagState::DISABLED, flag.state()); @@ -670,5 +648,14 @@ mod tests { assert!(!flag.is_fixed_read_only()); assert!(flag.is_exported()); } + + let mut parsed_flags = crate::test::parse_test_flags(); + parsed_flags.parsed_flag.retain_mut(|pf| !pf.is_exported()); + let error = + modify_parsed_flags_based_on_mode(parsed_flags, CodegenMode::Exported).unwrap_err(); + assert_eq!( + format!("{} library contains no exported flags.", CodegenMode::Exported), + format!("{:?}", error) + ); } } 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 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