aconfig: use proto struct directly
Remove the hand-crafted wrappers around the structures auto-generated
from protos/aconfig.proto, and use the auto-generated structs directly
intead. This gets rid of a lot of manual repetition, and its inherent
risk.
Also unify how individual fields read from text proto are verified (e.g.
is the flag.name field a valid identifier).
Also change the intermediate cache format from JSON to binary protobuf.
The concept of a 'cache' as an intermediate internal format to represent
parsed input stays. The command line interface still refers to caches.
At the moment a cache file is identical to a parsed_file protbuf, and
the code exploits this internally.
A couple of points regarding the auto-generated structs:
- Vectors are named in the singular (e.g. parsed_flags.parsed_flag is
a Vec<ProtoParsedFlag>) because this improves ergonomics for all
devs working with aconfig input files
- The auto-generated structs have fields that are of type Option<T>
and convenience methods (named the same as the fields) to access T
Test: atest aconfig.test aconfig.test.java
Bug: 283910447
Change-Id: I512820cc4bc6c543dea9f6a4356f863120a10be3
diff --git a/tools/aconfig/src/codegen_cpp.rs b/tools/aconfig/src/codegen_cpp.rs
index 37b058d..2944e8a 100644
--- a/tools/aconfig/src/codegen_cpp.rs
+++ b/tools/aconfig/src/codegen_cpp.rs
@@ -18,15 +18,16 @@
use serde::Serialize;
use tinytemplate::TinyTemplate;
-use crate::aconfig::{FlagState, Permission};
-use crate::cache::{Cache, Item};
use crate::codegen;
use crate::commands::OutputFile;
+use crate::protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag};
-pub fn generate_cpp_code(cache: &Cache) -> Result<OutputFile> {
- let package = cache.package();
+pub fn generate_cpp_code<'a, I>(package: &str, parsed_flags_iter: I) -> Result<OutputFile>
+where
+ I: Iterator<Item = &'a ProtoParsedFlag>,
+{
let class_elements: Vec<ClassElement> =
- cache.iter().map(|item| create_class_element(package, item)).collect();
+ parsed_flags_iter.map(|pf| create_class_element(package, pf)).collect();
let readwrite = class_elements.iter().any(|item| item.readwrite);
let header = package.replace('.', "_");
let cpp_namespace = package.replace('.', "::");
@@ -63,162 +64,68 @@
pub device_config_flag: String,
}
-fn create_class_element(package: &str, item: &Item) -> ClassElement {
+fn create_class_element(package: &str, pf: &ProtoParsedFlag) -> ClassElement {
ClassElement {
- readwrite: item.permission == Permission::ReadWrite,
- default_value: if item.state == FlagState::Enabled {
+ readwrite: pf.permission() == ProtoFlagPermission::READ_WRITE,
+ default_value: if pf.state() == ProtoFlagState::ENABLED {
"true".to_string()
} else {
"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"),
+ flag_name: pf.name().to_string(),
+ device_config_namespace: pf.namespace().to_string(),
+ device_config_flag: codegen::create_device_config_ident(package, pf.name())
+ .expect("values checked at flag parse time"),
}
}
#[cfg(test)]
mod tests {
use super::*;
- use crate::aconfig::{FlagDeclaration, FlagState, FlagValue, Permission};
- use crate::cache::CacheBuilder;
- use crate::commands::Source;
#[test]
- fn test_cpp_codegen_build_time_flag_only() {
- let package = "com.example";
- let mut builder = CacheBuilder::new(package.to_string()).unwrap();
- builder
- .add_flag_declaration(
- Source::File("aconfig_one.txt".to_string()),
- FlagDeclaration {
- name: "my_flag_one".to_string(),
- namespace: "ns".to_string(),
- description: "buildtime disable".to_string(),
- },
- )
- .unwrap()
- .add_flag_value(
- Source::Memory,
- FlagValue {
- package: package.to_string(),
- name: "my_flag_one".to_string(),
- state: FlagState::Disabled,
- permission: Permission::ReadOnly,
- },
- )
- .unwrap()
- .add_flag_declaration(
- Source::File("aconfig_two.txt".to_string()),
- FlagDeclaration {
- name: "my_flag_two".to_string(),
- namespace: "ns".to_string(),
- description: "buildtime enable".to_string(),
- },
- )
- .unwrap()
- .add_flag_value(
- Source::Memory,
- FlagValue {
- package: package.to_string(),
- name: "my_flag_two".to_string(),
- state: FlagState::Enabled,
- permission: Permission::ReadOnly,
- },
- )
- .unwrap();
- let cache = builder.build();
- let expect_content = r#"#ifndef com_example_HEADER_H
- #define com_example_HEADER_H
+ fn test_generate_cpp_code() {
+ let parsed_flags = crate::test::parse_test_flags();
+ let generated =
+ generate_cpp_code(crate::test::TEST_PACKAGE, parsed_flags.parsed_flag.iter()).unwrap();
+ assert_eq!("aconfig/com_android_aconfig_test.h", format!("{}", generated.path.display()));
+ let expected = r#"
+#ifndef com_android_aconfig_test_HEADER_H
+#define com_android_aconfig_test_HEADER_H
+#include <server_configurable_flags/get_flags.h>
- namespace com::example {
+using namespace server_configurable_flags;
- static const bool my_flag_one() {
- return false;
- }
-
- static const bool my_flag_two() {
- return true;
- }
-
- }
- #endif
- "#;
- let file = generate_cpp_code(&cache).unwrap();
- assert_eq!("aconfig/com_example.h", file.path.to_str().unwrap());
- assert_eq!(
- expect_content.replace(' ', ""),
- String::from_utf8(file.contents).unwrap().replace(' ', "")
- );
+namespace com::android::aconfig::test {
+ static const bool disabled_ro() {
+ return false;
}
- #[test]
- fn test_cpp_codegen_runtime_flag() {
- let package = "com.example";
- let mut builder = CacheBuilder::new(package.to_string()).unwrap();
- builder
- .add_flag_declaration(
- Source::File("aconfig_one.txt".to_string()),
- FlagDeclaration {
- name: "my_flag_one".to_string(),
- namespace: "ns".to_string(),
- description: "buildtime disable".to_string(),
- },
- )
- .unwrap()
- .add_flag_declaration(
- Source::File("aconfig_two.txt".to_string()),
- FlagDeclaration {
- name: "my_flag_two".to_string(),
- namespace: "ns".to_string(),
- description: "runtime enable".to_string(),
- },
- )
- .unwrap()
- .add_flag_value(
- Source::Memory,
- FlagValue {
- package: package.to_string(),
- name: "my_flag_two".to_string(),
- state: FlagState::Enabled,
- permission: Permission::ReadWrite,
- },
- )
- .unwrap();
- let cache = builder.build();
- let expect_content = r#"#ifndef com_example_HEADER_H
- #define com_example_HEADER_H
+ static const bool disabled_rw() {
+ return GetServerConfigurableFlag(
+ "aconfig_test",
+ "com.android.aconfig.test.disabled_rw",
+ "false") == "true";
+ }
- #include <server_configurable_flags/get_flags.h>
- using namespace server_configurable_flags;
+ static const bool enabled_ro() {
+ return true;
+ }
- namespace com::example {
-
- static const bool my_flag_one() {
- return GetServerConfigurableFlag(
- "ns",
- "com.example.my_flag_one",
- "false") == "true";
- }
-
- static const bool my_flag_two() {
- return GetServerConfigurableFlag(
- "ns",
- "com.example.my_flag_two",
- "true") == "true";
- }
-
- }
- #endif
- "#;
- let file = generate_cpp_code(&cache).unwrap();
- assert_eq!("aconfig/com_example.h", file.path.to_str().unwrap());
+ static const bool enabled_rw() {
+ return GetServerConfigurableFlag(
+ "aconfig_test",
+ "com.android.aconfig.test.enabled_rw",
+ "true") == "true";
+ }
+}
+#endif
+"#;
assert_eq!(
None,
crate::test::first_significant_code_diff(
- expect_content,
- &String::from_utf8(file.contents).unwrap()
+ expected,
+ &String::from_utf8(generated.contents).unwrap()
)
);
}