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
diff --git a/tools/aconfig/protos/aconfig.proto b/tools/aconfig/protos/aconfig.proto
index 489939c..9f6424f 100644
--- a/tools/aconfig/protos/aconfig.proto
+++ b/tools/aconfig/protos/aconfig.proto
@@ -36,7 +36,8 @@
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 parsed_flag {
required string package = 1;
required string name = 2;
- required string description = 3;
- required flag_state state = 4;
- required flag_permission permission = 5;
- repeated tracepoint trace = 6;
+ required string namespace = 3;
+ required string description = 4;
+ required flag_state state = 5;
+ required flag_permission permission = 6;
+ repeated tracepoint trace = 7;
}
message parsed_flags {
diff --git a/tools/aconfig/src/aconfig.rs b/tools/aconfig/src/aconfig.rs
index 696e154..5e7c861 100644
--- a/tools/aconfig/src/aconfig.rs
+++ b/tools/aconfig/src/aconfig.rs
@@ -81,6 +81,7 @@
#[derive(Debug, PartialEq, Eq)]
pub struct FlagDeclaration {
pub name: String,
+ pub namespace: String,
pub description: String,
}
@@ -100,10 +101,13 @@
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 @@
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 @@
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 @@
let expected = FlagDeclarations {
package: "com.example".to_owned(),
flags: vec![
- FlagDeclaration { name: "a".to_owned(), description: "A".to_owned() },
- FlagDeclaration { name: "b".to_owned(), description: "B".to_owned() },
+ FlagDeclaration {
+ 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 @@
package: "com.example"
flag {
name: "a"
+ namespace: "ns"
description: "A"
}
flag {
name: "b"
+ namespace: "ns"
description: "B"
}
"#;
diff --git a/tools/aconfig/src/cache.rs b/tools/aconfig/src/cache.rs
index cdb0992..dd54480 100644
--- a/tools/aconfig/src/cache.rs
+++ b/tools/aconfig/src/cache.rs
@@ -40,6 +40,7 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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();
diff --git a/tools/aconfig/src/codegen.rs b/tools/aconfig/src/codegen.rs
index 8ef0e0b..fea9961 100644
--- a/tools/aconfig/src/codegen.rs
+++ b/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 @@
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 @@
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()
+ );
+ }
}
diff --git a/tools/aconfig/src/codegen_cpp.rs b/tools/aconfig/src/codegen_cpp.rs
index 65a1de1..65f95de 100644
--- a/tools/aconfig/src/codegen_cpp.rs
+++ b/tools/aconfig/src/codegen_cpp.rs
@@ -24,14 +24,20 @@
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 =
- Context { header: header.clone(), cpp_namespace, package, readwrite, class_elements };
+ let context = Context {
+ 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 @@
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 @@
"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 @@
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 @@
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 @@
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 @@
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 @@
public:
virtual const bool value() {
return GetServerConfigurableFlag(
- "com.example",
- "my_flag_one",
+ "ns",
+ "com.example.my_flag_one",
"false") == "true";
}
}
@@ -200,8 +215,8 @@
public:
virtual const bool value() {
return GetServerConfigurableFlag(
- "com.example",
- "my_flag_two",
+ "ns",
+ "com.example.my_flag_two",
"true") == "true";
}
}
diff --git a/tools/aconfig/src/codegen_java.rs b/tools/aconfig/src/codegen_java.rs
index 3a96de4..1c1cf77 100644
--- a/tools/aconfig/src/codegen_java.rs
+++ b/tools/aconfig/src/codegen_java.rs
@@ -21,12 +21,14 @@
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 @@
pub method_name: String,
pub readwrite: bool,
pub default_value: String,
- pub feature_name: 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 {
+ 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 @@
} else {
"false".to_string()
},
- feature_name: item.name.clone(),
- flag_name: item.name.clone(),
+ device_config_namespace: item.namespace.clone(),
+ device_config_flag,
}
}
@@ -83,6 +87,7 @@
Source::File("test.txt".to_string()),
FlagDeclaration {
name: "test".to_string(),
+ namespace: "ns".to_string(),
description: "buildtime enable".to_string(),
},
)
@@ -91,6 +96,7 @@
Source::File("test2.txt".to_string()),
FlagDeclaration {
name: "test2".to_string(),
+ namespace: "ns".to_string(),
description: "runtime disable".to_string(),
},
)
@@ -118,8 +124,8 @@
public static boolean test2() {
return DeviceConfig.getBoolean(
- "com.example",
- "test2__test2",
+ "ns",
+ "com.example.test2",
false
);
}
diff --git a/tools/aconfig/src/codegen_rust.rs b/tools/aconfig/src/codegen_rust.rs
index e27aa9c..cfe7316 100644
--- a/tools/aconfig/src/codegen_rust.rs
+++ b/tools/aconfig/src/codegen_rust.rs
@@ -20,11 +20,13 @@
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 @@
#[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 @@
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 @@
#[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 @@
#[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"
}
}
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs
index d252bc4..8be11cc 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -191,10 +191,12 @@
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 @@
package: "com.other"
flag {
name: "c"
+ namespace: "ns"
description: "Description of c"
}
"#;
diff --git a/tools/aconfig/templates/cpp.template b/tools/aconfig/templates/cpp.template
index 331baaf..93f9b80 100644
--- a/tools/aconfig/templates/cpp.template
+++ b/tools/aconfig/templates/cpp.template
@@ -11,8 +11,8 @@
virtual const bool value() \{
{{ if item.readwrite- }}
return GetServerConfigurableFlag(
- "{package}",
- "{item.flag_name}",
+ "{item.device_config_namespace}",
+ "{item.device_config_flag}",
"{item.default_value}") == "true";
{{ -else- }}
return {item.default_value};
diff --git a/tools/aconfig/templates/java.template b/tools/aconfig/templates/java.template
index 709af44..a3d3319 100644
--- a/tools/aconfig/templates/java.template
+++ b/tools/aconfig/templates/java.template
@@ -7,8 +7,8 @@
public static boolean {item.method_name}() \{
{{ if item.readwrite- }}
return DeviceConfig.getBoolean(
- "{package}",
- "{item.feature_name}__{item.flag_name}",
+ "{item.device_config_namespace}",
+ "{item.device_config_flag}",
{item.default_value}
);
{{ -else- }}
diff --git a/tools/aconfig/templates/rust.template b/tools/aconfig/templates/rust.template
index 81b407c..d914943 100644
--- a/tools/aconfig/templates/rust.template
+++ b/tools/aconfig/templates/rust.template
@@ -19,7 +19,7 @@
{{- 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 -}}
diff --git a/tools/aconfig/testdata/test.aconfig b/tools/aconfig/testdata/test.aconfig
index 015f295..d09396a 100644
--- a/tools/aconfig/testdata/test.aconfig
+++ b/tools/aconfig/testdata/test.aconfig
@@ -5,6 +5,7 @@
# - first.values: DISABLED + READ_ONLY
flag {
name: "disabled_ro"
+ namespace: "aconfig_test"
description: "This flag is DISABLED + READ_ONLY"
}
@@ -12,6 +13,7 @@
# - test.aconfig: DISABLED + READ_WRITE (default)
flag {
name: "disabled_rw"
+ namespace: "aconfig_test"
description: "This flag is DISABLED + READ_WRITE"
}
@@ -21,6 +23,7 @@
# - second.values: ENABLED + READ_ONLY
flag {
name: "enabled_ro"
+ namespace: "aconfig_test"
description: "This flag is ENABLED + READ_ONLY"
}
@@ -29,5 +32,6 @@
# - first.values: ENABLED + READ_WRITE
flag {
name: "enabled_rw"
+ namespace: "aconfig_test"
description: "This flag is ENABLED + READ_WRITE"
}