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
diff --git a/tools/aconfig/protos/aconfig.proto b/tools/aconfig/protos/aconfig.proto
index 1a80b04..ed4b24c 100644
--- a/tools/aconfig/protos/aconfig.proto
+++ b/tools/aconfig/protos/aconfig.proto
@@ -41,8 +41,23 @@
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 @@
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 37ee79e..1e80d26 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -24,7 +24,8 @@
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 @@
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 @@
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 @@
#[cfg(test)]
mod tests {
use super::*;
+ use crate::protos::ProtoFlagPurpose;
#[test]
fn test_parse_flags() {
@@ -339,6 +350,7 @@
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 @@
}
#[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 3d9089c..37eb67d 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 @@
// 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 @@
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 @@
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 8df4493..2c63fef 100644
--- a/tools/aconfig/src/test.rs
+++ b/tools/aconfig/src/test.rs
@@ -44,6 +44,9 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 e44d154..310d0a6 100644
--- a/tools/aconfig/tests/test.aconfig
+++ b/tools/aconfig/tests/test.aconfig
@@ -10,6 +10,9 @@
namespace: "aconfig_test"
description: "This flag is ENABLED + READ_ONLY"
bug: "abc"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
}
# This flag's final value is calculated from: