aflags: list flags as "enabled|disabled" instead of raw bool value
To keep things consistent and to reduce the risk of confusion, aconfig
flags are "enabled" or "disabled", not "true" or "false".
Do not represent flag states via a String. Instead, introduce a new
FlagValue enum and use this enum instead. FlagValue implements ToString
which returns "enabled" or "disabled".
Bug: N/A
Test: adb shell aflags list # verify that flags are "enabled" or "disabled"
Test: atest aflags.test
Change-Id: I9be998d7891656d118b7cc971449da58e17d5031
diff --git a/tools/aconfig/aflags/src/device_config_source.rs b/tools/aconfig/aflags/src/device_config_source.rs
index 1cea6ed..2589f3d 100644
--- a/tools/aconfig/aflags/src/device_config_source.rs
+++ b/tools/aconfig/aflags/src/device_config_source.rs
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-use crate::{Flag, FlagPermission, FlagSource, ValuePickedFrom};
+use crate::{Flag, FlagPermission, FlagSource, FlagValue, ValuePickedFrom};
use aconfig_protos::ProtoFlagPermission as ProtoPermission;
use aconfig_protos::ProtoFlagState as ProtoState;
use aconfig_protos::ProtoParsedFlag;
@@ -40,10 +40,9 @@
};
let value = match flag.state() {
- ProtoState::ENABLED => "true",
- ProtoState::DISABLED => "false",
- }
- .to_string();
+ ProtoState::ENABLED => FlagValue::Enabled,
+ ProtoState::DISABLED => FlagValue::Disabled,
+ };
let permission = match flag.permission() {
ProtoPermission::READ_ONLY => FlagPermission::ReadOnly,
@@ -86,14 +85,16 @@
Ok(flags.values().cloned().collect())
}
-fn parse_device_config(raw: &str) -> Result<HashMap<String, String>> {
+fn parse_device_config(raw: &str) -> Result<HashMap<String, FlagValue>> {
let mut flags = HashMap::new();
let regex = Regex::new(r"(?m)^([[[:alnum:]]_]+/[[[:alnum:]]_\.]+)=(true|false)$")?;
for capture in regex.captures_iter(raw) {
let key =
capture.get(1).ok_or(anyhow!("invalid device_config output"))?.as_str().to_string();
- let value = capture.get(2).ok_or(anyhow!("invalid device_config output"))?.as_str();
- flags.insert(key, value.to_string());
+ let value = FlagValue::try_from(
+ capture.get(2).ok_or(anyhow!("invalid device_config output"))?.as_str(),
+ )?;
+ flags.insert(key, value);
}
Ok(flags)
}
@@ -112,24 +113,24 @@
Ok(str::from_utf8(&output.stdout)?.to_string())
}
-fn read_device_config_flags() -> Result<HashMap<String, String>> {
+fn read_device_config_flags() -> Result<HashMap<String, FlagValue>> {
let list_output = read_device_config_output(&["list"])?;
parse_device_config(&list_output)
}
-fn reconcile(pb_flags: &[Flag], dc_flags: HashMap<String, String>) -> Vec<Flag> {
+fn reconcile(pb_flags: &[Flag], dc_flags: HashMap<String, FlagValue>) -> Vec<Flag> {
pb_flags
.iter()
.map(|f| {
dc_flags
.get(&format!("{}/{}.{}", f.namespace, f.package, f.name))
.map(|value| {
- if value.eq(&f.value) {
+ if *value == f.value {
Flag { value_picked_from: ValuePickedFrom::Default, ..f.clone() }
} else {
Flag {
value_picked_from: ValuePickedFrom::Server,
- value: value.to_string(),
+ value: *value,
..f.clone()
}
}
@@ -167,9 +168,9 @@
namespace_two/android.flag_two=nonsense
"#;
let expected = HashMap::from([
- ("namespace_one/com.foo.bar.flag_one".to_string(), "true".to_string()),
- ("namespace_one/com.foo.bar.flag_two".to_string(), "false".to_string()),
- ("namespace_two/android.flag_one".to_string(), "true".to_string()),
+ ("namespace_one/com.foo.bar.flag_one".to_string(), FlagValue::Enabled),
+ ("namespace_one/com.foo.bar.flag_two".to_string(), FlagValue::Disabled),
+ ("namespace_two/android.flag_one".to_string(), FlagValue::Enabled),
]);
let actual = parse_device_config(input).unwrap();
assert_eq!(expected, actual);
diff --git a/tools/aconfig/aflags/src/main.rs b/tools/aconfig/aflags/src/main.rs
index 7ca70a2..30275e3 100644
--- a/tools/aconfig/aflags/src/main.rs
+++ b/tools/aconfig/aflags/src/main.rs
@@ -52,13 +52,40 @@
}
}
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+enum FlagValue {
+ Enabled,
+ Disabled,
+}
+
+impl TryFrom<&str> for FlagValue {
+ type Error = anyhow::Error;
+
+ fn try_from(value: &str) -> std::result::Result<Self, Self::Error> {
+ match value {
+ "true" | "enabled" => Ok(Self::Enabled),
+ "false" | "disabled" => Ok(Self::Disabled),
+ _ => Err(anyhow!("cannot convert string '{}' to FlagValue", value)),
+ }
+ }
+}
+
+impl ToString for FlagValue {
+ fn to_string(&self) -> String {
+ match &self {
+ Self::Enabled => "enabled".into(),
+ Self::Disabled => "disabled".into(),
+ }
+ }
+}
+
#[derive(Clone)]
struct Flag {
namespace: String,
name: String,
package: String,
container: String,
- value: String,
+ value: FlagValue,
permission: FlagPermission,
value_picked_from: ValuePickedFrom,
}
@@ -126,7 +153,7 @@
let full_name = flag.qualified_name();
let p0 = info.longest_flag_col + 1;
- let val = &flag.value;
+ let val = flag.value.to_string();
let p1 = info.longest_val_col + 1;
let value_picked_from = flag.value_picked_from.to_string();
@@ -161,7 +188,7 @@
let flags = DeviceConfigSource::list_flags()?;
let padding_info = PaddingInfo {
longest_flag_col: flags.iter().map(|f| f.qualified_name().len()).max().unwrap_or(0),
- longest_val_col: flags.iter().map(|f| f.value.len()).max().unwrap_or(0),
+ longest_val_col: flags.iter().map(|f| f.value.to_string().len()).max().unwrap_or(0),
longest_value_picked_from_col: flags
.iter()
.map(|f| f.value_picked_from.to_string().len())