From 2937566c559ace9daea2a6b54dc30cd722a81579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Fri, 5 May 2023 10:57:19 +0200 Subject: [PATCH 1/3] aconfig: Source: remove unnecessary #[derive(Serialize, Deserialize)] The Source struct is no longer serialized/deserialized. Remove the unused derives. Bug: 279485059 Test: atest aconfig.test Change-Id: Ifd78988ed8134ab43013314b4437e428a8927981 --- tools/aconfig/src/commands.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs index 76b853b405..4976fb2e98 100644 --- a/tools/aconfig/src/commands.rs +++ b/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, From 416330b060dd89d3748541df62d01682931edfc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Fri, 5 May 2023 11:10:01 +0200 Subject: [PATCH 2/3] aconfig: add read/write permission Introduce the concept of flag read/write permissions: a read-only flag can only have its value set during the build; a writable flag can by updated in runtime. Bug: 279485059 Test: atest aconfig.test Change-Id: I3ec5c9571faa54de5666120ccd60090d3db9e331 --- tools/aconfig/protos/aconfig.proto | 9 ++- tools/aconfig/src/aconfig.rs | 100 +++++++++++++++++++++-------- tools/aconfig/src/cache.rs | 53 +++++++++------ tools/aconfig/src/commands.rs | 2 + tools/aconfig/src/protos.rs | 6 ++ 5 files changed, 123 insertions(+), 47 deletions(-) diff --git a/tools/aconfig/protos/aconfig.proto b/tools/aconfig/protos/aconfig.proto index 65817ca682..d95fd5031a 100644 --- a/tools/aconfig/protos/aconfig.proto +++ b/tools/aconfig/protos/aconfig.proto @@ -20,9 +20,15 @@ syntax = "proto2"; package android.aconfig; +enum permission { + READ_ONLY = 1; + READ_WRITE = 2; +} + message value { required bool value = 1; - optional uint32 since = 2; + required permission permission = 2; + optional uint32 since = 3; } message flag { @@ -38,6 +44,7 @@ message android_config { message override { required string id = 1; required bool value = 2; + required permission permission = 3; }; message override_config { diff --git a/tools/aconfig/src/aconfig.rs b/tools/aconfig/src/aconfig.rs index 22fcb884b1..dc185f0d58 100644 --- a/tools/aconfig/src/aconfig.rs +++ b/tools/aconfig/src/aconfig.rs @@ -15,25 +15,46 @@ */ use anyhow::{anyhow, Context, Error, Result}; +use protobuf::{Enum, EnumOrUnknown}; +use serde::{Deserialize, Serialize}; use crate::protos::{ - ProtoAndroidConfig, ProtoFlag, ProtoOverride, ProtoOverrideConfig, ProtoValue, + ProtoAndroidConfig, ProtoFlag, ProtoOverride, ProtoOverrideConfig, ProtoPermission, ProtoValue, }; +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Copy)] +pub enum Permission { + ReadOnly, + ReadWrite, +} + +impl TryFrom> for Permission { + type Error = Error; + + fn try_from(proto: EnumOrUnknown) -> Result { + 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, + permission: Permission, since: Option, } #[allow(dead_code)] // only used in unit tests impl Value { - pub fn new(value: bool, since: u32) -> Value { - Value { value, since: Some(since) } + pub fn new(value: bool, permission: Permission, since: u32) -> Value { + Value { value, permission, since: Some(since) } } - pub fn default(value: bool) -> Value { - Value { value, since: None } + pub fn default(value: bool, permission: Permission) -> Value { + Value { value, permission, since: None } } } @@ -44,7 +65,11 @@ impl TryFrom for Value { let Some(value) = proto.value else { return Err(anyhow!("missing 'value' field")); }; - Ok(Value { value, since: proto.since }) + 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 }) } } @@ -72,15 +97,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) -> (bool, Permission) { let mut value = self.values[0].value; + 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; + permission = candidate.permission; } } - value + (value, permission) } } @@ -120,6 +147,7 @@ impl TryFrom for Flag { pub struct Override { pub id: String, pub value: bool, + pub permission: Permission, } impl Override { @@ -145,7 +173,11 @@ impl TryFrom for Override { let Some(value) = proto.value else { return Err(anyhow!("missing 'value' field")); }; - Ok(Override { id, value }) + let Some(proto_permission) = proto.permission else { + return Err(anyhow!("missing 'permission' field")); + }; + let permission = proto_permission.try_into()?; + Ok(Override { id, value, permission }) } } @@ -158,7 +190,10 @@ 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(false, Permission::ReadOnly), + Value::new(true, Permission::ReadWrite, 8), + ], }; let s = r#" @@ -166,9 +201,11 @@ mod tests { description: "Description of the flag" value { value: false + permission: READ_ONLY } value { value: true + permission: READ_WRITE since: 8 } "#; @@ -190,6 +227,7 @@ mod tests { description: "Description of the flag" value { value: true + permission: READ_ONLY } "#; let error = Flag::try_from_text_proto(s).unwrap_err(); @@ -200,9 +238,11 @@ mod tests { description: "Description of the flag" value { value: true + permission: READ_ONLY } value { value: true + permission: READ_ONLY } "#; let error = Flag::try_from_text_proto(s).unwrap_err(); @@ -215,12 +255,12 @@ mod tests { Flag { id: "a".to_owned(), description: "A".to_owned(), - values: vec![Value::default(true)], + values: vec![Value::default(true, Permission::ReadOnly)], }, Flag { id: "b".to_owned(), description: "B".to_owned(), - values: vec![Value::default(false)], + values: vec![Value::default(false, Permission::ReadWrite)], }, ]; @@ -230,6 +270,7 @@ mod tests { description: "A" value { value: true + permission: READ_ONLY } } flag { @@ -237,6 +278,7 @@ mod tests { description: "B" value { value: false + permission: READ_WRITE } } "#; @@ -247,11 +289,13 @@ 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(), value: true, permission: Permission::ReadOnly }; let s = r#" id: "1234" value: true + permission: READ_ONLY "#; let actual = Override::try_from_text_proto(s).unwrap(); @@ -259,26 +303,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::new(false, 10), - Value::new(true, 20), - Value::new(false, 30), + Value::default(false, Permission::ReadOnly), + Value::new(false, Permission::ReadWrite, 10), + Value::new(true, Permission::ReadOnly, 20), + Value::new(true, Permission::ReadWrite, 30), ], }; - assert!(flag.resolve_value(0)); - assert!(flag.resolve_value(9)); - assert!(!flag.resolve_value(10)); - assert!(!flag.resolve_value(11)); - assert!(!flag.resolve_value(19)); - assert!(flag.resolve_value(20)); - assert!(flag.resolve_value(21)); - assert!(flag.resolve_value(29)); - assert!(!flag.resolve_value(30)); - assert!(!flag.resolve_value(10_000)); + 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)); } } diff --git a/tools/aconfig/src/cache.rs b/tools/aconfig/src/cache.rs index d27459dcf5..f7b0f2d7cc 100644 --- a/tools/aconfig/src/cache.rs +++ b/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, 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 permission: Permission, pub debug: Vec, } -#[derive(Serialize, Deserialize)] +#[derive(Serialize, Deserialize, Debug)] pub struct Cache { build_id: u32, items: Vec, @@ -56,12 +57,13 @@ impl Cache { source, )); } - let value = flag.resolve_value(self.build_id); + let (value, permission) = flag.resolve(self.build_id); self.items.push(Item { id: flag.id.clone(), description: flag.description, value, - debug: vec![format!("{}:{}", source, value)], + permission, + debug: vec![format!("{}:{} {:?}", source, value, permission)], }); Ok(()) } @@ -71,7 +73,10 @@ impl Cache { return Err(anyhow!("failed to override flag {}: unknown flag", override_.id)); }; existing_item.value = override_.value; - existing_item.debug.push(format!("{}:{}", source, override_.value)); + existing_item.permission = override_.permission; + existing_item + .debug + .push(format!("{}:{} {:?}", source, override_.value, override_.permission)); Ok(()) } @@ -85,7 +90,7 @@ impl Item {} #[cfg(test)] mod tests { use super::*; - use crate::aconfig::Value; + use crate::aconfig::{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(true, 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(false, Permission::ReadOnly)], }, ) .unwrap_err(); @@ -118,13 +123,17 @@ mod tests { #[test] fn test_add_override() { - fn check_value(cache: &Cache, id: &str, expected: bool) -> bool { - cache.iter().find(|&item| item.id == id).unwrap().value == expected + fn check(cache: &Cache, id: &str, expected: (bool, Permission)) -> bool { + let item = cache.iter().find(|&item| item.id == id).unwrap(); + item.value == 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(), value: true, permission: Permission::ReadOnly }, + ) .unwrap_err(); assert_eq!(&format!("{:?}", error), "failed to override flag foo: unknown flag"); @@ -134,20 +143,28 @@ mod tests { Flag { id: "foo".to_string(), description: "desc".to_string(), - values: vec![Value::default(true)], + values: vec![Value::default(true, Permission::ReadOnly)], }, ) .unwrap(); - assert!(check_value(&cache, "foo", true)); + dbg!(&cache); + assert!(check(&cache, "foo", (true, Permission::ReadOnly))); cache - .add_override(Source::Memory, Override { id: "foo".to_string(), value: false }) + .add_override( + Source::Memory, + Override { id: "foo".to_string(), value: false, permission: Permission::ReadWrite }, + ) .unwrap(); - assert!(check_value(&cache, "foo", false)); + dbg!(&cache); + assert!(check(&cache, "foo", (false, Permission::ReadWrite))); cache - .add_override(Source::Memory, Override { id: "foo".to_string(), value: true }) + .add_override( + Source::Memory, + Override { id: "foo".to_string(), value: true, permission: Permission::ReadWrite }, + ) .unwrap(); - assert!(check_value(&cache, "foo", true)); + assert!(check(&cache, "foo", (true, Permission::ReadWrite))); } } diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs index 4976fb2e98..bc22363d3a 100644 --- a/tools/aconfig/src/commands.rs +++ b/tools/aconfig/src/commands.rs @@ -103,6 +103,7 @@ mod tests { description: "Description of a" value { value: true + permission: READ_ONLY } } "#; @@ -111,6 +112,7 @@ mod tests { override { id: "a" value: false + permission: READ_ONLY } "#; let overrides = vec![Input { source: Source::Memory, reader: Box::new(o.as_bytes()) }]; diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs index 3c156b3d90..e9a1ab5ff8 100644 --- a/tools/aconfig/src/protos.rs +++ b/tools/aconfig/src/protos.rs @@ -42,6 +42,9 @@ 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; + // ---- When building with cargo ---- #[cfg(feature = "cargo")] include!(concat!(env!("OUT_DIR"), "/aconfig_proto/mod.rs")); @@ -61,6 +64,9 @@ 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; + // ---- Common for both the Android tool-chain and cargo ---- use anyhow::Result; From c68c4eabea41cda1a7f4254385b5c2c6fcd8033c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Fri, 5 May 2023 16:20:09 +0200 Subject: [PATCH 3/3] 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 --- tools/aconfig/protos/aconfig.proto | 9 ++- tools/aconfig/src/aconfig.rs | 114 +++++++++++++++++------------ tools/aconfig/src/cache.rs | 50 ++++++++----- tools/aconfig/src/commands.rs | 16 ++-- tools/aconfig/src/protos.rs | 6 ++ 5 files changed, 122 insertions(+), 73 deletions(-) diff --git a/tools/aconfig/protos/aconfig.proto b/tools/aconfig/protos/aconfig.proto index d95fd5031a..6eac414b1e 100644 --- a/tools/aconfig/protos/aconfig.proto +++ b/tools/aconfig/protos/aconfig.proto @@ -20,13 +20,18 @@ 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; required permission permission = 2; optional uint32 since = 3; } @@ -43,7 +48,7 @@ message android_config { 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 dc185f0d58..f10ca1ff8b 100644 --- a/tools/aconfig/src/aconfig.rs +++ b/tools/aconfig/src/aconfig.rs @@ -19,9 +19,28 @@ use protobuf::{Enum, EnumOrUnknown}; 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> for FlagState { + type Error = Error; + + fn try_from(proto: EnumOrUnknown) -> Result { + 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, @@ -42,19 +61,19 @@ impl TryFrom> for Permission { #[derive(Debug, PartialEq, Eq)] pub struct Value { - value: bool, + state: FlagState, permission: Permission, since: Option, } #[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 @@ impl TryFrom for Value { type Error = Error; fn try_from(proto: ProtoValue) -> Result { - 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 @@ impl Flag { 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 @@ impl TryFrom for Flag { #[derive(Debug, PartialEq, Eq)] pub struct Override { pub id: String, - pub value: bool, + pub state: FlagState, pub permission: Permission, } @@ -170,14 +190,15 @@ impl TryFrom for Override { 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 @@ mod tests { 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 @@ mod tests { 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 @@ mod tests { let s = r#" description: "Description of the flag" value { - value: true + state: ENABLED permission: READ_ONLY } "#; @@ -237,11 +258,11 @@ 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 } "#; @@ -255,12 +276,12 @@ mod tests { 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 @@ mod tests { id: "a" description: "A" value { - value: true + state: ENABLED permission: READ_ONLY } } @@ -277,7 +298,7 @@ mod tests { id: "b" description: "B" value { - value: false + state: DISABLED permission: READ_WRITE } } @@ -289,12 +310,15 @@ mod tests { #[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 @@ mod tests { 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 f7b0f2d7cc..20c5de5dd5 100644 --- a/tools/aconfig/src/cache.rs +++ b/tools/aconfig/src/cache.rs @@ -18,14 +18,14 @@ use anyhow::{anyhow, Result}; 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, } @@ -57,13 +57,13 @@ impl Cache { 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 @@ 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.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 @@ impl Item {} #[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 @@ mod tests { 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 @@ mod tests { 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 @@ mod tests { #[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 @@ mod tests { 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 bc22363d3a..dc3e6bc995 100644 --- a/tools/aconfig/src/commands.rs +++ b/tools/aconfig/src/commands.rs @@ -79,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); } } } @@ -94,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() { @@ -102,8 +103,8 @@ mod tests { id: "a" description: "Description of a" value { - value: true - permission: READ_ONLY + state: ENABLED + permission: READ_WRITE } } "#; @@ -111,13 +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; - 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 e9a1ab5ff8..604eca4424 100644 --- a/tools/aconfig/src/protos.rs +++ b/tools/aconfig/src/protos.rs @@ -45,6 +45,9 @@ 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")); @@ -67,6 +70,9 @@ 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;