Merge changes from topic "aconfig-part-2" am: 80bff11aef

Original change: https://android-review.googlesource.com/c/platform/build/+/2578555

Change-Id: I9f41660ea3cefad70f405b93587baa005b5b4f6e
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

tools/aconfig/protos/aconfig.proto

@@ -20,9 +20,20 @@ syntax = "proto2";
 
 package android.aconfig;
 
+enum flag_state {
+  ENABLED = 1;
+  DISABLED = 2;
+}
+
+enum permission {
+  READ_ONLY = 1;
+  READ_WRITE = 2;
+}
+
 message value {
-  required bool value = 1;
+  required flag_state state = 1;
-  optional uint32 since = 2;
+  required permission permission = 2;
+  optional uint32 since = 3;
 }
 
 message flag {
@@ -37,7 +48,8 @@ message android_config {
 
 message override {
   required string id = 1;
-  required bool value = 2;
+  required flag_state state = 2;
+  required permission permission = 3;
 };
 
 message override_config {

tools/aconfig/src/aconfig.rs

@@ -15,25 +15,65 @@
  */
 
 use anyhow::{anyhow, Context, Error, Result};
+use protobuf::{Enum, EnumOrUnknown};
+use serde::{Deserialize, Serialize};
 
 use crate::protos::{
-    ProtoAndroidConfig, ProtoFlag, ProtoOverride, ProtoOverrideConfig, ProtoValue,
+    ProtoAndroidConfig, ProtoFlag, ProtoFlagState, ProtoOverride, ProtoOverrideConfig,
+    ProtoPermission, ProtoValue,
 };
 
+#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Copy)]
+pub enum FlagState {
+    Enabled,
+    Disabled,
+}
+
+impl TryFrom<EnumOrUnknown<ProtoFlagState>> for FlagState {
+    type Error = Error;
+
+    fn try_from(proto: EnumOrUnknown<ProtoFlagState>) -> Result<Self, Self::Error> {
+        match ProtoFlagState::from_i32(proto.value()) {
+            Some(ProtoFlagState::ENABLED) => Ok(FlagState::Enabled),
+            Some(ProtoFlagState::DISABLED) => Ok(FlagState::Disabled),
+            None => Err(anyhow!("unknown flag state enum value {}", proto.value())),
+        }
+    }
+}
+
+#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Copy)]
+pub enum Permission {
+    ReadOnly,
+    ReadWrite,
+}
+
+impl TryFrom<EnumOrUnknown<ProtoPermission>> for Permission {
+    type Error = Error;
+
+    fn try_from(proto: EnumOrUnknown<ProtoPermission>) -> Result<Self, Self::Error> {
+        match ProtoPermission::from_i32(proto.value()) {
+            Some(ProtoPermission::READ_ONLY) => Ok(Permission::ReadOnly),
+            Some(ProtoPermission::READ_WRITE) => Ok(Permission::ReadWrite),
+            None => Err(anyhow!("unknown permission enum value {}", proto.value())),
+        }
+    }
+}
+
 #[derive(Debug, PartialEq, Eq)]
 pub struct Value {
-    value: bool,
+    state: FlagState,
+    permission: Permission,
     since: Option<u32>,
 }
 
 #[allow(dead_code)] // only used in unit tests
 impl Value {
-    pub fn new(value: bool, since: u32) -> Value {
+    pub fn new(state: FlagState, permission: Permission, since: u32) -> Value {
-        Value { value, since: Some(since) }
+        Value { state, permission, since: Some(since) }
     }
 
-    pub fn default(value: bool) -> Value {
+    pub fn default(state: FlagState, permission: Permission) -> Value {
-        Value { value, since: None }
+        Value { state, permission, since: None }
     }
 }
 
@@ -41,10 +81,15 @@ impl TryFrom<ProtoValue> for Value {
     type Error = Error;
 
     fn try_from(proto: ProtoValue) -> Result<Self, Self::Error> {
-        let Some(value) = proto.value else {
+        let Some(proto_state) = proto.state else {
-            return Err(anyhow!("missing 'value' field"));
+            return Err(anyhow!("missing 'state' field"));
         };
-        Ok(Value { value, since: proto.since })
+        let state = proto_state.try_into()?;
+        let Some(proto_permission) = proto.permission else {
+            return Err(anyhow!("missing 'permission' field"));
+        };
+        let permission = proto_permission.try_into()?;
+        Ok(Value { state, permission, since: proto.since })
     }
 }
 
@@ -72,15 +117,17 @@ impl Flag {
         proto.flag.into_iter().map(|proto_flag| proto_flag.try_into()).collect()
     }
 
-    pub fn resolve_value(&self, build_id: u32) -> bool {
+    pub fn resolve(&self, build_id: u32) -> (FlagState, Permission) {
-        let mut value = self.values[0].value;
+        let mut state = self.values[0].state;
+        let mut permission = self.values[0].permission;
         for candidate in self.values.iter().skip(1) {
             let since = candidate.since.expect("invariant: non-defaults values have Some(since)");
             if since <= build_id {
-                value = candidate.value;
+                state = candidate.state;
+                permission = candidate.permission;
             }
         }
-        value
+        (state, permission)
     }
 }
 
@@ -119,7 +166,8 @@ impl TryFrom<ProtoFlag> for Flag {
 #[derive(Debug, PartialEq, Eq)]
 pub struct Override {
     pub id: String,
-    pub value: bool,
+    pub state: FlagState,
+    pub permission: Permission,
 }
 
 impl Override {
@@ -142,10 +190,15 @@ impl TryFrom<ProtoOverride> for Override {
         let Some(id) = proto.id else {
             return Err(anyhow!("missing 'id' field"));
         };
-        let Some(value) = proto.value else {
+        let Some(proto_state) = proto.state else {
-            return Err(anyhow!("missing 'value' field"));
+            return Err(anyhow!("missing 'state' field"));
         };
-        Ok(Override { id, value })
+        let state = proto_state.try_into()?;
+        let Some(proto_permission) = proto.permission else {
+            return Err(anyhow!("missing 'permission' field"));
+        };
+        let permission = proto_permission.try_into()?;
+        Ok(Override { id, state, permission })
     }
 }
 
@@ -158,17 +211,22 @@ mod tests {
         let expected = Flag {
             id: "1234".to_owned(),
             description: "Description of the flag".to_owned(),
-            values: vec![Value::default(false), Value::new(true, 8)],
+            values: vec![
+                Value::default(FlagState::Disabled, Permission::ReadOnly),
+                Value::new(FlagState::Enabled, Permission::ReadWrite, 8),
+            ],
         };
 
         let s = r#"
         id: "1234"
         description: "Description of the flag"
         value {
-            value: false
+            state: DISABLED
+            permission: READ_ONLY
         }
         value {
-            value: true
+            state: ENABLED
+            permission: READ_WRITE
             since: 8
         }
         "#;
@@ -189,7 +247,8 @@ mod tests {
         let s = r#"
         description: "Description of the flag"
         value {
-            value: true
+            state: ENABLED
+            permission: READ_ONLY
         }
         "#;
         let error = Flag::try_from_text_proto(s).unwrap_err();
@@ -199,10 +258,12 @@ mod tests {
         id: "a"
         description: "Description of the flag"
         value {
-            value: true
+            state: ENABLED
+            permission: READ_ONLY
         }
         value {
-            value: true
+            state: ENABLED
+            permission: READ_ONLY
         }
         "#;
         let error = Flag::try_from_text_proto(s).unwrap_err();
@@ -215,12 +276,12 @@ mod tests {
             Flag {
                 id: "a".to_owned(),
                 description: "A".to_owned(),
-                values: vec![Value::default(true)],
+                values: vec![Value::default(FlagState::Enabled, Permission::ReadOnly)],
             },
             Flag {
                 id: "b".to_owned(),
                 description: "B".to_owned(),
-                values: vec![Value::default(false)],
+                values: vec![Value::default(FlagState::Disabled, Permission::ReadWrite)],
             },
         ];
 
@@ -229,14 +290,16 @@ mod tests {
             id: "a"
             description: "A"
             value {
-                value: true
+                state: ENABLED
+                permission: READ_ONLY
             }
         }
         flag {
             id: "b"
             description: "B"
             value {
-                value: false
+                state: DISABLED
+                permission: READ_WRITE
             }
         }
         "#;
@@ -247,11 +310,16 @@ mod tests {
 
     #[test]
     fn test_override_try_from_text_proto_list() {
-        let expected = Override { id: "1234".to_owned(), value: true };
+        let expected = Override {
+            id: "1234".to_owned(),
+            state: FlagState::Enabled,
+            permission: Permission::ReadOnly,
+        };
 
         let s = r#"
         id: "1234"
-        value: true
+        state: ENABLED
+        permission: READ_ONLY
         "#;
         let actual = Override::try_from_text_proto(s).unwrap();
 
@@ -259,26 +327,26 @@ mod tests {
     }
 
     #[test]
-    fn test_resolve_value() {
+    fn test_flag_resolve() {
         let flag = Flag {
             id: "a".to_owned(),
             description: "A".to_owned(),
             values: vec![
-                Value::default(true),
+                Value::default(FlagState::Disabled, Permission::ReadOnly),
-                Value::new(false, 10),
+                Value::new(FlagState::Disabled, Permission::ReadWrite, 10),
-                Value::new(true, 20),
+                Value::new(FlagState::Enabled, Permission::ReadOnly, 20),
-                Value::new(false, 30),
+                Value::new(FlagState::Enabled, Permission::ReadWrite, 30),
             ],
         };
-        assert!(flag.resolve_value(0));
+        assert_eq!((FlagState::Disabled, Permission::ReadOnly), flag.resolve(0));
-        assert!(flag.resolve_value(9));
+        assert_eq!((FlagState::Disabled, Permission::ReadOnly), flag.resolve(9));
-        assert!(!flag.resolve_value(10));
+        assert_eq!((FlagState::Disabled, Permission::ReadWrite), flag.resolve(10));
-        assert!(!flag.resolve_value(11));
+        assert_eq!((FlagState::Disabled, Permission::ReadWrite), flag.resolve(11));
-        assert!(!flag.resolve_value(19));
+        assert_eq!((FlagState::Disabled, Permission::ReadWrite), flag.resolve(19));
-        assert!(flag.resolve_value(20));
+        assert_eq!((FlagState::Enabled, Permission::ReadOnly), flag.resolve(20));
-        assert!(flag.resolve_value(21));
+        assert_eq!((FlagState::Enabled, Permission::ReadOnly), flag.resolve(21));
-        assert!(flag.resolve_value(29));
+        assert_eq!((FlagState::Enabled, Permission::ReadOnly), flag.resolve(29));
-        assert!(!flag.resolve_value(30));
+        assert_eq!((FlagState::Enabled, Permission::ReadWrite), flag.resolve(30));
-        assert!(!flag.resolve_value(10_000));
+        assert_eq!((FlagState::Enabled, Permission::ReadWrite), flag.resolve(10_000));
     }
 }

tools/aconfig/src/cache.rs

@@ -18,18 +18,19 @@ use anyhow::{anyhow, Result};
 use serde::{Deserialize, Serialize};
 use std::io::{Read, Write};
 
-use crate::aconfig::{Flag, Override};
+use crate::aconfig::{Flag, FlagState, Override, Permission};
 use crate::commands::Source;
 
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
 pub struct Item {
     pub id: String,
     pub description: String,
-    pub value: bool,
+    pub state: FlagState,
+    pub permission: Permission,
     pub debug: Vec<String>,
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
 pub struct Cache {
     build_id: u32,
     items: Vec<Item>,
@@ -56,12 +57,13 @@ impl Cache {
                 source,
             ));
         }
-        let value = flag.resolve_value(self.build_id);
+        let (state, permission) = flag.resolve(self.build_id);
         self.items.push(Item {
             id: flag.id.clone(),
             description: flag.description,
-            value,
+            state,
-            debug: vec![format!("{}:{}", source, value)],
+            permission,
+            debug: vec![format!("{}:{:?} {:?}", source, state, permission)],
         });
         Ok(())
     }
@@ -70,8 +72,11 @@ impl Cache {
         let Some(existing_item) = self.items.iter_mut().find(|item| item.id == override_.id) else {
             return Err(anyhow!("failed to override flag {}: unknown flag", override_.id));
         };
-        existing_item.value = override_.value;
+        existing_item.state = override_.state;
-        existing_item.debug.push(format!("{}:{}", source, override_.value));
+        existing_item.permission = override_.permission;
+        existing_item
+            .debug
+            .push(format!("{}:{:?} {:?}", source, override_.state, override_.permission));
         Ok(())
     }
 
@@ -85,7 +90,7 @@ impl Item {}
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::aconfig::Value;
+    use crate::aconfig::{FlagState, Permission, Value};
 
     #[test]
     fn test_add_flag() {
@@ -96,7 +101,7 @@ mod tests {
                 Flag {
                     id: "foo".to_string(),
                     description: "desc".to_string(),
-                    values: vec![Value::default(true)],
+                    values: vec![Value::default(FlagState::Enabled, Permission::ReadOnly)],
                 },
             )
             .unwrap();
@@ -106,7 +111,7 @@ mod tests {
                 Flag {
                     id: "foo".to_string(),
                     description: "desc".to_string(),
-                    values: vec![Value::default(false)],
+                    values: vec![Value::default(FlagState::Disabled, Permission::ReadOnly)],
                 },
             )
             .unwrap_err();
@@ -118,13 +123,21 @@ mod tests {
 
     #[test]
     fn test_add_override() {
-        fn check_value(cache: &Cache, id: &str, expected: bool) -> bool {
+        fn check(cache: &Cache, id: &str, expected: (FlagState, Permission)) -> bool {
-            cache.iter().find(|&item| item.id == id).unwrap().value == expected
+            let item = cache.iter().find(|&item| item.id == id).unwrap();
+            item.state == expected.0 && item.permission == expected.1
         }
 
         let mut cache = Cache::new(1);
         let error = cache
-            .add_override(Source::Memory, Override { id: "foo".to_string(), value: true })
+            .add_override(
+                Source::Memory,
+                Override {
+                    id: "foo".to_string(),
+                    state: FlagState::Enabled,
+                    permission: Permission::ReadOnly,
+                },
+            )
             .unwrap_err();
         assert_eq!(&format!("{:?}", error), "failed to override flag foo: unknown flag");
 
@@ -134,20 +147,36 @@ mod tests {
                 Flag {
                     id: "foo".to_string(),
                     description: "desc".to_string(),
-                    values: vec![Value::default(true)],
+                    values: vec![Value::default(FlagState::Enabled, Permission::ReadOnly)],
                 },
             )
             .unwrap();
-        assert!(check_value(&cache, "foo", true));
+        dbg!(&cache);
+        assert!(check(&cache, "foo", (FlagState::Enabled, Permission::ReadOnly)));
 
         cache
-            .add_override(Source::Memory, Override { id: "foo".to_string(), value: false })
+            .add_override(
+                Source::Memory,
+                Override {
+                    id: "foo".to_string(),
+                    state: FlagState::Disabled,
+                    permission: Permission::ReadWrite,
+                },
+            )
             .unwrap();
-        assert!(check_value(&cache, "foo", false));
+        dbg!(&cache);
+        assert!(check(&cache, "foo", (FlagState::Disabled, Permission::ReadWrite)));
 
         cache
-            .add_override(Source::Memory, Override { id: "foo".to_string(), value: true })
+            .add_override(
+                Source::Memory,
+                Override {
+                    id: "foo".to_string(),
+                    state: FlagState::Enabled,
+                    permission: Permission::ReadWrite,
+                },
+            )
             .unwrap();
-        assert!(check_value(&cache, "foo", true));
+        assert!(check(&cache, "foo", (FlagState::Enabled, Permission::ReadWrite)));
     }
 }

tools/aconfig/src/commands.rs

@@ -16,14 +16,13 @@
 
 use anyhow::{Context, Result};
 use clap::ValueEnum;
-use serde::{Deserialize, Serialize};
 use std::fmt;
 use std::io::Read;
 
 use crate::aconfig::{Flag, Override};
 use crate::cache::Cache;
 
-#[derive(Clone, Serialize, Deserialize)]
+#[derive(Clone)]
 pub enum Source {
     #[allow(dead_code)] // only used in unit tests
     Memory,
@@ -80,12 +79,12 @@ pub fn dump_cache(cache: Cache, format: Format) -> Result<()> {
     match format {
         Format::Text => {
             for item in cache.iter() {
-                println!("{}: {}", item.id, item.value);
+                println!("{}: {:?}", item.id, item.state);
             }
         }
         Format::Debug => {
             for item in cache.iter() {
-                println!("{}: {} ({:?})", item.id, item.value, item.debug);
+                println!("{:?}", item);
             }
         }
     }
@@ -95,6 +94,7 @@ pub fn dump_cache(cache: Cache, format: Format) -> Result<()> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::aconfig::{FlagState, Permission};
 
     #[test]
     fn test_create_cache() {
@@ -103,7 +103,8 @@ mod tests {
             id: "a"
             description: "Description of a"
             value {
-                value: true
+                state: ENABLED
+                permission: READ_WRITE
             }
         }
         "#;
@@ -111,12 +112,14 @@ mod tests {
         let o = r#"
         override {
             id: "a"
-            value: false
+            state: DISABLED
+            permission: READ_ONLY
         }
         "#;
         let overrides = vec![Input { source: Source::Memory, reader: Box::new(o.as_bytes()) }];
         let cache = create_cache(1, aconfigs, overrides).unwrap();
-        let value = cache.iter().find(|&item| item.id == "a").unwrap().value;
+        let item = cache.iter().find(|&item| item.id == "a").unwrap();
-        assert!(!value);
+        assert_eq!(FlagState::Disabled, item.state);
+        assert_eq!(Permission::ReadOnly, item.permission);
     }
 }

tools/aconfig/src/protos.rs

@@ -42,6 +42,12 @@ pub use aconfig_protos::aconfig::Override_config as ProtoOverrideConfig;
 #[cfg(not(feature = "cargo"))]
 pub use aconfig_protos::aconfig::Override as ProtoOverride;
 
+#[cfg(not(feature = "cargo"))]
+pub use aconfig_protos::aconfig::Permission as ProtoPermission;
+
+#[cfg(not(feature = "cargo"))]
+pub use aconfig_protos::aconfig::Flag_state as ProtoFlagState;
+
 // ---- When building with cargo ----
 #[cfg(feature = "cargo")]
 include!(concat!(env!("OUT_DIR"), "/aconfig_proto/mod.rs"));
@@ -61,6 +67,12 @@ pub use aconfig::Override_config as ProtoOverrideConfig;
 #[cfg(feature = "cargo")]
 pub use aconfig::Override as ProtoOverride;
 
+#[cfg(feature = "cargo")]
+pub use aconfig::Permission as ProtoPermission;
+
+#[cfg(feature = "cargo")]
+pub use aconfig::Flag_state as ProtoFlagState;
+
 // ---- Common for both the Android tool-chain and cargo ----
 use anyhow::Result;