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

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

aconfig: sort items in cache by name 

Introduce a builder pattern for constructing a cache from flag
declarations and flag values. Teach the builder to sort the flags by
name as the last step. This will ensure consistent dump output
regardless of the order flags are specified in the input files.

Bug: 279485059
Test: atest aconfig.test
Change-Id: Icdd62f51fa3761a469663f17581a83d9909e9ffe
parent af677038

tools/aconfig/src/cache.rs

+113 −52
Original line number Diff line number Diff line
@@ -51,35 +51,83 @@ pub struct Cache { 
    items: Vec<Item>,

}



impl Cache {

    pub fn new(namespace: String) -> Result<Cache> {

        ensure!(!namespace.is_empty(), "empty namespace");

        Ok(Cache { namespace, items: vec![] })

// TODO: replace this function with Iterator.is_sorted_by_key(...)) when that API becomes stable

fn iter_is_sorted_by_key<'a, T: 'a, F, K>(iter: impl Iterator<Item = &'a T>, f: F) -> bool

where

    F: FnMut(&'a T) -> K,

    K: PartialOrd<K>,

{

    let mut last: Option<K> = None;

    for current in iter.map(f) {

        if let Some(l) = last {

            if l > current {

                return false;

            }

        }

        last = Some(current);

    }

    true

}



impl Cache {

    pub fn read_from_reader(reader: impl Read) -> Result<Cache> {

        serde_json::from_reader(reader).map_err(|e| e.into())

        let cache: Cache = serde_json::from_reader(reader)?;

        ensure!(

            iter_is_sorted_by_key(cache.iter(), |item| &item.name),

            "internal error: flags in cache file not sorted"

        );

        Ok(cache)

    }



    pub fn write_to_writer(&self, writer: impl Write) -> Result<()> {

        ensure!(

            iter_is_sorted_by_key(self.iter(), |item| &item.name),

            "internal error: flags in cache file not sorted"

        );

        serde_json::to_writer(writer, self).map_err(|e| e.into())

    }



    pub fn iter(&self) -> impl Iterator<Item = &Item> {

        self.items.iter()

    }



    pub fn into_iter(self) -> impl Iterator<Item = Item> {

        self.items.into_iter()

    }



    pub fn namespace(&self) -> &str {

        debug_assert!(!self.namespace.is_empty());

        &self.namespace

    }

}



#[derive(Debug)]

pub struct CacheBuilder {

    cache: Cache,

}



impl CacheBuilder {

    pub fn new(namespace: String) -> Result<CacheBuilder> {

        ensure!(!namespace.is_empty(), "empty namespace");

        let cache = Cache { namespace, items: vec![] };

        Ok(CacheBuilder { cache })

    }



    pub fn add_flag_declaration(

        &mut self,

        source: Source,

        declaration: FlagDeclaration,

    ) -> Result<()> {

    ) -> Result<&mut CacheBuilder> {

        ensure!(!declaration.name.is_empty(), "empty flag name");

        ensure!(!declaration.description.is_empty(), "empty flag description");

        ensure!(

            self.items.iter().all(|item| item.name != declaration.name),

            self.cache.items.iter().all(|item| item.name != declaration.name),

            "failed to declare flag {} from {}: flag already declared",

            declaration.name,

            source

        );

        self.items.push(Item {

            namespace: self.namespace.clone(),

        self.cache.items.push(Item {

            namespace: self.cache.namespace.clone(),

            name: declaration.name.clone(),

            description: declaration.description,

            state: DEFAULT_FLAG_STATE,
@@ -90,21 +138,25 @@ impl Cache { 
                permission: DEFAULT_FLAG_PERMISSION,

            }],

        });

        Ok(())

        Ok(self)

    }



    pub fn add_flag_value(&mut self, source: Source, value: FlagValue) -> Result<()> {

    pub fn add_flag_value(

        &mut self,

        source: Source,

        value: FlagValue,

    ) -> Result<&mut CacheBuilder> {

        ensure!(!value.namespace.is_empty(), "empty flag namespace");

        ensure!(!value.name.is_empty(), "empty flag name");

        ensure!(

            value.namespace == self.namespace,

            value.namespace == self.cache.namespace,

            "failed to set values for flag {}/{} from {}: expected namespace {}",

            value.namespace,

            value.name,

            source,

            self.namespace

            self.cache.namespace

        );

        let Some(existing_item) = self.items.iter_mut().find(|item| item.name == value.name) else {

        let Some(existing_item) = self.cache.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 = value.state;
@@ -114,20 +166,12 @@ impl Cache { 
            state: value.state,

            permission: value.permission,

        });

        Ok(())

    }



    pub fn iter(&self) -> impl Iterator<Item = &Item> {

        self.items.iter()

        Ok(self)

    }



    pub fn into_iter(self) -> impl Iterator<Item = Item> {

        self.items.into_iter()

    }



    pub fn namespace(&self) -> &str {

        debug_assert!(!self.namespace.is_empty());

        &self.namespace

    pub fn build(mut self) -> Cache {

        self.cache.items.sort_by_cached_key(|item| item.name.clone());

        self.cache

    }

}
@@ -138,14 +182,14 @@ mod tests { 


    #[test]

    fn test_add_flag_declaration() {

        let mut cache = Cache::new("ns".to_string()).unwrap();

        cache

        let mut builder = CacheBuilder::new("ns".to_string()).unwrap();

        builder

            .add_flag_declaration(

                Source::File("first.txt".to_string()),

                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },

            )

            .unwrap();

        let error = cache

        let error = builder

            .add_flag_declaration(

                Source::File("second.txt".to_string()),

                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
@@ -155,17 +199,26 @@ mod tests { 
            &format!("{:?}", error),

            "failed to declare flag foo from second.txt: flag already declared"

        );

        builder

            .add_flag_declaration(

                Source::File("first.txt".to_string()),

                FlagDeclaration { name: "bar".to_string(), description: "desc".to_string() },

            )

            .unwrap();



        let cache = builder.build();



        // check flags are sorted by name

        assert_eq!(

            cache.into_iter().map(|item| item.name).collect::<Vec<_>>(),

            vec!["bar".to_string(), "foo".to_string()]

        );

    }



    #[test]

    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("ns".to_string()).unwrap();

        let error = cache

        let mut builder = CacheBuilder::new("ns".to_string()).unwrap();

        let error = builder

            .add_flag_value(

                Source::Memory,

                FlagValue {
@@ -181,15 +234,14 @@ mod tests { 
            "failed to set values for flag ns/foo from <memory>: flag not declared"

        );



        cache

        builder

            .add_flag_declaration(

                Source::File("first.txt".to_string()),

                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },

            )

            .unwrap();

        assert!(check(&cache, "foo", (DEFAULT_FLAG_STATE, DEFAULT_FLAG_PERMISSION)));



        cache

        builder

            .add_flag_value(

                Source::Memory,

                FlagValue {
@@ -200,9 +252,8 @@ mod tests { 
                },

            )

            .unwrap();

        assert!(check(&cache, "foo", (FlagState::Disabled, Permission::ReadOnly)));



        cache

        builder

            .add_flag_value(

                Source::Memory,

                FlagValue {
@@ -213,10 +264,9 @@ mod tests { 
                },

            )

            .unwrap();

        assert!(check(&cache, "foo", (FlagState::Enabled, Permission::ReadWrite)));



        // different namespace -> no-op

        let error = cache

        let error = builder

            .add_flag_value(

                Source::Memory,

                FlagValue {
@@ -228,19 +278,23 @@ mod tests { 
            )

            .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)));



        let cache = builder.build();

        let item = cache.iter().find(|&item| item.name == "foo").unwrap();

        assert_eq!(FlagState::Enabled, item.state);

        assert_eq!(Permission::ReadWrite, item.permission);

    }



    #[test]

    fn test_reject_empty_cache_namespace() {

        Cache::new("".to_string()).unwrap_err();

        CacheBuilder::new("".to_string()).unwrap_err();

    }



    #[test]

    fn test_reject_empty_flag_declaration_fields() {

        let mut cache = Cache::new("ns".to_string()).unwrap();

        let mut builder = CacheBuilder::new("ns".to_string()).unwrap();



        let error = cache

        let error = builder

            .add_flag_declaration(

                Source::Memory,

                FlagDeclaration { name: "".to_string(), description: "Description".to_string() },
@@ -248,7 +302,7 @@ mod tests { 
            .unwrap_err();

        assert_eq!(&format!("{:?}", error), "empty flag name");



        let error = cache

        let error = builder

            .add_flag_declaration(

                Source::Memory,

                FlagDeclaration { name: "foo".to_string(), description: "".to_string() },
@@ -259,15 +313,15 @@ mod tests { 


    #[test]

    fn test_reject_empty_flag_value_files() {

        let mut cache = Cache::new("ns".to_string()).unwrap();

        cache

        let mut builder = CacheBuilder::new("ns".to_string()).unwrap();

        builder

            .add_flag_declaration(

                Source::Memory,

                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },

            )

            .unwrap();



        let error = cache

        let error = builder

            .add_flag_value(

                Source::Memory,

                FlagValue {
@@ -280,7 +334,7 @@ mod tests { 
            .unwrap_err();

        assert_eq!(&format!("{:?}", error), "empty flag namespace");



        let error = cache

        let error = builder

            .add_flag_value(

                Source::Memory,

                FlagValue {
@@ -293,4 +347,11 @@ mod tests { 
            .unwrap_err();

        assert_eq!(&format!("{:?}", error), "empty flag name");

    }



    #[test]

    fn test_iter_is_sorted_by_key() {

        assert!(iter_is_sorted_by_key(["a", "b", "c"].iter(), |s| s));

        assert!(iter_is_sorted_by_key(Vec::<&str>::new().iter(), |s| s));

        assert!(!iter_is_sorted_by_key(["a", "c", "b"].iter(), |s| s));

    }

}

tools/aconfig/src/codegen_cpp.rs

+12 −14
Original line number Diff line number Diff line
@@ -64,13 +64,14 @@ fn create_class_element(item: &Item) -> ClassElement { 
mod tests {

    use super::*;

    use crate::aconfig::{FlagDeclaration, FlagState, FlagValue, Permission};

    use crate::cache::CacheBuilder;

    use crate::commands::Source;



    #[test]

    fn test_cpp_codegen_build_time_flag_only() {

        let namespace = "my_namespace";

        let mut cache = Cache::new(namespace.to_string()).unwrap();

        cache

        let mut builder = CacheBuilder::new(namespace.to_string()).unwrap();

        builder

            .add_flag_declaration(

                Source::File("aconfig_one.txt".to_string()),

                FlagDeclaration {
@@ -78,8 +79,7 @@ mod tests { 
                    description: "buildtime disable".to_string(),

                },

            )

            .unwrap();

        cache

            .unwrap()

            .add_flag_value(

                Source::Memory,

                FlagValue {
@@ -89,8 +89,7 @@ mod tests { 
                    permission: Permission::ReadOnly,

                },

            )

            .unwrap();

        cache

            .unwrap()

            .add_flag_declaration(

                Source::File("aconfig_two.txt".to_string()),

                FlagDeclaration {
@@ -98,8 +97,7 @@ mod tests { 
                    description: "buildtime enable".to_string(),

                },

            )

            .unwrap();

        cache

            .unwrap()

            .add_flag_value(

                Source::Memory,

                FlagValue {
@@ -110,6 +108,7 @@ mod tests { 
                },

            )

            .unwrap();

        let cache = builder.build();

        let expect_content = r#"#ifndef my_namespace_HEADER_H

        #define my_namespace_HEADER_H

        #include "my_namespace.h"
@@ -144,8 +143,8 @@ mod tests { 
    #[test]

    fn test_cpp_codegen_runtime_flag() {

        let namespace = "my_namespace";

        let mut cache = Cache::new(namespace.to_string()).unwrap();

        cache

        let mut builder = CacheBuilder::new(namespace.to_string()).unwrap();

        builder

            .add_flag_declaration(

                Source::File("aconfig_one.txt".to_string()),

                FlagDeclaration {
@@ -153,8 +152,7 @@ mod tests { 
                    description: "buildtime disable".to_string(),

                },

            )

            .unwrap();

        cache

            .unwrap()

            .add_flag_declaration(

                Source::File("aconfig_two.txt".to_string()),

                FlagDeclaration {
@@ -162,8 +160,7 @@ mod tests { 
                    description: "runtime enable".to_string(),

                },

            )

            .unwrap();

        cache

            .unwrap()

            .add_flag_value(

                Source::Memory,

                FlagValue {
@@ -174,6 +171,7 @@ mod tests { 
                },

            )

            .unwrap();

        let cache = builder.build();

        let expect_content = r#"#ifndef my_namespace_HEADER_H

        #define my_namespace_HEADER_H

        #include "my_namespace.h"

tools/aconfig/src/codegen_java.rs

+6 −6
Original line number Diff line number Diff line
@@ -71,13 +71,14 @@ fn create_class_element(item: &Item) -> ClassElement { 
mod tests {

    use super::*;

    use crate::aconfig::{FlagDeclaration, FlagValue};

    use crate::cache::CacheBuilder;

    use crate::commands::Source;



    #[test]

    fn test_generate_java_code() {

        let namespace = "com.example";

        let mut cache = Cache::new(namespace.to_string()).unwrap();

        cache

        let mut builder = CacheBuilder::new(namespace.to_string()).unwrap();

        builder

            .add_flag_declaration(

                Source::File("test.txt".to_string()),

                FlagDeclaration {
@@ -85,8 +86,7 @@ mod tests { 
                    description: "buildtime enable".to_string(),

                },

            )

            .unwrap();

        cache

            .unwrap()

            .add_flag_declaration(

                Source::File("test2.txt".to_string()),

                FlagDeclaration {
@@ -94,8 +94,7 @@ mod tests { 
                    description: "runtime disable".to_string(),

                },

            )

            .unwrap();

        cache

            .unwrap()

            .add_flag_value(

                Source::Memory,

                FlagValue {
@@ -106,6 +105,7 @@ mod tests { 
                },

            )

            .unwrap();

        let cache = builder.build();

        let expect_content = r#"package com.example;



        import android.provider.DeviceConfig;

tools/aconfig/src/commands.rs

+5 −5
Original line number Diff line number Diff line
@@ -23,7 +23,7 @@ use std::io::Read; 
use std::path::PathBuf;



use crate::aconfig::{FlagDeclarations, FlagValue};

use crate::cache::Cache;

use crate::cache::{Cache, CacheBuilder};

use crate::codegen_cpp::generate_cpp_code;

use crate::codegen_java::generate_java_code;

use crate::protos::ProtoParsedFlags;
@@ -59,7 +59,7 @@ pub fn create_cache( 
    declarations: Vec<Input>,

    values: Vec<Input>,

) -> Result<Cache> {

    let mut cache = Cache::new(namespace.to_owned())?;

    let mut builder = CacheBuilder::new(namespace.to_owned())?;



    for mut input in declarations {

        let mut contents = String::new();
@@ -74,7 +74,7 @@ pub fn create_cache( 
            dec_list.namespace

        );

        for d in dec_list.flags.into_iter() {

            cache.add_flag_declaration(input.source.clone(), d)?;

            builder.add_flag_declaration(input.source.clone(), d)?;

        }

    }
@@ -85,11 +85,11 @@ pub fn create_cache( 
            .with_context(|| format!("Failed to parse {}", input.source))?;

        for v in values_list {

            // TODO: warn about flag values that do not take effect?

            let _ = cache.add_flag_value(input.source.clone(), v);

            let _ = builder.add_flag_value(input.source.clone(), v);

        }

    }



    Ok(cache)

    Ok(builder.build())

}



pub fn create_java_lib(cache: &Cache) -> Result<OutputFile> {