aconfig: change flag values to enabled/disabled enum
Change the underlying type of a flag's value from bool to an explicit
enum (Disabled, Enabled): this will hopefully reduce future confusion on
how flags are intended to be used.
Bug: 279485059
Test: atest aconfig.test
Change-Id: I9535f9b23baf93ad5916ca06fb7d21277b4573eb
diff --git a/tools/aconfig/protos/aconfig.proto b/tools/aconfig/protos/aconfig.proto
index d95fd50..6eac414 100644
--- a/tools/aconfig/protos/aconfig.proto
+++ b/tools/aconfig/protos/aconfig.proto
@@ -20,13 +20,18 @@
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;
required permission permission = 2;
optional uint32 since = 3;
}
@@ -43,7 +48,7 @@
message override {
required string id = 1;
- required bool value = 2;
+ required flag_state state = 2;
required permission permission = 3;
};
diff --git a/tools/aconfig/src/aconfig.rs b/tools/aconfig/src/aconfig.rs
index dc185f0..f10ca1f 100644
--- a/tools/aconfig/src/aconfig.rs
+++ b/tools/aconfig/src/aconfig.rs
@@ -19,10 +19,29 @@
use serde::{Deserialize, Serialize};
use crate::protos::{
- ProtoAndroidConfig, ProtoFlag, ProtoOverride, ProtoOverrideConfig, ProtoPermission, 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,
@@ -42,19 +61,19 @@
#[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, permission: Permission, since: u32) -> Value {
- Value { value, permission, since: Some(since) }
+ pub fn new(state: FlagState, permission: Permission, since: u32) -> Value {
+ Value { state, permission, since: Some(since) }
}
- pub fn default(value: bool, permission: Permission) -> Value {
- Value { value, permission, since: None }
+ pub fn default(state: FlagState, permission: Permission) -> Value {
+ Value { state, permission, since: None }
}
}
@@ -62,14 +81,15 @@
type Error = Error;
fn try_from(proto: ProtoValue) -> Result<Self, Self::Error> {
- let Some(value) = proto.value else {
- return Err(anyhow!("missing 'value' field"));
+ let Some(proto_state) = proto.state else {
+ return Err(anyhow!("missing 'state' field"));
};
+ 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 { value, permission, since: proto.since })
+ Ok(Value { state, permission, since: proto.since })
}
}
@@ -97,17 +117,17 @@
proto.flag.into_iter().map(|proto_flag| proto_flag.try_into()).collect()
}
- pub fn resolve(&self, build_id: u32) -> (bool, Permission) {
- let mut value = self.values[0].value;
+ pub fn resolve(&self, build_id: u32) -> (FlagState, Permission) {
+ 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, permission)
+ (state, permission)
}
}
@@ -146,7 +166,7 @@
#[derive(Debug, PartialEq, Eq)]
pub struct Override {
pub id: String,
- pub value: bool,
+ pub state: FlagState,
pub permission: Permission,
}
@@ -170,14 +190,15 @@
let Some(id) = proto.id else {
return Err(anyhow!("missing 'id' field"));
};
- let Some(value) = proto.value else {
- return Err(anyhow!("missing 'value' field"));
+ let Some(proto_state) = proto.state else {
+ return Err(anyhow!("missing 'state' field"));
};
+ 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, value, permission })
+ Ok(Override { id, state, permission })
}
}
@@ -191,8 +212,8 @@
id: "1234".to_owned(),
description: "Description of the flag".to_owned(),
values: vec![
- Value::default(false, Permission::ReadOnly),
- Value::new(true, Permission::ReadWrite, 8),
+ Value::default(FlagState::Disabled, Permission::ReadOnly),
+ Value::new(FlagState::Enabled, Permission::ReadWrite, 8),
],
};
@@ -200,11 +221,11 @@
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
}
@@ -226,7 +247,7 @@
let s = r#"
description: "Description of the flag"
value {
- value: true
+ state: ENABLED
permission: READ_ONLY
}
"#;
@@ -237,11 +258,11 @@
id: "a"
description: "Description of the flag"
value {
- value: true
+ state: ENABLED
permission: READ_ONLY
}
value {
- value: true
+ state: ENABLED
permission: READ_ONLY
}
"#;
@@ -255,12 +276,12 @@
Flag {
id: "a".to_owned(),
description: "A".to_owned(),
- values: vec![Value::default(true, Permission::ReadOnly)],
+ values: vec![Value::default(FlagState::Enabled, Permission::ReadOnly)],
},
Flag {
id: "b".to_owned(),
description: "B".to_owned(),
- values: vec![Value::default(false, Permission::ReadWrite)],
+ values: vec![Value::default(FlagState::Disabled, Permission::ReadWrite)],
},
];
@@ -269,7 +290,7 @@
id: "a"
description: "A"
value {
- value: true
+ state: ENABLED
permission: READ_ONLY
}
}
@@ -277,7 +298,7 @@
id: "b"
description: "B"
value {
- value: false
+ state: DISABLED
permission: READ_WRITE
}
}
@@ -289,12 +310,15 @@
#[test]
fn test_override_try_from_text_proto_list() {
- let expected =
- Override { id: "1234".to_owned(), value: true, permission: Permission::ReadOnly };
+ 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();
@@ -308,21 +332,21 @@
id: "a".to_owned(),
description: "A".to_owned(),
values: vec![
- Value::default(false, Permission::ReadOnly),
- Value::new(false, Permission::ReadWrite, 10),
- Value::new(true, Permission::ReadOnly, 20),
- Value::new(true, Permission::ReadWrite, 30),
+ Value::default(FlagState::Disabled, Permission::ReadOnly),
+ Value::new(FlagState::Disabled, Permission::ReadWrite, 10),
+ Value::new(FlagState::Enabled, Permission::ReadOnly, 20),
+ Value::new(FlagState::Enabled, Permission::ReadWrite, 30),
],
};
- assert_eq!((false, Permission::ReadOnly), flag.resolve(0));
- assert_eq!((false, Permission::ReadOnly), flag.resolve(9));
- assert_eq!((false, Permission::ReadWrite), flag.resolve(10));
- assert_eq!((false, Permission::ReadWrite), flag.resolve(11));
- assert_eq!((false, Permission::ReadWrite), flag.resolve(19));
- assert_eq!((true, Permission::ReadOnly), flag.resolve(20));
- assert_eq!((true, Permission::ReadOnly), flag.resolve(21));
- assert_eq!((true, Permission::ReadOnly), flag.resolve(29));
- assert_eq!((true, Permission::ReadWrite), flag.resolve(30));
- assert_eq!((true, Permission::ReadWrite), flag.resolve(10_000));
+ assert_eq!((FlagState::Disabled, Permission::ReadOnly), flag.resolve(0));
+ assert_eq!((FlagState::Disabled, Permission::ReadOnly), flag.resolve(9));
+ assert_eq!((FlagState::Disabled, Permission::ReadWrite), flag.resolve(10));
+ assert_eq!((FlagState::Disabled, Permission::ReadWrite), flag.resolve(11));
+ assert_eq!((FlagState::Disabled, Permission::ReadWrite), flag.resolve(19));
+ assert_eq!((FlagState::Enabled, Permission::ReadOnly), flag.resolve(20));
+ assert_eq!((FlagState::Enabled, Permission::ReadOnly), flag.resolve(21));
+ assert_eq!((FlagState::Enabled, Permission::ReadOnly), flag.resolve(29));
+ assert_eq!((FlagState::Enabled, Permission::ReadWrite), flag.resolve(30));
+ assert_eq!((FlagState::Enabled, Permission::ReadWrite), flag.resolve(10_000));
}
}
diff --git a/tools/aconfig/src/cache.rs b/tools/aconfig/src/cache.rs
index f7b0f2d..20c5de5 100644
--- a/tools/aconfig/src/cache.rs
+++ b/tools/aconfig/src/cache.rs
@@ -18,14 +18,14 @@
use serde::{Deserialize, Serialize};
use std::io::{Read, Write};
-use crate::aconfig::{Flag, Override, Permission};
+use crate::aconfig::{Flag, FlagState, Override, Permission};
use crate::commands::Source;
#[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>,
}
@@ -57,13 +57,13 @@
source,
));
}
- let (value, permission) = flag.resolve(self.build_id);
+ let (state, permission) = flag.resolve(self.build_id);
self.items.push(Item {
id: flag.id.clone(),
description: flag.description,
- value,
+ state,
permission,
- debug: vec![format!("{}:{} {:?}", source, value, permission)],
+ debug: vec![format!("{}:{:?} {:?}", source, state, permission)],
});
Ok(())
}
@@ -72,11 +72,11 @@
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.permission = override_.permission;
existing_item
.debug
- .push(format!("{}:{} {:?}", source, override_.value, override_.permission));
+ .push(format!("{}:{:?} {:?}", source, override_.state, override_.permission));
Ok(())
}
@@ -90,7 +90,7 @@
#[cfg(test)]
mod tests {
use super::*;
- use crate::aconfig::{Permission, Value};
+ use crate::aconfig::{FlagState, Permission, Value};
#[test]
fn test_add_flag() {
@@ -101,7 +101,7 @@
Flag {
id: "foo".to_string(),
description: "desc".to_string(),
- values: vec![Value::default(true, Permission::ReadOnly)],
+ values: vec![Value::default(FlagState::Enabled, Permission::ReadOnly)],
},
)
.unwrap();
@@ -111,7 +111,7 @@
Flag {
id: "foo".to_string(),
description: "desc".to_string(),
- values: vec![Value::default(false, Permission::ReadOnly)],
+ values: vec![Value::default(FlagState::Disabled, Permission::ReadOnly)],
},
)
.unwrap_err();
@@ -123,16 +123,20 @@
#[test]
fn test_add_override() {
- fn check(cache: &Cache, id: &str, expected: (bool, Permission)) -> bool {
+ fn check(cache: &Cache, id: &str, expected: (FlagState, Permission)) -> bool {
let item = cache.iter().find(|&item| item.id == id).unwrap();
- item.value == expected.0 && item.permission == expected.1
+ 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, permission: Permission::ReadOnly },
+ Override {
+ id: "foo".to_string(),
+ state: FlagState::Enabled,
+ permission: Permission::ReadOnly,
+ },
)
.unwrap_err();
assert_eq!(&format!("{:?}", error), "failed to override flag foo: unknown flag");
@@ -143,28 +147,36 @@
Flag {
id: "foo".to_string(),
description: "desc".to_string(),
- values: vec![Value::default(true, Permission::ReadOnly)],
+ values: vec![Value::default(FlagState::Enabled, Permission::ReadOnly)],
},
)
.unwrap();
dbg!(&cache);
- assert!(check(&cache, "foo", (true, Permission::ReadOnly)));
+ assert!(check(&cache, "foo", (FlagState::Enabled, Permission::ReadOnly)));
cache
.add_override(
Source::Memory,
- Override { id: "foo".to_string(), value: false, permission: Permission::ReadWrite },
+ Override {
+ id: "foo".to_string(),
+ state: FlagState::Disabled,
+ permission: Permission::ReadWrite,
+ },
)
.unwrap();
dbg!(&cache);
- assert!(check(&cache, "foo", (false, Permission::ReadWrite)));
+ assert!(check(&cache, "foo", (FlagState::Disabled, Permission::ReadWrite)));
cache
.add_override(
Source::Memory,
- Override { id: "foo".to_string(), value: true, permission: Permission::ReadWrite },
+ Override {
+ id: "foo".to_string(),
+ state: FlagState::Enabled,
+ permission: Permission::ReadWrite,
+ },
)
.unwrap();
- assert!(check(&cache, "foo", (true, Permission::ReadWrite)));
+ assert!(check(&cache, "foo", (FlagState::Enabled, Permission::ReadWrite)));
}
}
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs
index bc22363..dc3e6bc 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -79,12 +79,12 @@
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);
}
}
}
@@ -94,6 +94,7 @@
#[cfg(test)]
mod tests {
use super::*;
+ use crate::aconfig::{FlagState, Permission};
#[test]
fn test_create_cache() {
@@ -102,8 +103,8 @@
id: "a"
description: "Description of a"
value {
- value: true
- permission: READ_ONLY
+ state: ENABLED
+ permission: READ_WRITE
}
}
"#;
@@ -111,13 +112,14 @@
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;
- assert!(!value);
+ let item = cache.iter().find(|&item| item.id == "a").unwrap();
+ assert_eq!(FlagState::Disabled, item.state);
+ assert_eq!(Permission::ReadOnly, item.permission);
}
}
diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs
index e9a1ab5..604eca4 100644
--- a/tools/aconfig/src/protos.rs
+++ b/tools/aconfig/src/protos.rs
@@ -45,6 +45,9 @@
#[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"));
@@ -67,6 +70,9 @@
#[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;