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

Commit 71f9dabe authored by Mårten Kongstad's avatar Mårten Kongstad Committed by Gerrit Code Review
Browse files

Merge changes from topic "aconfig-part-5"

* changes:
  aconfig: follow Java conventions for Java file paths
  aconfig: separate flag declarations and flag values
parents 8d5d2257 d42eeeba
Loading
Loading
Loading
Loading
+11 −18
Original line number Diff line number Diff line
@@ -12,8 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License

// This is the schema definition for of Aconfig files. Modifications need to be
// either backwards compatible, or include updates to all Aconfig files in the
// This is the schema definition for aconfig files. Modifications need to be
// either backwards compatible, or include updates to all aconfig files in the
// Android tree.

syntax = "proto2";
@@ -32,40 +32,33 @@ enum flag_permission {
  READ_WRITE = 2;
}

// aconfig input messages: configuration and override data
// aconfig input messages: flag declarations and values

message flag_value {
  required flag_state state = 1;
  required flag_permission permission = 2;
  optional uint32 since = 3;
}

message flag_definition {
message flag_declaration {
  required string name = 1;
  required string description = 2;
  repeated flag_value value = 3;
};

message namespace {
message flag_declarations {
  required string namespace = 1;
  repeated flag_definition flag = 2;
  repeated flag_declaration flag = 2;
};

message flag_override {
message flag_value {
  required string namespace = 1;
  required string name = 2;
  required flag_state state = 3;
  required flag_permission permission = 4;
};

message flag_overrides {
  repeated flag_override flag_override = 1;
message flag_values {
  repeated flag_value flag_value = 1;
};

// aconfig output messages: parsed and verified configuration and override data
// aconfig output messages: parsed and verified flag declarations and values

message tracepoint {
  // path to config or override file releative to $TOP
  // path to declaration or value file relative to $TOP
  required string source = 1;
  required flag_state state = 2;
  required flag_permission permission = 3;
+37 −180
Original line number Diff line number Diff line
@@ -20,9 +20,8 @@ use serde::{Deserialize, Serialize};

use crate::cache::{Cache, Item, Tracepoint};
use crate::protos::{
    ProtoFlagDefinition, ProtoFlagDefinitionValue, ProtoFlagOverride, ProtoFlagOverrides,
    ProtoFlagPermission, ProtoFlagState, ProtoNamespace, ProtoParsedFlag, ProtoParsedFlags,
    ProtoTracepoint,
    ProtoFlagDeclaration, ProtoFlagDeclarations, ProtoFlagPermission, ProtoFlagState,
    ProtoFlagValue, ProtoFlagValues, ProtoParsedFlag, ProtoParsedFlags, ProtoTracepoint,
};

#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Copy)]
@@ -80,112 +79,43 @@ impl From<Permission> for ProtoFlagPermission {
}

#[derive(Debug, PartialEq, Eq)]
pub struct Value {
    state: FlagState,
    permission: Permission,
    since: Option<u32>,
}

#[allow(dead_code)] // only used in unit tests
impl Value {
    pub fn new(state: FlagState, permission: Permission, since: u32) -> Value {
        Value { state, permission, since: Some(since) }
    }

    pub fn default(state: FlagState, permission: Permission) -> Value {
        Value { state, permission, since: None }
    }
}

impl TryFrom<ProtoFlagDefinitionValue> for Value {
    type Error = Error;

    fn try_from(proto: ProtoFlagDefinitionValue) -> Result<Self, Self::Error> {
        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 { state, permission, since: proto.since })
    }
}

#[derive(Debug, PartialEq, Eq)]
pub struct Flag {
pub struct FlagDeclaration {
    pub name: String,
    pub description: String,

    // ordered by Value.since; guaranteed to contain at least one item (the default value, with
    // since == None)
    pub values: Vec<Value>,
}

impl Flag {
impl FlagDeclaration {
    #[allow(dead_code)] // only used in unit tests
    pub fn try_from_text_proto(text_proto: &str) -> Result<Flag> {
        let proto: ProtoFlagDefinition = crate::protos::try_from_text_proto(text_proto)
    pub fn try_from_text_proto(text_proto: &str) -> Result<FlagDeclaration> {
        let proto: ProtoFlagDeclaration = crate::protos::try_from_text_proto(text_proto)
            .with_context(|| text_proto.to_owned())?;
        proto.try_into()
    }

    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 {
                state = candidate.state;
                permission = candidate.permission;
            }
        }
        (state, permission)
    }
}

impl TryFrom<ProtoFlagDefinition> for Flag {
impl TryFrom<ProtoFlagDeclaration> for FlagDeclaration {
    type Error = Error;

    fn try_from(proto: ProtoFlagDefinition) -> Result<Self, Self::Error> {
    fn try_from(proto: ProtoFlagDeclaration) -> Result<Self, Self::Error> {
        let Some(name) = proto.name else {
            return Err(anyhow!("missing 'name' field"));
        };
        let Some(description) = proto.description else {
            return Err(anyhow!("missing 'description' field"));
        };
        if proto.value.is_empty() {
            return Err(anyhow!("missing 'value' field"));
        }

        let mut values: Vec<Value> = vec![];
        for proto_value in proto.value.into_iter() {
            let v: Value = proto_value.try_into()?;
            if values.iter().any(|w| v.since == w.since) {
                let msg = match v.since {
                    None => format!("flag {}: multiple default values", name),
                    Some(x) => format!("flag {}: multiple values for since={}", name, x),
                };
                return Err(anyhow!(msg));
            }
            values.push(v);
        }
        values.sort_by_key(|v| v.since);

        Ok(Flag { name, description, values })
        Ok(FlagDeclaration { name, description })
    }
}

#[derive(Debug, PartialEq, Eq)]
pub struct Namespace {
pub struct FlagDeclarations {
    pub namespace: String,
    pub flags: Vec<Flag>,
    pub flags: Vec<FlagDeclaration>,
}

impl Namespace {
    pub fn try_from_text_proto(text_proto: &str) -> Result<Namespace> {
        let proto: ProtoNamespace = crate::protos::try_from_text_proto(text_proto)
impl FlagDeclarations {
    pub fn try_from_text_proto(text_proto: &str) -> Result<FlagDeclarations> {
        let proto: ProtoFlagDeclarations = crate::protos::try_from_text_proto(text_proto)
            .with_context(|| text_proto.to_owned())?;
        let Some(namespace) = proto.namespace else {
            return Err(anyhow!("missing 'namespace' field"));
@@ -194,35 +124,35 @@ impl Namespace {
        for proto_flag in proto.flag.into_iter() {
            flags.push(proto_flag.try_into()?);
        }
        Ok(Namespace { namespace, flags })
        Ok(FlagDeclarations { namespace, flags })
    }
}

#[derive(Debug, PartialEq, Eq)]
pub struct Override {
pub struct FlagValue {
    pub namespace: String,
    pub name: String,
    pub state: FlagState,
    pub permission: Permission,
}

impl Override {
impl FlagValue {
    #[allow(dead_code)] // only used in unit tests
    pub fn try_from_text_proto(text_proto: &str) -> Result<Override> {
        let proto: ProtoFlagOverride = crate::protos::try_from_text_proto(text_proto)?;
    pub fn try_from_text_proto(text_proto: &str) -> Result<FlagValue> {
        let proto: ProtoFlagValue = crate::protos::try_from_text_proto(text_proto)?;
        proto.try_into()
    }

    pub fn try_from_text_proto_list(text_proto: &str) -> Result<Vec<Override>> {
        let proto: ProtoFlagOverrides = crate::protos::try_from_text_proto(text_proto)?;
        proto.flag_override.into_iter().map(|proto_flag| proto_flag.try_into()).collect()
    pub fn try_from_text_proto_list(text_proto: &str) -> Result<Vec<FlagValue>> {
        let proto: ProtoFlagValues = crate::protos::try_from_text_proto(text_proto)?;
        proto.flag_value.into_iter().map(|proto_flag| proto_flag.try_into()).collect()
    }
}

impl TryFrom<ProtoFlagOverride> for Override {
impl TryFrom<ProtoFlagValue> for FlagValue {
    type Error = Error;

    fn try_from(proto: ProtoFlagOverride) -> Result<Self, Self::Error> {
    fn try_from(proto: ProtoFlagValue) -> Result<Self, Self::Error> {
        let Some(namespace) = proto.namespace else {
            return Err(anyhow!("missing 'namespace' field"));
        };
@@ -237,7 +167,7 @@ impl TryFrom<ProtoFlagOverride> for Override {
            return Err(anyhow!("missing 'permission' field"));
        };
        let permission = proto_permission.try_into()?;
        Ok(Override { namespace, name, state, permission })
        Ok(FlagValue { namespace, name, state, permission })
    }
}

@@ -282,29 +212,16 @@ mod tests {

    #[test]
    fn test_flag_try_from_text_proto() {
        let expected = Flag {
        let expected = FlagDeclaration {
            name: "1234".to_owned(),
            description: "Description of the flag".to_owned(),
            values: vec![
                Value::default(FlagState::Disabled, Permission::ReadOnly),
                Value::new(FlagState::Enabled, Permission::ReadWrite, 8),
            ],
        };

        let s = r#"
        name: "1234"
        description: "Description of the flag"
        value {
            state: DISABLED
            permission: READ_ONLY
        }
        value {
            state: ENABLED
            permission: READ_WRITE
            since: 8
        }
        "#;
        let actual = Flag::try_from_text_proto(s).unwrap();
        let actual = FlagDeclaration::try_from_text_proto(s).unwrap();

        assert_eq!(expected, actual);
    }
@@ -313,52 +230,24 @@ mod tests {
    fn test_flag_try_from_text_proto_bad_input() {
        let s = r#"
        name: "a"
        description: "Description of the flag"
        "#;
        let error = Flag::try_from_text_proto(s).unwrap_err();
        assert_eq!(format!("{:?}", error), "missing 'value' field");

        let s = r#"
        description: "Description of the flag"
        value {
            state: ENABLED
            permission: READ_ONLY
        }
        "#;
        let error = Flag::try_from_text_proto(s).unwrap_err();
        let error = FlagDeclaration::try_from_text_proto(s).unwrap_err();
        assert!(format!("{:?}", error).contains("Message not initialized"));

        let s = r#"
        name: "a"
        description: "Description of the flag"
        value {
            state: ENABLED
            permission: READ_ONLY
        }
        value {
            state: ENABLED
            permission: READ_ONLY
        }
        "#;
        let error = Flag::try_from_text_proto(s).unwrap_err();
        assert_eq!(format!("{:?}", error), "flag a: multiple default values");
        let error = FlagDeclaration::try_from_text_proto(s).unwrap_err();
        assert!(format!("{:?}", error).contains("Message not initialized"));
    }

    #[test]
    fn test_namespace_try_from_text_proto() {
        let expected = Namespace {
        let expected = FlagDeclarations {
            namespace: "ns".to_owned(),
            flags: vec![
                Flag {
                    name: "a".to_owned(),
                    description: "A".to_owned(),
                    values: vec![Value::default(FlagState::Enabled, Permission::ReadOnly)],
                },
                Flag {
                    name: "b".to_owned(),
                    description: "B".to_owned(),
                    values: vec![Value::default(FlagState::Disabled, Permission::ReadWrite)],
                },
                FlagDeclaration { name: "a".to_owned(), description: "A".to_owned() },
                FlagDeclaration { name: "b".to_owned(), description: "B".to_owned() },
            ],
        };

@@ -367,28 +256,20 @@ mod tests {
        flag {
            name: "a"
            description: "A"
            value {
                state: ENABLED
                permission: READ_ONLY
            }
        }
        flag {
            name: "b"
            description: "B"
            value {
                state: DISABLED
                permission: READ_WRITE
            }
        }
        "#;
        let actual = Namespace::try_from_text_proto(s).unwrap();
        let actual = FlagDeclarations::try_from_text_proto(s).unwrap();

        assert_eq!(expected, actual);
    }

    #[test]
    fn test_override_try_from_text_proto_list() {
        let expected = Override {
    fn test_flag_declaration_try_from_text_proto_list() {
        let expected = FlagValue {
            namespace: "ns".to_owned(),
            name: "1234".to_owned(),
            state: FlagState::Enabled,
@@ -401,32 +282,8 @@ mod tests {
        state: ENABLED
        permission: READ_ONLY
        "#;
        let actual = Override::try_from_text_proto(s).unwrap();
        let actual = FlagValue::try_from_text_proto(s).unwrap();

        assert_eq!(expected, actual);
    }

    #[test]
    fn test_flag_resolve() {
        let flag = Flag {
            name: "a".to_owned(),
            description: "A".to_owned(),
            values: vec![
                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!((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));
    }
}
+69 −66
Original line number Diff line number Diff line
@@ -14,13 +14,16 @@
 * limitations under the License.
 */

use anyhow::{anyhow, Result};
use anyhow::{anyhow, bail, ensure, Result};
use serde::{Deserialize, Serialize};
use std::io::{Read, Write};

use crate::aconfig::{Flag, FlagState, Override, Permission};
use crate::aconfig::{FlagDeclaration, FlagState, FlagValue, Permission};
use crate::commands::Source;

const DEFAULT_FLAG_STATE: FlagState = FlagState::Disabled;
const DEFAULT_FLAG_PERMISSION: Permission = Permission::ReadWrite;

#[derive(Serialize, Deserialize, Debug)]
pub struct Tracepoint {
    pub source: Source,
@@ -44,14 +47,13 @@ pub struct Item {

#[derive(Serialize, Deserialize, Debug)]
pub struct Cache {
    build_id: u32,
    namespace: String,
    items: Vec<Item>,
}

impl Cache {
    pub fn new(build_id: u32, namespace: String) -> Cache {
        Cache { build_id, namespace, items: vec![] }
    pub fn new(namespace: String) -> Cache {
        Cache { namespace, items: vec![] }
    }

    pub fn read_from_reader(reader: impl Read) -> Result<Cache> {
@@ -62,40 +64,51 @@ impl Cache {
        serde_json::to_writer(writer, self).map_err(|e| e.into())
    }

    pub fn add_flag(&mut self, source: Source, flag: Flag) -> Result<()> {
        if self.items.iter().any(|item| item.name == flag.name) {
    pub fn add_flag_declaration(
        &mut self,
        source: Source,
        declaration: FlagDeclaration,
    ) -> Result<()> {
        if self.items.iter().any(|item| item.name == declaration.name) {
            return Err(anyhow!(
                "failed to add flag {} from {}: flag already defined",
                flag.name,
                "failed to declare flag {} from {}: flag already declared",
                declaration.name,
                source,
            ));
        }
        let (state, permission) = flag.resolve(self.build_id);
        self.items.push(Item {
            namespace: self.namespace.clone(),
            name: flag.name.clone(),
            description: flag.description,
            state,
            permission,
            trace: vec![Tracepoint { source, state, permission }],
            name: declaration.name.clone(),
            description: declaration.description,
            state: DEFAULT_FLAG_STATE,
            permission: DEFAULT_FLAG_PERMISSION,
            trace: vec![Tracepoint {
                source,
                state: DEFAULT_FLAG_STATE,
                permission: DEFAULT_FLAG_PERMISSION,
            }],
        });
        Ok(())
    }

    pub fn add_override(&mut self, source: Source, override_: Override) -> Result<()> {
        if override_.namespace != self.namespace {
            // TODO: print warning?
            return Ok(());
        }
        let Some(existing_item) = self.items.iter_mut().find(|item| item.name == override_.name) else {
            return Err(anyhow!("failed to override flag {}: unknown flag", override_.name));
    pub fn add_flag_value(&mut self, source: Source, value: FlagValue) -> Result<()> {
        ensure!(
            value.namespace == self.namespace,
            "failed to set values for flag {}/{} from {}: expected namespace {}",
            value.namespace,
            value.name,
            source,
            self.namespace
        );
        let Some(existing_item) = self.items.iter_mut().find(|item| item.name == value.name) else {
            bail!("failed to set values for flag {}/{} from {}: flag not declared", value.namespace, value.name, source);
        };
        existing_item.state = override_.state;
        existing_item.permission = override_.permission;
        existing_item.state = value.state;
        existing_item.permission = value.permission;
        existing_item.trace.push(Tracepoint {
            source,
            state: override_.state,
            permission: override_.permission,
            state: value.state,
            permission: value.permission,
        });
        Ok(())
    }
@@ -112,49 +125,41 @@ impl Cache {
#[cfg(test)]
mod tests {
    use super::*;
    use crate::aconfig::{FlagState, Permission, Value};
    use crate::aconfig::{FlagState, Permission};

    #[test]
    fn test_add_flag() {
        let mut cache = Cache::new(1, "ns".to_string());
    fn test_add_flag_declaration() {
        let mut cache = Cache::new("ns".to_string());
        cache
            .add_flag(
            .add_flag_declaration(
                Source::File("first.txt".to_string()),
                Flag {
                    name: "foo".to_string(),
                    description: "desc".to_string(),
                    values: vec![Value::default(FlagState::Enabled, Permission::ReadOnly)],
                },
                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
            )
            .unwrap();
        let error = cache
            .add_flag(
            .add_flag_declaration(
                Source::File("second.txt".to_string()),
                Flag {
                    name: "foo".to_string(),
                    description: "desc".to_string(),
                    values: vec![Value::default(FlagState::Disabled, Permission::ReadOnly)],
                },
                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
            )
            .unwrap_err();
        assert_eq!(
            &format!("{:?}", error),
            "failed to add flag foo from second.txt: flag already defined"
            "failed to declare flag foo from second.txt: flag already declared"
        );
    }

    #[test]
    fn test_add_override() {
    fn test_add_flag_value() {
        fn check(cache: &Cache, name: &str, expected: (FlagState, Permission)) -> bool {
            let item = cache.iter().find(|&item| item.name == name).unwrap();
            item.state == expected.0 && item.permission == expected.1
        }

        let mut cache = Cache::new(1, "ns".to_string());
        let mut cache = Cache::new("ns".to_string());
        let error = cache
            .add_override(
            .add_flag_value(
                Source::Memory,
                Override {
                FlagValue {
                    namespace: "ns".to_string(),
                    name: "foo".to_string(),
                    state: FlagState::Enabled,
@@ -162,39 +167,36 @@ mod tests {
                },
            )
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "failed to override flag foo: unknown flag");
        assert_eq!(
            &format!("{:?}", error),
            "failed to set values for flag ns/foo from <memory>: flag not declared"
        );

        cache
            .add_flag(
            .add_flag_declaration(
                Source::File("first.txt".to_string()),
                Flag {
                    name: "foo".to_string(),
                    description: "desc".to_string(),
                    values: vec![Value::default(FlagState::Enabled, Permission::ReadOnly)],
                },
                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
            )
            .unwrap();
        dbg!(&cache);
        assert!(check(&cache, "foo", (FlagState::Enabled, Permission::ReadOnly)));
        assert!(check(&cache, "foo", (DEFAULT_FLAG_STATE, DEFAULT_FLAG_PERMISSION)));

        cache
            .add_override(
            .add_flag_value(
                Source::Memory,
                Override {
                FlagValue {
                    namespace: "ns".to_string(),
                    name: "foo".to_string(),
                    state: FlagState::Disabled,
                    permission: Permission::ReadWrite,
                    permission: Permission::ReadOnly,
                },
            )
            .unwrap();
        dbg!(&cache);
        assert!(check(&cache, "foo", (FlagState::Disabled, Permission::ReadWrite)));
        assert!(check(&cache, "foo", (FlagState::Disabled, Permission::ReadOnly)));

        cache
            .add_override(
            .add_flag_value(
                Source::Memory,
                Override {
                FlagValue {
                    namespace: "ns".to_string(),
                    name: "foo".to_string(),
                    state: FlagState::Enabled,
@@ -205,17 +207,18 @@ mod tests {
        assert!(check(&cache, "foo", (FlagState::Enabled, Permission::ReadWrite)));

        // different namespace -> no-op
        cache
            .add_override(
        let error = cache
            .add_flag_value(
                Source::Memory,
                Override {
                FlagValue {
                    namespace: "some-other-namespace".to_string(),
                    name: "foo".to_string(),
                    state: FlagState::Enabled,
                    permission: Permission::ReadOnly,
                },
            )
            .unwrap();
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "failed to set values for flag some-other-namespace/foo from <memory>: expected namespace ns");
        assert!(check(&cache, "foo", (FlagState::Enabled, Permission::ReadWrite)));
    }
}
+33 −25

File changed.

Preview size limit exceeded, changes collapsed.

+31 −33

File changed.

Preview size limit exceeded, changes collapsed.

Loading