aconfig: add namespace field to flag_declaration and parsed_flag

Add a new field to the proto messages flag_declaration and parsed_flag.

The new field will be used verbatim as a parameter when calling
DeviceConfig.getBoolean to read the value of a READ_WRITE flag. See the
DeviceConfig API for more info.

Note: not to be confused with the old namespace field, which has been
renamed to package.

Bug: 285211724
Test: atest aconfig.test
Change-Id: I2181be7b5e98fc334e5277fb5f7e386f1fe0b550

tools/aconfig/protos/aconfig.proto

@@ -36,7 +36,8 @@ enum flag_permission {
 
 message flag_declaration {
   required string name = 1;
-  required string description = 2;
+  required string namespace = 2;
+  required string description = 3;
 };
 
 message flag_declarations {
@@ -67,10 +68,11 @@ message tracepoint {
 message parsed_flag {
   required string package = 1;
   required string name = 2;
-  required string description = 3;
+  required string namespace = 3;
-  required flag_state state = 4;
+  required string description = 4;
-  required flag_permission permission = 5;
+  required flag_state state = 5;
-  repeated tracepoint trace = 6;
+  required flag_permission permission = 6;
+  repeated tracepoint trace = 7;
 }
 
 message parsed_flags {

tools/aconfig/src/aconfig.rs

@@ -81,6 +81,7 @@ impl From<Permission> for ProtoFlagPermission {
 #[derive(Debug, PartialEq, Eq)]
 pub struct FlagDeclaration {
     pub name: String,
+    pub namespace: String,
     pub description: String,
 }
 
@@ -100,10 +101,13 @@ impl TryFrom<ProtoFlagDeclaration> for FlagDeclaration {
         let Some(name) = proto.name else {
             bail!("missing 'name' field");
         };
+        let Some(namespace) = proto.namespace else {
+            bail!("missing 'namespace' field");
+        };
         let Some(description) = proto.description else {
             bail!("missing 'description' field");
         };
-        Ok(FlagDeclaration { name, description })
+        Ok(FlagDeclaration { name, namespace, description })
     }
 }
 
@@ -186,6 +190,7 @@ impl From<Item> for ProtoParsedFlag {
         let mut proto = crate::protos::ProtoParsedFlag::new();
         proto.set_package(item.package.to_owned());
         proto.set_name(item.name.clone());
+        proto.set_namespace(item.namespace.clone());
         proto.set_description(item.description.clone());
         proto.set_state(item.state.into());
         proto.set_permission(item.permission.into());
@@ -214,11 +219,13 @@ mod tests {
     fn test_flag_try_from_text_proto() {
         let expected = FlagDeclaration {
             name: "1234".to_owned(),
+            namespace: "ns".to_owned(),
             description: "Description of the flag".to_owned(),
         };
 
         let s = r#"
         name: "1234"
+        namespace: "ns"
         description: "Description of the flag"
         "#;
         let actual = FlagDeclaration::try_from_text_proto(s).unwrap();
@@ -246,8 +253,16 @@ mod tests {
         let expected = FlagDeclarations {
             package: "com.example".to_owned(),
             flags: vec![
-                FlagDeclaration { name: "a".to_owned(), description: "A".to_owned() },
+                FlagDeclaration {
-                FlagDeclaration { name: "b".to_owned(), description: "B".to_owned() },
+                    name: "a".to_owned(),
+                    namespace: "ns".to_owned(),
+                    description: "A".to_owned(),
+                },
+                FlagDeclaration {
+                    name: "b".to_owned(),
+                    namespace: "ns".to_owned(),
+                    description: "B".to_owned(),
+                },
             ],
         };
 
@@ -255,10 +270,12 @@ mod tests {
         package: "com.example"
         flag {
             name: "a"
+            namespace: "ns"
             description: "A"
         }
         flag {
             name: "b"
+            namespace: "ns"
             description: "B"
         }
         "#;

tools/aconfig/src/cache.rs

@@ -40,6 +40,7 @@ pub struct Item {
     // really be a Cow<String>.
     pub package: String,
     pub name: String,
+    pub namespace: String,
     pub description: String,
     pub state: FlagState,
     pub permission: Permission,
@@ -120,6 +121,7 @@ impl CacheBuilder {
         declaration: FlagDeclaration,
     ) -> Result<&mut CacheBuilder> {
         ensure!(codegen::is_valid_name_ident(&declaration.name), "bad flag name");
+        ensure!(codegen::is_valid_name_ident(&declaration.namespace), "bad namespace");
         ensure!(!declaration.description.is_empty(), "empty flag description");
         ensure!(
             self.cache.items.iter().all(|item| item.name != declaration.name),
@@ -130,6 +132,7 @@ impl CacheBuilder {
         self.cache.items.push(Item {
             package: self.cache.package.clone(),
             name: declaration.name.clone(),
+            namespace: declaration.namespace.clone(),
             description: declaration.description,
             state: DEFAULT_FLAG_STATE,
             permission: DEFAULT_FLAG_PERMISSION,
@@ -186,13 +189,21 @@ mod tests {
         builder
             .add_flag_declaration(
                 Source::File("first.txt".to_string()),
-                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
+                FlagDeclaration {
+                    name: "foo".to_string(),
+                    namespace: "ns".to_string(),
+                    description: "desc".to_string(),
+                },
             )
             .unwrap();
         let error = builder
             .add_flag_declaration(
                 Source::File("second.txt".to_string()),
-                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
+                FlagDeclaration {
+                    name: "foo".to_string(),
+                    namespace: "ns".to_string(),
+                    description: "desc".to_string(),
+                },
             )
             .unwrap_err();
         assert_eq!(
@@ -202,7 +213,11 @@ mod tests {
         builder
             .add_flag_declaration(
                 Source::File("first.txt".to_string()),
-                FlagDeclaration { name: "bar".to_string(), description: "desc".to_string() },
+                FlagDeclaration {
+                    name: "bar".to_string(),
+                    namespace: "ns".to_string(),
+                    description: "desc".to_string(),
+                },
             )
             .unwrap();
 
@@ -237,7 +252,11 @@ mod tests {
         builder
             .add_flag_declaration(
                 Source::File("first.txt".to_string()),
-                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
+                FlagDeclaration {
+                    name: "foo".to_string(),
+                    namespace: "ns".to_string(),
+                    description: "desc".to_string(),
+                },
             )
             .unwrap();
 
@@ -297,7 +316,11 @@ mod tests {
         let error = builder
             .add_flag_declaration(
                 Source::Memory,
-                FlagDeclaration { name: "".to_string(), description: "Description".to_string() },
+                FlagDeclaration {
+                    name: "".to_string(),
+                    namespace: "ns".to_string(),
+                    description: "Description".to_string(),
+                },
             )
             .unwrap_err();
         assert_eq!(&format!("{:?}", error), "bad flag name");
@@ -305,7 +328,11 @@ mod tests {
         let error = builder
             .add_flag_declaration(
                 Source::Memory,
-                FlagDeclaration { name: "foo".to_string(), description: "".to_string() },
+                FlagDeclaration {
+                    name: "foo".to_string(),
+                    namespace: "ns".to_string(),
+                    description: "".to_string(),
+                },
             )
             .unwrap_err();
         assert_eq!(&format!("{:?}", error), "empty flag description");
@@ -317,7 +344,11 @@ mod tests {
         builder
             .add_flag_declaration(
                 Source::Memory,
-                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
+                FlagDeclaration {
+                    name: "foo".to_string(),
+                    namespace: "ns".to_string(),
+                    description: "desc".to_string(),
+                },
             )
             .unwrap();
 

tools/aconfig/src/codegen.rs

@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+use anyhow::{ensure, Result};
+
 pub fn is_valid_name_ident(s: &str) -> bool {
     // Identifiers must match [a-z][a-z0-9_]*
     let mut chars = s.chars();
@@ -30,6 +32,12 @@ pub fn is_valid_package_ident(s: &str) -> bool {
     s.split('.').all(is_valid_name_ident)
 }
 
+pub fn create_device_config_ident(package: &str, flag_name: &str) -> Result<String> {
+    ensure!(is_valid_package_ident(package), "bad package");
+    ensure!(is_valid_package_ident(flag_name), "bad flag name");
+    Ok(format!("{}.{}", package, flag_name))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -62,4 +70,12 @@ mod tests {
         assert!(!is_valid_package_ident("."));
         assert!(!is_valid_package_ident("foo..bar"));
     }
+
+    #[test]
+    fn test_create_device_config_ident() {
+        assert_eq!(
+            "com.foo.bar.some_flag",
+            create_device_config_ident("com.foo.bar", "some_flag").unwrap()
+        );
+    }
 }

tools/aconfig/src/codegen_cpp.rs

@@ -24,14 +24,20 @@ use crate::codegen;
 use crate::commands::OutputFile;
 
 pub fn generate_cpp_code(cache: &Cache) -> Result<OutputFile> {
-    let class_elements: Vec<ClassElement> = cache.iter().map(create_class_element).collect();
+    let package = cache.package();
+    let class_elements: Vec<ClassElement> =
+        cache.iter().map(|item| create_class_element(package, item)).collect();
     let readwrite = class_elements.iter().any(|item| item.readwrite);
-    let package = cache.package().to_string();
     let header = package.replace('.', "_");
     let cpp_namespace = package.replace('.', "::");
     ensure!(codegen::is_valid_name_ident(&header));
-    let context =
+    let context = Context {
-        Context { header: header.clone(), cpp_namespace, package, readwrite, class_elements };
+        header: header.clone(),
+        cpp_namespace,
+        package: package.to_string(),
+        readwrite,
+        class_elements,
+    };
     let mut template = TinyTemplate::new();
     template.add_template("cpp_code_gen", include_str!("../templates/cpp.template"))?;
     let contents = template.render("cpp_code_gen", &context)?;
@@ -53,9 +59,11 @@ struct ClassElement {
     pub readwrite: bool,
     pub default_value: String,
     pub flag_name: String,
+    pub device_config_namespace: String,
+    pub device_config_flag: String,
 }
 
-fn create_class_element(item: &Item) -> ClassElement {
+fn create_class_element(package: &str, item: &Item) -> ClassElement {
     ClassElement {
         readwrite: item.permission == Permission::ReadWrite,
         default_value: if item.state == FlagState::Enabled {
@@ -64,6 +72,9 @@ fn create_class_element(item: &Item) -> ClassElement {
             "false".to_string()
         },
         flag_name: item.name.clone(),
+        device_config_namespace: item.namespace.to_string(),
+        device_config_flag: codegen::create_device_config_ident(package, &item.name)
+            .expect("values checked at cache creation time"),
     }
 }
 
@@ -83,6 +94,7 @@ mod tests {
                 Source::File("aconfig_one.txt".to_string()),
                 FlagDeclaration {
                     name: "my_flag_one".to_string(),
+                    namespace: "ns".to_string(),
                     description: "buildtime disable".to_string(),
                 },
             )
@@ -101,6 +113,7 @@ mod tests {
                 Source::File("aconfig_two.txt".to_string()),
                 FlagDeclaration {
                     name: "my_flag_two".to_string(),
+                    namespace: "ns".to_string(),
                     description: "buildtime enable".to_string(),
                 },
             )
@@ -155,6 +168,7 @@ mod tests {
                 Source::File("aconfig_one.txt".to_string()),
                 FlagDeclaration {
                     name: "my_flag_one".to_string(),
+                    namespace: "ns".to_string(),
                     description: "buildtime disable".to_string(),
                 },
             )
@@ -163,6 +177,7 @@ mod tests {
                 Source::File("aconfig_two.txt".to_string()),
                 FlagDeclaration {
                     name: "my_flag_two".to_string(),
+                    namespace: "ns".to_string(),
                     description: "runtime enable".to_string(),
                 },
             )
@@ -190,8 +205,8 @@ mod tests {
                 public:
                     virtual const bool value() {
                         return GetServerConfigurableFlag(
-                            "com.example",
+                            "ns",
-                            "my_flag_one",
+                            "com.example.my_flag_one",
                             "false") == "true";
                     }
             }
@@ -200,8 +215,8 @@ mod tests {
                 public:
                     virtual const bool value() {
                         return GetServerConfigurableFlag(
-                            "com.example",
+                            "ns",
-                            "my_flag_two",
+                            "com.example.my_flag_two",
                             "true") == "true";
                     }
             }

tools/aconfig/src/codegen_java.rs

@@ -21,12 +21,14 @@ use tinytemplate::TinyTemplate;
 
 use crate::aconfig::{FlagState, Permission};
 use crate::cache::{Cache, Item};
+use crate::codegen;
 use crate::commands::OutputFile;
 
 pub fn generate_java_code(cache: &Cache) -> Result<OutputFile> {
-    let class_elements: Vec<ClassElement> = cache.iter().map(create_class_element).collect();
-    let readwrite = class_elements.iter().any(|item| item.readwrite);
     let package = cache.package();
+    let class_elements: Vec<ClassElement> =
+        cache.iter().map(|item| create_class_element(package, item)).collect();
+    let readwrite = class_elements.iter().any(|item| item.readwrite);
     let context = Context { package: package.to_string(), readwrite, class_elements };
     let mut template = TinyTemplate::new();
     template.add_template("java_code_gen", include_str!("../templates/java.template"))?;
@@ -49,11 +51,13 @@ struct ClassElement {
     pub method_name: String,
     pub readwrite: bool,
     pub default_value: String,
-    pub feature_name: String,
+    pub device_config_namespace: String,
-    pub flag_name: String,
+    pub device_config_flag: String,
 }
 
-fn create_class_element(item: &Item) -> ClassElement {
+fn create_class_element(package: &str, item: &Item) -> ClassElement {
+    let device_config_flag = codegen::create_device_config_ident(package, &item.name)
+        .expect("values checked at cache creation time");
     ClassElement {
         method_name: item.name.clone(),
         readwrite: item.permission == Permission::ReadWrite,
@@ -62,8 +66,8 @@ fn create_class_element(item: &Item) -> ClassElement {
         } else {
             "false".to_string()
         },
-        feature_name: item.name.clone(),
+        device_config_namespace: item.namespace.clone(),
-        flag_name: item.name.clone(),
+        device_config_flag,
     }
 }
 
@@ -83,6 +87,7 @@ mod tests {
                 Source::File("test.txt".to_string()),
                 FlagDeclaration {
                     name: "test".to_string(),
+                    namespace: "ns".to_string(),
                     description: "buildtime enable".to_string(),
                 },
             )
@@ -91,6 +96,7 @@ mod tests {
                 Source::File("test2.txt".to_string()),
                 FlagDeclaration {
                     name: "test2".to_string(),
+                    namespace: "ns".to_string(),
                     description: "runtime disable".to_string(),
                 },
             )
@@ -118,8 +124,8 @@ mod tests {
 
             public static boolean test2() {
                 return DeviceConfig.getBoolean(
-                    "com.example",
+                    "ns",
-                    "test2__test2",
+                    "com.example.test2",
                     false
                 );
             }

tools/aconfig/src/codegen_rust.rs

@@ -20,11 +20,13 @@ use tinytemplate::TinyTemplate;
 
 use crate::aconfig::{FlagState, Permission};
 use crate::cache::{Cache, Item};
+use crate::codegen;
 use crate::commands::OutputFile;
 
 pub fn generate_rust_code(cache: &Cache) -> Result<OutputFile> {
     let package = cache.package();
-    let parsed_flags: Vec<TemplateParsedFlag> = cache.iter().map(|item| item.into()).collect();
+    let parsed_flags: Vec<TemplateParsedFlag> =
+        cache.iter().map(|item| TemplateParsedFlag::new(package, item)).collect();
     let context = TemplateContext {
         package: package.to_string(),
         parsed_flags,
@@ -47,6 +49,8 @@ struct TemplateContext {
 #[derive(Serialize)]
 struct TemplateParsedFlag {
     pub name: String,
+    pub device_config_namespace: String,
+    pub device_config_flag: String,
 
     // TinyTemplate's conditionals are limited to single <bool> expressions; list all options here
     // Invariant: exactly one of these fields will be true
@@ -55,11 +59,14 @@ struct TemplateParsedFlag {
     pub is_read_write: bool,
 }
 
-impl From<&Item> for TemplateParsedFlag {
+impl TemplateParsedFlag {
     #[allow(clippy::nonminimal_bool)]
-    fn from(item: &Item) -> Self {
+    fn new(package: &str, item: &Item) -> Self {
         let template = TemplateParsedFlag {
             name: item.name.clone(),
+            device_config_namespace: item.namespace.to_string(),
+            device_config_flag: codegen::create_device_config_ident(package, &item.name)
+                .expect("values checked at cache creation time"),
             is_read_only_enabled: item.permission == Permission::ReadOnly
                 && item.state == FlagState::Enabled,
             is_read_only_disabled: item.permission == Permission::ReadOnly
@@ -101,7 +108,7 @@ pub const fn r#disabled_ro() -> bool {
 
 #[inline(always)]
 pub fn r#disabled_rw() -> bool {
-    flags_rust::GetServerConfigurableFlag("com.android.aconfig.test", "disabled_rw", "false") == "true"
+    flags_rust::GetServerConfigurableFlag("aconfig_test", "com.android.aconfig.test.disabled_rw", "false") == "true"
 }
 
 #[inline(always)]
@@ -111,7 +118,7 @@ pub const fn r#enabled_ro() -> bool {
 
 #[inline(always)]
 pub fn r#enabled_rw() -> bool {
-    flags_rust::GetServerConfigurableFlag("com.android.aconfig.test", "enabled_rw", "false") == "true"
+    flags_rust::GetServerConfigurableFlag("aconfig_test", "com.android.aconfig.test.enabled_rw", "false") == "true"
 }
 
 }

tools/aconfig/src/commands.rs

@@ -191,10 +191,12 @@ mod tests {
         package: "com.example"
         flag {
             name: "a"
+            namespace: "ns"
             description: "Description of a"
         }
         flag {
             name: "b"
+            namespace: "ns"
             description: "Description of b"
         }
         "#;
@@ -216,6 +218,7 @@ mod tests {
         package: "com.other"
         flag {
             name: "c"
+            namespace: "ns"
             description: "Description of c"
         }
         "#;

tools/aconfig/templates/cpp.template

@@ -11,8 +11,8 @@ namespace {cpp_namespace} \{
             virtual const bool value() \{
                 {{ if item.readwrite- }}
                 return GetServerConfigurableFlag(
-                    "{package}",
+                    "{item.device_config_namespace}",
-                    "{item.flag_name}",
+                    "{item.device_config_flag}",
                     "{item.default_value}") == "true";
                 {{ -else- }}
                 return {item.default_value};

tools/aconfig/templates/java.template

@@ -7,8 +7,8 @@ public final class Flags \{
     public static boolean {item.method_name}() \{
         {{ if item.readwrite- }}
         return DeviceConfig.getBoolean(
-            "{package}",
+            "{item.device_config_namespace}",
-            "{item.feature_name}__{item.flag_name}",
+            "{item.device_config_flag}",
             {item.default_value}
         );
         {{ -else- }}

tools/aconfig/templates/rust.template

@@ -19,7 +19,7 @@ pub const fn r#{parsed_flag.name}() -> bool \{
 {{- if parsed_flag.is_read_write -}}
 #[inline(always)]
 pub fn r#{parsed_flag.name}() -> bool \{
-    flags_rust::GetServerConfigurableFlag("{package}", "{parsed_flag.name}", "false") == "true"
+    flags_rust::GetServerConfigurableFlag("{parsed_flag.device_config_namespace}", "{parsed_flag.device_config_flag}", "false") == "true"
 }
 
 {{ endif -}}

tools/aconfig/testdata/test.aconfig

@@ -5,6 +5,7 @@ package: "com.android.aconfig.test"
 # - first.values: DISABLED + READ_ONLY
 flag {
     name: "disabled_ro"
+    namespace: "aconfig_test"
     description: "This flag is DISABLED + READ_ONLY"
 }
 
@@ -12,6 +13,7 @@ flag {
 # - test.aconfig: DISABLED + READ_WRITE (default)
 flag {
     name: "disabled_rw"
+    namespace: "aconfig_test"
     description: "This flag is DISABLED + READ_WRITE"
 }
 
@@ -21,6 +23,7 @@ flag {
 # - second.values: ENABLED + READ_ONLY
 flag {
     name: "enabled_ro"
+    namespace: "aconfig_test"
     description: "This flag is ENABLED + READ_ONLY"
 }
 
@@ -29,5 +32,6 @@ flag {
 # - first.values: ENABLED + READ_WRITE
 flag {
     name: "enabled_rw"
+    namespace: "aconfig_test"
     description: "This flag is ENABLED + READ_WRITE"
 }