Add metadata to aconfig

Flag metadata will be used both for release strategy, such as allowing faster rollouts for flags, and for test infrastructure such as which platform tests need running.

Doc: go/aconfig-metadata
Test: atest aconfig.test
Change-Id: Idc4b1c6b03c257a1cf92afdfb0b6e54b43741b06

5 files changed, 113 insertions, 8 deletions

diff --git a/tools/aconfig/protos/aconfig.proto b/tools/aconfig/protos/aconfig.proto
index 1a80b0420d..ed4b24c32c 100644
--- a/tools/aconfig/protos/aconfig.proto
+++ b/tools/aconfig/protos/aconfig.proto
@@ -41,8 +41,23 @@ message flag_declaration {
   repeated string bug = 4;
   optional bool is_fixed_read_only = 5;
   optional bool is_exported = 6;
+  optional flag_metadata metadata = 7;
 };
 
+// Optional metadata about the flag, such as its purpose and its intended form factors.
+// Can influence the applied policies and testing strategy.
+message flag_metadata {
+  enum flag_purpose {
+    PURPOSE_UNSPECIFIED = 0;
+    PURPOSE_FEATURE = 1;
+    PURPOSE_BUGFIX = 2;
+  }
+
+  optional flag_purpose purpose = 1;
+
+  // TODO(b/315025930): Add field to designate intended target device form factor(s), such as phone, watch or other.
+}
+
 message flag_declarations {
   optional string package = 1;
   repeated flag_declaration flag = 2;
@@ -81,6 +96,7 @@ message parsed_flag {
   optional bool is_fixed_read_only = 9;
   optional bool is_exported = 10;
   optional string container = 11;
+  optional flag_metadata metadata = 12;
 }
 
 message parsed_flags {
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs
index 37ee79e4da..1e80d267e2 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -24,7 +24,8 @@ use crate::codegen_cpp::generate_cpp_code;
 use crate::codegen_java::generate_java_code;
 use crate::codegen_rust::generate_rust_code;
 use crate::protos::{
-    ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag, ProtoParsedFlags, ProtoTracepoint,
+    ProtoFlagMetadata, ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag, ProtoParsedFlags,
+    ProtoTracepoint,
 };
 
 pub struct Input {
@@ -118,6 +119,11 @@ pub fn parse_flags(
             tracepoint.set_permission(flag_permission);
             parsed_flag.trace.push(tracepoint);
 
+            let mut metadata = ProtoFlagMetadata::new();
+            let purpose = flag_declaration.metadata.purpose();
+            metadata.set_purpose(purpose);
+            parsed_flag.metadata = Some(metadata).into();
+
             // verify ParsedFlag looks reasonable
             crate::protos::parsed_flag::verify_fields(&parsed_flag)?;
 
@@ -264,7 +270,11 @@ pub enum DumpFormat {
     Textproto,
 }
 
-pub fn dump_parsed_flags(mut input: Vec<Input>, format: DumpFormat, dedup: bool) -> Result<Vec<u8>> {
+pub fn dump_parsed_flags(
+    mut input: Vec<Input>,
+    format: DumpFormat,
+    dedup: bool,
+) -> Result<Vec<u8>> {
     let individually_parsed_flags: Result<Vec<ProtoParsedFlags>> =
         input.iter_mut().map(|i| i.try_parse_flags()).collect();
     let parsed_flags: ProtoParsedFlags =
@@ -325,6 +335,7 @@ fn find_unique_package(parsed_flags: &ProtoParsedFlags) -> Option<&str> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::protos::ProtoFlagPurpose;
 
     #[test]
     fn test_parse_flags() {
@@ -339,6 +350,7 @@ mod tests {
         assert_eq!("This flag is ENABLED + READ_ONLY", enabled_ro.description());
         assert_eq!(ProtoFlagState::ENABLED, enabled_ro.state());
         assert_eq!(ProtoFlagPermission::READ_ONLY, enabled_ro.permission());
+        assert_eq!(ProtoFlagPurpose::PURPOSE_BUGFIX, enabled_ro.metadata.purpose());
         assert_eq!(3, enabled_ro.trace.len());
         assert!(!enabled_ro.is_fixed_read_only());
         assert_eq!("tests/test.aconfig", enabled_ro.trace[0].source());
@@ -511,6 +523,41 @@ mod tests {
     }
 
     #[test]
+    fn test_parse_flags_metadata() {
+        let metadata_flag = r#"
+        package: "com.first"
+        flag {
+            name: "first"
+            namespace: "first_ns"
+            description: "This is the description of this feature flag."
+            bug: "123"
+            metadata {
+                purpose: PURPOSE_FEATURE
+            }
+        }
+        "#;
+        let declaration = vec![Input {
+            source: "memory".to_string(),
+            reader: Box::new(metadata_flag.as_bytes()),
+        }];
+        let value: Vec<Input> = vec![];
+
+        let flags_bytes = crate::commands::parse_flags(
+            "com.first",
+            None,
+            declaration,
+            value,
+            ProtoFlagPermission::READ_ONLY,
+        )
+        .unwrap();
+        let parsed_flags =
+            crate::protos::parsed_flags::try_from_binary_proto(&flags_bytes).unwrap();
+        assert_eq!(1, parsed_flags.parsed_flag.len());
+        let parsed_flag = parsed_flags.parsed_flag.first().unwrap();
+        assert_eq!(ProtoFlagPurpose::PURPOSE_FEATURE, parsed_flag.metadata.purpose());
+    }
+
+    #[test]
     fn test_create_device_config_defaults() {
         let input = parse_test_flags_as_input();
         let bytes = create_device_config_defaults(input).unwrap();
diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs
index 3d9089cb42..37eb67d85a 100644
--- a/tools/aconfig/src/protos.rs
+++ b/tools/aconfig/src/protos.rs
@@ -29,8 +29,10 @@
 // ---- When building with the Android tool-chain ----
 #[cfg(not(feature = "cargo"))]
 mod auto_generated {
+    pub use aconfig_protos::aconfig::flag_metadata::Flag_purpose as ProtoFlagPurpose;
     pub use aconfig_protos::aconfig::Flag_declaration as ProtoFlagDeclaration;
     pub use aconfig_protos::aconfig::Flag_declarations as ProtoFlagDeclarations;
+    pub use aconfig_protos::aconfig::Flag_metadata as ProtoFlagMetadata;
     pub use aconfig_protos::aconfig::Flag_permission as ProtoFlagPermission;
     pub use aconfig_protos::aconfig::Flag_state as ProtoFlagState;
     pub use aconfig_protos::aconfig::Flag_value as ProtoFlagValue;
@@ -47,8 +49,10 @@ mod auto_generated {
     // because this is only used during local development, and only if using cargo instead of the
     // Android tool-chain, we allow it
     include!(concat!(env!("OUT_DIR"), "/aconfig_proto/mod.rs"));
+    pub use aconfig::flag_metadata::Flag_purpose as ProtoFlagPurpose;
     pub use aconfig::Flag_declaration as ProtoFlagDeclaration;
     pub use aconfig::Flag_declarations as ProtoFlagDeclarations;
+    pub use aconfig::Flag_metadata as ProtoFlagMetadata;
     pub use aconfig::Flag_permission as ProtoFlagPermission;
     pub use aconfig::Flag_state as ProtoFlagState;
     pub use aconfig::Flag_value as ProtoFlagValue;
@@ -938,11 +942,13 @@ parsed_flag {
         assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.first.first (defined in flags.declarations and flags.declarations)");
 
         // two conflicting flags with dedup disabled
-        let error = parsed_flags::merge(vec![second.clone(), second_duplicate.clone()], false).unwrap_err();
+        let error =
+            parsed_flags::merge(vec![second.clone(), second_duplicate.clone()], false).unwrap_err();
         assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.second.second (defined in flags.declarations and duplicate/flags.declarations)");
 
         // two conflicting flags with dedup enabled
-        let error = parsed_flags::merge(vec![second.clone(), second_duplicate.clone()], true).unwrap_err();
+        let error =
+            parsed_flags::merge(vec![second.clone(), second_duplicate.clone()], true).unwrap_err();
         assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.second.second (defined in flags.declarations and duplicate/flags.declarations)");
 
         // valid cases
@@ -950,10 +956,22 @@ parsed_flag {
         assert!(parsed_flags::merge(vec![], true).unwrap().parsed_flag.is_empty());
         assert_eq!(first, parsed_flags::merge(vec![first.clone()], false).unwrap());
         assert_eq!(first, parsed_flags::merge(vec![first.clone()], true).unwrap());
-        assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()], false).unwrap());
-        assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()], true).unwrap());
-        assert_eq!(expected, parsed_flags::merge(vec![second.clone(), first.clone()], false).unwrap());
-        assert_eq!(expected, parsed_flags::merge(vec![second.clone(), first.clone()], true).unwrap());
+        assert_eq!(
+            expected,
+            parsed_flags::merge(vec![first.clone(), second.clone()], false).unwrap()
+        );
+        assert_eq!(
+            expected,
+            parsed_flags::merge(vec![first.clone(), second.clone()], true).unwrap()
+        );
+        assert_eq!(
+            expected,
+            parsed_flags::merge(vec![second.clone(), first.clone()], false).unwrap()
+        );
+        assert_eq!(
+            expected,
+            parsed_flags::merge(vec![second.clone(), first.clone()], true).unwrap()
+        );
 
         // two identical flags with dedup enabled
         assert_eq!(first, parsed_flags::merge(vec![first.clone(), first.clone()], true).unwrap());
diff --git a/tools/aconfig/src/test.rs b/tools/aconfig/src/test.rs
index 8df4493250..2c63fefdbc 100644
--- a/tools/aconfig/src/test.rs
+++ b/tools/aconfig/src/test.rs
@@ -44,6 +44,9 @@ parsed_flag {
   is_fixed_read_only: false
   is_exported: false
   container: "system"
+  metadata {
+    purpose: PURPOSE_UNSPECIFIED
+  }
 }
 parsed_flag {
   package: "com.android.aconfig.test"
@@ -61,6 +64,9 @@ parsed_flag {
   is_fixed_read_only: false
   is_exported: true
   container: "system"
+  metadata {
+    purpose: PURPOSE_UNSPECIFIED
+  }
 }
 parsed_flag {
   package: "com.android.aconfig.test"
@@ -83,6 +89,9 @@ parsed_flag {
   is_fixed_read_only: false
   is_exported: true
   container: "system"
+  metadata {
+    purpose: PURPOSE_UNSPECIFIED
+  }
 }
 parsed_flag {
   package: "com.android.aconfig.test"
@@ -105,6 +114,9 @@ parsed_flag {
   is_fixed_read_only: false
   is_exported: false
   container: "system"
+  metadata {
+    purpose: PURPOSE_UNSPECIFIED
+  }
 }
 parsed_flag {
   package: "com.android.aconfig.test"
@@ -127,6 +139,9 @@ parsed_flag {
   is_fixed_read_only: true
   is_exported: false
   container: "system"
+  metadata {
+    purpose: PURPOSE_UNSPECIFIED
+  }
 }
 parsed_flag {
   package: "com.android.aconfig.test"
@@ -154,6 +169,9 @@ parsed_flag {
   is_fixed_read_only: false
   is_exported: false
   container: "system"
+  metadata {
+    purpose: PURPOSE_BUGFIX
+  }
 }
 parsed_flag {
   package: "com.android.aconfig.test"
@@ -176,6 +194,9 @@ parsed_flag {
   is_fixed_read_only: false
   is_exported: false
   container: "system"
+  metadata {
+    purpose: PURPOSE_UNSPECIFIED
+  }
 }
 "#;
 
diff --git a/tools/aconfig/tests/test.aconfig b/tools/aconfig/tests/test.aconfig
index e44d15466e..310d0a640f 100644
--- a/tools/aconfig/tests/test.aconfig
+++ b/tools/aconfig/tests/test.aconfig
@@ -10,6 +10,9 @@ flag {
     namespace: "aconfig_test"
     description: "This flag is ENABLED + READ_ONLY"
     bug: "abc"
+    metadata {
+        purpose: PURPOSE_BUGFIX
+    }
 }
 
 # This flag's final value is calculated from: