Merge "Add metadata to aconfig" into main

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 {

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());
@@ -510,6 +522,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();

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!(
-        assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()], true).unwrap());
+            expected,
-        assert_eq!(expected, parsed_flags::merge(vec![second.clone(), first.clone()], false).unwrap());
+            parsed_flags::merge(vec![first.clone(), second.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()], 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());

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
+  }
 }
 "#;
 

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: