Merge "aconfig: cache flag value in generated java code" into main

tools/aconfig/fake_device_config/src/android/provider/DeviceConfig.java

@@ -26,4 +26,14 @@ public class DeviceConfig {
 	public static boolean getBoolean(String ns, String name, boolean def) {
 		return false;
 	}
+
+	public static Properties getProperties(String namespace, String... names) {
+		return new Properties();
+	}
+
+	public static class Properties {
+		public boolean getBoolean(String name, boolean def) {
+			return false;
+		}
+	}
 }

tools/aconfig/src/codegen_java.rs

@@ -16,6 +16,7 @@
 
 use anyhow::Result;
 use serde::Serialize;
+use std::collections::BTreeSet;
 use std::path::PathBuf;
 use tinytemplate::TinyTemplate;
 
@@ -31,12 +32,19 @@ pub fn generate_java_code<'a, I>(
 where
     I: Iterator<Item = &'a ProtoParsedFlag>,
 {
-    let class_elements: Vec<ClassElement> =
+    let flag_elements: Vec<FlagElement> =
-        parsed_flags_iter.map(|pf| create_class_element(package, pf)).collect();
+        parsed_flags_iter.map(|pf| create_flag_element(package, pf)).collect();
-    let is_read_write = class_elements.iter().any(|elem| elem.is_read_write);
+    let properties_set: BTreeSet<String> =
+        flag_elements.iter().map(|fe| format_property_name(&fe.device_config_namespace)).collect();
+    let is_read_write = flag_elements.iter().any(|elem| elem.is_read_write);
     let is_test_mode = codegen_mode == CodegenMode::Test;
-    let context =
+    let context = Context {
-        Context { class_elements, is_test_mode, is_read_write, package_name: package.to_string() };
+        flag_elements,
+        is_test_mode,
+        is_read_write,
+        properties_set,
+        package_name: package.to_string(),
+    };
     let mut template = TinyTemplate::new();
     template.add_template("Flags.java", include_str!("../templates/Flags.java.template"))?;
     template.add_template(
@@ -66,39 +74,46 @@ where
 
 #[derive(Serialize)]
 struct Context {
-    pub class_elements: Vec<ClassElement>,
+    pub flag_elements: Vec<FlagElement>,
     pub is_test_mode: bool,
     pub is_read_write: bool,
+    pub properties_set: BTreeSet<String>,
     pub package_name: String,
 }
 
 #[derive(Serialize)]
-struct ClassElement {
+struct FlagElement {
     pub default_value: bool,
     pub device_config_namespace: String,
     pub device_config_flag: String,
     pub flag_name_constant_suffix: String,
     pub is_read_write: bool,
     pub method_name: String,
+    pub properties: String,
 }
 
-fn create_class_element(package: &str, pf: &ProtoParsedFlag) -> ClassElement {
+fn create_flag_element(package: &str, pf: &ProtoParsedFlag) -> FlagElement {
     let device_config_flag = codegen::create_device_config_ident(package, pf.name())
         .expect("values checked at flag parse time");
-    ClassElement {
+    FlagElement {
         default_value: pf.state() == ProtoFlagState::ENABLED,
         device_config_namespace: pf.namespace().to_string(),
         device_config_flag,
         flag_name_constant_suffix: pf.name().to_ascii_uppercase(),
         is_read_write: pf.permission() == ProtoFlagPermission::READ_WRITE,
         method_name: format_java_method_name(pf.name()),
+        properties: format_property_name(pf.namespace()),
     }
 }
 
 fn format_java_method_name(flag_name: &str) -> String {
-    flag_name
+    let splits: Vec<&str> = flag_name.split('_').filter(|&word| !word.is_empty()).collect();
-        .split('_')
+    if splits.len() == 1 {
-        .filter(|&word| !word.is_empty())
+        let name = splits[0];
+        name[0..1].to_ascii_lowercase() + &name[1..]
+    } else {
+        splits
+            .iter()
             .enumerate()
             .map(|(index, word)| {
                 if index == 0 {
@@ -109,6 +124,12 @@ fn format_java_method_name(flag_name: &str) -> String {
             })
             .collect::<Vec<String>>()
             .join("")
+    }
+}
+
+fn format_property_name(property_name: &str) -> String {
+    let name = format_java_method_name(property_name);
+    format!("mProperties{}{}", &name[0..1].to_ascii_uppercase(), &name[1..])
 }
 
 #[cfg(test)]
@@ -265,8 +286,10 @@ mod tests {
         // TODO(b/303773055): Remove the annotation after access issue is resolved.
         import android.compat.annotation.UnsupportedAppUsage;
         import android.provider.DeviceConfig;
+        import android.provider.DeviceConfig.Properties;
         /** @hide */
         public final class FeatureFlagsImpl implements FeatureFlags {
+            private Properties mPropertiesAconfigTest;
             @Override
             @UnsupportedAppUsage
             public boolean disabledRo() {
@@ -275,8 +298,15 @@ mod tests {
             @Override
             @UnsupportedAppUsage
             public boolean disabledRw() {
-                return getValue(
+                if (mPropertiesAconfigTest == null) {
+                    mPropertiesAconfigTest =
+                        getProperties(
                             "aconfig_test",
+                            "com.android.aconfig.test.disabled_rw"
+                        );
+                }
+                return mPropertiesAconfigTest
+                    .getBoolean(
                         "com.android.aconfig.test.disabled_rw",
                         false
                     );
@@ -294,32 +324,36 @@ mod tests {
             @Override
             @UnsupportedAppUsage
             public boolean enabledRw() {
-                return getValue(
+                if (mPropertiesAconfigTest == null) {
+                    mPropertiesAconfigTest =
+                        getProperties(
                             "aconfig_test",
+                            "com.android.aconfig.test.enabled_rw"
+                        );
+                }
+                return mPropertiesAconfigTest
+                    .getBoolean(
                         "com.android.aconfig.test.enabled_rw",
                         true
                     );
             }
-            private boolean getValue(String nameSpace,
+            private Properties getProperties(
-                String flagName, boolean defaultValue) {
+                String namespace,
-                boolean value = defaultValue;
+                String flagName) {
+                Properties properties = null;
                 try {
-                    value = DeviceConfig.getBoolean(
+                    properties = DeviceConfig.getProperties(namespace);
-                        nameSpace,
-                        flagName,
-                        defaultValue
-                    );
                 } catch (NullPointerException e) {
                     throw new RuntimeException(
-                        "Cannot read value of flag " + flagName + " from DeviceConfig. " +
+                        "Cannot read value of flag " + flagName + " from DeviceConfig. "
-                        "It could be that the code using flag executed " +
+                        + "It could be that the code using flag executed "
-                        "before SettingsProvider initialization. " +
+                        + "before SettingsProvider initialization. "
-                        "Please use fixed read-only flag by adding " +
+                        + "Please use fixed read-only flag by adding "
-                        "is_fixed_read_only: true in flag declaration.",
+                        + "is_fixed_read_only: true in flag declaration.",
                         e
                     );
                 }
-                return value;
+                return properties;
             }
         }
         "#;
@@ -441,9 +475,45 @@ mod tests {
 
     #[test]
     fn test_format_java_method_name() {
-        let input = "____some_snake___name____";
         let expected = "someSnakeName";
+        let input = "____some_snake___name____";
+        let formatted_name = format_java_method_name(input);
+        assert_eq!(expected, formatted_name);
+
+        let input = "someSnakeName";
+        let formatted_name = format_java_method_name(input);
+        assert_eq!(expected, formatted_name);
+
+        let input = "SomeSnakeName";
+        let formatted_name = format_java_method_name(input);
+        assert_eq!(expected, formatted_name);
+
+        let input = "SomeSnakeName_";
+        let formatted_name = format_java_method_name(input);
+        assert_eq!(expected, formatted_name);
+
+        let input = "_SomeSnakeName";
         let formatted_name = format_java_method_name(input);
         assert_eq!(expected, formatted_name);
     }
+
+    #[test]
+    fn test_format_property_name() {
+        let expected = "mPropertiesSomeSnakeName";
+        let input = "____some_snake___name____";
+        let formatted_name = format_property_name(input);
+        assert_eq!(expected, formatted_name);
+
+        let input = "someSnakeName";
+        let formatted_name = format_property_name(input);
+        assert_eq!(expected, formatted_name);
+
+        let input = "SomeSnakeName";
+        let formatted_name = format_property_name(input);
+        assert_eq!(expected, formatted_name);
+
+        let input = "SomeSnakeName_";
+        let formatted_name = format_property_name(input);
+        assert_eq!(expected, formatted_name);
+    }
 }

tools/aconfig/templates/FakeFeatureFlagsImpl.java.template

@@ -11,7 +11,7 @@ public class FakeFeatureFlagsImpl implements FeatureFlags \{
         resetAll();
     }
 
-{{ for item in class_elements}}
+{{ for item in flag_elements}}
     @Override
     @UnsupportedAppUsage
     public boolean {item.method_name}() \{
@@ -41,7 +41,7 @@ public class FakeFeatureFlagsImpl implements FeatureFlags \{
 
     private Map<String, Boolean> mFlagMap = new HashMap<>(
         Map.ofEntries(
-            {{-for item in class_elements}}
+            {{-for item in flag_elements}}
             Map.entry(Flags.FLAG_{item.flag_name_constant_suffix}, false)
             {{ -if not @last }},{{ endif }}
             {{ -endfor }}

tools/aconfig/templates/FeatureFlags.java.template

@@ -4,7 +4,7 @@ import android.compat.annotation.UnsupportedAppUsage;
 
 /** @hide */
 public interface FeatureFlags \{
-{{ for item in class_elements}}
+{{ for item in flag_elements }}
 {{ -if not item.is_read_write }}
 {{ -if item.default_value }}
     @com.android.aconfig.annotations.AssumeTrueForR8

tools/aconfig/templates/FeatureFlagsImpl.java.template

@@ -4,45 +4,58 @@ import android.compat.annotation.UnsupportedAppUsage;
 {{ if not is_test_mode }}
 {{ if is_read_write- }}
 import android.provider.DeviceConfig;
+import android.provider.DeviceConfig.Properties;
 {{ endif }}
 /** @hide */
 public final class FeatureFlagsImpl implements FeatureFlags \{
-{{ for item in class_elements}}
+{{ if is_read_write- }}
+{{ for properties in properties_set }}
+    private Properties {properties};
+{{ endfor }}
+{{ endif- }}
+
+{{ for flag in flag_elements }}
     @Override
     @UnsupportedAppUsage
-    public boolean {item.method_name}() \{
+    public boolean {flag.method_name}() \{
-    {{ -if item.is_read_write }}
+    {{ -if flag.is_read_write }}
-        return getValue(
+        if ({flag.properties} == null) \{
-            "{item.device_config_namespace}",
+            {flag.properties} =
-            "{item.device_config_flag}",
+                getProperties(
-            {item.default_value}
+                    "{flag.device_config_namespace}",
+                    "{flag.device_config_flag}"
+                );
+        }
+        return {flag.properties}
+                .getBoolean(
+                    "{flag.device_config_flag}",
+                    {flag.default_value}
                 );
     {{ else }}
-        return {item.default_value};
+        return {flag.default_value};
     {{ endif- }}
     }
 {{ endfor }}
-{{ if is_read_write- }}
+
-    private boolean getValue(String nameSpace,
+{{ -if is_read_write }}
-        String flagName, boolean defaultValue) \{
+    private Properties getProperties(
-        boolean value = defaultValue;
+            String namespace,
+            String flagName) \{
+        Properties properties = null;
         try \{
-            value = DeviceConfig.getBoolean(
+            properties = DeviceConfig.getProperties(namespace);
-                nameSpace,
-                flagName,
-                defaultValue
-            );
         } catch (NullPointerException e) \{
             throw new RuntimeException(
-                "Cannot read value of flag " + flagName + " from DeviceConfig. " +
+                "Cannot read value of flag " + flagName + " from DeviceConfig. "
-                "It could be that the code using flag executed " +
+                + "It could be that the code using flag executed "
-                "before SettingsProvider initialization. " +
+                + "before SettingsProvider initialization. "
-                "Please use fixed read-only flag by adding " +
+                + "Please use fixed read-only flag by adding "
-                "is_fixed_read_only: true in flag declaration.",
+                + "is_fixed_read_only: true in flag declaration.",
                 e
             );
         }
-        return value;
+
+        return properties;
     }
 {{ endif- }}
 }
@@ -50,10 +63,10 @@ public final class FeatureFlagsImpl implements FeatureFlags \{
 {#- Generate only stub if in test mode #}
 /** @hide */
 public final class FeatureFlagsImpl implements FeatureFlags \{
-{{ for item in class_elements}}
+{{ for flag in flag_elements }}
     @Override
     @UnsupportedAppUsage
-    public boolean {item.method_name}() \{
+    public boolean {flag.method_name}() \{
         throw new UnsupportedOperationException(
             "Method is not implemented.");
     }

tools/aconfig/templates/Flags.java.template

@@ -5,11 +5,11 @@ import android.compat.annotation.UnsupportedAppUsage;
 
 /** @hide */
 public final class Flags \{
-{{- for item in class_elements}}
+{{- for item in flag_elements}}
     /** @hide */
     public static final String FLAG_{item.flag_name_constant_suffix} = "{item.device_config_flag}";
 {{- endfor }}
-{{ for item in class_elements}}
+{{ for item in flag_elements}}
 {{ -if not item.is_read_write }}
 {{ -if item.default_value }}
     @com.android.aconfig.annotations.AssumeTrueForR8