Jeff DeCew
c3bc24f33b
Generate CustomFeatureFlags
...
* Creates a new general-purpose CustomFeatureFlags class
* Simplifies FakeFeatureFlagsImpl to be based on that
* This allows teams to fake FeatureFlags without having to make changes per flag.
* This allows SetFlagsRule to inject an instance of FeatureFlags that would detect if a flag has been read.
Bug: 336768870
Flag: none
Test: presubmit
Change-Id: Id3c2530d484fa5584c46d11381fcfc0ab294f33f
NOTE FOR REVIEWERS - original patch and result patch are not identical.
PLEASE REVIEW CAREFULLY.
Diffs between the patches:
"CustomFeatureFlags.java",
> + include_str!("../../templates/CustomFeatureFlags.java.template"),
> + )?;
> + template.add_template(
> - ["Flags.java", "FeatureFlags.java", "FeatureFlagsImpl.java", "FakeFeatureFlagsImpl.java"]
> - .iter()
> - .map(|file| {
> - Ok(OutputFile {
> - contents: template.render(file, &context)?.into(),
> - path: path.join(file),
> - })
> - })
> - .collect::<Result<Vec<OutputFile>>>()
> + [
> + "Flags.java",
> + "FeatureFlags.java",
> + "FeatureFlagsImpl.java",
> + "CustomFeatureFlags.java",
> + "FakeFeatureFlagsImpl.java",
> + ]
> + .iter()
> + .map(|file| {
> + Ok(OutputFile { contents: template.render(file, &context)?.into(), path: path.join(file) })
> + })
> + .collect::<Result<Vec<OutputFile>>>()
> - const EXPECTED_FAKEFEATUREFLAGSIMPL_CONTENT: &str = r#"
> + const EXPECTED_CUSTOMFEATUREFLAGS_CONTENT: &str = r#"
> +
> - import java.util.HashMap;
> - import java.util.Map;
> + import java.util.List;
> + import java.util.function.BiPredicate;
> + import java.util.function.Predicate;
> +
> - public class FakeFeatureFlagsImpl implements FeatureFlags {
> - public FakeFeatureFlagsImpl() {
> - resetAll();
> + public class CustomFeatureFlags implements FeatureFlags {
> +
> + private BiPredicate<String, Predicate<FeatureFlags>> mGetValueImpl;
> +
> + public CustomFeatureFlags(BiPredicate<String, Predicate<FeatureFlags>> getValueImpl) {
> + mGetValueImpl = getValueImpl;
> +
> - return getValue(Flags.FLAG_DISABLED_RO);
> + return getValue(Flags.FLAG_DISABLED_RO,
> + FeatureFlags::disabledRo);
> - return getValue(Flags.FLAG_DISABLED_RW);
> + return getValue(Flags.FLAG_DISABLED_RW,
> + FeatureFlags::disabledRw);
> - return getValue(Flags.FLAG_DISABLED_RW_EXPORTED);
> + return getValue(Flags.FLAG_DISABLED_RW_EXPORTED,
> + FeatureFlags::disabledRwExported);
> - return getValue(Flags.FLAG_DISABLED_RW_IN_OTHER_NAMESPACE);
> + return getValue(Flags.FLAG_DISABLED_RW_IN_OTHER_NAMESPACE,
> + FeatureFlags::disabledRwInOtherNamespace);
> - return getValue(Flags.FLAG_ENABLED_FIXED_RO);
> + return getValue(Flags.FLAG_ENABLED_FIXED_RO,
> + FeatureFlags::enabledFixedRo);
> - return getValue(Flags.FLAG_ENABLED_FIXED_RO_EXPORTED);
> + return getValue(Flags.FLAG_ENABLED_FIXED_RO_EXPORTED,
> + FeatureFlags::enabledFixedRoExported);
> - return getValue(Flags.FLAG_ENABLED_RO);
> + return getValue(Flags.FLAG_ENABLED_RO,
> + FeatureFlags::enabledRo);
> - return getValue(Flags.FLAG_ENABLED_RO_EXPORTED);
> + return getValue(Flags.FLAG_ENABLED_RO_EXPORTED,
> + FeatureFlags::enabledRoExported);
> - return getValue(Flags.FLAG_ENABLED_RW);
> + return getValue(Flags.FLAG_ENABLED_RW,
> + FeatureFlags::enabledRw);
> - public void setFlag(String flagName, boolean value) {
> - if (!this.mFlagMap.containsKey(flagName)) {
> - throw new IllegalArgumentException("no such flag " + flagName);
> - }
> - this.mFlagMap.put(flagName, value);
> - }
> - public void resetAll() {
> - for (Map.Entry entry : mFlagMap.entrySet()) {
> - entry.setValue(null);
> - }
> - }
> +
> +
> - private boolean getValue(String flagName) {
> - Boolean value = this.mFlagMap.get(flagName);
> - if (value == null) {
> - throw new IllegalArgumentException(flagName + " is not set");
> - }
> - return value;
> +
> + protected boolean getValue(String flagName, Predicate<FeatureFlags> getter) {
> + return mGetValueImpl.test(flagName, getter);
> - private Map<String, Boolean> mFlagMap = new HashMap<>(
> - Map.ofEntries(
> - Map.entry(Flags.FLAG_DISABLED_RO, false),
> - Map.entry(Flags.FLAG_DISABLED_RW, false),
> - Map.entry(Flags.FLAG_DISABLED_RW_EXPORTED, false),
> - Map.entry(Flags.FLAG_DISABLED_RW_IN_OTHER_NAMESPACE, false),
> - Map.entry(Flags.FLAG_ENABLED_FIXED_RO, false),
> - Map.entry(Flags.FLAG_ENABLED_FIXED_RO_EXPORTED, false),
> - Map.entry(Flags.FLAG_ENABLED_RO, false),
> - Map.entry(Flags.FLAG_ENABLED_RO_EXPORTED, false),
> - Map.entry(Flags.FLAG_ENABLED_RW, false)
> - )
> - );
> +
> + public List<String> getFlagNames() {
> + return Arrays.asList(
> + Flags.FLAG_DISABLED_RO,
> + Flags.FLAG_DISABLED_RW,
> + Flags.FLAG_DISABLED_RW_EXPORTED,
> + Flags.FLAG_DISABLED_RW_IN_OTHER_NAMESPACE,
> + Flags.FLAG_ENABLED_FIXED_RO,
> + Flags.FLAG_ENABLED_FIXED_RO_EXPORTED,
> + Flags.FLAG_ENABLED_RO,
> + Flags.FLAG_ENABLED_RO_EXPORTED,
> + Flags.FLAG_ENABLED_RW
> + );
> + }
> +
> + const EXPECTED_FAKEFEATUREFLAGSIMPL_CONTENT: &str = r#"
> + package com.android.aconfig.test;
> +
> + import java.util.HashMap;
> + import java.util.Map;
> + import java.util.function.Predicate;
> +
> + /** @hide */
> + public class FakeFeatureFlagsImpl extends CustomFeatureFlags {
> + private Map<String, Boolean> mFlagMap = new HashMap<>();
> +
> + public FakeFeatureFlagsImpl() {
> + super(null);
> + // Initialize the map with null values
> + for (String flagName : getFlagNames()) {
> + mFlagMap.put(flagName, null);
> + }
> + }
> +
> + @Override
> + protected boolean getValue(String flagName, Predicate<FeatureFlags> getter) {
> + Boolean value = this.mFlagMap.get(flagName);
> + if (value == null) {
> + throw new IllegalArgumentException(flagName + " is not set");
> + }
> + return value;
> + }
> +
> + public void setFlag(String flagName, boolean value) {
> + if (!this.mFlagMap.containsKey(flagName)) {
> + throw new IllegalArgumentException("no such flag " + flagName);
> + }
> + this.mFlagMap.put(flagName, value);
> + }
> +
> + public void resetAll() {
> + for (Map.Entry entry : mFlagMap.entrySet()) {
> + entry.setValue(null);
> + }
> + }
> + }
> + "#;
> +
> + "com/android/aconfig/test/CustomFeatureFlags.java",
> + EXPECTED_CUSTOMFEATUREFLAGS_CONTENT,
> + ),
> + (
> - let expect_fake_feature_flags_impl_content = r#"
> + let expect_custom_feature_flags_content = r#"
> +
> - import java.util.HashMap;
> - import java.util.Map;
> + import java.util.List;
> + import java.util.function.BiPredicate;
> + import java.util.function.Predicate;
> +
> - public class FakeFeatureFlagsImpl implements FeatureFlags {
> - public FakeFeatureFlagsImpl() {
> - resetAll();
> + public class CustomFeatureFlags implements FeatureFlags {
> +
> + private BiPredicate<String, Predicate<FeatureFlags>> mGetValueImpl;
> +
> + public CustomFeatureFlags(BiPredicate<String, Predicate<FeatureFlags>> getValueImpl) {
> + mGetValueImpl = getValueImpl;
> +
> - return getValue(Flags.FLAG_DISABLED_RW_EXPORTED);
> + return getValue(Flags.FLAG_DISABLED_RW_EXPORTED,
> + FeatureFlags::disabledRwExported);
> - return getValue(Flags.FLAG_ENABLED_FIXED_RO_EXPORTED);
> + return getValue(Flags.FLAG_ENABLED_FIXED_RO_EXPORTED,
> + FeatureFlags::enabledFixedRoExported);
> - return getValue(Flags.FLAG_ENABLED_RO_EXPORTED);
> + return getValue(Flags.FLAG_ENABLED_RO_EXPORTED,
> + FeatureFlags::enabledRoExported);
> - public void setFlag(String flagName, boolean value) {
> - if (!this.mFlagMap.containsKey(flagName)) {
> - throw new IllegalArgumentException("no such flag " + flagName);
> - }
> - this.mFlagMap.put(flagName, value);
> +
> + protected boolean getValue(String flagName, Predicate<FeatureFlags> getter) {
> + return mGetValueImpl.test(flagName, getter);
> - public void resetAll() {
> - for (Map.Entry entry : mFlagMap.entrySet()) {
> - entry.setValue(null);
> - }
> +
> + public List<String> getFlagNames() {
> + return Arrays.asList(
> + Flags.FLAG_DISABLED_RW_EXPORTED,
> + Flags.FLAG_ENABLED_FIXED_RO_EXPORTED,
> + Flags.FLAG_ENABLED_RO_EXPORTED
> + );
> - private boolean getValue(String flagName) {
> - Boolean value = this.mFlagMap.get(flagName);
> - if (value == null) {
> - throw new IllegalArgumentException(flagName + " is not set");
> - }
> - return value;
> - }
> - private Map<String, Boolean> mFlagMap = new HashMap<>(
> - Map.ofEntries(
> - Map.entry(Flags.FLAG_DISABLED_RW_EXPORTED, false),
> - Map.entry(Flags.FLAG_ENABLED_FIXED_RO_EXPORTED, false),
> - Map.entry(Flags.FLAG_ENABLED_RO_EXPORTED, false)
> - )
> - );
> +
> + "com/android/aconfig/test/CustomFeatureFlags.java",
> + expect_custom_feature_flags_content,
> + ),
> + (
> - expect_fake_feature_flags_impl_content,
> + EXPECTED_FAKEFEATUREFLAGSIMPL_CONTENT,
> + "com/android/aconfig/test/CustomFeatureFlags.java",
> + EXPECTED_CUSTOMFEATUREFLAGS_CONTENT,
> + ),
> + (
> - let expect_fakefeatureflags_content = r#"
> + let expect_customfeatureflags_content = r#"
> +
> - import java.util.HashMap;
> - import java.util.Map;
> + import java.util.List;
> + import java.util.function.BiPredicate;
> + import java.util.function.Predicate;
> +
> - public class FakeFeatureFlagsImpl implements FeatureFlags {
> - public FakeFeatureFlagsImpl() {
> - resetAll();
> + public class CustomFeatureFlags implements FeatureFlags {
> +
> + private BiPredicate<String, Predicate<FeatureFlags>> mGetValueImpl;
> +
> + public CustomFeatureFlags(BiPredicate<String, Predicate<FeatureFlags>> getValueImpl) {
> + mGetValueImpl = getValueImpl;
> +
> - return getValue(Flags.FLAG_DISABLED_RO);
> + return getValue(Flags.FLAG_DISABLED_RO,
> + FeatureFlags::disabledRo);
> - return getValue(Flags.FLAG_DISABLED_RW);
> + return getValue(Flags.FLAG_DISABLED_RW,
> + FeatureFlags::disabledRw);
> - return getValue(Flags.FLAG_DISABLED_RW_IN_OTHER_NAMESPACE);
> + return getValue(Flags.FLAG_DISABLED_RW_IN_OTHER_NAMESPACE,
> + FeatureFlags::disabledRwInOtherNamespace);
> - return getValue(Flags.FLAG_ENABLED_FIXED_RO);
> + return getValue(Flags.FLAG_ENABLED_FIXED_RO,
> + FeatureFlags::enabledFixedRo);
> - return getValue(Flags.FLAG_ENABLED_RO);
> + return getValue(Flags.FLAG_ENABLED_RO,
> + FeatureFlags::enabledRo);
> - return getValue(Flags.FLAG_ENABLED_RW);
> + return getValue(Flags.FLAG_ENABLED_RW,
> + FeatureFlags::enabledRw);
> - public void setFlag(String flagName, boolean value) {
> - if (!this.mFlagMap.containsKey(flagName)) {
> - throw new IllegalArgumentException("no such flag " + flagName);
> - }
> - this.mFlagMap.put(flagName, value);
> - }
> - public void resetAll() {
> - for (Map.Entry entry : mFlagMap.entrySet()) {
> - entry.setValue(null);
> - }
> - }
> +
> +
> - private boolean getValue(String flagName) {
> - Boolean value = this.mFlagMap.get(flagName);
> - if (value == null) {
> - throw new IllegalArgumentException(flagName + " is not set");
> - }
> - return value;
> +
> + protected boolean getValue(String flagName, Predicate<FeatureFlags> getter) {
> + return mGetValueImpl.test(flagName, getter);
> - private Map<String, Boolean> mFlagMap = new HashMap<>(
> - Map.ofEntries(
> - Map.entry(Flags.FLAG_DISABLED_RO, false),
> - Map.entry(Flags.FLAG_DISABLED_RW, false),
> - Map.entry(Flags.FLAG_DISABLED_RW_IN_OTHER_NAMESPACE, false),
> - Map.entry(Flags.FLAG_ENABLED_FIXED_RO, false),
> - Map.entry(Flags.FLAG_ENABLED_RO, false),
> - Map.entry(Flags.FLAG_ENABLED_RW, false)
> - )
> - );
> +
> + public List<String> getFlagNames() {
> + return Arrays.asList(
> + Flags.FLAG_DISABLED_RO,
> + Flags.FLAG_DISABLED_RW,
> + Flags.FLAG_DISABLED_RW_IN_OTHER_NAMESPACE,
> + Flags.FLAG_ENABLED_FIXED_RO,
> + Flags.FLAG_ENABLED_RO,
> + Flags.FLAG_ENABLED_RW
> + );
> + }
> +
> +
> - ("com/android/aconfig/test/FakeFeatureFlagsImpl.java", expect_fakefeatureflags_content),
> + ("com/android/aconfig/test/CustomFeatureFlags.java", expect_customfeatureflags_content),
> + (
> + "com/android/aconfig/test/FakeFeatureFlagsImpl.java",
> + EXPECTED_FAKEFEATUREFLAGSIMPL_CONTENT,
> + ),
> --- /dev/null
> +++ tools/aconfig/aconfig/templates/CustomFeatureFlags.java.template
> +package {package_name};
> +
> +{{ if not library_exported- }}
> +// TODO(b/303773055): Remove the annotation after access issue is resolved.
> +import android.compat.annotation.UnsupportedAppUsage;
> +{{ -endif }}
> +import java.util.Arrays;
> +import java.util.HashSet;
> +import java.util.List;
> +import java.util.Set;
> +import java.util.function.BiPredicate;
> +import java.util.function.Predicate;
> +
> +/** @hide */
> +public class CustomFeatureFlags implements FeatureFlags \{
> +
> + private BiPredicate<String, Predicate<FeatureFlags>> mGetValueImpl;
> +
> + public CustomFeatureFlags(BiPredicate<String, Predicate<FeatureFlags>> getValueImpl) \{
> + mGetValueImpl = getValueImpl;
> + }
> +
> +{{ -for item in flag_elements}}
> + @Override
> +{{ if not library_exported }} @UnsupportedAppUsage{{ -endif }}
> + public boolean {item.method_name}() \{
> + return getValue(Flags.FLAG_{item.flag_name_constant_suffix},
> + FeatureFlags::{item.method_name});
> + }
> +{{ endfor }}
> +
> +{{ -if not library_exported }}
> + public boolean isFlagReadOnlyOptimized(String flagName) \{
> + if (mReadOnlyFlagsSet.contains(flagName) &&
> + isOptimizationEnabled()) \{
> + return true;
> + }
> + return false;
> + }
> +
> + @com.android.aconfig.annotations.AssumeTrueForR8
> + private boolean isOptimizationEnabled() \{
> + return false;
> + }
> +{{ -endif }}
> +
> + protected boolean getValue(String flagName, Predicate<FeatureFlags> getter) \{
> + return mGetValueImpl.test(flagName, getter);
> + }
> +
> + public List<String> getFlagNames() \{
> + return Arrays.asList(
> + {{ -for item in flag_elements }}
> + Flags.FLAG_{item.flag_name_constant_suffix}
> + {{ -if not @last }},{{ endif }}
> + {{ -endfor }}
> + );
> + }
> +
> + private Set<String> mReadOnlyFlagsSet = new HashSet<>(
> + Arrays.asList(
> + {{ -for item in flag_elements }}
> + {{ -if not item.is_read_write }}
> + Flags.FLAG_{item.flag_name_constant_suffix},
> + {{ -endif }}
> + {{ -endfor }}
> + ""{# The empty string here is to resolve the ending comma #}
> + )
> + );
> +}
> --- tools/aconfig/aconfig/templates/FakeFeatureFlagsImpl.java.template
> +++ tools/aconfig/aconfig/templates/FakeFeatureFlagsImpl.java.template
> -{{ if not library_exported- }}
> -// TODO(b/303773055): Remove the annotation after access issue is resolved.
> -import android.compat.annotation.UnsupportedAppUsage;
> -{{ -endif }}
> -import java.util.Arrays;
> +
> -import java.util.HashSet;
> -import java.util.Set;
> +import java.util.function.Predicate;
> -public class FakeFeatureFlagsImpl implements FeatureFlags \{
> +public class FakeFeatureFlagsImpl extends CustomFeatureFlags \{
> + private Map<String, Boolean> mFlagMap = new HashMap<>();
> +
> - resetAll();
> + super(null);
> + // Initialize the map with null values
> + for (String flagName : getFlagNames()) \{
> + mFlagMap.put(flagName, null);
> + }
> -{{ for item in flag_elements}}
> -{{ if not library_exported }} @UnsupportedAppUsage{{ -endif }}
> - public boolean {item.method_name}() \{
> - return getValue(Flags.FLAG_{item.flag_name_constant_suffix});
> + protected boolean getValue(String flagName, Predicate<FeatureFlags> getter) \{
> + Boolean value = this.mFlagMap.get(flagName);
> + if (value == null) \{
> + throw new IllegalArgumentException(flagName + " is not set");
> + }
> + return value;
> -{{ endfor}}
> +
> -{{ if not library_exported }}
> - public boolean isFlagReadOnlyOptimized(String flagName) \{
> - if (mReadOnlyFlagsSet.contains(flagName) &&
> - isOptimizationEnabled()) \{
> - return true;
> - }
> - return false;
> - }
> -
> - @com.android.aconfig.annotations.AssumeTrueForR8
> - private boolean isOptimizationEnabled() \{
> - return false;
> - }
> -{{ -endif }}
> - private boolean getValue(String flagName) \{
> - Boolean value = this.mFlagMap.get(flagName);
> - if (value == null) \{
> - throw new IllegalArgumentException(flagName + " is not set");
> - }
> - return value;
> - }
> -
> -
> - private Map<String, Boolean> mFlagMap = new HashMap<>(
> - Map.ofEntries(
> - {{ -for item in flag_elements }}
> - Map.entry(Flags.FLAG_{item.flag_name_constant_suffix}, false)
> - {{ -if not @last }},{{ endif }}
> - {{ -endfor }}
> - )
> - );
> -
> - private Set<String> mReadOnlyFlagsSet = new HashSet<>(
> - Arrays.asList(
> - {{ -for item in flag_elements }}
> - {{ -if not item.is_read_write }}
> - Flags.FLAG_{item.flag_name_constant_suffix},
> - {{ -endif }}
> - {{ -endfor }}
> - ""{# The empty string here is to resolve the ending comma #}
> - )
> - );
Original patch:
diff --git a/tools/aconfig/aconfig/src/codegen/java.rs b/tools/aconfig/aconfig/src/codegen/java.rs
old mode 100644
new mode 100644
--- a/tools/aconfig/aconfig/src/codegen/java.rs
+++ b/tools/aconfig/aconfig/src/codegen/java.rs
@@ -63,21 +63,28 @@
"FeatureFlags.java",
include_str!("../../templates/FeatureFlags.java.template"),
)?;
+ template.add_template(
+ "CustomFeatureFlags.java",
+ include_str!("../../templates/CustomFeatureFlags.java.template"),
+ )?;
template.add_template(
"FakeFeatureFlagsImpl.java",
include_str!("../../templates/FakeFeatureFlagsImpl.java.template"),
)?;
let path: PathBuf = package.split('.').collect();
- ["Flags.java", "FeatureFlags.java", "FeatureFlagsImpl.java", "FakeFeatureFlagsImpl.java"]
- .iter()
- .map(|file| {
- Ok(OutputFile {
- contents: template.render(file, &context)?.into(),
- path: path.join(file),
- })
- })
- .co
[[[Original patch trimmed due to size. Decoded string size: 26318. Decoded string SHA1: 7db34b7baf0a0bbaa1cff48b6ccab9c65408e743.]]]
Result patch:
diff --git a/tools/aconfig/aconfig/src/codegen/java.rs b/tools/aconfig/aconfig/src/codegen/java.rs
index 18a4be5..9abc892 100644
--- a/tools/aconfig/aconfig/src/codegen/java.rs
+++ b/tools/aconfig/aconfig/src/codegen/java.rs
@@ -64,20 +64,27 @@
include_str!("../../templates/FeatureFlags.java.template"),
)?;
template.add_template(
+ "CustomFeatureFlags.java",
+ include_str!("../../templates/CustomFeatureFlags.java.template"),
+ )?;
+ template.add_template(
"FakeFeatureFlagsImpl.java",
include_str!("../../templates/FakeFeatureFlagsImpl.java.template"),
)?;
let path: PathBuf = package.split('.').collect();
- ["Flags.java", "FeatureFlags.java", "FeatureFlagsImpl.java", "FakeFeatureFlagsImpl.java"]
- .iter()
- .map(|file| {
- Ok(OutputFile {
- contents: template.render(file, &context)?.into(),
- path: path.join(file),
- })
- })
- .collect::<Result<Vec<OutputFile>>>
[[[Result patch trimmed due to size. Decoded string size: 26308. Decoded string SHA1: bb07ee948a630a6bc4cfbbf2a8df178f3e6d3103.]]]
Change-Id: I94184633303b9d76f7943451fb3b2c93d071ec1c
2024-04-24 14:43:27 +00:00
