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

Commit c68c4eab authored by Mårten Kongstad's avatar Mårten Kongstad
Browse files

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
parent 416330b0
Loading
Loading
Loading
Loading
+7 −2
Original line number Diff line number Diff line
@@ -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;
};

+69 −45
Original line number Diff line number Diff line
@@ -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<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,
@@ -42,19 +61,19 @@ impl TryFrom<EnumOrUnknown<ProtoPermission>> for Permission {

#[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 @@ impl TryFrom<ProtoValue> for Value {
    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 @@ 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<ProtoFlag> 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<ProtoOverride> 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));
    }
}
+31 −19
Original line number Diff line number Diff line
@@ -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<String>,
}
@@ -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)));
    }
}
+9 −7
Original line number Diff line number Diff line
@@ -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);
    }
}
+6 −0
Original line number Diff line number Diff line
@@ -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;