Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1a1a08a3 authored by Micha Schwab's avatar Micha Schwab
Browse files

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
parent 952df85c

tools/aconfig/protos/aconfig.proto

+16 −0
Original line number Diff line number Diff line
@@ -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

+49 −2
Original line number Diff line number Diff line
@@ -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

+24 −6
Original line number Diff line number Diff line
@@ -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());

tools/aconfig/src/test.rs

+21 −0
Original line number Diff line number Diff line
@@ -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

+3 −0
Original line number Diff line number Diff line
@@ -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: