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

Commit d4a32797 authored by Treehugger Robot's avatar Treehugger Robot Committed by Gerrit Code Review
Browse files

Merge changes from topic "aconfig-cleanup-1"

* changes:
  aconfig: cache: reject empty namespace and name fields
  aconfig: remove calls to unwrap (outside tests)
  aconfig: rename enum Format -> enum DumpFormat
parents 7f1171e4 e17ba5f4
Loading
Loading
Loading
Loading
+77 −4
Original line number Diff line number Diff line
@@ -52,8 +52,9 @@ pub struct Cache {
}

impl Cache {
    pub fn new(namespace: String) -> Cache {
        Cache { namespace, items: vec![] }
    pub fn new(namespace: String) -> Result<Cache> {
        ensure!(!namespace.is_empty(), "empty namespace");
        Ok(Cache { namespace, items: vec![] })
    }

    pub fn read_from_reader(reader: impl Read) -> Result<Cache> {
@@ -69,6 +70,8 @@ impl Cache {
        source: Source,
        declaration: FlagDeclaration,
    ) -> Result<()> {
        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),
            "failed to declare flag {} from {}: flag already declared",
@@ -91,6 +94,8 @@ impl Cache {
    }

    pub fn add_flag_value(&mut self, source: Source, value: FlagValue) -> Result<()> {
        ensure!(!value.namespace.is_empty(), "empty flag namespace");
        ensure!(!value.name.is_empty(), "empty flag name");
        ensure!(
            value.namespace == self.namespace,
            "failed to set values for flag {}/{} from {}: expected namespace {}",
@@ -119,6 +124,11 @@ impl Cache {
    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
    }
}

#[cfg(test)]
@@ -128,7 +138,7 @@ mod tests {

    #[test]
    fn test_add_flag_declaration() {
        let mut cache = Cache::new("ns".to_string());
        let mut cache = Cache::new("ns".to_string()).unwrap();
        cache
            .add_flag_declaration(
                Source::File("first.txt".to_string()),
@@ -154,7 +164,7 @@ mod tests {
            item.state == expected.0 && item.permission == expected.1
        }

        let mut cache = Cache::new("ns".to_string());
        let mut cache = Cache::new("ns".to_string()).unwrap();
        let error = cache
            .add_flag_value(
                Source::Memory,
@@ -220,4 +230,67 @@ mod tests {
        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)));
    }

    #[test]
    fn test_reject_empty_cache_namespace() {
        Cache::new("".to_string()).unwrap_err();
    }

    #[test]
    fn test_reject_empty_flag_declaration_fields() {
        let mut cache = Cache::new("ns".to_string()).unwrap();

        let error = cache
            .add_flag_declaration(
                Source::Memory,
                FlagDeclaration { name: "".to_string(), description: "Description".to_string() },
            )
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "empty flag name");

        let error = cache
            .add_flag_declaration(
                Source::Memory,
                FlagDeclaration { name: "foo".to_string(), description: "".to_string() },
            )
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "empty flag description");
    }

    #[test]
    fn test_reject_empty_flag_value_files() {
        let mut cache = Cache::new("ns".to_string()).unwrap();
        cache
            .add_flag_declaration(
                Source::Memory,
                FlagDeclaration { name: "foo".to_string(), description: "desc".to_string() },
            )
            .unwrap();

        let error = cache
            .add_flag_value(
                Source::Memory,
                FlagValue {
                    namespace: "".to_string(),
                    name: "foo".to_string(),
                    state: FlagState::Enabled,
                    permission: Permission::ReadOnly,
                },
            )
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "empty flag namespace");

        let error = cache
            .add_flag_value(
                Source::Memory,
                FlagValue {
                    namespace: "ns".to_string(),
                    name: "".to_string(),
                    state: FlagState::Enabled,
                    permission: Permission::ReadOnly,
                },
            )
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "empty flag name");
    }
}
+2 −4
Original line number Diff line number Diff line
@@ -25,9 +25,7 @@ use crate::commands::OutputFile;
pub fn generate_java_code(cache: &Cache) -> Result<OutputFile> {
    let class_elements: Vec<ClassElement> = cache.iter().map(create_class_element).collect();
    let readwrite = class_elements.iter().any(|item| item.readwrite);
    let namespace = uppercase_first_letter(
        cache.iter().find(|item| !item.namespace.is_empty()).unwrap().namespace.as_str(),
    );
    let namespace = uppercase_first_letter(cache.namespace());
    let context = Context { namespace: namespace.clone(), readwrite, class_elements };
    let mut template = TinyTemplate::new();
    template.add_template("java_code_gen", include_str!("../templates/java.template"))?;
@@ -90,7 +88,7 @@ mod tests {
    #[test]
    fn test_generate_java_code() {
        let namespace = "TeSTFlaG";
        let mut cache = Cache::new(namespace.to_string());
        let mut cache = Cache::new(namespace.to_string()).unwrap();
        cache
            .add_flag_declaration(
                Source::File("test.txt".to_string()),
+8 −8
Original line number Diff line number Diff line
@@ -58,7 +58,7 @@ pub fn create_cache(
    declarations: Vec<Input>,
    values: Vec<Input>,
) -> Result<Cache> {
    let mut cache = Cache::new(namespace.to_owned());
    let mut cache = Cache::new(namespace.to_owned())?;

    for mut input in declarations {
        let mut contents = String::new();
@@ -96,29 +96,29 @@ pub fn generate_code(cache: &Cache) -> Result<OutputFile> {
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, ValueEnum)]
pub enum Format {
pub enum DumpFormat {
    Text,
    Debug,
    Protobuf,
}

pub fn dump_cache(cache: Cache, format: Format) -> Result<Vec<u8>> {
pub fn dump_cache(cache: Cache, format: DumpFormat) -> Result<Vec<u8>> {
    match format {
        Format::Text => {
        DumpFormat::Text => {
            let mut lines = vec![];
            for item in cache.iter() {
                lines.push(format!("{}: {:?}\n", item.name, item.state));
            }
            Ok(lines.concat().into())
        }
        Format::Debug => {
        DumpFormat::Debug => {
            let mut lines = vec![];
            for item in cache.iter() {
                lines.push(format!("{:?}\n", item));
            }
            Ok(lines.concat().into())
        }
        Format::Protobuf => {
        DumpFormat::Protobuf => {
            let parsed_flags: ProtoParsedFlags = cache.into();
            let mut output = vec![];
            parsed_flags.write_to_vec(&mut output)?;
@@ -168,7 +168,7 @@ mod tests {
    #[test]
    fn test_dump_text_format() {
        let cache = create_test_cache();
        let bytes = dump_cache(cache, Format::Text).unwrap();
        let bytes = dump_cache(cache, DumpFormat::Text).unwrap();
        let text = std::str::from_utf8(&bytes).unwrap();
        assert!(text.contains("a: Disabled"));
    }
@@ -179,7 +179,7 @@ mod tests {
        use protobuf::Message;

        let cache = create_test_cache();
        let bytes = dump_cache(cache, Format::Protobuf).unwrap();
        let bytes = dump_cache(cache, DumpFormat::Protobuf).unwrap();
        let actual = ProtoParsedFlags::parse_from_bytes(&bytes).unwrap();

        assert_eq!(
+21 −11
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@

use anyhow::{anyhow, ensure, Result};
use clap::{builder::ArgAction, builder::EnumValueParser, Arg, ArgMatches, Command};
use core::any::Any;
use std::fs;
use std::io;
use std::io::Write;
@@ -30,7 +31,7 @@ mod commands;
mod protos;

use crate::cache::Cache;
use commands::{Input, OutputFile, Source};
use commands::{DumpFormat, Input, OutputFile, Source};

fn cli() -> Command {
    Command::new("aconfig")
@@ -53,13 +54,22 @@ fn cli() -> Command {
                .arg(
                    Arg::new("format")
                        .long("format")
                        .value_parser(EnumValueParser::<commands::Format>::new())
                        .value_parser(EnumValueParser::<commands::DumpFormat>::new())
                        .default_value("text"),
                )
                .arg(Arg::new("out").long("out").default_value("-")),
        )
}

fn get_required_arg<'a, T>(matches: &'a ArgMatches, arg_name: &str) -> Result<&'a T>
where
    T: Any + Clone + Send + Sync + 'static,
{
    matches
        .get_one::<T>(arg_name)
        .ok_or(anyhow!("internal error: required argument '{}' not found", arg_name))
}

fn open_zero_or_more_files(matches: &ArgMatches, arg_name: &str) -> Result<Vec<Input>> {
    let mut opened_files = vec![];
    for path in matches.get_many::<String>(arg_name).unwrap_or_default() {
@@ -89,30 +99,30 @@ fn main() -> Result<()> {
    let matches = cli().get_matches();
    match matches.subcommand() {
        Some(("create-cache", sub_matches)) => {
            let namespace = sub_matches.get_one::<String>("namespace").unwrap();
            let namespace = get_required_arg::<String>(sub_matches, "namespace")?;
            let declarations = open_zero_or_more_files(sub_matches, "declarations")?;
            let values = open_zero_or_more_files(sub_matches, "values")?;
            let cache = commands::create_cache(namespace, declarations, values)?;
            let path = sub_matches.get_one::<String>("cache").unwrap();
            let path = get_required_arg::<String>(sub_matches, "cache")?;
            let file = fs::File::create(path)?;
            cache.write_to_writer(file)?;
        }
        Some(("create-java-lib", sub_matches)) => {
            let path = sub_matches.get_one::<String>("cache").unwrap();
            let path = get_required_arg::<String>(sub_matches, "cache")?;
            let file = fs::File::open(path)?;
            let cache = Cache::read_from_reader(file)?;
            let dir = PathBuf::from(sub_matches.get_one::<String>("out").unwrap());
            let generated_file = commands::generate_code(&cache).unwrap();
            let dir = PathBuf::from(get_required_arg::<String>(sub_matches, "out")?);
            let generated_file = commands::generate_code(&cache)?;
            write_output_file_realtive_to_dir(&dir, &generated_file)?;
        }
        Some(("dump", sub_matches)) => {
            let path = sub_matches.get_one::<String>("cache").unwrap();
            let path = get_required_arg::<String>(sub_matches, "cache")?;
            let file = fs::File::open(path)?;
            let cache = Cache::read_from_reader(file)?;
            let format = sub_matches.get_one("format").unwrap();
            let format = get_required_arg::<DumpFormat>(sub_matches, "format")?;
            let output = commands::dump_cache(cache, *format)?;
            let path = sub_matches.get_one::<String>("out").unwrap();
            let mut file: Box<dyn Write> = if path == "-" {
            let path = get_required_arg::<String>(sub_matches, "out")?;
            let mut file: Box<dyn Write> = if *path == "-" {
                Box::new(io::stdout())
            } else {
                Box::new(fs::File::create(path)?)