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

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

aconfig: allow dots in package fields

Allow package fields to include dots.

Update the generated code based on the package name: if the package name
is com.android.example:

  - java: package com.android.example; ...
  - C++: namespace com::android::example { ... }
  - Rust: mod com { mod android { mod example { ... } } }

Also, update examples to use dots in the package fields.

Also, remove unnecessary #include from the auto-generated C++ code: the
header should not include itself.

Bug: 285000854
Test: atest aconfig.test
Change-Id: I8a5352e25c64c34dee0725202a1b7c9957819de8
parent 9fb58965
Loading
Loading
Loading
Loading
+4 −4
Original line number Diff line number Diff line
@@ -244,7 +244,7 @@ mod tests {
    #[test]
    fn test_package_try_from_text_proto() {
        let expected = FlagDeclarations {
            package: "ns".to_owned(),
            package: "com.example".to_owned(),
            flags: vec![
                FlagDeclaration { name: "a".to_owned(), description: "A".to_owned() },
                FlagDeclaration { name: "b".to_owned(), description: "B".to_owned() },
@@ -252,7 +252,7 @@ mod tests {
        };

        let s = r#"
        package: "ns"
        package: "com.example"
        flag {
            name: "a"
            description: "A"
@@ -270,14 +270,14 @@ mod tests {
    #[test]
    fn test_flag_declaration_try_from_text_proto_list() {
        let expected = FlagValue {
            package: "ns".to_owned(),
            package: "com.example".to_owned(),
            name: "1234".to_owned(),
            state: FlagState::Enabled,
            permission: Permission::ReadOnly,
        };

        let s = r#"
        package: "ns"
        package: "com.example"
        name: "1234"
        state: ENABLED
        permission: READ_ONLY
+14 −14
Original line number Diff line number Diff line
@@ -109,7 +109,7 @@ pub struct CacheBuilder {

impl CacheBuilder {
    pub fn new(package: String) -> Result<CacheBuilder> {
        ensure!(codegen::is_valid_identifier(&package), "bad package");
        ensure!(codegen::is_valid_package_ident(&package), "bad package");
        let cache = Cache { package, items: vec![] };
        Ok(CacheBuilder { cache })
    }
@@ -119,7 +119,7 @@ impl CacheBuilder {
        source: Source,
        declaration: FlagDeclaration,
    ) -> Result<&mut CacheBuilder> {
        ensure!(codegen::is_valid_identifier(&declaration.name), "bad flag name");
        ensure!(codegen::is_valid_name_ident(&declaration.name), "bad flag name");
        ensure!(!declaration.description.is_empty(), "empty flag description");
        ensure!(
            self.cache.items.iter().all(|item| item.name != declaration.name),
@@ -147,8 +147,8 @@ impl CacheBuilder {
        source: Source,
        value: FlagValue,
    ) -> Result<&mut CacheBuilder> {
        ensure!(codegen::is_valid_identifier(&value.package), "bad flag package");
        ensure!(codegen::is_valid_identifier(&value.name), "bad flag name");
        ensure!(codegen::is_valid_package_ident(&value.package), "bad flag package");
        ensure!(codegen::is_valid_name_ident(&value.name), "bad flag name");
        ensure!(
            value.package == self.cache.package,
            "failed to set values for flag {}/{} from {}: expected package {}",
@@ -182,7 +182,7 @@ mod tests {

    #[test]
    fn test_add_flag_declaration() {
        let mut builder = CacheBuilder::new("ns".to_string()).unwrap();
        let mut builder = CacheBuilder::new("com.example".to_string()).unwrap();
        builder
            .add_flag_declaration(
                Source::File("first.txt".to_string()),
@@ -217,12 +217,12 @@ mod tests {

    #[test]
    fn test_add_flag_value() {
        let mut builder = CacheBuilder::new("ns".to_string()).unwrap();
        let mut builder = CacheBuilder::new("com.example".to_string()).unwrap();
        let error = builder
            .add_flag_value(
                Source::Memory,
                FlagValue {
                    package: "ns".to_string(),
                    package: "com.example".to_string(),
                    name: "foo".to_string(),
                    state: FlagState::Enabled,
                    permission: Permission::ReadOnly,
@@ -231,7 +231,7 @@ mod tests {
            .unwrap_err();
        assert_eq!(
            &format!("{:?}", error),
            "failed to set values for flag ns/foo from <memory>: flag not declared"
            "failed to set values for flag com.example/foo from <memory>: flag not declared"
        );

        builder
@@ -245,7 +245,7 @@ mod tests {
            .add_flag_value(
                Source::Memory,
                FlagValue {
                    package: "ns".to_string(),
                    package: "com.example".to_string(),
                    name: "foo".to_string(),
                    state: FlagState::Disabled,
                    permission: Permission::ReadOnly,
@@ -257,7 +257,7 @@ mod tests {
            .add_flag_value(
                Source::Memory,
                FlagValue {
                    package: "ns".to_string(),
                    package: "com.example".to_string(),
                    name: "foo".to_string(),
                    state: FlagState::Enabled,
                    permission: Permission::ReadWrite,
@@ -277,7 +277,7 @@ mod tests {
                },
            )
            .unwrap_err();
        assert_eq!(&format!("{:?}", error), "failed to set values for flag some_other_package/foo from <memory>: expected package ns");
        assert_eq!(&format!("{:?}", error), "failed to set values for flag some_other_package/foo from <memory>: expected package com.example");

        let cache = builder.build();
        let item = cache.iter().find(|&item| item.name == "foo").unwrap();
@@ -292,7 +292,7 @@ mod tests {

    #[test]
    fn test_reject_empty_flag_declaration_fields() {
        let mut builder = CacheBuilder::new("ns".to_string()).unwrap();
        let mut builder = CacheBuilder::new("com.example".to_string()).unwrap();

        let error = builder
            .add_flag_declaration(
@@ -313,7 +313,7 @@ mod tests {

    #[test]
    fn test_reject_empty_flag_value_files() {
        let mut builder = CacheBuilder::new("ns".to_string()).unwrap();
        let mut builder = CacheBuilder::new("com.example".to_string()).unwrap();
        builder
            .add_flag_declaration(
                Source::Memory,
@@ -338,7 +338,7 @@ mod tests {
            .add_flag_value(
                Source::Memory,
                FlagValue {
                    package: "ns".to_string(),
                    package: "com.example".to_string(),
                    name: "".to_string(),
                    state: FlagState::Enabled,
                    permission: Permission::ReadOnly,
+30 −8
Original line number Diff line number Diff line
@@ -14,7 +14,7 @@
 * limitations under the License.
 */

pub fn is_valid_identifier(s: &str) -> bool {
pub fn is_valid_name_ident(s: &str) -> bool {
    // Identifiers must match [a-z][a-z0-9_]*
    let mut chars = s.chars();
    let Some(first) = chars.next() else {
@@ -26,18 +26,40 @@ pub fn is_valid_identifier(s: &str) -> bool {
    chars.all(|ch| ch.is_ascii_lowercase() || ch.is_ascii_digit() || ch == '_')
}

pub fn is_valid_package_ident(s: &str) -> bool {
    s.split('.').all(is_valid_name_ident)
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_is_valid_identifier() {
        assert!(is_valid_identifier("foo"));
        assert!(is_valid_identifier("foo_bar_123"));
    fn test_is_valid_name_ident() {
        assert!(is_valid_name_ident("foo"));
        assert!(is_valid_name_ident("foo_bar_123"));

        assert!(!is_valid_name_ident(""));
        assert!(!is_valid_name_ident("123_foo"));
        assert!(!is_valid_name_ident("foo-bar"));
        assert!(!is_valid_name_ident("foo-b\u{00e5}r"));
    }

    #[test]
    fn test_is_valid_package_ident() {
        assert!(is_valid_package_ident("foo"));
        assert!(is_valid_package_ident("foo_bar_123"));
        assert!(is_valid_package_ident("foo.bar"));
        assert!(is_valid_package_ident("foo.bar.a123"));

        assert!(!is_valid_identifier(""));
        assert!(!is_valid_identifier("123_foo"));
        assert!(!is_valid_identifier("foo-bar"));
        assert!(!is_valid_identifier("foo-b\u{00e5}r"));
        assert!(!is_valid_package_ident(""));
        assert!(!is_valid_package_ident("123_foo"));
        assert!(!is_valid_package_ident("foo-bar"));
        assert!(!is_valid_package_ident("foo-b\u{00e5}r"));
        assert!(!is_valid_package_ident("foo.bar.123"));
        assert!(!is_valid_package_ident(".foo.bar"));
        assert!(!is_valid_package_ident("foo.bar."));
        assert!(!is_valid_package_ident("."));
        assert!(!is_valid_package_ident("foo..bar"));
    }
}
+23 −18
Original line number Diff line number Diff line
@@ -14,28 +14,35 @@
 * limitations under the License.
 */

use anyhow::Result;
use anyhow::{ensure, Result};
use serde::Serialize;
use tinytemplate::TinyTemplate;

use crate::aconfig::{FlagState, Permission};
use crate::cache::{Cache, Item};
use crate::codegen;
use crate::commands::OutputFile;

pub fn generate_cpp_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 package = cache.package().to_lowercase();
    let context = Context { package: package.clone(), readwrite, class_elements };
    let package = cache.package().to_string();
    let header = package.replace('.', "_");
    let cpp_namespace = package.replace('.', "::");
    ensure!(codegen::is_valid_name_ident(&header));
    let context =
        Context { header: header.clone(), cpp_namespace, package, readwrite, class_elements };
    let mut template = TinyTemplate::new();
    template.add_template("cpp_code_gen", include_str!("../templates/cpp.template"))?;
    let contents = template.render("cpp_code_gen", &context)?;
    let path = ["aconfig", &(package + ".h")].iter().collect();
    let path = ["aconfig", &(header + ".h")].iter().collect();
    Ok(OutputFile { contents: contents.into(), path })
}

#[derive(Serialize)]
struct Context {
    pub header: String,
    pub cpp_namespace: String,
    pub package: String,
    pub readwrite: bool,
    pub class_elements: Vec<ClassElement>,
@@ -69,7 +76,7 @@ mod tests {

    #[test]
    fn test_cpp_codegen_build_time_flag_only() {
        let package = "my_package";
        let package = "com.example";
        let mut builder = CacheBuilder::new(package.to_string()).unwrap();
        builder
            .add_flag_declaration(
@@ -109,11 +116,10 @@ mod tests {
            )
            .unwrap();
        let cache = builder.build();
        let expect_content = r#"#ifndef my_package_HEADER_H
        #define my_package_HEADER_H
        #include "my_package.h"
        let expect_content = r#"#ifndef com_example_HEADER_H
        #define com_example_HEADER_H

        namespace my_package {
        namespace com::example {

            class my_flag_one {
                public:
@@ -133,7 +139,7 @@ mod tests {
        #endif
        "#;
        let file = generate_cpp_code(&cache).unwrap();
        assert_eq!("aconfig/my_package.h", file.path.to_str().unwrap());
        assert_eq!("aconfig/com_example.h", file.path.to_str().unwrap());
        assert_eq!(
            expect_content.replace(' ', ""),
            String::from_utf8(file.contents).unwrap().replace(' ', "")
@@ -142,7 +148,7 @@ mod tests {

    #[test]
    fn test_cpp_codegen_runtime_flag() {
        let package = "my_package";
        let package = "com.example";
        let mut builder = CacheBuilder::new(package.to_string()).unwrap();
        builder
            .add_flag_declaration(
@@ -172,20 +178,19 @@ mod tests {
            )
            .unwrap();
        let cache = builder.build();
        let expect_content = r#"#ifndef my_package_HEADER_H
        #define my_package_HEADER_H
        #include "my_package.h"
        let expect_content = r#"#ifndef com_example_HEADER_H
        #define com_example_HEADER_H

        #include <server_configurable_flags/get_flags.h>
        using namespace server_configurable_flags;

        namespace my_package {
        namespace com::example {

            class my_flag_one {
                public:
                    virtual const bool value() {
                        return GetServerConfigurableFlag(
                            "my_package",
                            "com.example",
                            "my_flag_one",
                            "false") == "true";
                    }
@@ -195,7 +200,7 @@ mod tests {
                public:
                    virtual const bool value() {
                        return GetServerConfigurableFlag(
                            "my_package",
                            "com.example",
                            "my_flag_two",
                            "true") == "true";
                    }
@@ -205,7 +210,7 @@ mod tests {
        #endif
        "#;
        let file = generate_cpp_code(&cache).unwrap();
        assert_eq!("aconfig/my_package.h", file.path.to_str().unwrap());
        assert_eq!("aconfig/com_example.h", file.path.to_str().unwrap());
        assert_eq!(
            expect_content.replace(' ', ""),
            String::from_utf8(file.contents).unwrap().replace(' ', "")
+5 −5
Original line number Diff line number Diff line
@@ -31,7 +31,7 @@ pub fn generate_java_code(cache: &Cache) -> Result<OutputFile> {
    let mut template = TinyTemplate::new();
    template.add_template("java_code_gen", include_str!("../templates/java.template"))?;
    let contents = template.render("java_code_gen", &context)?;
    let mut path: PathBuf = ["aconfig", package].iter().collect();
    let mut path: PathBuf = package.split('.').collect();
    // TODO: Allow customization of the java class name
    path.push("Flags.java");
    Ok(OutputFile { contents: contents.into(), path })
@@ -76,7 +76,7 @@ mod tests {

    #[test]
    fn test_generate_java_code() {
        let package = "example";
        let package = "com.example";
        let mut builder = CacheBuilder::new(package.to_string()).unwrap();
        builder
            .add_flag_declaration(
@@ -106,7 +106,7 @@ mod tests {
            )
            .unwrap();
        let cache = builder.build();
        let expect_content = r#"package aconfig.example;
        let expect_content = r#"package com.example;

        import android.provider.DeviceConfig;

@@ -118,7 +118,7 @@ mod tests {

            public static boolean test2() {
                return DeviceConfig.getBoolean(
                    "example",
                    "com.example",
                    "test2__test2",
                    false
                );
@@ -127,7 +127,7 @@ mod tests {
        }
        "#;
        let file = generate_java_code(&cache).unwrap();
        assert_eq!("aconfig/example/Flags.java", file.path.to_str().unwrap());
        assert_eq!("com/example/Flags.java", file.path.to_str().unwrap());
        assert_eq!(
            expect_content.replace(' ', ""),
            String::from_utf8(file.contents).unwrap().replace(' ', "")
Loading