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/protos.rs b/tools/aconfig/src/protos.rs
index 604fd35..fb5dab4 100644
--- a/tools/aconfig/src/protos.rs
+++ b/tools/aconfig/src/protos.rs
@@ -63,10 +63,622 @@
use anyhow::Result;
-pub fn try_from_text_proto<T>(s: &str) -> Result<T>
+fn try_from_text_proto<T>(s: &str) -> Result<T>
where
T: protobuf::MessageFull,
{
// warning: parse_from_str does not check if required fields are set
protobuf::text_format::parse_from_str(s).map_err(|e| e.into())
}
+
+pub mod flag_declaration {
+ use super::*;
+ use crate::codegen;
+ use anyhow::ensure;
+
+ pub fn verify_fields(pdf: &ProtoFlagDeclaration) -> Result<()> {
+ ensure!(codegen::is_valid_name_ident(pdf.name()), "bad flag declaration: bad name");
+ ensure!(codegen::is_valid_name_ident(pdf.namespace()), "bad flag declaration: bad name");
+ ensure!(!pdf.description().is_empty(), "bad flag declaration: empty description");
+ Ok(())
+ }
+}
+
+pub mod flag_declarations {
+ use super::*;
+ use crate::codegen;
+ use anyhow::ensure;
+
+ pub fn try_from_text_proto(s: &str) -> Result<ProtoFlagDeclarations> {
+ let pdf: ProtoFlagDeclarations = super::try_from_text_proto(s)?;
+ verify_fields(&pdf)?;
+ Ok(pdf)
+ }
+
+ pub fn verify_fields(pdf: &ProtoFlagDeclarations) -> Result<()> {
+ ensure!(
+ codegen::is_valid_package_ident(pdf.package()),
+ "bad flag declarations: bad package"
+ );
+ for flag_declaration in pdf.flag.iter() {
+ super::flag_declaration::verify_fields(flag_declaration)?;
+ }
+ Ok(())
+ }
+}
+
+pub mod flag_value {
+ use super::*;
+ use crate::codegen;
+ use anyhow::ensure;
+
+ pub fn verify_fields(fv: &ProtoFlagValue) -> Result<()> {
+ ensure!(codegen::is_valid_package_ident(fv.package()), "bad flag value: bad package");
+ ensure!(codegen::is_valid_name_ident(fv.name()), "bad flag value: bad name");
+ Ok(())
+ }
+}
+
+pub mod flag_values {
+ use super::*;
+
+ pub fn try_from_text_proto(s: &str) -> Result<ProtoFlagValues> {
+ let pfv: ProtoFlagValues = super::try_from_text_proto(s)?;
+ verify_fields(&pfv)?;
+ Ok(pfv)
+ }
+
+ pub fn verify_fields(pfv: &ProtoFlagValues) -> Result<()> {
+ for flag_value in pfv.flag_value.iter() {
+ super::flag_value::verify_fields(flag_value)?;
+ }
+ Ok(())
+ }
+}
+
+pub mod tracepoint {
+ use super::*;
+ use anyhow::ensure;
+
+ pub fn verify_fields(tp: &ProtoTracepoint) -> Result<()> {
+ ensure!(!tp.source().is_empty(), "bad tracepoint: empty source");
+ Ok(())
+ }
+}
+
+pub mod parsed_flag {
+ use super::*;
+ use crate::codegen;
+ use anyhow::ensure;
+
+ pub fn verify_fields(pf: &ProtoParsedFlag) -> Result<()> {
+ ensure!(codegen::is_valid_package_ident(pf.package()), "bad parsed flag: bad package");
+ ensure!(codegen::is_valid_name_ident(pf.name()), "bad parsed flag: bad name");
+ ensure!(codegen::is_valid_name_ident(pf.namespace()), "bad parsed flag: bad namespace");
+ ensure!(!pf.description().is_empty(), "bad parsed flag: empty description");
+ ensure!(!pf.trace.is_empty(), "bad parsed flag: empty trace");
+ for tp in pf.trace.iter() {
+ super::tracepoint::verify_fields(tp)?;
+ }
+ Ok(())
+ }
+}
+
+pub mod parsed_flags {
+ use super::*;
+ use anyhow::bail;
+ use std::cmp::Ordering;
+
+ pub fn try_from_binary_proto(bytes: &[u8]) -> Result<ProtoParsedFlags> {
+ let message: ProtoParsedFlags = protobuf::Message::parse_from_bytes(bytes)?;
+ verify_fields(&message)?;
+ Ok(message)
+ }
+
+ pub fn verify_fields(pf: &ProtoParsedFlags) -> Result<()> {
+ let mut previous: Option<&ProtoParsedFlag> = None;
+ for parsed_flag in pf.parsed_flag.iter() {
+ if let Some(prev) = previous {
+ let a = create_sorting_key(prev);
+ let b = create_sorting_key(parsed_flag);
+ match a.cmp(&b) {
+ Ordering::Less => {}
+ Ordering::Equal => bail!("bad parsed flags: duplicate flag {}", a),
+ Ordering::Greater => {
+ bail!("bad parsed flags: not sorted: {} comes before {}", a, b)
+ }
+ }
+ }
+ super::parsed_flag::verify_fields(parsed_flag)?;
+ previous = Some(parsed_flag);
+ }
+ Ok(())
+ }
+
+ pub fn merge(parsed_flags: Vec<ProtoParsedFlags>) -> Result<ProtoParsedFlags> {
+ let mut merged = ProtoParsedFlags::new();
+ for mut pfs in parsed_flags.into_iter() {
+ merged.parsed_flag.append(&mut pfs.parsed_flag);
+ }
+ merged.parsed_flag.sort_by_cached_key(create_sorting_key);
+ verify_fields(&merged)?;
+ Ok(merged)
+ }
+
+ fn create_sorting_key(pf: &ProtoParsedFlag) -> String {
+ format!("{}.{}", pf.package(), pf.name())
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn test_flag_declarations_try_from_text_proto() {
+ // valid input
+ let flag_declarations = flag_declarations::try_from_text_proto(
+ r#"
+package: "com.foo.bar"
+flag {
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+}
+flag {
+ name: "second"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+}
+"#,
+ )
+ .unwrap();
+ assert_eq!(flag_declarations.package(), "com.foo.bar");
+ let first = flag_declarations.flag.iter().find(|pf| pf.name() == "first").unwrap();
+ assert_eq!(first.name(), "first");
+ assert_eq!(first.namespace(), "first_ns");
+ assert_eq!(first.description(), "This is the description of the first flag.");
+ let second = flag_declarations.flag.iter().find(|pf| pf.name() == "second").unwrap();
+ assert_eq!(second.name(), "second");
+ assert_eq!(second.namespace(), "second_ns");
+ assert_eq!(second.description(), "This is the description of the second flag.");
+
+ // bad input: missing package in flag declarations
+ let error = flag_declarations::try_from_text_proto(
+ r#"
+flag {
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+}
+flag {
+ name: "second"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+}
+"#,
+ )
+ .unwrap_err();
+ assert!(format!("{:?}", error).contains("Message not initialized"));
+
+ // bad input: missing namespace in flag declaration
+ let error = flag_declarations::try_from_text_proto(
+ r#"
+package: "com.foo.bar"
+flag {
+ name: "first"
+ description: "This is the description of the first flag."
+}
+flag {
+ name: "second"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+}
+"#,
+ )
+ .unwrap_err();
+ assert!(format!("{:?}", error).contains("Message not initialized"));
+
+ // bad input: bad package name in flag declarations
+ let error = flag_declarations::try_from_text_proto(
+ r#"
+package: "_com.FOO__BAR"
+flag {
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+}
+flag {
+ name: "second"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+}
+"#,
+ )
+ .unwrap_err();
+ assert!(format!("{:?}", error).contains("bad flag declarations: bad package"));
+
+ // bad input: bad name in flag declaration
+ let error = flag_declarations::try_from_text_proto(
+ r#"
+package: "com.foo.bar"
+flag {
+ name: "FIRST"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+}
+flag {
+ name: "second"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+}
+"#,
+ )
+ .unwrap_err();
+ assert!(format!("{:?}", error).contains("bad flag declaration: bad name"));
+ }
+
+ #[test]
+ fn test_flag_values_try_from_text_proto() {
+ // valid input
+ let flag_values = flag_values::try_from_text_proto(
+ r#"
+flag_value {
+ package: "com.first"
+ name: "first"
+ state: DISABLED
+ permission: READ_ONLY
+}
+flag_value {
+ package: "com.second"
+ name: "second"
+ state: ENABLED
+ permission: READ_WRITE
+}
+"#,
+ )
+ .unwrap();
+ let first = flag_values.flag_value.iter().find(|fv| fv.name() == "first").unwrap();
+ assert_eq!(first.package(), "com.first");
+ assert_eq!(first.name(), "first");
+ assert_eq!(first.state(), ProtoFlagState::DISABLED);
+ assert_eq!(first.permission(), ProtoFlagPermission::READ_ONLY);
+ let second = flag_values.flag_value.iter().find(|fv| fv.name() == "second").unwrap();
+ assert_eq!(second.package(), "com.second");
+ assert_eq!(second.name(), "second");
+ assert_eq!(second.state(), ProtoFlagState::ENABLED);
+ assert_eq!(second.permission(), ProtoFlagPermission::READ_WRITE);
+
+ // bad input: bad package in flag value
+ let error = flag_values::try_from_text_proto(
+ r#"
+flag_value {
+ package: "COM.FIRST"
+ name: "first"
+ state: DISABLED
+ permission: READ_ONLY
+}
+"#,
+ )
+ .unwrap_err();
+ assert!(format!("{:?}", error).contains("bad flag value: bad package"));
+
+ // bad input: bad name in flag value
+ let error = flag_values::try_from_text_proto(
+ r#"
+flag_value {
+ package: "com.first"
+ name: "FIRST"
+ state: DISABLED
+ permission: READ_ONLY
+}
+"#,
+ )
+ .unwrap_err();
+ assert!(format!("{:?}", error).contains("bad flag value: bad name"));
+
+ // bad input: missing state in flag value
+ let error = flag_values::try_from_text_proto(
+ r#"
+flag_value {
+ package: "com.first"
+ name: "first"
+ permission: READ_ONLY
+}
+"#,
+ )
+ .unwrap_err();
+ assert!(format!("{:?}", error).contains("Message not initialized"));
+
+ // bad input: missing permission in flag value
+ let error = flag_values::try_from_text_proto(
+ r#"
+flag_value {
+ package: "com.first"
+ name: "first"
+ state: DISABLED
+}
+"#,
+ )
+ .unwrap_err();
+ assert!(format!("{:?}", error).contains("Message not initialized"));
+ }
+
+ fn try_from_binary_proto_from_text_proto(text_proto: &str) -> Result<ProtoParsedFlags> {
+ use protobuf::Message;
+
+ let parsed_flags: ProtoParsedFlags = try_from_text_proto(text_proto)?;
+ let mut binary_proto = Vec::new();
+ parsed_flags.write_to_vec(&mut binary_proto)?;
+ parsed_flags::try_from_binary_proto(&binary_proto)
+ }
+
+ #[test]
+ fn test_parsed_flags_try_from_text_proto() {
+ // valid input
+ let text_proto = r#"
+parsed_flag {
+ package: "com.first"
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+ state: DISABLED
+ permission: READ_ONLY
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+parsed_flag {
+ package: "com.second"
+ name: "second"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+ state: ENABLED
+ permission: READ_WRITE
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+ trace {
+ source: "flags.values"
+ state: ENABLED
+ permission: READ_WRITE
+ }
+}
+"#;
+ let parsed_flags = try_from_binary_proto_from_text_proto(text_proto).unwrap();
+ assert_eq!(parsed_flags.parsed_flag.len(), 2);
+ let second = parsed_flags.parsed_flag.iter().find(|fv| fv.name() == "second").unwrap();
+ assert_eq!(second.package(), "com.second");
+ assert_eq!(second.name(), "second");
+ assert_eq!(second.namespace(), "second_ns");
+ assert_eq!(second.description(), "This is the description of the second flag.");
+ assert_eq!(second.state(), ProtoFlagState::ENABLED);
+ assert_eq!(second.permission(), ProtoFlagPermission::READ_WRITE);
+ assert_eq!(2, second.trace.len());
+ assert_eq!(second.trace[0].source(), "flags.declarations");
+ assert_eq!(second.trace[0].state(), ProtoFlagState::DISABLED);
+ assert_eq!(second.trace[0].permission(), ProtoFlagPermission::READ_ONLY);
+ assert_eq!(second.trace[1].source(), "flags.values");
+ assert_eq!(second.trace[1].state(), ProtoFlagState::ENABLED);
+ assert_eq!(second.trace[1].permission(), ProtoFlagPermission::READ_WRITE);
+
+ // valid input: empty
+ let parsed_flags = try_from_binary_proto_from_text_proto("").unwrap();
+ assert!(parsed_flags.parsed_flag.is_empty());
+
+ // bad input: empty trace
+ let text_proto = r#"
+parsed_flag {
+ package: "com.first"
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+ state: DISABLED
+ permission: READ_ONLY
+}
+"#;
+ let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();
+ assert_eq!(format!("{:?}", error), "bad parsed flag: empty trace");
+
+ // bad input: missing fields in parsed_flag
+ let text_proto = r#"
+parsed_flag {
+ package: "com.first"
+ name: "first"
+ description: "This is the description of the first flag."
+ state: DISABLED
+ permission: READ_ONLY
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+"#;
+ let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();
+ assert!(format!("{:?}", error).contains("Message not initialized"));
+
+ // bad input: parsed_flag not sorted by package
+ let text_proto = r#"
+parsed_flag {
+ package: "bbb"
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+ state: DISABLED
+ permission: READ_ONLY
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+parsed_flag {
+ package: "aaa"
+ name: "second"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+ state: ENABLED
+ permission: READ_WRITE
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+"#;
+ let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();
+ assert_eq!(
+ format!("{:?}", error),
+ "bad parsed flags: not sorted: bbb.first comes before aaa.second"
+ );
+
+ // bad input: parsed_flag not sorted by name
+ let text_proto = r#"
+parsed_flag {
+ package: "com.foo"
+ name: "bbb"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+ state: DISABLED
+ permission: READ_ONLY
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+parsed_flag {
+ package: "com.foo"
+ name: "aaa"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+ state: ENABLED
+ permission: READ_WRITE
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+"#;
+ let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();
+ assert_eq!(
+ format!("{:?}", error),
+ "bad parsed flags: not sorted: com.foo.bbb comes before com.foo.aaa"
+ );
+
+ // bad input: duplicate flags
+ let text_proto = r#"
+parsed_flag {
+ package: "com.foo"
+ name: "bar"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+ state: DISABLED
+ permission: READ_ONLY
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+parsed_flag {
+ package: "com.foo"
+ name: "bar"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+ state: ENABLED
+ permission: READ_WRITE
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+"#;
+ let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err();
+ assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.foo.bar");
+ }
+
+ #[test]
+ fn test_parsed_flags_merge() {
+ let text_proto = r#"
+parsed_flag {
+ package: "com.first"
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+ state: DISABLED
+ permission: READ_ONLY
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+parsed_flag {
+ package: "com.second"
+ name: "second"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+ state: ENABLED
+ permission: READ_WRITE
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+"#;
+ let expected = try_from_binary_proto_from_text_proto(text_proto).unwrap();
+
+ let text_proto = r#"
+parsed_flag {
+ package: "com.first"
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+ state: DISABLED
+ permission: READ_ONLY
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+"#;
+ let first = try_from_binary_proto_from_text_proto(text_proto).unwrap();
+
+ let text_proto = r#"
+parsed_flag {
+ package: "com.second"
+ name: "second"
+ namespace: "second_ns"
+ description: "This is the description of the second flag."
+ state: ENABLED
+ permission: READ_WRITE
+ trace {
+ source: "flags.declarations"
+ state: DISABLED
+ permission: READ_ONLY
+ }
+}
+"#;
+ let second = try_from_binary_proto_from_text_proto(text_proto).unwrap();
+
+ // bad cases
+ let error = parsed_flags::merge(vec![first.clone(), first.clone()]).unwrap_err();
+ assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.first.first");
+
+ // valid cases
+ assert!(parsed_flags::merge(vec![]).unwrap().parsed_flag.is_empty());
+ assert_eq!(first, parsed_flags::merge(vec![first.clone()]).unwrap());
+ assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()]).unwrap());
+ assert_eq!(expected, parsed_flags::merge(vec![second, first]).unwrap());
+ }
+}